diff --git a/crates/acp_thread/src/mention.rs b/crates/acp_thread/src/mention.rs index 67c1ddb9416..12827acc833 100644 --- a/crates/acp_thread/src/mention.rs +++ b/crates/acp_thread/src/mention.rs @@ -359,6 +359,7 @@ impl MentionUri { match self { MentionUri::Skill { name, source, .. } => { if source.is_empty() { + // Must match `SkillSource::display_label()` in agent_skills. format!("{} (global)", name) } else { format!("{} ({})", name, source) diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index eda50ab5637..ffe24590169 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -32,7 +32,7 @@ use acp_thread::{ use agent_client_protocol::schema as acp; use agent_skills::{ MAX_SKILL_DESCRIPTIONS_SIZE, Skill, SkillLoadError, SkillScopeId, SkillSource, SkillSummary, - global_skills_dir, load_skills_from_directory, project_skills_relative_path, + builtin_skills, global_skills_dir, load_skills_from_directory, project_skills_relative_path, }; use anyhow::{Context as _, Result, anyhow}; use chrono::{DateTime, Utc}; @@ -104,7 +104,7 @@ impl From<&Skill> for NativeAvailableSkill { Self { name: skill.name.clone(), description: skill.description.clone(), - source: skill.source.scope_prefix().to_string().into(), + source: skill.source.display_label().to_string().into(), skill_file_path: skill.skill_file_path.clone(), } } @@ -1644,14 +1644,18 @@ impl NativeAgent { // Read the body on demand here — bodies live on disk between // materializations to keep memory cost O(total frontmatter) // rather than O(total file size). - let body = agent_skills::read_skill_body(fs.as_ref(), &skill.skill_file_path) - .await - .with_context(|| { - format!( - "Failed to read skill body from {}", - skill.skill_file_path.display() - ) - })?; + let body = if let Some(embedded) = skill.embedded_body { + embedded.to_string() + } else { + agent_skills::read_skill_body(fs.as_ref(), &skill.skill_file_path) + .await + .with_context(|| { + format!( + "Failed to read skill body from {}", + skill.skill_file_path.display() + ) + })? + }; let envelope = crate::tools::render_skill_envelope(&skill, &body); let envelope_block = acp::ContentBlock::Text(acp::TextContent::new(envelope)); @@ -2245,9 +2249,12 @@ impl acp_thread::AgentConnection for NativeAgentConnection { // we don't clone the entire skill list on every prompt // (including prompts like `/help` that aren't skills at // all). The resolution rule matches the override-applied - // view: prefer a project-local with the matching name, - // falling back to a global, so the slash command picks the - // same entry the model sees in its catalog. + // view: among skills with the matching name, pick the one + // with the highest source precedence, so the slash command + // picks the same entry the model sees in its catalog. + // Ties (e.g. two project-local skills from different + // worktrees) resolve to the first in iteration order to + // match `apply_skill_overrides`. if parsed_command.explicit_server_id.is_none() && parsed_command.skill_scope.is_none() && !project_state.skills.is_empty() @@ -2256,15 +2263,13 @@ impl acp_thread::AgentConnection for NativeAgentConnection { let resolved = project_state .skills .iter() - .find(|skill| { - skill.name == prompt_name - && matches!(skill.source, SkillSource::ProjectLocal { .. }) - }) - .or_else(|| { - project_state - .skills - .iter() - .find(|skill| skill.name == prompt_name) + .filter(|skill| skill.name == prompt_name) + .reduce(|best, candidate| { + if candidate.source.precedence() > best.source.precedence() { + candidate + } else { + best + } }); if let Some(skill) = resolved { let skill = skill.clone(); @@ -2960,7 +2965,9 @@ fn combine_skills( global: Vec>, project: impl Iterator>, ) -> (Vec, Vec) { - let mut skills = Vec::new(); + // Built-in skills go first (lowest priority) so that global and + // project-local skills with the same name shadow them. + let mut skills = builtin_skills(); let mut errors = Vec::new(); for result in global.into_iter().chain(project) { match result { @@ -2979,17 +2986,16 @@ fn log_skill_conflicts(skills: &[Skill]) { let mut by_name: HashMap<&str, &Skill> = HashMap::default(); for skill in skills { match by_name.get(skill.name.as_str()) { - Some(existing) => match (&existing.source, &skill.source) { - (SkillSource::Global, SkillSource::ProjectLocal { .. }) => { + Some(existing) => { + if skill.source.precedence() > existing.source.precedence() { log::warn!( - "Project skill '{}' at '{}' overrides global skill at '{}' for the model; both appear in the slash-command popup with their source", + "Skill '{}' at '{}' overrides skill at '{}' for the model; both appear in the slash-command popup with their source", skill.name, skill.skill_file_path.display(), existing.skill_file_path.display(), ); by_name.insert(skill.name.as_str(), skill); - } - _ => { + } else { log::warn!( "Skill '{}' at '{}' conflicts with skill at '{}'; the model will see the first one, but both appear in the slash-command popup with their source", skill.name, @@ -2997,7 +3003,7 @@ fn log_skill_conflicts(skills: &[Skill]) { existing.skill_file_path.display(), ); } - }, + } None => { by_name.insert(skill.name.as_str(), skill); } @@ -3024,9 +3030,7 @@ fn apply_skill_overrides(skills: &[Skill]) -> Vec { for skill in skills { match indices.get(skill.name.as_str()).copied() { Some(idx) => { - if matches!(result[idx].source, SkillSource::Global) - && matches!(skill.source, SkillSource::ProjectLocal { .. }) - { + if skill.source.precedence() > result[idx].source.precedence() { result[idx] = skill.clone(); } } @@ -3064,6 +3068,7 @@ mod internal_tests { directory_path: PathBuf::from(format!("/home/user/.agents/skills/{name}")), skill_file_path: PathBuf::from(format!("/home/user/.agents/skills/{name}/SKILL.md")), disable_model_invocation: false, + embedded_body: None, } } @@ -3078,9 +3083,30 @@ mod internal_tests { directory_path: PathBuf::from(format!("/{worktree}/.agents/skills/{name}")), skill_file_path: PathBuf::from(format!("/{worktree}/.agents/skills/{name}/SKILL.md")), disable_model_invocation: false, + embedded_body: None, } } + fn make_builtin_skill(name: &str, description: &str) -> Skill { + Skill { + name: name.to_string(), + description: description.to_string(), + source: SkillSource::BuiltIn, + directory_path: PathBuf::from(format!("/builtin/{name}")), + skill_file_path: PathBuf::from(format!("/builtin/{name}/SKILL.md")), + disable_model_invocation: false, + embedded_body: Some("built-in body"), + } + } + + /// Filter to only user-defined (non-built-in) skills for test assertions. + fn user_skills(skills: &[Skill]) -> Vec<&Skill> { + skills + .iter() + .filter(|s| !matches!(s.source, SkillSource::BuiltIn)) + .collect() + } + #[test] fn test_combine_skills_keeps_every_entry_for_autocomplete() { // The autocomplete popup needs both same-named entries so the @@ -3092,9 +3118,10 @@ mod internal_tests { let (skills, errors) = combine_skills(vec![Ok(global)], vec![Ok(project)].into_iter()); assert!(errors.is_empty()); - assert_eq!(skills.len(), 2); - assert!(matches!(skills[0].source, SkillSource::Global)); - assert!(matches!(skills[1].source, SkillSource::ProjectLocal { .. })); + let user = user_skills(&skills); + assert_eq!(user.len(), 2); + assert!(matches!(user[0].source, SkillSource::Global)); + assert!(matches!(user[1].source, SkillSource::ProjectLocal { .. })); } #[test] @@ -3130,6 +3157,51 @@ mod internal_tests { assert_eq!(resolved[0].description, "First"); } + #[test] + fn test_apply_skill_overrides_global_wins_over_builtin() { + // A global skill with the same name as a built-in must shadow + // the built-in in the model-facing projection, regardless of + // iteration order. + let built_in = make_builtin_skill("create-skill", "Built-in version"); + let global = make_global_skill("create-skill", "User override"); + + let resolved = apply_skill_overrides(&[built_in, global]); + + assert_eq!(resolved.len(), 1); + assert_eq!(resolved[0].description, "User override"); + assert!(matches!(resolved[0].source, SkillSource::Global)); + } + + #[test] + fn test_apply_skill_overrides_project_wins_over_builtin() { + let built_in = make_builtin_skill("create-skill", "Built-in version"); + let project = make_project_skill("create-skill", "Project override", "my-project"); + + let resolved = apply_skill_overrides(&[built_in, project]); + + assert_eq!(resolved.len(), 1); + assert_eq!(resolved[0].description, "Project override"); + assert!(matches!( + resolved[0].source, + SkillSource::ProjectLocal { .. } + )); + } + + #[test] + fn test_apply_skill_overrides_project_wins_over_builtin_and_global() { + // All three sources present — the project-local must win and + // both lower-precedence entries must be dropped from the + // model-facing projection. + let built_in = make_builtin_skill("create-skill", "Built-in"); + let global = make_global_skill("create-skill", "Global"); + let project = make_project_skill("create-skill", "Project", "my-project"); + + let resolved = apply_skill_overrides(&[built_in, global, project]); + + assert_eq!(resolved.len(), 1); + assert_eq!(resolved[0].description, "Project"); + } + #[test] fn test_apply_skill_overrides_preserves_unique_skills() { let global_a = make_global_skill("alpha", "a"); @@ -3201,6 +3273,7 @@ mod internal_tests { directory_path: PathBuf::from(format!("/skills/{name}")), skill_file_path: PathBuf::from(format!("/skills/{name}/SKILL.md")), disable_model_invocation: false, + embedded_body: None, }); } @@ -3275,6 +3348,7 @@ mod internal_tests { directory_path: PathBuf::from("/skills/skill-01-first"), skill_file_path: PathBuf::from("/skills/skill-01-first/SKILL.md"), disable_model_invocation: false, + embedded_body: None, }; let second = Skill { name: "skill-02-overflows".to_string(), @@ -3283,6 +3357,7 @@ mod internal_tests { directory_path: PathBuf::from("/skills/skill-02-overflows"), skill_file_path: PathBuf::from("/skills/skill-02-overflows/SKILL.md"), disable_model_invocation: false, + embedded_body: None, }; let third = Skill { name: "skill-03-would-fit".to_string(), @@ -3291,6 +3366,7 @@ mod internal_tests { directory_path: PathBuf::from("/skills/skill-03-would-fit"), skill_file_path: PathBuf::from("/skills/skill-03-would-fit/SKILL.md"), disable_model_invocation: false, + embedded_body: None, }; // Sanity-check the test setup: the third skill is small enough @@ -3346,6 +3422,7 @@ mod internal_tests { directory_path: PathBuf::from("/skills/hidden-huge"), skill_file_path: PathBuf::from("/skills/hidden-huge/SKILL.md"), disable_model_invocation: true, + embedded_body: None, }; let visible = Skill { name: "visible".to_string(), @@ -3354,6 +3431,7 @@ mod internal_tests { directory_path: PathBuf::from("/skills/visible"), skill_file_path: PathBuf::from("/skills/visible/SKILL.md"), disable_model_invocation: false, + embedded_body: None, }; let (kept, errors) = select_catalog_skills(&[hidden, visible]); @@ -3496,9 +3574,10 @@ mod internal_tests { // The pre-existing skill should be loaded into the project state. agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project.entity_id()).unwrap(); - assert_eq!(state.skills.len(), 1); - assert_eq!(state.skills[0].name, "my-skill"); - assert_eq!(state.skills[0].description, "First version"); + let user = user_skills(&state.skills); + assert_eq!(user.len(), 1); + assert_eq!(user[0].name, "my-skill"); + assert_eq!(user[0].description, "First version"); }); // Modify the SKILL.md and verify the project context refreshes. @@ -3512,8 +3591,9 @@ mod internal_tests { agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project.entity_id()).unwrap(); - assert_eq!(state.skills.len(), 1); - assert_eq!(state.skills[0].description, "Second version"); + let user = user_skills(&state.skills); + assert_eq!(user.len(), 1); + assert_eq!(user[0].description, "Second version"); }); } @@ -3559,8 +3639,8 @@ mod internal_tests { agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project.entity_id()).unwrap(); assert!( - state.skills.is_empty(), - "expected no skills before the global skills dir exists, got {:?}", + user_skills(&state.skills).is_empty(), + "expected no user skills before the global skills dir exists, got {:?}", state.skills ); }); @@ -3585,9 +3665,10 @@ mod internal_tests { agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project.entity_id()).unwrap(); - assert_eq!(state.skills.len(), 1); - assert_eq!(state.skills[0].name, "late-skill"); - assert_eq!(state.skills[0].description, "Created after startup"); + let user = user_skills(&state.skills); + assert_eq!(user.len(), 1); + assert_eq!(user[0].name, "late-skill"); + assert_eq!(user[0].description, "Created after startup"); }); } @@ -3638,8 +3719,8 @@ mod internal_tests { agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project_id).unwrap(); assert!( - state.skills.is_empty(), - "expected no skills before the global skills dir exists, got {:?}", + user_skills(&state.skills).is_empty(), + "expected no user skills before the global skills dir exists, got {:?}", state.skills ); }); @@ -3656,7 +3737,12 @@ mod internal_tests { // empty list — NOT the snapshot that `Thread::new` would have // captured. cx.update(|cx| { - assert!(resolve(cx).is_empty()); + let all = resolve(cx); + let user: Vec<_> = all + .iter() + .filter(|s| !matches!(s.source, SkillSource::BuiltIn)) + .collect(); + assert!(user.is_empty()); }); // Now create a SKILL.md AFTER the session was registered. With @@ -3681,15 +3767,20 @@ mod internal_tests { // `state.skills` reflects the new skill (the watcher ran). agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project_id).unwrap(); - assert_eq!(state.skills.len(), 1); - assert_eq!(state.skills[0].name, "my-skill"); + let user = user_skills(&state.skills); + assert_eq!(user.len(), 1); + assert_eq!(user[0].name, "my-skill"); }); // The resolver the `SkillTool` uses must see it too. This is the // crux of the regression test: the tool's view of skills is // resolved at invocation time, not at thread-construction time. cx.update(|cx| { - let snapshot = resolve(cx); + let all = resolve(cx); + let snapshot: Vec<_> = all + .iter() + .filter(|s| !matches!(s.source, SkillSource::BuiltIn)) + .collect(); assert_eq!( snapshot.len(), 1, @@ -3777,7 +3868,11 @@ mod internal_tests { let parent_resolve = cx.update(|_cx| super::skills_resolver_for_project(agent.downgrade(), project_id)); cx.update(|cx| { - let parent_skills = parent_resolve(cx); + let all = parent_resolve(cx); + let parent_skills: Vec<_> = all + .iter() + .filter(|s| !matches!(s.source, SkillSource::BuiltIn)) + .collect(); assert_eq!(parent_skills.len(), 1); assert_eq!(parent_skills[0].name, "shared-skill"); }); @@ -3823,7 +3918,11 @@ mod internal_tests { let subagent_resolve = cx .update(|_cx| super::skills_resolver_for_project(agent.downgrade(), parent_project_id)); cx.update(|cx| { - let subagent_skills = subagent_resolve(cx); + let all = subagent_resolve(cx); + let subagent_skills: Vec<_> = all + .iter() + .filter(|s| !matches!(s.source, SkillSource::BuiltIn)) + .collect(); assert_eq!(subagent_skills.len(), 1); assert_eq!(subagent_skills[0].name, "shared-skill"); }); @@ -3919,7 +4018,14 @@ mod internal_tests { .iter() .map(|s| s.name.as_str()) .collect(); - assert_eq!(catalog, vec!["visible-skill"]); + assert!( + catalog.contains(&"visible-skill"), + "visible skill missing from catalog: {catalog:?}" + ); + assert!( + !catalog.contains(&"deploy"), + "deploy should be excluded from catalog: {catalog:?}" + ); }); } @@ -3986,7 +4092,7 @@ mod internal_tests { agent.read_with(cx, |agent, cx| { let state = agent.projects.get(&project_id).unwrap(); assert!( - state.skills.is_empty(), + user_skills(&state.skills).is_empty(), "untrusted worktree skills should not load: {:?}", state .skills @@ -4019,7 +4125,8 @@ mod internal_tests { agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project_id).unwrap(); - let names: Vec<&str> = state.skills.iter().map(|s| s.name.as_str()).collect(); + let user = user_skills(&state.skills); + let names: Vec<&str> = user.iter().map(|s| s.name.as_str()).collect(); assert_eq!(names, vec!["my-skill"]); }); diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 2fe5bc99303..ae5c5510764 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -364,11 +364,7 @@ impl UserMessage { .ok(); } MentionUri::Skill { name, source, .. } => { - let label = if source.is_empty() { - format!("{} (global)", name) - } else { - format!("{} ({})", name, source) - }; + let label = format!("{} ({})", name, source); write!(&mut skills_context, "\nSkill: {}\n{}\n", label, content).ok(); } } diff --git a/crates/agent/src/tools/skill_tool.rs b/crates/agent/src/tools/skill_tool.rs index d45633da505..978a24f6968 100644 --- a/crates/agent/src/tools/skill_tool.rs +++ b/crates/agent/src/tools/skill_tool.rs @@ -46,11 +46,12 @@ fn neutralize_envelope_tags(input: &str) -> String { /// frontmatter), not O(total file size). pub fn render_skill_envelope(skill: &Skill, body: &str) -> String { let source = match &skill.source { + agent_skills::SkillSource::BuiltIn => "built-in", agent_skills::SkillSource::Global => "global", agent_skills::SkillSource::ProjectLocal { .. } => "project-local", }; let worktree = match &skill.source { - agent_skills::SkillSource::Global => None, + agent_skills::SkillSource::BuiltIn | agent_skills::SkillSource::Global => None, agent_skills::SkillSource::ProjectLocal { worktree_root_name, .. } => Some(worktree_root_name.clone()), @@ -200,31 +201,33 @@ impl AgentTool for SkillTool { (skill.clone(), path_string) }; - // Read the body on demand. Bodies are not kept in memory - // between materializations — see `agent_skills::read_skill_body`. - let body = agent_skills::read_skill_body(self.fs.as_ref(), &skill.skill_file_path) - .await - .map_err(|e| SkillToolOutput::Error { - error: e.to_string(), - })?; + // For built-in skills the body is already in memory (compiled + // into the binary). For user skills, read on demand from disk. + let body = if let Some(embedded) = skill.embedded_body { + embedded.to_string() + } else { + agent_skills::read_skill_body(self.fs.as_ref(), &skill.skill_file_path) + .await + .map_err(|e| SkillToolOutput::Error { + error: e.to_string(), + })? + }; let rendered = render_skill_envelope(&skill, &body); - // Activations go through the standard tool-permission flow so - // they participate in the same Allow-Once / Always-Allow UX as - // every other built-in tool. The auth context value is the - // skill's absolute SKILL.md path so that "always allow this - // specific skill" is keyed to a specific file: editing the - // SKILL.md will change the path's content but not the path, - // so for content-change re-trust we'd want a hash too — but - // at minimum, two skills with the same name from different - // locations get independent trust grants. - let authorize = cx.update(|cx| { - let context = crate::ToolPermissionContext::new(Self::NAME, vec![skill_file_path]); - event_stream.authorize(self.initial_title(Ok(input), cx), context, cx) - }); - authorize.await.map_err(|e| SkillToolOutput::Error { - error: e.to_string(), - })?; + // Built-in skills ship with Zed and are trusted by default, + // so they skip the authorization prompt. User-installed skills + // go through the standard Allow-Once / Always-Allow UX. + let is_builtin = skill.source == agent_skills::SkillSource::BuiltIn; + if !is_builtin { + let authorize = cx.update(|cx| { + let context = + crate::ToolPermissionContext::new(Self::NAME, vec![skill_file_path]); + event_stream.authorize(self.initial_title(Ok(input), cx), context, cx) + }); + authorize.await.map_err(|e| SkillToolOutput::Error { + error: e.to_string(), + })?; + } Ok(SkillToolOutput::Found { rendered }) }) diff --git a/crates/agent_skills/agent_skills.rs b/crates/agent_skills/agent_skills.rs index 63c506bbf64..8f185ff4ad9 100644 --- a/crates/agent_skills/agent_skills.rs +++ b/crates/agent_skills/agent_skills.rs @@ -64,11 +64,19 @@ pub struct Skill { /// `skill` tool refuses to load it. The user can still invoke it as a /// slash command. pub disable_model_invocation: bool, + /// For built-in skills whose content is compiled into the binary, + /// this holds the full SKILL.md body so the skill tool can serve it + /// without a filesystem read. + pub embedded_body: Option<&'static str>, } /// Indicates where a skill was loaded from. #[derive(Debug, Clone, PartialEq, Eq)] pub enum SkillSource { + /// Compiled into the Zed binary. These are always available and have + /// the lowest override priority (global and project-local skills can + /// shadow them). + BuiltIn, /// From ~/.agents/skills/ Global, /// From {project}/.agents/skills/ @@ -79,6 +87,23 @@ pub enum SkillSource { } impl SkillSource { + /// Precedence for resolving same-named skills. Higher values shadow + /// lower ones: `ProjectLocal` > `Global` > `BuiltIn`. Two sources + /// returning equal precedence (e.g. two project-local skills from + /// different worktrees) leave the winner up to the caller, which by + /// convention keeps the first one in iteration order. + /// + /// Adding a new `SkillSource` variant should be a one-line change + /// here — every consumer routes through this method so the hierarchy + /// stays in sync. + pub fn precedence(&self) -> u8 { + match self { + Self::BuiltIn => 0, + Self::Global => 1, + Self::ProjectLocal { .. } => 2, + } + } + /// Scope prefix used in the `/:` slash-command /// syntax that the autocomplete popup inserts. Global skills use /// an empty prefix (so the inserted text is `/:`), and @@ -91,9 +116,21 @@ impl SkillSource { /// invoked as `/:`, and the worktree's skill is invoked as /// `/global:`. The two grammars never collide on the /// inserted text. + /// Human-readable label for this source, used in the UI to + /// distinguish skills from different origins. + pub fn display_label(&self) -> &str { + match self { + Self::BuiltIn => "built-in", + Self::Global => "global", + Self::ProjectLocal { + worktree_root_name, .. + } => worktree_root_name.as_ref(), + } + } + pub fn scope_prefix(&self) -> &str { match self { - Self::Global => "", + Self::BuiltIn | Self::Global => "", Self::ProjectLocal { worktree_root_name, .. } => worktree_root_name.as_ref(), @@ -112,7 +149,7 @@ impl SkillSource { /// strictness only affects users typing by memory. pub fn matches_scope(&self, scope: &str) -> bool { match self { - Self::Global => scope.is_empty(), + Self::BuiltIn | Self::Global => scope.is_empty(), Self::ProjectLocal { worktree_root_name, .. } => !scope.is_empty() && worktree_root_name.as_ref() == scope, @@ -211,6 +248,7 @@ pub fn parse_skill_frontmatter( directory_path, skill_file_path: skill_file_path.to_path_buf(), disable_model_invocation: metadata.disable_model_invocation, + embedded_body: None, }) } @@ -600,6 +638,53 @@ pub async fn read_skill_body( Ok(body.trim().to_string()) } +/// Content of the built-in `create-skill` SKILL.md, embedded at compile time. +const CREATE_SKILL_CONTENT: &str = include_str!("builtin/create-skill/SKILL.md"); + +/// Returns the set of skills that are compiled into the Zed binary. +pub fn builtin_skills() -> Vec { + let mut skills = Vec::new(); + if let Ok(skill) = parse_builtin_skill("create-skill", CREATE_SKILL_CONTENT) { + skills.push(skill); + } + skills +} + +/// Parse a built-in skill from its embedded SKILL.md content. The skill +/// gets a synthetic `` path since it doesn't live on disk. +fn parse_builtin_skill(name: &str, content: &'static str) -> Result { + let (metadata, body) = extract_frontmatter(content)?; + validate_name(&metadata.name)?; + validate_description(&metadata.description)?; + + let synthetic_dir = PathBuf::from(format!("/{}", name)); + let synthetic_path = synthetic_dir.join(SKILL_FILE_NAME); + + Ok(Skill { + name: metadata.name, + description: metadata.description, + source: SkillSource::BuiltIn, + directory_path: synthetic_dir, + skill_file_path: synthetic_path, + disable_model_invocation: metadata.disable_model_invocation, + embedded_body: Some(body.trim()), + }) +} + +/// All built-in skills as `(name, raw_content)` pairs. Used by +/// `builtin_skill_content` to serve the full SKILL.md without disk I/O. +const BUILTIN_SKILL_ENTRIES: &[(&str, &str)] = &[("create-skill", CREATE_SKILL_CONTENT)]; + +/// Look up the full embedded content of a built-in skill by its +/// synthetic file path. Returns `None` if the path doesn't match any +/// built-in skill. +pub fn builtin_skill_content(skill_file_path: &Path) -> Option<&'static str> { + BUILTIN_SKILL_ENTRIES.iter().find_map(|(name, content)| { + let expected = PathBuf::from(format!("/{}", name)).join(SKILL_FILE_NAME); + (expected == skill_file_path).then_some(*content) + }) +} + /// Returns the global skills directory: `~/.agents/skills`. /// /// Other agents (e.g. Claude Code) already write skill files into this @@ -663,6 +748,34 @@ mod tests { use fs::FakeFs; use gpui::TestAppContext; + #[test] + fn test_skill_source_precedence_is_total_and_ordered() { + // Pin the hierarchy: project-local > global > built-in. Every + // override and conflict-resolution site routes through this, + // so the rest of the codebase relies on it being correct. + let built_in = SkillSource::BuiltIn.precedence(); + let global = SkillSource::Global.precedence(); + let project = SkillSource::ProjectLocal { + worktree_id: SkillScopeId(1), + worktree_root_name: "my-project".into(), + } + .precedence(); + + assert!(built_in < global, "global must shadow built-in"); + assert!(global < project, "project-local must shadow global"); + + // Two project-local skills from different worktrees tie. The + // "first wins" convention is enforced by the callers, but the + // precedence itself must be equal so neither silently shadows + // the other. + let other_project = SkillSource::ProjectLocal { + worktree_id: SkillScopeId(2), + worktree_root_name: "other-project".into(), + } + .precedence(); + assert_eq!(project, other_project); + } + #[test] fn test_parse_valid_skill() { let content = r#"--- @@ -1532,6 +1645,7 @@ description: A skill with no body content directory_path: PathBuf::from("/skills/test-skill"), skill_file_path: PathBuf::from("/skills/test-skill/SKILL.md"), disable_model_invocation: false, + embedded_body: None, }; let summary = SkillSummary::from(&skill); diff --git a/crates/agent_skills/builtin/create-skill/SKILL.md b/crates/agent_skills/builtin/create-skill/SKILL.md new file mode 100644 index 00000000000..c5c76d80b8f --- /dev/null +++ b/crates/agent_skills/builtin/create-skill/SKILL.md @@ -0,0 +1,95 @@ +--- +name: create-skill +description: Helps users create new agent skills for Zed. Use this when a user wants to create a skill, asks about SKILL.md structure, or wants to package reusable agent instructions. +--- + +# Creating a Zed Agent Skill + +Use this skill when the user wants to create, edit, or understand agent skills in Zed. + +## What is a Skill? + +A skill is a reusable set of instructions that an agent can load on demand. Each skill lives in its own directory and is defined by a `SKILL.md` file with YAML frontmatter. + +## Where Skills Live + +Skills can be placed in two locations: + +| Scope | Path | When to use | +|-------|------|-------------| +| Global | `~/.agents/skills//SKILL.md` | Personal skills, available in all projects | +| Project-local | `/.agents/skills//SKILL.md` | Project-specific skills, shared with collaborators through version control | + +Prefer project-local when the skill is specific to a repository. Prefer global when the skill is a personal workflow the user wants everywhere. + +## SKILL.md Format + +Every `SKILL.md` must start with YAML frontmatter between `---` delimiters: + +```markdown +--- +name: my-skill-name +description: A clear, specific description of what this skill does and when to use it. +--- + +# Skill Title + +Instructions for the agent go here. Write them as if you're telling the agent +what to do when this skill is activated. +``` + +### Required Frontmatter Fields + +- **`name`** (required): Must be 1–64 characters, lowercase alphanumeric with single-hyphen separators. Must match the containing directory name exactly. Regex: `^[a-z0-9]+(-[a-z0-9]+)*$` +- **`description`** (required): Must be 1–1024 characters. This is what the agent sees when deciding whether to use the skill — make it specific and actionable. + +### Optional Frontmatter Fields + +- **`disable-model-invocation`**: When set to `true`, the skill is hidden from the agent's automatic catalog. The user can still invoke it manually via the `/` slash command menu. Useful for skills that should only run when explicitly requested. + +## Naming Rules + +The skill name must: +- Be lowercase letters and numbers only, with single hyphens as separators +- Not start or end with `-` +- Not contain consecutive `--` +- Match the directory name that contains the `SKILL.md` + +Good: `git-release`, `pr-review`, `rust-patterns` +Bad: `Git-Release`, `pr--review`, `-my-skill`, `my_skill` + +## Writing Good Skill Instructions + +The body of the SKILL.md (after the frontmatter) contains the instructions the agent will follow. Guidelines: + +1. **Be direct**: Write instructions as if talking to the agent. "Do X", "Check Y", "Ask the user about Z". +2. **Be specific**: Include concrete file paths, commands, formats, and patterns. +3. **Include when-to-use guidance**: Help the agent understand the right context for this skill. +4. **Reference supporting files**: Skills can include additional files in their directory. Reference them with relative paths (e.g., `templates/component.tsx`). The agent can read these files when the skill is activated. +5. **Keep descriptions actionable**: The `description` field is the agent's primary signal for whether to load this skill. "Helps with code" is too vague. "Generate React components following the project's design system patterns" is specific. + +## Supporting Files + +A skill directory can contain additional files beyond `SKILL.md`: + +``` +~/.agents/skills/react-component/ +├── SKILL.md +├── templates/ +│ ├── component.tsx +│ └── test.tsx +└── examples/ + └── button.tsx +``` + +Reference these in the skill body. The agent can read them using the file path shown in the `` tag of the skill envelope. + +## Step-by-Step: Creating a Skill + +1. Decide on scope (global vs project-local) based on the user's needs. +2. Choose a descriptive, hyphenated name. +3. Create the directory structure. +4. Write the `SKILL.md` with frontmatter and instructions. +5. Optionally add supporting files (templates, examples, references). + +After creating the skill, it will be automatically discovered by Zed's agent on the next conversation (no restart needed for global skills if the `~/.agents/skills/` directory already exists). diff --git a/crates/agent_ui/src/mention_set.rs b/crates/agent_ui/src/mention_set.rs index 31bb31c046c..d1335d31811 100644 --- a/crates/agent_ui/src/mention_set.rs +++ b/crates/agent_ui/src/mention_set.rs @@ -491,6 +491,14 @@ impl MentionSet { skill_file_path: PathBuf, cx: &mut Context, ) -> Task> { + // Built-in skills have synthetic paths that don't exist on disk; + // serve their content directly from the compiled-in data. + if let Some(content) = agent_skills::builtin_skill_content(&skill_file_path) { + return Task::ready(Ok(Mention::Text { + content: content.to_string(), + tracked_buffers: Vec::new(), + })); + } cx.background_spawn(async move { let content = std::fs::read_to_string(&skill_file_path).map_err(|e| { anyhow!( diff --git a/crates/agent_ui/src/ui/mention_crease.rs b/crates/agent_ui/src/ui/mention_crease.rs index 4d4b282cf0b..f71ecedae21 100644 --- a/crates/agent_ui/src/ui/mention_crease.rs +++ b/crates/agent_ui/src/ui/mention_crease.rs @@ -203,6 +203,43 @@ fn open_skill_file( window: &mut Window, cx: &mut Context, ) { + // Built-in skills have synthetic paths that don't exist on disk. + // Open a read-only buffer with the embedded content instead. + if let Some(content) = agent_skills::builtin_skill_content(&skill_file_path) { + let project = workspace.project().clone(); + let languages = project.read(cx).languages().clone(); + let buffer = project.update(cx, |project, cx| { + project.create_local_buffer(content, None, false, cx) + }); + // Set markdown highlighting asynchronously — the buffer + // opens instantly and the highlighting appears once loaded. + cx.spawn({ + let buffer = buffer.clone(); + async move |_, cx| { + if let Ok(markdown) = languages.language_for_name("Markdown").await { + buffer.update(cx, |buffer, cx| buffer.set_language(Some(markdown), cx)); + } + } + }) + .detach(); + let editor = cx.new(|cx| { + let mut editor = Editor::for_buffer(buffer, None, window, cx); + editor.set_read_only(true); + let title = skill_file_path + .parent() + .and_then(|p| p.file_name()) + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| "built-in skill".into()); + editor + .buffer() + .update(cx, |buffer, cx| buffer.set_title(title, cx)); + editor + }); + let pane = workspace.active_pane().clone(); + workspace.add_item(pane, Box::new(editor), None, true, true, window, cx); + return; + } + workspace .open_abs_path( skill_file_path, diff --git a/crates/prompt_store/src/prompts.rs b/crates/prompt_store/src/prompts.rs index b3194dd1d61..6417f49f85b 100644 --- a/crates/prompt_store/src/prompts.rs +++ b/crates/prompt_store/src/prompts.rs @@ -163,6 +163,7 @@ mod tests { directory_path: PathBuf::from("/skills/oversized"), skill_file_path: PathBuf::from("/skills/oversized/SKILL.md"), disable_model_invocation: false, + embedded_body: None, }; let summary = SkillSummary::from(&skill);