Skip to content
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
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()
{
#[cfg(feature = "telemetry")]
crate::posthog::emit_custom_slash_command_used();
}
Expand Down
Loading
Loading