From 4f51b9e1010e086a16c099cd8d8218ca974a5e18 Mon Sep 17 00:00:00 2001 From: Alex Rosenzweig <64241648+shellz-n-stuff@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:24:25 -0500 Subject: [PATCH] fix(mcp): support Goose discovery handshake (#8037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Goose 1.52 starts MCP connections with `server/discover`. `buzz-dev-mcp` rejects that request with `ExpectedInitializeRequest`, preventing Goose from loading its Buzz tools. Upgrade the tool server to rmcp 3.4.1 and adopt the renamed SDK types. Subprocess tests exercise discovery and legacy initialization, tool listing, file reads/edits, invalid arguments, connection survival, and EOF shutdown. Include the server in both Rust unit-test runners and make the cancellation regression approve its shell call before testing cancellation. The native agent retains its independent rmcp 1.x client. Cargo also unifies the existing 3.x dependency used by the mesh smoke harness. ## Validation - Normal `git push` passed all hooks, including Rust tests, Tauri checks, and repository policy checks. - `just ci` passed on the final stack: repository checks, Rust and Tauri tests/lint, desktop tests/build, web build, and mobile checks/tests. - MCP: 78 existing tests and two subprocess lifecycle tests passed; the discovery regression fails with the old SDK. - All 54 native-agent regressions passed on the final stack, including cancellation of a real MCP shell process. - Previously exercised unmodified Goose 1.52 and 1.47 with Sandpit and the real tool server. Allowed workspace operations and permitted network traffic succeeded; disallowed file and network operations were denied. - Live staging desktop check with Goose 1.52: the reply appeared in Buzz, an allowed request returned HTTP 200, and a disallowed request returned CONNECT 403. ## Stack 1. **#8042 → `main`**: deterministic handoff regression. 2. **This PR → #8042**: Goose MCP discovery compatibility. Merge the lower PR first, then retarget this PR to `main`. This diff contains only the compatibility change and its tests; the relay build repair is already on `main` in #8036. --------- Signed-off-by: Alex Rosenzweig --- Cargo.lock | 51 ++++----- Justfile | 2 + crates/buzz-agent/tests/regressions.rs | 4 +- crates/buzz-dev-mcp/Cargo.toml | 4 +- crates/buzz-dev-mcp/src/lib.rs | 6 +- crates/buzz-dev-mcp/src/shell.rs | 10 +- crates/buzz-dev-mcp/src/todo.rs | 6 +- crates/buzz-dev-mcp/src/view_image.rs | 8 +- crates/buzz-dev-mcp/tests/lifecycle.rs | 150 +++++++++++++++++++++++++ scripts/run-tests.sh | 4 + 10 files changed, 200 insertions(+), 45 deletions(-) create mode 100644 crates/buzz-dev-mcp/tests/lifecycle.rs diff --git a/Cargo.lock b/Cargo.lock index 0e100811c6..dd15b2c222 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1241,7 +1241,7 @@ dependencies = [ "nix 0.31.3", "nostr 0.44.7", "reqwest 0.13.4", - "rmcp 1.8.0", + "rmcp 3.4.1", "rustls", "schemars 1.2.1", "serde", @@ -5553,7 +5553,7 @@ dependencies = [ "rand 0.10.1", "regex-lite", "reqwest 0.13.4", - "rmcp 3.2.0", + "rmcp 3.4.1", "rpassword", "rustls", "same-file", @@ -5665,7 +5665,7 @@ dependencies = [ "prost 0.14.3", "prost-build 0.14.3", "protoc-bin-vendored", - "rmcp 3.2.0", + "rmcp 3.4.1", "schemars 1.2.1", "serde", "serde_json", @@ -7655,6 +7655,20 @@ dependencies = [ "windows 0.62.2", ] +[[package]] +name = "process-wrap" +version = "10.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f21b97672d2dc848e7b25701ab4535618b92f4861c13cc3f7f7bed52ad3c8da" +dependencies = [ + "futures", + "indexmap 2.14.0", + "nix 0.31.3", + "tokio", + "tracing", + "windows 0.62.2", +] + [[package]] name = "proptest" version = "1.11.0" @@ -8564,14 +8578,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d1f571c72940a19d9532fe52dbea8bc9912bf1d766c2970bb824056b86f3f59" dependencies = [ "async-trait", - "base64 0.22.1", "chrono", "futures", - "pastey", "pin-project-lite", - "process-wrap", - "rmcp-macros 1.8.0", - "schemars 1.2.1", + "process-wrap 9.1.0", "serde", "serde_json", "thiserror 2.0.18", @@ -8583,9 +8593,9 @@ dependencies = [ [[package]] name = "rmcp" -version = "3.2.0" +version = "3.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42b6914fac0be956fe704a38239c3f44a9f841d1b06a5713d2f638065593f5b5" +checksum = "5b6317cd8c13e3ec9033cf2aa5aa92cd743f0f4f8a93cddc42ea0ae6ce3b8898" dependencies = [ "async-trait", "base64 0.23.1", @@ -8598,10 +8608,10 @@ dependencies = [ "indexmap 2.14.0", "pastey", "pin-project-lite", - "process-wrap", + "process-wrap 10.0.1", "rand 0.10.1", "reqwest 0.13.4", - "rmcp-macros 3.2.0", + "rmcp-macros", "schemars 1.2.1", "serde", "serde_json", @@ -8617,22 +8627,9 @@ dependencies = [ [[package]] name = "rmcp-macros" -version = "1.8.0" +version = "3.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1aad0035b69380782d78ea95b508327e6deaa2235909053e596eea8f27b5e1d5" -dependencies = [ - "darling 0.23.0", - "proc-macro2", - "quote", - "serde_json", - "syn 2.0.117", -] - -[[package]] -name = "rmcp-macros" -version = "3.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdf1c49bd4d52014b94db0877410db273c2008f01628b0252a2e9460ad9b7fda" +checksum = "d2ee366004e5f87c103be91c6c09337811517ae8d2f919ece9da7c21444b85c5" dependencies = [ "darling 0.24.1", "proc-macro2", diff --git a/Justfile b/Justfile index 01afccffc5..dc02757a02 100644 --- a/Justfile +++ b/Justfile @@ -387,6 +387,8 @@ test-unit: # buzz-acp owns the relay-to-agent trust boundary. Run its tests here so # forged relay events cannot regain a path into agent routing unnoticed. cargo nextest run -p buzz-acp + # Exercise the real MCP server for modern and legacy clients. + cargo nextest run -p buzz-dev-mcp # buzz-db migrator/lint tests: pure SQL-parsing unit tests (no infra). # They guard the embedded-migrator invariant (the complete checked-in # additive migration set; legacy cutover/backfill remains an operator diff --git a/crates/buzz-agent/tests/regressions.rs b/crates/buzz-agent/tests/regressions.rs index d47aec6c3a..a6feb989b6 100644 --- a/crates/buzz-agent/tests/regressions.rs +++ b/crates/buzz-agent/tests/regressions.rs @@ -1444,8 +1444,8 @@ async fn cancel_kills_inflight_tool_via_mcp_notification() { ) .await; - // Wait for the tool call to be in-progress. - h.recv_until(|v| { + // Approve the tool so this test reaches execution before cancelling it. + h.recv_until_approving(|v| { v.get("params") .and_then(|p| p.get("update")) .and_then(|u| u.get("status")) diff --git a/crates/buzz-dev-mcp/Cargo.toml b/crates/buzz-dev-mcp/Cargo.toml index 08d1ffabe3..5b3d7a9c9d 100644 --- a/crates/buzz-dev-mcp/Cargo.toml +++ b/crates/buzz-dev-mcp/Cargo.toml @@ -20,7 +20,9 @@ tokio = { workspace = true } tokio-util = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } -rmcp = { workspace = true } +# Discovery-based clients (including Goose 1.52) require the modern lifecycle. +# Keep the native agent's independent MCP client dependency unchanged. +rmcp = { version = "3.4.1", features = ["server", "transport-io", "macros"] } schemars = { workspace = true } similar = "3" tempfile = "3" diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index 837ec292ab..00f48dccbd 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -2,7 +2,7 @@ #![cfg_attr(windows, deny(unsafe_code))] use rmcp::{ handler::server::{router::tool::ToolRouter, wrapper::Parameters}, - model::{CallToolResult, ServerCapabilities, ServerInfo}, + model::{CallToolResult, ServerCapabilities, ServerConfig}, tool, tool_handler, tool_router, transport::stdio, ErrorData, ServerHandler, ServiceExt, @@ -125,8 +125,8 @@ impl DevMcp { #[tool_handler(router = self.tool_router)] impl ServerHandler for DevMcp { - fn get_info(&self) -> ServerInfo { - ServerInfo::new(ServerCapabilities::builder().enable_tools().build()) + fn get_info(&self) -> ServerConfig { + ServerConfig::new(ServerCapabilities::builder().enable_tools().build()) .with_server_info(rmcp::model::Implementation::new( "buzz-dev-mcp", env!("CARGO_PKG_VERSION"), diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index 13e350689d..028e8ec8da 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -1,5 +1,5 @@ use crate::shim::Shim; -use rmcp::model::{CallToolResult, Content}; +use rmcp::model::{CallToolResult, ContentBlock}; use rmcp::ErrorData; use schemars::JsonSchema; use serde::Deserialize; @@ -161,7 +161,7 @@ pub async fn run( let bash = match &state.resolved_shell { Ok((path, _)) => path.clone(), - Err(msg) => return Ok(CallToolResult::error(vec![Content::text(msg.clone())])), + Err(msg) => return Ok(CallToolResult::error(vec![ContentBlock::text(msg.clone())])), }; let shell_arg = shell_flag(&bash); let mut cmd = Command::new(&bash); @@ -179,7 +179,7 @@ pub async fn run( let mut child = match cmd.spawn() { Ok(c) => c, Err(e) => { - return Ok(CallToolResult::error(vec![Content::text(format!( + return Ok(CallToolResult::error(vec![ContentBlock::text(format!( "failed to spawn shell: {e}" ))])); } @@ -230,7 +230,7 @@ pub async fn run( } stdout_handle.abort(); stderr_handle.abort(); - return Ok(CallToolResult::error(vec![Content::text("cancelled")])); + return Ok(CallToolResult::error(vec![ContentBlock::text("cancelled")])); } r = tokio::time::timeout(timeout_dur, child.wait()) => match r { Ok(Ok(s)) => (Some(s), false), @@ -316,7 +316,7 @@ pub async fn run( }); let text = serde_json::to_string_pretty(&body).unwrap_or_else(|_| "{}".into()); kill_group.disarm(); - Ok(CallToolResult::success(vec![Content::text(text)])) + Ok(CallToolResult::success(vec![ContentBlock::text(text)])) } /// The flag used to pass a command string to the shell. diff --git a/crates/buzz-dev-mcp/src/todo.rs b/crates/buzz-dev-mcp/src/todo.rs index de218356f2..6ab6ab2d8e 100644 --- a/crates/buzz-dev-mcp/src/todo.rs +++ b/crates/buzz-dev-mcp/src/todo.rs @@ -11,7 +11,7 @@ //! On replacement, open items that disappear without being marked done //! trigger a soft warning appended to the tool response. -use rmcp::model::{CallToolResult, Content}; +use rmcp::model::{CallToolResult, ContentBlock}; use rmcp::ErrorData; use schemars::JsonSchema; use serde::Deserialize; @@ -239,12 +239,12 @@ fn render_items(items: &[Item]) -> String { /// Wrap a string result as an MCP CallToolResult with text content. pub fn text_result(s: String) -> Result { - Ok(CallToolResult::success(vec![Content::text(s)])) + Ok(CallToolResult::success(vec![ContentBlock::text(s)])) } /// Wrap an error string as an MCP CallToolResult with isError=true. pub fn error_result(s: String) -> Result { - Ok(CallToolResult::error(vec![Content::text(s)])) + Ok(CallToolResult::error(vec![ContentBlock::text(s)])) } #[cfg(test)] diff --git a/crates/buzz-dev-mcp/src/view_image.rs b/crates/buzz-dev-mcp/src/view_image.rs index 8f8ea0ede7..f1f6978818 100644 --- a/crates/buzz-dev-mcp/src/view_image.rs +++ b/crates/buzz-dev-mcp/src/view_image.rs @@ -6,7 +6,7 @@ //! Design goals: tiny surface, no protocol-specific branching, and a //! "reasonable resolution" that fits comfortably inside both Anthropic's //! recommended ≤1568px / ≤5 MiB image budget and OpenAI's high-detail tile -//! size sweet spot. The MCP host translates `Content::image(data, mime)` +//! size sweet spot. The MCP host translates `ContentBlock::image(data, mime)` //! into the right provider-native shape on our behalf (see Goose's //! `providers::utils::convert_image` for a reference implementation). @@ -18,7 +18,7 @@ use image::{ DynamicImage, ExtendedColorType, ImageEncoder, ImageReader, Limits, }; use rmcp::{ - model::{CallToolResult, Content}, + model::{CallToolResult, ContentBlock}, ErrorData, }; use schemars::JsonSchema; @@ -101,8 +101,8 @@ pub async fn run(state: &SharedState, p: ViewImageParams) -> Result>, + work: tempfile::TempDir, + meta: Option, +} + +impl Server { + fn start(meta: Option) -> Self { + let work = tempfile::tempdir().unwrap(); + std::fs::write(work.path().join("fixture.txt"), "sunflower\n").unwrap(); + let mut child = Command::new(env!("CARGO_BIN_EXE_buzz-dev-mcp")) + .current_dir(work.path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .kill_on_drop(true) + .spawn() + .unwrap(); + Self { + input: child.stdin.take().unwrap(), + output: BufReader::new(child.stdout.take().unwrap()).lines(), + child, + work, + meta, + } + } + + async fn request(&mut self, id: u32, method: &str, mut params: Value) -> Value { + if let Some(meta) = &self.meta { + params["_meta"] = meta.clone(); + } + let frame = json!({"jsonrpc":"2.0", "id":id, "method":method, "params":params}); + self.input + .write_all(format!("{frame}\n").as_bytes()) + .await + .unwrap(); + let line = timeout(Duration::from_secs(10), self.output.next_line()) + .await + .expect("server response deadline") + .unwrap() + .expect("server closed stdout"); + let response: Value = serde_json::from_str(&line).unwrap(); + assert_eq!(response["id"], id); + response + } + + async fn check_tools_and_shutdown(mut self) { + let listed = self.request(2, "tools/list", json!({})).await; + let tools = listed["result"]["tools"].as_array().unwrap(); + for name in ["shell", "read_file", "str_replace"] { + assert!(tools.iter().any(|tool| tool["name"] == name), "{listed}"); + } + let read = self + .request( + 3, + "tools/call", + json!({ + "name":"read_file", "arguments":{"path":"fixture.txt"} + }), + ) + .await; + assert!(read["result"]["content"][0]["text"] + .as_str() + .unwrap() + .contains("sunflower")); + let edit = self.request(4, "tools/call", json!({ + "name":"str_replace", "arguments":{"path":"fixture.txt", "old_str":"sunflower", "new_str":"orchid"} + })).await; + assert!(edit.get("error").is_none(), "{edit}"); + assert_eq!( + std::fs::read_to_string(self.work.path().join("fixture.txt")).unwrap(), + "orchid\n" + ); + let bad = self + .request( + 5, + "tools/call", + json!({ + "name":"read_file", "arguments":{} + }), + ) + .await; + assert_eq!(bad["result"]["isError"], true, "{bad}"); + // A rejected tool call must leave the connection usable. + let listed = self.request(6, "tools/list", json!({})).await; + assert!(listed["result"]["tools"].is_array(), "{listed}"); + drop(self.input); + let status = timeout(Duration::from_secs(10), self.child.wait()) + .await + .unwrap() + .unwrap(); + assert!(status.success(), "{status}"); + } +} + +#[tokio::test] +async fn legacy_initialize_preserves_tools() { + for version in ["2024-11-05", "2025-11-25"] { + let mut server = Server::start(None); + let reply = server + .request( + 1, + "initialize", + json!({ + "protocolVersion":version, "capabilities":{}, + "clientInfo":{"name":"legacy-client", "version":"1"} + }), + ) + .await; + assert_eq!(reply["result"]["protocolVersion"], version, "{reply}"); + server + .input + .write_all(b"{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n") + .await + .unwrap(); + server.check_tools_and_shutdown().await; + } +} + +#[tokio::test] +async fn discover_preserves_tools() { + let mut server = Server::start(Some(json!({ + "io.modelcontextprotocol/protocolVersion":"2026-07-28", + "io.modelcontextprotocol/clientInfo":{"name":"discovery-client", "version":"1"}, + "io.modelcontextprotocol/clientCapabilities":{} + }))); + let reply = server.request(1, "server/discover", json!({})).await; + assert!( + reply["result"]["supportedVersions"] + .as_array() + .unwrap() + .contains(&json!("2026-07-28")), + "{reply}" + ); + assert_eq!( + reply["result"]["_meta"]["io.modelcontextprotocol/serverInfo"]["name"], + "buzz-dev-mcp" + ); + server.check_tools_and_shutdown().await; +} diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 524f60ed1b..96a16da9f2 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -110,6 +110,10 @@ run_unit_tests() { run_test_step "buzz-acp tests" \ cargo test -p buzz-acp -- --nocapture + # Keep MCP lifecycle coverage in step with the nextest path. + run_test_step "buzz-dev-mcp tests" \ + cargo test -p buzz-dev-mcp -- --nocapture + # buzz-db migrator/lint unit tests (no infra): guard the embedded-migrator # invariant (exactly the consolidated 0001; cutover/backfill stays an operator