diff --git a/Cargo.lock b/Cargo.lock index 96e1bf1ed..67e956711 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -751,6 +751,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.11.0", + "shlex", "thiserror 2.0.18", "tokio", "tokio-tungstenite 0.29.0", diff --git a/crates/buzz-acp/Cargo.toml b/crates/buzz-acp/Cargo.toml index d3afb5062..d26b39bf1 100644 --- a/crates/buzz-acp/Cargo.toml +++ b/crates/buzz-acp/Cargo.toml @@ -71,6 +71,9 @@ toml = "1.0" # Filter expressions evalexpr = { workspace = true } +# Shell-word splitting (MCP server command field) +shlex = "1" + # Process-group kill (safe wrapper around killpg) — Unix-only; kill_process_group # has a #[cfg(not(unix))] fallback in acp.rs. [target.'cfg(unix)'.dependencies] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 71afa0a9c..db15a141a 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3454,12 +3454,17 @@ 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(); + // Shell-word split: users may type `uv run "/My Bot/jambot"` as the command. + // shlex handles quoting so paths with spaces survive. On parse failure + // (unmatched quote), pass the whole string as the command verbatim — the + // spawn will fail, and D1 surfaces MCP spawn errors as warnings. + let (command, prefix_args) = match shlex::split(&server.command) { + Some(mut tokens) if !tokens.is_empty() => { + let cmd = tokens.remove(0); + (cmd, tokens) + } + _ => (server.command.clone(), vec![]), + }; let mut args = prefix_args; args.extend(server.args.iter().cloned()); @@ -4110,6 +4115,39 @@ mod build_mcp_servers_tests { assert_eq!(result.command, "npx"); assert_eq!(result.args, vec!["github-mcp"]); } + + #[test] + fn configured_mcp_server_quoted_path_with_spaces() { + let server = config::ConfiguredMcpServer { + name: "jambot".into(), + command: r#"uv run "/Users/me/My Bot/.venv/bin/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", "/Users/me/My Bot/.venv/bin/jambot", "--port", "8080"], + "quoted path must preserve internal spaces and strip quotes" + ); + } + + #[test] + fn configured_mcp_server_unmatched_quote_falls_back_to_verbatim() { + let server = config::ConfiguredMcpServer { + name: "broken".into(), + command: r#"uv run "/unclosed/path"#.into(), + args: vec!["--flag".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + // Unmatched quote → shlex returns None → verbatim fallback + assert_eq!(result.command, r#"uv run "/unclosed/path"#); + assert_eq!(result.args, vec!["--flag"]); + } } #[cfg(test)] diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 6b6f931c0..a1332590d 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -4900,4 +4900,190 @@ mod tests { "timestamp must not use +00:00 offset" ); } + + // ── MCP fallback (D1) regression tests ─────────────────────────────────── + + use crate::acp::{AcpClient, McpServer}; + use crate::relay::RestClient; + + /// Spawn a bash script as an AcpClient, matching the acp.rs test helper. + async fn spawn_script(script: &str) -> AcpClient { + AcpClient::spawn("bash", &["-c".into(), script.into()], &[], false) + .await + .expect("failed to spawn test script") + } + + /// Build a minimal `PromptContext` with the given MCP servers. + /// Only `mcp_servers` and `cwd` are exercised by `create_session_and_apply_model`; + /// other fields are inert defaults. + fn test_prompt_context(mcp_servers: Vec) -> PromptContext { + let keys = Keys::generate(); + PromptContext { + mcp_servers, + initial_message: None, + idle_timeout: Duration::from_secs(60), + max_turn_duration: Duration::from_secs(60), + turn_liveness_interval: Duration::ZERO, + dedup_mode: crate::config::DedupMode::Drop, + system_prompt: None, + heartbeat_prompt: None, + base_prompt: None, + cwd: "/tmp".to_string(), + rest_client: RestClient { + http: reqwest::Client::new(), + base_url: String::new(), + keys: keys.clone(), + auth_tag_json: None, + }, + channel_info: std::collections::HashMap::new(), + context_message_limit: 0, + max_turns_per_session: 0, + permission_mode: crate::config::PermissionMode::Default, + agent_keys: keys, + agent_owner_pubkey: None, + memory_enabled: false, + harness_name: "test".to_string(), + } + } + + /// Build a minimal `OwnedAgent` from a pre-initialized `AcpClient`. + fn test_owned_agent(acp: AcpClient) -> OwnedAgent { + OwnedAgent { + index: 0, + acp, + state: SessionState { + sessions: HashMap::new(), + heartbeat_session: None, + turn_counts: HashMap::new(), + heartbeat_turn_count: 0, + core_sections: HashMap::new(), + canvas_sections: HashMap::new(), + }, + model_capabilities: None, + desired_model: None, + model_overridden: false, + protocol_version: 1, + } + } + + #[tokio::test] + async fn test_mcp_spawn_error_retries_without_mcp_servers() { + // Script: respond to initialize, then return an MCP error on the first + // session/new, and a valid session on the second (which should carry + // empty mcpServers). + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ1 + echo '{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"mcp: spawn jambot: No such file or directory"}}' + read -t 2 REQ2 + echo '{"jsonrpc":"2.0","id":2,"result":{"sessionId":"ses_fallback","_retryRequest":'"$REQ2"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let mut agent = test_owned_agent(client); + let ctx = test_prompt_context(vec![McpServer { + name: "jambot".into(), + command: "jambot".into(), + args: vec![], + env: vec![], + }]); + + let result = create_session_and_apply_model(&mut agent, &ctx, None, None).await; + let session_id = result.expect("fallback session should succeed"); + assert_eq!(session_id, "ses_fallback"); + } + + #[tokio::test] + async fn test_mcp_retry_sends_empty_mcp_servers() { + // Capture the retry request to a temp file and assert mcpServers: []. + let capture_file = + std::env::temp_dir().join(format!("buzz_mcp_retry_test_{}.json", std::process::id())); + let capture_path = capture_file.display().to_string(); + let script = format!( + r#" + read -t 2 _init + echo '{{"jsonrpc":"2.0","id":0,"result":{{"protocolVersion":1,"agentCapabilities":{{}}}}}}' + read -t 2 _req1 + echo '{{"jsonrpc":"2.0","id":1,"error":{{"code":-32000,"message":"mcp: spawn failed"}}}}' + read -t 2 REQ2 + echo "$REQ2" > {capture_path} + echo '{{"jsonrpc":"2.0","id":2,"result":{{"sessionId":"ses_ok"}}}}' + sleep 1 + "# + ); + let mut client = spawn_script(&script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let mut agent = test_owned_agent(client); + let ctx = test_prompt_context(vec![McpServer { + name: "broken".into(), + command: "broken-server".into(), + args: vec![], + env: vec![], + }]); + + let result = create_session_and_apply_model(&mut agent, &ctx, None, None).await; + let session_id = result.expect("retry should succeed"); + assert_eq!(session_id, "ses_ok"); + + // Read the captured retry request and verify mcpServers is empty. + let captured = std::fs::read_to_string(&capture_file) + .expect("retry request should have been written to capture file"); + std::fs::remove_file(&capture_file).ok(); + let parsed: serde_json::Value = + serde_json::from_str(captured.trim()).expect("captured request should be valid JSON"); + let mcp_servers = parsed + .pointer("/params/mcpServers") + .expect("retry request must contain params.mcpServers"); + assert_eq!( + mcp_servers, + &serde_json::json!([]), + "retry must send empty mcpServers" + ); + } + + #[tokio::test] + async fn test_non_mcp_agent_error_propagates() { + // Non-MCP AgentError must NOT trigger the retry — it should propagate. + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 _req + echo '{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"authentication failed"}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let mut agent = test_owned_agent(client); + let ctx = test_prompt_context(vec![McpServer { + name: "server".into(), + command: "some-server".into(), + args: vec![], + env: vec![], + }]); + + let result = create_session_and_apply_model(&mut agent, &ctx, None, None).await; + assert!( + result.is_err(), + "non-MCP error must propagate, not trigger retry" + ); + let err = result.unwrap_err(); + assert!( + err.to_string().contains("authentication failed"), + "error message must be preserved: {err}" + ); + } } diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index 260de02c1..a58476302 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -708,10 +708,10 @@ packages: dependency: transitive description: name: meta - sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" + sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349" url: "https://pub.dev" source: hosted - version: "1.17.0" + version: "1.18.0" mime: dependency: transitive description: @@ -1137,26 +1137,26 @@ packages: dependency: transitive description: name: test - sha256: "280d6d890011ca966ad08df7e8a4ddfab0fb3aa49f96ed6de56e3521347a9ae7" + sha256: "8d9ceddbab833f180fbefed08afa76d7c03513dfdba87ffcec2718b02bbcbf20" url: "https://pub.dev" source: hosted - version: "1.30.0" + version: "1.31.0" test_api: dependency: transitive description: name: test_api - sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" + sha256: "949a932224383300f01be9221c39180316445ecb8e7547f70a41a35bf421fb9e" url: "https://pub.dev" source: hosted - version: "0.7.10" + version: "0.7.11" test_core: dependency: transitive description: name: test_core - sha256: "0381bd1585d1a924763c308100f2138205252fb90c9d4eeaf28489ee65ccde51" + sha256: "1991d4cfe85d5043241acac92962c3977c8d2f2add1ee73130c7b286417d1d34" url: "https://pub.dev" source: hosted - version: "0.6.16" + version: "0.6.17" tuple: dependency: transitive description: