From 99e412925a13df946f1b1fa53abd23d52680ffe8 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sat, 20 Dec 2025 16:32:06 +0800 Subject: [PATCH 1/3] refactor(acp): switch to sacp for simpler API Use sacp (symposium-dev/symposium-acp) which provides struct literals over builders and direct notification dispatch via JrConnectionCx. This crate is slated to become the canonical agentclientprotocol/rust-sdk implementation. Signed-off-by: Adrian Cole --- Cargo.lock | 196 +++---- crates/goose-cli/Cargo.toml | 2 +- crates/goose-cli/src/commands/acp.rs | 579 ++++++++++++--------- crates/goose/Cargo.toml | 2 +- crates/goose/tests/acp_integration_test.rs | 131 +++-- 5 files changed, 467 insertions(+), 443 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a9668c1c18e8..8df30d020723 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,15 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "addr2line" -version = "0.24.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfbe277e56a376000877090da837660b4427aad530e3028d44e0bffe4f89a1c1" -dependencies = [ - "gimli", -] - [[package]] name = "adler2" version = "2.0.0" @@ -28,35 +19,17 @@ dependencies = [ "cpufeatures", ] -[[package]] -name = "agent-client-protocol" -version = "0.9.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d3e527d7dfe0f334313d42d1d9318f0a79665f6f21c440d0798f230a77a7ed6c" -dependencies = [ - "agent-client-protocol-schema", - "anyhow", - "async-broadcast", - "async-trait", - "derive_more", - "futures", - "log", - "serde", - "serde_json", -] - [[package]] name = "agent-client-protocol-schema" -version = "0.10.5" +version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6903a00e8ac822f9bacac59a1932754d7387c72ebb7c9c7439ad021505591da4" +checksum = "16d08d095e8069115774caa50392e9c818e3fb1c482ef4f3153d26b4595482f2" dependencies = [ "anyhow", "derive_more", "schemars", "serde", "serde_json", - "strum", ] [[package]] @@ -252,18 +225,6 @@ dependencies = [ "serde_json", ] -[[package]] -name = "async-broadcast" -version = "0.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "435a87a52755b8f27fcf321ac4f04b2802e337c8c4872923137471ec39c37532" -dependencies = [ - "event-listener", - "event-listener-strategy", - "futures-core", - "pin-project-lite", -] - [[package]] name = "async-compression" version = "0.4.20" @@ -914,21 +875,6 @@ dependencies = [ "syn 2.0.111", ] -[[package]] -name = "backtrace" -version = "0.3.74" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d82cb332cdfaed17ae235a638438ac4d4839913cc2af585c3c6746e8f8bee1a" -dependencies = [ - "addr2line", - "cfg-if", - "libc", - "miniz_oxide", - "object", - "rustc-demangle", - "windows-targets 0.52.6", -] - [[package]] name = "base64" version = "0.13.1" @@ -1274,6 +1220,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3eeab4423108c5d7c744f4d234de88d18d636100093ae04caf4825134b9c3a32" +[[package]] +name = "boxfnonce" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5988cb1d626264ac94100be357308f29ff7cbdd3b36bda27f450a4ee3f713426" + [[package]] name = "brotli" version = "7.0.0" @@ -2573,16 +2525,6 @@ dependencies = [ "pin-project-lite", ] -[[package]] -name = "event-listener-strategy" -version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8be9f3dfaaffdae2972880079a491a1a8bb7cbed0b8dd7a347f668b4150a3b93" -dependencies = [ - "event-listener", - "pin-project-lite", -] - [[package]] name = "exr" version = "1.73.0" @@ -2954,6 +2896,15 @@ dependencies = [ "slab", ] +[[package]] +name = "fxhash" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c31b6d751ae2c7f11320402d34e41349dd1016f8d5d45e48c4312bc8625af50c" +dependencies = [ + "byteorder", +] + [[package]] name = "generator" version = "0.8.7" @@ -3013,12 +2964,6 @@ dependencies = [ "weezl", ] -[[package]] -name = "gimli" -version = "0.31.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" - [[package]] name = "git-version" version = "0.3.9" @@ -3075,7 +3020,6 @@ dependencies = [ name = "goose" version = "1.17.0" dependencies = [ - "agent-client-protocol", "ahash", "anyhow", "async-stream", @@ -3125,6 +3069,7 @@ dependencies = [ "regex", "reqwest 0.12.12", "rmcp 0.9.1", + "sacp", "schemars", "serde", "serde_json", @@ -3188,7 +3133,6 @@ dependencies = [ name = "goose-cli" version = "1.17.0" dependencies = [ - "agent-client-protocol", "anstream", "anyhow", "async-trait", @@ -3216,6 +3160,7 @@ dependencies = [ "regex", "rmcp 0.9.1", "rustyline", + "sacp", "serde", "serde_json", "serde_yaml", @@ -4362,6 +4307,16 @@ dependencies = [ "serde", ] +[[package]] +name = "jsonrpcmsg" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d833a15225c779251e13929203518c2ff26e2fe0f322d584b213f4f4dad37bd" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "jsonschema" version = "0.30.0" @@ -5119,15 +5074,6 @@ dependencies = [ "objc2", ] -[[package]] -name = "object" -version = "0.36.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62948e14d923ea95ea2c7c86c71013138b66525b86bdc08d2dcc262bdb497b87" -dependencies = [ - "memchr", -] - [[package]] name = "once_cell" version = "1.21.3" @@ -6442,12 +6388,6 @@ dependencies = [ "ordered-multimap", ] -[[package]] -name = "rustc-demangle" -version = "0.1.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" - [[package]] name = "rustc-hash" version = "1.1.0" @@ -6646,6 +6586,42 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd29631678d6fb0903b69223673e122c32e9ae559d0960a38d574695ebc0ea15" +[[package]] +name = "sacp" +version = "9.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7679525f5c2f4e1cb4b07e3684c4b050b34bafa0f908852879b7411522ff6626" +dependencies = [ + "agent-client-protocol-schema", + "anyhow", + "boxfnonce", + "futures", + "futures-concurrency", + "fxhash", + "jsonrpcmsg", + "rmcp 0.9.1", + "sacp-derive", + "schemars", + "serde", + "serde_json", + "thiserror 2.0.17", + "tokio", + "tokio-util", + "tracing", + "uuid", +] + +[[package]] +name = "sacp-derive" +version = "9.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70a36c27381224c2fc970935c887b26f48ff78105003d9abfa04c2e808569d3b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.111", +] + [[package]] name = "same-file" version = "1.0.6" @@ -7425,27 +7401,6 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" -[[package]] -name = "strum" -version = "0.27.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af23d6f6c1a224baef9d3f61e287d2761385a5b88fdab4eb4c6f11aeb54c4bcf" -dependencies = [ - "strum_macros", -] - -[[package]] -name = "strum_macros" -version = "0.27.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7695ce3845ea4b33927c055a39dc438a45b059f7c1b3d91d38d10355fb8cbca7" -dependencies = [ - "heck", - "proc-macro2", - "quote", - "syn 2.0.111", -] - [[package]] name = "subtle" version = "2.6.1" @@ -7921,20 +7876,19 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.43.1" +version = "1.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "492a604e2fd7f814268a378409e6c92b5525d747d10db9a229723f55a417958c" +checksum = "ff360e02eab121e0bc37a2d3b4d4dc622e6eda3a8e5253d5435ecf5bd4c68408" dependencies = [ - "backtrace", "bytes", "libc", "mio", "parking_lot", "pin-project-lite", "signal-hook-registry", - "socket2 0.5.8", + "socket2 0.6.1", "tokio-macros", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -7954,9 +7908,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.5.0" +version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e06d43f1345a3bcd39f6a56dbb7dcab2ba47e68e8ac134855e7e2bdbaf8cab8" +checksum = "af407857209536a95c8e56f8231ef2c2e2aff839b22e07a1ffcbc617e9db9fa5" dependencies = [ "proc-macro2", "quote", @@ -8662,12 +8616,14 @@ dependencies = [ [[package]] name = "uuid" -version = "1.15.1" +version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0f540e3240398cce6128b64ba83fdbdd86129c16a3aa1a3a252efd66eb3d587" +checksum = "e2e054861b4bd027cd373e18e8d8d8e6548085000e41290d95ce0c373a654b4a" dependencies = [ "getrandom 0.3.1", - "serde", + "js-sys", + "serde_core", + "wasm-bindgen", ] [[package]] diff --git a/crates/goose-cli/Cargo.toml b/crates/goose-cli/Cargo.toml index 47dfafcc3be8..1cba3409f3e3 100644 --- a/crates/goose-cli/Cargo.toml +++ b/crates/goose-cli/Cargo.toml @@ -19,7 +19,7 @@ goose = { path = "../goose" } goose-bench = { path = "../goose-bench" } goose-mcp = { path = "../goose-mcp" } rmcp = { workspace = true } -agent-client-protocol = "0.9.0" +sacp = "9.0.0" clap = { version = "4.4", features = ["derive"] } cliclack = "0.3.5" console = "0.16.1" diff --git a/crates/goose-cli/src/commands/acp.rs b/crates/goose-cli/src/commands/acp.rs index 0f3a53a6be2f..d74c6d67593e 100644 --- a/crates/goose-cli/src/commands/acp.rs +++ b/crates/goose-cli/src/commands/acp.rs @@ -1,7 +1,3 @@ -use agent_client_protocol::{ - self as acp, Client, Content, ContentChunk, EmbeddedResource, ExtResponse, ImageContent, - ProtocolVersion, SessionNotification, TextContent, ToolCallContent, -}; use anyhow::Result; use goose::agents::{Agent, SessionConfig}; use goose::config::{get_all_extensions, Config}; @@ -12,10 +8,20 @@ use goose::providers::create; use goose::session::session_manager::SessionType; use goose::session::SessionManager; use rmcp::model::{CallToolResult, RawContent, ResourceContents, Role}; +use sacp::schema::{ + AgentCapabilities, AuthenticateRequest, AuthenticateResponse, BlobResourceContents, + CancelNotification, ContentBlock, ContentChunk, EmbeddedResource, EmbeddedResourceResource, + ImageContent, InitializeRequest, InitializeResponse, LoadSessionRequest, LoadSessionResponse, + McpCapabilities, NewSessionRequest, NewSessionResponse, PromptCapabilities, PromptRequest, + PromptResponse, ResourceLink, SessionId, SessionNotification, SessionUpdate, StopReason, + TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, ToolCallLocation, + ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, +}; +use sacp::{AgentToClient, ByteStreams, Handled, JrConnectionCx, JrMessageHandler, MessageCx}; use std::collections::{HashMap, HashSet}; use std::fs; use std::sync::Arc; -use tokio::sync::{mpsc, oneshot, Mutex}; +use tokio::sync::Mutex; use tokio::task::JoinSet; use tokio_util::compat::{TokioAsyncReadCompatExt as _, TokioAsyncWriteCompatExt as _}; use tokio_util::sync::CancellationToken; @@ -30,21 +36,24 @@ struct GooseAcpSession { } struct GooseAcpAgent { - session_update_tx: mpsc::UnboundedSender<(SessionNotification, oneshot::Sender<()>)>, sessions: Arc>>, agent: Agent, // Shared agent instance } /// Create a ToolCallLocation with common defaults -fn create_tool_location(path: &str, line: Option) -> acp::ToolCallLocation { - acp::ToolCallLocation::new(path).line(line) +fn create_tool_location(path: &str, line: Option) -> ToolCallLocation { + ToolCallLocation { + path: path.into(), + line, + meta: None, + } } /// Extract file locations from tool request and response fn extract_tool_locations( tool_request: &goose::conversation::message::ToolRequest, tool_response: &goose::conversation::message::ToolResponse, -) -> Vec { +) -> Vec { let mut locations = Vec::new(); // Get the tool call details @@ -141,7 +150,7 @@ fn extract_first_line_number(text: &str) -> Option { None } -fn read_resource_link(link: acp::ResourceLink) -> Option { +fn read_resource_link(link: ResourceLink) -> Option { let url = Url::parse(&link.uri).ok()?; if url.scheme() == "file" { let path = url.to_file_path().ok()?; @@ -202,9 +211,7 @@ fn format_tool_name(tool_name: &str) -> String { } impl GooseAcpAgent { - async fn new( - session_update_tx: mpsc::UnboundedSender<(acp::SessionNotification, oneshot::Sender<()>)>, - ) -> Result { + async fn new() -> Result { let config = Config::global(); let provider_name: String = config @@ -277,30 +284,29 @@ impl GooseAcpAgent { .map_err(|_| anyhow::anyhow!("Failed to unwrap agent Arc"))?; Ok(Self { - session_update_tx, sessions: Arc::new(Mutex::new(HashMap::new())), agent, }) } - fn convert_acp_prompt_to_message(&self, prompt: Vec) -> Message { + fn convert_acp_prompt_to_message(&self, prompt: Vec) -> Message { let mut user_message = Message::user(); // Process all content blocks from the prompt for block in prompt { match block { - acp::ContentBlock::Text(text) => { + ContentBlock::Text(text) => { user_message = user_message.with_text(&text.text); } - acp::ContentBlock::Image(image) => { + ContentBlock::Image(image) => { // Goose supports images via base64 encoded data // The ACP ImageContent has data as a String directly user_message = user_message.with_image(&image.data, &image.mime_type); } - acp::ContentBlock::Resource(resource) => { + ContentBlock::Resource(resource) => { // Embed resource content as text with context match &resource.resource { - acp::EmbeddedResourceResource::TextResourceContents(text_resource) => { + EmbeddedResourceResource::TextResourceContents(text_resource) => { let header = format!("--- Resource: {} ---\n", text_resource.uri); let content = format!("{}{}\n---\n", header, text_resource.text); user_message = user_message.with_text(&content); @@ -310,13 +316,12 @@ impl GooseAcpAgent { } } } - acp::ContentBlock::ResourceLink(link) => { + ContentBlock::ResourceLink(link) => { if let Some(text) = read_resource_link(link) { user_message = user_message.with_text(text) } } - acp::ContentBlock::Audio(..) => (), - _ => (), + ContentBlock::Audio(..) => (), } } @@ -326,51 +331,48 @@ impl GooseAcpAgent { async fn handle_message_content( &self, content_item: &MessageContent, - session_id: &acp::SessionId, + session_id: &SessionId, session: &mut GooseAcpSession, - ) -> Result<(), acp::Error> { + cx: &JrConnectionCx, + ) -> Result<(), sacp::Error> { match content_item { MessageContent::Text(text) => { // Stream text to the client - let (tx, rx) = oneshot::channel(); - self.session_update_tx - .send(( - SessionNotification::new( - session_id.clone(), - acp::SessionUpdate::AgentMessageChunk(ContentChunk::new( - acp::ContentBlock::Text(TextContent::new(text.text.clone())), - )), - ), - tx, - )) - .map_err(|_| acp::Error::internal_error())?; - rx.await.map_err(|_| acp::Error::internal_error())?; + cx.send_notification(SessionNotification { + session_id: session_id.clone(), + update: SessionUpdate::AgentMessageChunk(ContentChunk { + content: ContentBlock::Text(TextContent { + text: text.text.clone(), + annotations: None, + meta: None, + }), + meta: None, + }), + meta: None, + })?; } MessageContent::ToolRequest(tool_request) => { - self.handle_tool_request(tool_request, session_id, session) + self.handle_tool_request(tool_request, session_id, session, cx) .await?; } MessageContent::ToolResponse(tool_response) => { - self.handle_tool_response(tool_response, session_id, session) + self.handle_tool_response(tool_response, session_id, session, cx) .await?; } MessageContent::Thinking(thinking) => { // Stream thinking/reasoning content as thought chunks - let (tx, rx) = oneshot::channel(); - self.session_update_tx - .send(( - SessionNotification::new( - session_id.clone(), - acp::SessionUpdate::AgentThoughtChunk(ContentChunk::new( - acp::ContentBlock::Text(TextContent::new( - thinking.thinking.clone(), - )), - )), - ), - tx, - )) - .map_err(|_| acp::Error::internal_error())?; - rx.await.map_err(|_| acp::Error::internal_error())?; + cx.send_notification(SessionNotification { + session_id: session_id.clone(), + update: SessionUpdate::AgentThoughtChunk(ContentChunk { + content: ContentBlock::Text(TextContent { + text: thinking.thinking.clone(), + annotations: None, + meta: None, + }), + meta: None, + }), + meta: None, + })?; } _ => { // Ignore other content types for now @@ -382,9 +384,10 @@ impl GooseAcpAgent { async fn handle_tool_request( &self, tool_request: &goose::conversation::message::ToolRequest, - session_id: &acp::SessionId, + session_id: &SessionId, session: &mut GooseAcpSession, - ) -> Result<(), acp::Error> { + cx: &JrConnectionCx, + ) -> Result<(), sacp::Error> { // Generate ACP tool call ID and track mapping let acp_tool_id = format!("tool_{}", uuid::Uuid::new_v4()); session @@ -404,23 +407,21 @@ impl GooseAcpAgent { // Send tool call notification with empty locations initially // We'll update with real locations when we get the response - let (tx, rx) = oneshot::channel(); - self.session_update_tx - .send(( - SessionNotification::new( - session_id.clone(), - acp::SessionUpdate::ToolCall( - acp::ToolCall::new( - acp::ToolCallId::new(acp_tool_id.clone()), - format_tool_name(&tool_name), - ) - .status(acp::ToolCallStatus::Pending), - ), - ), - tx, - )) - .map_err(|_| acp::Error::internal_error())?; - rx.await.map_err(|_| acp::Error::internal_error())?; + cx.send_notification(SessionNotification { + session_id: session_id.clone(), + update: SessionUpdate::ToolCall(ToolCall { + id: ToolCallId(acp_tool_id.clone().into()), + title: format_tool_name(&tool_name), + kind: ToolKind::default(), + status: ToolCallStatus::Pending, + content: vec![], + locations: vec![], + raw_input: None, + raw_output: None, + meta: None, + }), + meta: None, + })?; Ok(()) } @@ -428,16 +429,17 @@ impl GooseAcpAgent { async fn handle_tool_response( &self, tool_response: &goose::conversation::message::ToolResponse, - session_id: &acp::SessionId, + session_id: &SessionId, session: &mut GooseAcpSession, - ) -> Result<(), acp::Error> { + cx: &JrConnectionCx, + ) -> Result<(), sacp::Error> { // Look up the ACP tool call ID if let Some(acp_tool_id) = session.tool_call_ids.get(&tool_response.id) { // Determine if the tool call succeeded or failed let status = if tool_response.tool_result.is_ok() { - acp::ToolCallStatus::Completed + ToolCallStatus::Completed } else { - acp::ToolCallStatus::Failed + ToolCallStatus::Failed }; let content = build_tool_call_content(&tool_response.tool_result); @@ -451,27 +453,27 @@ impl GooseAcpAgent { }; // Send status update (completed or failed) with locations - let (tx, rx) = oneshot::channel(); - self.session_update_tx - .send(( - SessionNotification::new( - session_id.clone(), - acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( - acp::ToolCallId::new(acp_tool_id.clone()), - acp::ToolCallUpdateFields::new() - .status(status) - .content(content) - .locations(if locations.is_empty() { - None - } else { - Some(locations) - }), - )), - ), - tx, - )) - .map_err(|_| acp::Error::internal_error())?; - rx.await.map_err(|_| acp::Error::internal_error())?; + cx.send_notification(SessionNotification { + session_id: session_id.clone(), + update: SessionUpdate::ToolCallUpdate(ToolCallUpdate { + id: ToolCallId(acp_tool_id.clone().into()), + fields: ToolCallUpdateFields { + status: Some(status), + content: Some(content), + locations: if locations.is_empty() { + None + } else { + Some(locations) + }, + title: None, + kind: None, + raw_input: None, + raw_output: None, + }, + meta: None, + }), + meta: None, + })?; } Ok(()) @@ -485,37 +487,56 @@ fn build_tool_call_content(tool_result: &ToolResult) -> Vec Some(ToolCallContent::Content(Content::new( - acp::ContentBlock::Text(TextContent::new(val.text.clone())), - ))), - RawContent::Image(val) => Some(ToolCallContent::Content(Content::new( - acp::ContentBlock::Image(ImageContent::new( - val.data.clone(), - val.mime_type.clone(), - )), - ))), - RawContent::Resource(val) => Some(ToolCallContent::Content(Content::new( - acp::ContentBlock::Resource(EmbeddedResource::new(match &val.resource { - ResourceContents::TextResourceContents { - mime_type, - text, - uri, - .. - } => acp::EmbeddedResourceResource::TextResourceContents( - acp::TextResourceContents::new(text.clone(), uri.clone()) - .mime_type(mime_type.clone()), - ), - ResourceContents::BlobResourceContents { - mime_type, - blob, - uri, - .. - } => acp::EmbeddedResourceResource::BlobResourceContents( - acp::BlobResourceContents::new(blob.clone(), uri.clone()) - .mime_type(mime_type.clone()), - ), - })), - ))), + RawContent::Text(val) => Some(ToolCallContent::Content { + content: ContentBlock::Text(TextContent { + text: val.text.clone(), + annotations: None, + meta: None, + }), + }), + RawContent::Image(val) => Some(ToolCallContent::Content { + content: ContentBlock::Image(ImageContent { + data: val.data.clone(), + mime_type: val.mime_type.clone(), + uri: None, + annotations: None, + meta: None, + }), + }), + RawContent::Resource(val) => Some(ToolCallContent::Content { + content: ContentBlock::Resource(EmbeddedResource { + resource: match &val.resource { + ResourceContents::TextResourceContents { + mime_type, + text, + uri, + .. + } => EmbeddedResourceResource::TextResourceContents( + TextResourceContents { + text: text.clone(), + uri: uri.clone(), + mime_type: mime_type.clone(), + meta: None, + }, + ), + ResourceContents::BlobResourceContents { + mime_type, + blob, + uri, + .. + } => EmbeddedResourceResource::BlobResourceContents( + BlobResourceContents { + blob: blob.clone(), + uri: uri.clone(), + mime_type: mime_type.clone(), + meta: None, + }, + ), + }, + annotations: None, + meta: None, + }), + }), RawContent::Audio(_) => { // Audio content is not supported in ACP ContentBlock, skip it None @@ -530,40 +551,42 @@ fn build_tool_call_content(tool_result: &ToolResult) -> Vec Result { + args: InitializeRequest, + ) -> Result { info!("ACP: Received initialize request {:?}", args); // Advertise Goose's capabilities - let agent_capabilities = acp::AgentCapabilities::new() - .load_session(true) - .prompt_capabilities( - acp::PromptCapabilities::new() - .image(true) - .embedded_context(true), - ); - Ok( - acp::InitializeResponse::new(ProtocolVersion::V1) - .agent_capabilities(agent_capabilities), - ) - } - - async fn authenticate( - &self, - args: acp::AuthenticateRequest, - ) -> Result { - info!("ACP: Received authenticate request {:?}", args); - Ok(acp::AuthenticateResponse::new()) + Ok(InitializeResponse { + protocol_version: args.protocol_version, + agent_capabilities: AgentCapabilities { + load_session: true, + prompt_capabilities: PromptCapabilities { + image: true, + audio: false, + embedded_context: true, + meta: None, + }, + mcp_capabilities: McpCapabilities { + http: false, + sse: false, + meta: None, + }, + meta: None, + }, + auth_methods: vec![], + agent_info: None, + meta: None, + }) } - async fn new_session( + async fn on_new_session( &self, - args: acp::NewSessionRequest, - ) -> Result { + args: NewSessionRequest, + ) -> Result { info!("ACP: Received new session request {:?}", args); let goose_session = SessionManager::create_session( @@ -571,7 +594,11 @@ impl acp::Agent for GooseAcpAgent { "ACP Session".to_string(), // just an initial name - may be replaced by maybe_update_name SessionType::User, ) - .await?; + .await + .map_err(|e| { + error!("Failed to create session: {}", e); + sacp::Error::internal_error() + })?; let session = GooseAcpSession { messages: Conversation::new_unvalidated(Vec::new()), @@ -585,13 +612,18 @@ impl acp::Agent for GooseAcpAgent { info!("Created new ACP/goose session {}", goose_session.id); - Ok(acp::NewSessionResponse::new(goose_session.id)) + Ok(NewSessionResponse { + session_id: SessionId(goose_session.id.into()), + modes: None, + meta: None, + }) } - async fn load_session( + async fn on_load_session( &self, - args: acp::LoadSessionRequest, - ) -> Result { + args: LoadSessionRequest, + cx: &JrConnectionCx, + ) -> Result { info!("ACP: Received load session request {:?}", args); let session_id = args.session_id.0.to_string(); @@ -600,12 +632,12 @@ impl acp::Agent for GooseAcpAgent { .await .map_err(|e| { error!("Failed to load session {}: {}", session_id, e); - acp::Error::invalid_params() + sacp::Error::invalid_params() })?; let conversation = goose_session.conversation.ok_or_else(|| { error!("Session {} has no conversation data", session_id); - acp::Error::internal_error() + sacp::Error::internal_error() })?; SessionManager::update_session(&session_id) @@ -614,7 +646,7 @@ impl acp::Agent for GooseAcpAgent { .await .map_err(|e| { error!("Failed to update session working directory: {}", e); - acp::Error::internal_error() + sacp::Error::internal_error() })?; let mut session = GooseAcpSession { @@ -634,45 +666,50 @@ impl acp::Agent for GooseAcpAgent { for content_item in &message.content { match content_item { MessageContent::Text(text) => { + let chunk = ContentChunk { + content: ContentBlock::Text(TextContent { + annotations: None, + text: text.text.clone(), + meta: None, + }), + meta: None, + }; let update = match message.role { - Role::User => acp::SessionUpdate::UserMessageChunk(ContentChunk::new( - text.text.clone().into(), - )), - Role::Assistant => acp::SessionUpdate::AgentMessageChunk( - ContentChunk::new(text.text.clone().into()), - ), + Role::User => SessionUpdate::UserMessageChunk(chunk), + Role::Assistant => SessionUpdate::AgentMessageChunk(chunk), }; - let (tx, rx) = oneshot::channel(); - self.session_update_tx - .send(( - SessionNotification::new(args.session_id.clone(), update), - tx, - )) - .map_err(|_| acp::Error::internal_error())?; - rx.await.map_err(|_| acp::Error::internal_error())?; + cx.send_notification(SessionNotification { + session_id: args.session_id.clone(), + update, + meta: None, + })?; } MessageContent::ToolRequest(tool_request) => { - self.handle_tool_request(tool_request, &args.session_id, &mut session) + self.handle_tool_request(tool_request, &args.session_id, &mut session, cx) .await?; } MessageContent::ToolResponse(tool_response) => { - self.handle_tool_response(tool_response, &args.session_id, &mut session) - .await?; + self.handle_tool_response( + tool_response, + &args.session_id, + &mut session, + cx, + ) + .await?; } MessageContent::Thinking(thinking) => { - let (tx, rx) = oneshot::channel(); - self.session_update_tx - .send(( - SessionNotification::new( - args.session_id.clone(), - acp::SessionUpdate::AgentThoughtChunk(ContentChunk::new( - thinking.thinking.clone().into(), - )), - ), - tx, - )) - .map_err(|_| acp::Error::internal_error())?; - rx.await.map_err(|_| acp::Error::internal_error())?; + cx.send_notification(SessionNotification { + session_id: args.session_id.clone(), + update: SessionUpdate::AgentThoughtChunk(ContentChunk { + content: ContentBlock::Text(TextContent { + annotations: None, + text: thinking.thinking.clone(), + meta: None, + }), + meta: None, + }), + meta: None, + })?; } _ => { // Ignore other content types @@ -686,10 +723,17 @@ impl acp::Agent for GooseAcpAgent { info!("Loaded ACP session {}", session_id); - Ok(acp::LoadSessionResponse::new()) + Ok(LoadSessionResponse { + modes: None, + meta: None, + }) } - async fn prompt(&self, args: acp::PromptRequest) -> Result { + async fn on_prompt( + &self, + args: PromptRequest, + cx: &JrConnectionCx, + ) -> Result { let session_id = args.session_id.0.to_string(); let cancel_token = CancellationToken::new(); @@ -697,7 +741,7 @@ impl acp::Agent for GooseAcpAgent { let mut sessions = self.sessions.lock().await; let session = sessions .get_mut(&session_id) - .ok_or_else(acp::Error::invalid_params)?; + .ok_or_else(sacp::Error::invalid_params)?; session.cancel_token = Some(cancel_token.clone()); } @@ -716,7 +760,7 @@ impl acp::Agent for GooseAcpAgent { .await .map_err(|e| { error!("Error getting agent reply: {}", e); - acp::Error::internal_error() + sacp::Error::internal_error() })?; use futures::StreamExt; @@ -734,19 +778,19 @@ impl acp::Agent for GooseAcpAgent { let mut sessions = self.sessions.lock().await; let session = sessions .get_mut(&session_id) - .ok_or_else(acp::Error::invalid_params)?; + .ok_or_else(sacp::Error::invalid_params)?; session.messages.push(message.clone()); for content_item in &message.content { - self.handle_message_content(content_item, &args.session_id, session) + self.handle_message_content(content_item, &args.session_id, session, cx) .await?; } } Ok(_) => {} Err(e) => { error!("Error in agent response stream: {}", e); - return Err(acp::Error::internal_error()); + return Err(sacp::Error::internal_error()); } } } @@ -756,14 +800,17 @@ impl acp::Agent for GooseAcpAgent { session.cancel_token = None; } - Ok(acp::PromptResponse::new(if was_cancelled { - acp::StopReason::Cancelled - } else { - acp::StopReason::EndTurn - })) + Ok(PromptResponse { + stop_reason: if was_cancelled { + StopReason::Cancelled + } else { + StopReason::EndTurn + }, + meta: None, + }) } - async fn cancel(&self, args: acp::CancelNotification) -> Result<(), acp::Error> { + async fn on_cancel(&self, args: CancelNotification) -> Result<(), sacp::Error> { info!("ACP: Received cancel request {:?}", args); let session_id = args.session_id.0.to_string(); @@ -780,24 +827,66 @@ impl acp::Agent for GooseAcpAgent { Ok(()) } +} - async fn set_session_mode( - &self, - _args: acp::SetSessionModeRequest, - ) -> Result { - Err(acp::Error::method_not_found()) - } +struct GooseAcpHandler { + agent: Arc, +} + +impl JrMessageHandler for GooseAcpHandler { + type Role = AgentToClient; - async fn ext_method(&self, _args: acp::ExtRequest) -> Result { - Err(acp::Error::method_not_found()) + fn describe_chain(&self) -> impl std::fmt::Debug { + "goose-acp" } - async fn ext_notification(&self, _args: acp::ExtNotification) -> Result<(), acp::Error> { - Ok(()) + async fn handle_message( + &mut self, + message: MessageCx, + cx: JrConnectionCx, + ) -> Result, sacp::Error> { + use sacp::util::MatchMessageFrom; + use sacp::JrRequestCx; + + MatchMessageFrom::new(message, &cx) + .if_request( + |req: InitializeRequest, req_cx: JrRequestCx| async { + req_cx.respond(self.agent.on_initialize(req).await?) + }, + ) + .await + .if_request( + |_req: AuthenticateRequest, req_cx: JrRequestCx| async { + req_cx.respond(AuthenticateResponse { meta: None }) + }, + ) + .await + .if_request( + |req: NewSessionRequest, req_cx: JrRequestCx| async { + req_cx.respond(self.agent.on_new_session(req).await?) + }, + ) + .await + .if_request( + |req: LoadSessionRequest, req_cx: JrRequestCx| async { + req_cx.respond(self.agent.on_load_session(req, &cx).await?) + }, + ) + .await + .if_request( + |req: PromptRequest, req_cx: JrRequestCx| async { + req_cx.respond(self.agent.on_prompt(req, &cx).await?) + }, + ) + .await + .if_notification(|notif: CancelNotification| async { + self.agent.on_cancel(notif).await + }) + .await + .done() } } -/// Run the ACP agent server pub async fn run_acp_agent() -> Result<()> { info!("Starting Goose ACP agent server on stdio"); eprintln!("Goose ACP agent started. Listening on stdio..."); @@ -805,38 +894,13 @@ pub async fn run_acp_agent() -> Result<()> { let outgoing = tokio::io::stdout().compat_write(); let incoming = tokio::io::stdin().compat(); - // The AgentSideConnection will spawn futures onto our Tokio runtime. - // LocalSet and spawn_local are used because the futures from the - // agent-client-protocol crate are not Send. - let local_set = tokio::task::LocalSet::new(); - local_set - .run_until(async move { - let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); - - // Start up the GooseAcpAgent connected to stdio. - let agent = GooseAcpAgent::new(tx) - .await - .map_err(|e| anyhow::anyhow!("Failed to create ACP agent: {}", e))?; - let (conn, handle_io) = - acp::AgentSideConnection::new(agent, outgoing, incoming, |fut| { - tokio::task::spawn_local(fut); - }); - - // Kick off a background task to send the agent's session notifications to the client. - tokio::task::spawn_local(async move { - while let Some((session_notification, tx)) = rx.recv().await { - let result = conn.session_notification(session_notification).await; - if let Err(e) = result { - error!("ACP session notification error: {}", e); - break; - } - tx.send(()).ok(); - } - }); + let agent = Arc::new(GooseAcpAgent::new().await?); + let handler = GooseAcpHandler { agent }; - // Run until stdin/stdout are closed. - handle_io.await - }) + AgentToClient::builder() + .name("goose-acp") + .with_handler(handler) + .serve(ByteStreams::new(outgoing, incoming)) .await?; Ok(()) @@ -844,7 +908,7 @@ pub async fn run_acp_agent() -> Result<()> { #[cfg(test)] mod tests { - use agent_client_protocol::ResourceLink; + use sacp::schema::ResourceLink; use std::io::Write; use tempfile::NamedTempFile; @@ -854,14 +918,21 @@ mod tests { let mut file = NamedTempFile::new()?; file.write_all(content.as_bytes())?; - let link = ResourceLink::new( - file.path() + let link = ResourceLink { + name: file + .path() .file_name() .unwrap() .to_string_lossy() .to_string(), - format!("file://{}", file.path().to_str().unwrap()), - ); + uri: format!("file://{}", file.path().to_str().unwrap()), + annotations: None, + description: None, + mime_type: None, + size: None, + title: None, + meta: None, + }; Ok((link, file)) } diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index 74b892e5d945..5eebeba53249 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -121,7 +121,7 @@ unbinder = "0.1.7" winapi = { version = "0.3", features = ["wincred"] } [dev-dependencies] -agent-client-protocol = "0.9.2" +sacp = "9.0.0" criterion = "0.5" serial_test = "3.2.0" mockall = "0.13.1" diff --git a/crates/goose/tests/acp_integration_test.rs b/crates/goose/tests/acp_integration_test.rs index 6af737becbcb..f50f5f9a00b3 100644 --- a/crates/goose/tests/acp_integration_test.rs +++ b/crates/goose/tests/acp_integration_test.rs @@ -1,8 +1,8 @@ -use agent_client_protocol::{ - self as acp, Agent, Client, ClientSideConnection, ContentBlock, InitializeRequest, - NewSessionRequest, PromptRequest, ProtocolVersion, SessionNotification, SessionUpdate, - TextContent, +use sacp::schema::{ + ContentBlock, ContentChunk, InitializeRequest, NewSessionRequest, PromptRequest, + SessionNotification, SessionUpdate, StopReason, TextContent, VERSION as PROTOCOL_VERSION, }; +use sacp::{ClientToAgent, JrConnectionCx}; use std::path::Path; use std::process::Stdio; use std::sync::{Arc, Mutex}; @@ -20,27 +20,31 @@ async fn test_acp_basic_completion() { let mock_server = setup_mock_openai(BASIC_RESPONSE).await; let work_dir = tempfile::tempdir().unwrap(); - let (client, updates) = TestClient::new(); + let updates = Arc::new(Mutex::new(Vec::::new())); let child = spawn_goose_acp(&mock_server).await; - run_acp_session( - client, - child, - work_dir.path(), - |conn, session_id| async move { - let response = conn - .prompt(PromptRequest::new( + run_acp_session(child, work_dir.path(), updates.clone(), |cx, session_id| { + let updates = updates.clone(); + async move { + let response = cx + .send_request(PromptRequest { session_id, - vec![ContentBlock::Text(TextContent::new("test message"))], - )) + prompt: vec![ContentBlock::Text(TextContent { + text: "test message".to_string(), + annotations: None, + meta: None, + })], + meta: None, + }) + .block_task() .await .unwrap(); - assert_eq!(response.stop_reason, acp::StopReason::EndTurn); + assert_eq!(response.stop_reason, StopReason::EndTurn); wait_for_text(&updates, BASIC_TEXT, Duration::from_secs(5)).await; - }, - ) + } + }) .await; } @@ -83,7 +87,7 @@ fn extract_text(updates: &[SessionNotification]) -> String { updates .iter() .filter_map(|n| match &n.update { - SessionUpdate::AgentMessageChunk(chunk) => match &chunk.content { + SessionUpdate::AgentMessageChunk(ContentChunk { content, .. }) => match content { ContentBlock::Text(t) => Some(t.text.clone()), _ => None, }, @@ -92,37 +96,6 @@ fn extract_text(updates: &[SessionNotification]) -> String { .collect() } -struct TestClient { - updates: Arc>>, -} - -impl TestClient { - fn new() -> (Self, Arc>>) { - let updates = Arc::new(Mutex::new(Vec::new())); - ( - Self { - updates: updates.clone(), - }, - updates, - ) - } -} - -#[async_trait::async_trait(?Send)] -impl Client for TestClient { - async fn request_permission( - &self, - _args: acp::RequestPermissionRequest, - ) -> acp::Result { - Err(acp::Error::method_not_found()) - } - - async fn session_notification(&self, args: SessionNotification) -> acp::Result<()> { - self.updates.lock().unwrap().push(args); - Ok(()) - } -} - async fn spawn_goose_acp(mock_server: &MockServer) -> Child { Command::new("cargo") .args(["run", "-p", "goose-cli", "--", "acp"]) @@ -138,33 +111,57 @@ async fn spawn_goose_acp(mock_server: &MockServer) -> Child { .unwrap() } -async fn run_acp_session(client: TestClient, mut child: Child, work_dir: &Path, test_fn: F) -where - F: FnOnce(ClientSideConnection, acp::SessionId) -> Fut, +async fn run_acp_session( + mut child: Child, + work_dir: &Path, + updates: Arc>>, + test_fn: F, +) where + F: FnOnce(JrConnectionCx, sacp::schema::SessionId) -> Fut, Fut: std::future::Future, { let outgoing = child.stdin.take().unwrap().compat_write(); let incoming = child.stdout.take().unwrap().compat(); let work_dir = work_dir.to_path_buf(); - let local_set = tokio::task::LocalSet::new(); - local_set - .run_until(async move { - let (conn, handle_io) = ClientSideConnection::new(client, outgoing, incoming, |fut| { - tokio::task::spawn_local(fut); - }); - tokio::task::spawn_local(handle_io); - - conn.initialize(InitializeRequest::new(ProtocolVersion::V1)) + let transport = sacp::ByteStreams::new(outgoing, incoming); + + ClientToAgent::builder() + .on_receive_notification( + { + let updates = updates.clone(); + async move |notification: SessionNotification, _cx| { + updates.lock().unwrap().push(notification); + Ok(()) + } + }, + sacp::on_receive_notification!(), + ) + .with_client(transport, |cx: JrConnectionCx| async move { + cx.send_request(InitializeRequest { + protocol_version: PROTOCOL_VERSION, + client_capabilities: Default::default(), + client_info: Default::default(), + meta: None, + }) + .block_task() + .await + .unwrap(); + + let session = cx + .send_request(NewSessionRequest { + mcp_servers: vec![], + cwd: work_dir, + meta: None, + }) + .block_task() .await .unwrap(); - let session = conn - .new_session(NewSessionRequest::new(&work_dir)) - .await - .unwrap(); + test_fn(cx.clone(), session.session_id).await; - test_fn(conn, session.session_id).await; + Ok(()) }) - .await; + .await + .unwrap(); } From 426422193370b250233c7ba8ba9ca12d1cece011 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 21 Dec 2025 18:09:57 +0800 Subject: [PATCH 2/3] Add ACP support for MCP servers (Stdio, Http transports) - Convert McpServer from ACP schema to ExtensionConfig - Stdio: command, args, env vars - Http: url, headers (StreamableHttp transport) - SSE: explicitly not supported (panics) - Inline Calculator MCP server in integration tests - Implement permission flow using await_when_result_received pattern - Use provider tool call ID directly (matching other ACP agents) - Add serde snake_case rename to Permission enum for drift-free conversion Note: rmcp version (0.9.1) matches sacp from https://github.com/symposium-dev/symposium-acp Fixes #6111 Signed-off-by: Adrian Cole --- Cargo.lock | 127 ++-- crates/goose-cli/src/commands/acp.rs | 549 ++++++++++++++---- crates/goose-mcp/Cargo.toml | 2 +- crates/goose/Cargo.toml | 1 + crates/goose/src/agents/extension.rs | 4 +- .../src/permission/permission_confirmation.rs | 2 +- crates/goose/tests/acp_integration_test.rs | 281 +++++++-- crates/goose/tests/common.rs | 43 ++ crates/goose/tests/mcp_integration_test.rs | 57 +- .../tests/test_data/openai_basic_response.txt | 9 + .../openai_chat_completion_streaming.txt | 27 - .../test_data/openai_session_description.json | 1 + .../test_data/openai_tool_call_response.txt | 9 + .../test_data/openai_tool_result_response.txt | 9 + 14 files changed, 830 insertions(+), 291 deletions(-) create mode 100644 crates/goose/tests/common.rs create mode 100644 crates/goose/tests/test_data/openai_basic_response.txt delete mode 100644 crates/goose/tests/test_data/openai_chat_completion_streaming.txt create mode 100644 crates/goose/tests/test_data/openai_session_description.json create mode 100644 crates/goose/tests/test_data/openai_tool_call_response.txt create mode 100644 crates/goose/tests/test_data/openai_tool_result_response.txt diff --git a/Cargo.lock b/Cargo.lock index 8df30d020723..4ee20da797d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2916,7 +2916,7 @@ dependencies = [ "libc", "log", "rustversion", - "windows 0.59.0", + "windows 0.61.3", ] [[package]] @@ -3214,7 +3214,7 @@ dependencies = [ "rayon", "regex", "reqwest 0.11.27", - "rmcp 0.8.5", + "rmcp 0.10.0", "schemars", "serde", "serde_json", @@ -5334,6 +5334,12 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" +[[package]] +name = "pastey" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b867cad97c0791bbd3aaa6472142568c6c9e8f71937e98379f584cfb0cf35bec" + [[package]] name = "path_abs" version = "0.5.1" @@ -5720,16 +5726,16 @@ dependencies = [ [[package]] name = "process-wrap" -version = "8.2.0" +version = "8.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d35f4dc9988d1326b065b4def5e950c3ed727aa03e3151b86cc9e2aec6b03f54" +checksum = "a3ef4f2f0422f23a82ec9f628ea2acd12871c81a9362b02c43c1aa86acfc3ba1" dependencies = [ "futures", "indexmap 2.12.0", - "nix 0.29.0", + "nix 0.30.1", "tokio", "tracing", - "windows 0.59.0", + "windows 0.61.3", ] [[package]] @@ -6269,60 +6275,67 @@ dependencies = [ [[package]] name = "rmcp" -version = "0.8.5" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5947688160b56fb6c827e3c20a72c90392a1d7e9dec74749197aa1780ac42ca" +checksum = "eaa07b85b779d1e1df52dd79f6c6bffbe005b191f07290136cc42a142da3409a" dependencies = [ + "async-trait", "base64 0.22.1", + "bytes", "chrono", "futures", + "http 1.2.0", + "http-body 1.0.1", + "http-body-util", + "oauth2", "paste", "pin-project-lite", - "rmcp-macros 0.8.5", + "process-wrap", + "rand 0.9.2", + "reqwest 0.12.12", + "rmcp-macros 0.9.1", "schemars", "serde", "serde_json", + "sse-stream", "thiserror 2.0.17", "tokio", "tokio-stream", "tokio-util", + "tower-service", "tracing", + "url", + "uuid", ] [[package]] name = "rmcp" -version = "0.9.1" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eaa07b85b779d1e1df52dd79f6c6bffbe005b191f07290136cc42a142da3409a" +checksum = "38b18323edc657390a6ed4d7a9110b0dec2dc3ed128eb2a123edfbafabdbddc5" dependencies = [ "async-trait", "base64 0.22.1", "chrono", "futures", - "http 1.2.0", - "oauth2", - "paste", + "pastey", "pin-project-lite", - "process-wrap", - "reqwest 0.12.12", - "rmcp-macros 0.9.1", + "rmcp-macros 0.10.0", "schemars", "serde", "serde_json", - "sse-stream", "thiserror 2.0.17", "tokio", "tokio-stream", "tokio-util", "tracing", - "url", ] [[package]] name = "rmcp-macros" -version = "0.8.5" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01263441d3f8635c628e33856c468b96ebbce1af2d3699ea712ca71432d4ee7a" +checksum = "0f6fa09933cac0d0204c8a5d647f558425538ed6a0134b1ebb1ae4dc00c96db3" dependencies = [ "darling 0.21.0", "proc-macro2", @@ -6333,9 +6346,9 @@ dependencies = [ [[package]] name = "rmcp-macros" -version = "0.9.1" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f6fa09933cac0d0204c8a5d647f558425538ed6a0134b1ebb1ae4dc00c96db3" +checksum = "c75d0a62676bf8c8003c4e3c348e2ceb6a7b3e48323681aaf177fdccdac2ce50" dependencies = [ "darling 0.21.0", "proc-macro2", @@ -8965,12 +8978,24 @@ dependencies = [ [[package]] name = "windows" -version = "0.59.0" +version = "0.61.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f919aee0a93304be7f62e8e5027811bbba96bcb1de84d6618be56e43f8a32a1" +checksum = "9babd3a767a4c1aef6900409f85f5d53ce2544ccdfaa86dad48c91782c6d6893" dependencies = [ - "windows-core 0.59.0", - "windows-targets 0.53.3", + "windows-collections", + "windows-core 0.61.2", + "windows-future", + "windows-link 0.1.3", + "windows-numerics", +] + +[[package]] +name = "windows-collections" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8" +dependencies = [ + "windows-core 0.61.2", ] [[package]] @@ -9021,15 +9046,26 @@ dependencies = [ [[package]] name = "windows-core" -version = "0.59.0" +version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "810ce18ed2112484b0d4e15d022e5f598113e220c53e373fb31e67e21670c1ce" +checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" dependencies = [ - "windows-implement 0.59.0", + "windows-implement 0.60.2", "windows-interface 0.59.1", + "windows-link 0.1.3", "windows-result 0.3.4", - "windows-strings 0.3.1", - "windows-targets 0.53.3", + "windows-strings 0.4.2", +] + +[[package]] +name = "windows-future" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e" +dependencies = [ + "windows-core 0.61.2", + "windows-link 0.1.3", + "windows-threading", ] [[package]] @@ -9067,9 +9103,9 @@ dependencies = [ [[package]] name = "windows-implement" -version = "0.59.0" +version = "0.60.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83577b051e2f49a058c308f17f273b570a6a758386fc291b5f6a934dd84e48c1" +checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", @@ -9132,6 +9168,16 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-numerics" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" +dependencies = [ + "windows-core 0.61.2", + "windows-link 0.1.3", +] + [[package]] name = "windows-registry" version = "0.2.0" @@ -9182,9 +9228,9 @@ dependencies = [ [[package]] name = "windows-strings" -version = "0.3.1" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87fa48cc5d406560701792be122a10132491cff9d0aeb23583cc2dcafc847319" +checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" dependencies = [ "windows-link 0.1.3", ] @@ -9306,6 +9352,15 @@ dependencies = [ "windows_x86_64_msvc 0.53.0", ] +[[package]] +name = "windows-threading" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66463ad2e0ea3bbf808b7f1d371311c80e115c0b71d60efc142cafbcfb057a6" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.42.2" diff --git a/crates/goose-cli/src/commands/acp.rs b/crates/goose-cli/src/commands/acp.rs index d74c6d67593e..9d736ab23d5d 100644 --- a/crates/goose-cli/src/commands/acp.rs +++ b/crates/goose-cli/src/commands/acp.rs @@ -1,9 +1,12 @@ use anyhow::Result; -use goose::agents::{Agent, SessionConfig}; +use goose::agents::extension::Envs; +use goose::agents::{Agent, ExtensionConfig, SessionConfig}; use goose::config::{get_all_extensions, Config}; -use goose::conversation::message::{Message, MessageContent}; +use goose::conversation::message::{ActionRequiredData, Message, MessageContent}; use goose::conversation::Conversation; use goose::mcp_utils::ToolResult; +use goose::permission::permission_confirmation::PrincipalType; +use goose::permission::{Permission, PermissionConfirmation}; use goose::providers::create; use goose::session::session_manager::SessionType; use goose::session::SessionManager; @@ -12,10 +15,12 @@ use sacp::schema::{ AgentCapabilities, AuthenticateRequest, AuthenticateResponse, BlobResourceContents, CancelNotification, ContentBlock, ContentChunk, EmbeddedResource, EmbeddedResourceResource, ImageContent, InitializeRequest, InitializeResponse, LoadSessionRequest, LoadSessionResponse, - McpCapabilities, NewSessionRequest, NewSessionResponse, PromptCapabilities, PromptRequest, - PromptResponse, ResourceLink, SessionId, SessionNotification, SessionUpdate, StopReason, - TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, ToolCallLocation, - ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, + McpCapabilities, McpServer, NewSessionRequest, NewSessionResponse, PermissionOption, + PermissionOptionId, PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse, + RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, SessionId, + SessionNotification, SessionUpdate, StopReason, TextContent, TextResourceContents, ToolCall, + ToolCallContent, ToolCallId, ToolCallLocation, ToolCallStatus, ToolCallUpdate, + ToolCallUpdateFields, ToolKind, }; use sacp::{AgentToClient, ByteStreams, Handled, JrConnectionCx, JrMessageHandler, MessageCx}; use std::collections::{HashMap, HashSet}; @@ -25,22 +30,59 @@ use tokio::sync::Mutex; use tokio::task::JoinSet; use tokio_util::compat::{TokioAsyncReadCompatExt as _, TokioAsyncWriteCompatExt as _}; use tokio_util::sync::CancellationToken; -use tracing::{error, info, warn}; +use tracing::{debug, error, info, warn}; use url::Url; struct GooseAcpSession { messages: Conversation, - tool_call_ids: HashMap, // Maps internal tool IDs to ACP tool call IDs - tool_requests: HashMap, // Store tool requests by ID for location extraction - cancel_token: Option, // Active cancellation token for prompt processing + tool_requests: HashMap, + cancel_token: Option, } struct GooseAcpAgent { sessions: Arc>>, - agent: Agent, // Shared agent instance + agent: Arc, +} + +fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result { + match mcp_server { + McpServer::Stdio { + name, + command, + args, + env, + .. + } => Ok(ExtensionConfig::Stdio { + name, + description: String::new(), + cmd: command.to_string_lossy().to_string(), + args, + envs: Envs::new(env.into_iter().map(|e| (e.name, e.value)).collect()), + env_keys: vec![], + timeout: None, + bundled: Some(false), + available_tools: vec![], + }), + McpServer::Http { + name, url, headers, .. + } => Ok(ExtensionConfig::StreamableHttp { + name, + description: String::new(), + uri: url, + envs: Envs::default(), + env_keys: vec![], + headers: headers.into_iter().map(|h| (h.name, h.value)).collect(), + timeout: None, + bundled: Some(false), + available_tools: vec![], + }), + McpServer::Sse { name, .. } => Err(format!( + "SSE transport is deprecated and not supported: {}", + name + )), + } } -/// Create a ToolCallLocation with common defaults fn create_tool_location(path: &str, line: Option) -> ToolCallLocation { ToolCallLocation { path: path.into(), @@ -49,7 +91,6 @@ fn create_tool_location(path: &str, line: Option) -> ToolCallLocation { } } -/// Extract file locations from tool request and response fn extract_tool_locations( tool_request: &goose::conversation::message::ToolRequest, tool_response: &goose::conversation::message::ToolResponse, @@ -124,9 +165,8 @@ fn extract_tool_locations( locations } -/// Extract line range from view command output (e.g., "### path/to/file.rs (lines 10-20)") fn extract_view_line_range(text: &str) -> Option<(usize, usize)> { - // Look for pattern like "(lines X-Y)" or "(lines X-end)" + // Pattern: "(lines X-Y)" or "(lines X-end)" let re = regex::Regex::new(r"\(lines (\d+)-(\d+|end)\)").ok()?; if let Some(caps) = re.captures(text) { let start = caps.get(1)?.as_str().parse::().ok()?; @@ -140,9 +180,8 @@ fn extract_view_line_range(text: &str) -> Option<(usize, usize)> { None } -/// Extract the first line number from code snippet (e.g., "123: some code") fn extract_first_line_number(text: &str) -> Option { - // Look for pattern like "123: " at the start of a line within a code block + // Pattern: "123: " at the start of a line within a code block let re = regex::Regex::new(r"```[^\n]*\n(\d+):").ok()?; if let Some(caps) = re.captures(text) { return caps.get(1)?.as_str().parse::().ok(); @@ -166,10 +205,7 @@ fn read_resource_link(link: ResourceLink) -> Option { } } -/// Format a tool name to be more human-friendly by splitting extension and tool names -/// and converting underscores to spaces with proper capitalization fn format_tool_name(tool_name: &str) -> String { - // Split on double underscore to separate extension from tool name if let Some((extension, tool)) = tool_name.split_once("__") { let formatted_extension = extension.replace('_', " "); let formatted_tool = tool.replace('_', " "); @@ -268,24 +304,21 @@ impl GooseAcpAgent { match result { Ok((name, Ok(_))) => { waiting_on.remove(&name); - info!("Loaded extension: {}", name); + info!(extension = %name, "extension loaded"); } Ok((name, Err(e))) => { - warn!("Failed to load extension '{}': {}", name, e); + warn!(extension = %name, error = %e, "extension load failed"); waiting_on.remove(&name); } Err(e) => { - error!("Task error while loading extension: {}", e); + error!(error = %e, "extension task error"); } } } - let agent = Arc::try_unwrap(agent_ptr) - .map_err(|_| anyhow::anyhow!("Failed to unwrap agent Arc"))?; - Ok(Self { sessions: Arc::new(Mutex::new(HashMap::new())), - agent, + agent: agent_ptr, }) } @@ -374,6 +407,24 @@ impl GooseAcpAgent { meta: None, })?; } + MessageContent::ActionRequired(action_required) => { + if let ActionRequiredData::ToolConfirmation { + id, + tool_name, + arguments, + prompt, + } = &action_required.data + { + self.handle_tool_permission_request( + id.clone(), + tool_name.clone(), + arguments.clone(), + prompt.clone(), + session_id, + cx, + )?; + } + } _ => { // Ignore other content types for now } @@ -388,12 +439,6 @@ impl GooseAcpAgent { session: &mut GooseAcpSession, cx: &JrConnectionCx, ) -> Result<(), sacp::Error> { - // Generate ACP tool call ID and track mapping - let acp_tool_id = format!("tool_{}", uuid::Uuid::new_v4()); - session - .tool_call_ids - .insert(tool_request.id.clone(), acp_tool_id.clone()); - // Store the tool request for later use in response handling session .tool_requests @@ -405,12 +450,11 @@ impl GooseAcpAgent { Err(_) => "error".to_string(), }; - // Send tool call notification with empty locations initially - // We'll update with real locations when we get the response + // Send tool call notification using the provider's tool call ID directly cx.send_notification(SessionNotification { session_id: session_id.clone(), update: SessionUpdate::ToolCall(ToolCall { - id: ToolCallId(acp_tool_id.clone().into()), + id: ToolCallId(tool_request.id.clone().into()), title: format_tool_name(&tool_name), kind: ToolKind::default(), status: ToolCallStatus::Pending, @@ -433,54 +477,166 @@ impl GooseAcpAgent { session: &mut GooseAcpSession, cx: &JrConnectionCx, ) -> Result<(), sacp::Error> { - // Look up the ACP tool call ID - if let Some(acp_tool_id) = session.tool_call_ids.get(&tool_response.id) { - // Determine if the tool call succeeded or failed - let status = if tool_response.tool_result.is_ok() { - ToolCallStatus::Completed - } else { - ToolCallStatus::Failed - }; + // Determine if the tool call succeeded or failed + let status = if tool_response.tool_result.is_ok() { + ToolCallStatus::Completed + } else { + ToolCallStatus::Failed + }; - let content = build_tool_call_content(&tool_response.tool_result); + let content = build_tool_call_content(&tool_response.tool_result); - // Extract locations from the tool request and response - let locations = if let Some(tool_request) = session.tool_requests.get(&tool_response.id) - { - extract_tool_locations(tool_request, tool_response) - } else { - Vec::new() - }; + // Extract locations from the tool request and response + let locations = if let Some(tool_request) = session.tool_requests.get(&tool_response.id) { + extract_tool_locations(tool_request, tool_response) + } else { + Vec::new() + }; - // Send status update (completed or failed) with locations - cx.send_notification(SessionNotification { - session_id: session_id.clone(), - update: SessionUpdate::ToolCallUpdate(ToolCallUpdate { - id: ToolCallId(acp_tool_id.clone().into()), - fields: ToolCallUpdateFields { - status: Some(status), - content: Some(content), - locations: if locations.is_empty() { - None - } else { - Some(locations) - }, - title: None, - kind: None, - raw_input: None, - raw_output: None, + // Send status update using provider's tool call ID directly + cx.send_notification(SessionNotification { + session_id: session_id.clone(), + update: SessionUpdate::ToolCallUpdate(ToolCallUpdate { + id: ToolCallId(tool_response.id.clone().into()), + fields: ToolCallUpdateFields { + status: Some(status), + content: Some(content), + locations: if locations.is_empty() { + None + } else { + Some(locations) }, - meta: None, + title: None, + kind: None, + raw_input: None, + raw_output: None, + }, + meta: None, + }), + meta: None, + })?; + + Ok(()) + } + + fn handle_tool_permission_request( + &self, + request_id: String, + tool_name: String, + arguments: serde_json::Map, + prompt: Option, + session_id: &SessionId, + cx: &JrConnectionCx, + ) -> Result<(), sacp::Error> { + let cx = cx.clone(); + let agent = self.agent.clone(); + let session_id = session_id.clone(); + + let formatted_name = format_tool_name(&tool_name); + + // Use the request_id (provider's tool call ID) directly + let tool_call_update = ToolCallUpdate { + id: ToolCallId(request_id.clone().into()), + fields: ToolCallUpdateFields { + title: Some(formatted_name), + kind: Some(ToolKind::default()), + status: Some(ToolCallStatus::Pending), + content: prompt.map(|p| { + vec![ToolCallContent::Content { + content: ContentBlock::Text(TextContent { + text: p, + annotations: None, + meta: None, + }), + }] }), + locations: None, + raw_input: Some(serde_json::Value::Object(arguments)), + raw_output: None, + }, + meta: None, + }; + + fn option(kind: PermissionOptionKind) -> PermissionOption { + let id = serde_json::to_value(kind) + .unwrap() + .as_str() + .unwrap() + .to_string(); + PermissionOption { + id: PermissionOptionId::from(id.clone()), + name: id, + kind, meta: None, - })?; + } } + let options = vec![ + option(PermissionOptionKind::AllowAlways), + option(PermissionOptionKind::AllowOnce), + option(PermissionOptionKind::RejectOnce), + ]; + + let permission_request = RequestPermissionRequest { + session_id, + tool_call: tool_call_update, + options, + meta: None, + }; + + cx.send_request(permission_request) + .await_when_result_received(move |result| async move { + match result { + Ok(response) => { + agent + .handle_confirmation( + request_id, + outcome_to_confirmation(&response.outcome), + ) + .await; + Ok(()) + } + Err(e) => { + error!(error = ?e, "permission request failed"); + agent + .handle_confirmation( + request_id, + PermissionConfirmation { + principal_type: PrincipalType::Tool, + permission: Permission::Cancel, + }, + ) + .await; + Ok(()) + } + } + })?; Ok(()) } } -/// Build tool call content from tool result +fn outcome_to_confirmation(outcome: &RequestPermissionOutcome) -> PermissionConfirmation { + let permission = match outcome { + RequestPermissionOutcome::Cancelled => Permission::Cancel, + RequestPermissionOutcome::Selected { option_id } => { + match serde_json::from_value::(serde_json::Value::String( + option_id.0.to_string(), + )) { + Ok(PermissionOptionKind::AllowAlways) => Permission::AlwaysAllow, + Ok(PermissionOptionKind::AllowOnce) => Permission::AllowOnce, + Ok(PermissionOptionKind::RejectOnce | PermissionOptionKind::RejectAlways) => { + Permission::DenyOnce + } + Err(_) => Permission::Cancel, + } + } + }; + PermissionConfirmation { + principal_type: PrincipalType::Tool, + permission, + } +} + fn build_tool_call_content(tool_result: &ToolResult) -> Vec { match tool_result { Ok(result) => result @@ -551,13 +707,12 @@ fn build_tool_call_content(tool_result: &ToolResult) -> Vec Result { - info!("ACP: Received initialize request {:?}", args); + debug!(?args, "initialize request"); // Advertise Goose's capabilities Ok(InitializeResponse { @@ -571,8 +726,8 @@ impl GooseAcpAgent { meta: None, }, mcp_capabilities: McpCapabilities { - http: false, - sse: false, + http: true, + sse: false, // SSE is deprecated; rmcp drops support after 0.10.0 meta: None, }, meta: None, @@ -587,7 +742,7 @@ impl GooseAcpAgent { &self, args: NewSessionRequest, ) -> Result { - info!("ACP: Received new session request {:?}", args); + debug!(?args, "new session request"); let goose_session = SessionManager::create_session( std::env::current_dir().unwrap_or_default(), @@ -595,14 +750,14 @@ impl GooseAcpAgent { SessionType::User, ) .await - .map_err(|e| { - error!("Failed to create session: {}", e); - sacp::Error::internal_error() + .map_err(|e| sacp::Error { + code: sacp::ErrorCode::INTERNAL_ERROR.code, + message: format!("Failed to create session: {}", e), + data: None, })?; let session = GooseAcpSession { messages: Conversation::new_unvalidated(Vec::new()), - tool_call_ids: HashMap::new(), tool_requests: HashMap::new(), cancel_token: None, }; @@ -610,7 +765,33 @@ impl GooseAcpAgent { let mut sessions = self.sessions.lock().await; sessions.insert(goose_session.id.clone(), session); - info!("Created new ACP/goose session {}", goose_session.id); + // Add MCP servers specified in the session request + for mcp_server in args.mcp_servers { + let config = match mcp_server_to_extension_config(mcp_server) { + Ok(c) => c, + Err(msg) => { + return Err(sacp::Error { + code: sacp::ErrorCode::INVALID_PARAMS.code, + message: msg, + data: None, + }); + } + }; + let name = config.name().to_string(); + if let Err(e) = self.agent.add_extension(config).await { + return Err(sacp::Error { + code: sacp::ErrorCode::INTERNAL_ERROR.code, + message: format!("Failed to add MCP server '{}': {}", name, e), + data: None, + }); + } + } + + info!( + session_id = %goose_session.id, + session_type = "acp", + "Session started" + ); Ok(NewSessionResponse { session_id: SessionId(goose_session.id.into()), @@ -624,34 +805,36 @@ impl GooseAcpAgent { args: LoadSessionRequest, cx: &JrConnectionCx, ) -> Result { - info!("ACP: Received load session request {:?}", args); + debug!(?args, "load session request"); let session_id = args.session_id.0.to_string(); let goose_session = SessionManager::get_session(&session_id, true) .await - .map_err(|e| { - error!("Failed to load session {}: {}", session_id, e); - sacp::Error::invalid_params() + .map_err(|e| sacp::Error { + code: sacp::ErrorCode::INVALID_PARAMS.code, + message: format!("Failed to load session {}: {}", session_id, e), + data: None, })?; - let conversation = goose_session.conversation.ok_or_else(|| { - error!("Session {} has no conversation data", session_id); - sacp::Error::internal_error() + let conversation = goose_session.conversation.ok_or_else(|| sacp::Error { + code: sacp::ErrorCode::INTERNAL_ERROR.code, + message: format!("Session {} has no conversation data", session_id), + data: None, })?; SessionManager::update_session(&session_id) .working_dir(args.cwd.clone()) .apply() .await - .map_err(|e| { - error!("Failed to update session working directory: {}", e); - sacp::Error::internal_error() + .map_err(|e| sacp::Error { + code: sacp::ErrorCode::INTERNAL_ERROR.code, + message: format!("Failed to update session working directory: {}", e), + data: None, })?; let mut session = GooseAcpSession { messages: conversation.clone(), - tool_call_ids: HashMap::new(), tool_requests: HashMap::new(), cancel_token: None, }; @@ -721,7 +904,11 @@ impl GooseAcpAgent { let mut sessions = self.sessions.lock().await; sessions.insert(session_id.clone(), session); - info!("Loaded ACP session {}", session_id); + info!( + session_id = %session_id, + session_type = "acp", + "Session loaded" + ); Ok(LoadSessionResponse { modes: None, @@ -739,9 +926,11 @@ impl GooseAcpAgent { { let mut sessions = self.sessions.lock().await; - let session = sessions - .get_mut(&session_id) - .ok_or_else(sacp::Error::invalid_params)?; + let session = sessions.get_mut(&session_id).ok_or_else(|| sacp::Error { + code: sacp::ErrorCode::INVALID_PARAMS.code, + message: format!("Session not found: {}", session_id), + data: None, + })?; session.cancel_token = Some(cancel_token.clone()); } @@ -758,9 +947,10 @@ impl GooseAcpAgent { .agent .reply(user_message, session_config, Some(cancel_token.clone())) .await - .map_err(|e| { - error!("Error getting agent reply: {}", e); - sacp::Error::internal_error() + .map_err(|e| sacp::Error { + code: sacp::ErrorCode::INTERNAL_ERROR.code, + message: format!("Error getting agent reply: {}", e), + data: None, })?; use futures::StreamExt; @@ -776,9 +966,11 @@ impl GooseAcpAgent { match event { Ok(goose::agents::AgentEvent::Message(message)) => { let mut sessions = self.sessions.lock().await; - let session = sessions - .get_mut(&session_id) - .ok_or_else(sacp::Error::invalid_params)?; + let session = sessions.get_mut(&session_id).ok_or_else(|| sacp::Error { + code: sacp::ErrorCode::INVALID_PARAMS.code, + message: format!("Session not found: {}", session_id), + data: None, + })?; session.messages.push(message.clone()); @@ -789,8 +981,11 @@ impl GooseAcpAgent { } Ok(_) => {} Err(e) => { - error!("Error in agent response stream: {}", e); - return Err(sacp::Error::internal_error()); + return Err(sacp::Error { + code: sacp::ErrorCode::INTERNAL_ERROR.code, + message: format!("Error in agent response stream: {}", e), + data: None, + }); } } } @@ -811,18 +1006,18 @@ impl GooseAcpAgent { } async fn on_cancel(&self, args: CancelNotification) -> Result<(), sacp::Error> { - info!("ACP: Received cancel request {:?}", args); + debug!(?args, "cancel request"); let session_id = args.session_id.0.to_string(); let mut sessions = self.sessions.lock().await; if let Some(session) = sessions.get_mut(&session_id) { if let Some(ref token) = session.cancel_token { - info!("Cancelling active prompt for session {}", session_id); + info!(session_id = %session_id, "prompt cancelled"); token.cancel(); } } else { - warn!("Cancel request for non-existent session: {}", session_id); + warn!(session_id = %session_id, "cancel request for unknown session"); } Ok(()) @@ -875,7 +1070,22 @@ impl JrMessageHandler for GooseAcpHandler { .await .if_request( |req: PromptRequest, req_cx: JrRequestCx| async { - req_cx.respond(self.agent.on_prompt(req, &cx).await?) + // Spawn the prompt processing in a task so we don't block the event loop. + // This allows permission responses to be processed while the agent is working. + let agent = self.agent.clone(); + let cx_clone = cx.clone(); + cx.spawn(async move { + match agent.on_prompt(req, &cx_clone).await { + Ok(response) => { + req_cx.respond(response)?; + } + Err(e) => { + req_cx.respond_with_error(e)?; + } + } + Ok(()) + })?; + Ok(()) }, ) .await @@ -888,8 +1098,7 @@ impl JrMessageHandler for GooseAcpHandler { } pub async fn run_acp_agent() -> Result<()> { - info!("Starting Goose ACP agent server on stdio"); - eprintln!("Goose ACP agent started. Listening on stdio..."); + info!("listening on stdio"); let outgoing = tokio::io::stdout().compat_write(); let incoming = tokio::io::stdin().compat(); @@ -908,11 +1117,86 @@ pub async fn run_acp_agent() -> Result<()> { #[cfg(test)] mod tests { - use sacp::schema::ResourceLink; + use super::*; + use sacp::schema::{EnvVariable, HttpHeader, McpServer, ResourceLink}; use std::io::Write; + use std::path::PathBuf; use tempfile::NamedTempFile; - - use crate::commands::acp::{format_tool_name, read_resource_link}; + use test_case::test_case; + + use crate::commands::acp::{ + format_tool_name, mcp_server_to_extension_config, read_resource_link, + }; + use goose::agents::ExtensionConfig; + + #[test_case( + McpServer::Stdio { + name: "github".into(), + command: PathBuf::from("/path/to/github-mcp-server"), + args: vec!["stdio".into()], + env: vec![EnvVariable { + name: "GITHUB_PERSONAL_ACCESS_TOKEN".into(), + value: "ghp_xxxxxxxxxxxx".into(), + meta: None, + }], + }, + Ok(ExtensionConfig::Stdio { + name: "github".into(), + description: String::new(), + cmd: "/path/to/github-mcp-server".into(), + args: vec!["stdio".into()], + envs: Envs::new( + [( + "GITHUB_PERSONAL_ACCESS_TOKEN".into(), + "ghp_xxxxxxxxxxxx".into() + )] + .into() + ), + env_keys: vec![], + timeout: None, + bundled: Some(false), + available_tools: vec![], + }) + )] + #[test_case( + McpServer::Http { + name: "github".into(), + url: "https://api.githubcopilot.com/mcp/".into(), + headers: vec![HttpHeader { + name: "Authorization".into(), + value: "Bearer ghp_xxxxxxxxxxxx".into(), + meta: None, + }], + }, + Ok(ExtensionConfig::StreamableHttp { + name: "github".into(), + description: String::new(), + uri: "https://api.githubcopilot.com/mcp/".into(), + envs: Envs::default(), + env_keys: vec![], + headers: HashMap::from([( + "Authorization".into(), + "Bearer ghp_xxxxxxxxxxxx".into() + )]), + timeout: None, + bundled: Some(false), + available_tools: vec![], + }) + )] + #[test_case( + McpServer::Sse { + name: "test-sse".into(), + url: "https://example.com/sse".into(), + headers: vec![], + }, + Err("SSE transport is deprecated and not supported: test-sse".to_string()) + )] + fn test_mcp_server_to_extension_config( + input: McpServer, + expected: Result, + ) { + assert_eq!(mcp_server_to_extension_config(input), expected); + } fn new_resource_link(content: &str) -> anyhow::Result<(ResourceLink, NamedTempFile)> { let mut file = NamedTempFile::new()?; @@ -981,4 +1265,41 @@ print(\"hello, world\") assert_eq!(format_tool_name("extension__"), "Extension: "); assert_eq!(format_tool_name("__tool"), ": Tool"); } + + #[test_case( + RequestPermissionOutcome::Selected { option_id: PermissionOptionId::from("allow_once".to_string()) }, + PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::AllowOnce }; + "allow_once_maps_to_allow_once" + )] + #[test_case( + RequestPermissionOutcome::Selected { option_id: PermissionOptionId::from("allow_always".to_string()) }, + PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::AlwaysAllow }; + "allow_always_maps_to_always_allow" + )] + #[test_case( + RequestPermissionOutcome::Selected { option_id: PermissionOptionId::from("reject_once".to_string()) }, + PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::DenyOnce }; + "reject_once_maps_to_deny_once" + )] + #[test_case( + RequestPermissionOutcome::Selected { option_id: PermissionOptionId::from("reject_always".to_string()) }, + PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::DenyOnce }; + "reject_always_maps_to_deny_once" + )] + #[test_case( + RequestPermissionOutcome::Selected { option_id: PermissionOptionId::from("unknown".to_string()) }, + PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::Cancel }; + "unknown_option_maps_to_cancel" + )] + #[test_case( + RequestPermissionOutcome::Cancelled, + PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::Cancel }; + "cancelled_maps_to_cancel" + )] + fn test_outcome_to_confirmation( + input: RequestPermissionOutcome, + expected: PermissionConfirmation, + ) { + assert_eq!(outcome_to_confirmation(&input), expected); + } } diff --git a/crates/goose-mcp/Cargo.toml b/crates/goose-mcp/Cargo.toml index 785cd5639c8a..6d2844d8e7d6 100644 --- a/crates/goose-mcp/Cargo.toml +++ b/crates/goose-mcp/Cargo.toml @@ -11,7 +11,7 @@ description.workspace = true workspace = true [dependencies] -rmcp = { version = "0.8.1", features = ["server", "client", "transport-io", "macros"] } +rmcp = { version = "0.10.0", features = ["server", "client", "transport-io", "macros"] } anyhow = "1.0.94" tokio = { version = "1", features = ["full"] } tokio-stream = { version = "0.1", features = ["io-util"] } diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index 5eebeba53249..aad4c1144301 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -133,6 +133,7 @@ dotenvy = "0.15.7" ctor = "0.2.9" test-case = "3.3" env-lock = "1.0.1" +rmcp = { workspace = true, features = ["transport-streamable-http-server"] } [[example]] name = "agent" diff --git a/crates/goose/src/agents/extension.rs b/crates/goose/src/agents/extension.rs index 3287a40b06e2..f89d9dfc0c46 100644 --- a/crates/goose/src/agents/extension.rs +++ b/crates/goose/src/agents/extension.rs @@ -142,7 +142,7 @@ pub enum ExtensionError { pub type ExtensionResult = Result; -#[derive(Debug, Clone, Deserialize, Serialize, Default, ToSchema)] +#[derive(Debug, Clone, Deserialize, Serialize, Default, ToSchema, PartialEq)] pub struct Envs { /// A map of environment variables to set, e.g. API_KEY -> some_secret, HOST -> host #[serde(default)] @@ -232,7 +232,7 @@ impl Envs { } /// Represents the different types of MCP extensions that can be added to the manager -#[derive(Debug, Clone, Deserialize, Serialize, ToSchema)] +#[derive(Debug, Clone, Deserialize, Serialize, ToSchema, PartialEq)] #[serde(tag = "type")] pub enum ExtensionConfig { /// Server-sent events client with a URI endpoint diff --git a/crates/goose/src/permission/permission_confirmation.rs b/crates/goose/src/permission/permission_confirmation.rs index 9e9d7e54fb3d..f56da1172000 100644 --- a/crates/goose/src/permission/permission_confirmation.rs +++ b/crates/goose/src/permission/permission_confirmation.rs @@ -15,7 +15,7 @@ pub enum PrincipalType { Tool, } -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] pub struct PermissionConfirmation { pub principal_type: PrincipalType, pub permission: Permission, diff --git a/crates/goose/tests/acp_integration_test.rs b/crates/goose/tests/acp_integration_test.rs index f50f5f9a00b3..e1928e932838 100644 --- a/crates/goose/tests/acp_integration_test.rs +++ b/crates/goose/tests/acp_integration_test.rs @@ -1,36 +1,89 @@ +mod common; + +use rmcp::transport::streamable_http_server::{ + session::local::LocalSessionManager, StreamableHttpServerConfig, StreamableHttpService, +}; +use rmcp::{ + handler::server::{router::tool::ToolRouter, wrapper::Parameters}, + model::*, + tool, tool_handler, tool_router, ErrorData as McpError, ServerHandler, +}; use sacp::schema::{ - ContentBlock, ContentChunk, InitializeRequest, NewSessionRequest, PromptRequest, + ContentBlock, ContentChunk, InitializeRequest, McpServer, NewSessionRequest, PromptRequest, + RequestPermissionOutcome, RequestPermissionRequest, RequestPermissionResponse, SessionNotification, SessionUpdate, StopReason, TextContent, VERSION as PROTOCOL_VERSION, }; use sacp::{ClientToAgent, JrConnectionCx}; -use std::path::Path; +use std::collections::VecDeque; use std::process::Stdio; use std::sync::{Arc, Mutex}; use std::time::Duration; use tokio::process::{Child, Command}; +use tokio::task::JoinHandle; use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt}; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; -const BASIC_RESPONSE: &str = include_str!("./test_data/openai_chat_completion_streaming.txt"); -const BASIC_TEXT: &str = "Hello! How can I assist you today? 🌍"; - #[tokio::test] async fn test_acp_basic_completion() { - let mock_server = setup_mock_openai(BASIC_RESPONSE).await; - let work_dir = tempfile::tempdir().unwrap(); + let prompt = "what is 1+1"; + let mock_server = setup_mock_openai(vec![( + format!(r#"\n{prompt}","role":"user""#), + include_str!("./test_data/openai_basic_response.txt"), + )]) + .await; + + run_acp_session(&mock_server, vec![], |cx, session_id, updates| async move { + let response = cx + .send_request(PromptRequest { + session_id, + prompt: vec![ContentBlock::Text(TextContent { + text: prompt.to_string(), + annotations: None, + meta: None, + })], + meta: None, + }) + .block_task() + .await + .unwrap(); + + assert_eq!(response.stop_reason, StopReason::EndTurn); + wait_for_text(&updates, "2", Duration::from_secs(5)).await; + }) + .await; +} - let updates = Arc::new(Mutex::new(Vec::::new())); - let child = spawn_goose_acp(&mock_server).await; +#[tokio::test] +async fn test_acp_with_mcp_http_server() { + let prompt = "Use the sum tool to calculate 1+1 and output only the resulting number."; + let (mcp_url, _handle) = spawn_mcp_http_server().await; + + let mock_server = setup_mock_openai(vec![ + ( + format!(r#"\n{prompt}","role":"user""#), + include_str!("./test_data/openai_tool_call_response.txt"), + ), + ( + r#""content":"2","role":"tool""#.to_string(), + include_str!("./test_data/openai_tool_result_response.txt"), + ), + ]) + .await; - run_acp_session(child, work_dir.path(), updates.clone(), |cx, session_id| { - let updates = updates.clone(); - async move { + run_acp_session( + &mock_server, + vec![McpServer::Http { + name: "calculator".into(), + url: mcp_url, + headers: vec![], + }], + |cx, session_id, updates| async move { let response = cx .send_request(PromptRequest { session_id, prompt: vec![ContentBlock::Text(TextContent { - text: "test message".to_string(), + text: prompt.to_string(), annotations: None, meta: None, })], @@ -41,10 +94,9 @@ async fn test_acp_basic_completion() { .unwrap(); assert_eq!(response.stop_reason, StopReason::EndTurn); - - wait_for_text(&updates, BASIC_TEXT, Duration::from_secs(5)).await; - } - }) + wait_for_text(&updates, "2", Duration::from_secs(5)).await; + }, + ) .await; } @@ -67,16 +119,50 @@ async fn wait_for_text( } } -async fn setup_mock_openai(streaming_response: &str) -> MockServer { +/// Each entry is (expected_body_substring, response_body). +/// Session description requests are handled automatically. +async fn setup_mock_openai(exchanges: Vec<(String, &'static str)>) -> MockServer { let mock_server = MockServer::start().await; + let queue: VecDeque<(String, &'static str)> = exchanges.into_iter().collect(); + let queue = Arc::new(Mutex::new(queue)); Mock::given(method("POST")) .and(path("/v1/chat/completions")) - .respond_with( - ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_string(streaming_response), - ) + .respond_with({ + let queue = queue.clone(); + move |req: &wiremock::Request| { + let body = String::from_utf8_lossy(&req.body); + + if body.contains("Reply with only a description in four words or less") { + return ResponseTemplate::new(200) + .insert_header("content-type", "application/json") + .set_body_string(include_str!( + "./test_data/openai_session_description.json" + )); + } + + let (expected, response) = { + let mut q = queue.lock().unwrap(); + match q.pop_front() { + Some(item) => item, + None => { + return ResponseTemplate::new(500) + .set_body_string(format!("unexpected request: {body}")); + } + } + }; + + if !body.contains(&expected) { + return ResponseTemplate::new(500).set_body_string(format!( + "expected body to contain: {expected}\nactual: {body}" + )); + } + + ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_string(response) + } + }) .mount(&mock_server) .await; @@ -87,43 +173,50 @@ fn extract_text(updates: &[SessionNotification]) -> String { updates .iter() .filter_map(|n| match &n.update { - SessionUpdate::AgentMessageChunk(ContentChunk { content, .. }) => match content { - ContentBlock::Text(t) => Some(t.text.clone()), - _ => None, - }, + SessionUpdate::AgentMessageChunk(ContentChunk { + content: ContentBlock::Text(t), + .. + }) => Some(t.text.clone()), _ => None, }) .collect() } async fn spawn_goose_acp(mock_server: &MockServer) -> Child { - Command::new("cargo") - .args(["run", "-p", "goose-cli", "--", "acp"]) + Command::new(&*common::GOOSE_BINARY) + .args(["acp"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .env("GOOSE_PROVIDER", "openai") .env("GOOSE_MODEL", "gpt-5-nano") + .env("GOOSE_MODE", "approve") .env("OPENAI_HOST", mock_server.uri()) .env("OPENAI_API_KEY", "test-key") + .env( + "RUST_LOG", + std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()), + ) .kill_on_drop(true) .spawn() .unwrap() } -async fn run_acp_session( - mut child: Child, - work_dir: &Path, - updates: Arc>>, - test_fn: F, -) where - F: FnOnce(JrConnectionCx, sacp::schema::SessionId) -> Fut, +async fn run_acp_session(mock_server: &MockServer, mcp_servers: Vec, test_fn: F) +where + F: FnOnce( + JrConnectionCx, + sacp::schema::SessionId, + Arc>>, + ) -> Fut, Fut: std::future::Future, { + let mut child = spawn_goose_acp(mock_server).await; + let work_dir = tempfile::tempdir().unwrap(); + let updates = Arc::new(Mutex::new(Vec::new())); let outgoing = child.stdin.take().unwrap().compat_write(); let incoming = child.stdout.take().unwrap().compat(); - let work_dir = work_dir.to_path_buf(); let transport = sacp::ByteStreams::new(outgoing, incoming); ClientToAgent::builder() @@ -137,31 +230,109 @@ async fn run_acp_session( }, sacp::on_receive_notification!(), ) - .with_client(transport, |cx: JrConnectionCx| async move { - cx.send_request(InitializeRequest { - protocol_version: PROTOCOL_VERSION, - client_capabilities: Default::default(), - client_info: Default::default(), - meta: None, - }) - .block_task() - .await - .unwrap(); - - let session = cx - .send_request(NewSessionRequest { - mcp_servers: vec![], - cwd: work_dir, + .on_receive_request( + async move |request: RequestPermissionRequest, request_cx, _connection_cx| { + let option_id = request.options.first().map(|opt| opt.id.clone()); + match option_id { + Some(id) => request_cx.respond(RequestPermissionResponse { + outcome: RequestPermissionOutcome::Selected { option_id: id }, + meta: None, + }), + None => request_cx.respond(RequestPermissionResponse { + outcome: RequestPermissionOutcome::Cancelled, + meta: None, + }), + } + }, + sacp::on_receive_request!(), + ) + .with_client( + transport, + move |cx: JrConnectionCx| async move { + cx.send_request(InitializeRequest { + protocol_version: PROTOCOL_VERSION, + client_capabilities: Default::default(), + client_info: Default::default(), meta: None, }) .block_task() .await .unwrap(); - test_fn(cx.clone(), session.session_id).await; + let session = cx + .send_request(NewSessionRequest { + mcp_servers, + cwd: work_dir.path().to_path_buf(), + meta: None, + }) + .block_task() + .await + .unwrap(); - Ok(()) - }) + test_fn(cx.clone(), session.session_id, updates).await; + Ok(()) + }, + ) .await .unwrap(); } + +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +struct SumRequest { + a: i32, + b: i32, +} + +#[derive(Clone)] +struct Calculator { + tool_router: ToolRouter, +} + +#[tool_router] +impl Calculator { + fn new() -> Self { + Self { + tool_router: Self::tool_router(), + } + } + + #[tool(description = "Calculate the sum of two numbers")] + fn sum( + &self, + Parameters(SumRequest { a, b }): Parameters, + ) -> Result { + Ok(CallToolResult::success(vec![Content::text( + (a + b).to_string(), + )])) + } +} + +#[tool_handler] +impl ServerHandler for Calculator { + fn get_info(&self) -> ServerInfo { + ServerInfo { + protocol_version: ProtocolVersion::V_2025_03_26, + capabilities: ServerCapabilities::builder().enable_tools().build(), + server_info: Implementation::from_build_env(), + instructions: Some("Calculator server with sum tool.".into()), + } + } +} + +async fn spawn_mcp_http_server() -> (String, JoinHandle<()>) { + let service = StreamableHttpService::new( + || Ok(Calculator::new()), + LocalSessionManager::default().into(), + StreamableHttpServerConfig::default(), + ); + let router = axum::Router::new().nest_service("/mcp", service); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let url = format!("http://{addr}/mcp"); + + let handle = tokio::spawn(async move { + axum::serve(listener, router).await.unwrap(); + }); + + (url, handle) +} diff --git a/crates/goose/tests/common.rs b/crates/goose/tests/common.rs new file mode 100644 index 000000000000..137179319dae --- /dev/null +++ b/crates/goose/tests/common.rs @@ -0,0 +1,43 @@ +use std::path::PathBuf; +use std::process::Command; +use std::sync::LazyLock; + +/// Build a binary from a package and return its path. +pub fn build_binary(package: &str, bin_name: &str) -> PathBuf { + let output = Command::new("cargo") + .args([ + "build", + "-p", + package, + "--bin", + bin_name, + "--message-format=json", + ]) + .output() + .expect("failed to build binary"); + + if !output.status.success() { + panic!("build failed: {}", String::from_utf8_lossy(&output.stderr)); + } + + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| serde_json::from_str::(line).ok()) + .filter(|msg| msg["reason"] == "compiler-artifact") + .filter(|msg| msg["target"]["name"] == bin_name) + .filter(|msg| { + msg["target"]["kind"] + .as_array() + .map(|k| k.iter().any(|v| v == "bin")) + .unwrap_or(false) + }) + .filter_map(|msg| msg["executable"].as_str().map(PathBuf::from)) + .next() + .expect("failed to find binary path in cargo output") +} + +#[allow(dead_code)] +pub static GOOSE_BINARY: LazyLock = LazyLock::new(|| build_binary("goose-cli", "goose")); +#[allow(dead_code)] +pub static CAPTURE_BINARY: LazyLock = + LazyLock::new(|| build_binary("goose-test", "capture")); diff --git a/crates/goose/tests/mcp_integration_test.rs b/crates/goose/tests/mcp_integration_test.rs index 57c0ef59441f..08849135815f 100644 --- a/crates/goose/tests/mcp_integration_test.rs +++ b/crates/goose/tests/mcp_integration_test.rs @@ -1,4 +1,4 @@ -use serde::Deserialize; +mod common; use std::collections::HashMap; use std::fs::File; @@ -20,21 +20,6 @@ use async_trait::async_trait; use goose::conversation::message::Message; use goose::providers::base::{Provider, ProviderMetadata, ProviderUsage, Usage}; use goose::providers::errors::ProviderError; -use once_cell::sync::Lazy; -use std::process::Command; - -#[derive(Deserialize)] -struct CargoBuildMessage { - reason: String, - target: Target, - executable: String, -} - -#[derive(Deserialize)] -struct Target { - name: String, - kind: Vec, -} #[derive(Clone)] pub struct MockProvider { @@ -75,44 +60,6 @@ impl Provider for MockProvider { } } -fn build_and_get_binary_path() -> PathBuf { - let output = Command::new("cargo") - .args([ - "build", - "--frozen", - "-p", - "goose-test", - "--bin", - "capture", - "--message-format=json", - ]) - .output() - .expect("failed to build binary"); - - if !output.status.success() { - panic!("build failed: {}", String::from_utf8_lossy(&output.stderr)); - } - - String::from_utf8_lossy(&output.stdout) - .lines() - .map(serde_json::from_str::) - .filter_map(Result::ok) - .filter(|message| message.reason == "compiler-artifact") - .filter_map(|message| { - if message.target.name == "capture" - && message.target.kind.contains(&String::from("bin")) - { - Some(PathBuf::from(message.executable)) - } else { - None - } - }) - .next() - .expect("failed to parse binary path") -} - -static REPLAY_BINARY_PATH: Lazy = Lazy::new(build_and_get_binary_path); - enum TestMode { Record, Playback, @@ -214,7 +161,7 @@ async fn test_replayed_session( TestMode::Record => "record", TestMode::Playback => "playback", }; - let cmd = REPLAY_BINARY_PATH.to_string_lossy().to_string(); + let cmd = common::CAPTURE_BINARY.to_string_lossy().to_string(); let mut args = vec!["stdio", mode_arg] .into_iter() .map(str::to_string) diff --git a/crates/goose/tests/test_data/openai_basic_response.txt b/crates/goose/tests/test_data/openai_basic_response.txt new file mode 100644 index 000000000000..4c3d0c69a071 --- /dev/null +++ b/crates/goose/tests/test_data/openai_basic_response.txt @@ -0,0 +1,9 @@ +data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}]} + +data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"content":"2"},"finish_reason":null}]} + +data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[{"index":0,"delta":{},"finish_reason":"stop"}]} + +data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[],"usage":{"prompt_tokens":100,"completion_tokens":10,"total_tokens":110}} + +data: [DONE] diff --git a/crates/goose/tests/test_data/openai_chat_completion_streaming.txt b/crates/goose/tests/test_data/openai_chat_completion_streaming.txt deleted file mode 100644 index 5f33282d9546..000000000000 --- a/crates/goose/tests/test_data/openai_chat_completion_streaming.txt +++ /dev/null @@ -1,27 +0,0 @@ -data: {"choices":[],"created":0,"id":"","prompt_filter_results":[{"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"prompt_index":0}]} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":"","role":"assistant"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":"Hello"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":"!"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" How"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" can"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" I"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" assist"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" you"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" today"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":"?"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":" 🌍"}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: {"choices":[{"finish_reason":"stop","index":0,"content_filter_offsets":{"check_offset":3458,"start_offset":3458,"end_offset":3494},"content_filter_results":{"hate":{"filtered":false,"severity":"safe"},"self_harm":{"filtered":false,"severity":"safe"},"sexual":{"filtered":false,"severity":"safe"},"violence":{"filtered":false,"severity":"safe"}},"delta":{"content":null}}],"created":1747592466,"id":"chatcmpl-BYcvCkaKJjQIM7e2j6vg08RIcY8qp","usage":{"completion_tokens":13,"completion_tokens_details":{"accepted_prediction_tokens":0,"rejected_prediction_tokens":0},"prompt_tokens":1675,"prompt_tokens_details":{"cached_tokens":1536},"total_tokens":1688},"model":"gpt-4o-2024-11-20","system_fingerprint":"fp_ee1d74bde0"} - -data: [DONE] diff --git a/crates/goose/tests/test_data/openai_session_description.json b/crates/goose/tests/test_data/openai_session_description.json new file mode 100644 index 000000000000..aab91b540205 --- /dev/null +++ b/crates/goose/tests/test_data/openai_session_description.json @@ -0,0 +1 @@ +{"id":"chatcmpl-test","object":"chat.completion","created":1766229622,"model":"gpt-5-nano","choices":[{"index":0,"message":{"role":"assistant","content":"Test session"},"finish_reason":"stop"}],"usage":{"prompt_tokens":79,"completion_tokens":10,"total_tokens":89}} diff --git a/crates/goose/tests/test_data/openai_tool_call_response.txt b/crates/goose/tests/test_data/openai_tool_call_response.txt new file mode 100644 index 000000000000..1a0c7b4cd261 --- /dev/null +++ b/crates/goose/tests/test_data/openai_tool_call_response.txt @@ -0,0 +1,9 @@ +data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":null,"tool_calls":[{"index":0,"id":"call_abc123","type":"function","function":{"name":"calculator__sum","arguments":""}}],"refusal":null},"finish_reason":null}]} + +data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\"a\":1,\"b\":1}"}}]},"finish_reason":null}]} + +data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]} + +data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[],"usage":{"prompt_tokens":100,"completion_tokens":50,"total_tokens":150}} + +data: [DONE] diff --git a/crates/goose/tests/test_data/openai_tool_result_response.txt b/crates/goose/tests/test_data/openai_tool_result_response.txt new file mode 100644 index 000000000000..6b1cf14b4fa4 --- /dev/null +++ b/crates/goose/tests/test_data/openai_tool_result_response.txt @@ -0,0 +1,9 @@ +data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}]} + +data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"content":"2"},"finish_reason":null}]} + +data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{},"finish_reason":"stop"}]} + +data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[],"usage":{"prompt_tokens":200,"completion_tokens":10,"total_tokens":210}} + +data: [DONE] From 9e3d6b52d1411b9b49c9524ca4903838e799fde9 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 23 Dec 2025 10:49:11 +0800 Subject: [PATCH 3/3] no-mem Signed-off-by: Adrian Cole --- Cargo.lock | 56 +++---------------- crates/goose-mcp/Cargo.toml | 2 +- crates/goose/tests/acp_integration_test.rs | 46 +++++++-------- .../test_data/openai_tool_call_response.txt | 4 +- .../test_data/openai_tool_result_response.txt | 2 +- 5 files changed, 30 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4ee20da797d6..b92d21d6c280 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3068,7 +3068,7 @@ dependencies = [ "rand 0.8.5", "regex", "reqwest 0.12.12", - "rmcp 0.9.1", + "rmcp", "sacp", "schemars", "serde", @@ -3120,7 +3120,7 @@ dependencies = [ "once_cell", "paste", "regex", - "rmcp 0.9.1", + "rmcp", "serde", "serde_json", "tokio", @@ -3158,7 +3158,7 @@ dependencies = [ "open", "rand 0.8.5", "regex", - "rmcp 0.9.1", + "rmcp", "rustyline", "sacp", "serde", @@ -3214,7 +3214,7 @@ dependencies = [ "rayon", "regex", "reqwest 0.11.27", - "rmcp 0.10.0", + "rmcp", "schemars", "serde", "serde_json", @@ -3269,7 +3269,7 @@ dependencies = [ "http 1.2.0", "rand 0.9.2", "reqwest 0.12.12", - "rmcp 0.9.1", + "rmcp", "rustls 0.23.31", "schemars", "serde", @@ -5334,12 +5334,6 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" -[[package]] -name = "pastey" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b867cad97c0791bbd3aaa6472142568c6c9e8f71937e98379f584cfb0cf35bec" - [[package]] name = "path_abs" version = "0.5.1" @@ -6293,7 +6287,7 @@ dependencies = [ "process-wrap", "rand 0.9.2", "reqwest 0.12.12", - "rmcp-macros 0.9.1", + "rmcp-macros", "schemars", "serde", "serde_json", @@ -6308,29 +6302,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "rmcp" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38b18323edc657390a6ed4d7a9110b0dec2dc3ed128eb2a123edfbafabdbddc5" -dependencies = [ - "async-trait", - "base64 0.22.1", - "chrono", - "futures", - "pastey", - "pin-project-lite", - "rmcp-macros 0.10.0", - "schemars", - "serde", - "serde_json", - "thiserror 2.0.17", - "tokio", - "tokio-stream", - "tokio-util", - "tracing", -] - [[package]] name = "rmcp-macros" version = "0.9.1" @@ -6344,19 +6315,6 @@ dependencies = [ "syn 2.0.111", ] -[[package]] -name = "rmcp-macros" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c75d0a62676bf8c8003c4e3c348e2ceb6a7b3e48323681aaf177fdccdac2ce50" -dependencies = [ - "darling 0.21.0", - "proc-macro2", - "quote", - "serde_json", - "syn 2.0.111", -] - [[package]] name = "ron" version = "0.12.0" @@ -6612,7 +6570,7 @@ dependencies = [ "futures-concurrency", "fxhash", "jsonrpcmsg", - "rmcp 0.9.1", + "rmcp", "sacp-derive", "schemars", "serde", diff --git a/crates/goose-mcp/Cargo.toml b/crates/goose-mcp/Cargo.toml index 6d2844d8e7d6..60a52a6d6ea1 100644 --- a/crates/goose-mcp/Cargo.toml +++ b/crates/goose-mcp/Cargo.toml @@ -11,7 +11,7 @@ description.workspace = true workspace = true [dependencies] -rmcp = { version = "0.10.0", features = ["server", "client", "transport-io", "macros"] } +rmcp = { workspace = true, features = ["server", "client", "transport-io", "macros"] } anyhow = "1.0.94" tokio = { version = "1", features = ["full"] } tokio-stream = { version = "0.1", features = ["io-util"] } diff --git a/crates/goose/tests/acp_integration_test.rs b/crates/goose/tests/acp_integration_test.rs index e1928e932838..e0a669af50ed 100644 --- a/crates/goose/tests/acp_integration_test.rs +++ b/crates/goose/tests/acp_integration_test.rs @@ -4,9 +4,8 @@ use rmcp::transport::streamable_http_server::{ session::local::LocalSessionManager, StreamableHttpServerConfig, StreamableHttpService, }; use rmcp::{ - handler::server::{router::tool::ToolRouter, wrapper::Parameters}, - model::*, - tool, tool_handler, tool_router, ErrorData as McpError, ServerHandler, + handler::server::router::tool::ToolRouter, model::*, tool, tool_handler, tool_router, + ErrorData as McpError, ServerHandler, }; use sacp::schema::{ ContentBlock, ContentChunk, InitializeRequest, McpServer, NewSessionRequest, PromptRequest, @@ -24,6 +23,9 @@ use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt}; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; +/// Fake code returned by the MCP server - an LLM couldn't know this from memory +const FAKE_CODE: &str = "test-uuid-12345-67890"; + #[tokio::test] async fn test_acp_basic_completion() { let prompt = "what is 1+1"; @@ -56,7 +58,7 @@ async fn test_acp_basic_completion() { #[tokio::test] async fn test_acp_with_mcp_http_server() { - let prompt = "Use the sum tool to calculate 1+1 and output only the resulting number."; + let prompt = "Use the get_code tool and output only its result."; let (mcp_url, _handle) = spawn_mcp_http_server().await; let mock_server = setup_mock_openai(vec![ @@ -65,7 +67,7 @@ async fn test_acp_with_mcp_http_server() { include_str!("./test_data/openai_tool_call_response.txt"), ), ( - r#""content":"2","role":"tool""#.to_string(), + format!(r#""content":"{FAKE_CODE}","role":"tool""#), include_str!("./test_data/openai_tool_result_response.txt"), ), ]) @@ -74,7 +76,7 @@ async fn test_acp_with_mcp_http_server() { run_acp_session( &mock_server, vec![McpServer::Http { - name: "calculator".into(), + name: "lookup".into(), url: mcp_url, headers: vec![], }], @@ -94,7 +96,7 @@ async fn test_acp_with_mcp_http_server() { .unwrap(); assert_eq!(response.stop_reason, StopReason::EndTurn); - wait_for_text(&updates, "2", Duration::from_secs(5)).await; + wait_for_text(&updates, FAKE_CODE, Duration::from_secs(5)).await; }, ) .await; @@ -277,51 +279,41 @@ where .unwrap(); } -#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] -struct SumRequest { - a: i32, - b: i32, -} - #[derive(Clone)] -struct Calculator { - tool_router: ToolRouter, +struct Lookup { + tool_router: ToolRouter, } #[tool_router] -impl Calculator { +impl Lookup { fn new() -> Self { Self { tool_router: Self::tool_router(), } } - #[tool(description = "Calculate the sum of two numbers")] - fn sum( - &self, - Parameters(SumRequest { a, b }): Parameters, - ) -> Result { - Ok(CallToolResult::success(vec![Content::text( - (a + b).to_string(), - )])) + /// Returns a fake code that an LLM couldn't know from memory + #[tool(description = "Get the code")] + fn get_code(&self) -> Result { + Ok(CallToolResult::success(vec![Content::text(FAKE_CODE)])) } } #[tool_handler] -impl ServerHandler for Calculator { +impl ServerHandler for Lookup { fn get_info(&self) -> ServerInfo { ServerInfo { protocol_version: ProtocolVersion::V_2025_03_26, capabilities: ServerCapabilities::builder().enable_tools().build(), server_info: Implementation::from_build_env(), - instructions: Some("Calculator server with sum tool.".into()), + instructions: Some("Lookup server with get_code tool.".into()), } } } async fn spawn_mcp_http_server() -> (String, JoinHandle<()>) { let service = StreamableHttpService::new( - || Ok(Calculator::new()), + || Ok(Lookup::new()), LocalSessionManager::default().into(), StreamableHttpServerConfig::default(), ); diff --git a/crates/goose/tests/test_data/openai_tool_call_response.txt b/crates/goose/tests/test_data/openai_tool_call_response.txt index 1a0c7b4cd261..fa022560a79a 100644 --- a/crates/goose/tests/test_data/openai_tool_call_response.txt +++ b/crates/goose/tests/test_data/openai_tool_call_response.txt @@ -1,6 +1,6 @@ -data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":null,"tool_calls":[{"index":0,"id":"call_abc123","type":"function","function":{"name":"calculator__sum","arguments":""}}],"refusal":null},"finish_reason":null}]} +data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":null,"tool_calls":[{"index":0,"id":"call_abc123","type":"function","function":{"name":"lookup__get_code","arguments":""}}],"refusal":null},"finish_reason":null}]} -data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\"a\":1,\"b\":1}"}}]},"finish_reason":null}]} +data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{}"}}]},"finish_reason":null}]} data: {"id":"chatcmpl-test1","object":"chat.completion.chunk","created":1766309803,"model":"gpt-5-nano","choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]} diff --git a/crates/goose/tests/test_data/openai_tool_result_response.txt b/crates/goose/tests/test_data/openai_tool_result_response.txt index 6b1cf14b4fa4..2a34ecc813cb 100644 --- a/crates/goose/tests/test_data/openai_tool_result_response.txt +++ b/crates/goose/tests/test_data/openai_tool_result_response.txt @@ -1,6 +1,6 @@ data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}]} -data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"content":"2"},"finish_reason":null}]} +data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"content":"test-uuid-12345-67890"},"finish_reason":null}]} data: {"id":"chatcmpl-test2","object":"chat.completion.chunk","created":1766309809,"model":"gpt-5-nano","choices":[{"index":0,"delta":{},"finish_reason":"stop"}]}