From a6ce7a37da82ae100a737af1e90eb2df9d465085 Mon Sep 17 00:00:00 2001 From: Alex Hancock Date: Tue, 7 Apr 2026 11:03:29 -0400 Subject: [PATCH] Revert "refactor: skills as its own platform ext (#8244)" (#8375) --- .../src/routes/config_management.rs | 18 +- crates/goose/src/agents/execute_commands.rs | 5 +- .../src/agents/platform_extensions/mod.rs | 76 -- .../src/agents/platform_extensions/skills.rs | 410 -------- .../src/agents/platform_extensions/summon.rs | 914 ++++++++++++++++-- ...nager__tests__all_platform_extensions.snap | 11 +- 6 files changed, 838 insertions(+), 596 deletions(-) delete mode 100644 crates/goose/src/agents/platform_extensions/skills.rs diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index 1d18e2b4..bbf119aa 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -427,13 +427,19 @@ pub async fn get_slash_commands( let working_dir = query.working_dir.map(std::path::PathBuf::from); for source in - goose::agents::platform_extensions::skills::list_installed_skills(working_dir.as_deref()) + goose::agents::platform_extensions::summon::list_installed_sources(working_dir.as_deref()) { - commands.push(SlashCommand { - command: source.name, - help: source.description, - command_type: CommandType::Skill, - }); + if matches!( + source.kind, + goose::agents::platform_extensions::summon::SourceKind::Skill + | goose::agents::platform_extensions::summon::SourceKind::BuiltinSkill + ) { + commands.push(SlashCommand { + command: source.name, + help: source.description, + command_type: CommandType::Skill, + }); + } } Ok(Json(SlashCommandsResponse { commands })) diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index b4971385..363c04c7 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -140,8 +140,7 @@ impl Agent { } async fn handle_skills_command(&self, session_id: &str) -> Result> { - use super::platform_extensions::skills::list_installed_skills; - use super::platform_extensions::SourceKind; + use super::platform_extensions::summon::{list_installed_sources, SourceKind}; let working_dir = self .config @@ -150,7 +149,7 @@ impl Agent { .await .ok() .map(|s| s.working_dir); - let sources = list_installed_skills(working_dir.as_deref()); + let sources = list_installed_sources(working_dir.as_deref()); let skills: Vec<_> = sources .iter() .filter(|s| matches!(s.kind, SourceKind::Skill | SourceKind::BuiltinSkill)) diff --git a/crates/goose/src/agents/platform_extensions/mod.rs b/crates/goose/src/agents/platform_extensions/mod.rs index 743552f4..5a337b4e 100644 --- a/crates/goose/src/agents/platform_extensions/mod.rs +++ b/crates/goose/src/agents/platform_extensions/mod.rs @@ -6,79 +6,16 @@ pub mod code_execution; pub mod developer; pub mod ext_manager; pub mod orchestrator; -pub mod skills; pub mod summarize; pub mod summon; pub mod todo; pub mod tom; use std::collections::HashMap; -use std::path::PathBuf; use crate::agents::mcp_client::McpClientTrait; use crate::session::Session; use once_cell::sync::Lazy; -use serde::Deserialize; -use tracing::warn; - -#[derive(Debug, Clone)] -pub struct Source { - pub name: String, - pub kind: SourceKind, - pub description: String, - pub path: PathBuf, - pub content: String, - pub supporting_files: Vec, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] -pub enum SourceKind { - Subrecipe, - Recipe, - Skill, - Agent, - BuiltinSkill, -} - -impl std::fmt::Display for SourceKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - SourceKind::Subrecipe => write!(f, "subrecipe"), - SourceKind::Recipe => write!(f, "recipe"), - SourceKind::Skill => write!(f, "skill"), - SourceKind::Agent => write!(f, "agent"), - SourceKind::BuiltinSkill => write!(f, "builtin skill"), - } - } -} - -impl Source { - pub fn to_load_text(&self) -> String { - format!( - "## {} ({})\n\n{}\n\n### Content\n\n{}", - self.name, self.kind, self.description, self.content - ) - } -} - -pub fn parse_frontmatter Deserialize<'de>>(content: &str) -> Option<(T, String)> { - let parts: Vec<&str> = content.split("---").collect(); - if parts.len() < 3 { - return None; - } - - let yaml_content = parts[1].trim(); - let metadata: T = match serde_yaml::from_str(yaml_content) { - Ok(m) => m, - Err(e) => { - warn!("Failed to parse frontmatter: {}", e); - return None; - } - }; - - let body = parts[2..].join("---").trim().to_string(); - Some((metadata, body)) -} pub use ext_manager::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; @@ -252,19 +189,6 @@ pub static PLATFORM_EXTENSIONS: Lazy }, ); - map.insert( - skills::EXTENSION_NAME, - PlatformExtensionDef { - name: skills::EXTENSION_NAME, - display_name: "Skills", - description: "Discover and provide skill instructions from filesystem and builtins", - default_enabled: true, - unprefixed_tools: false, - hidden: true, - client_factory: |ctx| Box::new(skills::SkillsClient::new(ctx).unwrap()), - }, - ); - map }, ); diff --git a/crates/goose/src/agents/platform_extensions/skills.rs b/crates/goose/src/agents/platform_extensions/skills.rs deleted file mode 100644 index 650d716a..00000000 --- a/crates/goose/src/agents/platform_extensions/skills.rs +++ /dev/null @@ -1,410 +0,0 @@ -use super::{parse_frontmatter, Source, SourceKind}; -use crate::agents::builtin_skills; -use crate::agents::extension::PlatformExtensionContext; -use crate::agents::mcp_client::{Error, McpClientTrait}; -use crate::agents::tool_execution::ToolCallContext; -use crate::config::paths::Paths; -use async_trait::async_trait; -use rmcp::model::{ - CallToolResult, Implementation, InitializeResult, JsonObject, ListToolsResult, - ServerCapabilities, ServerNotification, -}; -use serde::Deserialize; -use std::collections::HashSet; -use std::path::{Path, PathBuf}; -use tokio::sync::mpsc; -use tokio_util::sync::CancellationToken; -use tracing::warn; - -pub static EXTENSION_NAME: &str = "skills"; - -#[derive(Debug, Deserialize)] -struct SkillMetadata { - name: String, - description: String, -} - -pub fn parse_skill_content(content: &str, path: PathBuf) -> Option { - let (metadata, body): (SkillMetadata, String) = parse_frontmatter(content)?; - - if metadata.name.contains('/') { - warn!( - "Skill name '{}' contains '/' which is not allowed, skipping", - metadata.name - ); - return None; - } - - Some(Source { - name: metadata.name, - kind: SourceKind::Skill, - description: metadata.description, - path, - content: body, - supporting_files: Vec::new(), - }) -} - -pub fn scan_skills_from_dir(dir: &Path, seen: &mut HashSet) -> Vec { - let mut sources = Vec::new(); - let mut visited_dirs = HashSet::new(); - for skill_file in collect_skill_files(dir, &mut visited_dirs) { - let Some(skill_dir) = skill_file.parent() else { - continue; - }; - let content = match std::fs::read_to_string(&skill_file) { - Ok(c) => c, - Err(e) => { - warn!("Failed to read skill file {}: {}", skill_file.display(), e); - continue; - } - }; - - if let Some(mut source) = parse_skill_content(&content, skill_dir.to_path_buf()) { - if !seen.contains(&source.name) { - let mut visited_support_dirs = HashSet::new(); - source.supporting_files = - find_supporting_files(skill_dir, &mut visited_support_dirs); - seen.insert(source.name.clone()); - sources.push(source); - } - } - } - sources -} - -fn collect_skill_files(dir: &Path, visited_dirs: &mut HashSet) -> Vec { - let mut skill_files = Vec::new(); - - walk_files_recursively( - dir, - visited_dirs, - &mut |path| !should_skip_dir(path), - &mut |path| { - if path.file_name().and_then(|name| name.to_str()) == Some("SKILL.md") { - skill_files.push(path.to_path_buf()); - } - }, - ); - - skill_files -} - -fn should_skip_dir(path: &Path) -> bool { - matches!( - path.file_name().and_then(|name| name.to_str()), - Some(".git") | Some(".hg") | Some(".svn") - ) -} - -fn walk_files_recursively( - dir: &Path, - visited_dirs: &mut HashSet, - should_descend: &mut G, - visit_file: &mut F, -) where - F: FnMut(&Path), - G: FnMut(&Path) -> bool, -{ - let canonical_dir = match std::fs::canonicalize(dir) { - Ok(path) => path, - Err(_) => return, - }; - - if !visited_dirs.insert(canonical_dir) { - return; - } - - let entries = match std::fs::read_dir(dir) { - Ok(e) => e, - Err(_) => return, - }; - - for entry in entries.flatten() { - let path = entry.path(); - - if path.is_dir() { - if should_descend(&path) { - walk_files_recursively(&path, visited_dirs, should_descend, visit_file); - } - } else if path.is_file() { - visit_file(&path); - } - } -} - -pub fn find_supporting_files( - directory: &Path, - visited_dirs: &mut HashSet, -) -> Vec { - let mut files = Vec::new(); - - walk_files_recursively( - directory, - visited_dirs, - &mut |path| !should_skip_dir(path) && !path.join("SKILL.md").is_file(), - &mut |path| { - let is_skill_md = path - .file_name() - .and_then(|n| n.to_str()) - .map(|n| n == "SKILL.md") - .unwrap_or(false); - if !is_skill_md { - files.push(path.to_path_buf()); - } - }, - ); - - files -} - -fn skill_dirs(working_dir: &Path) -> (Vec, Vec) { - let home = dirs::home_dir(); - let config = Paths::config_dir(); - - let local = vec![ - working_dir.join(".goose/skills"), - working_dir.join(".claude/skills"), - working_dir.join(".agents/skills"), - ]; - - let global = [ - home.as_ref().map(|h| h.join(".agents/skills")), - Some(config.join("skills")), - home.as_ref().map(|h| h.join(".claude/skills")), - home.as_ref().map(|h| h.join(".config/agents/skills")), - ] - .into_iter() - .flatten() - .collect(); - - (local, global) -} - -pub fn discover_skills(working_dir: &Path) -> Vec { - let mut sources = Vec::new(); - let mut seen = HashSet::new(); - - let (local_dirs, global_dirs) = skill_dirs(working_dir); - - for dir in local_dirs { - sources.extend(scan_skills_from_dir(&dir, &mut seen)); - } - - for dir in global_dirs { - sources.extend(scan_skills_from_dir(&dir, &mut seen)); - } - - for content in builtin_skills::get_all() { - if let Some(source) = parse_skill_content(content, PathBuf::new()) { - if !seen.contains(&source.name) { - seen.insert(source.name.clone()); - sources.push(Source { - kind: SourceKind::BuiltinSkill, - ..source - }); - } - } - } - - sources -} - -pub fn list_installed_skills(working_dir: Option<&Path>) -> Vec { - let dir = working_dir - .map(|p| p.to_path_buf()) - .unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); - discover_skills(&dir) -} - -fn build_skill_instructions(skills: &[&Source]) -> String { - let mut instructions = String::new(); - if !skills.is_empty() { - instructions.push_str( - "\n\nYou have these skills at your disposal, when it is clear they can help you solve a problem or you are asked to use them:", - ); - for skill in skills { - instructions.push_str(&format!("\n• {} - {}", skill.name, skill.description)); - } - } - instructions -} - -pub struct SkillsClient { - info: InitializeResult, -} - -impl SkillsClient { - pub fn new(context: PlatformExtensionContext) -> anyhow::Result { - let instructions = if let Some(session) = &context.session { - let sources = discover_skills(&session.working_dir); - let mut skills: Vec<&Source> = sources - .iter() - .filter(|s| s.kind == SourceKind::Skill || s.kind == SourceKind::BuiltinSkill) - .collect(); - skills.sort_by(|a, b| (&a.name, &a.path).cmp(&(&b.name, &b.path))); - build_skill_instructions(&skills) - } else { - String::new() - }; - - let info = InitializeResult::new(ServerCapabilities::builder().build()) - .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Skills")) - .with_instructions(instructions); - - Ok(Self { info }) - } -} - -#[async_trait] -impl McpClientTrait for SkillsClient { - async fn list_tools( - &self, - _session_id: &str, - _next_cursor: Option, - _cancellation_token: CancellationToken, - ) -> Result { - Ok(ListToolsResult { - tools: vec![], - next_cursor: None, - meta: None, - }) - } - - async fn call_tool( - &self, - _ctx: &ToolCallContext, - name: &str, - _arguments: Option, - _cancellation_token: CancellationToken, - ) -> Result { - Ok(CallToolResult::error(vec![rmcp::model::Content::text( - format!("Error: Unknown tool: {}", name), - )])) - } - - fn get_info(&self) -> Option<&InitializeResult> { - Some(&self.info) - } - - async fn subscribe(&self) -> mpsc::Receiver { - let (_tx, rx) = mpsc::channel(1); - rx - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - use std::sync::Arc; - use tempfile::TempDir; - - #[test] - fn test_parse_skill_content() { - let skill = "---\nname: test-skill\ndescription: A test skill\n---\nSkill body here."; - let source = parse_skill_content(skill, PathBuf::new()).unwrap(); - assert_eq!(source.name, "test-skill"); - assert_eq!(source.kind, SourceKind::Skill); - assert!(source.content.contains("Skill body")); - } - - #[test] - fn test_parse_skill_rejects_slash_in_name() { - let skill = "---\nname: bad/skill\ndescription: A skill\n---\nContent."; - assert!(parse_skill_content(skill, PathBuf::new()).is_none()); - } - - #[test] - fn test_parse_skill_rejects_invalid_frontmatter() { - assert!(parse_skill_content("no frontmatter", PathBuf::new()).is_none()); - assert!(parse_skill_content("---\nunclosed", PathBuf::new()).is_none()); - } - - #[test] - fn test_discover_skills_from_filesystem() { - let temp_dir = TempDir::new().unwrap(); - - let goose_skill = temp_dir.path().join(".goose/skills/my-skill"); - fs::create_dir_all(&goose_skill).unwrap(); - fs::write( - goose_skill.join("SKILL.md"), - "---\nname: my-skill\ndescription: goose version\n---\nContent", - ) - .unwrap(); - - let claude_skill = temp_dir.path().join(".claude/skills/my-skill"); - fs::create_dir_all(&claude_skill).unwrap(); - fs::write( - claude_skill.join("SKILL.md"), - "---\nname: my-skill\ndescription: claude version\n---\nContent", - ) - .unwrap(); - - let sources = discover_skills(temp_dir.path()); - let skill = sources.iter().find(|s| s.name == "my-skill").unwrap(); - assert_eq!(skill.description, "goose version"); - } - - #[test] - fn test_discover_skills_includes_builtins() { - let temp_dir = TempDir::new().unwrap(); - let sources = discover_skills(temp_dir.path()); - assert!(sources.iter().any(|s| s.kind == SourceKind::BuiltinSkill)); - } - - #[test] - fn test_skill_supporting_files() { - let temp_dir = TempDir::new().unwrap(); - - let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); - fs::create_dir_all(skill_dir.join("templates/nested")).unwrap(); - fs::write( - skill_dir.join("SKILL.md"), - "---\nname: my-skill\ndescription: A skill\n---\nContent", - ) - .unwrap(); - fs::write(skill_dir.join("myscript.sh"), "#!/bin/bash\necho ok").unwrap(); - fs::write(skill_dir.join("templates/report.txt"), "template").unwrap(); - fs::write(skill_dir.join("templates/nested/checklist.txt"), "nested").unwrap(); - - let sources = discover_skills(temp_dir.path()); - let skill = sources.iter().find(|s| s.name == "my-skill").unwrap(); - assert_eq!(skill.supporting_files.len(), 3); - } - - #[test] - fn test_build_skill_instructions_empty() { - let empty: &[&Source] = &[]; - assert_eq!(build_skill_instructions(empty), ""); - } - - #[test] - fn test_build_skill_instructions_with_skills() { - let skill = Source { - name: "test".to_string(), - kind: SourceKind::Skill, - description: "A test skill".to_string(), - path: PathBuf::new(), - content: String::new(), - supporting_files: vec![], - }; - let instructions = build_skill_instructions(&[&skill]); - assert!(instructions.contains("test - A test skill")); - } - - #[tokio::test] - async fn test_skills_client_no_tools() { - let context = PlatformExtensionContext { - extension_manager: None, - session_manager: Arc::new(crate::session::SessionManager::instance()), - session: None, - }; - let client = SkillsClient::new(context).unwrap(); - let result = client - .list_tools("test", None, CancellationToken::new()) - .await - .unwrap(); - assert!(result.tools.is_empty()); - } -} diff --git a/crates/goose/src/agents/platform_extensions/summon.rs b/crates/goose/src/agents/platform_extensions/summon.rs index 3f642dae..b32611b5 100644 --- a/crates/goose/src/agents/platform_extensions/summon.rs +++ b/crates/goose/src/agents/platform_extensions/summon.rs @@ -1,4 +1,10 @@ -use super::{parse_frontmatter, Source, SourceKind}; +//! Summon Extension - Unified tooling for recipes, skills, and subagents +//! +//! Provides two tools: +//! - `load`: Inject knowledge into current context or discover available sources +//! - `delegate`: Run tasks in isolated subagents (sync or async) + +use crate::agents::builtin_skills; use crate::agents::extension::PlatformExtensionContext; use crate::agents::mcp_client::{Error, McpClientTrait}; use crate::agents::subagent_handler::{run_subagent_task, OnMessageCallback, SubagentRunParams}; @@ -20,7 +26,7 @@ use rmcp::model::{ ServerCapabilities, ServerNotification, Tool, }; use serde::Deserialize; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; use std::sync::Arc; @@ -33,12 +39,54 @@ use tracing::{info, warn}; pub static EXTENSION_NAME: &str = "summon"; +#[derive(Debug, Clone)] +pub struct Source { + pub name: String, + pub kind: SourceKind, + pub description: String, + pub path: PathBuf, + pub content: String, + pub supporting_files: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum SourceKind { + Subrecipe, + Recipe, + Skill, + Agent, + BuiltinSkill, +} + +impl std::fmt::Display for SourceKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SourceKind::Subrecipe => write!(f, "subrecipe"), + SourceKind::Recipe => write!(f, "recipe"), + SourceKind::Skill => write!(f, "skill"), + SourceKind::Agent => write!(f, "agent"), + SourceKind::BuiltinSkill => write!(f, "builtin skill"), + } + } +} + +impl Source { + /// Format the source content for loading into context + pub fn to_load_text(&self) -> String { + format!( + "## {} ({})\n\n{}\n\n### Content\n\n{}", + self.name, self.kind, self.description, self.content + ) + } +} + fn kind_plural(kind: SourceKind) -> &'static str { match kind { SourceKind::Subrecipe => "Subrecipes", SourceKind::Recipe => "Recipes", + SourceKind::Skill => "Skills", SourceKind::Agent => "Agents", - _ => "Other", + SourceKind::BuiltinSkill => "Builtin Skills", } } @@ -86,6 +134,12 @@ pub struct CompletedTask { pub duration: Duration, } +#[derive(Debug, Deserialize)] +struct SkillMetadata { + name: String, + description: String, +} + #[derive(Debug, Deserialize)] struct AgentMetadata { name: String, @@ -95,6 +149,46 @@ struct AgentMetadata { model: Option, } +fn parse_frontmatter Deserialize<'de>>(content: &str) -> Option<(T, String)> { + let parts: Vec<&str> = content.split("---").collect(); + if parts.len() < 3 { + return None; + } + + let yaml_content = parts[1].trim(); + let metadata: T = match serde_yaml::from_str(yaml_content) { + Ok(m) => m, + Err(e) => { + warn!("Failed to parse frontmatter: {}", e); + return None; + } + }; + + let body = parts[2..].join("---").trim().to_string(); + Some((metadata, body)) +} + +fn parse_skill_content(content: &str, path: PathBuf) -> Option { + let (metadata, body): (SkillMetadata, String) = parse_frontmatter(content)?; + + if metadata.name.contains('/') { + warn!( + "Skill name '{}' contains '/' which is not allowed, skipping", + metadata.name + ); + return None; + } + + Some(Source { + name: metadata.name, + kind: SourceKind::Skill, + description: metadata.description, + path, + content: body, + supporting_files: Vec::new(), + }) +} + fn parse_agent_content(content: &str, path: PathBuf) -> Option { let (metadata, body): (AgentMetadata, String) = parse_frontmatter(content)?; @@ -117,6 +211,96 @@ fn parse_agent_content(content: &str, path: PathBuf) -> Option { }) } +/// Scan a directory for skill subdirectories containing SKILL.md files. +/// Returns discovered skills, skipping any whose names are already in `seen`. +fn scan_skills_from_dir(dir: &Path, seen: &mut std::collections::HashSet) -> Vec { + let mut sources = Vec::new(); + let mut visited_dirs = HashSet::new(); + for skill_file in collect_skill_files(dir, &mut visited_dirs) { + let Some(skill_dir) = skill_file.parent() else { + continue; + }; + let content = match std::fs::read_to_string(&skill_file) { + Ok(c) => c, + Err(e) => { + warn!("Failed to read skill file {}: {}", skill_file.display(), e); + continue; + } + }; + + if let Some(mut source) = parse_skill_content(&content, skill_dir.to_path_buf()) { + if !seen.contains(&source.name) { + let mut visited_support_dirs = HashSet::new(); + source.supporting_files = + find_supporting_files(skill_dir, &mut visited_support_dirs); + seen.insert(source.name.clone()); + sources.push(source); + } + } + } + sources +} + +fn collect_skill_files(dir: &Path, visited_dirs: &mut HashSet) -> Vec { + let mut skill_files = Vec::new(); + + walk_files_recursively( + dir, + visited_dirs, + &mut |path| !should_skip_skill_walk_dir(path), + &mut |path| { + if path.file_name().and_then(|name| name.to_str()) == Some("SKILL.md") { + skill_files.push(path.to_path_buf()); + } + }, + ); + + skill_files +} + +fn should_skip_skill_walk_dir(path: &Path) -> bool { + matches!( + path.file_name().and_then(|name| name.to_str()), + Some(".git") | Some(".hg") | Some(".svn") + ) +} + +fn walk_files_recursively( + dir: &Path, + visited_dirs: &mut HashSet, + should_descend: &mut G, + visit_file: &mut F, +) where + F: FnMut(&Path), + G: FnMut(&Path) -> bool, +{ + let canonical_dir = match std::fs::canonicalize(dir) { + Ok(path) => path, + Err(_) => return, + }; + + if !visited_dirs.insert(canonical_dir) { + return; + } + + let entries = match std::fs::read_dir(dir) { + Ok(e) => e, + Err(_) => return, + }; + + for entry in entries.flatten() { + let path = entry.path(); + + if path.is_dir() { + if should_descend(&path) { + walk_files_recursively(&path, visited_dirs, should_descend, visit_file); + } + } else if path.is_file() { + visit_file(&path); + } + } +} + fn scan_recipes_from_dir( dir: &Path, kind: SourceKind, @@ -206,6 +390,16 @@ fn scan_agents_from_dir( } } +/// Returns all discovered sources (skills, recipes, agents) from the given working directory. +/// If no directory is provided, falls back to `std::env::current_dir()`. +/// This is useful for listing what's available without needing a full SummonClient instance. +pub fn list_installed_sources(working_dir: Option<&Path>) -> Vec { + let dir = working_dir + .map(|p| p.to_path_buf()) + .unwrap_or_else(|| std::env::current_dir().unwrap_or_default()); + discover_filesystem_sources(&dir) +} + fn discover_filesystem_sources(working_dir: &Path) -> Vec { let mut sources: Vec = Vec::new(); let mut seen: std::collections::HashSet = std::collections::HashSet::new(); @@ -236,6 +430,22 @@ fn discover_filesystem_sources(working_dir: &Path) -> Vec { ) .collect(); + let local_skill_dirs: Vec = vec![ + working_dir.join(".goose/skills"), + working_dir.join(".claude/skills"), + working_dir.join(".agents/skills"), + ]; + + let global_skill_dirs: Vec = [ + home.as_ref().map(|h| h.join(".agents/skills")), + Some(config.join("skills")), + home.as_ref().map(|h| h.join(".claude/skills")), + home.as_ref().map(|h| h.join(".config/agents/skills")), + ] + .into_iter() + .flatten() + .collect(); + let local_agent_dirs: Vec = vec![ working_dir.join(".goose/agents"), working_dir.join(".claude/agents"), @@ -255,6 +465,10 @@ fn discover_filesystem_sources(working_dir: &Path) -> Vec { scan_recipes_from_dir(&dir, SourceKind::Recipe, &mut sources, &mut seen); } + for dir in local_skill_dirs { + sources.extend(scan_skills_from_dir(&dir, &mut seen)); + } + for dir in local_agent_dirs { scan_agents_from_dir(&dir, &mut sources, &mut seen); } @@ -263,13 +477,52 @@ fn discover_filesystem_sources(working_dir: &Path) -> Vec { scan_recipes_from_dir(&dir, SourceKind::Recipe, &mut sources, &mut seen); } + for dir in global_skill_dirs { + sources.extend(scan_skills_from_dir(&dir, &mut seen)); + } + for dir in global_agent_dirs { scan_agents_from_dir(&dir, &mut sources, &mut seen); } + for content in builtin_skills::get_all() { + if let Some(source) = parse_skill_content(content, PathBuf::new()) { + if !seen.contains(&source.name) { + seen.insert(source.name.clone()); + sources.push(Source { + kind: SourceKind::BuiltinSkill, + ..source + }); + } + } + } + sources } +/// Collect all files in a skill directory recursively, excluding SKILL.md itself. +fn find_supporting_files(directory: &Path, visited_dirs: &mut HashSet) -> Vec { + let mut files = Vec::new(); + + walk_files_recursively( + directory, + visited_dirs, + &mut |path| !should_skip_skill_walk_dir(path) && !path.join("SKILL.md").is_file(), + &mut |path| { + let is_skill_md = path + .file_name() + .and_then(|n| n.to_str()) + .map(|n| n == "SKILL.md") + .unwrap_or(false); + if !is_skill_md { + files.push(path.to_path_buf()); + } + }, + ); + + files +} + fn round_duration(d: Duration) -> String { let secs = d.as_secs(); if secs < 60 { @@ -320,8 +573,31 @@ impl Drop for SummonClient { impl SummonClient { pub fn new(context: PlatformExtensionContext) -> Result { + let instructions = if let Some(session) = &context.session { + let mut instructions = "".to_string(); + let sources = discover_filesystem_sources(&session.working_dir); + + let mut skills: Vec<&Source> = sources + .iter() + .filter(|s| s.kind == SourceKind::Skill || s.kind == SourceKind::BuiltinSkill) + .collect(); + + skills.sort_by(|a, b| (&a.name, &a.path).cmp(&(&b.name, &b.path))); + + if !skills.is_empty() { + instructions.push_str("\n\nYou have these skills at your disposal, when it is clear they can help you solve a problem or you are asked to use them:"); + for skill in &skills { + instructions.push_str(&format!("\n• {} - {}", skill.name, skill.description)); + } + } + Some(instructions) + } else { + None + }; + let info = InitializeResult::new(ServerCapabilities::builder().enable_tools().build()) - .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Summon")); + .with_server_info(Implementation::new(EXTENSION_NAME, "1.0.0").with_title("Summon")) + .with_instructions(instructions.unwrap_or_default()); Ok(Self { info, @@ -374,13 +650,13 @@ impl SummonClient { Tool::new( "load", "Load knowledge into your current context or discover available sources.\n\n\ - Call with no arguments to list all available sources (subrecipes, recipes, agents).\n\ + Call with no arguments to list all available sources (subrecipes, recipes, skills, agents).\n\ Call with a source name to load its content into your context.\n\ For background tasks: load(source: \"task_id\") waits for the task and returns the result.\n\ To cancel a running task: load(source: \"task_id\", cancel: true) stops and returns output.\n\n\ Examples:\n\ - load() → Lists available sources\n\ - - load(source: \"deploy\") → Loads the deploy recipe\n\ + - load(source: \"rust-patterns\") → Loads the rust-patterns skill\n\ - load(source: \"20260219_1\") → Waits for background task, then returns result" .to_string(), schema.as_object().unwrap().clone(), @@ -397,7 +673,7 @@ impl SummonClient { }, "source": { "type": "string", - "description": "Name of a recipe or agent to run." + "description": "Name of a recipe, skill, or agent to run." }, "parameters": { "type": "object", @@ -439,8 +715,8 @@ impl SummonClient { "Delegate a task to a subagent that runs independently with its own context.\n\n\ Modes:\n\ 1. Ad-hoc: Provide `instructions` for a custom task\n\ - 2. Source-based: Provide `source` name to run a subrecipe, recipe, or agent\n\ - 3. Combined: Pair a source with a task (e.g., source: \"deploy\", instructions: \"deploy to staging\")\n\n\ + 2. Source-based: Provide `source` name to run a subrecipe, recipe, skill, or agent\n\ + 3. Combined: Pair a source with a task (e.g., source: \"rust-patterns\", instructions: \"review auth.rs\")\n\n\ Effective Delegation:\n\ - Delegates know only instructions + source content\n\ - Delegates cannot coordinate. Same-file work = conflicts.\n\ @@ -510,6 +786,47 @@ impl SummonClient { return Ok(Some(source)); } + if let Some((skill_name, raw_relative_path)) = name.split_once('/') { + let relative_path = raw_relative_path.replace('\\', "/"); + if let Some(skill) = sources.iter().find(|s| { + s.name == skill_name + && matches!(s.kind, SourceKind::Skill | SourceKind::BuiltinSkill) + }) { + let canonical_skill_dir = skill + .path + .canonicalize() + .unwrap_or_else(|_| skill.path.clone()); + + for file_path in &skill.supporting_files { + if let Ok(rel) = file_path.strip_prefix(&skill.path) { + let rel_normalized = rel.to_string_lossy().replace('\\', "/"); + if rel_normalized == relative_path { + let canonical_file = file_path + .canonicalize() + .map_err(|e| format!("Failed to resolve '{}': {}", name, e))?; + if !canonical_file.starts_with(&canonical_skill_dir) { + return Err(format!( + "Refusing to load '{}': file resolves outside the skill directory", + name + )); + } + return match std::fs::read_to_string(&canonical_file) { + Ok(content) => Ok(Some(Source { + name: name.to_string(), + kind: SourceKind::Skill, + description: format!("Supporting file for {}", skill_name), + path: file_path.clone(), + content, + supporting_files: vec![], + })), + Err(e) => Err(format!("Failed to read '{}': {}", name, e)), + }; + } + } + } + } + } + Ok(None) } @@ -802,9 +1119,10 @@ impl SummonClient { "No sources available for load/delegate.\n\n\ Sources are discovered from:\n\ • Current recipe's sub_recipes\n\ - • .agents/recipes/, .agents/agents/ (project-level)\n\ - • ~/.agents/agents/ (global)\n\ - • GOOSE_RECIPE_PATH directories", + • .agents/skills/, .agents/recipes/, .agents/agents/ (project-level)\n\ + • ~/.agents/skills/, ~/.agents/agents/ (global)\n\ + • GOOSE_RECIPE_PATH directories\n\ + • Builtin skills", )]); } @@ -827,7 +1145,13 @@ impl SummonClient { } } - for kind in [SourceKind::Subrecipe, SourceKind::Recipe, SourceKind::Agent] { + for kind in [ + SourceKind::Subrecipe, + SourceKind::Recipe, + SourceKind::Skill, + SourceKind::Agent, + SourceKind::BuiltinSkill, + ] { let kind_sources: Vec<_> = sources.iter().filter(|s| s.kind == kind).collect(); if !kind_sources.is_empty() { output.push_str(&format!("\n{}:\n", kind_plural(kind))); @@ -859,16 +1183,72 @@ impl SummonClient { Some(source) => { let content = source.to_load_text(); - let output = format!( - "# Loaded: {} ({})\n\n{}\n\n---\nThis knowledge is now available in your context.", + let mut output = format!( + "# Loaded: {} ({})\n\n{}\n", source.name, source.kind, content ); + if !source.supporting_files.is_empty() { + output.push_str(&format!( + "\n## Supporting Files\n\nSkill directory: {}\n\nThe following supporting files are available:\n", + source.path.display() + )); + for file in &source.supporting_files { + if let Ok(relative) = file.strip_prefix(&source.path) { + let rel_str = relative.to_string_lossy().replace('\\', "/"); + output.push_str(&format!( + "- {} → load(source: \"{}/{}\")\n", + rel_str, source.name, rel_str + )); + } + } + output.push_str( + "\nUse load(source: \"/\") to load individual files into context, or use file tools to read/run them directly.\n", + ); + } + + output.push_str("\n---\nThis knowledge is now available in your context."); + Ok(vec![Content::text(output)]) } None => { let sources = self.get_sources(session_id, working_dir).await; + if let Some((skill_name, _)) = name.split_once('/') { + if let Some(skill) = sources.iter().find(|s| { + s.name == skill_name + && matches!(s.kind, SourceKind::Skill | SourceKind::BuiltinSkill) + }) { + let available: Vec = skill + .supporting_files + .iter() + .filter_map(|f| { + f.strip_prefix(&skill.path) + .ok() + .map(|r| r.to_string_lossy().replace('\\', "/")) + }) + .collect(); + if !available.is_empty() { + let total = available.len(); + let display: Vec<_> = available.into_iter().take(10).collect(); + let suffix = if total > 10 { + format!(" (and {} more)", total - 10) + } else { + String::new() + }; + return Err(format!( + "Source '{}' not found. Available files for {}: {}{}", + name, + skill_name, + display.join(", "), + suffix + )); + } else { + return Err(format!("Skill '{}' has no supporting files.", skill_name)); + } + } + } + let suggestions: Vec<&str> = sources .iter() .filter(|s| { @@ -1066,18 +1446,24 @@ impl SummonClient { .await? .ok_or_else(|| format!("Source '{}' not found", source_name))?; + if source_name.contains('/') + && matches!(source.kind, SourceKind::Skill | SourceKind::BuiltinSkill) + { + return Err(format!( + "Cannot delegate to supporting file '{}'. Use load() to read it instead.", + source_name + )); + } + let mut recipe = match source.kind { SourceKind::Recipe | SourceKind::Subrecipe => { self.build_recipe_from_source(&source, params, session_id) .await? } - SourceKind::Agent => self.build_recipe_from_agent(&source, params)?, - _ => { - return Err(format!( - "Source '{}' has kind '{}' which cannot be delegated from summon", - source_name, source.kind - )) + SourceKind::Skill | SourceKind::BuiltinSkill => { + self.build_recipe_from_skill(&source, params)? } + SourceKind::Agent => self.build_recipe_from_agent(&source, params)?, }; if let Some(extra_instructions) = ¶ms.instructions { @@ -1170,6 +1556,26 @@ impl SummonClient { .map_err(|e| format!("Failed to build recipe: {}", e)) } + fn build_recipe_from_skill( + &self, + source: &Source, + params: &DelegateParams, + ) -> Result { + let mut builder = Recipe::builder() + .version("1.0.0") + .title(format!("Skill: {}", source.name)) + .description(source.description.clone()) + .instructions(&source.content); + + if params.instructions.is_none() { + builder = builder.prompt("Apply the skill knowledge to produce a useful result."); + } + + builder + .build() + .map_err(|e| format!("Failed to build recipe from skill: {}", e)) + } + fn build_recipe_from_agent( &self, source: &Source, @@ -1660,7 +2066,17 @@ mod tests { } #[test] - fn test_agent_frontmatter_parsing() { + fn test_frontmatter_parsing() { + let skill = r#"--- +name: test-skill +description: A test skill +--- +Skill body here."#; + let source = parse_skill_content(skill, PathBuf::new()).unwrap(); + assert_eq!(source.name, "test-skill"); + assert_eq!(source.kind, SourceKind::Skill); + assert!(source.content.contains("Skill body")); + let agent = r#"--- name: reviewer model: sonnet @@ -1669,155 +2085,465 @@ You review code."#; let source = parse_agent_content(agent, PathBuf::new()).unwrap(); assert_eq!(source.name, "reviewer"); assert!(source.description.contains("sonnet")); + + assert!(parse_skill_content("no frontmatter", PathBuf::new()).is_none()); + assert!(parse_skill_content("---\nunclosed", PathBuf::new()).is_none()); } #[tokio::test] - async fn test_discover_recipes_and_agents() { + async fn test_source_discovery_and_priority() { let temp_dir = TempDir::new().unwrap(); + let goose_skill = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(&goose_skill).unwrap(); + fs::write( + goose_skill.join("SKILL.md"), + "---\nname: my-skill\ndescription: goose version\n---\nContent", + ) + .unwrap(); + + let claude_skill = temp_dir.path().join(".claude/skills/my-skill"); + fs::create_dir_all(&claude_skill).unwrap(); + fs::write( + claude_skill.join("SKILL.md"), + "---\nname: my-skill\ndescription: claude version\n---\nContent", + ) + .unwrap(); + let recipes = temp_dir.path().join(".goose/recipes"); fs::create_dir_all(&recipes).unwrap(); fs::write( - recipes.join("deploy.yaml"), - "title: Deploy\ndescription: Deploy to production\ninstructions: Run deploy steps", - ) - .unwrap(); - - let agents = temp_dir.path().join(".goose/agents"); - fs::create_dir_all(&agents).unwrap(); - fs::write( - agents.join("reviewer.md"), - "---\nname: reviewer\nmodel: sonnet\ndescription: Code reviewer\n---\nYou review code.", + recipes.join("test.yaml"), + "title: Test\ndescription: A recipe\ninstructions: Do it", ) .unwrap(); let client = SummonClient::new(create_test_context()).unwrap(); let sources = client.discover_filesystem_sources(temp_dir.path()); - let recipe = sources - .iter() - .find(|s| s.name == "deploy" && s.kind == SourceKind::Recipe) - .unwrap(); - assert_eq!(recipe.description, "Deploy to production"); - assert_eq!(recipe.content, "Run deploy steps"); + let skill = sources.iter().find(|s| s.name == "my-skill").unwrap(); + assert_eq!(skill.description, "goose version"); - let agent = sources + assert!(sources .iter() - .find(|s| s.name == "reviewer" && s.kind == SourceKind::Agent) - .unwrap(); - assert_eq!(agent.description, "Code reviewer"); - assert!(agent.content.contains("You review code")); + .any(|s| s.name == "test" && s.kind == SourceKind::Recipe)); + + assert!(sources.iter().any(|s| s.kind == SourceKind::BuiltinSkill)); } #[tokio::test] - async fn test_recipe_deduplication_local_wins() { + async fn test_skill_supporting_files_discovered() { let temp_dir = TempDir::new().unwrap(); - let local = temp_dir.path().join(".goose/recipes"); - fs::create_dir_all(&local).unwrap(); + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(skill_dir.join("templates/nested")).unwrap(); fs::write( - local.join("deploy.yaml"), - "title: Deploy\ndescription: Local deploy\ninstructions: local steps", + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill with scripts\n---\nRun check_all.sh", ) .unwrap(); - - let also_local = temp_dir.path().join(".agents/recipes"); - fs::create_dir_all(&also_local).unwrap(); + fs::write(skill_dir.join("myscript.sh"), "#!/bin/bash\necho ok").unwrap(); + fs::write(skill_dir.join("templates/report.txt"), "template content").unwrap(); fs::write( - also_local.join("deploy.yaml"), - "title: Deploy\ndescription: Agents deploy\ninstructions: agents steps", + skill_dir.join("templates/nested/checklist.txt"), + "nested template content", ) .unwrap(); let client = SummonClient::new(create_test_context()).unwrap(); let sources = client.discover_filesystem_sources(temp_dir.path()); - let deploys: Vec<_> = sources.iter().filter(|s| s.name == "deploy").collect(); - assert_eq!(deploys.len(), 1); + let skill = sources.iter().find(|s| s.name == "my-skill").unwrap(); + assert_eq!(skill.path, skill_dir); + assert_eq!(skill.supporting_files.len(), 3); + + let file_names: Vec = skill + .supporting_files + .iter() + .filter_map(|f| f.file_name().map(|n| n.to_string_lossy().to_string())) + .collect(); + assert!(file_names.contains(&"myscript.sh".to_string())); + assert!(file_names.contains(&"report.txt".to_string())); + assert!(file_names.contains(&"checklist.txt".to_string())); } #[tokio::test] - async fn test_load_recipe_source() { + async fn test_nested_claude_catalog_skills_discovered() { let temp_dir = TempDir::new().unwrap(); - let recipes = temp_dir.path().join(".goose/recipes"); - fs::create_dir_all(&recipes).unwrap(); + let root_skill_file = temp_dir.path().join(".claude/skills/SKILL.md"); + fs::create_dir_all(root_skill_file.parent().unwrap()).unwrap(); fs::write( - recipes.join("deploy.yaml"), - "title: Deploy\ndescription: Deploy to production\ninstructions: Run deploy steps", + &root_skill_file, + "---\nname: root-skill\ndescription: Root level skill\n---\nRoot content", + ) + .unwrap(); + + let nested_skill_dir = temp_dir.path().join(".claude/skills/catalog/internal/ai"); + fs::create_dir_all(&nested_skill_dir).unwrap(); + fs::write( + nested_skill_dir.join("SKILL.md"), + "---\nname: nested-skill\ndescription: Nested catalog skill\n---\nNested content", ) .unwrap(); let client = SummonClient::new(create_test_context()).unwrap(); - let result = client - .handle_load_source("test", "deploy", temp_dir.path()) - .await - .unwrap(); + let sources = client.discover_filesystem_sources(temp_dir.path()); - let text = &result[0].as_text().expect("expected text content").text; - assert!(text.contains("deploy")); - assert!(text.contains("Run deploy steps")); - assert!(text.contains("now available in your context")); + let root_skill = sources.iter().find(|s| s.name == "root-skill").unwrap(); + assert_eq!(root_skill.path, temp_dir.path().join(".claude/skills")); + + let nested_skill = sources.iter().find(|s| s.name == "nested-skill").unwrap(); + assert_eq!(nested_skill.path, nested_skill_dir); } #[tokio::test] - async fn test_load_agent_source() { + async fn test_root_skill_supporting_files_exclude_nested_skill_subtrees() { let temp_dir = TempDir::new().unwrap(); - let agents = temp_dir.path().join(".goose/agents"); - fs::create_dir_all(&agents).unwrap(); + let root_skill_dir = temp_dir.path().join(".claude/skills"); + fs::create_dir_all(&root_skill_dir).unwrap(); fs::write( - agents.join("reviewer.md"), - "---\nname: reviewer\nmodel: sonnet\ndescription: Code reviewer\n---\nYou review code carefully.", + root_skill_dir.join("SKILL.md"), + "---\nname: root-skill\ndescription: Root level skill\n---\nRoot content", + ) + .unwrap(); + fs::write(root_skill_dir.join("README.md"), "root readme").unwrap(); + + let nested_skill_dir = root_skill_dir.join("catalog/internal/ai"); + fs::create_dir_all(&nested_skill_dir).unwrap(); + fs::write( + nested_skill_dir.join("SKILL.md"), + "---\nname: nested-skill\ndescription: Nested catalog skill\n---\nNested content", + ) + .unwrap(); + fs::write(nested_skill_dir.join("notes.md"), "nested notes").unwrap(); + + let client = SummonClient::new(create_test_context()).unwrap(); + let sources = client.discover_filesystem_sources(temp_dir.path()); + + let root_skill = sources.iter().find(|s| s.name == "root-skill").unwrap(); + assert!(root_skill + .supporting_files + .contains(&root_skill_dir.join("README.md"))); + assert!(!root_skill + .supporting_files + .contains(&nested_skill_dir.join("SKILL.md"))); + assert!(!root_skill + .supporting_files + .contains(&nested_skill_dir.join("notes.md"))); + } + + #[tokio::test] + async fn test_skill_discovery_preserves_dot_prefixed_paths() { + let temp_dir = TempDir::new().unwrap(); + + let dot_skill_dir = temp_dir.path().join(".claude/skills/.team"); + fs::create_dir_all(&dot_skill_dir).unwrap(); + fs::write( + dot_skill_dir.join("SKILL.md"), + "---\nname: team-skill\ndescription: Dot skill\n---\nTeam content", + ) + .unwrap(); + fs::write(dot_skill_dir.join(".env.example"), "EXAMPLE=1").unwrap(); + + let git_skill_dir = temp_dir.path().join(".claude/skills/.git/hidden-skill"); + fs::create_dir_all(&git_skill_dir).unwrap(); + fs::write( + git_skill_dir.join("SKILL.md"), + "---\nname: hidden-git-skill\ndescription: Hidden git skill\n---\nHidden content", ) .unwrap(); + let client = SummonClient::new(create_test_context()).unwrap(); + let sources = client.discover_filesystem_sources(temp_dir.path()); + + let dot_skill = sources.iter().find(|s| s.name == "team-skill").unwrap(); + assert_eq!(dot_skill.path, dot_skill_dir); + assert!(dot_skill + .supporting_files + .contains(&dot_skill_dir.join(".env.example"))); + assert!(!sources.iter().any(|s| s.name == "hidden-git-skill")); + } + + #[cfg(unix)] + #[tokio::test] + async fn test_symlinked_skill_directory_is_discovered() { + let temp_dir = TempDir::new().unwrap(); + + let shared_skill_dir = temp_dir.path().join("shared-skills/ai"); + fs::create_dir_all(&shared_skill_dir).unwrap(); + fs::write( + shared_skill_dir.join("SKILL.md"), + "---\nname: shared-ai\ndescription: Shared skill\n---\nShared content", + ) + .unwrap(); + fs::write(shared_skill_dir.join("notes.md"), "shared notes").unwrap(); + + let linked_catalog_dir = temp_dir.path().join(".claude/skills/catalog/internal"); + fs::create_dir_all(&linked_catalog_dir).unwrap(); + std::os::unix::fs::symlink(&shared_skill_dir, linked_catalog_dir.join("ai")).unwrap(); + + let client = SummonClient::new(create_test_context()).unwrap(); + let sources = client.discover_filesystem_sources(temp_dir.path()); + + let skill = sources.iter().find(|s| s.name == "shared-ai").unwrap(); + assert_eq!(skill.path, linked_catalog_dir.join("ai")); + assert!(skill + .supporting_files + .contains(&linked_catalog_dir.join("ai/notes.md"))); + } + + #[cfg(unix)] + #[tokio::test] + async fn test_skill_discovery_avoids_symlink_cycles() { + let temp_dir = TempDir::new().unwrap(); + + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(skill_dir.join("refs")).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill with a loop\n---\nLoop safe", + ) + .unwrap(); + fs::write(skill_dir.join("refs/guide.md"), "guide content").unwrap(); + std::os::unix::fs::symlink(&skill_dir, skill_dir.join("refs/loop")).unwrap(); + + let client = SummonClient::new(create_test_context()).unwrap(); + let sources = client.discover_filesystem_sources(temp_dir.path()); + + let skill = sources.iter().find(|s| s.name == "my-skill").unwrap(); + assert_eq!(skill.path, skill_dir); + assert!(skill + .supporting_files + .contains(&skill_dir.join("refs/guide.md"))); + assert_eq!( + skill + .supporting_files + .iter() + .filter(|path| *path == &skill_dir.join("refs/guide.md")) + .count(), + 1 + ); + } + + #[tokio::test] + async fn test_load_source_lists_supporting_files_with_load_names() { + let temp_dir = TempDir::new().unwrap(); + + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(skill_dir.join("references")).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill\n---\nSee references.", + ) + .unwrap(); + fs::write( + skill_dir.join("references/ops.md"), + "# Ops Guide\n\nDo the thing.", + ) + .unwrap(); + fs::write(skill_dir.join("run.sh"), "#!/bin/bash\necho ok").unwrap(); + let client = SummonClient::new(create_test_context()).unwrap(); let result = client - .handle_load_source("test", "reviewer", temp_dir.path()) + .handle_load_source("test", "my-skill", temp_dir.path()) .await .unwrap(); let text = &result[0].as_text().expect("expected text content").text; - assert!(text.contains("reviewer")); - assert!(text.contains("You review code carefully")); - assert!(text.contains("now available in your context")); + + assert!( + !text.contains("Ops Guide"), + "md file content should not be inlined" + ); + assert!( + !text.contains("Do the thing."), + "md file content should not be inlined" + ); + assert!( + !text.contains("#!/bin/bash"), + "script content should not be inlined" + ); + assert!( + text.contains("load(source: \"my-skill/references/ops.md\")"), + "md file should be listed with load() name" + ); + assert!( + text.contains("load(source: \"my-skill/run.sh\")"), + "script should be listed with load() name" + ); + assert!( + text.contains("load(source: \"/\")"), + "should include usage hint" + ); } #[tokio::test] - async fn test_load_nonexistent_source_suggests_similar() { + async fn test_load_supporting_file_by_path() { let temp_dir = TempDir::new().unwrap(); - let recipes = temp_dir.path().join(".goose/recipes"); - fs::create_dir_all(&recipes).unwrap(); + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(skill_dir.join("references")).unwrap(); fs::write( - recipes.join("deploy.yaml"), - "title: Deploy\ndescription: Deploy to production\ninstructions: steps", + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill\n---\nSee references.", + ) + .unwrap(); + fs::write( + skill_dir.join("references/ops.md"), + "# Ops Guide\n\nDo the thing.", + ) + .unwrap(); + fs::write(skill_dir.join("run.sh"), "#!/bin/bash\necho ok").unwrap(); + + let client = SummonClient::new(create_test_context()).unwrap(); + + let md_result = client + .handle_load_source("test", "my-skill/references/ops.md", temp_dir.path()) + .await + .unwrap(); + let md_text = &md_result[0].as_text().expect("expected text content").text; + assert!( + md_text.contains("Ops Guide"), + "markdown content should be loaded" + ); + assert!(md_text.contains("Do the thing.")); + + let sh_result = client + .handle_load_source("test", "my-skill/run.sh", temp_dir.path()) + .await + .unwrap(); + let sh_text = &sh_result[0].as_text().expect("expected text content").text; + assert!( + sh_text.contains("#!/bin/bash"), + "script content should be loaded" + ); + } + + #[tokio::test] + async fn test_load_supporting_file_not_found_suggests_available() { + let temp_dir = TempDir::new().unwrap(); + + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(skill_dir.join("references")).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill\n---\nSee references.", + ) + .unwrap(); + fs::write( + skill_dir.join("references/ops.md"), + "# Ops Guide\n\nDo the thing.", ) .unwrap(); let client = SummonClient::new(create_test_context()).unwrap(); let err = client - .handle_load_source("test", "deploy-prod", temp_dir.path()) + .handle_load_source( + "test", + "my-skill/references/nonexistent.md", + temp_dir.path(), + ) .await .unwrap_err(); - assert!(err.contains("not found")); - assert!(err.contains("deploy"), "should suggest 'deploy': {}", err); + assert!( + err.contains("references/ops.md"), + "error should list available files: {}", + err + ); + assert!( + err.contains("my-skill"), + "error should name the skill: {}", + err + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn test_resolve_source_blocks_symlink_outside_skill_dir() { + let temp_dir = TempDir::new().unwrap(); + let outside_dir = TempDir::new().unwrap(); + + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill\n---\nContent.", + ) + .unwrap(); + + let secret_file = outside_dir.path().join("secret.txt"); + fs::write(&secret_file, "top secret data").unwrap(); + std::os::unix::fs::symlink(&secret_file, skill_dir.join("evil.md")).unwrap(); + + let client = SummonClient::new(create_test_context()).unwrap(); + let result = client + .handle_load_source("test", "my-skill/evil.md", temp_dir.path()) + .await; + + assert!( + result.is_err(), + "symlink outside skill dir should be blocked" + ); + let err = result.unwrap_err(); + assert!( + err.contains("resolves outside the skill directory"), + "error should mention path traversal: {}", + err + ); } #[tokio::test] - async fn test_load_completely_unknown_source() { + async fn test_resolve_source_blocks_path_traversal_input() { let temp_dir = TempDir::new().unwrap(); - let client = SummonClient::new(create_test_context()).unwrap(); - let err = client - .handle_load_source("test", "zzz-nonexistent", temp_dir.path()) - .await - .unwrap_err(); + let skill_dir = temp_dir.path().join(".goose/skills/my-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: my-skill\ndescription: A skill\n---\nContent.", + ) + .unwrap(); + fs::write(skill_dir.join("legit.md"), "legit content").unwrap(); - assert!(err.contains("not found")); - assert!(err.contains("Use load()")); + let client = SummonClient::new(create_test_context()).unwrap(); + + // ../../../etc/passwd won't match any supporting_files entry, so it returns Ok (not found) + // which becomes the "not found" error path in handle_load_source + let result = client + .handle_load_source("test", "my-skill/../../../etc/passwd", temp_dir.path()) + .await; + + assert!(result.is_err(), "traversal path should not load content"); + let err = result.unwrap_err(); + assert!( + !err.contains("root:"), + "should not contain /etc/passwd content: {}", + err + ); + } + + #[tokio::test] + async fn test_skill_name_with_slash_is_rejected() { + let temp_dir = TempDir::new().unwrap(); + + let skill_dir = temp_dir.path().join(".goose/skills/bad-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: bad/skill\ndescription: A skill with slash\n---\nContent.", + ) + .unwrap(); + + let client = SummonClient::new(create_test_context()).unwrap(); + let sources = client.get_sources("test", temp_dir.path()).await; + + assert!( + !sources.iter().any(|s| s.name == "bad/skill"), + "skill with '/' in name should be rejected" + ); } #[tokio::test] diff --git a/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap b/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap index df7355c9..a4bc66fa 100644 --- a/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap +++ b/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap @@ -93,19 +93,16 @@ Use write and edit to efficiently make changes. Test and verify as appropriate. ### Instructions Manage agent sessions: list, view, start, send messages, and interrupt agents. -## skills +## summarize + + +## summon ### Instructions You have these skills at your disposal, when it is clear they can help you solve a problem or you are asked to use them: • goose-doc-guide - Reference goose documentation to create, configure, or explain goose-specific features like recipes, extensions, sessions, and providers. You MUST fetch relevant goose docs before answering. You MUST NOT rely on training data or assumptions for any goose-specific fields, values, names, syntax, or commands. -## summarize - - -## summon - - ## todo ### Instructions