From 4d3544dc0e131c2a0140b7ee68c2583801684d69 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 14:16:08 -0400 Subject: [PATCH] fix(desktop): shell-split MCP command field and retry on spawn failure D2: The MCP editor's command field is passed verbatim to Command::new(), so "uv run /path/to/jambot" tries to find a single binary with that full string as its name. Shell-split in configured_mcp_server(): first whitespace token becomes the executable, remaining tokens prepend before user-supplied args. D1: When a user-configured MCP server fails to spawn, the ACP agent returns a JSON-RPC error that previously killed the entire turn. Catch MCP-related AgentError in create_session_and_apply_model and retry session creation with empty mcp_servers so the agent can still respond. Surface the original error via observer as a warning. --- crates/buzz-acp/src/lib.rs | 46 +++++++++++++++++++++++++++++++++++-- crates/buzz-acp/src/pool.rs | 29 +++++++++++++++++++++-- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 59635dcbb..71afa0a9c 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3454,10 +3454,20 @@ fn build_mcp_servers(config: &Config) -> Vec { } fn configured_mcp_server(server: &config::ConfiguredMcpServer) -> McpServer { + // Shell-split: users may type `uv run /path/to/jambot` as the command. + // First whitespace-delimited token is the executable; remaining tokens + // are prepended before `server.args`. + let mut parts = server.command.split_whitespace(); + let command = parts.next().unwrap_or_default().to_string(); + let prefix_args: Vec = parts.map(String::from).collect(); + + let mut args = prefix_args; + args.extend(server.args.iter().cloned()); + McpServer { name: server.name.clone(), - command: server.command.clone(), - args: server.args.clone(), + command, + args, env: server .env .iter() @@ -4068,6 +4078,38 @@ mod build_mcp_servers_tests { "Path::new(\".\").file_stem() is None — should fall back to \"mcp\"" ); } + + #[test] + fn configured_mcp_server_shell_splits_command() { + let server = config::ConfiguredMcpServer { + name: "jambot".into(), + command: "uv run /path/to/jambot".into(), + args: vec!["--port".into(), "8080".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + assert_eq!(result.command, "uv"); + assert_eq!( + result.args, + vec!["run", "/path/to/jambot", "--port", "8080"], + ); + assert_eq!(result.name, "jambot"); + } + + #[test] + fn configured_mcp_server_bare_command_unchanged() { + let server = config::ConfiguredMcpServer { + name: "github".into(), + command: "npx".into(), + args: vec!["github-mcp".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + assert_eq!(result.command, "npx"); + assert_eq!(result.args, vec!["github-mcp"]); + } } #[cfg(test)] diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index e57f44882..6b6f931c0 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -707,14 +707,39 @@ async fn create_session_and_apply_model( None }; - let resp = agent + let resp = match agent .acp .session_new_full( &ctx.cwd, ctx.mcp_servers.clone(), combined_system_prompt.as_deref(), ) - .await?; + .await + { + Ok(r) => r, + Err(AcpError::AgentError { ref message, .. }) + if !ctx.mcp_servers.is_empty() && message.to_lowercase().contains("mcp") => + { + // MCP spawn failure — retry without MCP servers so the agent can + // still respond. Surface the original error as a warning. + tracing::warn!( + target: "pool::mcp", + "MCP server spawn failed ({message}); retrying session without MCP servers" + ); + agent.acp.observe( + "mcp_spawn_warning", + serde_json::json!({ + "error": message, + "action": "retried_without_mcp_servers", + }), + ); + agent + .acp + .session_new_full(&ctx.cwd, vec![], combined_system_prompt.as_deref()) + .await? + } + Err(e) => return Err(e), + }; // Populate model capabilities on first session creation. if agent.model_capabilities.is_none() {