diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index 0bc6b8736758..1c0df24aca69 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -1117,7 +1117,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-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index 82202f610b66..08646e939df9 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, + slash_commands::recipe_slash_command, }; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -469,7 +469,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_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 dcb452fca249..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, 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 = 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 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 b0b5e4f92dae..ac99fe02a896 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -1164,6 +1164,34 @@ fn validate_absolute_cwd(cwd: &Path) -> Result<(), agent_client_protocol::Error> } impl GooseAcpAgent { + fn available_commands_update(working_dir: &std::path::Path) -> AvailableCommandsUpdate { + 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); + 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, + working_dir: &std::path::Path, + ) -> Result<(), agent_client_protocol::Error> { + cx.send_notification(SessionNotification::new( + session_id.clone(), + SessionUpdate::AvailableCommandsUpdate(Self::available_commands_update(working_dir)), + )) + } + pub fn permission_manager(&self) -> Arc { Arc::clone(&self.permission_manager) } @@ -2301,92 +2329,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('/'); @@ -2647,6 +2589,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, @@ -2672,10 +2616,7 @@ impl GooseAcpAgent { SessionUpdate::UsageUpdate(usage_update), ))?; } - - self.send_available_commands_update(cx, &acp_session_id) - .await?; - + Self::send_available_commands_update(cx, &acp_session_id, &working_dir)?; debug!( target: "perf", sid = %sid, @@ -3046,10 +2987,7 @@ impl GooseAcpAgent { SessionUpdate::UsageUpdate(usage_update), ))?; } - - self.send_available_commands_update(cx, &args.session_id) - .await?; - + Self::send_available_commands_update(cx, &args.session_id, &args.cwd)?; debug!( target: "perf", sid = %sid, @@ -3082,7 +3020,9 @@ 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::slash_commands::recipe_slash_command::get_recipe_for_command( + &full_command, + ) { if recipe_path.exists() { cx.send_notification(SessionNotification::new( @@ -3573,11 +3513,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(), @@ -3587,8 +3529,6 @@ impl GooseAcpAgent { let meta = session_meta(&new_session); - let acp_session_id = SessionId::new(new_session_id); - let mut response = ForkSessionResponse::new(acp_session_id.clone()) .modes(mode_state) .meta(meta); @@ -3599,10 +3539,7 @@ impl GooseAcpAgent { if let Some(co) = config_options { response = response.config_options(co); } - - self.send_available_commands_update(cx, &acp_session_id) - .await?; - + Self::send_available_commands_update(cx, &acp_session_id, &args.cwd)?; Ok(response) } diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 843093ee2e1a..3e4a00598d3a 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -1403,17 +1403,6 @@ impl Agent { .await; } - // Track custom slash command usage (don't track command name for privacy) - 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() { - #[cfg(feature = "telemetry")] - crate::posthog::emit_custom_slash_command_used(); - } - } - } - let command_result = self .execute_command(&message_text, &session_config.id) .await; diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index f3379faee39a..e4e1a8c34bc2 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -4,7 +4,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::slash_commands::{recipe_slash_command, skill_slash_command}; use super::Agent; @@ -113,7 +113,16 @@ impl Agent { "goal" => self.handle_goal_command(params_str).await, "grind" => self.handle_grind_command(params_str).await, _ => { - self.handle_recipe_command(command, params_str, session_id) + if let Some(message) = self + .handle_recipe_command(command, params_str, session_id) + .await? + { + #[cfg(feature = "telemetry")] + crate::posthog::emit_custom_slash_command_used(); + return Ok(Some(message)); + } + + self.handle_skill_command(command, params_str, session_id) .await } } @@ -170,9 +179,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 @@ -180,34 +186,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))) } @@ -366,110 +345,35 @@ impl Agent { params_str: &str, _session_id: &str, ) -> Result> { - let full_command = format!("/{}", command); - let recipe_path = match crate::slash_commands::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 param_values: Vec = if params_str.is_empty() { - vec![] - } 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)); + 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))) } - }; - - let param_values_len = param_values.len(); - - let recipe = match build_recipe_from_template_with_positional_params( - 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 - )))); - } - Err(e) => return Err(anyhow!("Failed to build recipe: {}", e)), - }; - - self.apply_recipe_components(recipe.response.clone(), true) - .await; + Err(text) => Ok(Some(Message::assistant().with_text(text))), + } + } - let prompt = [recipe.instructions.as_deref(), recipe.prompt.as_deref()] - .into_iter() - .flatten() - .collect::>() - .join("\n\n"); + 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); - 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))), + } } async fn handle_goal_command(&self, params_str: &str) -> Result> { 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 new file mode 100644 index 000000000000..beb1306e5bfb --- /dev/null +++ b/crates/goose/src/skills/arguments.rs @@ -0,0 +1,244 @@ +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 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 = + 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 13bf933568d8..e26f37dac581 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::loaded_skill_context_with_args; use crate::agents::extension::PlatformExtensionContext; use crate::agents::mcp_client::{Error, McpClientTrait}; use crate::agents::ToolCallContext; @@ -49,6 +50,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." } } }); @@ -60,6 +65,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(), @@ -97,36 +103,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) { - 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 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: {}", + 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 78e8299985b1..bc73e86803b8 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; @@ -11,6 +12,8 @@ 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 arguments::apply_skill_arguments; use goose_sdk::custom_requests::{SourceEntry, SourceType}; use serde::Deserialize; use serde_json::Value; @@ -96,6 +99,68 @@ pub(crate) fn validate_skill_name(name: &str) -> Result<(), Error> { Ok(()) } +fn loaded_skill_context(skill: &SourceEntry, content: &str) -> String { + let title = format!("{} ({})", skill.name, skill.source_type); + let mut output = format!( + "# Loaded Skill: {title}\n\n{}\n\n## Content\n\n{}\n", + skill.description, content + ); + + 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 +} + +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(loaded_skill_context(skill, &content)) +} + +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()) } @@ -425,3 +490,45 @@ 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 loaded_skill_context_with_args_replaces_arguments_placeholder_with_raw_args() { + let skill = skill_with_content("Review $ARGUMENTS carefully."); + + let rendered = loaded_skill_context_with_args(&skill, Some("src/foo.rs --strict")).unwrap(); + + assert!(rendered.contains("Review src/foo.rs --strict carefully.")); + } + + #[test] + fn loaded_skill_context_with_args_uses_context_without_args() { + let skill = skill_with_content("Review the code carefully."); + + let rendered = loaded_skill_context_with_args(&skill, None).unwrap(); + + assert!(rendered.contains("# Loaded Skill: test-skill (skill)")); + assert!(rendered.contains("## Content\n\nReview the code carefully.")); + } +} diff --git a/crates/goose/src/slash_commands.rs b/crates/goose/src/slash_commands.rs deleted file mode 100644 index 5e7065db0016..000000000000 --- a/crates/goose/src/slash_commands.rs +++ /dev/null @@ -1,74 +0,0 @@ -use std::path::PathBuf; - -use anyhow::Result; -use serde::{Deserialize, Serialize}; -use tracing::warn; - -use crate::config::Config; -use crate::recipe::Recipe; - -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 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) -} 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..78e80e272111 --- /dev/null +++ b/crates/goose/src/slash_commands/recipe_slash_command.rs @@ -0,0 +1,628 @@ +use std::collections::HashSet; +use std::path::PathBuf; + +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::build_recipe::{build_recipe_from_template, RecipeError}; +use crate::recipe::{RecipeParameter, RecipeParameterRequirement, Response}; + +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(" "), + ) +} + +fn invalid_recipe_msg(command: &str, reason: impl std::fmt::Display) -> 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 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> { + let tokens = crate::utils::split_command_args(params_str)?; + 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(); + let mut i = 0; + while i < tokens.len() { + let token = &tokens[i]; + if let Some(flag) = token.strip_prefix("--") { + 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 + .get(i + 1) + .filter(|v| !v.starts_with("--")) + .ok_or_else(|| anyhow!("Missing value for --{}", flag))?; + flags.push((flag.to_string(), value.clone())); + i += 2; + } else { + 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) +} + +#[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_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_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"); + 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"); + 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_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"); + 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(); + 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..095cb244ee92 --- /dev/null +++ b/crates/goose/src/slash_commands/skill_slash_command.rs @@ -0,0 +1,162 @@ +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 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() + .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; + use tempfile::TempDir; + + #[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 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() + .find(|command| command.name == "code-review") + .expect("project skill should be listed"); + + 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 { + 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..8a983fc3c3dd --- /dev/null +++ b/crates/goose/src/slash_commands/slash_command.rs @@ -0,0 +1,121 @@ +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: None, + }) + .collect() +} + +pub fn list_acp_commands(working_dir: Option<&Path>) -> Vec { + 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 recipes { + if reserved_names.insert(normalize_command_name(&command.name)) { + commands.push(command); + } + } + + commands.extend( + skills + .into_iter() + .filter(|command| !reserved_names.contains(&normalize_command_name(&command.name))), + ); + commands +} + +#[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", "goal", "grind"] + ); + assert!(commands + .iter() + .all(|command| command.source == SlashCommandSource::Builtin)); + } + + fn entry(name: &str, source: SlashCommandSource) -> SlashCommandEntry { + SlashCommandEntry { + name: name.to_string(), + description: format!("{name} description"), + source, + input_hint: None, + } + } + + #[test] + 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)], + ); + + 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 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 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!(merged.len(), 1); + assert_eq!(merged[0].source, SlashCommandSource::Builtin); + } +} 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() +} diff --git a/crates/goose/src/utils.rs b/crates/goose/src/utils.rs index c165928c8492..b78d4a4a6960 100644 --- a/crates/goose/src/utils.rs +++ b/crates/goose/src/utils.rs @@ -47,6 +47,38 @@ 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 || 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)); + } + } + _ => 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 +157,46 @@ 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_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()); + } } 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;