Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/goose-cli/src/commands/configure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
42 changes: 6 additions & 36 deletions crates/goose-cli/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,36 +239,6 @@ pub async fn classify_planner_response(
}
}

pub fn split_quoted(input: &str) -> Result<Vec<String>> {
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(
Expand Down Expand Up @@ -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<ExtensionConfig> {
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() {
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions crates/goose-server/src/routes/config_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -415,7 +415,7 @@ pub struct SlashCommandsQuery {
pub async fn get_slash_commands(
axum::extract::Query(query): axum::extract::Query<SlashCommandsQuery>,
) -> Result<Json<SlashCommandsResponse>, 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(),
Expand Down
6 changes: 3 additions & 3 deletions crates/goose-server/src/routes/recipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
Expand Down
141 changes: 39 additions & 102 deletions crates/goose/src/acp/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,34 @@ 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_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<Client>,
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<PermissionManager> {
Arc::clone(&self.permission_manager)
}
Expand Down Expand Up @@ -2160,92 +2188,6 @@ impl GooseAcpAgent {
Ok(())
}

fn input_hint_for_recipe(
params: Option<&Vec<crate::recipe::RecipeParameter>>,
) -> Option<String> {
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<AvailableCommand> {
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<Client>,
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('/');

Expand Down Expand Up @@ -2502,6 +2444,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,
Expand All @@ -2527,10 +2471,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,
Expand Down Expand Up @@ -2895,10 +2836,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)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use persisted cwd when publishing load_session commands

on_load_session builds the available-command list from args.cwd, but slash-skill execution later resolves skills from the session’s stored working directory (handle_skill_command reads it via session_manager.get_session). When those differ (for example clients that call load with a generic cwd like "~"), ACP advertises skills that are not actually runnable in that loaded session, causing /skill commands suggested by discovery to fail at runtime. Build the command update from goose_session.working_dir to keep discovery and execution aligned.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think args.cwd is intentional here. on_load_session updates the persisted session working_dir before publishing commands:

  self.session_manager
      .update(&session_id)
      .working_dir(args.cwd.clone())
      .apply()
      .await?;

Slash-skill execution later reloads the session from session_manager, so it should resolve skills from that same updated cwd.

debug!(
target: "perf",
sid = %sid,
Expand Down Expand Up @@ -2931,7 +2869,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(
Expand Down Expand Up @@ -3377,11 +3317,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(),
Expand All @@ -3391,8 +3333,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);
Expand All @@ -3403,10 +3343,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)
}

Expand Down
4 changes: 3 additions & 1 deletion crates/goose/src/agents/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::get_recipe_for_command(cmd).is_some() {
if crate::slash_commands::recipe_slash_command::get_recipe_for_command(cmd)
.is_some()
{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also pre-existing, but feels like this tracking should be inside Agent.execute_command after we parse

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree!

#[cfg(feature = "telemetry")]
crate::posthog::emit_custom_slash_command_used();
}
Expand Down
Loading
Loading