diff --git a/crates/goose/src/acp/fs.rs b/crates/goose/src/acp/fs.rs index 13755da1243a..e63849f97f82 100644 --- a/crates/goose/src/acp/fs.rs +++ b/crates/goose/src/acp/fs.rs @@ -1,3 +1,4 @@ +use crate::acp::tool_call_notifier::ToolCallNotifier; use crate::acp::tools::AcpAwareToolMeta; use crate::agents::mcp_client::{Error as McpError, McpClientTrait}; use crate::agents::platform_extensions::developer::edit::{ @@ -7,9 +8,9 @@ use crate::agents::platform_extensions::developer::shell::{ShellParams, OUTPUT_L use crate::agents::platform_extensions::developer::DeveloperClient; use agent_client_protocol::schema::v1::{ CreateTerminalRequest, Diff, EnvVariable, KillTerminalRequest, ReadTextFileRequest, - ReleaseTerminalRequest, SessionId, SessionNotification, SessionUpdate, Terminal, - TerminalOutputRequest, ToolCallContent, ToolCallId, ToolCallLocation, ToolCallUpdate, - ToolCallUpdateFields, ToolKind, WaitForTerminalExitRequest, WriteTextFileRequest, + ReleaseTerminalRequest, SessionId, Terminal, TerminalOutputRequest, ToolCallContent, + ToolCallId, ToolCallLocation, ToolCallUpdate, ToolCallUpdateFields, ToolKind, + WaitForTerminalExitRequest, WriteTextFileRequest, }; use agent_client_protocol::{Client, ConnectionTo}; use agent_client_protocol_schema::v1::TerminalId; @@ -64,6 +65,7 @@ pub(crate) struct AcpTools { pub(crate) inner: Arc, pub(crate) cx: ConnectionTo, pub(crate) session_id: SessionId, + pub(crate) tool_call_notifier: ToolCallNotifier, pub(crate) fs_read: bool, pub(crate) fs_write: bool, pub(crate) terminal: bool, @@ -107,14 +109,8 @@ impl AcpTools { fn update_tool_call(&self, ctx: &crate::agents::ToolCallContext, fields: ToolCallUpdateFields) { if let Some(ref req_id) = ctx.tool_call_request_id { let _ = self - .cx - .send_notification(SessionNotification::new( - self.session_id.clone(), - SessionUpdate::ToolCallUpdate(ToolCallUpdate::new( - ToolCallId::new(req_id.clone()), - fields, - )), - )) + .tool_call_notifier + .send_update(ToolCallUpdate::new(ToolCallId::new(req_id.clone()), fields)) .inspect_err(|e| tracing::error!("error updating tool call with client: {}", e)); } } diff --git a/crates/goose/src/acp/mod.rs b/crates/goose/src/acp/mod.rs index 935613ea07ae..40853040eb64 100644 --- a/crates/goose/src/acp/mod.rs +++ b/crates/goose/src/acp/mod.rs @@ -5,6 +5,7 @@ mod provider; mod response_builder; pub mod server; pub mod server_factory; +pub(crate) mod tool_call_notifier; pub(crate) mod tools; pub mod transport; diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index e05d697a045d..dfc9b057ea13 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -6,10 +6,9 @@ pub(super) use crate::acp::response_builder::{ build_session_info, build_session_setup_config, send_session_setup_notifications, session_meta, session_provider_selection, session_response_meta, should_refresh_inventory_for_session_init, }; -use crate::acp::tools::AcpAwareToolMeta; +use crate::acp::tool_call_notifier::ToolCallNotifier; use crate::acp::{PermissionDecision, ACP_CURRENT_MODEL}; use crate::agents::extension::{Envs, PLATFORM_EXTENSIONS}; -use crate::agents::extension_manager::TRUSTED_TOOL_UPDATE_META_KEY; use crate::agents::mcp_client::{GooseMcpHostInfo, McpClientTrait}; use crate::agents::platform_extensions::developer::DeveloperClient; use crate::agents::{ @@ -22,10 +21,9 @@ use crate::config::permission::PermissionManager; use crate::config::{Config, GooseMode}; use crate::conversation::message::{ ActionRequiredData, Message, MessageContent, SystemNotificationContent, SystemNotificationType, - ToolRequest, + ToolRequest, ToolResponse, }; use crate::execution::manager::{AgentManager, AgentManagerGetResult, RuntimeContext}; -use crate::mcp_utils::ToolResult; use crate::permission::permission_confirmation::PrincipalType; use crate::permission::{Permission, PermissionConfirmation}; use crate::providers::base::Provider; @@ -42,19 +40,19 @@ use crate::source_roots::SourceRoot; use crate::utils::sanitize_unicode_tags; use agent_client_protocol::schema::v1::{ AgentCapabilities, Annotations, AuthMethod, AuthMethodAgent, AuthenticateRequest, - AuthenticateResponse, BlobResourceContents, CancelNotification, CloseSessionRequest, - CloseSessionResponse, ConfigOptionUpdate, Content, ContentBlock, ContentChunk, Cost, - CurrentModeUpdate, EmbeddedResource, EmbeddedResourceResource, FileSystemCapabilities, - ForkSessionRequest, ForkSessionResponse, ImageContent, Implementation, InitializeRequest, - InitializeResponse, ListSessionsRequest, ListSessionsResponse, LoadSessionRequest, - LoadSessionResponse, McpCapabilities, McpServer, Meta, NewSessionRequest, NewSessionResponse, - PermissionOption, PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse, - RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, SessionCapabilities, - SessionCloseCapabilities, SessionConfigOption, SessionId, SessionInfoUpdate, - SessionListCapabilities, SessionNotification, SessionUpdate, SetSessionConfigOptionRequest, + AuthenticateResponse, CancelNotification, CloseSessionRequest, CloseSessionResponse, + ConfigOptionUpdate, Content, ContentBlock, ContentChunk, Cost, CurrentModeUpdate, + EmbeddedResourceResource, FileSystemCapabilities, ForkSessionRequest, ForkSessionResponse, + ImageContent, Implementation, InitializeRequest, InitializeResponse, ListSessionsRequest, + ListSessionsResponse, LoadSessionRequest, LoadSessionResponse, McpCapabilities, McpServer, + Meta, NewSessionRequest, NewSessionResponse, PermissionOption, PermissionOptionKind, + PromptCapabilities, PromptRequest, PromptResponse, RequestPermissionOutcome, + RequestPermissionRequest, ResourceLink, SessionCapabilities, SessionCloseCapabilities, + SessionConfigOption, SessionId, SessionInfoUpdate, SessionListCapabilities, + SessionNotification, SessionUpdate, SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModeRequest, SetSessionModeResponse, StopReason, - TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, ToolCallLocation, - ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, Usage, UsageUpdate, + TextContent, ToolCallContent, ToolCallId, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, + ToolKind, Usage, UsageUpdate, }; use agent_client_protocol::util::MatchDispatchFrom; use agent_client_protocol::{ @@ -65,9 +63,7 @@ use anyhow::Result; use fs_err as fs; use futures::future::{BoxFuture, FutureExt}; use futures::stream::{self, StreamExt}; -use rmcp::model::{ - AnnotateAble, CallToolResult, RawContent, RawTextContent, ResourceContents, Role, -}; +use rmcp::model::{AnnotateAble, RawTextContent, Role}; use serde::Deserialize; use std::collections::{HashMap, HashSet}; use std::panic::AssertUnwindSafe; @@ -80,6 +76,13 @@ use tracing::{debug, error, info, warn}; use url::Url; use uuid::Uuid; +use self::tool_calls::chain::{extend_chain_membership, ToolChain}; +use self::tool_calls::conversion::{ + extract_tool_call_update_meta, format_tool_name, pending_tool_call_from_request, + tool_call_identity_meta, tool_call_update_fields_from_response, +}; +use self::tool_calls::enrichment::{ChainSummaryEnrichmentContext, ToolTitleEnrichmentContext}; + mod agent_requests; pub use agent_requests::agent_request_schemas; mod agent_mentions; @@ -105,6 +108,7 @@ mod resources; mod schedule; mod slash_commands; mod sources; +mod tool_calls; mod tool_notifications; mod tools; @@ -187,18 +191,6 @@ struct ActivePromptRun { cancel_token: CancellationToken, } -/// A run of consecutive ToolRequest blocks within one assistant message, -/// tracked by [`GooseAcpSession::chain_membership`]. Used to drive a single -/// LLM summary for the whole run once every step has a recorded ToolResponse. -#[derive(Debug, Clone)] -struct ToolChain { - /// Tool call ids in document order. Always `len() >= 2`. - ids: Vec, - /// The message_id of the assistant message containing these tool calls. - /// Used to persist chain summaries back to the messages table. - message_id: String, -} - pub struct GooseAcpAgentOptions { pub provider_factory: AcpProviderFactory, pub builtins: Vec, @@ -466,136 +458,6 @@ async fn resolve_provider_default_model_config( }) } -fn get_requested_line(arguments: Option<&rmcp::model::JsonObject>) -> Option { - arguments - .and_then(|args| args.get("line")) - .and_then(|v| v.as_u64()) - .map(|l| l as u32) -} - -fn is_developer_file_tool(tool_name: &str) -> bool { - matches!(tool_name, "read" | "write" | "edit") -} - -fn extract_locations_from_meta( - tool_response: &crate::conversation::message::ToolResponse, -) -> Option> { - let result = tool_response.tool_result.as_ref().ok()?; - let meta = result.meta.as_ref()?; - let locations_val = meta.get("tool_locations")?; - let entries: Vec = serde_json::from_value(locations_val.clone()).ok()?; - let locations = entries - .into_iter() - .filter_map(|entry| { - let path = entry.get("path")?.as_str()?; - let line = entry.get("line").and_then(|v| v.as_u64()).map(|l| l as u32); - Some(ToolCallLocation::new(path).line(line)) - }) - .collect::>(); - if locations.is_empty() { - None - } else { - Some(locations) - } -} - -fn extract_tool_locations( - tool_request: &crate::conversation::message::ToolRequest, - tool_response: &crate::conversation::message::ToolResponse, -) -> Vec { - let mut locations = Vec::new(); - - if let Ok(tool_call) = &tool_request.tool_call { - if !is_developer_file_tool(tool_call.name.as_ref()) { - return locations; - } - - let tool_name = tool_call.name.as_ref(); - let path_str = tool_call - .arguments - .as_ref() - .and_then(|args| args.get("path")) - .and_then(|p| p.as_str()); - - if let Some(path_str) = path_str { - if matches!(tool_name, "read") { - let line = get_requested_line(tool_call.arguments.as_ref()); - locations.push(ToolCallLocation::new(path_str).line(line)); - return locations; - } - - if matches!(tool_name, "write" | "edit") { - locations.push(ToolCallLocation::new(path_str).line(1)); - return locations; - } - - let command = tool_call - .arguments - .as_ref() - .and_then(|args| args.get("command")) - .and_then(|c| c.as_str()); - - if let Ok(result) = &tool_response.tool_result { - for content in &result.content { - if let RawContent::Text(text_content) = &content.raw { - let text = &text_content.text; - - match command { - Some("view") => { - let line = extract_view_line_range(text) - .map(|range| range.0 as u32) - .or(Some(1)); - locations.push(ToolCallLocation::new(path_str).line(line)); - } - Some("str_replace") | Some("insert") => { - let line = extract_first_line_number(text) - .map(|l| l as u32) - .or(Some(1)); - locations.push(ToolCallLocation::new(path_str).line(line)); - } - Some("write") => { - locations.push(ToolCallLocation::new(path_str).line(1)); - } - _ => { - locations.push(ToolCallLocation::new(path_str).line(1)); - } - } - break; - } - } - } - - if locations.is_empty() { - locations.push(ToolCallLocation::new(path_str).line(1)); - } - } - } - - locations -} - -fn extract_view_line_range(text: &str) -> Option<(usize, usize)> { - 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()?; - let end = if caps.get(2)?.as_str() == "end" { - start - } else { - caps.get(2)?.as_str().parse::().ok()? - }; - return Some((start, end)); - } - None -} - -fn extract_first_line_number(text: &str) -> Option { - 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(); - } - None -} - fn read_resource_link(link: ResourceLink) -> Option { let url = Url::parse(&link.uri).ok()?; if url.scheme() == "file" { @@ -612,194 +474,6 @@ fn read_resource_link(link: ResourceLink) -> Option { } } -fn format_tool_name(tool_name: &str) -> String { - if let Some((extension, tool)) = tool_name.split_once("__") { - format!( - "{}: {}", - extension.replace('_', " "), - tool.replace('_', " ") - ) - } else { - tool_name.replace('_', " ") - } -} - -/// Build a short fallback title from the tool name and arguments by extracting -/// the most useful value (file path, command, query, url, etc.). -fn summarize_tool_call(tool_name: &str, arguments: Option<&serde_json::Value>) -> String { - let base = format_tool_name(tool_name); - - let detail = arguments.and_then(|args| { - let obj = args.as_object()?; - let keys = [ - "path", "file", "command", "query", "url", "uri", "name", "pattern", "source", - ]; - for key in &keys { - if let Some(v) = obj.get(*key) { - let s = match v { - serde_json::Value::String(s) => s.clone(), - other => other.to_string(), - }; - if !s.is_empty() { - let first_line = s.lines().next().unwrap_or(&s); - if first_line.len() > 60 { - return Some(format!("{}…", crate::utils::safe_truncate(first_line, 57))); - } - return Some(first_line.to_string()); - } - } - } - None - }); - - match detail { - Some(d) => format!("{base} · {d}"), - None => base, - } -} - -fn tool_call_identity_meta(tool_request: &ToolRequest) -> Option { - let tool_call = tool_request.tool_call.as_ref().ok()?; - let tool_name = tool_call.name.to_string(); - let extension_name = tool_request - .tool_meta - .as_ref() - .and_then(|meta| meta.get("goose_extension")) - .and_then(serde_json::Value::as_str) - .map(ToString::to_string) - .or_else(|| { - tool_name - .split_once("__") - .map(|(extension_name, _)| extension_name.to_string()) - }); - - let mut tool_call_meta = serde_json::Map::new(); - tool_call_meta.insert("toolName".to_string(), serde_json::Value::String(tool_name)); - if let Some(extension_name) = extension_name { - tool_call_meta.insert( - "extensionName".to_string(), - serde_json::Value::String(extension_name), - ); - } - - let mut goose_meta = serde_json::Map::new(); - goose_meta.insert( - "toolCall".to_string(), - serde_json::Value::Object(tool_call_meta), - ); - - let mut meta = serde_json::Map::new(); - meta.insert("goose".to_string(), serde_json::Value::Object(goose_meta)); - Some(meta) -} - -/// Add `goose.toolChainSummary = { summary, count }` to a `Meta` blob, -/// preserving any existing `goose.*` keys (e.g. `goose.toolCall` set by -/// [`tool_call_identity_meta`]). -fn with_tool_chain_summary_meta(base: Option, summary: &str, count: usize) -> Option { - let mut meta = base.unwrap_or_default(); - let goose_entry = meta - .entry("goose".to_string()) - .or_insert_with(|| serde_json::Value::Object(serde_json::Map::new())); - let goose_obj = match goose_entry { - serde_json::Value::Object(obj) => obj, - other => { - *other = serde_json::Value::Object(serde_json::Map::new()); - match other { - serde_json::Value::Object(obj) => obj, - _ => unreachable!(), - } - } - }; - let mut chain = serde_json::Map::new(); - chain.insert( - "summary".to_string(), - serde_json::Value::String(summary.to_string()), - ); - chain.insert( - "count".to_string(), - serde_json::Value::Number(serde_json::Number::from(count)), - ); - goose_obj.insert( - "toolChainSummary".to_string(), - serde_json::Value::Object(chain), - ); - Some(meta) -} - -struct PendingToolCall { - tool_call: ToolCall, - identity_meta: Option, - fallback_title: String, -} - -/// If `buffer` holds a multi-tool run (≥ 2 tool requests), (re)register a -/// [`ToolChain`] in `chain_membership` anchored on the **first** tool's -/// message_id (the row [`SessionManager::update_tool_request_meta`] will patch -/// when persisting the LLM-generated summary). Does **not** clear the buffer -/// — chains can grow as more tools arrive (sequential tool use), so callers -/// keep accumulating and re-registering with the larger set of ids. -/// -/// The buffer contains `(tool_call_id, message_id)` pairs in arrival order, -/// fed by the prompt stream loop. Sequential tool use (Bedrock/Anthropic) -/// interleaves request → response → request → response across separate -/// `AgentEvent::Message` events, so a per-event view would only see length-1 -/// chains and miss the run. Tool responses are chain-neutral (they don't -/// split the run); only non-tool content (text, thinking, image, etc.) does, -/// matching the frontend's `groupContentSections` behavior. -fn extend_chain_membership( - buffer: &[(String, String)], - chain_membership: &mut HashMap>, -) { - if buffer.len() >= 2 { - let ids: Vec = buffer.iter().map(|(id, _)| id.clone()).collect(); - let message_id = buffer[0].1.clone(); - let chain = Arc::new(ToolChain { - ids: ids.clone(), - message_id, - }); - for id in ids { - chain_membership.insert(id, chain.clone()); - } - } -} - -fn pending_tool_call_from_request(tool_request: &ToolRequest) -> PendingToolCall { - let tool_name = match &tool_request.tool_call { - Ok(tool_call) => tool_call.name.to_string(), - Err(_) => "error".to_string(), - }; - let args_value = tool_request - .tool_call - .as_ref() - .ok() - .and_then(|tc| tc.arguments.as_ref()) - .map(|a| serde_json::Value::Object(a.clone())); - let fallback_title = summarize_tool_call(&tool_name, args_value.as_ref()); - let identity_meta = tool_call_identity_meta(tool_request); - - // Prefer the persisted LLM-generated title when available so replay (and - // any subsequent live initial ToolCall after the title task has already - // resolved) emits the nice title up front, with no flash of the - // deterministic fallback. - let initial_title = tool_request - .persisted_title() - .map(|s| s.to_string()) - .unwrap_or_else(|| fallback_title.clone()); - - let mut tool_call = ToolCall::new(ToolCallId::new(tool_request.id.clone()), initial_title) - .status(ToolCallStatus::Pending); - if let Some(args) = args_value { - tool_call = tool_call.raw_input(args); - } - - PendingToolCall { - tool_call, - identity_meta, - fallback_title, - } -} - fn builtin_to_extension_config(name: &str) -> ExtensionConfig { if let Some(def) = PLATFORM_EXTENSIONS.get(name) { ExtensionConfig::Platform { @@ -1118,10 +792,12 @@ impl GooseAcpAgent { } }; + let session_id = SessionId::new(session.id.clone()); let client: Arc = Arc::new(AcpTools { inner: Arc::new(dev_client), cx: cx.clone(), - session_id: SessionId::new(session.id.clone()), + session_id: session_id.clone(), + tool_call_notifier: ToolCallNotifier::new(cx, &session_id), fs_read: client_fs_capabilities.read_text_file, fs_write: client_fs_capabilities.write_text_file, terminal: client_terminal, @@ -1490,10 +1166,8 @@ impl GooseAcpAgent { let initial_tool_call = pending_tool_call .tool_call .meta(pending_tool_call.identity_meta.clone()); - cx.send_notification(SessionNotification::new( - session_id.clone(), - SessionUpdate::ToolCall(initial_tool_call), - ))?; + let tool_call_notifier = ToolCallNotifier::new(cx, session_id); + tool_call_notifier.send_initial(initial_tool_call)?; if Config::global() .get_goose_disable_tool_call_summary() @@ -1503,160 +1177,20 @@ impl GooseAcpAgent { } if let Ok(tool_call) = &tool_request.tool_call { - let agent = session.agent.clone(); - let sid = session_id.clone(); - let request_id = tool_request.id.clone(); - let cx = cx.clone(); - let name = tool_call.name.to_string(); - let identity_meta = pending_tool_call.identity_meta.clone(); - let fallback_title = pending_tool_call.fallback_title.clone(); - let session_id_for_persist = session_id_for_persist.to_string(); - let message_id_for_persist = message_id.map(|s| s.to_string()); - let session_manager = self.session_manager.clone(); - let args_json = tool_call - .arguments - .as_ref() - .map(|a| { - let s = serde_json::to_string(a).unwrap_or_default(); - if s.len() > 300 { - format!("{}…", crate::utils::safe_truncate(&s, 300)) - } else { - s - } - }) - .unwrap_or_default(); - - tokio::spawn(async move { - let (title, from_llm) = match agent.provider().await { - Ok(provider) => { - if provider.manages_own_context() { - return; - } - - let system = - "Summarize this tool call in a short lowercase phrase (3-8 words). \ - No punctuation. No quotes. Examples: reading project configuration, \ - checking network connectivity, listing files in src directory"; - let user_text = format!("Tool: {name}\nArguments: {args_json}"); - let message = Message::user().with_text(&user_text); - let model_config = match agent.model_config_for_session(&sid.0).await { - Ok(config) => config, - Err(_) => return, - }; - let fast_model_config = match crate::model_config::get_fast_model( - provider.get_name(), - &model_config, - ) - .await - { - Ok(config) => config, - Err(_) => return, - }; - // The fast model occasionally returns an empty response - // under load (rate limiting, transient network). One - // retry with a short backoff is enough to recover the - // common cases without paying for the regular model. - let mut llm_outcome: Option = None; - for attempt in 0..2 { - match crate::session_context::with_session_id( - Some(sid.0.to_string()), - provider.complete( - &fast_model_config, - system, - std::slice::from_ref(&message), - &[], - ), - ) - .await - { - Ok((response, _)) => { - let summary: String = response - .content - .iter() - .filter_map(|c: &MessageContent| c.as_text()) - .collect::() - .trim() - .to_string(); - if !summary.is_empty() { - llm_outcome = Some(summary); - break; - } - if attempt == 0 { - warn!( - "tool call summary: fast_complete returned empty for {request_id} ({name}), retrying once", - ); - tokio::time::sleep(std::time::Duration::from_millis(150)) - .await; - } - } - Err(e) => { - if attempt == 0 { - warn!( - "tool call summary: fast_complete errored for {request_id} ({name}): {e}, retrying once", - ); - tokio::time::sleep(std::time::Duration::from_millis(150)) - .await; - } else { - warn!( - "tool call summary: fast_complete errored for {request_id} ({name}) after retry: {e}", - ); - } - } - } - } - match llm_outcome { - Some(summary) => (summary, true), - None => { - warn!( - "tool call summary: falling back to deterministic title for {request_id} ({name}) — replay will not show an LLM summary for this call", - ); - (fallback_title.clone(), false) - } - } - } - Err(e) => { - warn!("tool call summary: failed to get provider: {e}"); - (fallback_title.clone(), false) - } - }; - - let fields = ToolCallUpdateFields::new().title(title.clone()); - let _ = cx.send_notification(SessionNotification::new( - sid, - SessionUpdate::ToolCallUpdate( - ToolCallUpdate::new(ToolCallId::new(request_id.clone()), fields) - .meta(identity_meta), - ), - )); - - // Best-effort persistence: only persist the LLM-generated title - // (not the deterministic fallback) so reload uses fallback_title - // for older or failed cases just like today. - if from_llm { - if let Some(msg_id) = message_id_for_persist { - let patch = serde_json::json!({ - crate::conversation::message::TOOL_META_TITLE_KEY: title, - }); - if let Err(e) = session_manager - .update_tool_request_meta( - &session_id_for_persist, - &msg_id, - &request_id, - patch, - ) - .await - { - warn!( - "tool call summary: persist failed for {request_id} in {msg_id}: {e}", - ); - } - } else { - warn!( - "tool call summary: missing message_id for {request_id} — title will not survive reload", - ); - } - } - }); + ToolTitleEnrichmentContext::new( + &session.agent, + session_id, + &tool_call_notifier, + &self.session_manager, + session_id_for_persist, + message_id, + ) + .spawn_title_enrichment( + tool_request.id.clone(), + tool_call, + pending_tool_call.identity_meta.clone(), + pending_tool_call.fallback_title.clone(), + ); } Ok(()) @@ -1664,54 +1198,33 @@ impl GooseAcpAgent { async fn handle_tool_response( &self, - tool_response: &crate::conversation::message::ToolResponse, + tool_response: &ToolResponse, session_id: &SessionId, session_id_str: &str, message_id: Option<&str>, session: &mut GooseAcpSession, cx: &ConnectionTo, ) -> Result<(), agent_client_protocol::Error> { - let status = match &tool_response.tool_result { - Ok(result) if result.is_error == Some(true) => ToolCallStatus::Failed, - Ok(_) => ToolCallStatus::Completed, - Err(_) => ToolCallStatus::Failed, - }; - - let mut fields = ToolCallUpdateFields::new().status(status); - if let Some(raw_output) = extract_tool_raw_output(&tool_response.tool_result) { - fields = fields.raw_output(raw_output); - } - if !tool_response - .tool_result - .as_ref() - .is_ok_and(|r| r.is_acp_aware()) - { - let content = build_tool_call_content(&tool_response.tool_result); - fields = fields.content(content); - - let locations = extract_locations_from_meta(tool_response).unwrap_or_else(|| { - if let Some(tool_request) = session.tool_requests.get(&tool_response.id) { - extract_tool_locations(tool_request, tool_response) - } else { - Vec::new() - } - }); - if !locations.is_empty() { - fields = fields.locations(locations); - } - } + let fields = tool_call_update_fields_from_response( + tool_response, + session.tool_requests.get(&tool_response.id), + ); let update = ToolCallUpdate::new(ToolCallId::new(tool_response.id.clone()), fields) .meta(extract_tool_call_update_meta(tool_response)); - cx.send_notification(SessionNotification::new( - session_id.clone(), - SessionUpdate::ToolCallUpdate(update), - ))?; + let tool_call_notifier = ToolCallNotifier::new(cx, session_id); + tool_call_notifier.send_update(update)?; // Chain summarization: when this response completes a multi-tool // chain, fire one LLM summary covering the run. session.responded_tool_ids.insert(tool_response.id.clone()); - self.maybe_summarize_chain(&tool_response.id, session_id, session_id_str, session, cx); + self.maybe_summarize_chain( + &tool_response.id, + session_id, + session_id_str, + session, + &tool_call_notifier, + ); let _ = message_id; Ok(()) @@ -1728,7 +1241,7 @@ impl GooseAcpAgent { session_id: &SessionId, _session_id_str: &str, session: &mut GooseAcpSession, - cx: &ConnectionTo, + tool_call_notifier: &ToolCallNotifier, ) { let Some(chain) = session.chain_membership.get(tool_call_id).cloned() else { warn!( @@ -1768,8 +1281,6 @@ impl GooseAcpAgent { return; } - let agent = session.agent.clone(); - // Snapshot (name, args_json) for each step in document order. let steps: Vec<(String, String)> = chain .ids @@ -1800,133 +1311,19 @@ impl GooseAcpAgent { .get(first_id) .and_then(tool_call_identity_meta); - let sid = session_id.clone(); - let chain_for_task = chain.clone(); - let cx = cx.clone(); - let session_manager = self.session_manager.clone(); - - let first_id = first_id.clone(); - tokio::spawn(async move { - let provider = match agent.provider().await { - Ok(p) => p, - Err(e) => { - warn!( - "tool chain summary: failed to get provider for chain anchored at {first_id}: {e}", - ); - return; - } - }; - if provider.manages_own_context() { - warn!( - "tool chain summary: provider manages own context; skipping chain anchored at {first_id}", - ); - return; - } - - let system = "Summarize this sequence of tool calls in a short lowercase phrase \ - (3-8 words). No punctuation. No quotes. \ - Examples: applied dark mode polish, scanned for security issues, \ - refactored config loading"; - - let mut user_text = String::from("Tool call sequence:\n"); - for (i, (name, args)) in steps.iter().enumerate() { - user_text.push_str(&format!("Step {}: {} {}\n", i + 1, name, args)); - } - let message = Message::user().with_text(&user_text); - let model_config = match agent.model_config_for_session(&sid.0).await { - Ok(config) => config, - Err(_) => return, - }; - let fast_model_config = - match crate::model_config::get_fast_model(provider.get_name(), &model_config).await - { - Ok(config) => config, - Err(_) => return, - }; - - // Match the per-tool retry policy: one retry on empty/error keeps - // the chain header reliable when the fast model is rate-limited or - // momentarily flaky, without escalating to the regular model. - let mut summary: Option = None; - for attempt in 0..2 { - match crate::session_context::with_session_id( - Some(sid.0.to_string()), - provider.complete( - &fast_model_config, - system, - std::slice::from_ref(&message), - &[], - ), - ) - .await - { - Ok((response, _)) => { - let s = response - .content - .iter() - .filter_map(|c: &MessageContent| c.as_text()) - .collect::() - .trim() - .to_string(); - if !s.is_empty() { - summary = Some(s); - break; - } - if attempt == 0 { - warn!( - "tool chain summary: fast_complete returned empty for chain anchored at {first_id} ({} steps), retrying once", - steps.len(), - ); - tokio::time::sleep(std::time::Duration::from_millis(150)).await; - } - } - Err(e) => { - if attempt == 0 { - warn!( - "tool chain summary: fast_complete errored for chain anchored at {first_id}: {e}, retrying once", - ); - tokio::time::sleep(std::time::Duration::from_millis(150)).await; - } else { - warn!( - "tool chain summary: fast_complete errored for chain anchored at {first_id} after retry: {e}", - ); - } - } - } - } - let Some(summary) = summary else { - warn!( - "tool chain summary: no LLM summary produced for chain anchored at {first_id} — replay will fall back to the deterministic phrase", - ); - return; - }; - - let count = chain_for_task.ids.len(); - let patch = serde_json::json!({ - crate::conversation::message::TOOL_META_CHAIN_SUMMARY_KEY: { - "summary": &summary, - "count": count, - }, - }); - if let Err(e) = session_manager - .update_tool_request_meta(&sid.0, &chain_for_task.message_id, &first_id, patch) - .await - { - warn!( - "tool chain summary: persist failed for chain anchored at {first_id} in {}: {e}", - chain_for_task.message_id, - ); - } - - let meta = with_tool_chain_summary_meta(identity_meta, &summary, count); - let fields = ToolCallUpdateFields::new(); - let _ = cx.send_notification(SessionNotification::new( - sid, - SessionUpdate::ToolCallUpdate( - ToolCallUpdate::new(ToolCallId::new(first_id), fields).meta(meta), - ), - )); - }); + ChainSummaryEnrichmentContext::new( + &session.agent, + session_id, + tool_call_notifier, + &self.session_manager, + ) + .spawn_chain_summary( + first_id.clone(), + chain.message_id.clone(), + steps, + identity_meta, + chain.ids.len(), + ); } #[allow(clippy::too_many_arguments)] @@ -2173,21 +1570,6 @@ fn message_update_meta(message_id: Option<&str>, created: i64, steer: bool) -> M meta } -fn extract_tool_call_update_meta( - tool_response: &crate::conversation::message::ToolResponse, -) -> Option { - let tool_result = tool_response.tool_result.as_ref().ok()?; - let goose_meta = tool_result - .meta - .as_ref()? - .0 - .get(TRUSTED_TOOL_UPDATE_META_KEY)? - .clone(); - let mut meta_map = serde_json::Map::new(); - meta_map.insert("goose".to_string(), goose_meta); - Some(meta_map) -} - fn replay_message_meta(message: &Message) -> Meta { let mut meta = serde_json::Map::new(); meta.insert( @@ -2227,57 +1609,6 @@ fn merge_replay_message_meta(meta: Option, message: &Message) -> Meta { meta } -fn build_tool_call_content(tool_result: &ToolResult) -> Vec { - match tool_result { - Ok(result) => result - .content - .iter() - .filter_map(|content| match &content.raw { - RawContent::Text(val) => Some(ToolCallContent::Content(Content::new( - ContentBlock::Text(TextContent::new(val.text.clone())), - ))), - RawContent::Image(val) => Some(ToolCallContent::Content(Content::new( - ContentBlock::Image(ImageContent::new(val.data.clone(), val.mime_type.clone())), - ))), - RawContent::Resource(val) => { - let resource = match &val.resource { - ResourceContents::TextResourceContents { - mime_type, - text, - uri, - .. - } => EmbeddedResourceResource::TextResourceContents( - TextResourceContents::new(text.clone(), uri.clone()) - .mime_type(mime_type.clone()), - ), - ResourceContents::BlobResourceContents { - mime_type, - blob, - uri, - .. - } => EmbeddedResourceResource::BlobResourceContents( - BlobResourceContents::new(blob.clone(), uri.clone()) - .mime_type(mime_type.clone()), - ), - }; - Some(ToolCallContent::Content(Content::new( - ContentBlock::Resource(EmbeddedResource::new(resource)), - ))) - } - RawContent::Audio(_) | RawContent::ResourceLink(_) => None, - }) - .collect(), - Err(_) => Vec::new(), - } -} - -fn extract_tool_raw_output(tool_result: &ToolResult) -> Option { - tool_result - .as_ref() - .ok() - .and_then(|result| result.structured_content.clone()) -} - impl GooseAcpAgent { async fn on_initialize( &self, @@ -2773,10 +2104,8 @@ impl GooseAcpAgent { if let Some(update) = tool_notifications::tool_notification_update(request_id, notification) { - cx.send_notification(SessionNotification::new( - args.session_id.clone(), - update, - ))?; + let tool_call_notifier = ToolCallNotifier::new(cx, &args.session_id); + tool_call_notifier.send_update(update)?; } } Ok(crate::agents::AgentEvent::MessageUsage { message_id, usage }) => { @@ -3233,14 +2562,15 @@ pub async fn run(builtins: Vec) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use crate::conversation::message::{ToolRequest, ToolResponse}; + use crate::acp::server::tool_calls::enrichment::with_tool_chain_summary_meta; + use crate::conversation::message::ToolRequest; use crate::session::session_manager::SessionType; use agent_client_protocol::schema::v1::{ EnvVariable, HttpHeader, McpServer, McpServerHttp, McpServerSse, McpServerStdio, PermissionOptionId, ResourceLink, SelectedPermissionOutcome, }; use goose_providers::conversation::token_usage::Usage as TokenUsage; - use rmcp::model::{CallToolRequestParams, Content as RmcpContent}; + use rmcp::model::CallToolRequestParams; use std::io::Write; use std::path::PathBuf; use tempfile::NamedTempFile; @@ -3336,200 +2666,6 @@ print(\"hello, world\") assert_eq!(result, expected,) } - #[test] - fn test_format_tool_name_with_extension() { - assert_eq!(format_tool_name("developer__edit"), "developer: edit"); - assert_eq!( - format_tool_name("platform__manage_extensions"), - "platform: manage extensions" - ); - assert_eq!(format_tool_name("todo__write"), "todo: write"); - } - - #[test] - fn test_format_tool_name_without_extension() { - assert_eq!(format_tool_name("simple_tool"), "simple tool"); - assert_eq!(format_tool_name("another_name"), "another name"); - assert_eq!(format_tool_name("single"), "single"); - } - - #[test] - fn test_summarize_tool_call_no_args() { - assert_eq!( - summarize_tool_call("developer__shell", None), - "developer: shell" - ); - } - - #[test] - fn test_summarize_tool_call_with_path() { - let args = serde_json::json!({"path": "/src/main.rs", "content": "fn main() {}"}); - assert_eq!( - summarize_tool_call("developer__edit", Some(&args)), - "developer: edit · /src/main.rs" - ); - } - - #[test] - fn test_summarize_tool_call_with_command() { - let args = serde_json::json!({"command": "cargo build"}); - assert_eq!( - summarize_tool_call("developer__shell", Some(&args)), - "developer: shell · cargo build" - ); - } - - #[test] - fn test_tool_call_identity_meta_uses_goose_extension_metadata() { - let request = ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("context7__query-docs")), - metadata: None, - tool_meta: Some(serde_json::json!({"goose_extension": "context7"})), - }; - - let meta = tool_call_identity_meta(&request).expect("expected metadata"); - - assert_eq!( - meta.get("goose"), - Some(&serde_json::json!({ - "toolCall": { - "toolName": "context7__query-docs", - "extensionName": "context7", - }, - })), - ); - } - - fn buf_entry(tool_id: &str, msg_id: &str) -> (String, String) { - (tool_id.to_string(), msg_id.to_string()) - } - - #[test] - fn extend_chain_membership_skips_singleton_and_leaves_buffer() { - let mut membership: HashMap> = HashMap::new(); - let buffer = vec![buf_entry("a", "row_1")]; - - extend_chain_membership(&buffer, &mut membership); - - assert_eq!(buffer.len(), 1, "buffer is left intact for caller"); - assert!( - membership.is_empty(), - "single-tool runs should not register a chain", - ); - } - - #[test] - fn extend_chain_membership_registers_each_id_against_shared_chain() { - let mut membership: HashMap> = HashMap::new(); - let buffer = vec![ - buf_entry("a", "row_first"), - buf_entry("b", "row_second"), - buf_entry("c", "row_third"), - ]; - - extend_chain_membership(&buffer, &mut membership); - - assert_eq!(membership.len(), 3); - let chain_a = membership.get("a").expect("a registered"); - let chain_b = membership.get("b").expect("b registered"); - let chain_c = membership.get("c").expect("c registered"); - assert!( - Arc::ptr_eq(chain_a, chain_b) && Arc::ptr_eq(chain_b, chain_c), - "every id in the run must point at the same ToolChain Arc", - ); - assert_eq!( - chain_a.ids, - vec!["a".to_string(), "b".to_string(), "c".to_string()], - ); - } - - #[test] - fn extend_chain_membership_anchors_on_first_row_for_split_messages() { - // Sequential tool use (Bedrock/Anthropic) emits each tool request as - // its own assistant message, with the tool response interleaved in - // between. The chain should still form, anchored on the *first* - // tool's row id so `update_tool_request_meta` can find that - // ToolRequest when persisting the summary. - let mut membership: HashMap> = HashMap::new(); - let buffer = vec![ - buf_entry("toolu_bdrk_1", "row_for_tool_1"), - buf_entry("toolu_bdrk_2", "row_for_tool_2"), - ]; - - extend_chain_membership(&buffer, &mut membership); - - let chain = membership - .get("toolu_bdrk_1") - .expect("first tool registered"); - assert_eq!( - chain.ids, - vec!["toolu_bdrk_1".to_string(), "toolu_bdrk_2".to_string()], - ); - let chain_via_second = membership - .get("toolu_bdrk_2") - .expect("second tool registered"); - assert!(Arc::ptr_eq(chain, chain_via_second)); - } - - #[test] - fn extend_chain_membership_grows_chain_as_more_requests_arrive() { - // The streaming loop re-registers eagerly each time a new request - // arrives, so a chain that started at length 2 must grow to include - // a third tool whose response is yet to come. Both the original - // members and the new member must point at the new (extended) chain. - let mut membership: HashMap> = HashMap::new(); - let mut buffer = vec![buf_entry("a", "row_1"), buf_entry("b", "row_2")]; - extend_chain_membership(&buffer, &mut membership); - - buffer.push(buf_entry("c", "row_3")); - extend_chain_membership(&buffer, &mut membership); - - let chain_a = membership.get("a").expect("a present"); - let chain_b = membership.get("b").expect("b present"); - let chain_c = membership.get("c").expect("c present"); - assert!(Arc::ptr_eq(chain_a, chain_b) && Arc::ptr_eq(chain_b, chain_c)); - assert_eq!( - chain_a.ids, - vec!["a".to_string(), "b".to_string(), "c".to_string()], - ); - } - - #[test] - fn with_tool_chain_summary_meta_creates_fresh_when_none() { - let meta = with_tool_chain_summary_meta(None, "applied dark mode", 4) - .expect("meta should be created"); - assert_eq!( - meta.get("goose"), - Some(&serde_json::json!({ - "toolChainSummary": { "summary": "applied dark mode", "count": 4 }, - })), - ); - } - - #[test] - fn with_tool_chain_summary_meta_preserves_existing_tool_call_identity() { - let existing = tool_call_identity_meta(&ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("developer__shell")), - metadata: None, - tool_meta: None, - }); - let meta = with_tool_chain_summary_meta(existing, "ran two commands", 2) - .expect("meta should be created"); - let goose = meta.get("goose").expect("goose key"); - assert_eq!( - goose.get("toolCall"), - Some( - &serde_json::json!({ "toolName": "developer__shell", "extensionName": "developer" }) - ) - ); - assert_eq!( - goose.get("toolChainSummary"), - Some(&serde_json::json!({ "summary": "ran two commands", "count": 2 })) - ); - } - #[test] fn replay_attaches_chain_summary_meta_for_first_tool_request_with_persisted_summary() { let tool_request = ToolRequest { @@ -3585,15 +2721,6 @@ print(\"hello, world\") ); } - #[test] - fn test_summarize_tool_call_long_value_truncated() { - let long_path = "a".repeat(80); - let args = serde_json::json!({"path": long_path}); - let result = summarize_tool_call("developer__read_file", Some(&args)); - assert!(result.ends_with('…')); - assert!(result.len() < 90); - } - #[test_case( RequestPermissionOutcome::Selected(SelectedPermissionOutcome::new(PermissionOptionId::from("allow_once".to_string()))), PermissionConfirmation { principal_type: PrincipalType::Tool, permission: Permission::AllowOnce }; @@ -3631,187 +2758,6 @@ print(\"hello, world\") assert_eq!(outcome_to_confirmation(&input), expected); } - fn json_object(pairs: Vec<(&str, serde_json::Value)>) -> rmcp::model::JsonObject { - pairs.into_iter().map(|(k, v)| (k.to_string(), v)).collect() - } - - #[test_case(None => None ; "none arguments")] - #[test_case(Some(json_object(vec![])) => None ; "missing line key")] - #[test_case(Some(json_object(vec![("line", serde_json::json!(5))])) => Some(5) ; "line present")] - #[test_case(Some(json_object(vec![("line", serde_json::json!("not_a_number"))])) => None ; "line not a number")] - fn test_get_requested_line(arguments: Option) -> Option { - get_requested_line(arguments.as_ref()) - } - - #[test_case("read", true ; "read is developer file tool")] - #[test_case("write", true ; "write is developer file tool")] - #[test_case("edit", true ; "edit is developer file tool")] - #[test_case("shell", false ; "shell is not developer file tool")] - #[test_case("analyze", false ; "analyze is not developer file tool")] - fn test_is_developer_file_tool(tool_name: &str, expected: bool) { - assert_eq!(is_developer_file_tool(tool_name), expected); - } - - #[test_case( - ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("read").with_arguments(serde_json::json!({"path": "/tmp/f.txt", "line": 5}).as_object().unwrap().clone())), - metadata: None, tool_meta: None, - }, - ToolResponse { - id: "req_1".to_string(), - tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), - metadata: None, - } - => vec![(PathBuf::from("/tmp/f.txt"), Some(5))] - ; "read returns requested line" - )] - #[test_case( - ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("read").with_arguments(serde_json::json!({"path": "/tmp/f.txt"}).as_object().unwrap().clone())), - metadata: None, tool_meta: None, - }, - ToolResponse { - id: "req_1".to_string(), - tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), - metadata: None, - } - => vec![(PathBuf::from("/tmp/f.txt"), None)] - ; "read without line" - )] - #[test_case( - ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("write").with_arguments(serde_json::json!({"path": "/tmp/f.txt", "content": "hi"}).as_object().unwrap().clone())), - metadata: None, tool_meta: None, - }, - ToolResponse { - id: "req_1".to_string(), - tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), - metadata: None, - } - => vec![(PathBuf::from("/tmp/f.txt"), Some(1))] - ; "write returns line 1" - )] - #[test_case( - ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("edit").with_arguments(serde_json::json!({"path": "/tmp/f.txt", "before": "a", "after": "b"}).as_object().unwrap().clone())), - metadata: None, tool_meta: None, - }, - ToolResponse { - id: "req_1".to_string(), - tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), - metadata: None, - } - => vec![(PathBuf::from("/tmp/f.txt"), Some(1))] - ; "edit returns line 1" - )] - #[test_case( - ToolRequest { - id: "req_1".to_string(), - tool_call: Ok(CallToolRequestParams::new("shell").with_arguments(serde_json::json!({"command": "ls"}).as_object().unwrap().clone())), - metadata: None, tool_meta: None, - }, - ToolResponse { - id: "req_1".to_string(), - tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), - metadata: None, - } - => Vec::<(PathBuf, Option)>::new() - ; "non file tool returns empty" - )] - fn test_extract_tool_locations( - request: ToolRequest, - response: ToolResponse, - ) -> Vec<(PathBuf, Option)> { - extract_tool_locations(&request, &response) - .into_iter() - .map(|loc| (loc.path, loc.line)) - .collect() - } - - fn response_with_meta(meta: Option) -> ToolResponse { - let mut result = CallToolResult::success(vec![RmcpContent::text("")]); - result.meta = meta.map(|v| serde_json::from_value(v).unwrap()); - ToolResponse { - id: "req_1".to_string(), - tool_result: Ok(result), - metadata: None, - } - } - - #[test_case( - response_with_meta(Some(serde_json::json!({"tool_locations": [{"path": "/tmp/f.txt", "line": 5}]}))) - => Some(vec![(PathBuf::from("/tmp/f.txt"), Some(5))]) - ; "meta with path and line" - )] - #[test_case( - response_with_meta(Some(serde_json::json!({"tool_locations": [{"path": "/tmp/f.txt"}]}))) - => Some(vec![(PathBuf::from("/tmp/f.txt"), None)]) - ; "meta with path no line" - )] - #[test_case( - response_with_meta(Some(serde_json::json!({}))) - => None - ; "meta without tool_locations key" - )] - #[test_case( - response_with_meta(None) - => None - ; "no meta" - )] - fn test_extract_locations_from_meta( - response: ToolResponse, - ) -> Option)>> { - extract_locations_from_meta(&response) - .map(|locs| locs.into_iter().map(|loc| (loc.path, loc.line)).collect()) - } - - #[test] - fn test_extract_tool_call_update_meta_ignores_untrusted_goose_meta() { - let response = response_with_meta(Some(serde_json::json!({ - "goose": { - "mcpApp": { - "resourceUri": "ui://spoofed/app", - }, - }, - }))); - - assert_eq!(extract_tool_call_update_meta(&response), None); - } - - #[test] - fn test_extract_tool_call_update_meta_uses_trusted_meta_only() { - let response = response_with_meta(Some(serde_json::json!({ - "goose": { - "mcpApp": { - "resourceUri": "ui://spoofed/app", - }, - }, - TRUSTED_TOOL_UPDATE_META_KEY: { - "mcpApp": { - "resourceUri": "ui://trusted/app", - "extensionName": "weather", - "toolName": "weather__render", - }, - }, - }))); - - let extracted = extract_tool_call_update_meta(&response).expect("expected trusted meta"); - assert_eq!( - extracted.get("goose"), - Some(&serde_json::json!({ - "mcpApp": { - "resourceUri": "ui://trusted/app", - "extensionName": "weather", - "toolName": "weather__render", - }, - })), - ); - } - #[test] fn test_merge_replay_message_meta_preserves_existing_goose_meta() { let message = Message::new(Role::Assistant, 1_700_000_000, vec![]).with_id("msg_1"); @@ -3950,31 +2896,6 @@ print(\"hello, world\") ); } - #[test] - fn test_extract_tool_raw_output_preserves_structured_content() { - let mut result = CallToolResult::success(vec![RmcpContent::text("fallback")]); - result.structured_content = Some(serde_json::json!({ - "restaurants": [ - { - "name": "Coffee Shop", - "unitToken": "unit-1", - }, - ], - })); - - assert_eq!( - extract_tool_raw_output(&Ok(result)), - Some(serde_json::json!({ - "restaurants": [ - { - "name": "Coffee Shop", - "unitToken": "unit-1", - }, - ], - })), - ); - } - fn make_session_with_usage(usage: TokenUsage, accumulated_usage: TokenUsage) -> Session { Session { id: "session-1".to_string(), diff --git a/crates/goose/src/acp/server/load_session.rs b/crates/goose/src/acp/server/load_session.rs index f2ae63a1ad0f..08364dc64752 100644 --- a/crates/goose/src/acp/server/load_session.rs +++ b/crates/goose/src/acp/server/load_session.rs @@ -1,3 +1,8 @@ +use super::tool_calls::conversion::{ + extract_tool_call_update_meta, pending_tool_call_from_request, + tool_call_update_fields_from_response, +}; +use super::tool_calls::enrichment::with_tool_chain_summary_meta; use super::*; fn replay_audience_annotations(audience: &[Role]) -> Annotations { @@ -33,6 +38,7 @@ fn replay_conversation_to_client( ) -> Result, agent_client_protocol::Error> { let session_id = SessionId::new(session.id.clone()); + let tool_call_notifier = ToolCallNotifier::new(cx, &session_id); let sid = sid_short(session_id.0.as_ref()); let messages = session @@ -90,44 +96,13 @@ fn replay_conversation_to_client( .tool_call .meta(merge_replay_message_meta(meta, message)); - cx.send_notification(SessionNotification::new( - session_id.clone(), - SessionUpdate::ToolCall(tool_call), - ))?; + tool_call_notifier.send_initial(tool_call)?; } MessageContent::ToolResponse(tool_response) => { - let status = match &tool_response.tool_result { - Ok(result) if result.is_error == Some(true) => ToolCallStatus::Failed, - Ok(_) => ToolCallStatus::Completed, - Err(_) => ToolCallStatus::Failed, - }; - - let mut fields = ToolCallUpdateFields::new().status(status); - if let Some(raw_output) = extract_tool_raw_output(&tool_response.tool_result) { - fields = fields.raw_output(raw_output); - } - if !tool_response - .tool_result - .as_ref() - .is_ok_and(|r| r.is_acp_aware()) - { - let content = build_tool_call_content(&tool_response.tool_result); - fields = fields.content(content); - - let locations = - extract_locations_from_meta(tool_response).unwrap_or_else(|| { - if let Some(tool_request) = - replay_tool_requests.get(&tool_response.id) - { - extract_tool_locations(tool_request, tool_response) - } else { - Vec::new() - } - }); - if !locations.is_empty() { - fields = fields.locations(locations); - } - } + let fields = tool_call_update_fields_from_response( + tool_response, + replay_tool_requests.get(&tool_response.id), + ); let update = ToolCallUpdate::new(ToolCallId::new(tool_response.id.clone()), fields) @@ -135,10 +110,7 @@ fn replay_conversation_to_client( extract_tool_call_update_meta(tool_response), message, )); - cx.send_notification(SessionNotification::new( - session_id.clone(), - SessionUpdate::ToolCallUpdate(update), - ))?; + tool_call_notifier.send_update(update)?; } MessageContent::Thinking(thinking) => { cx.send_notification(SessionNotification::new( diff --git a/crates/goose/src/acp/server/tool_calls/chain.rs b/crates/goose/src/acp/server/tool_calls/chain.rs new file mode 100644 index 000000000000..dd4ec370ed6e --- /dev/null +++ b/crates/goose/src/acp/server/tool_calls/chain.rs @@ -0,0 +1,148 @@ +use std::collections::HashMap; +use std::sync::Arc; + +/// A run of consecutive ToolRequest blocks within one assistant message, +/// tracked by `GooseAcpSession::chain_membership`. Used to drive a single +/// LLM summary for the whole run once every step has a recorded ToolResponse. +#[derive(Debug, Clone)] +pub(crate) struct ToolChain { + /// Tool call ids in document order. Always `len() >= 2`. + pub(crate) ids: Vec, + /// The message_id of the assistant message containing these tool calls. + /// Used to persist chain summaries back to the messages table. + pub(crate) message_id: String, +} + +/// If `buffer` holds a multi-tool run (≥ 2 tool requests), (re)register a +/// [`ToolChain`] in `chain_membership` anchored on the **first** tool's +/// message_id (the row `SessionManager::update_tool_request_meta` will patch +/// when persisting the LLM-generated summary). Does **not** clear the buffer +/// — chains can grow as more tools arrive (sequential tool use), so callers +/// keep accumulating and re-registering with the larger set of ids. +/// +/// The buffer contains `(tool_call_id, message_id)` pairs in arrival order, +/// fed by the prompt stream loop. Sequential tool use (Bedrock/Anthropic) +/// interleaves request → response → request → response across separate +/// `AgentEvent::Message` events, so a per-event view would only see length-1 +/// chains and miss the run. Tool responses are chain-neutral (they don't +/// split the run); only non-tool content (text, thinking, image, etc.) does, +/// matching the frontend's `groupContentSections` behavior. +pub(crate) fn extend_chain_membership( + buffer: &[(String, String)], + chain_membership: &mut HashMap>, +) { + if buffer.len() >= 2 { + let ids: Vec = buffer.iter().map(|(id, _)| id.clone()).collect(); + let message_id = buffer[0].1.clone(); + let chain = Arc::new(ToolChain { + ids: ids.clone(), + message_id, + }); + for id in ids { + chain_membership.insert(id, chain.clone()); + } + } +} + +#[cfg(test)] +mod tests { + mod extend_chain_membership { + use super::super::{extend_chain_membership, ToolChain}; + use std::collections::HashMap; + use std::sync::Arc; + + fn buf_entry(tool_id: &str, msg_id: &str) -> (String, String) { + (tool_id.to_string(), msg_id.to_string()) + } + + #[test] + fn skips_singleton_and_leaves_buffer() { + let mut membership: HashMap> = HashMap::new(); + let buffer = vec![buf_entry("a", "row_1")]; + + extend_chain_membership(&buffer, &mut membership); + + assert_eq!(buffer.len(), 1, "buffer is left intact for caller"); + assert!( + membership.is_empty(), + "single-tool runs should not register a chain", + ); + } + + #[test] + fn registers_each_id_against_shared_chain() { + let mut membership: HashMap> = HashMap::new(); + let buffer = vec![ + buf_entry("a", "row_first"), + buf_entry("b", "row_second"), + buf_entry("c", "row_third"), + ]; + + extend_chain_membership(&buffer, &mut membership); + + assert_eq!(membership.len(), 3); + let chain_a = membership.get("a").expect("a registered"); + let chain_b = membership.get("b").expect("b registered"); + let chain_c = membership.get("c").expect("c registered"); + assert!( + Arc::ptr_eq(chain_a, chain_b) && Arc::ptr_eq(chain_b, chain_c), + "every id in the run must point at the same ToolChain Arc", + ); + assert_eq!( + chain_a.ids, + vec!["a".to_string(), "b".to_string(), "c".to_string()], + ); + } + + #[test] + fn anchors_on_first_row_for_split_messages() { + // Sequential tool use (Bedrock/Anthropic) emits each tool request as + // its own assistant message, with the tool response interleaved in + // between. The chain should still form, anchored on the *first* + // tool's row id so `update_tool_request_meta` can find that + // ToolRequest when persisting the summary. + let mut membership: HashMap> = HashMap::new(); + let buffer = vec![ + buf_entry("toolu_bdrk_1", "row_for_tool_1"), + buf_entry("toolu_bdrk_2", "row_for_tool_2"), + ]; + + extend_chain_membership(&buffer, &mut membership); + + let chain = membership + .get("toolu_bdrk_1") + .expect("first tool registered"); + assert_eq!( + chain.ids, + vec!["toolu_bdrk_1".to_string(), "toolu_bdrk_2".to_string()], + ); + let chain_via_second = membership + .get("toolu_bdrk_2") + .expect("second tool registered"); + assert!(Arc::ptr_eq(chain, chain_via_second)); + } + + #[test] + fn grows_chain_as_more_requests_arrive() { + // The streaming loop re-registers eagerly each time a new request + // arrives, so a chain that started at length 2 must grow to include + // a third tool whose response is yet to come. Both the original + // members and the new member must point at the new (extended) chain. + let mut membership: HashMap> = HashMap::new(); + let mut buffer = vec![buf_entry("a", "row_1"), buf_entry("b", "row_2")]; + extend_chain_membership(&buffer, &mut membership); + + buffer.push(buf_entry("c", "row_3")); + extend_chain_membership(&buffer, &mut membership); + + let chain_a = membership.get("a").expect("a present"); + let chain_b = membership.get("b").expect("b present"); + let chain_c = membership.get("c").expect("c present"); + assert!(Arc::ptr_eq(chain_a, chain_b) && Arc::ptr_eq(chain_b, chain_c)); + assert_eq!( + chain_a.ids, + vec!["a".to_string(), "b".to_string(), "c".to_string()], + ); + } + } +} diff --git a/crates/goose/src/acp/server/tool_calls/conversion.rs b/crates/goose/src/acp/server/tool_calls/conversion.rs new file mode 100644 index 000000000000..afbdd602e416 --- /dev/null +++ b/crates/goose/src/acp/server/tool_calls/conversion.rs @@ -0,0 +1,799 @@ +use crate::acp::tools::AcpAwareToolMeta; +use crate::agents::extension_manager::TRUSTED_TOOL_UPDATE_META_KEY; +use crate::conversation::message::{ToolRequest, ToolResponse}; +use crate::mcp_utils::ToolResult; +use agent_client_protocol::schema::v1::{ + BlobResourceContents, Content, ContentBlock, EmbeddedResource, EmbeddedResourceResource, + ImageContent, Meta, TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, + ToolCallLocation, ToolCallStatus, ToolCallUpdateFields, +}; +use rmcp::model::{CallToolResult, RawContent, ResourceContents}; + +pub(crate) struct PendingToolCall { + pub(crate) tool_call: ToolCall, + pub(crate) identity_meta: Option, + pub(crate) fallback_title: String, +} + +pub(crate) fn format_tool_name(tool_name: &str) -> String { + if let Some((extension, tool)) = tool_name.split_once("__") { + format!( + "{}: {}", + extension.replace('_', " "), + tool.replace('_', " ") + ) + } else { + tool_name.replace('_', " ") + } +} + +/// Build a short fallback title from the tool name and arguments by extracting +/// the most useful value (file path, command, query, url, etc.). +fn summarize_tool_call(tool_name: &str, arguments: Option<&serde_json::Value>) -> String { + let base = format_tool_name(tool_name); + + let detail = arguments.and_then(|args| { + let obj = args.as_object()?; + let keys = [ + "path", "file", "command", "query", "url", "uri", "name", "pattern", "source", + ]; + for key in &keys { + if let Some(v) = obj.get(*key) { + let s = match v { + serde_json::Value::String(s) => s.clone(), + other => other.to_string(), + }; + if !s.is_empty() { + let first_line = s.lines().next().unwrap_or(&s); + if first_line.len() > 60 { + return Some(format!("{}…", crate::utils::safe_truncate(first_line, 57))); + } + return Some(first_line.to_string()); + } + } + } + None + }); + + match detail { + Some(d) => format!("{base} · {d}"), + None => base, + } +} + +pub(crate) fn tool_call_identity_meta(tool_request: &ToolRequest) -> Option { + let tool_call = tool_request.tool_call.as_ref().ok()?; + let tool_name = tool_call.name.to_string(); + let extension_name = tool_request + .tool_meta + .as_ref() + .and_then(|meta| meta.get("goose_extension")) + .and_then(serde_json::Value::as_str) + .map(ToString::to_string) + .or_else(|| { + tool_name + .split_once("__") + .map(|(extension_name, _)| extension_name.to_string()) + }); + + let mut tool_call_meta = serde_json::Map::new(); + tool_call_meta.insert("toolName".to_string(), serde_json::Value::String(tool_name)); + if let Some(extension_name) = extension_name { + tool_call_meta.insert( + "extensionName".to_string(), + serde_json::Value::String(extension_name), + ); + } + + let mut goose_meta = serde_json::Map::new(); + goose_meta.insert( + "toolCall".to_string(), + serde_json::Value::Object(tool_call_meta), + ); + + let mut meta = serde_json::Map::new(); + meta.insert("goose".to_string(), serde_json::Value::Object(goose_meta)); + Some(meta) +} + +pub(crate) fn pending_tool_call_from_request(tool_request: &ToolRequest) -> PendingToolCall { + let tool_name = match &tool_request.tool_call { + Ok(tool_call) => tool_call.name.to_string(), + Err(_) => "error".to_string(), + }; + let args_value = tool_request + .tool_call + .as_ref() + .ok() + .and_then(|tc| tc.arguments.as_ref()) + .map(|a| serde_json::Value::Object(a.clone())); + let fallback_title = summarize_tool_call(&tool_name, args_value.as_ref()); + let identity_meta = tool_call_identity_meta(tool_request); + + // Prefer the persisted LLM-generated title when available so replay (and + // any subsequent live initial ToolCall after the title task has already + // resolved) emits the nice title up front, with no flash of the + // deterministic fallback. + let initial_title = tool_request + .persisted_title() + .map(|s| s.to_string()) + .unwrap_or_else(|| fallback_title.clone()); + + let mut tool_call = ToolCall::new(ToolCallId::new(tool_request.id.clone()), initial_title) + .status(ToolCallStatus::Pending); + if let Some(args) = args_value { + tool_call = tool_call.raw_input(args); + } + + PendingToolCall { + tool_call, + identity_meta, + fallback_title, + } +} + +fn get_requested_line(arguments: Option<&rmcp::model::JsonObject>) -> Option { + arguments + .and_then(|args| args.get("line")) + .and_then(|v| v.as_u64()) + .map(|l| l as u32) +} + +fn is_developer_file_tool(tool_name: &str) -> bool { + matches!(tool_name, "read" | "write" | "edit") +} + +fn extract_locations_from_meta(tool_response: &ToolResponse) -> Option> { + let result = tool_response.tool_result.as_ref().ok()?; + let meta = result.meta.as_ref()?; + let locations_val = meta.get("tool_locations")?; + let entries: Vec = serde_json::from_value(locations_val.clone()).ok()?; + let locations = entries + .into_iter() + .filter_map(|entry| { + let path = entry.get("path")?.as_str()?; + let line = entry.get("line").and_then(|v| v.as_u64()).map(|l| l as u32); + Some(ToolCallLocation::new(path).line(line)) + }) + .collect::>(); + if locations.is_empty() { + None + } else { + Some(locations) + } +} + +fn extract_tool_locations( + tool_request: &ToolRequest, + tool_response: &ToolResponse, +) -> Vec { + let mut locations = Vec::new(); + + if let Ok(tool_call) = &tool_request.tool_call { + if !is_developer_file_tool(tool_call.name.as_ref()) { + return locations; + } + + let tool_name = tool_call.name.as_ref(); + let path_str = tool_call + .arguments + .as_ref() + .and_then(|args| args.get("path")) + .and_then(|p| p.as_str()); + + if let Some(path_str) = path_str { + if matches!(tool_name, "read") { + let line = get_requested_line(tool_call.arguments.as_ref()); + locations.push(ToolCallLocation::new(path_str).line(line)); + return locations; + } + + if matches!(tool_name, "write" | "edit") { + locations.push(ToolCallLocation::new(path_str).line(1)); + return locations; + } + + let command = tool_call + .arguments + .as_ref() + .and_then(|args| args.get("command")) + .and_then(|c| c.as_str()); + + if let Ok(result) = &tool_response.tool_result { + for content in &result.content { + if let RawContent::Text(text_content) = &content.raw { + let text = &text_content.text; + + match command { + Some("view") => { + let line = extract_view_line_range(text) + .map(|range| range.0 as u32) + .or(Some(1)); + locations.push(ToolCallLocation::new(path_str).line(line)); + } + Some("str_replace") | Some("insert") => { + let line = extract_first_line_number(text) + .map(|l| l as u32) + .or(Some(1)); + locations.push(ToolCallLocation::new(path_str).line(line)); + } + Some("write") => { + locations.push(ToolCallLocation::new(path_str).line(1)); + } + _ => { + locations.push(ToolCallLocation::new(path_str).line(1)); + } + } + break; + } + } + } + + if locations.is_empty() { + locations.push(ToolCallLocation::new(path_str).line(1)); + } + } + } + + locations +} + +fn extract_view_line_range(text: &str) -> Option<(usize, usize)> { + 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()?; + let end = if caps.get(2)?.as_str() == "end" { + start + } else { + caps.get(2)?.as_str().parse::().ok()? + }; + return Some((start, end)); + } + None +} + +fn extract_first_line_number(text: &str) -> Option { + 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(); + } + None +} + +pub(crate) fn extract_tool_call_update_meta(tool_response: &ToolResponse) -> Option { + let tool_result = tool_response.tool_result.as_ref().ok()?; + let goose_meta = tool_result + .meta + .as_ref()? + .0 + .get(TRUSTED_TOOL_UPDATE_META_KEY)? + .clone(); + let mut meta_map = serde_json::Map::new(); + meta_map.insert("goose".to_string(), goose_meta); + Some(meta_map) +} + +fn build_tool_call_content(tool_result: &ToolResult) -> Vec { + match tool_result { + Ok(result) => result + .content + .iter() + .filter_map(|content| match &content.raw { + RawContent::Text(val) => Some(ToolCallContent::Content(Content::new( + ContentBlock::Text(TextContent::new(val.text.clone())), + ))), + RawContent::Image(val) => Some(ToolCallContent::Content(Content::new( + ContentBlock::Image(ImageContent::new(val.data.clone(), val.mime_type.clone())), + ))), + RawContent::Resource(val) => { + let resource = match &val.resource { + ResourceContents::TextResourceContents { + mime_type, + text, + uri, + .. + } => EmbeddedResourceResource::TextResourceContents( + TextResourceContents::new(text.clone(), uri.clone()) + .mime_type(mime_type.clone()), + ), + ResourceContents::BlobResourceContents { + mime_type, + blob, + uri, + .. + } => EmbeddedResourceResource::BlobResourceContents( + BlobResourceContents::new(blob.clone(), uri.clone()) + .mime_type(mime_type.clone()), + ), + }; + Some(ToolCallContent::Content(Content::new( + ContentBlock::Resource(EmbeddedResource::new(resource)), + ))) + } + RawContent::Audio(_) | RawContent::ResourceLink(_) => None, + }) + .collect(), + Err(error) => vec![ToolCallContent::Content(Content::new(ContentBlock::Text( + TextContent::new(error.message.to_string()), + )))], + } +} + +fn extract_tool_raw_output(tool_result: &ToolResult) -> Option { + tool_result + .as_ref() + .ok() + .and_then(|result| result.structured_content.clone()) +} + +pub(crate) fn tool_call_update_fields_from_response( + tool_response: &ToolResponse, + tool_request: Option<&ToolRequest>, +) -> ToolCallUpdateFields { + let is_failed = match &tool_response.tool_result { + Ok(result) => result.is_error == Some(true), + Err(_) => true, + }; + let status = if is_failed { + ToolCallStatus::Failed + } else { + ToolCallStatus::Completed + }; + + let mut fields = ToolCallUpdateFields::new().status(status); + if let Some(raw_output) = extract_tool_raw_output(&tool_response.tool_result) { + fields = fields.raw_output(raw_output); + } + let is_acp_aware = tool_response + .tool_result + .as_ref() + .is_ok_and(|result| result.is_acp_aware()); + + if is_failed || !is_acp_aware { + fields = fields.content(build_tool_call_content(&tool_response.tool_result)); + } + + if !is_acp_aware { + let locations = extract_locations_from_meta(tool_response).unwrap_or_else(|| { + tool_request + .map(|request| extract_tool_locations(request, tool_response)) + .unwrap_or_default() + }); + if !locations.is_empty() { + fields = fields.locations(locations); + } + } + + fields +} + +#[cfg(test)] +mod tests { + use super::*; + use rmcp::model::{CallToolRequestParams, Content as RmcpContent}; + use std::path::PathBuf; + use test_case::test_case; + + mod format_tool_name { + use super::*; + + #[test] + fn with_extension() { + assert_eq!(format_tool_name("developer__edit"), "developer: edit"); + assert_eq!( + format_tool_name("platform__manage_extensions"), + "platform: manage extensions" + ); + assert_eq!(format_tool_name("todo__write"), "todo: write"); + } + + #[test] + fn without_extension() { + assert_eq!(format_tool_name("simple_tool"), "simple tool"); + assert_eq!(format_tool_name("another_name"), "another name"); + assert_eq!(format_tool_name("single"), "single"); + } + } + + mod summarize_tool_call { + use super::*; + + #[test] + fn no_args() { + assert_eq!( + summarize_tool_call("developer__shell", None), + "developer: shell" + ); + } + + #[test] + fn with_path() { + let args = serde_json::json!({"path": "/src/main.rs", "content": "fn main() {}"}); + assert_eq!( + summarize_tool_call("developer__edit", Some(&args)), + "developer: edit · /src/main.rs" + ); + } + + #[test] + fn with_command() { + let args = serde_json::json!({"command": "cargo build"}); + assert_eq!( + summarize_tool_call("developer__shell", Some(&args)), + "developer: shell · cargo build" + ); + } + + #[test] + fn long_value_is_truncated() { + let long_path = "a".repeat(80); + let args = serde_json::json!({"path": long_path}); + let result = summarize_tool_call("developer__read_file", Some(&args)); + assert!(result.ends_with('…')); + assert!(result.len() < 90); + } + } + + #[test] + fn test_tool_call_identity_meta_uses_goose_extension_metadata() { + let request = ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("context7__query-docs")), + metadata: None, + tool_meta: Some(serde_json::json!({"goose_extension": "context7"})), + }; + + let meta = tool_call_identity_meta(&request).expect("expected metadata"); + + assert_eq!( + meta.get("goose"), + Some(&serde_json::json!({ + "toolCall": { + "toolName": "context7__query-docs", + "extensionName": "context7", + }, + })), + ); + } + + fn json_object(pairs: Vec<(&str, serde_json::Value)>) -> rmcp::model::JsonObject { + pairs.into_iter().map(|(k, v)| (k.to_string(), v)).collect() + } + + #[test_case(None => None ; "none arguments")] + #[test_case(Some(json_object(vec![])) => None ; "missing line key")] + #[test_case(Some(json_object(vec![("line", serde_json::json!(5))])) => Some(5) ; "line present")] + #[test_case(Some(json_object(vec![("line", serde_json::json!("not_a_number"))])) => None ; "line not a number")] + fn test_get_requested_line(arguments: Option) -> Option { + get_requested_line(arguments.as_ref()) + } + + #[test_case("read", true ; "read is developer file tool")] + #[test_case("write", true ; "write is developer file tool")] + #[test_case("edit", true ; "edit is developer file tool")] + #[test_case("shell", false ; "shell is not developer file tool")] + #[test_case("analyze", false ; "analyze is not developer file tool")] + fn test_is_developer_file_tool(tool_name: &str, expected: bool) { + assert_eq!(is_developer_file_tool(tool_name), expected); + } + + #[test_case( + ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("read").with_arguments(serde_json::json!({"path": "/tmp/f.txt", "line": 5}).as_object().unwrap().clone())), + metadata: None, tool_meta: None, + }, + ToolResponse { + id: "req_1".to_string(), + tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), + metadata: None, + } + => vec![(PathBuf::from("/tmp/f.txt"), Some(5))] + ; "read returns requested line" + )] + #[test_case( + ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("read").with_arguments(serde_json::json!({"path": "/tmp/f.txt"}).as_object().unwrap().clone())), + metadata: None, tool_meta: None, + }, + ToolResponse { + id: "req_1".to_string(), + tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), + metadata: None, + } + => vec![(PathBuf::from("/tmp/f.txt"), None)] + ; "read without line" + )] + #[test_case( + ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("write").with_arguments(serde_json::json!({"path": "/tmp/f.txt", "content": "hi"}).as_object().unwrap().clone())), + metadata: None, tool_meta: None, + }, + ToolResponse { + id: "req_1".to_string(), + tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), + metadata: None, + } + => vec![(PathBuf::from("/tmp/f.txt"), Some(1))] + ; "write returns line 1" + )] + #[test_case( + ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("edit").with_arguments(serde_json::json!({"path": "/tmp/f.txt", "before": "a", "after": "b"}).as_object().unwrap().clone())), + metadata: None, tool_meta: None, + }, + ToolResponse { + id: "req_1".to_string(), + tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), + metadata: None, + } + => vec![(PathBuf::from("/tmp/f.txt"), Some(1))] + ; "edit returns line 1" + )] + #[test_case( + ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("shell").with_arguments(serde_json::json!({"command": "ls"}).as_object().unwrap().clone())), + metadata: None, tool_meta: None, + }, + ToolResponse { + id: "req_1".to_string(), + tool_result: Ok(CallToolResult::success(vec![RmcpContent::text("")])), + metadata: None, + } + => Vec::<(PathBuf, Option)>::new() + ; "non file tool returns empty" + )] + fn test_extract_tool_locations( + request: ToolRequest, + response: ToolResponse, + ) -> Vec<(PathBuf, Option)> { + extract_tool_locations(&request, &response) + .into_iter() + .map(|loc| (loc.path, loc.line)) + .collect() + } + + fn response_with_meta(meta: Option) -> ToolResponse { + let mut result = CallToolResult::success(vec![RmcpContent::text("")]); + result.meta = meta.map(|v| serde_json::from_value(v).unwrap()); + ToolResponse { + id: "req_1".to_string(), + tool_result: Ok(result), + metadata: None, + } + } + + #[test_case( + response_with_meta(Some(serde_json::json!({"tool_locations": [{"path": "/tmp/f.txt", "line": 5}]}))) + => Some(vec![(PathBuf::from("/tmp/f.txt"), Some(5))]) + ; "meta with path and line" + )] + #[test_case( + response_with_meta(Some(serde_json::json!({"tool_locations": [{"path": "/tmp/f.txt"}]}))) + => Some(vec![(PathBuf::from("/tmp/f.txt"), None)]) + ; "meta with path no line" + )] + #[test_case( + response_with_meta(Some(serde_json::json!({}))) + => None + ; "meta without tool_locations key" + )] + #[test_case( + response_with_meta(None) + => None + ; "no meta" + )] + fn test_extract_locations_from_meta( + response: ToolResponse, + ) -> Option)>> { + extract_locations_from_meta(&response) + .map(|locs| locs.into_iter().map(|loc| (loc.path, loc.line)).collect()) + } + + mod extract_tool_call_update_meta { + use super::*; + + #[test] + fn ignores_untrusted_goose_meta() { + let response = response_with_meta(Some(serde_json::json!({ + "goose": { + "mcpApp": { + "resourceUri": "ui://spoofed/app", + }, + }, + }))); + + assert_eq!(extract_tool_call_update_meta(&response), None); + } + + #[test] + fn uses_trusted_meta_only() { + let response = response_with_meta(Some(serde_json::json!({ + "goose": { + "mcpApp": { + "resourceUri": "ui://spoofed/app", + }, + }, + TRUSTED_TOOL_UPDATE_META_KEY: { + "mcpApp": { + "resourceUri": "ui://trusted/app", + "extensionName": "weather", + "toolName": "weather__render", + }, + }, + }))); + + let extracted = + extract_tool_call_update_meta(&response).expect("expected trusted meta"); + assert_eq!( + extracted.get("goose"), + Some(&serde_json::json!({ + "mcpApp": { + "resourceUri": "ui://trusted/app", + "extensionName": "weather", + "toolName": "weather__render", + }, + })), + ); + } + } + + #[test] + fn test_extract_tool_raw_output_preserves_structured_content() { + let mut result = CallToolResult::success(vec![RmcpContent::text("fallback")]); + result.structured_content = Some(serde_json::json!({ + "restaurants": [ + { + "name": "Coffee Shop", + "unitToken": "unit-1", + }, + ], + })); + + assert_eq!( + extract_tool_raw_output(&Ok(result)), + Some(serde_json::json!({ + "restaurants": [ + { + "name": "Coffee Shop", + "unitToken": "unit-1", + }, + ], + })), + ); + } + + fn response_from_tool_result(tool_result: ToolResult) -> ToolResponse { + ToolResponse { + id: "req_1".to_string(), + tool_result, + metadata: None, + } + } + + fn write_request(path: &str) -> ToolRequest { + ToolRequest { + id: "req_1".to_string(), + tool_call: Ok( + CallToolRequestParams::new("write").with_arguments(json_object(vec![ + ("path", serde_json::json!(path)), + ("content", serde_json::json!("updated")), + ])), + ), + metadata: None, + tool_meta: None, + } + } + + fn first_tool_call_text(fields: &ToolCallUpdateFields) -> Option<&str> { + fields.content.as_ref()?.iter().find_map(|content| { + let ToolCallContent::Content(content) = content else { + return None; + }; + let ContentBlock::Text(text) = &content.content else { + return None; + }; + Some(text.text.as_str()) + }) + } + + mod tool_call_update_fields_from_response { + use super::*; + + #[test] + fn includes_ordinary_success_details() { + let raw_output = serde_json::json!({ "changed": true }); + let mut result = CallToolResult::success(vec![RmcpContent::text("write completed")]); + result.structured_content = Some(raw_output.clone()); + let response = response_from_tool_result(Ok(result)); + let request = write_request("/tmp/request.txt"); + + let fields = tool_call_update_fields_from_response(&response, Some(&request)); + + assert_eq!(fields.status, Some(ToolCallStatus::Completed)); + assert_eq!(fields.raw_output, Some(raw_output)); + assert_eq!(first_tool_call_text(&fields), Some("write completed")); + let locations = fields.locations.as_deref().expect("expected location"); + assert_eq!(locations.len(), 1); + assert_eq!(locations[0].path, PathBuf::from("/tmp/request.txt")); + assert_eq!(locations[0].line, Some(1)); + } + + #[test] + fn includes_ordinary_error_content() { + let response = + response_from_tool_result(Ok(CallToolResult::error(vec![RmcpContent::text( + "write failed", + )]))); + + let fields = tool_call_update_fields_from_response(&response, None); + + assert_eq!(fields.status, Some(ToolCallStatus::Failed)); + assert_eq!(first_tool_call_text(&fields), Some("write failed")); + assert!(fields.locations.is_none()); + } + + #[test] + fn suppresses_acp_aware_success_details() { + let raw_output = serde_json::json!({ "changed": true }); + let mut result = CallToolResult::success(vec![RmcpContent::text("write completed")]); + result.structured_content = Some(raw_output.clone()); + let response = response_from_tool_result(Ok(result.with_acp_aware_meta())); + let request = write_request("/tmp/request.txt"); + + let fields = tool_call_update_fields_from_response(&response, Some(&request)); + + assert_eq!(fields.status, Some(ToolCallStatus::Completed)); + assert_eq!(fields.raw_output, Some(raw_output)); + assert!(fields.content.is_none()); + assert!(fields.locations.is_none()); + } + + #[test] + fn prefers_explicit_location() { + let response = response_with_meta(Some(serde_json::json!({ + "tool_locations": [{ "path": "/tmp/response.txt", "line": 7 }] + }))); + let request = write_request("/tmp/request.txt"); + + let fields = tool_call_update_fields_from_response(&response, Some(&request)); + + let locations = fields.locations.as_deref().expect("expected location"); + assert_eq!(locations.len(), 1); + assert_eq!(locations[0].path, PathBuf::from("/tmp/response.txt")); + assert_eq!(locations[0].line, Some(7)); + } + + #[test] + fn includes_acp_aware_error_content() { + let result = CallToolResult::error(vec![RmcpContent::text("write failed")]) + .with_acp_aware_meta(); + let response = response_from_tool_result(Ok(result)); + let request = write_request("/tmp/request.txt"); + + let fields = tool_call_update_fields_from_response(&response, Some(&request)); + + assert_eq!(fields.status, Some(ToolCallStatus::Failed)); + assert_eq!(first_tool_call_text(&fields), Some("write failed")); + assert!(fields.locations.is_none()); + } + + #[test] + fn includes_transport_error_content() { + let response = response_from_tool_result(Err(rmcp::model::ErrorData::new( + rmcp::model::ErrorCode::INTERNAL_ERROR, + "transport failed", + None, + ))); + + let fields = tool_call_update_fields_from_response(&response, None); + + assert_eq!(fields.status, Some(ToolCallStatus::Failed)); + assert_eq!(first_tool_call_text(&fields), Some("transport failed")); + } + } +} diff --git a/crates/goose/src/acp/server/tool_calls/enrichment.rs b/crates/goose/src/acp/server/tool_calls/enrichment.rs new file mode 100644 index 000000000000..6fa360637d8d --- /dev/null +++ b/crates/goose/src/acp/server/tool_calls/enrichment.rs @@ -0,0 +1,513 @@ +use crate::acp::tool_call_notifier::ToolCallNotifier; +use crate::agents::Agent; +use crate::conversation::message::{ + Message, MessageContent, TOOL_META_CHAIN_SUMMARY_KEY, TOOL_META_TITLE_KEY, +}; +use crate::model_config::get_fast_model; +use crate::session::SessionManager; +use crate::session_context::with_session_id; +use crate::utils::safe_truncate; +use agent_client_protocol::schema::v1::{ + Meta, SessionId, ToolCallId, ToolCallUpdate, ToolCallUpdateFields, +}; +use rmcp::model::CallToolRequestParams; +use serde_json::{json, to_string, Map, Number, Value}; +use std::slice::from_ref; +use std::sync::Arc; +use std::time::Duration; +use tokio::{spawn, time::sleep}; +use tracing::warn; + +/// Add `goose.toolChainSummary = { summary, count }` to a `Meta` blob, +/// preserving any existing `goose.*` keys such as `goose.toolCall`. +pub(crate) fn with_tool_chain_summary_meta( + base: Option, + summary: &str, + count: usize, +) -> Option { + let mut meta = base.unwrap_or_default(); + let goose_entry = meta + .entry("goose".to_string()) + .or_insert_with(|| Value::Object(Map::new())); + let goose_obj = match goose_entry { + Value::Object(obj) => obj, + other => { + *other = Value::Object(Map::new()); + match other { + Value::Object(obj) => obj, + _ => unreachable!(), + } + } + }; + let mut chain = Map::new(); + chain.insert("summary".to_string(), Value::String(summary.to_string())); + chain.insert("count".to_string(), Value::Number(Number::from(count))); + goose_obj.insert("toolChainSummary".to_string(), Value::Object(chain)); + Some(meta) +} + +pub(crate) struct ToolTitleEnrichmentContext { + agent: Arc, + session_id: SessionId, + tool_call_notifier: ToolCallNotifier, + session_manager: Arc, + session_id_for_persist: String, + message_id_for_persist: Option, +} + +impl ToolTitleEnrichmentContext { + pub(crate) fn new( + agent: &Arc, + session_id: &SessionId, + tool_call_notifier: &ToolCallNotifier, + session_manager: &Arc, + session_id_for_persist: &str, + message_id_for_persist: Option<&str>, + ) -> Self { + Self { + agent: agent.clone(), + session_id: session_id.clone(), + tool_call_notifier: tool_call_notifier.clone(), + session_manager: session_manager.clone(), + session_id_for_persist: session_id_for_persist.to_string(), + message_id_for_persist: message_id_for_persist.map(str::to_string), + } + } + + pub(crate) fn spawn_title_enrichment( + self, + request_id: String, + tool_call: &CallToolRequestParams, + identity_meta: Option, + fallback_title: String, + ) { + let args_json = tool_call + .arguments + .as_ref() + .map(|a| { + let s = to_string(a).unwrap_or_default(); + if s.len() > 300 { + format!("{}…", safe_truncate(&s, 300)) + } else { + s + } + }) + .unwrap_or_default(); + + let Self { + agent, + session_id, + tool_call_notifier, + session_manager, + session_id_for_persist, + message_id_for_persist, + } = self; + + ToolTitleEnrichmentJob { + agent, + sid: session_id, + request_id, + tool_call_notifier, + name: tool_call.name.to_string(), + identity_meta, + fallback_title, + session_id_for_persist, + message_id_for_persist, + session_manager, + args_json, + } + .spawn(); + } +} + +struct ToolTitleEnrichmentJob { + agent: Arc, + sid: SessionId, + request_id: String, + tool_call_notifier: ToolCallNotifier, + name: String, + identity_meta: Option, + fallback_title: String, + session_id_for_persist: String, + message_id_for_persist: Option, + session_manager: Arc, + args_json: String, +} + +impl ToolTitleEnrichmentJob { + fn spawn(self) { + spawn(async move { + let Self { + agent, + sid, + request_id, + tool_call_notifier, + name, + identity_meta, + fallback_title, + session_id_for_persist, + message_id_for_persist, + session_manager, + args_json, + } = self; + + let (title, from_llm) = match agent.provider().await { + Ok(provider) => { + if provider.manages_own_context() { + return; + } + + let system = + "Summarize this tool call in a short lowercase phrase (3-8 words). \ + No punctuation. No quotes. Examples: reading project configuration, \ + checking network connectivity, listing files in src directory"; + let user_text = format!("Tool: {name}\nArguments: {args_json}"); + let message = Message::user().with_text(&user_text); + let model_config = match agent.model_config_for_session(&sid.0).await { + Ok(config) => config, + Err(_) => return, + }; + let fast_model_config = + match get_fast_model(provider.get_name(), &model_config).await { + Ok(config) => config, + Err(_) => return, + }; + // The fast model occasionally returns an empty response + // under load (rate limiting, transient network). One + // retry with a short backoff is enough to recover the + // common cases without paying for the regular model. + let mut llm_outcome: Option = None; + for attempt in 0..2 { + match with_session_id( + Some(sid.0.to_string()), + provider.complete(&fast_model_config, system, from_ref(&message), &[]), + ) + .await + { + Ok((response, _)) => { + let summary: String = response + .content + .iter() + .filter_map(|c: &MessageContent| c.as_text()) + .collect::() + .trim() + .to_string(); + if !summary.is_empty() { + llm_outcome = Some(summary); + break; + } + if attempt == 0 { + warn!( + "tool call summary: fast_complete returned empty for {request_id} ({name}), retrying once", + ); + sleep(Duration::from_millis(150)).await; + } + } + Err(e) => { + if attempt == 0 { + warn!( + "tool call summary: fast_complete errored for {request_id} ({name}): {e}, retrying once", + ); + sleep(Duration::from_millis(150)).await; + } else { + warn!( + "tool call summary: fast_complete errored for {request_id} ({name}) after retry: {e}", + ); + } + } + } + } + match llm_outcome { + Some(summary) => (summary, true), + None => { + warn!( + "tool call summary: falling back to deterministic title for {request_id} ({name}) — replay will not show an LLM summary for this call", + ); + (fallback_title.clone(), false) + } + } + } + Err(e) => { + warn!("tool call summary: failed to get provider: {e}"); + (fallback_title.clone(), false) + } + }; + + let fields = ToolCallUpdateFields::new().title(title.clone()); + let _ = tool_call_notifier.send_update( + ToolCallUpdate::new(ToolCallId::new(request_id.clone()), fields) + .meta(identity_meta), + ); + + // Best-effort persistence: only persist the LLM-generated title + // (not the deterministic fallback) so reload uses fallback_title + // for older or failed cases just like today. + if from_llm { + if let Some(msg_id) = message_id_for_persist { + let patch = json!({ + (TOOL_META_TITLE_KEY): title, + }); + if let Err(e) = session_manager + .update_tool_request_meta( + &session_id_for_persist, + &msg_id, + &request_id, + patch, + ) + .await + { + warn!( + "tool call summary: persist failed for {request_id} in {msg_id}: {e}", + ); + } + } else { + warn!( + "tool call summary: missing message_id for {request_id} — title will not survive reload", + ); + } + } + }); + } +} + +pub(crate) struct ChainSummaryEnrichmentContext { + agent: Arc, + session_id: SessionId, + tool_call_notifier: ToolCallNotifier, + session_manager: Arc, +} + +impl ChainSummaryEnrichmentContext { + pub(crate) fn new( + agent: &Arc, + session_id: &SessionId, + tool_call_notifier: &ToolCallNotifier, + session_manager: &Arc, + ) -> Self { + Self { + agent: agent.clone(), + session_id: session_id.clone(), + tool_call_notifier: tool_call_notifier.clone(), + session_manager: session_manager.clone(), + } + } + + pub(crate) fn spawn_chain_summary( + self, + first_tool_call_id: String, + message_id_for_persist: String, + steps: Vec<(String, String)>, + identity_meta: Option, + chain_count: usize, + ) { + let Self { + agent, + session_id, + tool_call_notifier, + session_manager, + } = self; + + ChainSummaryEnrichmentJob { + agent, + sid: session_id, + first_tool_call_id, + message_id_for_persist, + steps, + identity_meta, + chain_count, + tool_call_notifier, + session_manager, + } + .spawn(); + } +} + +struct ChainSummaryEnrichmentJob { + agent: Arc, + sid: SessionId, + first_tool_call_id: String, + message_id_for_persist: String, + steps: Vec<(String, String)>, + identity_meta: Option, + chain_count: usize, + tool_call_notifier: ToolCallNotifier, + session_manager: Arc, +} + +impl ChainSummaryEnrichmentJob { + fn spawn(self) { + spawn(async move { + let Self { + agent, + sid, + first_tool_call_id, + message_id_for_persist, + steps, + identity_meta, + chain_count, + tool_call_notifier, + session_manager, + } = self; + + let provider = match agent.provider().await { + Ok(provider) => provider, + Err(error) => { + warn!( + "tool chain summary: failed to get provider for chain anchored at {first_tool_call_id}: {error}", + ); + return; + } + }; + if provider.manages_own_context() { + warn!( + "tool chain summary: provider manages own context; skipping chain anchored at {first_tool_call_id}", + ); + return; + } + + let system = "Summarize this sequence of tool calls in a short lowercase phrase \ + (3-8 words). No punctuation. No quotes. \ + Examples: applied dark mode polish, scanned for security issues, \ + refactored config loading"; + + let mut user_text = String::from("Tool call sequence:\n"); + for (index, (name, args)) in steps.iter().enumerate() { + user_text.push_str(&format!("Step {}: {} {}\n", index + 1, name, args)); + } + let message = Message::user().with_text(&user_text); + let model_config = match agent.model_config_for_session(&sid.0).await { + Ok(config) => config, + Err(_) => return, + }; + let fast_model_config = match get_fast_model(provider.get_name(), &model_config).await { + Ok(config) => config, + Err(_) => return, + }; + + // Match the per-tool retry policy: one retry on empty/error keeps + // the chain header reliable when the fast model is rate-limited or + // momentarily flaky, without escalating to the regular model. + let mut summary: Option = None; + for attempt in 0..2 { + match with_session_id( + Some(sid.0.to_string()), + provider.complete(&fast_model_config, system, from_ref(&message), &[]), + ) + .await + { + Ok((response, _)) => { + let generated_summary = response + .content + .iter() + .filter_map(|content: &MessageContent| content.as_text()) + .collect::() + .trim() + .to_string(); + if !generated_summary.is_empty() { + summary = Some(generated_summary); + break; + } + if attempt == 0 { + warn!( + "tool chain summary: fast_complete returned empty for chain anchored at {first_tool_call_id} ({} steps), retrying once", + steps.len(), + ); + sleep(Duration::from_millis(150)).await; + } + } + Err(error) => { + if attempt == 0 { + warn!( + "tool chain summary: fast_complete errored for chain anchored at {first_tool_call_id}: {error}, retrying once", + ); + sleep(Duration::from_millis(150)).await; + } else { + warn!( + "tool chain summary: fast_complete errored for chain anchored at {first_tool_call_id} after retry: {error}", + ); + } + } + } + } + let Some(summary) = summary else { + warn!( + "tool chain summary: no LLM summary produced for chain anchored at {first_tool_call_id} — replay will fall back to the deterministic phrase", + ); + return; + }; + + let patch = json!({ + (TOOL_META_CHAIN_SUMMARY_KEY): { + "summary": &summary, + "count": chain_count, + }, + }); + if let Err(error) = session_manager + .update_tool_request_meta( + &sid.0, + &message_id_for_persist, + &first_tool_call_id, + patch, + ) + .await + { + warn!( + "tool chain summary: persist failed for chain anchored at {first_tool_call_id} in {message_id_for_persist}: {error}", + ); + } + + let meta = with_tool_chain_summary_meta(identity_meta, &summary, chain_count); + let fields = ToolCallUpdateFields::new(); + let _ = tool_call_notifier.send_update( + ToolCallUpdate::new(ToolCallId::new(first_tool_call_id), fields).meta(meta), + ); + }); + } +} + +#[cfg(test)] +mod tests { + mod with_tool_chain_summary_meta { + use super::super::with_tool_chain_summary_meta; + use crate::acp::server::tool_calls::conversion::tool_call_identity_meta; + use crate::conversation::message::ToolRequest; + use rmcp::model::CallToolRequestParams; + use serde_json::json; + + #[test] + fn creates_fresh_when_none() { + let meta = with_tool_chain_summary_meta(None, "applied dark mode", 4) + .expect("meta should be created"); + assert_eq!( + meta.get("goose"), + Some(&json!({ + "toolChainSummary": { "summary": "applied dark mode", "count": 4 }, + })), + ); + } + + #[test] + fn preserves_existing_tool_call_identity() { + let existing = tool_call_identity_meta(&ToolRequest { + id: "req_1".to_string(), + tool_call: Ok(CallToolRequestParams::new("developer__shell")), + metadata: None, + tool_meta: None, + }); + let meta = with_tool_chain_summary_meta(existing, "ran two commands", 2) + .expect("meta should be created"); + let goose = meta.get("goose").expect("goose key"); + assert_eq!( + goose.get("toolCall"), + Some(&json!({ + "toolName": "developer__shell", + "extensionName": "developer", + })), + ); + assert_eq!( + goose.get("toolChainSummary"), + Some(&json!({ "summary": "ran two commands", "count": 2 })), + ); + } + } +} diff --git a/crates/goose/src/acp/server/tool_calls/mod.rs b/crates/goose/src/acp/server/tool_calls/mod.rs new file mode 100644 index 000000000000..b077de45f249 --- /dev/null +++ b/crates/goose/src/acp/server/tool_calls/mod.rs @@ -0,0 +1,3 @@ +pub(super) mod chain; +pub(super) mod conversion; +pub(super) mod enrichment; diff --git a/crates/goose/src/acp/server/tool_notifications.rs b/crates/goose/src/acp/server/tool_notifications.rs index 0ee404a9296f..4358ceb6f5b8 100644 --- a/crates/goose/src/acp/server/tool_notifications.rs +++ b/crates/goose/src/acp/server/tool_notifications.rs @@ -1,5 +1,5 @@ use agent_client_protocol::schema::v1::{ - Meta, SessionUpdate, ToolCallId, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, + Meta, ToolCallId, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, }; use rmcp::model::{LoggingMessageNotificationParam, ProgressNotificationParam, ServerNotification}; use serde::Serialize; @@ -21,7 +21,7 @@ enum ToolNotification { pub(super) fn tool_notification_update( tool_call_id: impl Into, notification: ServerNotification, -) -> Option { +) -> Option { let tool_notification = match notification { ServerNotification::LoggingMessageNotification(notification) => ToolNotification::Message { params: notification.params, @@ -45,18 +45,19 @@ pub(super) fn tool_notification_update( serde_json::to_value(tool_notification).ok()?, ); - Some(SessionUpdate::ToolCallUpdate( + Some( ToolCallUpdate::new( tool_call_id, ToolCallUpdateFields::new().status(ToolCallStatus::InProgress), ) .meta(meta), - )) + ) } #[cfg(test)] mod tests { use super::tool_notification_update; + use agent_client_protocol::schema::v1::SessionUpdate; use rmcp::model::{ CancelledNotificationParam, CustomNotification, LoggingLevel, LoggingMessageNotificationParam, Notification, NumberOrString, ProgressNotificationParam, @@ -82,7 +83,8 @@ mod tests { )); let update = tool_notification_update("tool_1", notification).expect("expected update"); - let value = serde_json::to_value(update).expect("update should serialize"); + let value = serde_json::to_value(SessionUpdate::ToolCallUpdate(update)) + .expect("update should serialize"); assert_eq!(value["sessionUpdate"], "tool_call_update"); assert_eq!(value["toolCallId"], "tool_1"); @@ -114,7 +116,8 @@ mod tests { )); let update = tool_notification_update("tool_1", notification).expect("expected update"); - let value = serde_json::to_value(update).expect("update should serialize"); + let value = serde_json::to_value(SessionUpdate::ToolCallUpdate(update)) + .expect("update should serialize"); assert_eq!(value["sessionUpdate"], "tool_call_update"); assert_eq!(value["toolCallId"], "tool_1"); @@ -159,7 +162,8 @@ mod tests { )); let update = tool_notification_update("tool_1", notification).expect("expected update"); - let value = serde_json::to_value(update).expect("update should serialize"); + let value = serde_json::to_value(SessionUpdate::ToolCallUpdate(update)) + .expect("update should serialize"); assert_eq!(value["sessionUpdate"], "tool_call_update"); assert_eq!(value["toolCallId"], "tool_1"); diff --git a/crates/goose/src/acp/tool_call_notifier.rs b/crates/goose/src/acp/tool_call_notifier.rs new file mode 100644 index 000000000000..081f3594a9e0 --- /dev/null +++ b/crates/goose/src/acp/tool_call_notifier.rs @@ -0,0 +1,39 @@ +use agent_client_protocol::schema::v1::{ + SessionId, SessionNotification, SessionUpdate, ToolCall, ToolCallUpdate, +}; +use agent_client_protocol::{Client, ConnectionTo}; + +#[derive(Clone)] +pub(crate) struct ToolCallNotifier { + connection: ConnectionTo, + session_id: SessionId, +} + +impl ToolCallNotifier { + pub(crate) fn new(connection: &ConnectionTo, session_id: &SessionId) -> Self { + Self { + connection: connection.clone(), + session_id: session_id.clone(), + } + } + + pub(crate) fn send_initial( + &self, + tool_call: ToolCall, + ) -> Result<(), agent_client_protocol::Error> { + self.connection.send_notification(SessionNotification::new( + self.session_id.clone(), + SessionUpdate::ToolCall(tool_call), + )) + } + + pub(crate) fn send_update( + &self, + update: ToolCallUpdate, + ) -> Result<(), agent_client_protocol::Error> { + self.connection.send_notification(SessionNotification::new( + self.session_id.clone(), + SessionUpdate::ToolCallUpdate(update), + )) + } +} diff --git a/crates/goose/tests/acp_common_tests/mod.rs b/crates/goose/tests/acp_common_tests/mod.rs index 60fc47a461d8..059a1f90e926 100644 --- a/crates/goose/tests/acp_common_tests/mod.rs +++ b/crates/goose/tests/acp_common_tests/mod.rs @@ -415,8 +415,22 @@ pub async fn run_fs_write_text_file_true() { .await .unwrap(); assert!(!output.text.is_empty()); + + let updates = session.session_updates(); + let initial_tool_call_id = updates + .iter() + .find_map(|update| match update { + SessionUpdate::ToolCall(tool_call) => Some(&tool_call.tool_call_id), + _ => None, + }) + .expect("expected an initial tool call"); + for update in &updates { + if let SessionUpdate::ToolCallUpdate(update) = update { + assert_eq!(&update.tool_call_id, initial_tool_call_id); + } + } assert_notifications( - &session.notifications(), + &fixtures::to_notifications(&updates), &[ Notification::ToolCall, Notification::ToolCallKind(ToolKind::Edit),