From 2b8d8e8d401f89cd4aec0dc492c9fa75298004c3 Mon Sep 17 00:00:00 2001 From: Alexis Dumas Date: Thu, 26 Feb 2026 15:36:15 -0500 Subject: [PATCH] add agent skills --- Cargo.lock | 27 + Cargo.toml | 1 + crates/agent/Cargo.toml | 2 + crates/agent/src/agent.rs | 2 + crates/agent/src/edit_agent/evals.rs | 1 + crates/agent/src/skills.rs | 782 ++++++++++++++++++ crates/agent/src/templates.rs | 50 ++ crates/agent/src/templates/skills_prompt.hbs | 39 + crates/agent/src/templates/system_prompt.hbs | 4 + crates/agent/src/tests/mod.rs | 42 + crates/agent/src/thread.rs | 23 +- crates/agent/src/tools/list_directory_tool.rs | 25 + crates/agent/src/tools/read_file_tool.rs | 36 + crates/agent_ui/src/agent_panel.rs | 1 + 14 files changed, 1032 insertions(+), 3 deletions(-) create mode 100644 crates/agent/src/skills.rs create mode 100644 crates/agent/src/templates/skills_prompt.hbs diff --git a/Cargo.lock b/Cargo.lock index 906c5e65456c604e5123bfde9ac1c39e261eedfd..4d4bf14c79b499896d711e3dbf3819fc2698ba21 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,6 +148,7 @@ dependencies = [ "agent_servers", "agent_settings", "anyhow", + "base64 0.22.1", "chrono", "client", "clock", @@ -191,6 +192,7 @@ dependencies = [ "schemars", "serde", "serde_json", + "serde_yml", "settings", "shell_command_parser", "smallvec", @@ -9693,6 +9695,16 @@ dependencies = [ "webrtc-sys", ] +[[package]] +name = "libyml" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3302702afa434ffa30847a83305f0a69d6abd74293b6554c18ec85c7ef30c980" +dependencies = [ + "anyhow", + "version_check", +] + [[package]] name = "libz-sys" version = "1.1.22" @@ -15590,6 +15602,21 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serde_yml" +version = "0.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59e2dd588bf1597a252c3b920e0143eb99b0f76e4e082f4c92ce34fbc9e71ddd" +dependencies = [ + "indexmap", + "itoa", + "libyml", + "memchr", + "ryu", + "serde", + "version_check", +] + [[package]] name = "serial2" version = "0.2.33" diff --git a/Cargo.toml b/Cargo.toml index 81bbb1176ddddcc117fc9082586cbc08dbb95d61..5733dd7eea1bfc822b8d4869b9924bfcf5932e51 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -697,6 +697,7 @@ serde_json_lenient = { version = "0.2", features = [ ] } serde_path_to_error = "0.1.17" serde_urlencoded = "0.7" +serde_yml = "0.0.12" sha2 = "0.10" shellexpand = "2.1.0" shlex = "1.3.0" diff --git a/crates/agent/Cargo.toml b/crates/agent/Cargo.toml index a5a4c2742a444bf2e8b0a12b0bb233c6e51684f2..7ec86e158c3056a4d368165c11f13ef7e3cd8ad9 100644 --- a/crates/agent/Cargo.toml +++ b/crates/agent/Cargo.toml @@ -17,6 +17,7 @@ e2e = [] workspace = true [dependencies] +base64.workspace = true acp_thread.workspace = true action_log.workspace = true agent-client-protocol.workspace = true @@ -56,6 +57,7 @@ rust-embed.workspace = true schemars.workspace = true serde.workspace = true serde_json.workspace = true +serde_yml.workspace = true settings.workspace = true shell_command_parser.workspace = true smallvec.workspace = true diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index b7aa9d1e311016f572928993e049798c2b5e3bb2..0bb1910b5a540b120b4bbc98a009d19d7047adef 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -4,6 +4,7 @@ mod legacy_thread; mod native_agent_server; pub mod outline; mod pattern_extraction; +mod skills; mod templates; #[cfg(test)] mod tests; @@ -18,6 +19,7 @@ use itertools::Itertools; pub use native_agent_server::NativeAgentServer; pub use pattern_extraction::*; pub use shell_command_parser::extract_commands; +pub use skills::*; pub use templates::*; pub use thread::*; pub use thread_store::*; diff --git a/crates/agent/src/edit_agent/evals.rs b/crates/agent/src/edit_agent/evals.rs index ba8b7ed867ea26bcdcdee7f8bf20390c2f9592b3..2de2c76c826ecf139e19f64bd543023c44ad62bc 100644 --- a/crates/agent/src/edit_agent/evals.rs +++ b/crates/agent/src/edit_agent/evals.rs @@ -1534,6 +1534,7 @@ impl EditAgentTest { let template = crate::SystemPromptTemplate { project: &project_context, available_tools: tool_names, + available_skills: String::new(), model_name: None, }; let templates = Templates::new(); diff --git a/crates/agent/src/skills.rs b/crates/agent/src/skills.rs new file mode 100644 index 0000000000000000000000000000000000000000..fb7ad901e48c90d90cc3b4886fd06370cfff7c21 --- /dev/null +++ b/crates/agent/src/skills.rs @@ -0,0 +1,782 @@ +//! Agent Skills discovery and formatting. +//! +//! This module discovers user-defined skills from global and worktree locations +//! and formats them for display in the agent's system prompt. + +use crate::{SkillContext, SkillsPromptTemplate, Template, Templates}; +use anyhow::{Result, anyhow}; +use collections::HashMap; +use gpui::{App, AppContext, Context, Entity}; +use serde::Deserialize; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; + +/// A minimal representation of a discovered skill for formatting. +#[derive(Clone, Debug)] +pub struct Skill { + name: String, + description: String, + path: PathBuf, +} + +/// Metadata extracted from a skill's YAML frontmatter. +#[derive(Deserialize, Debug)] +struct SkillMetadata { + name: String, + description: String, + #[allow(dead_code)] + license: Option, + compatibility: Option, + #[serde(default)] + #[allow(dead_code)] + metadata: HashMap, + #[allow(dead_code)] + allowed_tools: Option, +} + +impl SkillMetadata { + /// Validates that the skill metadata conforms to the Agent Skills specification. + fn validate(&self, expected_dir_name: &str) -> Result<()> { + if self.name != expected_dir_name { + return Err(anyhow!( + "skill name '{}' doesn't match directory name '{}'", + self.name, + expected_dir_name + )); + } + + if self.name.is_empty() { + return Err(anyhow!("skill name cannot be empty")); + } + + if self.name.len() > 64 { + return Err(anyhow!("skill name cannot exceed 64 characters")); + } + + if !self + .name + .chars() + .all(|c| c.is_lowercase() || c.is_numeric() || c == '-') + { + return Err(anyhow!( + "skill name must be lowercase alphanumeric + hyphens only: {}", + self.name + )); + } + + if self.name.contains("--") { + return Err(anyhow!( + "skill name must not contain consecutive hyphens: {}", + self.name + )); + } + + if self.name.starts_with('-') || self.name.ends_with('-') { + return Err(anyhow!( + "skill name must not start or end with hyphen: {}", + self.name + )); + } + + if self.description.is_empty() { + return Err(anyhow!("skill description cannot be empty")); + } + + if self.description.len() > 1024 { + return Err(anyhow!("skill description cannot exceed 1024 characters")); + } + + if let Some(ref compatibility) = self.compatibility { + if compatibility.len() > 500 { + return Err(anyhow!( + "skill compatibility exceeds 500 characters: {}", + compatibility.len() + )); + } + } + + Ok(()) + } +} + +/// Parses YAML frontmatter from a markdown file. +/// Returns the parsed metadata and the markdown body. +fn parse_skill_file(content: &str, expected_dir_name: &str) -> Result<(SkillMetadata, String)> { + let content = content.trim_start(); + + if !content.starts_with("---") { + return Err(anyhow!("SKILL.md must start with YAML frontmatter (---)")); + } + + let end_marker = content[3..].find("\n---"); + let (yaml_part, body) = match end_marker { + Some(end) => { + let yaml_end = 3 + end; + let yaml = content[3..yaml_end].trim().to_string(); + let body_start = yaml_end + 3; + let body = content[body_start..].trim_start().to_string(); + (yaml, body) + } + None => return Err(anyhow!("YAML frontmatter not properly closed with ---")), + }; + + let metadata: SkillMetadata = serde_yml::from_str(&yaml_part) + .map_err(|e| anyhow!("failed to parse YAML frontmatter: {}", e))?; + + metadata.validate(expected_dir_name)?; + + Ok((metadata, body)) +} + +/// Discovers all skills in the given directory. +/// Returns a map of skill name to Skill. +fn discover_skills_sync(skills_dir: &Path) -> HashMap> { + let mut skills = HashMap::default(); + + if !skills_dir.exists() || !skills_dir.is_dir() { + return skills; + } + + let entries = match std::fs::read_dir(skills_dir) { + Ok(entries) => entries, + Err(e) => { + log::warn!("failed to read skills directory: {}", e); + return skills; + } + }; + + for entry in entries.flatten() { + let path = entry.path(); + + if !path.is_dir() { + continue; + } + + let skill_file = path.join("SKILL.md"); + if !skill_file.exists() { + continue; + } + + let dir_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + + let content = match std::fs::read_to_string(&skill_file) { + Ok(content) => content, + Err(e) => { + log::warn!("failed to read {:?}: {}", skill_file, e); + continue; + } + }; + + let (metadata, _body) = match parse_skill_file(&content, dir_name) { + Ok(result) => result, + Err(e) => { + log::warn!("failed to parse {:?}: {}", skill_file, e); + continue; + } + }; + + let skill = Arc::new(Skill { + name: metadata.name, + description: metadata.description, + path, + }); + + skills.insert(skill.name.clone(), skill); + } + + skills +} + +/// Returns the canonicalized global skills directory path (~/.config/zed/skills). +/// Result is cached after first call. If canonicalization fails, returns the original path. +pub fn global_skills_dir() -> PathBuf { + paths::config_dir().join("skills") +} + +/// Discovers skills from both global and worktree locations. +/// Worktree skills take precedence over global skills with the same name. +pub fn discover_all_skills_sync(worktree_roots: &[PathBuf]) -> HashMap> { + let mut all_skills = discover_skills_sync(&global_skills_dir()); + + for worktree in worktree_roots { + let worktree_skills = discover_skills_sync(&worktree.join(".agents").join("skills")); + for (name, skill) in worktree_skills { + all_skills.insert(name, skill); + } + } + + all_skills +} + +/// Format skills for display in the system prompt using handlebars templating. +pub fn format_skills_for_prompt( + skills: &HashMap>, + templates: Arc, +) -> String { + let mut skill_list: Vec<_> = skills.values().collect(); + skill_list.sort_by(|a, b| a.name.cmp(&b.name)); + + let skill_contexts: Vec = skill_list + .into_iter() + .map(|skill| SkillContext { + name: skill.name.clone(), + description: if skill.description.len() > 1024 { + format!("{}...", &skill.description[..1021]) + } else { + skill.description.clone() + }, + path: skill.path.display().to_string(), + }) + .collect(); + + let template = SkillsPromptTemplate { + has_skills: !skill_contexts.is_empty(), + skills: skill_contexts, + }; + + template.render(&templates).unwrap_or_default() +} + +/// Context entity that holds formatted skills for the system prompt. +/// Populates itself asynchronously on creation. +pub struct SkillsContext { + formatted_skills: Option, +} + +impl SkillsContext { + /// Create a new SkillsContext and spawn background task to populate it. + pub fn new( + worktree_roots: Vec, + templates: Arc, + cx: &mut App, + ) -> Entity { + cx.new(|cx: &mut Context| { + // Spawn async task that will populate the skills + cx.spawn(async move |this, cx| { + let formatted = cx + .background_spawn(async move { + let skills = discover_all_skills_sync(&worktree_roots); + format_skills_for_prompt(&skills, templates) + }) + .await; + + this.update(cx, |this, _cx| { + this.formatted_skills = Some(formatted); + }) + .ok(); + }) + .detach(); + + Self { + formatted_skills: None, + } + }) + } + + /// Create a SkillsContext with pre-populated skills (for loading from DB). + pub fn from_formatted(formatted_skills: String, cx: &mut App) -> Entity { + cx.new(|_cx| Self { + formatted_skills: Some(formatted_skills), + }) + } + + /// Get the formatted skills string. + /// Returns empty string if not yet loaded. + pub fn formatted(&self) -> &str { + self.formatted_skills.as_deref().unwrap_or("") + } + + /// Check if skills have been loaded. + pub fn is_loaded(&self) -> bool { + self.formatted_skills.is_some() + } +} + +/// Checks if a path is within a skills directory (global or worktree-specific). +/// +/// Expands `~` to home directory, canonicalizes the path, and checks if it's within: +/// - The global skills directory (~/.config/zed/skills) +/// - Any worktree's .agents/skills directory +/// +/// Returns Some(canonical_path) if the path is within a skills directory. +/// Returns None if the path is not within any skills directory. +/// Check if a canonicalized path is within any skills directory. +/// This is the pure logic version that operates on already-canonicalized paths. +pub fn is_skills_path_canonical( + canonical_input: &Path, + worktree_roots: &[PathBuf], +) -> Option { + let global_skills_root = global_skills_dir(); + if canonical_input.starts_with(&global_skills_root) { + return Some(canonical_input.to_path_buf()); + } + + for worktree_root in worktree_roots { + let worktree_skills_path = worktree_root.join(".agents").join("skills"); + if canonical_input.starts_with(&worktree_skills_path) { + return Some(canonical_input.to_path_buf()); + } + } + + None +} + +/// Check if a path is within any skills directory. +/// Handles ~ expansion and canonicalization. +pub fn is_skills_path(input_path: &str, worktree_roots: &[PathBuf]) -> Option { + let path = if input_path.starts_with('~') { + let home = paths::home_dir().to_string_lossy().into_owned(); + PathBuf::from(input_path.replacen('~', &home, 1)) + } else { + PathBuf::from(input_path) + }; + + let canonical_input = std::fs::canonicalize(&path).ok()?; + + is_skills_path_canonical(&canonical_input, worktree_roots) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_validate_valid_metadata() { + let metadata = SkillMetadata { + name: "pdf-processing".to_string(), + description: "Extract text and tables from PDF files".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("pdf-processing").is_ok()); + } + + #[test] + fn test_validate_name_too_long() { + let metadata = SkillMetadata { + name: "a".repeat(65), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("toolongname").is_err()); + } + + #[test] + fn test_validate_name_empty() { + let metadata = SkillMetadata { + name: String::new(), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("").is_err()); + } + + #[test] + fn test_validate_name_invalid_chars() { + let metadata = SkillMetadata { + name: "invalid@name".to_string(), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("invalid@name").is_err()); + } + + #[test] + fn test_validate_name_starts_with_hyphen() { + let metadata = SkillMetadata { + name: "-invalid".to_string(), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("-invalid").is_err()); + } + + #[test] + fn test_validate_name_ends_with_hyphen() { + let metadata = SkillMetadata { + name: "invalid-".to_string(), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("invalid-").is_err()); + } + + #[test] + fn test_validate_name_consecutive_hyphens() { + let metadata = SkillMetadata { + name: "in--valid".to_string(), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("in--valid").is_err()); // Consecutive hyphens are allowed + } + + #[test] + fn test_validate_description_empty() { + let metadata = SkillMetadata { + name: "valid-name".to_string(), + description: String::new(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("valid-name").is_err()); + } + + #[test] + fn test_validate_description_too_long() { + let metadata = SkillMetadata { + name: "valid-name".to_string(), + description: "a".repeat(1025), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("valid-name").is_err()); + } + + #[test] + fn test_validate_compatibility_too_long() { + let metadata = SkillMetadata { + name: "valid-name".to_string(), + description: "Valid description".to_string(), + license: None, + compatibility: Some("a".repeat(501)), + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("valid-name").is_err()); + } + + #[test] + fn test_validate_name_mismatch() { + let mut metadata = SkillMetadata { + name: "bar".to_string(), + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + assert!(metadata.validate("foo").is_err()); + } + + #[test] + fn test_parse_valid_frontmatter() { + let content = r#"--- +name: test-skill +description: A test skill +--- +# Skill Content + +This is the skill content."#; + + let (metadata, body) = parse_skill_file(content, "test-skill").unwrap(); + assert_eq!(metadata.name, "test-skill"); + assert_eq!(metadata.description, "A test skill"); + assert!(body.contains("Skill Content")); + } + + #[test] + fn test_parse_no_frontmatter() { + let content = "# Just markdown content"; + assert!(parse_skill_file(content, "test").is_err()); + } + + #[test] + fn test_parse_unclosed_frontmatter() { + let content = "---\nname: test\n# No closing"; + assert!(parse_skill_file(content, "test").is_err()); + } + + #[test] + fn test_parse_invalid_yaml() { + let content = "---\ninvalid yaml\n---\ncontent"; + assert!(parse_skill_file(content, "test").is_err()); + } + + #[test] + fn test_format_skills_sorts_alphabetically() { + let mut skills = HashMap::default(); + skills.insert( + "z-skill".to_string(), + Arc::new(Skill { + name: "z-skill".to_string(), + description: "Z skill desc".to_string(), + path: PathBuf::from("/z"), + }), + ); + skills.insert( + "a-skill".to_string(), + Arc::new(Skill { + name: "a-skill".to_string(), + description: "A skill desc".to_string(), + path: PathBuf::from("/a"), + }), + ); + skills.insert( + "m-skill".to_string(), + Arc::new(Skill { + name: "m-skill".to_string(), + description: "M skill desc".to_string(), + path: PathBuf::from("/m"), + }), + ); + + let result = format_skills_for_prompt(&skills, Templates::new()); + + // Verify all skills are present + assert!(result.contains("a-skill")); + assert!(result.contains("m-skill")); + assert!(result.contains("z-skill")); + + // Verify alphabetical order: a-skill should appear before m-skill, which should appear before z-skill + let a_pos = result.find("a-skill").unwrap(); + let m_pos = result.find("m-skill").unwrap(); + let z_pos = result.find("z-skill").unwrap(); + assert!(a_pos < m_pos, "a-skill should appear before m-skill"); + assert!(m_pos < z_pos, "m-skill should appear before z-skill"); + } + + #[test] + fn test_format_skills_truncates_long_description() { + let mut skills = HashMap::default(); + let long_description = "a".repeat(1500); + + skills.insert( + "long-desc-skill".to_string(), + Arc::new(Skill { + name: "long-desc-skill".to_string(), + description: long_description.clone(), + path: PathBuf::from("/long"), + }), + ); + + let result = format_skills_for_prompt(&skills, Templates::new()); + + // The description should be truncated with "..." + assert!(result.contains("...")); + // The full description should NOT be present + assert!(!result.contains(&long_description)); + // The skill name should still be present + assert!(result.contains("long-desc-skill")); + } + + #[test] + fn test_format_skills_preserves_short_description() { + let mut skills = HashMap::default(); + + skills.insert( + "short-desc-skill".to_string(), + Arc::new(Skill { + name: "short-desc-skill".to_string(), + description: "Short description".to_string(), + path: PathBuf::from("/short"), + }), + ); + + let result = format_skills_for_prompt(&skills, Templates::new()); + + // Short descriptions should NOT be truncated (no "..." appended) + assert!(!result.contains("Short description...")); + assert!(result.contains("Short description")); + } + + #[test] + fn test_format_skills_includes_all_fields() { + let mut skills = HashMap::default(); + + skills.insert( + "test-skill".to_string(), + Arc::new(Skill { + name: "test-skill".to_string(), + description: "Test description".to_string(), + path: PathBuf::from("/path/to/skill"), + }), + ); + + let result = format_skills_for_prompt(&skills, Templates::new()); + + // All fields should appear in the output + assert!(result.contains("test-skill")); + assert!(result.contains("Test description")); + assert!(result.contains("/path/to/skill")); + } + + #[test] + fn test_format_skills_exactly_1024_char_description() { + let mut skills = HashMap::default(); + // Exactly 1024 characters should NOT be truncated + let exact_description = "b".repeat(1024); + + skills.insert( + "exact-skill".to_string(), + Arc::new(Skill { + name: "exact-skill".to_string(), + description: exact_description.clone(), + path: PathBuf::from("/exact"), + }), + ); + + let result = format_skills_for_prompt(&skills, Templates::new()); + + // Should NOT contain "..." since it's exactly 1024 chars + assert!(!result.contains("...")); + } + + #[test] + fn test_format_skills_empty() { + let skills = HashMap::default(); + let result = format_skills_for_prompt(&skills, Templates::new()); + // With no skills, template renders to empty string + assert!(result.is_empty()); + } + + #[test] + fn test_is_skills_path_canonical_global_directory() { + let worktree_roots: Vec = vec![]; + + let path = PathBuf::from("/home/user/.config/zed/skills/test.md"); + let result = is_skills_path_canonical(&path, &worktree_roots); + + // This will return Some if the actual global_skills_dir() matches, + // but since we don't know the user's home directory in tests, + // this test may return None on systems with different paths. + // The key assertion is that it doesn't panic and returns consistent types. + let _ = result; + } + + #[test] + fn test_is_skills_path_canonical_worktree_directory() { + let worktree_roots = vec![PathBuf::from("/home/user/projects/myproject")]; + + let path = PathBuf::from("/home/user/projects/myproject/.agents/skills/test.md"); + let result = is_skills_path_canonical(&path, &worktree_roots); + + assert!(result.is_some()); + assert_eq!(result.unwrap(), path); + } + + #[test] + fn test_is_skills_path_canonical_worktree_subdirectory() { + let worktree_roots = vec![PathBuf::from("/home/user/projects/myproject")]; + + let path = + PathBuf::from("/home/user/projects/myproject/.agents/skills/nested/deep/skill.md"); + let result = is_skills_path_canonical(&path, &worktree_roots); + + assert!(result.is_some()); + assert_eq!(result.unwrap(), path); + } + + #[test] + fn test_is_skills_path_canonical_not_in_skills() { + let worktree_roots = vec![PathBuf::from("/home/user/project")]; + + let path = PathBuf::from("/etc/passwd"); + let result = is_skills_path_canonical(&path, &worktree_roots); + + assert!(result.is_none()); + } + + #[test] + fn test_is_skills_path_canonical_sibling_of_skills() { + let worktree_roots = vec![PathBuf::from("/home/user/project")]; + + let path = PathBuf::from("/home/user/project/.agents/config.toml"); + let result = is_skills_path_canonical(&path, &worktree_roots); + + assert!(result.is_none()); + } + + #[test] + fn test_is_skills_path_canonical_different_worktree() { + let worktree_roots = vec![PathBuf::from("/home/user/projectA")]; + + let path = PathBuf::from("/home/user/projectB/.agents/skills/test.md"); + let result = is_skills_path_canonical(&path, &worktree_roots); + + assert!(result.is_none()); + } + + #[test] + fn test_is_skills_path_canonical_multiple_worktrees() { + let worktree_roots = vec![ + PathBuf::from("/home/user/projectA"), + PathBuf::from("/home/user/projectB"), + ]; + + // Path in first worktree + let path_a = PathBuf::from("/home/user/projectA/.agents/skills/skill.md"); + let result_a = is_skills_path_canonical(&path_a, &worktree_roots); + assert!(result_a.is_some()); + assert_eq!(result_a.unwrap(), path_a); + + // Path in second worktree + let path_b = PathBuf::from("/home/user/projectB/.agents/skills/skill.md"); + let result_b = is_skills_path_canonical(&path_b, &worktree_roots); + assert!(result_b.is_some()); + assert_eq!(result_b.unwrap(), path_b); + } + + #[test] + fn test_parse_with_extra_fields() { + let content = r#"--- +name: test-skill +description: A test skill +license: MIT +compatibility: 1.0 +metadata: + author: Test + version: 1.0 +allowed_tools: bash +--- +# Skill Content"#; + + let (metadata, body) = parse_skill_file(content, "test-skill").unwrap(); + assert_eq!(metadata.name, "test-skill"); + assert_eq!(metadata.license, Some("MIT".to_string())); + assert_eq!(metadata.compatibility, Some("1.0".to_string())); + assert!(!body.is_empty()); + } + + #[test] + fn test_validate_unicode_name() { + let metadata = SkillMetadata { + name: "测试-skill".to_string(), // Chinese characters + description: "Valid description".to_string(), + license: None, + compatibility: None, + metadata: HashMap::default(), + allowed_tools: None, + }; + // Unicode characters outside allowed set should fail + assert!(metadata.validate("测试-skill").is_err()); + } +} diff --git a/crates/agent/src/templates.rs b/crates/agent/src/templates.rs index 103fde17fd4d865b346a428e1f23e335005afe88..21a4787595becad1fff4867c69c0d92097f9494c 100644 --- a/crates/agent/src/templates.rs +++ b/crates/agent/src/templates.rs @@ -39,12 +39,32 @@ pub struct SystemPromptTemplate<'a> { pub project: &'a prompt_store::ProjectContext, pub available_tools: Vec, pub model_name: Option, + pub available_skills: String, } impl Template for SystemPromptTemplate<'_> { const TEMPLATE_NAME: &'static str = "system_prompt.hbs"; } +/// Context for a single skill in the skills prompt template. +#[derive(Serialize)] +pub struct SkillContext { + pub name: String, + pub description: String, + pub path: String, +} + +/// Template for rendering the available skills section of the system prompt. +#[derive(Serialize)] +pub struct SkillsPromptTemplate { + pub skills: Vec, + pub has_skills: bool, +} + +impl Template for SkillsPromptTemplate { + const TEMPLATE_NAME: &'static str = "skills_prompt.hbs"; +} + /// Handlebars helper for checking if an item is in a list fn contains( h: &handlebars::Helper, @@ -80,6 +100,7 @@ mod tests { let template = SystemPromptTemplate { project: &project, available_tools: vec!["echo".into()], + available_skills: String::new(), model_name: Some("test-model".to_string()), }; let templates = Templates::new(); @@ -88,4 +109,33 @@ mod tests { assert!(!rendered.contains("## Planning")); assert!(rendered.contains("test-model")); } + + #[test] + fn test_skills_prompt_template() { + let templates = Templates::new(); + let template = SkillsPromptTemplate { + skills: vec![SkillContext { + name: "test-skill".to_string(), + description: "A test skill description".to_string(), + path: "/home/user/.config/zed/skills/test-skill".to_string(), + }], + has_skills: true, + }; + let rendered = template.render(&templates).unwrap(); + assert!(rendered.contains("## Available Agent Skills")); + assert!(rendered.contains( + "| test-skill | A test skill description | /home/user/.config/zed/skills/test-skill |" + )); + } + + #[test] + fn test_skills_prompt_template_empty() { + let templates = Templates::new(); + let template = SkillsPromptTemplate { + skills: vec![], + has_skills: false, + }; + let rendered = template.render(&templates).unwrap(); + assert!(rendered.is_empty()); + } } diff --git a/crates/agent/src/templates/skills_prompt.hbs b/crates/agent/src/templates/skills_prompt.hbs new file mode 100644 index 0000000000000000000000000000000000000000..780a3873fc1536a0d857657ced8f25b15cb560af --- /dev/null +++ b/crates/agent/src/templates/skills_prompt.hbs @@ -0,0 +1,39 @@ +{{#if has_skills}} +## Available Agent Skills + +Skills are located in: +- Global: `~/.config/zed/skills//` +- Worktree-specific: `/.agents/skills//` + +Each skill contains: +- `SKILL.md` - Main instructions +- `scripts/` - Executable scripts +- `references/` - Additional documentation +- `assets/` - Templates, data files, images + +To use a skill, you should use the read_file and list_directory tools to explore skill files, and only use bash for running executable scripts: + +**For reading and exploring (use internal tools):** +- Read SKILL.md: `read_file` with path `~/.config/zed/skills//SKILL.md` +- List skill references: `list_directory` with path `~/.config/zed/skills//references/` +- List skill scripts: `list_directory` with path `~/.config/zed/skills//scripts/` +- Read references: `read_file` with path `~/.config/zed/skills//references/doc.md` + +**For running scripts only (use bash):** +- Run scripts: `bash ~/.config/zed/skills//scripts/script.sh` + +Note: You typically want to run tools for the current project you're working on, which is why you should use your worktree root as the working directory. + +| Skill | Description | Location | +|-------|-------------|----------| +{{#each skills}} +| {{name}} | {{description}} | {{path}} | +{{/each}} + +When a skill is relevant to the user's request: +1. **Always start here:** Read SKILL.md to load the skill's instructions using `read_file` with path `~/.config/zed/skills//SKILL.md` +2. **Then, based on the instructions in SKILL.md and the user's specific needs:** + - Use `list_directory` to explore available resources + - Use `read_file` to read reference files + - Use `bash` only to run executable scripts +{{/if}} diff --git a/crates/agent/src/templates/system_prompt.hbs b/crates/agent/src/templates/system_prompt.hbs index 67c920707289173ac4c7c1c9d98a8cd64126eb89..7c73af56dad9814e8a89d988df2254e7f6e837a9 100644 --- a/crates/agent/src/templates/system_prompt.hbs +++ b/crates/agent/src/templates/system_prompt.hbs @@ -200,6 +200,10 @@ Default Shell: {{shell}} You are powered by the model named {{model_name}}. +{{/if}} +{{#if available_skills}} +{{{available_skills}}} + {{/if}} {{#if (or has_rules has_user_rules)}} ## User's Custom Instructions diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 9808b95dd0812f9a857da8a9c39e78fde40af1f9..0c998b893a44e0e5fbc26a5606daca103cae6f82 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -5152,6 +5152,48 @@ async fn test_subagent_tool_resume_session(cx: &mut TestAppContext) { ); } +#[gpui::test] +async fn test_subagent_tool_is_present_when_feature_flag_enabled(cx: &mut TestAppContext) { + init_test(cx); + + cx.update(|cx| { + cx.update_flags(true, vec!["subagents".to_string()]); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/test"), json!({})).await; + let project = Project::test(fs, [path!("/test").as_ref()], cx).await; + let project_context = cx.new(|_cx| ProjectContext::default()); + let context_server_store = project.read_with(cx, |project, _| project.context_server_store()); + let context_server_registry = + cx.new(|cx| ContextServerRegistry::new(context_server_store.clone(), cx)); + let model = Arc::new(FakeLanguageModel::default()); + + let environment = Rc::new(cx.update(|cx| { + FakeThreadEnvironment::default().with_terminal(FakeTerminalHandle::new_never_exits(cx)) + })); + + let thread = cx.new(|cx| { + let mut thread = Thread::new( + project.clone(), + project_context, + context_server_registry, + Templates::new(), + Some(model), + cx, + ); + thread.add_default_tools(environment, cx); + thread + }); + + thread.read_with(cx, |thread, _| { + assert!( + thread.has_registered_tool(SpawnAgentTool::NAME), + "subagent tool should be present when feature flag is enabled" + ); + }); +} + #[gpui::test] async fn test_subagent_thread_inherits_parent_thread_properties(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index b61df1b8af84d312d7f186fb85e5a1d04ab59dfd..f90787c3aa3ee5e2a545c342f6e732f32db6e143 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -2,9 +2,9 @@ use crate::{ ContextServerRegistry, CopyPathTool, CreateDirectoryTool, DbLanguageModel, DbThread, DeletePathTool, DiagnosticsTool, EditFileTool, FetchTool, FindPathTool, GrepTool, ListDirectoryTool, MovePathTool, NowTool, OpenTool, ProjectSnapshot, ReadFileTool, - RestoreFileFromDiskTool, SaveFileTool, SpawnAgentTool, StreamingEditFileTool, - SystemPromptTemplate, Template, Templates, TerminalTool, ToolPermissionDecision, - UpdatePlanTool, WebSearchTool, decide_permission_from_settings, + RestoreFileFromDiskTool, SaveFileTool, SkillsContext, SpawnAgentTool, StreamingEditFileTool, + SystemPromptTemplate, Template, Templates, TerminalTool, ToolPermissionDecision, WebSearchTool, + decide_permission_from_settings, }; use acp_thread::{MentionUri, UserMessageId}; use action_log::ActionLog; @@ -950,6 +950,8 @@ pub struct Thread { profile_id: AgentProfileId, project_context: Entity, pub(crate) templates: Arc, + /// Formatted available skills for the system prompt. + available_skills: Entity, model: Option>, summarization_model: Option>, thinking_enabled: bool, @@ -1043,6 +1045,12 @@ impl Thread { .and_then(|model| model.effort.clone()); let (prompt_capabilities_tx, prompt_capabilities_rx) = watch::channel(Self::prompt_capabilities(model.as_deref())); + let worktree_roots: Vec = project + .read(cx) + .visible_worktrees(cx) + .map(|worktree| worktree.read(cx).abs_path().as_ref().to_path_buf()) + .collect(); + let available_skills = SkillsContext::new(worktree_roots, templates.clone(), cx); Self { id: acp::SessionId::new(uuid::Uuid::new_v4().to_string()), prompt_id: PromptId::new(), @@ -1069,6 +1077,7 @@ impl Thread { profile_id, project_context, templates, + available_skills, model, summarization_model: None, thinking_enabled: enable_thinking, @@ -1259,6 +1268,12 @@ impl Thread { watch::channel(Self::prompt_capabilities(model.as_deref())); let action_log = cx.new(|_| ActionLog::new(project.clone())); + let worktree_roots: Vec = project + .read(cx) + .visible_worktrees(cx) + .map(|worktree| worktree.read(cx).abs_path().as_ref().to_path_buf()) + .collect(); + let available_skills = SkillsContext::new(worktree_roots, templates.clone(), cx); Self { id, @@ -1284,6 +1299,7 @@ impl Thread { profile_id, project_context, templates, + available_skills, model, summarization_model: None, thinking_enabled: db_thread.thinking_enabled, @@ -2915,6 +2931,7 @@ impl Thread { let system_prompt = SystemPromptTemplate { project: self.project_context.read(cx), available_tools, + available_skills: self.available_skills.read(cx).formatted().to_string(), model_name: self.model.as_ref().map(|m| m.name().0.to_string()), } .render(&self.templates) diff --git a/crates/agent/src/tools/list_directory_tool.rs b/crates/agent/src/tools/list_directory_tool.rs index 7abbe1ed4c488210b9079e59765dddc8d5208bed..e822d7b670f1e5884da1f9cb72b8f5dd80098442 100644 --- a/crates/agent/src/tools/list_directory_tool.rs +++ b/crates/agent/src/tools/list_directory_tool.rs @@ -5,12 +5,14 @@ use super::tool_permissions::{ use crate::{AgentTool, ToolCallEventStream, ToolInput}; use agent_client_protocol::ToolKind; use anyhow::{Context as _, Result, anyhow}; +use futures::StreamExt; use gpui::{App, Entity, SharedString, Task}; use project::{Project, ProjectPath, WorktreeSettings}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::Settings; use std::fmt::Write; +use std::path::PathBuf; use std::sync::Arc; use util::markdown::MarkdownInlineCode; @@ -182,6 +184,29 @@ impl AgentTool for ListDirectoryTool { let fs = project.read_with(cx, |project, _cx| project.fs().clone()); let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await; + if let Some(canonical_input) = crate::skills::is_skills_path(&input.path, &canonical_roots) { + // Skills directory access - list directly via FS + if !fs.is_dir(&canonical_input).await { + return Err(format!("{} is not a directory.", input.path)); + } + + let mut entries = fs.read_dir(&canonical_input).await.map_err(|e| e.to_string())?; + let mut output = String::new(); + + while let Some(entry) = entries.next().await { + let path = entry.map_err(|e| e.to_string())?; + let name = path.file_name().unwrap_or_default().to_string_lossy(); + let is_dir = fs.is_dir(&path).await; + if is_dir { + writeln!(output, "{}/", name).ok(); + } else { + writeln!(output, "{}", name).ok(); + } + } + + return Ok(output); + } + let (project_path, symlink_canonical_target) = project.read_with(cx, |project, cx| -> anyhow::Result<_> { let resolved = resolve_project_path(project, &input.path, &canonical_roots, cx)?; diff --git a/crates/agent/src/tools/read_file_tool.rs b/crates/agent/src/tools/read_file_tool.rs index 093a8580892cfc4cec0a061bcc10717b28c608f2..fb33588c140155c2fab7a5696166c2257deaa05a 100644 --- a/crates/agent/src/tools/read_file_tool.rs +++ b/crates/agent/src/tools/read_file_tool.rs @@ -10,6 +10,7 @@ use project::{AgentLocation, ImageItem, Project, WorktreeSettings, image_store}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::Settings; +use std::path::PathBuf; use std::sync::Arc; use util::markdown::MarkdownCodeBlock; @@ -126,8 +127,43 @@ impl AgentTool for ReadFileTool { .await .map_err(tool_content_err)?; let fs = project.read_with(cx, |project, _cx| project.fs().clone()); + let canonical_roots = canonicalize_worktree_roots(&project, &fs, cx).await; + if let Some(canonical_input) = crate::skills::is_skills_path(&input.path, &canonical_roots) { + // Skills directory access - read directly via FS + if !fs.is_file(&canonical_input).await { + return Err(tool_content_err(format!("{} not found", input.path))); + } + + cx.update(|_cx| { + event_stream.update_fields(ToolCallUpdateFields::new().locations(vec![ + acp::ToolCallLocation::new(&canonical_input) + .line(input.start_line.map(|line| line.saturating_sub(1))), + ])); + }); + + // Read file directly + let content = fs.load(&canonical_input).await.map_err(tool_content_err)?; + + // Apply line range filtering if specified + let content = if input.start_line.is_some() || input.end_line.is_some() { + let lines: Vec<&str> = content.lines().collect(); + let start = input.start_line.unwrap_or(1).max(1) as usize; + let start_idx = start.saturating_sub(1); + let end = input.end_line.unwrap_or(u32::MAX) as usize; + if end <= start_idx { + lines.get(start_idx).copied().unwrap_or("").to_string() + } else { + lines[start_idx..end.min(lines.len())].join("\n") + } + } else { + content + }; + + return Ok(LanguageModelToolResultContent::Text(content.into())); + } + let (project_path, symlink_canonical_target) = project.read_with(cx, |project, cx| { let resolved = diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 5fd39509df4ec2263e47c7e87b3e4b7852eaf154..134a8df8a18e70f412b36c78a90430092c62ad82 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -26,6 +26,7 @@ use zed_actions::agent::{ }; use crate::thread_metadata_store::ThreadMetadataStore; +use crate::ui::{AcpOnboardingModal, ClaudeCodeOnboardingModal, HoldForDefault}; use crate::{ AddContextServer, AgentDiffPane, ConversationView, CopyThreadToClipboard, CycleStartThreadIn, Follow, InlineAssistant, LoadThreadFromClipboard, NewThread, OpenActiveThreadAsMarkdown,