From 7feaf1b89a48105810a2de9fce0a6e6f9e2517c4 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 12 May 2026 18:00:00 +1000 Subject: [PATCH 01/17] return built-in commands in acp available commands --- crates/goose/src/acp/server.rs | 64 +++++++++++++++++------- crates/goose/src/lib.rs | 1 + crates/goose/src/slash_command.rs | 82 +++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 crates/goose/src/slash_command.rs diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 5db5f7cde356..dc84c6506e5f 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -27,22 +27,23 @@ use crate::source_roots::SourceRoot; use crate::utils::sanitize_unicode_tags; use agent_client_protocol::schema::{ AgentCapabilities, Annotations, AuthMethod, AuthMethodAgent, AuthenticateRequest, - AuthenticateResponse, BlobResourceContents, CancelNotification, CloseSessionRequest, - CloseSessionResponse, ConfigOptionUpdate, Content, ContentBlock, ContentChunk, - CurrentModeUpdate, EmbeddedResource, EmbeddedResourceResource, FileSystemCapabilities, - ForkSessionRequest, ForkSessionResponse, ImageContent, InitializeRequest, InitializeResponse, - ListSessionsRequest, ListSessionsResponse, LoadSessionRequest, LoadSessionResponse, - McpCapabilities, McpServer, Meta, ModelId, ModelInfo, NewSessionRequest, NewSessionResponse, - PermissionOption, PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse, - RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, SessionCapabilities, - SessionCloseCapabilities, SessionConfigOption, SessionConfigOptionCategory, - SessionConfigSelectOption, SessionId, SessionInfo, SessionInfoUpdate, SessionListCapabilities, - SessionMode, SessionModeId, SessionModeState, SessionModelState, SessionNotification, - SessionUpdate, SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, - SetSessionModeRequest, SetSessionModeResponse, SetSessionModelRequest, SetSessionModelResponse, - StopReason, TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, - ToolCallLocation, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, Usage, - UsageUpdate, + AuthenticateResponse, AvailableCommand, AvailableCommandInput, AvailableCommandsUpdate, + BlobResourceContents, CancelNotification, CloseSessionRequest, CloseSessionResponse, + ConfigOptionUpdate, Content, ContentBlock, ContentChunk, CurrentModeUpdate, EmbeddedResource, + EmbeddedResourceResource, FileSystemCapabilities, ForkSessionRequest, ForkSessionResponse, + ImageContent, InitializeRequest, InitializeResponse, ListSessionsRequest, ListSessionsResponse, + LoadSessionRequest, LoadSessionResponse, McpCapabilities, McpServer, Meta, ModelId, ModelInfo, + NewSessionRequest, NewSessionResponse, PermissionOption, PermissionOptionKind, + PromptCapabilities, PromptRequest, PromptResponse, RequestPermissionOutcome, + RequestPermissionRequest, ResourceLink, SessionCapabilities, SessionCloseCapabilities, + SessionConfigOption, SessionConfigOptionCategory, SessionConfigSelectOption, SessionId, + SessionInfo, SessionInfoUpdate, SessionListCapabilities, SessionMode, SessionModeId, + SessionModeState, SessionModelState, SessionNotification, SessionUpdate, + SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModeRequest, + SetSessionModeResponse, SetSessionModelRequest, SetSessionModelResponse, StopReason, + TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, ToolCallLocation, + ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, UnstructuredCommandInput, + Usage, UsageUpdate, }; use agent_client_protocol::util::MatchDispatchFrom; use agent_client_protocol::{ @@ -1044,6 +1045,33 @@ fn build_usage_update(session: &Session, context_limit: usize) -> UsageUpdate { } impl GooseAcpAgent { + fn available_commands_update() -> AvailableCommandsUpdate { + let commands = crate::slash_command::list_acp_commands() + .into_iter() + .map(|entry| { + let mut command = AvailableCommand::new(entry.name, entry.description); + if let Some(input_hint) = entry.input_hint { + command = command.input(AvailableCommandInput::Unstructured( + UnstructuredCommandInput::new(input_hint), + )); + } + command + }) + .collect(); + + AvailableCommandsUpdate::new(commands) + } + + fn send_available_commands_update( + cx: &ConnectionTo, + session_id: &SessionId, + ) -> Result<(), agent_client_protocol::Error> { + cx.send_notification(SessionNotification::new( + session_id.clone(), + SessionUpdate::AvailableCommandsUpdate(Self::available_commands_update()), + )) + } + pub fn permission_manager(&self) -> Arc { Arc::clone(&self.permission_manager) } @@ -2417,10 +2445,11 @@ impl GooseAcpAgent { } if let Some(usage_update) = initial_usage_update { cx.send_notification(SessionNotification::new( - acp_session_id, + acp_session_id.clone(), SessionUpdate::UsageUpdate(usage_update), ))?; } + Self::send_available_commands_update(cx, &acp_session_id)?; debug!( target: "perf", sid = %sid, @@ -2785,6 +2814,7 @@ impl GooseAcpAgent { SessionUpdate::UsageUpdate(usage_update), ))?; } + Self::send_available_commands_update(cx, &args.session_id)?; debug!( target: "perf", sid = %sid, diff --git a/crates/goose/src/lib.rs b/crates/goose/src/lib.rs index e1897cb84c10..a7106040cf18 100644 --- a/crates/goose/src/lib.rs +++ b/crates/goose/src/lib.rs @@ -41,6 +41,7 @@ pub mod security; pub mod session; pub mod session_context; pub mod skills; +pub mod slash_command; pub mod slash_commands; pub mod source_roots; pub mod sources; diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs new file mode 100644 index 000000000000..013971b65957 --- /dev/null +++ b/crates/goose/src/slash_command.rs @@ -0,0 +1,82 @@ +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SlashCommandSource { + Builtin, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SlashCommandEntry { + pub name: String, + pub description: String, + pub source: SlashCommandSource, + pub input_hint: Option, +} + +pub fn list_builtin_commands() -> Vec { + crate::agents::execute_commands::list_commands() + .iter() + .map(|command| SlashCommandEntry { + name: command.name.to_string(), + description: command.description.to_string(), + source: SlashCommandSource::Builtin, + input_hint: builtin_input_hint(command.name).map(str::to_string), + }) + .collect() +} + +pub fn list_acp_commands() -> Vec { + list_builtin_commands() +} + +fn builtin_input_hint(command: &str) -> Option<&'static str> { + match command { + "prompt" => Some(" [--info] [key=value...]"), + "prompts" => Some("[--extension ]"), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lists_acp_safe_builtin_commands() { + let commands = list_builtin_commands(); + let names: Vec<_> = commands + .iter() + .map(|command| command.name.as_str()) + .collect(); + + assert_eq!( + names, + vec!["prompts", "prompt", "compact", "clear", "skills", "doctor"] + ); + assert!(commands + .iter() + .all(|command| command.source == SlashCommandSource::Builtin)); + } + + #[test] + fn includes_input_hints_for_argument_taking_builtins() { + let commands = list_builtin_commands(); + let prompt = commands + .iter() + .find(|command| command.name == "prompt") + .expect("prompt command should be listed"); + let prompts = commands + .iter() + .find(|command| command.name == "prompts") + .expect("prompts command should be listed"); + let compact = commands + .iter() + .find(|command| command.name == "compact") + .expect("compact command should be listed"); + + assert_eq!( + prompt.input_hint.as_deref(), + Some(" [--info] [key=value...]") + ); + assert_eq!(prompts.input_hint.as_deref(), Some("[--extension ]")); + assert_eq!(compact.input_hint, None); + } +} From 4ea16e4a5a21cf5f649670e21a5eed45844bc093 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 14 May 2026 11:55:32 +1000 Subject: [PATCH 02/17] added skill slash command --- crates/goose/src/acp/server.rs | 20 ++-- crates/goose/src/agents/execute_commands.rs | 47 ++++++++- crates/goose/src/skills/client.rs | 29 +----- crates/goose/src/skills/mod.rs | 29 ++++++ crates/goose/src/slash_command.rs | 107 +++++++++++++++++++- 5 files changed, 196 insertions(+), 36 deletions(-) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index dc84c6506e5f..f06be1996ee3 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -1045,8 +1045,8 @@ fn build_usage_update(session: &Session, context_limit: usize) -> UsageUpdate { } impl GooseAcpAgent { - fn available_commands_update() -> AvailableCommandsUpdate { - let commands = crate::slash_command::list_acp_commands() + fn available_commands_update(working_dir: &std::path::Path) -> AvailableCommandsUpdate { + let commands = crate::slash_command::list_acp_commands(Some(working_dir)) .into_iter() .map(|entry| { let mut command = AvailableCommand::new(entry.name, entry.description); @@ -1065,10 +1065,11 @@ impl GooseAcpAgent { fn send_available_commands_update( cx: &ConnectionTo, session_id: &SessionId, + working_dir: &std::path::Path, ) -> Result<(), agent_client_protocol::Error> { cx.send_notification(SessionNotification::new( session_id.clone(), - SessionUpdate::AvailableCommandsUpdate(Self::available_commands_update()), + SessionUpdate::AvailableCommandsUpdate(Self::available_commands_update(working_dir)), )) } @@ -2424,6 +2425,8 @@ impl GooseAcpAgent { .prepare_session_init_config(&resolved, &mode_state, &goose_session) .await; + let working_dir = goose_session.working_dir.clone(); + self.spawn_agent_setup( cx, agent_tx, @@ -2449,7 +2452,7 @@ impl GooseAcpAgent { SessionUpdate::UsageUpdate(usage_update), ))?; } - Self::send_available_commands_update(cx, &acp_session_id)?; + Self::send_available_commands_update(cx, &acp_session_id, &working_dir)?; debug!( target: "perf", sid = %sid, @@ -2814,7 +2817,7 @@ impl GooseAcpAgent { SessionUpdate::UsageUpdate(usage_update), ))?; } - Self::send_available_commands_update(cx, &args.session_id)?; + Self::send_available_commands_update(cx, &args.session_id, &args.cwd)?; debug!( target: "perf", sid = %sid, @@ -3270,11 +3273,13 @@ impl GooseAcpAgent { .prepare_session_init_config(&resolved, &mode_state, &goose_session) .await; + let acp_session_id = SessionId::new(new_session_id.clone()); + self.spawn_agent_setup( cx, agent_tx, AgentSetupRequest { - session_id: SessionId::new(new_session_id.clone()), + session_id: acp_session_id.clone(), goose_session, mcp_servers: args.mcp_servers, resolved_provider: resolved.ok(), @@ -3284,7 +3289,7 @@ impl GooseAcpAgent { let meta = session_meta(&new_session); - let mut response = ForkSessionResponse::new(SessionId::new(new_session_id)) + let mut response = ForkSessionResponse::new(acp_session_id.clone()) .modes(mode_state) .meta(meta); if let Some(ms) = model_state { @@ -3293,6 +3298,7 @@ impl GooseAcpAgent { if let Some(co) = config_options { response = response.config_options(co); } + Self::send_available_commands_update(cx, &acp_session_id, &args.cwd)?; Ok(response) } diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 66d5d34bed81..6dcd2991b571 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -83,7 +83,14 @@ impl Agent { "skills" => self.handle_skills_command(session_id).await, "doctor" => Ok(Some(crate::doctor::run(self, session_id).await?)), _ => { - self.handle_recipe_command(command, params_str, session_id) + if let Some(message) = self + .handle_recipe_command(command, params_str, session_id) + .await? + { + return Ok(Some(message)); + } + + self.handle_skill_command(command, params_str, session_id) .await } } @@ -441,4 +448,42 @@ impl Agent { Ok(Some(Message::user().with_text(prompt))) } + + async fn handle_skill_command( + &self, + command: &str, + params_str: &str, + session_id: &str, + ) -> Result> { + let working_dir = self + .config + .session_manager + .get_session(session_id, false) + .await + .ok() + .map(|session| session.working_dir); + + let skill = crate::skills::list_installed_skills(working_dir.as_deref()) + .into_iter() + .find(|skill| skill.name.eq_ignore_ascii_case(command)); + + let Some(skill) = skill else { + return Ok(None); + }; + + let task = if params_str.is_empty() { + "Use this skill for the current task." + } else { + params_str + }; + + let prompt = format!( + "The user invoked the Goose skill `{}`.\n\n{}\n\nUser request:\n{}", + skill.name, + crate::skills::render_loaded_skill(&skill), + task + ); + + Ok(Some(Message::user().with_text(prompt))) + } } diff --git a/crates/goose/src/skills/client.rs b/crates/goose/src/skills/client.rs index 888dfd45d875..65ae64206ab5 100644 --- a/crates/goose/src/skills/client.rs +++ b/crates/goose/src/skills/client.rs @@ -123,32 +123,9 @@ impl McpClientTrait for SkillsClient { let skills = discover_skills(Some(&self.working_dir)); if let Some(skill) = skills.iter().find(|s| s.name == skill_name) { - let mut output = format!( - "# Loaded Skill: {} ({})\n\n{}\n", - skill.name, - skill.source_type, - skill.to_load_text() - ); - - if !skill.supporting_files.is_empty() { - let skill_dir = Path::new(&skill.path); - output.push_str(&format!( - "\n## Supporting Files\n\nSkill directory: {}\n\n", - skill.path - )); - for file in &skill.supporting_files { - if let Ok(relative) = Path::new(file).strip_prefix(skill_dir) { - let rel_str = relative.to_string_lossy().replace('\\', "/"); - output.push_str(&format!( - "- {} → load_skill(name: \"{}/{}\")\n", - rel_str, skill.name, rel_str - )); - } - } - } - - output.push_str("\n---\nThis knowledge is now available in your context."); - return Ok(CallToolResult::success(vec![Content::text(output)])); + return Ok(CallToolResult::success(vec![Content::text( + crate::skills::render_loaded_skill(skill), + )])); } if let Some((parent_skill_name, raw_relative_path)) = skill_name.split_once('/') { diff --git a/crates/goose/src/skills/mod.rs b/crates/goose/src/skills/mod.rs index 78e8299985b1..1409696fef0b 100644 --- a/crates/goose/src/skills/mod.rs +++ b/crates/goose/src/skills/mod.rs @@ -96,6 +96,35 @@ pub(crate) fn validate_skill_name(name: &str) -> Result<(), Error> { Ok(()) } +pub fn render_loaded_skill(skill: &SourceEntry) -> String { + let mut output = format!( + "# Loaded Skill: {} ({})\n\n{}\n", + skill.name, + skill.source_type, + skill.to_load_text() + ); + + if !skill.supporting_files.is_empty() { + let skill_dir = Path::new(&skill.path); + output.push_str(&format!( + "\n## Supporting Files\n\nSkill directory: {}\n\n", + skill.path + )); + for file in &skill.supporting_files { + if let Ok(relative) = Path::new(file).strip_prefix(skill_dir) { + let rel_str = relative.to_string_lossy().replace('\\', "/"); + output.push_str(&format!( + "- {} → load_skill(name: \"{}/{}\")\n", + rel_str, skill.name, rel_str + )); + } + } + } + + output.push_str("\n---\nThis knowledge is now available in your context."); + output +} + fn canonicalize_or_original(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) } diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index 013971b65957..1b3b1b95847d 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -1,6 +1,12 @@ +use std::collections::HashSet; +use std::path::Path; + +use goose_sdk::custom_requests::SourceEntry; + #[derive(Debug, Clone, PartialEq, Eq)] pub enum SlashCommandSource { Builtin, + Skill, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -23,8 +29,41 @@ pub fn list_builtin_commands() -> Vec { .collect() } -pub fn list_acp_commands() -> Vec { - list_builtin_commands() +pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { + let mut commands = list_builtin_commands(); + let reserved_names: HashSet = commands + .iter() + .map(|command| normalize_command_name(&command.name)) + .collect(); + commands.extend( + skill_commands(crate::skills::list_installed_skills(working_dir)) + .into_iter() + .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))), + ); + commands +} + +fn skill_commands(sources: Vec) -> Vec { + sources + .into_iter() + .filter_map(|source| { + let name = normalize_command_name(&source.name); + if name.is_empty() { + return None; + } + + Some(SlashCommandEntry { + name, + description: source.description, + source: SlashCommandSource::Skill, + input_hint: None, + }) + }) + .collect() +} + +fn normalize_command_name(name: &str) -> String { + name.trim_start_matches('/').to_lowercase() } fn builtin_input_hint(command: &str) -> Option<&'static str> { @@ -38,6 +77,9 @@ fn builtin_input_hint(command: &str) -> Option<&'static str> { #[cfg(test)] mod tests { use super::*; + use goose_sdk::custom_requests::SourceType; + use std::collections::HashMap; + use tempfile::TempDir; #[test] fn lists_acp_safe_builtin_commands() { @@ -79,4 +121,65 @@ mod tests { assert_eq!(prompts.input_hint.as_deref(), Some("[--extension ]")); assert_eq!(compact.input_hint, None); } + + #[test] + fn lists_project_skills_as_acp_commands() { + let tmp = TempDir::new().unwrap(); + let skill_dir = tmp + .path() + .join(".agents") + .join("skills") + .join("code-review"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: code-review\ndescription: Review changed code\n---\nReview the diff.", + ) + .unwrap(); + + let commands = list_acp_commands(Some(tmp.path())); + let command = commands + .iter() + .find(|command| command.name == "code-review") + .expect("project skill should be listed as an ACP command"); + + assert_eq!(command.description, "Review changed code"); + assert_eq!(command.source, SlashCommandSource::Skill); + assert_eq!(command.input_hint, None); + } + + #[test] + fn skill_commands_do_not_override_builtins() { + let reserved_names = list_builtin_commands() + .into_iter() + .map(|command| normalize_command_name(&command.name)) + .collect(); + let commands: Vec<_> = skill_commands(vec![ + source_entry(SourceType::Skill, "compact", "Skill named compact"), + source_entry(SourceType::Skill, "review", "Review code"), + ]) + .into_iter() + .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))) + .collect(); + let names: Vec<_> = commands + .iter() + .map(|command| command.name.as_str()) + .collect(); + + assert_eq!(names, vec!["review"]); + } + + fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { + SourceEntry { + source_type, + name: name.to_string(), + description: description.to_string(), + content: String::new(), + path: String::new(), + global: false, + writable: false, + supporting_files: Vec::new(), + properties: HashMap::new(), + } + } } From eec874cce30c1215d1c79e963131665a7be5c1f1 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 14 May 2026 12:07:12 +1000 Subject: [PATCH 03/17] return recipe slash command in acp response --- crates/goose/src/slash_command.rs | 105 +++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index 1b3b1b95847d..673aed366424 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -6,6 +6,7 @@ use goose_sdk::custom_requests::SourceEntry; #[derive(Debug, Clone, PartialEq, Eq)] pub enum SlashCommandSource { Builtin, + Recipe, Skill, } @@ -31,10 +32,18 @@ pub fn list_builtin_commands() -> Vec { pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { let mut commands = list_builtin_commands(); - let reserved_names: HashSet = commands + let mut reserved_names: HashSet = commands .iter() .map(|command| normalize_command_name(&command.name)) .collect(); + + for command in recipe_commands(crate::slash_commands::list_commands()) { + let name = normalize_command_name(&command.name); + if reserved_names.insert(name) { + commands.push(command); + } + } + commands.extend( skill_commands(crate::skills::list_installed_skills(working_dir)) .into_iter() @@ -43,6 +52,27 @@ pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { commands } +fn recipe_commands( + mappings: Vec, +) -> Vec { + mappings + .into_iter() + .filter_map(|mapping| { + let name = normalize_command_name(&mapping.command); + if name.is_empty() { + return None; + } + + Some(SlashCommandEntry { + name, + description: recipe_description(&mapping.recipe_path), + source: SlashCommandSource::Recipe, + input_hint: None, + }) + }) + .collect() +} + fn skill_commands(sources: Vec) -> Vec { sources .into_iter() @@ -66,6 +96,22 @@ fn normalize_command_name(name: &str) -> String { name.trim_start_matches('/').to_lowercase() } +fn recipe_description(recipe_path: &str) -> String { + let default_description = "Run recipe slash command".to_string(); + let Ok(recipe_content) = std::fs::read_to_string(recipe_path) else { + return default_description; + }; + let Ok(recipe) = crate::recipe::Recipe::from_content(&recipe_content) else { + return default_description; + }; + + if recipe.description.is_empty() { + recipe.title + } else { + recipe.description + } +} + fn builtin_input_hint(command: &str) -> Option<&'static str> { match command { "prompt" => Some(" [--info] [key=value...]"), @@ -169,6 +215,63 @@ mod tests { assert_eq!(names, vec!["review"]); } + #[test] + fn recipe_commands_use_recipe_description() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("review.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", + ) + .unwrap(); + + let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + command: "/review".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].name, "review"); + assert_eq!(commands[0].description, "Review with a recipe"); + assert_eq!(commands[0].source, SlashCommandSource::Recipe); + assert_eq!(commands[0].input_hint, None); + } + + #[test] + fn recipe_commands_reserve_names_before_skills() { + let mut commands = list_builtin_commands(); + let mut reserved_names: HashSet = commands + .iter() + .map(|command| normalize_command_name(&command.name)) + .collect(); + for command in recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + command: "review".to_string(), + recipe_path: "missing.yaml".to_string(), + }]) { + let name = normalize_command_name(&command.name); + if reserved_names.insert(name) { + commands.push(command); + } + } + commands.extend( + skill_commands(vec![source_entry( + SourceType::Skill, + "review", + "Review code", + )]) + .into_iter() + .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))), + ); + + let review_commands: Vec<_> = commands + .iter() + .filter(|command| command.name == "review") + .collect(); + + assert_eq!(review_commands.len(), 1); + assert_eq!(review_commands[0].source, SlashCommandSource::Recipe); + } + fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { SourceEntry { source_type, From a3e78e27d48db963e76e0c8da32d743f0980b4c1 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 14 May 2026 13:38:03 +1000 Subject: [PATCH 04/17] handle the merge conflicts --- crates/goose/src/acp/server.rs | 86 ---------------------- crates/goose/src/slash_command.rs | 115 +++++++++++++++++++++++++----- 2 files changed, 96 insertions(+), 105 deletions(-) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 6ef56a2d5d2a..75840d7ffaba 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -2188,92 +2188,6 @@ impl GooseAcpAgent { Ok(()) } - fn input_hint_for_recipe( - params: Option<&Vec>, - ) -> Option { - let params = params?; - - params - .iter() - .find(|p| p.key == "args") - .or_else(|| params.iter().find(|p| p.default.is_none())) - .or_else(|| params.first()) - .map(|p| p.description.clone()) - } - - async fn build_available_commands_from_slash_commands() -> Vec { - let mut commands = Vec::new(); - - for mapping in crate::slash_commands::list_commands() { - if Self::is_builtin_agent_command(&mapping.command) { - continue; - } - - let recipe_path = std::path::PathBuf::from(&mapping.recipe_path); - - if !recipe_path.exists() { - continue; - } - - let Ok(recipe_content) = tokio::fs::read_to_string(&recipe_path).await else { - continue; - }; - - let Some(recipe_dir) = recipe_path.parent() else { - continue; - }; - - let recipe_dir_str = recipe_dir.display().to_string(); - - let Ok(validation_result) = - crate::recipe::validate_recipe::validate_recipe_template_from_content( - &recipe_content, - Some(recipe_dir_str), - ) - else { - continue; - }; - - let required_param_count = validation_result - .parameters - .as_ref() - .map(|params| params.iter().filter(|p| p.default.is_none()).count()) - .unwrap_or(0); - - if required_param_count > 1 { - continue; - } - - let mut command = - AvailableCommand::new(mapping.command, validation_result.description.clone()); - - if let Some(hint) = Self::input_hint_for_recipe(validation_result.parameters.as_ref()) { - command = command.input(AvailableCommandInput::Unstructured( - UnstructuredCommandInput::new(hint), - )); - } - - commands.push(command); - } - - commands - } - - async fn send_available_commands_update( - &self, - cx: &ConnectionTo, - session_id: &SessionId, - ) -> Result<(), agent_client_protocol::Error> { - let commands = Self::build_available_commands_from_slash_commands().await; - - cx.send_notification(SessionNotification::new( - session_id.clone(), - SessionUpdate::AvailableCommandsUpdate(AvailableCommandsUpdate::new(commands)), - ))?; - - Ok(()) - } - fn is_builtin_agent_command(command: &str) -> bool { let normalized = command.trim_start_matches('/'); diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index 673aed366424..c70db1d067c0 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -63,16 +63,54 @@ fn recipe_commands( return None; } + let metadata = recipe_entry(&mapping.recipe_path)?; + Some(SlashCommandEntry { name, - description: recipe_description(&mapping.recipe_path), + description: metadata.description, source: SlashCommandSource::Recipe, - input_hint: None, + input_hint: metadata.input_hint, }) }) .collect() } +struct RecipeCommandMetadata { + description: String, + input_hint: Option, +} + +fn recipe_entry(recipe_path: &str) -> Option { + let recipe_path = std::path::PathBuf::from(recipe_path); + if !recipe_path.exists() { + return None; + } + + let recipe_content = std::fs::read_to_string(&recipe_path).ok()?; + let recipe_dir = recipe_path.parent()?; + let recipe_dir_str = recipe_dir.display().to_string(); + let validation_result = crate::recipe::validate_recipe::validate_recipe_template_from_content( + &recipe_content, + Some(recipe_dir_str), + ) + .ok()?; + + let required_param_count = validation_result + .parameters + .as_ref() + .map(|params| params.iter().filter(|p| p.default.is_none()).count()) + .unwrap_or(0); + + if required_param_count > 1 { + return None; + } + + Some(RecipeCommandMetadata { + description: validation_result.description, + input_hint: input_hint_for_recipe(validation_result.parameters.as_ref()), + }) +} + fn skill_commands(sources: Vec) -> Vec { sources .into_iter() @@ -96,20 +134,15 @@ fn normalize_command_name(name: &str) -> String { name.trim_start_matches('/').to_lowercase() } -fn recipe_description(recipe_path: &str) -> String { - let default_description = "Run recipe slash command".to_string(); - let Ok(recipe_content) = std::fs::read_to_string(recipe_path) else { - return default_description; - }; - let Ok(recipe) = crate::recipe::Recipe::from_content(&recipe_content) else { - return default_description; - }; - - if recipe.description.is_empty() { - recipe.title - } else { - recipe.description - } +fn input_hint_for_recipe(params: Option<&Vec>) -> Option { + let params = params?; + + params + .iter() + .find(|p| p.key == "args") + .or_else(|| params.iter().find(|p| p.default.is_none())) + .or_else(|| params.first()) + .map(|p| p.description.clone()) } fn builtin_input_hint(command: &str) -> Option<&'static str> { @@ -221,7 +254,7 @@ mod tests { let recipe_path = tmp.path().join("review.yaml"); std::fs::write( &recipe_path, - "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", + "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\nparameters:\n - key: args\n description: Describe what to review\n", ) .unwrap(); @@ -234,11 +267,21 @@ mod tests { assert_eq!(commands[0].name, "review"); assert_eq!(commands[0].description, "Review with a recipe"); assert_eq!(commands[0].source, SlashCommandSource::Recipe); - assert_eq!(commands[0].input_hint, None); + assert_eq!( + commands[0].input_hint.as_deref(), + Some("Describe what to review") + ); } #[test] fn recipe_commands_reserve_names_before_skills() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("review.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", + ) + .unwrap(); let mut commands = list_builtin_commands(); let mut reserved_names: HashSet = commands .iter() @@ -246,7 +289,7 @@ mod tests { .collect(); for command in recipe_commands(vec![crate::slash_commands::SlashCommandMapping { command: "review".to_string(), - recipe_path: "missing.yaml".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), }]) { let name = normalize_command_name(&command.name); if reserved_names.insert(name) { @@ -272,6 +315,40 @@ mod tests { assert_eq!(review_commands[0].source, SlashCommandSource::Recipe); } + #[test] + fn recipe_commands_skip_missing_invalid_and_multi_required_param_recipes() { + let tmp = TempDir::new().unwrap(); + let invalid_recipe_path = tmp.path().join("invalid.yaml"); + std::fs::write(&invalid_recipe_path, "not: a recipe").unwrap(); + let multi_param_recipe_path = tmp.path().join("multi.yaml"); + std::fs::write( + &multi_param_recipe_path, + "version: 1.0.0\ntitle: Multi Param Recipe\ndescription: Has too many required params\ninstructions: Review the change\nparameters:\n - key: first\n description: First param\n - key: second\n description: Second param\n", + ) + .unwrap(); + + let commands = recipe_commands(vec![ + crate::slash_commands::SlashCommandMapping { + command: "missing".to_string(), + recipe_path: tmp + .path() + .join("missing.yaml") + .to_string_lossy() + .to_string(), + }, + crate::slash_commands::SlashCommandMapping { + command: "invalid".to_string(), + recipe_path: invalid_recipe_path.to_string_lossy().to_string(), + }, + crate::slash_commands::SlashCommandMapping { + command: "multi".to_string(), + recipe_path: multi_param_recipe_path.to_string_lossy().to_string(), + }, + ]); + + assert!(commands.is_empty()); + } + fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { SourceEntry { source_type, From 29730d20a4fbee749857e0d1d674acb02fa65dfb Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 14 May 2026 17:36:49 +1000 Subject: [PATCH 05/17] parse skill argument-hints and arguments --- crates/goose/src/skills/mod.rs | 25 +++++++++++++++++++++++++ crates/goose/src/slash_command.rs | 6 +++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/goose/src/skills/mod.rs b/crates/goose/src/skills/mod.rs index 1409696fef0b..fdda453b9eff 100644 --- a/crates/goose/src/skills/mod.rs +++ b/crates/goose/src/skills/mod.rs @@ -125,6 +125,31 @@ pub fn render_loaded_skill(skill: &SourceEntry) -> String { output } +pub fn skill_argument_hint(skill: &SourceEntry) -> Option { + skill + .properties + .get("argument-hint") + .and_then(|value| value.as_str()) + .filter(|hint| !hint.is_empty()) + .map(str::to_string) +} + +pub fn skill_argument_names(skill: &SourceEntry) -> Vec { + skill + .properties + .get("arguments") + .and_then(|value| value.as_array()) + .map(|items| { + items + .iter() + .filter_map(|item| item.as_str()) + .filter(|name| !name.is_empty()) + .map(str::to_string) + .collect() + }) + .unwrap_or_default() +} + fn canonicalize_or_original(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) } diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index c70db1d067c0..1f230e34469d 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -124,7 +124,7 @@ fn skill_commands(sources: Vec) -> Vec { name, description: source.description, source: SlashCommandSource::Skill, - input_hint: None, + input_hint: crate::skills::skill_argument_hint(&source), }) }) .collect() @@ -212,7 +212,7 @@ mod tests { std::fs::create_dir_all(&skill_dir).unwrap(); std::fs::write( skill_dir.join("SKILL.md"), - "---\nname: code-review\ndescription: Review changed code\n---\nReview the diff.", + "---\nname: code-review\ndescription: Review changed code\nmetadata:\n argument-hint: \"[task]\"\n arguments:\n - task\n---\nReview the diff.", ) .unwrap(); @@ -224,7 +224,7 @@ mod tests { assert_eq!(command.description, "Review changed code"); assert_eq!(command.source, SlashCommandSource::Skill); - assert_eq!(command.input_hint, None); + assert_eq!(command.input_hint.as_deref(), Some("[task]")); } #[test] From fffba4e0ddb2d3adbb7075a1bbd395bce1548fca Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 14 May 2026 18:07:15 +1000 Subject: [PATCH 06/17] parse skill command arguments and substitute --- crates/goose-cli/src/commands/configure.rs | 2 +- crates/goose-cli/src/session/mod.rs | 42 +----- crates/goose/src/agents/execute_commands.rs | 3 +- crates/goose/src/skills/client.rs | 20 ++- crates/goose/src/skills/mod.rs | 136 ++++++++++++++++++++ crates/goose/src/slash_command.rs | 5 +- crates/goose/src/utils.rs | 63 +++++++++ 7 files changed, 228 insertions(+), 43 deletions(-) diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index c682724276cd..6fcec0567c8a 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -1110,7 +1110,7 @@ fn configure_stdio_extension() -> anyhow::Result<()> { let timeout = prompt_extension_timeout()?; - let mut parts = crate::session::split_quoted(&command_str)?; + let mut parts = goose::utils::split_command_args(&command_str)?; let cmd = if parts.is_empty() { String::new() } else { diff --git a/crates/goose-cli/src/session/mod.rs b/crates/goose-cli/src/session/mod.rs index 8093a007ce2e..11713ed9f520 100644 --- a/crates/goose-cli/src/session/mod.rs +++ b/crates/goose-cli/src/session/mod.rs @@ -239,36 +239,6 @@ pub async fn classify_planner_response( } } -pub fn split_quoted(input: &str) -> Result> { - let mut parts = Vec::new(); - let mut current = String::new(); - let mut in_double_quote = false; - let mut in_single_quote = false; - - for c in input.chars() { - match c { - '"' if !in_single_quote => in_double_quote = !in_double_quote, - '\'' if !in_double_quote => in_single_quote = !in_single_quote, - c if c.is_whitespace() && !in_double_quote && !in_single_quote => { - if !current.is_empty() { - parts.push(std::mem::take(&mut current)); - } - } - _ => current.push(c), - } - } - - if in_double_quote || in_single_quote { - return Err(anyhow::anyhow!("Unmatched quote in command")); - } - - if !current.is_empty() { - parts.push(current); - } - - Ok(parts) -} - impl CliSession { #[allow(clippy::too_many_arguments)] pub async fn new( @@ -311,7 +281,7 @@ impl CliSession { /// Parse a stdio extension command string into an ExtensionConfig /// Format: "ENV1=val1 ENV2=val2 command args..." pub fn parse_stdio_extension(extension_command: &str) -> Result { - let mut parts = split_quoted(extension_command)?; + let mut parts = goose::utils::split_command_args(extension_command)?; let mut envs = HashMap::new(); while let Some(part) = parts.first() { @@ -2250,20 +2220,20 @@ mod tests { } #[test] - fn test_split_quoted_windows_paths() { + fn test_split_command_args_windows_paths() { assert_eq!( - split_quoted(r"C:\tools\mcp.exe --arg value").unwrap(), + goose::utils::split_command_args(r"C:\tools\mcp.exe --arg value").unwrap(), vec![r"C:\tools\mcp.exe", "--arg", "value"] ); assert_eq!( - split_quoted(r#""C:\Program Files\server\mcp.exe" --arg"#).unwrap(), + goose::utils::split_command_args(r#""C:\Program Files\server\mcp.exe" --arg"#).unwrap(), vec![r"C:\Program Files\server\mcp.exe", "--arg"] ); } #[test] - fn test_split_quoted_unmatched_quote() { - assert!(split_quoted(r#""unmatched"#).is_err()); + fn test_split_command_args_unmatched_quote() { + assert!(goose::utils::split_command_args(r#""unmatched"#).is_err()); } #[test_case( diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 076efb1cc54e..2a3fe34dbb47 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -5,6 +5,7 @@ use anyhow::{anyhow, Result}; use crate::context_mgmt::compact_messages; use crate::conversation::message::{Message, SystemNotificationType}; use crate::recipe::build_recipe::build_recipe_from_template_with_positional_params; +use crate::skills::render_loaded_skill_with_args; use super::Agent; @@ -499,7 +500,7 @@ impl Agent { let prompt = format!( "The user invoked the Goose skill `{}`.\n\n{}\n\nUser request:\n{}", skill.name, - crate::skills::render_loaded_skill(&skill), + render_loaded_skill_with_args(&skill, (!params_str.is_empty()).then_some(params_str))?, task ); diff --git a/crates/goose/src/skills/client.rs b/crates/goose/src/skills/client.rs index 65ae64206ab5..53a8574177b3 100644 --- a/crates/goose/src/skills/client.rs +++ b/crates/goose/src/skills/client.rs @@ -1,4 +1,5 @@ use super::discover_skills; +use super::render_loaded_skill_with_args; use crate::agents::extension::PlatformExtensionContext; use crate::agents::mcp_client::{Error, McpClientTrait}; use crate::agents::ToolCallContext; @@ -71,6 +72,10 @@ impl McpClientTrait for SkillsClient { "name": { "type": "string", "description": "Name of the skill to load. Use \"skill-name/path\" to load a supporting file." + }, + "args": { + "type": "string", + "description": "Optional arguments to provide when loading the skill." } } }); @@ -82,6 +87,7 @@ impl McpClientTrait for SkillsClient { load it first to get the detailed instructions.\n\n\ Examples:\n\ - load_skill(name: \"gdrive\") → Loads the gdrive skill instructions\n\ + - load_skill(name: \"my-skill\", args: \"the arguments for the skill\") → Loads a skill with arguments\n\ - load_skill(name: \"my-skill/template.md\") → Loads a supporting file" .to_string(), schema.as_object().unwrap().clone(), @@ -119,13 +125,21 @@ impl McpClientTrait for SkillsClient { "Missing required parameter: name", )])); } + let args = arguments + .as_ref() + .and_then(|args| args.get("args")) + .and_then(|v| v.as_str()); let skills = discover_skills(Some(&self.working_dir)); if let Some(skill) = skills.iter().find(|s| s.name == skill_name) { - return Ok(CallToolResult::success(vec![Content::text( - crate::skills::render_loaded_skill(skill), - )])); + return match render_loaded_skill_with_args(skill, args) { + Ok(rendered) => Ok(CallToolResult::success(vec![Content::text(rendered)])), + Err(e) => Ok(CallToolResult::error(vec![Content::text(format!( + "Failed to parse skill arguments: {}", + e + ))])), + }; } if let Some((parent_skill_name, raw_relative_path)) = skill_name.split_once('/') { diff --git a/crates/goose/src/skills/mod.rs b/crates/goose/src/skills/mod.rs index fdda453b9eff..ca3a51b67ebf 100644 --- a/crates/goose/src/skills/mod.rs +++ b/crates/goose/src/skills/mod.rs @@ -11,11 +11,14 @@ use crate::config::paths::Paths; use crate::plugins::installed_plugin_skill_dirs; use crate::sources::parse_frontmatter; use agent_client_protocol::Error; +use anyhow::Result; use goose_sdk::custom_requests::{SourceEntry, SourceType}; +use regex::{Captures, Regex}; use serde::Deserialize; use serde_json::Value; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; +use std::sync::LazyLock; use tracing::warn; #[derive(Debug, Deserialize)] @@ -125,6 +128,17 @@ pub fn render_loaded_skill(skill: &SourceEntry) -> String { output } +pub fn render_loaded_skill_with_args(skill: &SourceEntry, args: Option<&str>) -> Result { + let mut rendered_skill = skill.clone(); + rendered_skill.content = render_skill_content_with_args( + &skill.content, + args.unwrap_or_default(), + &skill_argument_names(skill), + )?; + + Ok(render_loaded_skill(&rendered_skill)) +} + pub fn skill_argument_hint(skill: &SourceEntry) -> Option { skill .properties @@ -150,6 +164,66 @@ pub fn skill_argument_names(skill: &SourceEntry) -> Vec { .unwrap_or_default() } +fn render_skill_content_with_args( + content: &str, + raw_args: &str, + argument_names: &[String], +) -> Result { + static VARIABLE_RE: LazyLock = LazyLock::new(|| { + Regex::new(r"\$ARGUMENTS(?:\[(\d+)\])?|\$(\d+)|\$([A-Za-z_][A-Za-z0-9_-]*)") + .expect("skill argument variable regex should compile") + }); + + let values = crate::utils::split_command_args(raw_args)?; + let named_values: HashMap<&str, &str> = argument_names + .iter() + .zip(values.iter()) + .map(|(name, value)| (name.as_str(), value.as_str())) + .collect(); + + Ok(VARIABLE_RE + .replace_all(content, |captures: &Captures<'_>| { + if let Some(index) = captures.get(1) { + return index + .as_str() + .parse::() + .ok() + .and_then(|index| values.get(index)) + .cloned() + .unwrap_or_default(); + } + + if captures + .get(0) + .is_some_and(|value| value.as_str() == "$ARGUMENTS") + { + return raw_args.to_string(); + } + + if let Some(position) = captures.get(2) { + return position + .as_str() + .parse::() + .ok() + .and_then(|position| position.checked_sub(1)) + .and_then(|index| values.get(index)) + .cloned() + .unwrap_or_default(); + } + + if let Some(name) = captures.get(3) { + return named_values + .get(name.as_str()) + .copied() + .unwrap_or_default() + .to_string(); + } + + String::new() + }) + .into_owned()) +} + fn canonicalize_or_original(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) } @@ -479,3 +553,65 @@ pub fn list_installed_skills(working_dir: Option<&Path>) -> Vec { }; discover_skills(wd) } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn skill_with_content(content: &str) -> SourceEntry { + SourceEntry { + source_type: SourceType::Skill, + name: "test-skill".to_string(), + description: "Test skill".to_string(), + content: content.to_string(), + path: String::new(), + global: false, + writable: true, + supporting_files: Vec::new(), + properties: HashMap::from([( + "arguments".to_string(), + json!(["component", "from", "to"]), + )]), + } + } + + #[test] + fn render_loaded_skill_with_args_replaces_positional_and_named_values() { + let skill = + skill_with_content("Migrate $component from $from to $to. First: $1. Raw: $ARGUMENTS."); + + let rendered = + render_loaded_skill_with_args(&skill, Some(r#""Button Group" old-lib new-lib"#)) + .unwrap(); + + assert!(rendered.contains( + "Migrate Button Group from old-lib to new-lib. First: Button Group. Raw: \"Button Group\" old-lib new-lib." + )); + } + + #[test] + fn render_loaded_skill_with_args_replaces_argument_indexes() { + let skill = skill_with_content("Second: $ARGUMENTS[1]. Missing: '$ARGUMENTS[9]' '$4'."); + + let rendered = render_loaded_skill_with_args(&skill, Some("one two three")).unwrap(); + + assert!(rendered.contains("Second: two. Missing: '' ''.")); + } + + #[test] + fn render_loaded_skill_with_args_preserves_windows_paths() { + let skill = skill_with_content("Path: $component"); + + let rendered = render_loaded_skill_with_args(&skill, Some(r#""C:\path\""#)).unwrap(); + + assert!(rendered.contains(r"Path: C:\path\")); + } + + #[test] + fn render_loaded_skill_with_args_returns_unmatched_quote_error() { + let skill = skill_with_content("Path: $component"); + + assert!(render_loaded_skill_with_args(&skill, Some(r#""unterminated"#)).is_err()); + } +} diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index 1f230e34469d..27fe5c987dd3 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -119,12 +119,13 @@ fn skill_commands(sources: Vec) -> Vec { if name.is_empty() { return None; } + let input_hint = crate::skills::skill_argument_hint(&source); Some(SlashCommandEntry { name, description: source.description, source: SlashCommandSource::Skill, - input_hint: crate::skills::skill_argument_hint(&source), + input_hint, }) }) .collect() @@ -229,7 +230,7 @@ mod tests { #[test] fn skill_commands_do_not_override_builtins() { - let reserved_names = list_builtin_commands() + let reserved_names: HashSet = list_builtin_commands() .into_iter() .map(|command| normalize_command_name(&command.name)) .collect(); diff --git a/crates/goose/src/utils.rs b/crates/goose/src/utils.rs index c165928c8492..9ca485eb15cf 100644 --- a/crates/goose/src/utils.rs +++ b/crates/goose/src/utils.rs @@ -47,6 +47,36 @@ pub fn is_token_cancelled(cancellation_token: &Option) -> boo .is_some_and(|t| t.is_cancelled()) } +pub fn split_command_args(input: &str) -> anyhow::Result> { + let mut parts = Vec::new(); + let mut current = String::new(); + let mut in_double_quote = false; + let mut in_single_quote = false; + + for c in input.chars() { + match c { + '"' if !in_single_quote => in_double_quote = !in_double_quote, + '\'' if !in_double_quote => in_single_quote = !in_single_quote, + c if c.is_whitespace() && !in_double_quote && !in_single_quote => { + if !current.is_empty() { + parts.push(std::mem::take(&mut current)); + } + } + _ => current.push(c), + } + } + + if in_double_quote || in_single_quote { + return Err(anyhow::anyhow!("Unmatched quote in command")); + } + + if !current.is_empty() { + parts.push(current); + } + + Ok(parts) +} + #[cfg(test)] mod tests { use super::*; @@ -125,4 +155,37 @@ mod tests { assert_eq!(safe_truncate(mixed, 20), mixed); assert_eq!(safe_truncate(mixed, 8), "Hello..."); } + + #[test] + fn test_split_command_args_windows_paths() { + assert_eq!( + split_command_args(r"C:\tools\mcp.exe --arg value").unwrap(), + vec![r"C:\tools\mcp.exe", "--arg", "value"] + ); + assert_eq!( + split_command_args(r#""C:\Program Files\server\mcp.exe" --arg"#).unwrap(), + vec![r"C:\Program Files\server\mcp.exe", "--arg"] + ); + assert_eq!( + split_command_args(r#""C:\path\" next"#).unwrap(), + vec![r"C:\path\", "next"] + ); + } + + #[test] + fn test_split_command_args_quotes() { + assert_eq!( + split_command_args(r#""Button Group" old-lib new-lib"#).unwrap(), + vec!["Button Group", "old-lib", "new-lib"] + ); + assert_eq!( + split_command_args(r#"'my name "abc"' second third"#).unwrap(), + vec![r#"my name "abc""#, "second", "third"] + ); + } + + #[test] + fn test_split_command_args_unmatched_quote() { + assert!(split_command_args(r#""unmatched"#).is_err()); + } } From 4b8f1a81ef9112dbabdb125ebd51566fb4ff61c2 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 14 May 2026 21:27:48 +1000 Subject: [PATCH 07/17] better skill argument handling --- crates/goose/src/agents/execute_commands.rs | 16 +- crates/goose/src/skills/arguments.rs | 232 ++++++++++++++++++++ crates/goose/src/skills/client.rs | 4 +- crates/goose/src/skills/mod.rs | 126 ++--------- 4 files changed, 259 insertions(+), 119 deletions(-) create mode 100644 crates/goose/src/skills/arguments.rs diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 2a3fe34dbb47..82636b11d67d 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -5,7 +5,7 @@ use anyhow::{anyhow, Result}; use crate::context_mgmt::compact_messages; use crate::conversation::message::{Message, SystemNotificationType}; use crate::recipe::build_recipe::build_recipe_from_template_with_positional_params; -use crate::skills::render_loaded_skill_with_args; +use crate::skills::loaded_skill_context_with_args; use super::Agent; @@ -491,18 +491,8 @@ impl Agent { return Ok(None); }; - let task = if params_str.is_empty() { - "Use this skill for the current task." - } else { - params_str - }; - - let prompt = format!( - "The user invoked the Goose skill `{}`.\n\n{}\n\nUser request:\n{}", - skill.name, - render_loaded_skill_with_args(&skill, (!params_str.is_empty()).then_some(params_str))?, - task - ); + let prompt = + loaded_skill_context_with_args(&skill, (!params_str.is_empty()).then_some(params_str))?; Ok(Some(Message::user().with_text(prompt))) } diff --git a/crates/goose/src/skills/arguments.rs b/crates/goose/src/skills/arguments.rs new file mode 100644 index 000000000000..07a3eb86c681 --- /dev/null +++ b/crates/goose/src/skills/arguments.rs @@ -0,0 +1,232 @@ +use crate::utils::split_command_args; +use anyhow::Result; +use regex::{Captures, Regex}; +use std::sync::LazyLock; + +static PLACEHOLDER_RE: LazyLock = LazyLock::new(|| { + Regex::new( + r"\$ARGUMENTS\[(?P\d+)\]|\$ARGUMENTS\b|\$(?P\d+)|\$(?P[A-Za-z_][A-Za-z0-9_-]*)", + ) + .expect("skill argument regex should compile") +}); + +fn is_resolvable(caps: &Captures<'_>, names: &[String]) -> bool { + caps.name("name") + .map(|m| names.iter().any(|n| n == m.as_str())) + .unwrap_or(true) +} + +pub(super) fn apply_skill_arguments( + content: &str, + raw_args: &str, + argument_names: &[String], +) -> Result { + if !PLACEHOLDER_RE + .captures_iter(content) + .any(|caps| is_resolvable(&caps, argument_names)) + { + return Ok(format!("{content}\n\nARGUMENTS: {raw_args}")); + } + + let tokens = split_command_args(raw_args)?; + let nth = |i: usize| tokens.get(i).cloned().unwrap_or_default(); + + let rendered = PLACEHOLDER_RE.replace_all(content, |caps: &Captures<'_>| { + if let Some(n) = caps.name("idx") { + return nth(n.as_str().parse().unwrap_or(usize::MAX)); + } + if let Some(n) = caps.name("pos") { + let p: usize = n.as_str().parse().unwrap_or(0); + return p.checked_sub(1).map_or_else(String::new, nth); + } + if let Some(name) = caps.name("name") { + return argument_names + .iter() + .position(|n| n == name.as_str()) + .map_or_else(|| caps[0].to_string(), nth); + } + raw_args.to_string() + }); + + Ok(rendered.into_owned()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn names(items: &[&str]) -> Vec { + items.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn arguments_placeholder_is_replaced_with_raw_args() { + let out = apply_skill_arguments("Run $ARGUMENTS now.", "alpha beta", &[]).unwrap(); + assert_eq!(out, "Run alpha beta now."); + } + + #[test] + fn positional_is_one_indexed() { + let out = apply_skill_arguments("first=$1, second=$2, third=$3", "alpha beta gamma", &[]) + .unwrap(); + assert_eq!(out, "first=alpha, second=beta, third=gamma"); + } + + #[test] + fn positional_out_of_range_is_empty() { + let out = apply_skill_arguments("[$5]", "only-one", &[]).unwrap(); + assert_eq!(out, "[]"); + } + + #[test] + fn arguments_index_is_zero_indexed() { + let out = apply_skill_arguments( + "a=$ARGUMENTS[0], b=$ARGUMENTS[1], c=$ARGUMENTS[2]", + "one two three", + &[], + ) + .unwrap(); + assert_eq!(out, "a=one, b=two, c=three"); + } + + #[test] + fn arguments_index_out_of_range_is_empty() { + let out = apply_skill_arguments("[$ARGUMENTS[9]]", "one two", &[]).unwrap(); + assert_eq!(out, "[]"); + } + + #[test] + fn named_arg_maps_to_position() { + let out = apply_skill_arguments( + "Migrate $component from $from to $to.", + "Button old-lib new-lib", + &names(&["component", "from", "to"]), + ) + .unwrap(); + assert_eq!(out, "Migrate Button from old-lib to new-lib."); + } + + #[test] + fn undeclared_named_arg_stays_literal() { + let out = + apply_skill_arguments("Hello $first $missing", "Andy", &names(&["first"])).unwrap(); + assert_eq!(out, "Hello Andy $missing"); + } + + #[test] + fn quoted_token_groups_multiple_words() { + let out = apply_skill_arguments( + "name=$name, addr=$addr", + r#"Andy "57 Collins""#, + &names(&["name", "addr"]), + ) + .unwrap(); + assert_eq!(out, "name=Andy, addr=57 Collins"); + } + + #[test] + fn arguments_keeps_raw_quotes_while_positional_strips_them() { + let out = + apply_skill_arguments("raw=$ARGUMENTS pos=$1", r#""hello world" tail"#, &[]).unwrap(); + assert_eq!(out, r#"raw="hello world" tail pos=hello world"#); + } + + #[test] + fn no_placeholder_appends_arguments_marker() { + let out = apply_skill_arguments("Just static instructions.", "src/foo.rs", &[]).unwrap(); + assert_eq!(out, "Just static instructions.\n\nARGUMENTS: src/foo.rs"); + } + + #[test] + fn undeclared_named_only_does_not_count_as_placeholder() { + let out = + apply_skill_arguments("Just $missing here.", "src/foo.rs", &names(&["other"])).unwrap(); + assert_eq!(out, "Just $missing here.\n\nARGUMENTS: src/foo.rs"); + } + + #[test] + fn unmatched_quote_returns_error_when_placeholders_present() { + let result = apply_skill_arguments("$1", r#""unterminated"#, &[]); + assert!(result.is_err()); + } + + #[test] + fn unmatched_quote_is_fine_when_no_placeholders_present() { + let out = apply_skill_arguments("no placeholders", r#""unterminated"#, &[]).unwrap(); + assert!(out.ends_with(r#"ARGUMENTS: "unterminated"#)); + } + + #[test] + fn arguments_followed_by_non_index_bracket_is_replaced() { + let out = apply_skill_arguments("$ARGUMENTS[abc]", "x y", &[]).unwrap(); + assert_eq!(out, "x y[abc]"); + } + + #[test] + fn extra_tokens_beyond_named_args_reachable_via_positional() { + let out = + apply_skill_arguments("$first / $1 / $2 / $3", "a b c", &names(&["first"])).unwrap(); + assert_eq!(out, "a / a / b / c"); + } + + #[test] + fn declared_name_without_token_substitutes_empty() { + let out = apply_skill_arguments("[$first][$second]", "only", &names(&["first", "second"])) + .unwrap(); + assert_eq!(out, "[only][]"); + } + + #[test] + fn dollar_sign_with_no_match_stays_literal() { + let out = apply_skill_arguments("price is $100USD or $$ or $", "ignored", &[]).unwrap(); + assert!(out.contains("price is ")); + assert!(out.contains("$$")); + } + + #[test] + fn empty_args_with_arguments_placeholder_substitutes_empty() { + let out = apply_skill_arguments("Run $ARGUMENTS done.", "", &[]).unwrap(); + assert_eq!(out, "Run done."); + } + + #[test] + fn empty_args_with_no_placeholder_still_appends_marker() { + let out = apply_skill_arguments("Just instructions.", "", &[]).unwrap(); + assert_eq!(out, "Just instructions.\n\nARGUMENTS: "); + } + + #[test] + fn repeated_positional_substitutes_every_occurrence() { + let out = apply_skill_arguments("$1 $1 $1", "alpha", &[]).unwrap(); + assert_eq!(out, "alpha alpha alpha"); + } + + #[test] + fn multiple_arguments_placeholders_substitute_every_occurrence() { + let out = apply_skill_arguments("first=$ARGUMENTS / again=$ARGUMENTS", "a b", &[]).unwrap(); + assert_eq!(out, "first=a b / again=a b"); + } + + #[test] + fn preserves_windows_backslash_paths() { + let out = apply_skill_arguments("Path: $path", r#""C:\path\""#, &names(&["path"])).unwrap(); + assert_eq!(out, r"Path: C:\path\"); + } + + #[test] + fn adjacent_positional_placeholders_substitute_independently() { + let out = apply_skill_arguments("$1$2", "alpha beta", &[]).unwrap(); + assert_eq!(out, "alphabeta"); + } + + #[test] + fn hyphenated_and_underscored_named_args_resolve() { + let out = apply_skill_arguments( + "$my-arg / $_internal", + "foo bar", + &names(&["my-arg", "_internal"]), + ) + .unwrap(); + assert_eq!(out, "foo / bar"); + } +} diff --git a/crates/goose/src/skills/client.rs b/crates/goose/src/skills/client.rs index 53a8574177b3..ca68434c44a5 100644 --- a/crates/goose/src/skills/client.rs +++ b/crates/goose/src/skills/client.rs @@ -1,5 +1,5 @@ use super::discover_skills; -use super::render_loaded_skill_with_args; +use super::loaded_skill_context_with_args; use crate::agents::extension::PlatformExtensionContext; use crate::agents::mcp_client::{Error, McpClientTrait}; use crate::agents::ToolCallContext; @@ -133,7 +133,7 @@ impl McpClientTrait for SkillsClient { let skills = discover_skills(Some(&self.working_dir)); if let Some(skill) = skills.iter().find(|s| s.name == skill_name) { - return match render_loaded_skill_with_args(skill, args) { + return match loaded_skill_context_with_args(skill, args) { Ok(rendered) => Ok(CallToolResult::success(vec![Content::text(rendered)])), Err(e) => Ok(CallToolResult::error(vec![Content::text(format!( "Failed to parse skill arguments: {}", diff --git a/crates/goose/src/skills/mod.rs b/crates/goose/src/skills/mod.rs index ca3a51b67ebf..4b827a5a0da9 100644 --- a/crates/goose/src/skills/mod.rs +++ b/crates/goose/src/skills/mod.rs @@ -2,6 +2,7 @@ //! built-ins) and the runtime MCP client (`client` submodule). User-facing //! CRUD lives in `crate::sources`, which generalizes across source types. +mod arguments; mod builtin; pub mod client; @@ -12,13 +13,12 @@ use crate::plugins::installed_plugin_skill_dirs; use crate::sources::parse_frontmatter; use agent_client_protocol::Error; use anyhow::Result; +use arguments::apply_skill_arguments; use goose_sdk::custom_requests::{SourceEntry, SourceType}; -use regex::{Captures, Regex}; use serde::Deserialize; use serde_json::Value; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; -use std::sync::LazyLock; use tracing::warn; #[derive(Debug, Deserialize)] @@ -99,12 +99,11 @@ pub(crate) fn validate_skill_name(name: &str) -> Result<(), Error> { Ok(()) } -pub fn render_loaded_skill(skill: &SourceEntry) -> String { +fn loaded_skill_context(skill: &SourceEntry, content: &str) -> String { + let title = format!("{} ({})", skill.name, skill.source_type); let mut output = format!( - "# Loaded Skill: {} ({})\n\n{}\n", - skill.name, - skill.source_type, - skill.to_load_text() + "# Loaded Skill: {title}\n\n{}\n\n## Content\n\n{}\n", + skill.description, content ); if !skill.supporting_files.is_empty() { @@ -128,15 +127,14 @@ pub fn render_loaded_skill(skill: &SourceEntry) -> String { output } -pub fn render_loaded_skill_with_args(skill: &SourceEntry, args: Option<&str>) -> Result { - let mut rendered_skill = skill.clone(); - rendered_skill.content = render_skill_content_with_args( - &skill.content, - args.unwrap_or_default(), - &skill_argument_names(skill), - )?; +pub fn loaded_skill_context_with_args(skill: &SourceEntry, args: Option<&str>) -> Result { + let content = if let Some(args) = args { + apply_skill_arguments(&skill.content, args, &skill_argument_names(skill))? + } else { + skill.content.clone() + }; - Ok(render_loaded_skill(&rendered_skill)) + Ok(loaded_skill_context(skill, &content)) } pub fn skill_argument_hint(skill: &SourceEntry) -> Option { @@ -164,66 +162,6 @@ pub fn skill_argument_names(skill: &SourceEntry) -> Vec { .unwrap_or_default() } -fn render_skill_content_with_args( - content: &str, - raw_args: &str, - argument_names: &[String], -) -> Result { - static VARIABLE_RE: LazyLock = LazyLock::new(|| { - Regex::new(r"\$ARGUMENTS(?:\[(\d+)\])?|\$(\d+)|\$([A-Za-z_][A-Za-z0-9_-]*)") - .expect("skill argument variable regex should compile") - }); - - let values = crate::utils::split_command_args(raw_args)?; - let named_values: HashMap<&str, &str> = argument_names - .iter() - .zip(values.iter()) - .map(|(name, value)| (name.as_str(), value.as_str())) - .collect(); - - Ok(VARIABLE_RE - .replace_all(content, |captures: &Captures<'_>| { - if let Some(index) = captures.get(1) { - return index - .as_str() - .parse::() - .ok() - .and_then(|index| values.get(index)) - .cloned() - .unwrap_or_default(); - } - - if captures - .get(0) - .is_some_and(|value| value.as_str() == "$ARGUMENTS") - { - return raw_args.to_string(); - } - - if let Some(position) = captures.get(2) { - return position - .as_str() - .parse::() - .ok() - .and_then(|position| position.checked_sub(1)) - .and_then(|index| values.get(index)) - .cloned() - .unwrap_or_default(); - } - - if let Some(name) = captures.get(3) { - return named_values - .get(name.as_str()) - .copied() - .unwrap_or_default() - .to_string(); - } - - String::new() - }) - .into_owned()) -} - fn canonicalize_or_original(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) } @@ -577,41 +515,21 @@ mod tests { } #[test] - fn render_loaded_skill_with_args_replaces_positional_and_named_values() { - let skill = - skill_with_content("Migrate $component from $from to $to. First: $1. Raw: $ARGUMENTS."); - - let rendered = - render_loaded_skill_with_args(&skill, Some(r#""Button Group" old-lib new-lib"#)) - .unwrap(); - - assert!(rendered.contains( - "Migrate Button Group from old-lib to new-lib. First: Button Group. Raw: \"Button Group\" old-lib new-lib." - )); - } - - #[test] - fn render_loaded_skill_with_args_replaces_argument_indexes() { - let skill = skill_with_content("Second: $ARGUMENTS[1]. Missing: '$ARGUMENTS[9]' '$4'."); + fn loaded_skill_context_with_args_replaces_arguments_placeholder_with_raw_args() { + let skill = skill_with_content("Review $ARGUMENTS carefully."); - let rendered = render_loaded_skill_with_args(&skill, Some("one two three")).unwrap(); + let rendered = loaded_skill_context_with_args(&skill, Some("src/foo.rs --strict")).unwrap(); - assert!(rendered.contains("Second: two. Missing: '' ''.")); + assert!(rendered.contains("Review src/foo.rs --strict carefully.")); } #[test] - fn render_loaded_skill_with_args_preserves_windows_paths() { - let skill = skill_with_content("Path: $component"); - - let rendered = render_loaded_skill_with_args(&skill, Some(r#""C:\path\""#)).unwrap(); + fn loaded_skill_context_with_args_uses_context_without_args() { + let skill = skill_with_content("Review the code carefully."); - assert!(rendered.contains(r"Path: C:\path\")); - } - - #[test] - fn render_loaded_skill_with_args_returns_unmatched_quote_error() { - let skill = skill_with_content("Path: $component"); + let rendered = loaded_skill_context_with_args(&skill, None).unwrap(); - assert!(render_loaded_skill_with_args(&skill, Some(r#""unterminated"#)).is_err()); + assert!(rendered.contains("# Loaded Skill: test-skill (skill)")); + assert!(rendered.contains("## Content\n\nReview the code carefully.")); } } From 83580cc3a2c7b80d65444c30e81aa0e28d1313fa Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 00:10:24 +1000 Subject: [PATCH 08/17] recipe arguments --- crates/goose/src/agents/execute_commands.rs | 223 +++++++++++++++----- crates/goose/src/slash_command.rs | 141 ++++++++++--- crates/goose/src/slash_commands.rs | 13 -- 3 files changed, 285 insertions(+), 92 deletions(-) diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 82636b11d67d..2eb41b175ec6 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -4,7 +4,8 @@ use anyhow::{anyhow, Result}; use crate::context_mgmt::compact_messages; use crate::conversation::message::{Message, SystemNotificationType}; -use crate::recipe::build_recipe::build_recipe_from_template_with_positional_params; +use crate::recipe::build_recipe::build_recipe_from_template; +use crate::recipe::{RecipeParameter, RecipeParameterRequirement}; use crate::skills::loaded_skill_context_with_args; use super::Agent; @@ -49,6 +50,51 @@ pub struct ParsedSlashCommand<'a> { pub params_str: &'a str, } +fn parse_recipe_args( + params_str: &str, + required: &[&RecipeParameter], + optional: &[&RecipeParameter], +) -> Result> { + use std::collections::HashSet; + + let tokens = crate::utils::split_command_args(params_str)?; + let known_keys: HashSet<&str> = required + .iter() + .chain(optional.iter()) + .map(|p| p.key.as_str()) + .collect(); + + let mut result = Vec::new(); + let mut required_idx = 0; + let mut i = 0; + + while i < tokens.len() { + let token = &tokens[i]; + if let Some(flag) = token.strip_prefix("--") { + if !known_keys.contains(flag) { + return Err(anyhow!("Unknown parameter: --{}", flag)); + } + let value = tokens + .get(i + 1) + .ok_or_else(|| anyhow!("Missing value for --{}", flag))?; + if value.starts_with("--") { + return Err(anyhow!("Missing value for --{}", flag)); + } + result.push((flag.to_string(), value.clone())); + i += 2; + } else { + if required_idx >= required.len() { + return Err(anyhow!("Unexpected positional argument: {}", token)); + } + result.push((required[required_idx].key.clone(), token.clone())); + required_idx += 1; + i += 1; + } + } + + Ok(result) +} + pub fn parse_slash_command(message_text: &str) -> Option> { let mut trimmed = message_text.trim(); @@ -388,58 +434,36 @@ impl Agent { ) .map_err(|e| anyhow!("Failed to parse recipe: {}", e))?; - let param_values: Vec = if params_str.is_empty() { + let empty_params: Vec = Vec::new(); + let all_params = validation_result + .parameters + .as_ref() + .unwrap_or(&empty_params); + let required: Vec<&RecipeParameter> = all_params + .iter() + .filter(|p| { + matches!( + p.requirement, + RecipeParameterRequirement::Required | RecipeParameterRequirement::UserPrompt + ) + }) + .collect(); + let optional: Vec<&RecipeParameter> = all_params + .iter() + .filter(|p| matches!(p.requirement, RecipeParameterRequirement::Optional)) + .collect(); + + let param_values: Vec<(String, String)> = if params_str.is_empty() { vec![] + } else if required.len() == 1 && optional.is_empty() { + vec![(required[0].key.clone(), params_str.to_string())] } else { - let params_without_default = validation_result - .parameters - .as_ref() - .map(|params| params.iter().filter(|p| p.default.is_none()).count()) - .unwrap_or(0); - - if params_without_default <= 1 { - vec![params_str.to_string()] - } else { - let param_names: Vec = validation_result - .parameters - .as_ref() - .map(|params| { - params - .iter() - .filter(|p| p.default.is_none()) - .map(|p| p.key.clone()) - .collect() - }) - .unwrap_or_default(); - - let error_message = format!( - "The /{} recipe requires {} parameters: {}.\n\n\ - Slash command recipes only support 1 parameter.\n\n\ - **To use this recipe:**\n\ - • **CLI:** `goose run --recipe {} {}`\n\ - • **Desktop:** Launch from the recipes sidebar to fill in parameters", - command, - params_without_default, - param_names - .iter() - .map(|name| format!("**{}**", name)) - .collect::>() - .join(", "), - command, - param_names - .iter() - .map(|name| format!("--params {}=\"...\"", name)) - .collect::>() - .join(" ") - ); - - return Err(anyhow!(error_message)); - } + parse_recipe_args(params_str, &required, &optional)? }; let param_values_len = param_values.len(); - let recipe = match build_recipe_from_template_with_positional_params( + let recipe = match build_recipe_from_template( recipe_content, recipe_dir, param_values, @@ -501,6 +525,29 @@ impl Agent { #[cfg(test)] mod tests { use super::*; + use crate::recipe::RecipeParameterInputType; + + fn required_param(key: &str) -> RecipeParameter { + RecipeParameter { + key: key.to_string(), + input_type: RecipeParameterInputType::String, + requirement: RecipeParameterRequirement::Required, + description: format!("{key} parameter"), + default: None, + options: None, + } + } + + fn optional_param(key: &str) -> RecipeParameter { + RecipeParameter { + key: key.to_string(), + input_type: RecipeParameterInputType::String, + requirement: RecipeParameterRequirement::Optional, + description: format!("{key} parameter"), + default: Some("default".to_string()), + options: None, + } + } #[test] fn parse_slash_command_splits_on_literal_space() { @@ -520,4 +567,86 @@ mod tests { assert_eq!(parsed.command, "speckit.plan\nhello"); assert_eq!(parsed.params_str, ""); } + + #[test] + fn parse_recipe_args_maps_required_positionals_and_optional_flags() { + let component = required_param("component"); + let from = required_param("from"); + let to = optional_param("to"); + let scope = optional_param("scope"); + let required = vec![&component, &from]; + let optional = vec![&to, &scope]; + + let parsed = parse_recipe_args( + r#""Button Group" old-lib --to new-lib"#, + &required, + &optional, + ) + .unwrap(); + + assert_eq!( + parsed, + vec![ + ("component".to_string(), "Button Group".to_string()), + ("from".to_string(), "old-lib".to_string()), + ("to".to_string(), "new-lib".to_string()), + ] + ); + } + + #[test] + fn parse_recipe_args_allows_values_containing_equals() { + let component = required_param("component"); + let note = optional_param("note"); + let required = vec![&component]; + let optional = vec![¬e]; + + let parsed = parse_recipe_args(r#"Button --note "a=b""#, &required, &optional).unwrap(); + + assert_eq!( + parsed, + vec![ + ("component".to_string(), "Button".to_string()), + ("note".to_string(), "a=b".to_string()), + ] + ); + } + + #[test] + fn parse_recipe_args_errors_when_flag_value_is_another_flag() { + let component = required_param("component"); + let from = required_param("from"); + let to = optional_param("to"); + let scope = optional_param("scope"); + let required = vec![&component, &from]; + let optional = vec![&to, &scope]; + + let err = + parse_recipe_args("Button old-lib --to --scope all", &required, &optional).unwrap_err(); + + assert!(err.to_string().contains("Missing value for --to")); + } + + #[test] + fn parse_recipe_args_errors_on_extra_positionals() { + let component = required_param("component"); + let from = required_param("from"); + let required = vec![&component, &from]; + + let err = parse_recipe_args("Button old-lib extra", &required, &[]).unwrap_err(); + + assert!(err + .to_string() + .contains("Unexpected positional argument: extra")); + } + + #[test] + fn parse_recipe_args_errors_on_unknown_flag() { + let component = required_param("component"); + let required = vec![&component]; + + let err = parse_recipe_args("Button --unknown value", &required, &[]).unwrap_err(); + + assert!(err.to_string().contains("Unknown parameter: --unknown")); + } } diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index 27fe5c987dd3..48c9aa47957f 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -3,6 +3,8 @@ use std::path::Path; use goose_sdk::custom_requests::SourceEntry; +use crate::recipe::RecipeParameterRequirement; + #[derive(Debug, Clone, PartialEq, Eq)] pub enum SlashCommandSource { Builtin, @@ -38,8 +40,7 @@ pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { .collect(); for command in recipe_commands(crate::slash_commands::list_commands()) { - let name = normalize_command_name(&command.name); - if reserved_names.insert(name) { + if reserved_names.insert(command.name.clone()) { commands.push(command); } } @@ -47,7 +48,7 @@ pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { commands.extend( skill_commands(crate::skills::list_installed_skills(working_dir)) .into_iter() - .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))), + .filter(|command| !reserved_names.contains(&command.name)), ); commands } @@ -95,16 +96,6 @@ fn recipe_entry(recipe_path: &str) -> Option { ) .ok()?; - let required_param_count = validation_result - .parameters - .as_ref() - .map(|params| params.iter().filter(|p| p.default.is_none()).count()) - .unwrap_or(0); - - if required_param_count > 1 { - return None; - } - Some(RecipeCommandMetadata { description: validation_result.description, input_hint: input_hint_for_recipe(validation_result.parameters.as_ref()), @@ -137,13 +128,31 @@ fn normalize_command_name(name: &str) -> String { fn input_hint_for_recipe(params: Option<&Vec>) -> Option { let params = params?; + if params.is_empty() { + return None; + } - params - .iter() - .find(|p| p.key == "args") - .or_else(|| params.iter().find(|p| p.default.is_none())) - .or_else(|| params.first()) - .map(|p| p.description.clone()) + let mut required = Vec::new(); + let mut optional = Vec::new(); + + for p in params { + match p.requirement { + RecipeParameterRequirement::Required | RecipeParameterRequirement::UserPrompt => { + required.push(format!("<{}>", p.key)); + } + RecipeParameterRequirement::Optional => { + optional.push(format!("[--{} <{}>]", p.key, p.key)); + } + } + } + + Some( + required + .into_iter() + .chain(optional) + .collect::>() + .join(" "), + ) } fn builtin_input_hint(command: &str) -> Option<&'static str> { @@ -255,7 +264,7 @@ mod tests { let recipe_path = tmp.path().join("review.yaml"); std::fs::write( &recipe_path, - "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\nparameters:\n - key: args\n description: Describe what to review\n", + "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", ) .unwrap(); @@ -268,9 +277,65 @@ mod tests { assert_eq!(commands[0].name, "review"); assert_eq!(commands[0].description, "Review with a recipe"); assert_eq!(commands[0].source, SlashCommandSource::Recipe); + } + + #[test] + fn recipe_commands_omit_hint_for_no_param_recipe() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("status.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Status\ndescription: Check status\ninstructions: Check status\n", + ) + .unwrap(); + + let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + command: "status".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].input_hint, None); + } + + #[test] + fn recipe_commands_render_one_required_param_hint() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("review.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Review\ndescription: Review target\ninstructions: \"Review {{ target }}\"\nparameters:\n - key: target\n input_type: string\n requirement: required\n description: Target\n", + ) + .unwrap(); + + let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + command: "review".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].input_hint.as_deref(), Some("")); + } + + #[test] + fn recipe_commands_do_not_special_case_args_hint() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("deploy.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Deploy\ndescription: Deploy\ninstructions: \"Deploy {{ component }} with {{ args }}\"\nparameters:\n - key: component\n input_type: string\n requirement: required\n description: Component\n - key: args\n input_type: string\n requirement: optional\n default: default args\n description: Args\n", + ) + .unwrap(); + + let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + command: "deploy".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); assert_eq!( commands[0].input_hint.as_deref(), - Some("Describe what to review") + Some(" [--args ]") ); } @@ -317,16 +382,10 @@ mod tests { } #[test] - fn recipe_commands_skip_missing_invalid_and_multi_required_param_recipes() { + fn recipe_commands_skip_missing_and_invalid_recipes() { let tmp = TempDir::new().unwrap(); let invalid_recipe_path = tmp.path().join("invalid.yaml"); std::fs::write(&invalid_recipe_path, "not: a recipe").unwrap(); - let multi_param_recipe_path = tmp.path().join("multi.yaml"); - std::fs::write( - &multi_param_recipe_path, - "version: 1.0.0\ntitle: Multi Param Recipe\ndescription: Has too many required params\ninstructions: Review the change\nparameters:\n - key: first\n description: First param\n - key: second\n description: Second param\n", - ) - .unwrap(); let commands = recipe_commands(vec![ crate::slash_commands::SlashCommandMapping { @@ -341,15 +400,33 @@ mod tests { command: "invalid".to_string(), recipe_path: invalid_recipe_path.to_string_lossy().to_string(), }, - crate::slash_commands::SlashCommandMapping { - command: "multi".to_string(), - recipe_path: multi_param_recipe_path.to_string_lossy().to_string(), - }, ]); assert!(commands.is_empty()); } + #[test] + fn recipe_commands_render_multi_param_hint() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("deploy.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Deploy\ndescription: Deploy a service\ninstructions: \"Deploy {{ component }} from {{ from }} to {{ to }} scope {{ scope }}\"\nparameters:\n - key: component\n input_type: string\n requirement: required\n description: Component\n - key: from\n input_type: string\n requirement: required\n description: From\n - key: to\n input_type: string\n requirement: optional\n default: prod\n description: To\n - key: scope\n input_type: string\n requirement: optional\n default: all\n description: Scope\n", + ) + .unwrap(); + + let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + command: "deploy".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!( + commands[0].input_hint.as_deref(), + Some(" [--to ] [--scope ]") + ); + } + fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { SourceEntry { source_type, diff --git a/crates/goose/src/slash_commands.rs b/crates/goose/src/slash_commands.rs index 5e7065db0016..a14ef8064d4e 100644 --- a/crates/goose/src/slash_commands.rs +++ b/crates/goose/src/slash_commands.rs @@ -5,7 +5,6 @@ use serde::{Deserialize, Serialize}; use tracing::warn; use crate::config::Config; -use crate::recipe::Recipe; const SLASH_COMMANDS_CONFIG_KEY: &str = "slash_commands"; @@ -60,15 +59,3 @@ pub fn get_recipe_for_command(command: &str) -> Option { .find(|mapping| mapping.command == normalized) .map(|mapping| PathBuf::from(mapping.recipe_path)) } - -pub fn resolve_slash_command(command: &str) -> Option { - let recipe_path = get_recipe_for_command(command)?; - - if !recipe_path.exists() { - return None; - } - let recipe_content = std::fs::read_to_string(&recipe_path).ok()?; - let recipe = Recipe::from_content(&recipe_content).ok()?; - - Some(recipe) -} From 722cb3689fd254cde2ec57e384d0dfc0d63063f0 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 00:50:50 +1000 Subject: [PATCH 09/17] renaming --- .../src/routes/config_management.rs | 4 ++-- crates/goose-server/src/routes/recipe.rs | 6 +++--- crates/goose/src/acp/server.rs | 2 +- crates/goose/src/agents/agent.rs | 2 +- crates/goose/src/agents/execute_commands.rs | 2 +- crates/goose/src/lib.rs | 2 +- ...h_commands.rs => recipe_slash_commands.rs} | 0 crates/goose/src/slash_command.rs | 20 +++++++++---------- 8 files changed, 19 insertions(+), 19 deletions(-) rename crates/goose/src/{slash_commands.rs => recipe_slash_commands.rs} (100%) diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index 1f93f5e29886..dbecade70596 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -23,7 +23,7 @@ use goose::providers::create_with_default_model; use goose::providers::providers as get_providers; use goose::{ agents::execute_commands, agents::ExtensionConfig, config::permission::PermissionLevel, - slash_commands, + recipe_slash_commands, }; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -415,7 +415,7 @@ pub struct SlashCommandsQuery { pub async fn get_slash_commands( axum::extract::Query(query): axum::extract::Query, ) -> Result, ErrorResponse> { - let mut commands: Vec<_> = slash_commands::list_commands() + let mut commands: Vec<_> = recipe_slash_commands::list_commands() .iter() .map(|command| SlashCommand { command: command.command.clone(), diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index dcb452fca249..7dd1f8f7075e 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -9,7 +9,7 @@ use axum::{extract::State, http::StatusCode, routing::post, Json, Router}; use goose::recipe::local_recipes; use goose::recipe::validate_recipe::validate_recipe_template_from_content; use goose::recipe::{strip_error_location, Recipe}; -use goose::{recipe_deeplink, slash_commands}; +use goose::{recipe_deeplink, recipe_slash_commands}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -321,7 +321,7 @@ async fn list_recipes( .map(|j| (PathBuf::from(j.source), j.cron)) .collect(); - let all_commands = slash_commands::list_commands(); + let all_commands = recipe_slash_commands::list_commands(); let slash_map: HashMap<_, _> = all_commands .into_iter() .map(|sc| (PathBuf::from(sc.recipe_path), sc.command)) @@ -422,7 +422,7 @@ async fn set_recipe_slash_command( Err(err) => return Err(err.status), }; - match slash_commands::set_recipe_slash_command(file_path, request.slash_command) { + match recipe_slash_commands::set_recipe_slash_command(file_path, request.slash_command) { Ok(_) => Ok(StatusCode::OK), Err(e) => { tracing::error!("Failed to set slash command: {}", e); diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 75840d7ffaba..c14573df2408 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -2869,7 +2869,7 @@ impl GooseAcpAgent { if !Self::is_builtin_agent_command(parsed.command) { if let Some(recipe_path) = - crate::slash_commands::get_recipe_for_command(&full_command) + crate::recipe_slash_commands::get_recipe_for_command(&full_command) { if recipe_path.exists() { cx.send_notification(SessionNotification::new( diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index b2fe6d08ce99..eb3d653d2b9f 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -1282,7 +1282,7 @@ impl Agent { if message_text.trim().starts_with('/') { let command = message_text.split_whitespace().next(); if let Some(cmd) = command { - if crate::slash_commands::get_recipe_for_command(cmd).is_some() { + if crate::recipe_slash_commands::get_recipe_for_command(cmd).is_some() { #[cfg(feature = "telemetry")] crate::posthog::emit_custom_slash_command_used(); } diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 2eb41b175ec6..1bcd085e75b0 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -410,7 +410,7 @@ impl Agent { _session_id: &str, ) -> Result> { let full_command = format!("/{}", command); - let recipe_path = match crate::slash_commands::get_recipe_for_command(&full_command) { + let recipe_path = match crate::recipe_slash_commands::get_recipe_for_command(&full_command) { Some(path) => path, None => return Ok(None), }; diff --git a/crates/goose/src/lib.rs b/crates/goose/src/lib.rs index a7106040cf18..af5da1e9be34 100644 --- a/crates/goose/src/lib.rs +++ b/crates/goose/src/lib.rs @@ -35,6 +35,7 @@ pub mod prompt_template; pub mod providers; pub mod recipe; pub mod recipe_deeplink; +pub mod recipe_slash_commands; pub mod scheduler; pub mod scheduler_trait; pub mod security; @@ -42,7 +43,6 @@ pub mod session; pub mod session_context; pub mod skills; pub mod slash_command; -pub mod slash_commands; pub mod source_roots; pub mod sources; pub mod subprocess; diff --git a/crates/goose/src/slash_commands.rs b/crates/goose/src/recipe_slash_commands.rs similarity index 100% rename from crates/goose/src/slash_commands.rs rename to crates/goose/src/recipe_slash_commands.rs diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs index 48c9aa47957f..f82576628084 100644 --- a/crates/goose/src/slash_command.rs +++ b/crates/goose/src/slash_command.rs @@ -39,7 +39,7 @@ pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { .map(|command| normalize_command_name(&command.name)) .collect(); - for command in recipe_commands(crate::slash_commands::list_commands()) { + for command in recipe_commands(crate::recipe_slash_commands::list_commands()) { if reserved_names.insert(command.name.clone()) { commands.push(command); } @@ -54,7 +54,7 @@ pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { } fn recipe_commands( - mappings: Vec, + mappings: Vec, ) -> Vec { mappings .into_iter() @@ -268,7 +268,7 @@ mod tests { ) .unwrap(); - let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { command: "/review".to_string(), recipe_path: recipe_path.to_string_lossy().to_string(), }]); @@ -289,7 +289,7 @@ mod tests { ) .unwrap(); - let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { command: "status".to_string(), recipe_path: recipe_path.to_string_lossy().to_string(), }]); @@ -308,7 +308,7 @@ mod tests { ) .unwrap(); - let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { command: "review".to_string(), recipe_path: recipe_path.to_string_lossy().to_string(), }]); @@ -327,7 +327,7 @@ mod tests { ) .unwrap(); - let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { command: "deploy".to_string(), recipe_path: recipe_path.to_string_lossy().to_string(), }]); @@ -353,7 +353,7 @@ mod tests { .iter() .map(|command| normalize_command_name(&command.name)) .collect(); - for command in recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + for command in recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { command: "review".to_string(), recipe_path: recipe_path.to_string_lossy().to_string(), }]) { @@ -388,7 +388,7 @@ mod tests { std::fs::write(&invalid_recipe_path, "not: a recipe").unwrap(); let commands = recipe_commands(vec![ - crate::slash_commands::SlashCommandMapping { + crate::recipe_slash_commands::SlashCommandMapping { command: "missing".to_string(), recipe_path: tmp .path() @@ -396,7 +396,7 @@ mod tests { .to_string_lossy() .to_string(), }, - crate::slash_commands::SlashCommandMapping { + crate::recipe_slash_commands::SlashCommandMapping { command: "invalid".to_string(), recipe_path: invalid_recipe_path.to_string_lossy().to_string(), }, @@ -415,7 +415,7 @@ mod tests { ) .unwrap(); - let commands = recipe_commands(vec![crate::slash_commands::SlashCommandMapping { + let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { command: "deploy".to_string(), recipe_path: recipe_path.to_string_lossy().to_string(), }]); From d87edee3b572189896295a460ba67ebc30dea3ed Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 08:56:20 +1000 Subject: [PATCH 10/17] create slash_commands module and refactor --- .../src/routes/config_management.rs | 4 +- crates/goose-server/src/routes/recipe.rs | 6 +- crates/goose/src/acp/server.rs | 4 +- crates/goose/src/agents/agent.rs | 2 +- crates/goose/src/agents/execute_commands.rs | 35 +- crates/goose/src/lib.rs | 3 +- crates/goose/src/recipe_slash_commands.rs | 61 --- crates/goose/src/slash_command.rs | 443 ------------------ crates/goose/src/slash_commands/mod.rs | 5 + .../slash_commands/recipe_slash_command.rs | 275 +++++++++++ .../src/slash_commands/skill_slash_command.rs | 137 ++++++ .../goose/src/slash_commands/slash_command.rs | 165 +++++++ crates/goose/src/slash_commands/types.rs | 14 + crates/goose/src/slash_commands/util.rs | 3 + 14 files changed, 611 insertions(+), 546 deletions(-) delete mode 100644 crates/goose/src/recipe_slash_commands.rs delete mode 100644 crates/goose/src/slash_command.rs create mode 100644 crates/goose/src/slash_commands/mod.rs create mode 100644 crates/goose/src/slash_commands/recipe_slash_command.rs create mode 100644 crates/goose/src/slash_commands/skill_slash_command.rs create mode 100644 crates/goose/src/slash_commands/slash_command.rs create mode 100644 crates/goose/src/slash_commands/types.rs create mode 100644 crates/goose/src/slash_commands/util.rs diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index dbecade70596..36815eac61cf 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -23,7 +23,7 @@ use goose::providers::create_with_default_model; use goose::providers::providers as get_providers; use goose::{ agents::execute_commands, agents::ExtensionConfig, config::permission::PermissionLevel, - recipe_slash_commands, + slash_commands::recipe_slash_command, }; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -415,7 +415,7 @@ pub struct SlashCommandsQuery { pub async fn get_slash_commands( axum::extract::Query(query): axum::extract::Query, ) -> Result, ErrorResponse> { - let mut commands: Vec<_> = recipe_slash_commands::list_commands() + let mut commands: Vec<_> = recipe_slash_command::list_commands() .iter() .map(|command| SlashCommand { command: command.command.clone(), diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index 7dd1f8f7075e..a6f516e3b928 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -9,7 +9,7 @@ use axum::{extract::State, http::StatusCode, routing::post, Json, Router}; use goose::recipe::local_recipes; use goose::recipe::validate_recipe::validate_recipe_template_from_content; use goose::recipe::{strip_error_location, Recipe}; -use goose::{recipe_deeplink, recipe_slash_commands}; +use goose::{recipe_deeplink, slash_commands::recipe_slash_command}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -321,7 +321,7 @@ async fn list_recipes( .map(|j| (PathBuf::from(j.source), j.cron)) .collect(); - let all_commands = recipe_slash_commands::list_commands(); + let all_commands = recipe_slash_command::list_commands(); let slash_map: HashMap<_, _> = all_commands .into_iter() .map(|sc| (PathBuf::from(sc.recipe_path), sc.command)) @@ -422,7 +422,7 @@ async fn set_recipe_slash_command( Err(err) => return Err(err.status), }; - match recipe_slash_commands::set_recipe_slash_command(file_path, request.slash_command) { + match recipe_slash_command::set_recipe_slash_command(file_path, request.slash_command) { Ok(_) => Ok(StatusCode::OK), Err(e) => { tracing::error!("Failed to set slash command: {}", e); diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index c14573df2408..8cba21b05ee2 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -1046,7 +1046,7 @@ fn build_usage_update(session: &Session, context_limit: usize) -> UsageUpdate { impl GooseAcpAgent { fn available_commands_update(working_dir: &std::path::Path) -> AvailableCommandsUpdate { - let commands = crate::slash_command::list_acp_commands(Some(working_dir)) + let commands = crate::slash_commands::slash_command::list_acp_commands(Some(working_dir)) .into_iter() .map(|entry| { let mut command = AvailableCommand::new(entry.name, entry.description); @@ -2869,7 +2869,7 @@ impl GooseAcpAgent { if !Self::is_builtin_agent_command(parsed.command) { if let Some(recipe_path) = - crate::recipe_slash_commands::get_recipe_for_command(&full_command) + crate::slash_commands::recipe_slash_command::get_recipe_for_command(&full_command) { if recipe_path.exists() { cx.send_notification(SessionNotification::new( diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index eb3d653d2b9f..f1bbc61967bd 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -1282,7 +1282,7 @@ impl Agent { if message_text.trim().starts_with('/') { let command = message_text.split_whitespace().next(); if let Some(cmd) = command { - if crate::recipe_slash_commands::get_recipe_for_command(cmd).is_some() { + if crate::slash_commands::recipe_slash_command::get_recipe_for_command(cmd).is_some() { #[cfg(feature = "telemetry")] crate::posthog::emit_custom_slash_command_used(); } diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 1bcd085e75b0..57c52e396834 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -7,6 +7,7 @@ use crate::conversation::message::{Message, SystemNotificationType}; use crate::recipe::build_recipe::build_recipe_from_template; use crate::recipe::{RecipeParameter, RecipeParameterRequirement}; use crate::skills::loaded_skill_context_with_args; +use crate::slash_commands::{recipe_slash_command, skill_slash_command}; use super::Agent; @@ -213,9 +214,6 @@ impl Agent { } async fn handle_skills_command(&self, session_id: &str) -> Result> { - use crate::skills::list_installed_skills; - use goose_sdk::custom_requests::SourceType; - let working_dir = self .config .session_manager @@ -223,34 +221,7 @@ impl Agent { .await .ok() .map(|s| s.working_dir); - let sources = list_installed_skills(working_dir.as_deref()); - let skills: Vec<_> = sources - .iter() - .filter(|s| matches!(s.source_type, SourceType::Skill | SourceType::BuiltinSkill)) - .collect(); - - let mut output = String::new(); - if skills.is_empty() { - output.push_str("No skills installed.\n\n"); - output.push_str("Skills are loaded from SKILL.md files in:\n"); - output.push_str(" - ~/.agents/skills/ (global)\n"); - output.push_str(" - ~/.agents/plugins/*/skills/ (installed plugins)\n"); - output.push_str(" - .agents/skills/ (in current project)\n"); - } else { - output.push_str(&format!("**Installed skills ({}):**\n\n", skills.len())); - for skill in &skills { - let kind_label = if skill.source_type == SourceType::BuiltinSkill { - " *(builtin)*" - } else { - "" - }; - output.push_str(&format!( - "- **{}**{}: {}\n", - skill.name, kind_label, skill.description - )); - } - } - + let output = skill_slash_command::format_installed_skills(working_dir.as_deref()); Ok(Some(Message::assistant().with_text(output))) } @@ -410,7 +381,7 @@ impl Agent { _session_id: &str, ) -> Result> { let full_command = format!("/{}", command); - let recipe_path = match crate::recipe_slash_commands::get_recipe_for_command(&full_command) { + let recipe_path = match recipe_slash_command::get_recipe_for_command(&full_command) { Some(path) => path, None => return Ok(None), }; diff --git a/crates/goose/src/lib.rs b/crates/goose/src/lib.rs index af5da1e9be34..e1897cb84c10 100644 --- a/crates/goose/src/lib.rs +++ b/crates/goose/src/lib.rs @@ -35,14 +35,13 @@ pub mod prompt_template; pub mod providers; pub mod recipe; pub mod recipe_deeplink; -pub mod recipe_slash_commands; pub mod scheduler; pub mod scheduler_trait; pub mod security; pub mod session; pub mod session_context; pub mod skills; -pub mod slash_command; +pub mod slash_commands; pub mod source_roots; pub mod sources; pub mod subprocess; diff --git a/crates/goose/src/recipe_slash_commands.rs b/crates/goose/src/recipe_slash_commands.rs deleted file mode 100644 index a14ef8064d4e..000000000000 --- a/crates/goose/src/recipe_slash_commands.rs +++ /dev/null @@ -1,61 +0,0 @@ -use std::path::PathBuf; - -use anyhow::Result; -use serde::{Deserialize, Serialize}; -use tracing::warn; - -use crate::config::Config; - -const SLASH_COMMANDS_CONFIG_KEY: &str = "slash_commands"; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct SlashCommandMapping { - pub command: String, - pub recipe_path: String, -} - -pub fn list_commands() -> Vec { - Config::global() - .get_param(SLASH_COMMANDS_CONFIG_KEY) - .unwrap_or_else(|err| { - warn!( - "Failed to load {}: {}. Falling back to empty list.", - SLASH_COMMANDS_CONFIG_KEY, err - ); - Vec::new() - }) -} - -fn save_slash_commands(commands: Vec) -> Result<()> { - Config::global() - .set_param(SLASH_COMMANDS_CONFIG_KEY, &commands) - .map_err(|e| anyhow::anyhow!("Failed to save slash commands: {}", e)) -} - -pub fn set_recipe_slash_command(recipe_path: PathBuf, command: Option) -> Result<()> { - let recipe_path_str = recipe_path.to_string_lossy().to_string(); - - let mut commands = list_commands(); - commands.retain(|mapping| mapping.recipe_path != recipe_path_str); - - if let Some(cmd) = command { - let normalized_cmd = cmd.trim_start_matches('/').to_lowercase(); - if !normalized_cmd.is_empty() { - commands.push(SlashCommandMapping { - command: normalized_cmd, - recipe_path: recipe_path_str, - }); - } - } - - save_slash_commands(commands) -} - -pub fn get_recipe_for_command(command: &str) -> Option { - let normalized = command.trim_start_matches('/').to_lowercase(); - let commands = list_commands(); - commands - .into_iter() - .find(|mapping| mapping.command == normalized) - .map(|mapping| PathBuf::from(mapping.recipe_path)) -} diff --git a/crates/goose/src/slash_command.rs b/crates/goose/src/slash_command.rs deleted file mode 100644 index f82576628084..000000000000 --- a/crates/goose/src/slash_command.rs +++ /dev/null @@ -1,443 +0,0 @@ -use std::collections::HashSet; -use std::path::Path; - -use goose_sdk::custom_requests::SourceEntry; - -use crate::recipe::RecipeParameterRequirement; - -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum SlashCommandSource { - Builtin, - Recipe, - Skill, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SlashCommandEntry { - pub name: String, - pub description: String, - pub source: SlashCommandSource, - pub input_hint: Option, -} - -pub fn list_builtin_commands() -> Vec { - crate::agents::execute_commands::list_commands() - .iter() - .map(|command| SlashCommandEntry { - name: command.name.to_string(), - description: command.description.to_string(), - source: SlashCommandSource::Builtin, - input_hint: builtin_input_hint(command.name).map(str::to_string), - }) - .collect() -} - -pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { - let mut commands = list_builtin_commands(); - let mut reserved_names: HashSet = commands - .iter() - .map(|command| normalize_command_name(&command.name)) - .collect(); - - for command in recipe_commands(crate::recipe_slash_commands::list_commands()) { - if reserved_names.insert(command.name.clone()) { - commands.push(command); - } - } - - commands.extend( - skill_commands(crate::skills::list_installed_skills(working_dir)) - .into_iter() - .filter(|command| !reserved_names.contains(&command.name)), - ); - commands -} - -fn recipe_commands( - mappings: Vec, -) -> Vec { - mappings - .into_iter() - .filter_map(|mapping| { - let name = normalize_command_name(&mapping.command); - if name.is_empty() { - return None; - } - - let metadata = recipe_entry(&mapping.recipe_path)?; - - Some(SlashCommandEntry { - name, - description: metadata.description, - source: SlashCommandSource::Recipe, - input_hint: metadata.input_hint, - }) - }) - .collect() -} - -struct RecipeCommandMetadata { - description: String, - input_hint: Option, -} - -fn recipe_entry(recipe_path: &str) -> Option { - let recipe_path = std::path::PathBuf::from(recipe_path); - if !recipe_path.exists() { - return None; - } - - let recipe_content = std::fs::read_to_string(&recipe_path).ok()?; - let recipe_dir = recipe_path.parent()?; - let recipe_dir_str = recipe_dir.display().to_string(); - let validation_result = crate::recipe::validate_recipe::validate_recipe_template_from_content( - &recipe_content, - Some(recipe_dir_str), - ) - .ok()?; - - Some(RecipeCommandMetadata { - description: validation_result.description, - input_hint: input_hint_for_recipe(validation_result.parameters.as_ref()), - }) -} - -fn skill_commands(sources: Vec) -> Vec { - sources - .into_iter() - .filter_map(|source| { - let name = normalize_command_name(&source.name); - if name.is_empty() { - return None; - } - let input_hint = crate::skills::skill_argument_hint(&source); - - Some(SlashCommandEntry { - name, - description: source.description, - source: SlashCommandSource::Skill, - input_hint, - }) - }) - .collect() -} - -fn normalize_command_name(name: &str) -> String { - name.trim_start_matches('/').to_lowercase() -} - -fn input_hint_for_recipe(params: Option<&Vec>) -> Option { - let params = params?; - if params.is_empty() { - return None; - } - - let mut required = Vec::new(); - let mut optional = Vec::new(); - - for p in params { - match p.requirement { - RecipeParameterRequirement::Required | RecipeParameterRequirement::UserPrompt => { - required.push(format!("<{}>", p.key)); - } - RecipeParameterRequirement::Optional => { - optional.push(format!("[--{} <{}>]", p.key, p.key)); - } - } - } - - Some( - required - .into_iter() - .chain(optional) - .collect::>() - .join(" "), - ) -} - -fn builtin_input_hint(command: &str) -> Option<&'static str> { - match command { - "prompt" => Some(" [--info] [key=value...]"), - "prompts" => Some("[--extension ]"), - _ => None, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use goose_sdk::custom_requests::SourceType; - use std::collections::HashMap; - use tempfile::TempDir; - - #[test] - fn lists_acp_safe_builtin_commands() { - let commands = list_builtin_commands(); - let names: Vec<_> = commands - .iter() - .map(|command| command.name.as_str()) - .collect(); - - assert_eq!( - names, - vec!["prompts", "prompt", "compact", "clear", "skills", "doctor"] - ); - assert!(commands - .iter() - .all(|command| command.source == SlashCommandSource::Builtin)); - } - - #[test] - fn includes_input_hints_for_argument_taking_builtins() { - let commands = list_builtin_commands(); - let prompt = commands - .iter() - .find(|command| command.name == "prompt") - .expect("prompt command should be listed"); - let prompts = commands - .iter() - .find(|command| command.name == "prompts") - .expect("prompts command should be listed"); - let compact = commands - .iter() - .find(|command| command.name == "compact") - .expect("compact command should be listed"); - - assert_eq!( - prompt.input_hint.as_deref(), - Some(" [--info] [key=value...]") - ); - assert_eq!(prompts.input_hint.as_deref(), Some("[--extension ]")); - assert_eq!(compact.input_hint, None); - } - - #[test] - fn lists_project_skills_as_acp_commands() { - let tmp = TempDir::new().unwrap(); - let skill_dir = tmp - .path() - .join(".agents") - .join("skills") - .join("code-review"); - std::fs::create_dir_all(&skill_dir).unwrap(); - std::fs::write( - skill_dir.join("SKILL.md"), - "---\nname: code-review\ndescription: Review changed code\nmetadata:\n argument-hint: \"[task]\"\n arguments:\n - task\n---\nReview the diff.", - ) - .unwrap(); - - let commands = list_acp_commands(Some(tmp.path())); - let command = commands - .iter() - .find(|command| command.name == "code-review") - .expect("project skill should be listed as an ACP command"); - - assert_eq!(command.description, "Review changed code"); - assert_eq!(command.source, SlashCommandSource::Skill); - assert_eq!(command.input_hint.as_deref(), Some("[task]")); - } - - #[test] - fn skill_commands_do_not_override_builtins() { - let reserved_names: HashSet = list_builtin_commands() - .into_iter() - .map(|command| normalize_command_name(&command.name)) - .collect(); - let commands: Vec<_> = skill_commands(vec![ - source_entry(SourceType::Skill, "compact", "Skill named compact"), - source_entry(SourceType::Skill, "review", "Review code"), - ]) - .into_iter() - .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))) - .collect(); - let names: Vec<_> = commands - .iter() - .map(|command| command.name.as_str()) - .collect(); - - assert_eq!(names, vec!["review"]); - } - - #[test] - fn recipe_commands_use_recipe_description() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("review.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", - ) - .unwrap(); - - let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { - command: "/review".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }]); - - assert_eq!(commands.len(), 1); - assert_eq!(commands[0].name, "review"); - assert_eq!(commands[0].description, "Review with a recipe"); - assert_eq!(commands[0].source, SlashCommandSource::Recipe); - } - - #[test] - fn recipe_commands_omit_hint_for_no_param_recipe() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("status.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Status\ndescription: Check status\ninstructions: Check status\n", - ) - .unwrap(); - - let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { - command: "status".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }]); - - assert_eq!(commands.len(), 1); - assert_eq!(commands[0].input_hint, None); - } - - #[test] - fn recipe_commands_render_one_required_param_hint() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("review.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Review\ndescription: Review target\ninstructions: \"Review {{ target }}\"\nparameters:\n - key: target\n input_type: string\n requirement: required\n description: Target\n", - ) - .unwrap(); - - let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { - command: "review".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }]); - - assert_eq!(commands.len(), 1); - assert_eq!(commands[0].input_hint.as_deref(), Some("")); - } - - #[test] - fn recipe_commands_do_not_special_case_args_hint() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("deploy.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Deploy\ndescription: Deploy\ninstructions: \"Deploy {{ component }} with {{ args }}\"\nparameters:\n - key: component\n input_type: string\n requirement: required\n description: Component\n - key: args\n input_type: string\n requirement: optional\n default: default args\n description: Args\n", - ) - .unwrap(); - - let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { - command: "deploy".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }]); - - assert_eq!(commands.len(), 1); - assert_eq!( - commands[0].input_hint.as_deref(), - Some(" [--args ]") - ); - } - - #[test] - fn recipe_commands_reserve_names_before_skills() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("review.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", - ) - .unwrap(); - let mut commands = list_builtin_commands(); - let mut reserved_names: HashSet = commands - .iter() - .map(|command| normalize_command_name(&command.name)) - .collect(); - for command in recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { - command: "review".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }]) { - let name = normalize_command_name(&command.name); - if reserved_names.insert(name) { - commands.push(command); - } - } - commands.extend( - skill_commands(vec![source_entry( - SourceType::Skill, - "review", - "Review code", - )]) - .into_iter() - .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))), - ); - - let review_commands: Vec<_> = commands - .iter() - .filter(|command| command.name == "review") - .collect(); - - assert_eq!(review_commands.len(), 1); - assert_eq!(review_commands[0].source, SlashCommandSource::Recipe); - } - - #[test] - fn recipe_commands_skip_missing_and_invalid_recipes() { - let tmp = TempDir::new().unwrap(); - let invalid_recipe_path = tmp.path().join("invalid.yaml"); - std::fs::write(&invalid_recipe_path, "not: a recipe").unwrap(); - - let commands = recipe_commands(vec![ - crate::recipe_slash_commands::SlashCommandMapping { - command: "missing".to_string(), - recipe_path: tmp - .path() - .join("missing.yaml") - .to_string_lossy() - .to_string(), - }, - crate::recipe_slash_commands::SlashCommandMapping { - command: "invalid".to_string(), - recipe_path: invalid_recipe_path.to_string_lossy().to_string(), - }, - ]); - - assert!(commands.is_empty()); - } - - #[test] - fn recipe_commands_render_multi_param_hint() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("deploy.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Deploy\ndescription: Deploy a service\ninstructions: \"Deploy {{ component }} from {{ from }} to {{ to }} scope {{ scope }}\"\nparameters:\n - key: component\n input_type: string\n requirement: required\n description: Component\n - key: from\n input_type: string\n requirement: required\n description: From\n - key: to\n input_type: string\n requirement: optional\n default: prod\n description: To\n - key: scope\n input_type: string\n requirement: optional\n default: all\n description: Scope\n", - ) - .unwrap(); - - let commands = recipe_commands(vec![crate::recipe_slash_commands::SlashCommandMapping { - command: "deploy".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }]); - - assert_eq!(commands.len(), 1); - assert_eq!( - commands[0].input_hint.as_deref(), - Some(" [--to ] [--scope ]") - ); - } - - fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { - SourceEntry { - source_type, - name: name.to_string(), - description: description.to_string(), - content: String::new(), - path: String::new(), - global: false, - writable: false, - supporting_files: Vec::new(), - properties: HashMap::new(), - } - } -} diff --git a/crates/goose/src/slash_commands/mod.rs b/crates/goose/src/slash_commands/mod.rs new file mode 100644 index 000000000000..a55601ba49d6 --- /dev/null +++ b/crates/goose/src/slash_commands/mod.rs @@ -0,0 +1,5 @@ +pub mod recipe_slash_command; +pub mod skill_slash_command; +pub mod slash_command; +pub mod types; +pub mod util; diff --git a/crates/goose/src/slash_commands/recipe_slash_command.rs b/crates/goose/src/slash_commands/recipe_slash_command.rs new file mode 100644 index 000000000000..d19a29e08397 --- /dev/null +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -0,0 +1,275 @@ +use std::path::PathBuf; + +use anyhow::Result; +use serde::{Deserialize, Serialize}; +use tracing::warn; + +use super::types::{SlashCommandEntry, SlashCommandSource}; +use super::util::normalize_command_name; +use crate::config::Config; +use crate::recipe::{RecipeParameter, RecipeParameterRequirement}; + +const SLASH_COMMANDS_CONFIG_KEY: &str = "slash_commands"; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SlashCommandMapping { + pub command: String, + pub recipe_path: String, +} + +pub fn list_commands() -> Vec { + Config::global() + .get_param(SLASH_COMMANDS_CONFIG_KEY) + .unwrap_or_else(|err| { + warn!( + "Failed to load {}: {}. Falling back to empty list.", + SLASH_COMMANDS_CONFIG_KEY, err + ); + Vec::new() + }) +} + +fn save_slash_commands(commands: Vec) -> Result<()> { + Config::global() + .set_param(SLASH_COMMANDS_CONFIG_KEY, &commands) + .map_err(|e| anyhow::anyhow!("Failed to save slash commands: {}", e)) +} + +pub fn set_recipe_slash_command(recipe_path: PathBuf, command: Option) -> Result<()> { + let recipe_path_str = recipe_path.to_string_lossy().to_string(); + + let mut commands = list_commands(); + commands.retain(|mapping| mapping.recipe_path != recipe_path_str); + + if let Some(cmd) = command { + let normalized_cmd = cmd.trim_start_matches('/').to_lowercase(); + if !normalized_cmd.is_empty() { + commands.push(SlashCommandMapping { + command: normalized_cmd, + recipe_path: recipe_path_str, + }); + } + } + + save_slash_commands(commands) +} + +pub fn get_recipe_for_command(command: &str) -> Option { + let normalized = command.trim_start_matches('/').to_lowercase(); + let commands = list_commands(); + commands + .into_iter() + .find(|mapping| mapping.command == normalized) + .map(|mapping| PathBuf::from(mapping.recipe_path)) +} + +pub(super) fn commands_from_mappings( + mappings: Vec, +) -> Vec { + mappings + .into_iter() + .filter_map(|mapping| { + let name = normalize_command_name(&mapping.command); + if name.is_empty() { + return None; + } + + let metadata = recipe_entry(&mapping.recipe_path)?; + + Some(SlashCommandEntry { + name, + description: metadata.description, + source: SlashCommandSource::Recipe, + input_hint: metadata.input_hint, + }) + }) + .collect() +} + +struct RecipeCommandMetadata { + description: String, + input_hint: Option, +} + +fn recipe_entry(recipe_path: &str) -> Option { + let recipe_path = PathBuf::from(recipe_path); + if !recipe_path.exists() { + return None; + } + + let recipe_content = std::fs::read_to_string(&recipe_path).ok()?; + let recipe_dir = recipe_path.parent()?; + let recipe_dir_str = recipe_dir.display().to_string(); + let validation_result = crate::recipe::validate_recipe::validate_recipe_template_from_content( + &recipe_content, + Some(recipe_dir_str), + ) + .ok()?; + + Some(RecipeCommandMetadata { + description: validation_result.description, + input_hint: input_hint_for_recipe(validation_result.parameters.as_ref()), + }) +} + +fn input_hint_for_recipe(params: Option<&Vec>) -> Option { + let params = params?; + if params.is_empty() { + return None; + } + + let mut required = Vec::new(); + let mut optional = Vec::new(); + + for p in params { + match p.requirement { + RecipeParameterRequirement::Required | RecipeParameterRequirement::UserPrompt => { + required.push(format!("<{}>", p.key)); + } + RecipeParameterRequirement::Optional => { + optional.push(format!("[--{} <{}>]", p.key, p.key)); + } + } + } + + Some( + required + .into_iter() + .chain(optional) + .collect::>() + .join(" "), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn commands_from_mappings_use_recipe_description() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("review.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", + ) + .unwrap(); + + let commands = commands_from_mappings(vec![SlashCommandMapping { + command: "/review".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].name, "review"); + assert_eq!(commands[0].description, "Review with a recipe"); + assert_eq!(commands[0].source, SlashCommandSource::Recipe); + } + + #[test] + fn commands_from_mappings_omit_hint_for_no_param_recipe() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("status.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Status\ndescription: Check status\ninstructions: Check status\n", + ) + .unwrap(); + + let commands = commands_from_mappings(vec![SlashCommandMapping { + command: "status".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].input_hint, None); + } + + #[test] + fn commands_from_mappings_render_one_required_param_hint() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("review.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Review\ndescription: Review target\ninstructions: \"Review {{ target }}\"\nparameters:\n - key: target\n input_type: string\n requirement: required\n description: Target\n", + ) + .unwrap(); + + let commands = commands_from_mappings(vec![SlashCommandMapping { + command: "review".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].input_hint.as_deref(), Some("")); + } + + #[test] + fn commands_from_mappings_do_not_special_case_args_hint() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("deploy.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Deploy\ndescription: Deploy\ninstructions: \"Deploy {{ component }} with {{ args }}\"\nparameters:\n - key: component\n input_type: string\n requirement: required\n description: Component\n - key: args\n input_type: string\n requirement: optional\n default: default args\n description: Args\n", + ) + .unwrap(); + + let commands = commands_from_mappings(vec![SlashCommandMapping { + command: "deploy".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!( + commands[0].input_hint.as_deref(), + Some(" [--args ]") + ); + } + + #[test] + fn commands_from_mappings_skip_missing_and_invalid_recipes() { + let tmp = TempDir::new().unwrap(); + let invalid_recipe_path = tmp.path().join("invalid.yaml"); + std::fs::write(&invalid_recipe_path, "not: a recipe").unwrap(); + + let commands = commands_from_mappings(vec![ + SlashCommandMapping { + command: "missing".to_string(), + recipe_path: tmp + .path() + .join("missing.yaml") + .to_string_lossy() + .to_string(), + }, + SlashCommandMapping { + command: "invalid".to_string(), + recipe_path: invalid_recipe_path.to_string_lossy().to_string(), + }, + ]); + + assert!(commands.is_empty()); + } + + #[test] + fn commands_from_mappings_render_multi_param_hint() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("deploy.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Deploy\ndescription: Deploy a service\ninstructions: \"Deploy {{ component }} from {{ from }} to {{ to }} scope {{ scope }}\"\nparameters:\n - key: component\n input_type: string\n requirement: required\n description: Component\n - key: from\n input_type: string\n requirement: required\n description: From\n - key: to\n input_type: string\n requirement: optional\n default: prod\n description: To\n - key: scope\n input_type: string\n requirement: optional\n default: all\n description: Scope\n", + ) + .unwrap(); + + let commands = commands_from_mappings(vec![SlashCommandMapping { + command: "deploy".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }]); + + assert_eq!(commands.len(), 1); + assert_eq!( + commands[0].input_hint.as_deref(), + Some(" [--to ] [--scope ]") + ); + } +} diff --git a/crates/goose/src/slash_commands/skill_slash_command.rs b/crates/goose/src/slash_commands/skill_slash_command.rs new file mode 100644 index 000000000000..0dc52008123f --- /dev/null +++ b/crates/goose/src/slash_commands/skill_slash_command.rs @@ -0,0 +1,137 @@ +use std::path::Path; + +use goose_sdk::custom_requests::{SourceEntry, SourceType}; + +use super::types::{SlashCommandEntry, SlashCommandSource}; +use super::util::normalize_command_name; + +pub fn list_commands(working_dir: Option<&Path>) -> Vec { + commands_from_sources(crate::skills::list_installed_skills(working_dir)) +} + +pub fn format_installed_skills(working_dir: Option<&Path>) -> String { + let sources = crate::skills::list_installed_skills(working_dir); + let skills: Vec<_> = sources + .iter() + .filter(|s| matches!(s.source_type, SourceType::Skill | SourceType::BuiltinSkill)) + .collect(); + + let mut output = String::new(); + if skills.is_empty() { + output.push_str("No skills installed.\n\n"); + output.push_str("Skills are loaded from SKILL.md files in:\n"); + output.push_str(" - ~/.agents/skills/ (global)\n"); + output.push_str(" - ~/.agents/plugins/*/skills/ (installed plugins)\n"); + output.push_str(" - .agents/skills/ (in current project)\n"); + } else { + output.push_str(&format!("**Installed skills ({}):**\n\n", skills.len())); + for skill in &skills { + let kind_label = if skill.source_type == SourceType::BuiltinSkill { + " *(builtin)*" + } else { + "" + }; + output.push_str(&format!( + "- **{}**{}: {}\n", + skill.name, kind_label, skill.description + )); + } + } + output +} + +pub(super) fn commands_from_sources(sources: Vec) -> Vec { + sources + .into_iter() + .filter_map(|source| { + let name = normalize_command_name(&source.name); + if name.is_empty() { + return None; + } + let input_hint = crate::skills::skill_argument_hint(&source); + + Some(SlashCommandEntry { + name, + description: source.description, + source: SlashCommandSource::Skill, + input_hint, + }) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use goose_sdk::custom_requests::SourceType; + use std::collections::{HashMap, HashSet}; + + #[test] + fn commands_from_sources_marks_entries_as_skill() { + let commands = commands_from_sources(vec![ + source_entry(SourceType::Skill, "review", "Review code"), + source_entry(SourceType::Skill, "summarize", "Summarize text"), + ]); + + let names: Vec<_> = commands.iter().map(|c| c.name.as_str()).collect(); + assert_eq!(names, vec!["review", "summarize"]); + assert!(commands + .iter() + .all(|c| c.source == SlashCommandSource::Skill)); + } + + #[test] + fn commands_from_sources_normalizes_names() { + let commands = commands_from_sources(vec![source_entry( + SourceType::Skill, + "/Code-Review", + "Review", + )]); + + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].name, "code-review"); + } + + #[test] + fn commands_from_sources_skips_empty_names() { + let commands = + commands_from_sources(vec![source_entry(SourceType::Skill, "/", "Empty name")]); + + assert!(commands.is_empty()); + } + + #[test] + fn skill_commands_do_not_override_builtins() { + let reserved_names: HashSet = super::super::slash_command::list_builtin_commands() + .into_iter() + .map(|command| normalize_command_name(&command.name)) + .collect(); + let commands: Vec<_> = commands_from_sources(vec![ + source_entry(SourceType::Skill, "compact", "Skill named compact"), + source_entry(SourceType::Skill, "review", "Review code"), + ]) + .into_iter() + .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))) + .collect(); + let names: Vec<_> = commands + .iter() + .map(|command| command.name.as_str()) + .collect(); + + assert_eq!(names, vec!["review"]); + } + + fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { + SourceEntry { + source_type, + name: name.to_string(), + description: description.to_string(), + content: String::new(), + path: String::new(), + global: false, + writable: false, + supporting_files: Vec::new(), + properties: HashMap::new(), + } + } +} diff --git a/crates/goose/src/slash_commands/slash_command.rs b/crates/goose/src/slash_commands/slash_command.rs new file mode 100644 index 000000000000..5be4fcb5f578 --- /dev/null +++ b/crates/goose/src/slash_commands/slash_command.rs @@ -0,0 +1,165 @@ +use std::collections::HashSet; +use std::path::Path; + +use super::types::{SlashCommandEntry, SlashCommandSource}; +use super::util::normalize_command_name; + +pub fn list_builtin_commands() -> Vec { + crate::agents::execute_commands::list_commands() + .iter() + .map(|command| SlashCommandEntry { + name: command.name.to_string(), + description: command.description.to_string(), + source: SlashCommandSource::Builtin, + input_hint: builtin_input_hint(command.name).map(str::to_string), + }) + .collect() +} + +pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { + let mut commands = list_builtin_commands(); + let mut reserved_names: HashSet = commands + .iter() + .map(|command| normalize_command_name(&command.name)) + .collect(); + + for command in super::recipe_slash_command::commands_from_mappings( + super::recipe_slash_command::list_commands(), + ) { + if reserved_names.insert(command.name.clone()) { + commands.push(command); + } + } + + commands.extend( + super::skill_slash_command::list_commands(working_dir) + .into_iter() + .filter(|command| !reserved_names.contains(&command.name)), + ); + commands +} + +fn builtin_input_hint(command: &str) -> Option<&'static str> { + match command { + "prompt" => Some(" [--info] [key=value...]"), + "prompts" => Some("[--extension ]"), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn lists_acp_safe_builtin_commands() { + let commands = list_builtin_commands(); + let names: Vec<_> = commands + .iter() + .map(|command| command.name.as_str()) + .collect(); + + assert_eq!( + names, + vec!["prompts", "prompt", "compact", "clear", "skills", "doctor"] + ); + assert!(commands + .iter() + .all(|command| command.source == SlashCommandSource::Builtin)); + } + + #[test] + fn includes_input_hints_for_argument_taking_builtins() { + let commands = list_builtin_commands(); + let prompt = commands + .iter() + .find(|command| command.name == "prompt") + .expect("prompt command should be listed"); + let prompts = commands + .iter() + .find(|command| command.name == "prompts") + .expect("prompts command should be listed"); + let compact = commands + .iter() + .find(|command| command.name == "compact") + .expect("compact command should be listed"); + + assert_eq!( + prompt.input_hint.as_deref(), + Some(" [--info] [key=value...]") + ); + assert_eq!(prompts.input_hint.as_deref(), Some("[--extension ]")); + assert_eq!(compact.input_hint, None); + } + + #[test] + fn lists_project_skills_as_acp_commands() { + let tmp = TempDir::new().unwrap(); + let skill_dir = tmp + .path() + .join(".agents") + .join("skills") + .join("code-review"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: code-review\ndescription: Review changed code\nmetadata:\n argument-hint: \"[task]\"\n arguments:\n - task\n---\nReview the diff.", + ) + .unwrap(); + + let commands = list_acp_commands(Some(tmp.path())); + let command = commands + .iter() + .find(|command| command.name == "code-review") + .expect("project skill should be listed as an ACP command"); + + assert_eq!(command.description, "Review changed code"); + assert_eq!(command.source, SlashCommandSource::Skill); + assert_eq!(command.input_hint.as_deref(), Some("[task]")); + } + + #[test] + fn recipe_commands_reserve_names_before_skills() { + let tmp = TempDir::new().unwrap(); + let recipe_path = tmp.path().join("review.yaml"); + std::fs::write( + &recipe_path, + "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", + ) + .unwrap(); + let mut commands = list_builtin_commands(); + let mut reserved_names: HashSet = commands + .iter() + .map(|command| normalize_command_name(&command.name)) + .collect(); + for command in super::recipe_slash_command::commands_from_mappings(vec![ + super::recipe_slash_command::SlashCommandMapping { + command: "review".to_string(), + recipe_path: recipe_path.to_string_lossy().to_string(), + }, + ]) { + let name = normalize_command_name(&command.name); + if reserved_names.insert(name) { + commands.push(command); + } + } + let skill_command = SlashCommandEntry { + name: "review".to_string(), + description: "Review code".to_string(), + source: SlashCommandSource::Skill, + input_hint: None, + }; + if !reserved_names.contains(&normalize_command_name(&skill_command.name)) { + commands.push(skill_command); + } + + let review_commands: Vec<_> = commands + .iter() + .filter(|command| command.name == "review") + .collect(); + + assert_eq!(review_commands.len(), 1); + assert_eq!(review_commands[0].source, SlashCommandSource::Recipe); + } +} diff --git a/crates/goose/src/slash_commands/types.rs b/crates/goose/src/slash_commands/types.rs new file mode 100644 index 000000000000..53783d9b5785 --- /dev/null +++ b/crates/goose/src/slash_commands/types.rs @@ -0,0 +1,14 @@ +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SlashCommandSource { + Builtin, + Recipe, + Skill, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SlashCommandEntry { + pub name: String, + pub description: String, + pub source: SlashCommandSource, + pub input_hint: Option, +} diff --git a/crates/goose/src/slash_commands/util.rs b/crates/goose/src/slash_commands/util.rs new file mode 100644 index 000000000000..a194b2aa013a --- /dev/null +++ b/crates/goose/src/slash_commands/util.rs @@ -0,0 +1,3 @@ +pub fn normalize_command_name(name: &str) -> String { + name.trim_start_matches('/').to_lowercase() +} From 218e9c8e8c402fbd1ee4b883ef62ee7afac4170c Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 10:56:50 +1000 Subject: [PATCH 11/17] refactor recipe, skill execute command --- crates/goose/src/acp/server.rs | 4 +- crates/goose/src/agents/agent.rs | 4 +- crates/goose/src/agents/execute_commands.rs | 258 +----------------- .../slash_commands/recipe_slash_command.rs | 243 ++++++++++++++++- .../src/slash_commands/skill_slash_command.rs | 19 ++ 5 files changed, 275 insertions(+), 253 deletions(-) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 8cba21b05ee2..d1246f4af216 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -2869,7 +2869,9 @@ impl GooseAcpAgent { if !Self::is_builtin_agent_command(parsed.command) { if let Some(recipe_path) = - crate::slash_commands::recipe_slash_command::get_recipe_for_command(&full_command) + crate::slash_commands::recipe_slash_command::get_recipe_for_command( + &full_command, + ) { if recipe_path.exists() { cx.send_notification(SessionNotification::new( diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index f1bbc61967bd..d481d8505ba2 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -1282,7 +1282,9 @@ impl Agent { if message_text.trim().starts_with('/') { let command = message_text.split_whitespace().next(); if let Some(cmd) = command { - if crate::slash_commands::recipe_slash_command::get_recipe_for_command(cmd).is_some() { + if crate::slash_commands::recipe_slash_command::get_recipe_for_command(cmd) + .is_some() + { #[cfg(feature = "telemetry")] crate::posthog::emit_custom_slash_command_used(); } diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 57c52e396834..fb76c1110978 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -4,9 +4,6 @@ use anyhow::{anyhow, Result}; use crate::context_mgmt::compact_messages; use crate::conversation::message::{Message, SystemNotificationType}; -use crate::recipe::build_recipe::build_recipe_from_template; -use crate::recipe::{RecipeParameter, RecipeParameterRequirement}; -use crate::skills::loaded_skill_context_with_args; use crate::slash_commands::{recipe_slash_command, skill_slash_command}; use super::Agent; @@ -51,51 +48,6 @@ pub struct ParsedSlashCommand<'a> { pub params_str: &'a str, } -fn parse_recipe_args( - params_str: &str, - required: &[&RecipeParameter], - optional: &[&RecipeParameter], -) -> Result> { - use std::collections::HashSet; - - let tokens = crate::utils::split_command_args(params_str)?; - let known_keys: HashSet<&str> = required - .iter() - .chain(optional.iter()) - .map(|p| p.key.as_str()) - .collect(); - - let mut result = Vec::new(); - let mut required_idx = 0; - let mut i = 0; - - while i < tokens.len() { - let token = &tokens[i]; - if let Some(flag) = token.strip_prefix("--") { - if !known_keys.contains(flag) { - return Err(anyhow!("Unknown parameter: --{}", flag)); - } - let value = tokens - .get(i + 1) - .ok_or_else(|| anyhow!("Missing value for --{}", flag))?; - if value.starts_with("--") { - return Err(anyhow!("Missing value for --{}", flag)); - } - result.push((flag.to_string(), value.clone())); - i += 2; - } else { - if required_idx >= required.len() { - return Err(anyhow!("Unexpected positional argument: {}", token)); - } - result.push((required[required_idx].key.clone(), token.clone())); - required_idx += 1; - i += 1; - } - } - - Ok(result) -} - pub fn parse_slash_command(message_text: &str) -> Option> { let mut trimmed = message_text.trim(); @@ -380,88 +332,14 @@ impl Agent { params_str: &str, _session_id: &str, ) -> Result> { - let full_command = format!("/{}", command); - let recipe_path = match recipe_slash_command::get_recipe_for_command(&full_command) { - Some(path) => path, - None => return Ok(None), - }; - - if !recipe_path.exists() { - return Ok(None); - } - - let recipe_content = std::fs::read_to_string(&recipe_path) - .map_err(|e| anyhow!("Failed to read recipe file: {}", e))?; - - let recipe_dir = recipe_path - .parent() - .ok_or_else(|| anyhow!("Recipe path has no parent directory"))?; - - let recipe_dir_str = recipe_dir.display().to_string(); - let validation_result = - crate::recipe::validate_recipe::validate_recipe_template_from_content( - &recipe_content, - Some(recipe_dir_str), - ) - .map_err(|e| anyhow!("Failed to parse recipe: {}", e))?; - - let empty_params: Vec = Vec::new(); - let all_params = validation_result - .parameters - .as_ref() - .unwrap_or(&empty_params); - let required: Vec<&RecipeParameter> = all_params - .iter() - .filter(|p| { - matches!( - p.requirement, - RecipeParameterRequirement::Required | RecipeParameterRequirement::UserPrompt - ) - }) - .collect(); - let optional: Vec<&RecipeParameter> = all_params - .iter() - .filter(|p| matches!(p.requirement, RecipeParameterRequirement::Optional)) - .collect(); - - let param_values: Vec<(String, String)> = if params_str.is_empty() { - vec![] - } else if required.len() == 1 && optional.is_empty() { - vec![(required[0].key.clone(), params_str.to_string())] - } else { - parse_recipe_args(params_str, &required, &optional)? - }; - - let param_values_len = param_values.len(); - - let recipe = match build_recipe_from_template( - recipe_content, - recipe_dir, - param_values, - None:: Result>, - ) { - Ok(recipe) => recipe, - Err(crate::recipe::build_recipe::RecipeError::MissingParams { parameters }) => { - return Ok(Some(Message::assistant().with_text(format!( - "Recipe requires {} parameter(s): {}. Provided: {}", - parameters.len(), - parameters.join(", "), - param_values_len - )))); + match recipe_slash_command::resolve_command(command, params_str) { + Ok(None) => Ok(None), + Ok(Some((response, prompt))) => { + self.apply_recipe_components(response, true).await; + Ok(Some(Message::user().with_text(prompt))) } - Err(e) => return Err(anyhow!("Failed to build recipe: {}", e)), - }; - - self.apply_recipe_components(recipe.response.clone(), true) - .await; - - let prompt = [recipe.instructions.as_deref(), recipe.prompt.as_deref()] - .into_iter() - .flatten() - .collect::>() - .join("\n\n"); - - Ok(Some(Message::user().with_text(prompt))) + Err(text) => Ok(Some(Message::assistant().with_text(text))), + } } async fn handle_skill_command( @@ -478,47 +356,17 @@ impl Agent { .ok() .map(|session| session.working_dir); - let skill = crate::skills::list_installed_skills(working_dir.as_deref()) - .into_iter() - .find(|skill| skill.name.eq_ignore_ascii_case(command)); - - let Some(skill) = skill else { - return Ok(None); - }; - - let prompt = - loaded_skill_context_with_args(&skill, (!params_str.is_empty()).then_some(params_str))?; - - Ok(Some(Message::user().with_text(prompt))) + match skill_slash_command::resolve_command(command, params_str, working_dir.as_deref()) { + Ok(None) => Ok(None), + Ok(Some(prompt)) => Ok(Some(Message::user().with_text(prompt))), + Err(text) => Ok(Some(Message::assistant().with_text(text))), + } } } #[cfg(test)] mod tests { use super::*; - use crate::recipe::RecipeParameterInputType; - - fn required_param(key: &str) -> RecipeParameter { - RecipeParameter { - key: key.to_string(), - input_type: RecipeParameterInputType::String, - requirement: RecipeParameterRequirement::Required, - description: format!("{key} parameter"), - default: None, - options: None, - } - } - - fn optional_param(key: &str) -> RecipeParameter { - RecipeParameter { - key: key.to_string(), - input_type: RecipeParameterInputType::String, - requirement: RecipeParameterRequirement::Optional, - description: format!("{key} parameter"), - default: Some("default".to_string()), - options: None, - } - } #[test] fn parse_slash_command_splits_on_literal_space() { @@ -538,86 +386,4 @@ mod tests { assert_eq!(parsed.command, "speckit.plan\nhello"); assert_eq!(parsed.params_str, ""); } - - #[test] - fn parse_recipe_args_maps_required_positionals_and_optional_flags() { - let component = required_param("component"); - let from = required_param("from"); - let to = optional_param("to"); - let scope = optional_param("scope"); - let required = vec![&component, &from]; - let optional = vec![&to, &scope]; - - let parsed = parse_recipe_args( - r#""Button Group" old-lib --to new-lib"#, - &required, - &optional, - ) - .unwrap(); - - assert_eq!( - parsed, - vec![ - ("component".to_string(), "Button Group".to_string()), - ("from".to_string(), "old-lib".to_string()), - ("to".to_string(), "new-lib".to_string()), - ] - ); - } - - #[test] - fn parse_recipe_args_allows_values_containing_equals() { - let component = required_param("component"); - let note = optional_param("note"); - let required = vec![&component]; - let optional = vec![¬e]; - - let parsed = parse_recipe_args(r#"Button --note "a=b""#, &required, &optional).unwrap(); - - assert_eq!( - parsed, - vec![ - ("component".to_string(), "Button".to_string()), - ("note".to_string(), "a=b".to_string()), - ] - ); - } - - #[test] - fn parse_recipe_args_errors_when_flag_value_is_another_flag() { - let component = required_param("component"); - let from = required_param("from"); - let to = optional_param("to"); - let scope = optional_param("scope"); - let required = vec![&component, &from]; - let optional = vec![&to, &scope]; - - let err = - parse_recipe_args("Button old-lib --to --scope all", &required, &optional).unwrap_err(); - - assert!(err.to_string().contains("Missing value for --to")); - } - - #[test] - fn parse_recipe_args_errors_on_extra_positionals() { - let component = required_param("component"); - let from = required_param("from"); - let required = vec![&component, &from]; - - let err = parse_recipe_args("Button old-lib extra", &required, &[]).unwrap_err(); - - assert!(err - .to_string() - .contains("Unexpected positional argument: extra")); - } - - #[test] - fn parse_recipe_args_errors_on_unknown_flag() { - let component = required_param("component"); - let required = vec![&component]; - - let err = parse_recipe_args("Button --unknown value", &required, &[]).unwrap_err(); - - assert!(err.to_string().contains("Unknown parameter: --unknown")); - } } diff --git a/crates/goose/src/slash_commands/recipe_slash_command.rs b/crates/goose/src/slash_commands/recipe_slash_command.rs index d19a29e08397..42ce3fbd59a3 100644 --- a/crates/goose/src/slash_commands/recipe_slash_command.rs +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -1,13 +1,14 @@ use std::path::PathBuf; -use anyhow::Result; +use anyhow::{anyhow, Result}; use serde::{Deserialize, Serialize}; use tracing::warn; use super::types::{SlashCommandEntry, SlashCommandSource}; use super::util::normalize_command_name; use crate::config::Config; -use crate::recipe::{RecipeParameter, RecipeParameterRequirement}; +use crate::recipe::build_recipe::{build_recipe_from_template, RecipeError}; +use crate::recipe::{RecipeParameter, RecipeParameterRequirement, Response}; const SLASH_COMMANDS_CONFIG_KEY: &str = "slash_commands"; @@ -63,9 +64,7 @@ pub fn get_recipe_for_command(command: &str) -> Option { .map(|mapping| PathBuf::from(mapping.recipe_path)) } -pub(super) fn commands_from_mappings( - mappings: Vec, -) -> Vec { +pub(super) fn commands_from_mappings(mappings: Vec) -> Vec { mappings .into_iter() .filter_map(|mapping| { @@ -141,11 +140,245 @@ fn input_hint_for_recipe(params: Option<&Vec>) -> Option String { + format!("Recipe /{} is not valid: {}", command, reason) +} + +pub fn resolve_command( + command: &str, + params_str: &str, +) -> Result, String)>, String> { + let full_command = format!("/{}", command); + let Some(recipe_path) = get_recipe_for_command(&full_command) else { + return Ok(None); + }; + + if !recipe_path.exists() { + return Ok(None); + } + + let recipe_content = + std::fs::read_to_string(&recipe_path).map_err(|e| invalid_recipe_msg(command, e))?; + + let recipe_dir = recipe_path + .parent() + .ok_or_else(|| invalid_recipe_msg(command, "unable to resolve recipe directory"))?; + + let recipe_dir_str = recipe_dir.display().to_string(); + let validation_result = crate::recipe::validate_recipe::validate_recipe_template_from_content( + &recipe_content, + Some(recipe_dir_str), + ) + .map_err(|e| invalid_recipe_msg(command, e))?; + + let empty_params: Vec = Vec::new(); + let all_params = validation_result + .parameters + .as_ref() + .unwrap_or(&empty_params); + let required: Vec<&RecipeParameter> = all_params + .iter() + .filter(|p| { + matches!( + p.requirement, + RecipeParameterRequirement::Required | RecipeParameterRequirement::UserPrompt + ) + }) + .collect(); + let optional: Vec<&RecipeParameter> = all_params + .iter() + .filter(|p| matches!(p.requirement, RecipeParameterRequirement::Optional)) + .collect(); + + let param_values: Vec<(String, String)> = if params_str.is_empty() { + vec![] + } else if required.len() == 1 && optional.is_empty() { + vec![(required[0].key.clone(), params_str.to_string())] + } else { + parse_recipe_args(params_str, &required, &optional) + .map_err(|e| format!("Recipe /{}: {}", command, e))? + }; + + let param_values_len = param_values.len(); + + let recipe = build_recipe_from_template( + recipe_content, + recipe_dir, + param_values, + None:: Result>, + ) + .map_err(|e| match e { + RecipeError::MissingParams { parameters } => invalid_recipe_msg( + command, + format!("requires parameter(s): {}.", parameters.join(", "),), + ), + other => invalid_recipe_msg(command, other), + })?; + + let prompt = [recipe.instructions.as_deref(), recipe.prompt.as_deref()] + .into_iter() + .flatten() + .collect::>() + .join("\n\n"); + + Ok(Some((recipe.response, prompt))) +} + +fn parse_recipe_args( + params_str: &str, + required: &[&RecipeParameter], + optional: &[&RecipeParameter], +) -> Result> { + use std::collections::HashSet; + + let tokens = crate::utils::split_command_args(params_str)?; + let known_keys: HashSet<&str> = required + .iter() + .chain(optional.iter()) + .map(|p| p.key.as_str()) + .collect(); + + let mut result = Vec::new(); + let mut required_idx = 0; + let mut i = 0; + + while i < tokens.len() { + let token = &tokens[i]; + if let Some(flag) = token.strip_prefix("--") { + if !known_keys.contains(flag) { + return Err(anyhow!("Unknown parameter: --{}", flag)); + } + let value = tokens + .get(i + 1) + .ok_or_else(|| anyhow!("Missing value for --{}", flag))?; + if value.starts_with("--") { + return Err(anyhow!("Missing value for --{}", flag)); + } + result.push((flag.to_string(), value.clone())); + i += 2; + } else { + if required_idx >= required.len() { + return Err(anyhow!("Unexpected positional argument: {}", token)); + } + result.push((required[required_idx].key.clone(), token.clone())); + required_idx += 1; + i += 1; + } + } + + Ok(result) +} + #[cfg(test)] mod tests { use super::*; + use crate::recipe::RecipeParameterInputType; use tempfile::TempDir; + fn required_param(key: &str) -> RecipeParameter { + RecipeParameter { + key: key.to_string(), + input_type: RecipeParameterInputType::String, + requirement: RecipeParameterRequirement::Required, + description: format!("{key} parameter"), + default: None, + options: None, + } + } + + fn optional_param(key: &str) -> RecipeParameter { + RecipeParameter { + key: key.to_string(), + input_type: RecipeParameterInputType::String, + requirement: RecipeParameterRequirement::Optional, + description: format!("{key} parameter"), + default: Some("default".to_string()), + options: None, + } + } + + #[test] + fn parse_recipe_args_maps_required_positionals_and_optional_flags() { + let component = required_param("component"); + let from = required_param("from"); + let to = optional_param("to"); + let scope = optional_param("scope"); + let required = vec![&component, &from]; + let optional = vec![&to, &scope]; + + let parsed = parse_recipe_args( + r#""Button Group" old-lib --to new-lib"#, + &required, + &optional, + ) + .unwrap(); + + assert_eq!( + parsed, + vec![ + ("component".to_string(), "Button Group".to_string()), + ("from".to_string(), "old-lib".to_string()), + ("to".to_string(), "new-lib".to_string()), + ] + ); + } + + #[test] + fn parse_recipe_args_allows_values_containing_equals() { + let component = required_param("component"); + let note = optional_param("note"); + let required = vec![&component]; + let optional = vec![¬e]; + + let parsed = parse_recipe_args(r#"Button --note "a=b""#, &required, &optional).unwrap(); + + assert_eq!( + parsed, + vec![ + ("component".to_string(), "Button".to_string()), + ("note".to_string(), "a=b".to_string()), + ] + ); + } + + #[test] + fn parse_recipe_args_errors_when_flag_value_is_another_flag() { + let component = required_param("component"); + let from = required_param("from"); + let to = optional_param("to"); + let scope = optional_param("scope"); + let required = vec![&component, &from]; + let optional = vec![&to, &scope]; + + let err = + parse_recipe_args("Button old-lib --to --scope all", &required, &optional).unwrap_err(); + + assert!(err.to_string().contains("Missing value for --to")); + } + + #[test] + fn parse_recipe_args_errors_on_extra_positionals() { + let component = required_param("component"); + let from = required_param("from"); + let required = vec![&component, &from]; + + let err = parse_recipe_args("Button old-lib extra", &required, &[]).unwrap_err(); + + assert!(err + .to_string() + .contains("Unexpected positional argument: extra")); + } + + #[test] + fn parse_recipe_args_errors_on_unknown_flag() { + let component = required_param("component"); + let required = vec![&component]; + + let err = parse_recipe_args("Button --unknown value", &required, &[]).unwrap_err(); + + assert!(err.to_string().contains("Unknown parameter: --unknown")); + } + #[test] fn commands_from_mappings_use_recipe_description() { let tmp = TempDir::new().unwrap(); diff --git a/crates/goose/src/slash_commands/skill_slash_command.rs b/crates/goose/src/slash_commands/skill_slash_command.rs index 0dc52008123f..e1d605ff0aed 100644 --- a/crates/goose/src/slash_commands/skill_slash_command.rs +++ b/crates/goose/src/slash_commands/skill_slash_command.rs @@ -40,6 +40,25 @@ pub fn format_installed_skills(working_dir: Option<&Path>) -> String { output } +pub fn resolve_command( + command: &str, + params_str: &str, + working_dir: Option<&Path>, +) -> Result, String> { + let Some(skill) = crate::skills::list_installed_skills(working_dir) + .into_iter() + .find(|skill| skill.name.eq_ignore_ascii_case(command)) + else { + return Ok(None); + }; + + let args = (!params_str.is_empty()).then_some(params_str); + let prompt = crate::skills::loaded_skill_context_with_args(&skill, args) + .map_err(|e| format!("Skill /{}: {}", command, e))?; + + Ok(Some(prompt)) +} + pub(super) fn commands_from_sources(sources: Vec) -> Vec { sources .into_iter() From 3afe8678fe61a59aa22a324f0a5f076b3febbc3f Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 11:17:18 +1000 Subject: [PATCH 12/17] fixed format --- crates/goose/src/slash_commands/recipe_slash_command.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/goose/src/slash_commands/recipe_slash_command.rs b/crates/goose/src/slash_commands/recipe_slash_command.rs index 42ce3fbd59a3..3fcd6c0482ae 100644 --- a/crates/goose/src/slash_commands/recipe_slash_command.rs +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -199,8 +199,6 @@ pub fn resolve_command( .map_err(|e| format!("Recipe /{}: {}", command, e))? }; - let param_values_len = param_values.len(); - let recipe = build_recipe_from_template( recipe_content, recipe_dir, From 1cfb00b5e6663bc859c3ace78b7e6a3f2ea261c8 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 11:42:34 +1000 Subject: [PATCH 13/17] fixed tests --- .../src/slash_commands/skill_slash_command.rs | 40 +++--- .../goose/src/slash_commands/slash_command.rs | 126 ++++++++---------- 2 files changed, 80 insertions(+), 86 deletions(-) diff --git a/crates/goose/src/slash_commands/skill_slash_command.rs b/crates/goose/src/slash_commands/skill_slash_command.rs index e1d605ff0aed..095cb244ee92 100644 --- a/crates/goose/src/slash_commands/skill_slash_command.rs +++ b/crates/goose/src/slash_commands/skill_slash_command.rs @@ -83,7 +83,8 @@ pub(super) fn commands_from_sources(sources: Vec) -> Vec = super::super::slash_command::list_builtin_commands() - .into_iter() - .map(|command| normalize_command_name(&command.name)) - .collect(); - let commands: Vec<_> = commands_from_sources(vec![ - source_entry(SourceType::Skill, "compact", "Skill named compact"), - source_entry(SourceType::Skill, "review", "Review code"), - ]) - .into_iter() - .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))) - .collect(); - let names: Vec<_> = commands + fn list_commands_loads_project_skill_from_disk() { + let tmp = TempDir::new().unwrap(); + let skill_dir = tmp + .path() + .join(".agents") + .join("skills") + .join("code-review"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: code-review\ndescription: Review changed code\nmetadata:\n argument-hint: \"[task]\"\n arguments:\n - task\n---\nReview the diff.", + ) + .unwrap(); + + let commands = list_commands(Some(tmp.path())); + let command = commands .iter() - .map(|command| command.name.as_str()) - .collect(); + .find(|command| command.name == "code-review") + .expect("project skill should be listed"); - assert_eq!(names, vec!["review"]); + assert_eq!(command.description, "Review changed code"); + assert_eq!(command.source, SlashCommandSource::Skill); + assert_eq!(command.input_hint.as_deref(), Some("[task]")); } fn source_entry(source_type: SourceType, name: &str, description: &str) -> SourceEntry { diff --git a/crates/goose/src/slash_commands/slash_command.rs b/crates/goose/src/slash_commands/slash_command.rs index 5be4fcb5f578..e1c000dd6094 100644 --- a/crates/goose/src/slash_commands/slash_command.rs +++ b/crates/goose/src/slash_commands/slash_command.rs @@ -17,24 +17,36 @@ pub fn list_builtin_commands() -> Vec { } pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { - let mut commands = list_builtin_commands(); + merge_command_sources( + list_builtin_commands(), + super::recipe_slash_command::commands_from_mappings( + super::recipe_slash_command::list_commands(), + ), + super::skill_slash_command::list_commands(working_dir), + ) +} + +pub(super) fn merge_command_sources( + builtins: Vec, + recipes: Vec, + skills: Vec, +) -> Vec { + let mut commands = builtins; let mut reserved_names: HashSet = commands .iter() .map(|command| normalize_command_name(&command.name)) .collect(); - for command in super::recipe_slash_command::commands_from_mappings( - super::recipe_slash_command::list_commands(), - ) { - if reserved_names.insert(command.name.clone()) { + for command in recipes { + if reserved_names.insert(normalize_command_name(&command.name)) { commands.push(command); } } commands.extend( - super::skill_slash_command::list_commands(working_dir) + skills .into_iter() - .filter(|command| !reserved_names.contains(&command.name)), + .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))), ); commands } @@ -50,7 +62,6 @@ fn builtin_input_hint(command: &str) -> Option<&'static str> { #[cfg(test)] mod tests { use super::*; - use tempfile::TempDir; #[test] fn lists_acp_safe_builtin_commands() { @@ -93,73 +104,50 @@ mod tests { assert_eq!(compact.input_hint, None); } + fn entry(name: &str, source: SlashCommandSource) -> SlashCommandEntry { + SlashCommandEntry { + name: name.to_string(), + description: format!("{name} description"), + source, + input_hint: None, + } + } + #[test] - fn lists_project_skills_as_acp_commands() { - let tmp = TempDir::new().unwrap(); - let skill_dir = tmp - .path() - .join(".agents") - .join("skills") - .join("code-review"); - std::fs::create_dir_all(&skill_dir).unwrap(); - std::fs::write( - skill_dir.join("SKILL.md"), - "---\nname: code-review\ndescription: Review changed code\nmetadata:\n argument-hint: \"[task]\"\n arguments:\n - task\n---\nReview the diff.", - ) - .unwrap(); - - let commands = list_acp_commands(Some(tmp.path())); - let command = commands - .iter() - .find(|command| command.name == "code-review") - .expect("project skill should be listed as an ACP command"); + fn merge_recipe_wins_over_skill_on_name_collision() { + let merged = merge_command_sources( + vec![entry("compact", SlashCommandSource::Builtin)], + vec![entry("review", SlashCommandSource::Recipe)], + vec![entry("review", SlashCommandSource::Skill)], + ); - assert_eq!(command.description, "Review changed code"); - assert_eq!(command.source, SlashCommandSource::Skill); - assert_eq!(command.input_hint.as_deref(), Some("[task]")); + let review: Vec<_> = merged.iter().filter(|c| c.name == "review").collect(); + assert_eq!(review.len(), 1); + assert_eq!(review[0].source, SlashCommandSource::Recipe); } #[test] - fn recipe_commands_reserve_names_before_skills() { - let tmp = TempDir::new().unwrap(); - let recipe_path = tmp.path().join("review.yaml"); - std::fs::write( - &recipe_path, - "version: 1.0.0\ntitle: Review Recipe\ndescription: Review with a recipe\ninstructions: Review the change\n", - ) - .unwrap(); - let mut commands = list_builtin_commands(); - let mut reserved_names: HashSet = commands - .iter() - .map(|command| normalize_command_name(&command.name)) - .collect(); - for command in super::recipe_slash_command::commands_from_mappings(vec![ - super::recipe_slash_command::SlashCommandMapping { - command: "review".to_string(), - recipe_path: recipe_path.to_string_lossy().to_string(), - }, - ]) { - let name = normalize_command_name(&command.name); - if reserved_names.insert(name) { - commands.push(command); - } - } - let skill_command = SlashCommandEntry { - name: "review".to_string(), - description: "Review code".to_string(), - source: SlashCommandSource::Skill, - input_hint: None, - }; - if !reserved_names.contains(&normalize_command_name(&skill_command.name)) { - commands.push(skill_command); - } + fn merge_builtin_wins_over_recipe_and_skill() { + let merged = merge_command_sources( + vec![entry("compact", SlashCommandSource::Builtin)], + vec![entry("compact", SlashCommandSource::Recipe)], + vec![entry("compact", SlashCommandSource::Skill)], + ); - let review_commands: Vec<_> = commands - .iter() - .filter(|command| command.name == "review") - .collect(); + let compact: Vec<_> = merged.iter().filter(|c| c.name == "compact").collect(); + assert_eq!(compact.len(), 1); + assert_eq!(compact[0].source, SlashCommandSource::Builtin); + } + + #[test] + fn merge_dedupes_by_normalized_name() { + let merged = merge_command_sources( + vec![entry("Compact", SlashCommandSource::Builtin)], + vec![entry("/compact", SlashCommandSource::Recipe)], + vec![entry("COMPACT", SlashCommandSource::Skill)], + ); - assert_eq!(review_commands.len(), 1); - assert_eq!(review_commands[0].source, SlashCommandSource::Recipe); + assert_eq!(merged.len(), 1); + assert_eq!(merged[0].source, SlashCommandSource::Builtin); } } From 3f30d691a5dd6be948dd2affe25c741ae9f47276 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 13:57:02 +1000 Subject: [PATCH 14/17] fixed single requirement param with optional param for recipe command --- .../slash_commands/recipe_slash_command.rs | 108 +++++++++++++++--- ui/desktop/src/components/MentionPopover.tsx | 5 +- 2 files changed, 95 insertions(+), 18 deletions(-) diff --git a/crates/goose/src/slash_commands/recipe_slash_command.rs b/crates/goose/src/slash_commands/recipe_slash_command.rs index 3fcd6c0482ae..40591bfb3ba8 100644 --- a/crates/goose/src/slash_commands/recipe_slash_command.rs +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::path::PathBuf; use anyhow::{anyhow, Result}; @@ -227,8 +228,6 @@ fn parse_recipe_args( required: &[&RecipeParameter], optional: &[&RecipeParameter], ) -> Result> { - use std::collections::HashSet; - let tokens = crate::utils::split_command_args(params_str)?; let known_keys: HashSet<&str> = required .iter() @@ -236,10 +235,9 @@ fn parse_recipe_args( .map(|p| p.key.as_str()) .collect(); - let mut result = Vec::new(); - let mut required_idx = 0; + let mut positionals: Vec = Vec::new(); + let mut flags: Vec<(String, String)> = Vec::new(); let mut i = 0; - while i < tokens.len() { let token = &tokens[i]; if let Some(flag) = token.strip_prefix("--") { @@ -248,22 +246,28 @@ fn parse_recipe_args( } let value = tokens .get(i + 1) + .filter(|v| !v.starts_with("--")) .ok_or_else(|| anyhow!("Missing value for --{}", flag))?; - if value.starts_with("--") { - return Err(anyhow!("Missing value for --{}", flag)); - } - result.push((flag.to_string(), value.clone())); + flags.push((flag.to_string(), value.clone())); i += 2; } else { - if required_idx >= required.len() { - return Err(anyhow!("Unexpected positional argument: {}", token)); - } - result.push((required[required_idx].key.clone(), token.clone())); - required_idx += 1; + positionals.push(token.clone()); i += 1; } } + let mut result = Vec::new(); + if required.len() == 1 && !positionals.is_empty() { + result.push((required[0].key.clone(), positionals.join(" "))); + } else { + for (idx, value) in positionals.into_iter().enumerate() { + if idx >= required.len() { + return Err(anyhow!("Unexpected positional argument: {}", value)); + } + result.push((required[idx].key.clone(), value)); + } + } + result.extend(flags); Ok(result) } @@ -321,6 +325,82 @@ mod tests { ); } + #[test] + fn parse_recipe_args_greedy_captures_multi_word_required_with_optional_flag() { + let location = required_param("location"); + let theme = optional_param("theme"); + let required = vec![&location]; + let optional = vec![&theme]; + + let parsed = + parse_recipe_args("Melbourne weather --theme culture", &required, &optional).unwrap(); + + assert_eq!( + parsed, + vec![ + ("location".to_string(), "Melbourne weather".to_string()), + ("theme".to_string(), "culture".to_string()), + ] + ); + } + + #[test] + fn parse_recipe_args_greedy_captures_multi_word_required_without_flags() { + let location = required_param("location"); + let theme = optional_param("theme"); + let required = vec![&location]; + let optional = vec![&theme]; + + let parsed = parse_recipe_args("Melbourne weather", &required, &optional).unwrap(); + + assert_eq!( + parsed, + vec![("location".to_string(), "Melbourne weather".to_string())] + ); + } + + #[test] + fn parse_recipe_args_greedy_still_accepts_quoted_required_value() { + let location = required_param("location"); + let theme = optional_param("theme"); + let required = vec![&location]; + let optional = vec![&theme]; + + let parsed = parse_recipe_args( + r#""Melbourne weather" --theme culture"#, + &required, + &optional, + ) + .unwrap(); + + assert_eq!( + parsed, + vec![ + ("location".to_string(), "Melbourne weather".to_string()), + ("theme".to_string(), "culture".to_string()), + ] + ); + } + + #[test] + fn parse_recipe_args_greedy_handles_flag_before_required_positional() { + let location = required_param("location"); + let theme = optional_param("theme"); + let required = vec![&location]; + let optional = vec![&theme]; + + let parsed = + parse_recipe_args("--theme culture Melbourne weather", &required, &optional).unwrap(); + + assert_eq!( + parsed, + vec![ + ("location".to_string(), "Melbourne weather".to_string()), + ("theme".to_string(), "culture".to_string()), + ] + ); + } + #[test] fn parse_recipe_args_allows_values_containing_equals() { let component = required_param("component"); diff --git a/ui/desktop/src/components/MentionPopover.tsx b/ui/desktop/src/components/MentionPopover.tsx index 65c78c3bc21d..80d0933abd20 100644 --- a/ui/desktop/src/components/MentionPopover.tsx +++ b/ui/desktop/src/components/MentionPopover.tsx @@ -455,10 +455,7 @@ const MentionPopover = forwardRef< if (item.itemType === 'Agent') { return '@' + item.name + ' '; } - if (item.itemType === 'Skill') { - return `Use the ${item.name} skill to `; - } - if (['Builtin', 'Recipe'].includes(item.itemType)) { + if (['Builtin', 'Recipe', 'Skill'].includes(item.itemType)) { return '/' + item.name; } return item.extra; From 357de40ed0b920d692a10b66dc46699587ff663a Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 14:29:03 +1000 Subject: [PATCH 15/17] address comments and clenaup --- crates/goose/src/recipe/build_recipe/mod.rs | 40 ------------------- crates/goose/src/skills/arguments.rs | 12 ++++++ .../slash_commands/recipe_slash_command.rs | 23 +++++++++++ crates/goose/src/utils.rs | 13 +++++- 4 files changed, 47 insertions(+), 41 deletions(-) diff --git a/crates/goose/src/recipe/build_recipe/mod.rs b/crates/goose/src/recipe/build_recipe/mod.rs index a69abfb705d9..a08d3fc27b36 100644 --- a/crates/goose/src/recipe/build_recipe/mod.rs +++ b/crates/goose/src/recipe/build_recipe/mod.rs @@ -75,46 +75,6 @@ where Ok(recipe) } -pub fn build_recipe_from_template_with_positional_params( - recipe_content: String, - recipe_dir: &Path, - params: Vec, - user_prompt_fn: Option, -) -> Result -where - F: Fn(&str, &str) -> Result, -{ - let recipe_dir_str = recipe_dir.display().to_string(); - - let recipe_parameters = - validate_recipe_template_from_content(&recipe_content, Some(recipe_dir_str.clone())) - .map_err(|source| RecipeError::Invalid { source })? - .parameters; - - let param_pairs: Vec<(String, String)> = if let Some(recipe_params) = &recipe_parameters { - let required_count = recipe_params.iter().filter(|p| p.default.is_none()).count(); - if params.len() < required_count { - let required_keys: Vec = recipe_params - .iter() - .filter(|p| p.default.is_none()) - .map(|p| p.key.clone()) - .collect(); - return Err(RecipeError::MissingParams { - parameters: required_keys, - }); - } - recipe_params - .iter() - .zip(params.iter()) - .map(|(rp, p)| (rp.key.clone(), p.clone())) - .collect() - } else { - vec![] - }; - - build_recipe_from_template(recipe_content, recipe_dir, param_pairs, user_prompt_fn) -} - pub fn apply_values_to_parameters( user_params: &[(String, String)], recipe_parameters: Option>, diff --git a/crates/goose/src/skills/arguments.rs b/crates/goose/src/skills/arguments.rs index 07a3eb86c681..beb1306e5bfb 100644 --- a/crates/goose/src/skills/arguments.rs +++ b/crates/goose/src/skills/arguments.rs @@ -124,6 +124,18 @@ mod tests { assert_eq!(out, "name=Andy, addr=57 Collins"); } + #[test] + fn apostrophes_in_unquoted_arguments_stay_literal() { + let out = apply_skill_arguments( + "author=$author, contraction=$contraction", + "O'Reilly don't", + &names(&["author", "contraction"]), + ) + .unwrap(); + + assert_eq!(out, "author=O'Reilly, contraction=don't"); + } + #[test] fn arguments_keeps_raw_quotes_while_positional_strips_them() { let out = diff --git a/crates/goose/src/slash_commands/recipe_slash_command.rs b/crates/goose/src/slash_commands/recipe_slash_command.rs index 40591bfb3ba8..2f19ac88cc28 100644 --- a/crates/goose/src/slash_commands/recipe_slash_command.rs +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -344,6 +344,29 @@ mod tests { ); } + #[test] + fn parse_recipe_args_keeps_apostrophes_in_unquoted_values() { + let topic = required_param("topic"); + let theme = optional_param("theme"); + let required = vec![&topic]; + let optional = vec![&theme]; + + let parsed = parse_recipe_args( + "O'Reilly's guide --theme author's-pick", + &required, + &optional, + ) + .unwrap(); + + assert_eq!( + parsed, + vec![ + ("topic".to_string(), "O'Reilly's guide".to_string()), + ("theme".to_string(), "author's-pick".to_string()), + ] + ); + } + #[test] fn parse_recipe_args_greedy_captures_multi_word_required_without_flags() { let location = required_param("location"); diff --git a/crates/goose/src/utils.rs b/crates/goose/src/utils.rs index 9ca485eb15cf..b78d4a4a6960 100644 --- a/crates/goose/src/utils.rs +++ b/crates/goose/src/utils.rs @@ -56,7 +56,9 @@ pub fn split_command_args(input: &str) -> anyhow::Result> { for c in input.chars() { match c { '"' if !in_single_quote => in_double_quote = !in_double_quote, - '\'' if !in_double_quote => in_single_quote = !in_single_quote, + '\'' if !in_double_quote && (in_single_quote || current.is_empty()) => { + in_single_quote = !in_single_quote + } c if c.is_whitespace() && !in_double_quote && !in_single_quote => { if !current.is_empty() { parts.push(std::mem::take(&mut current)); @@ -184,8 +186,17 @@ mod tests { ); } + #[test] + fn test_split_command_args_apostrophes_in_unquoted_words() { + assert_eq!( + split_command_args("O'Reilly wrote don't split").unwrap(), + vec!["O'Reilly", "wrote", "don't", "split"] + ); + } + #[test] fn test_split_command_args_unmatched_quote() { assert!(split_command_args(r#""unmatched"#).is_err()); + assert!(split_command_args("'unmatched").is_err()); } } From 6dbcad75a7a8584627ab1c6ff3621e6a1a418139 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 14:51:53 +1000 Subject: [PATCH 16/17] remove builtin command hint --- .../goose/src/slash_commands/slash_command.rs | 34 +------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/crates/goose/src/slash_commands/slash_command.rs b/crates/goose/src/slash_commands/slash_command.rs index e1c000dd6094..add3f543606c 100644 --- a/crates/goose/src/slash_commands/slash_command.rs +++ b/crates/goose/src/slash_commands/slash_command.rs @@ -11,7 +11,7 @@ pub fn list_builtin_commands() -> Vec { name: command.name.to_string(), description: command.description.to_string(), source: SlashCommandSource::Builtin, - input_hint: builtin_input_hint(command.name).map(str::to_string), + input_hint: None, }) .collect() } @@ -51,14 +51,6 @@ pub(super) fn merge_command_sources( commands } -fn builtin_input_hint(command: &str) -> Option<&'static str> { - match command { - "prompt" => Some(" [--info] [key=value...]"), - "prompts" => Some("[--extension ]"), - _ => None, - } -} - #[cfg(test)] mod tests { use super::*; @@ -80,30 +72,6 @@ mod tests { .all(|command| command.source == SlashCommandSource::Builtin)); } - #[test] - fn includes_input_hints_for_argument_taking_builtins() { - let commands = list_builtin_commands(); - let prompt = commands - .iter() - .find(|command| command.name == "prompt") - .expect("prompt command should be listed"); - let prompts = commands - .iter() - .find(|command| command.name == "prompts") - .expect("prompts command should be listed"); - let compact = commands - .iter() - .find(|command| command.name == "compact") - .expect("compact command should be listed"); - - assert_eq!( - prompt.input_hint.as_deref(), - Some(" [--info] [key=value...]") - ); - assert_eq!(prompts.input_hint.as_deref(), Some("[--extension ]")); - assert_eq!(compact.input_hint, None); - } - fn entry(name: &str, source: SlashCommandSource) -> SlashCommandEntry { SlashCommandEntry { name: name.to_string(), From 24c91ba67b1f84cfafb0735d99753fe35b20c43a Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 15 May 2026 15:08:37 +1000 Subject: [PATCH 17/17] more recipe command argument parsing --- .../slash_commands/recipe_slash_command.rs | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/goose/src/slash_commands/recipe_slash_command.rs b/crates/goose/src/slash_commands/recipe_slash_command.rs index 2f19ac88cc28..78e80e272111 100644 --- a/crates/goose/src/slash_commands/recipe_slash_command.rs +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -229,11 +229,8 @@ fn parse_recipe_args( optional: &[&RecipeParameter], ) -> Result> { let tokens = crate::utils::split_command_args(params_str)?; - let known_keys: HashSet<&str> = required - .iter() - .chain(optional.iter()) - .map(|p| p.key.as_str()) - .collect(); + let required_keys: HashSet<&str> = required.iter().map(|p| p.key.as_str()).collect(); + let optional_keys: HashSet<&str> = optional.iter().map(|p| p.key.as_str()).collect(); let mut positionals: Vec = Vec::new(); let mut flags: Vec<(String, String)> = Vec::new(); @@ -241,7 +238,14 @@ fn parse_recipe_args( while i < tokens.len() { let token = &tokens[i]; if let Some(flag) = token.strip_prefix("--") { - if !known_keys.contains(flag) { + if required_keys.contains(flag) { + return Err(anyhow!( + "Parameter '{}' is required; pass it positionally, not as --{}", + flag, + flag + )); + } + if !optional_keys.contains(flag) { return Err(anyhow!("Unknown parameter: --{}", flag)); } let value = tokens @@ -470,6 +474,21 @@ mod tests { .contains("Unexpected positional argument: extra")); } + #[test] + fn parse_recipe_args_rejects_required_param_passed_as_flag() { + let component = required_param("component"); + let from = required_param("from"); + let required = vec![&component, &from]; + + let err = parse_recipe_args("--component Button old-lib", &required, &[]).unwrap_err(); + + let msg = err.to_string(); + assert!( + msg.contains("'component' is required") && msg.contains("--component"), + "unexpected error: {msg}" + ); + } + #[test] fn parse_recipe_args_errors_on_unknown_flag() { let component = required_param("component");