diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 239f2f27..5d055de8 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -57,7 +57,7 @@ use sacp::{ Responder, }; use serde::Deserialize; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::panic::AssertUnwindSafe; use std::sync::Arc; use strum::{EnumMessage, VariantNames}; @@ -156,12 +156,36 @@ struct GooseAcpSession { agent: AgentHandle, internal_session_id: String, tool_requests: HashMap, + /// For each tool_call_id that belongs to a multi-tool chain (run of + /// consecutive ToolRequest blocks within one assistant message), the chain + /// it belongs to. Populated when the assistant message is processed. + /// Used by `handle_tool_response` to detect when a chain has fully + /// completed and fire a single LLM summary covering the run. + chain_membership: HashMap>, + /// Set of tool_call_ids whose ToolResponse has already been processed. + /// Drives the "all responses present" check for chain completion. + responded_tool_ids: HashSet, + /// Tool_call_ids of chains that have already had a summary task fired. + /// Idempotence guard so we summarize each chain at most once. + summarized_chains: HashSet, cancel_token: Option, /// Working directory set while the agent was still loading. /// Applied once the agent becomes ready. pending_working_dir: Option, } +/// 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 { + /// Assistant message id where every tool request in this chain lives. + /// This is also the row we patch when persisting the chain summary. + message_id: String, + /// Tool call ids in document order. Always `len() >= 2`. + ids: Vec, +} + /// Progress stages signalled by the background agent setup task via the watch /// channel. `ProviderReady` fires as soon as the provider (and goose-mode) /// are initialized — before extensions finish loading. `FullyReady` fires @@ -617,12 +641,122 @@ fn tool_call_identity_meta(tool_request: &ToolRequest) -> Option { 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, } +/// Extract chains (runs of consecutive `MessageContent::ToolRequest` blocks) +/// from a single message's content. Mirrors the frontend's chain detection in +/// `MessageBubble.groupContentSections`: any non-tool block (text, thinking, +/// image, etc.) breaks the run. +/// +/// Returns one inner Vec per detected chain, holding the tool_call_ids in +/// document order. Single-tool runs are included; callers (chain +/// summarization) gate on `chain.len() >= 2`. +/// +/// Note: this is the per-message view, kept around for tests and potential +/// replay use. The live runtime path uses a streaming buffer fed by +/// [`register_chain_buffer`] so chains that span multiple `AgentEvent::Message` +/// events (e.g. Bedrock-style streaming, where one LLM message is split across +/// rows — see `f087fa63c`) are still detected. +#[allow(dead_code)] +fn extract_tool_chains( + content: &[crate::conversation::message::MessageContent], +) -> Vec> { + use crate::conversation::message::MessageContent; + let mut chains: Vec> = Vec::new(); + let mut current: Vec = Vec::new(); + + for block in content { + match block { + MessageContent::ToolRequest(tr) => current.push(tr.id.clone()), + MessageContent::ToolResponse(_) => { + // Server-side, assistant messages don't carry responses; + // responses arrive in subsequent messages. Treat as + // chain-neutral so a stray response doesn't split a chain + // if the data shape ever changes. + } + _ => { + if !current.is_empty() { + chains.push(std::mem::take(&mut current)); + } + } + } + } + if !current.is_empty() { + chains.push(current); + } + chains +} + +/// 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 [`ThreadManager::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 per-event `extract_tool_chains` only +/// sees length-1 chains and would 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 anchor_message_id = buffer[0].1.clone(); + let ids: Vec = buffer.iter().map(|(id, _)| id.clone()).collect(); + let chain = Arc::new(ToolChain { + message_id: anchor_message_id, + ids: ids.clone(), + }); + 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(), @@ -637,11 +771,17 @@ fn pending_tool_call_from_request(tool_request: &ToolRequest) -> PendingToolCall let fallback_title = summarize_tool_call(&tool_name, args_value.as_ref()); let identity_meta = tool_call_identity_meta(tool_request); - let mut tool_call = ToolCall::new( - ToolCallId::new(tool_request.id.clone()), - fallback_title.clone(), - ) - .status(ToolCallStatus::Pending); + // 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); } @@ -1434,10 +1574,13 @@ impl GooseAcpAgent { message } + #[allow(clippy::too_many_arguments)] async fn handle_message_content( &self, content_item: &MessageContent, session_id: &SessionId, + thread_id: &str, + message_id: Option<&str>, agent: &Arc, session: &mut GooseAcpSession, cx: &ConnectionTo, @@ -1452,12 +1595,26 @@ impl GooseAcpAgent { ))?; } MessageContent::ToolRequest(tool_request) => { - self.handle_tool_request(tool_request, session_id, session, cx) - .await?; + self.handle_tool_request( + tool_request, + session_id, + thread_id, + message_id, + session, + cx, + ) + .await?; } MessageContent::ToolResponse(tool_response) => { - self.handle_tool_response(tool_response, session_id, session, cx) - .await?; + self.handle_tool_response( + tool_response, + session_id, + thread_id, + message_id, + session, + cx, + ) + .await?; } MessageContent::Thinking(thinking) => { cx.send_notification(SessionNotification::new( @@ -1495,6 +1652,8 @@ impl GooseAcpAgent { &self, tool_request: &crate::conversation::message::ToolRequest, session_id: &SessionId, + thread_id: &str, + message_id: Option<&str>, session: &mut GooseAcpSession, cx: &ConnectionTo, ) -> Result<(), sacp::Error> { @@ -1535,8 +1694,12 @@ impl GooseAcpAgent { }) .unwrap_or_default(); + let thread_id_for_persist = thread_id.to_string(); + let message_id_for_persist = message_id.map(|s| s.to_string()); + let thread_manager = self.thread_manager.clone(); + tokio::spawn(async move { - let title = match agent.provider().await { + let (title, from_llm) = match agent.provider().await { Ok(provider) => { if provider.manages_own_context() { return; @@ -1548,44 +1711,105 @@ impl GooseAcpAgent { 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); - match provider - .complete_fast(&sid.0, system, &[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() { - fallback_title.clone() - } else { - summary + // 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 provider + .complete_fast(&sid.0, 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}", + ); + } } } - Err(e) => { - warn!("tool call summary: fast_complete failed: {e}"); - fallback_title.clone() + } + 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() + (fallback_title.clone(), false) } }; - let fields = ToolCallUpdateFields::new().title(title); + let fields = ToolCallUpdateFields::new().title(title.clone()); let _ = cx.send_notification(SessionNotification::new( sid, SessionUpdate::ToolCallUpdate( - ToolCallUpdate::new(ToolCallId::new(request_id), fields) + 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. Surface persist + // errors at warn level so the "occasional bad replay" symptom is + // diagnosable from logs alone. + 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) = thread_manager + .update_tool_request_meta( + &thread_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", + ); + } + } }); } @@ -1596,6 +1820,8 @@ impl GooseAcpAgent { &self, tool_response: &crate::conversation::message::ToolResponse, session_id: &SessionId, + thread_id: &str, + message_id: Option<&str>, session: &mut GooseAcpSession, cx: &ConnectionTo, ) -> Result<(), sacp::Error> { @@ -1636,9 +1862,224 @@ impl GooseAcpAgent { SessionUpdate::ToolCallUpdate(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, thread_id, session, cx); + let _ = message_id; + Ok(()) } + /// If `tool_call_id` belongs to a multi-tool chain and every step in that + /// chain has now had its response processed, spawn a single LLM + /// summarization task that persists the chain summary on the first tool + /// request and notifies the client. Idempotent — fires at most once per + /// chain. + fn maybe_summarize_chain( + &self, + tool_call_id: &str, + session_id: &SessionId, + thread_id: &str, + session: &mut GooseAcpSession, + cx: &ConnectionTo, + ) { + let Some(chain) = session.chain_membership.get(tool_call_id).cloned() else { + warn!( + "tool chain summary: skipped — no chain registered for tool_call_id {tool_call_id}", + ); + return; + }; + if !chain + .ids + .iter() + .all(|id| session.responded_tool_ids.contains(id)) + { + let total = chain.ids.len(); + let responded = chain + .ids + .iter() + .filter(|id| session.responded_tool_ids.contains(*id)) + .count(); + let missing: Vec<&String> = chain + .ids + .iter() + .filter(|id| !session.responded_tool_ids.contains(*id)) + .collect(); + warn!( + "tool chain summary: waiting on {pending}/{total} responses for chain anchored at {anchor:?} (missing: {missing:?})", + pending = total - responded, + anchor = chain.ids.first(), + ); + return; + } + let Some(first_id) = chain.ids.first() else { + warn!("tool chain summary: skipped — empty chain.ids for tool_call_id {tool_call_id}"); + return; + }; + if !session.summarized_chains.insert(first_id.clone()) { + debug!("tool chain summary: chain anchored at {first_id} already summarized; skipping"); + return; + } + + let agent = match &session.agent { + AgentHandle::Ready(a) => a.clone(), + AgentHandle::Loading(_) => { + warn!( + "tool chain summary: agent still loading; skipping chain anchored at {first_id}", + ); + return; + } + }; + + // Snapshot (name, args_json) for each step in document order. + let steps: Vec<(String, String)> = chain + .ids + .iter() + .filter_map(|id| { + let req = session.tool_requests.get(id)?; + let tool_call = req.tool_call.as_ref().ok()?; + let name = tool_call.name.to_string(); + let args = tool_call + .arguments + .as_ref() + .map(|a| serde_json::to_string(a).unwrap_or_default()) + .unwrap_or_default(); + let args = if args.len() > 200 { + format!("{}…", crate::utils::safe_truncate(&args, 200)) + } else { + args + }; + Some((name, args)) + }) + .collect(); + if steps.len() < 2 { + return; + } + + let identity_meta = session + .tool_requests + .get(first_id) + .and_then(tool_call_identity_meta); + + let sid = session_id.clone(); + let thread_id_for_persist = thread_id.to_string(); + let chain_for_task = chain.clone(); + let cx = cx.clone(); + let thread_manager = self.thread_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); + + // 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 provider + .complete_fast(&sid.0, 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) = thread_manager + .update_tool_request_meta( + &thread_id_for_persist, + &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), + ), + )); + }); + } + #[allow(clippy::too_many_arguments)] fn handle_tool_permission_request( &self, @@ -1935,6 +2376,9 @@ impl GooseAcpAgent { agent: AgentHandle::Loading(agent_rx), internal_session_id: internal_session_id.clone(), tool_requests: HashMap::new(), + chain_membership: HashMap::new(), + responded_tool_ids: HashSet::new(), + summarized_chains: HashSet::new(), cancel_token: None, pending_working_dir: None, }; @@ -2250,9 +2694,21 @@ impl GooseAcpAgent { replay_tool_requests.insert(tool_request.id.clone(), tool_request.clone()); let pending_tool_call = pending_tool_call_from_request(tool_request); - let tool_call = pending_tool_call.tool_call.meta( - merge_replay_message_meta(pending_tool_call.identity_meta, message), - ); + let mut meta = pending_tool_call.identity_meta; + // If this tool request is the first of a chain whose + // summary was persisted at completion time, attach the + // chain summary to the initial ToolCall so the chain + // header is correct on first paint after reload. + if let Some(chain_summary) = tool_request.persisted_chain_summary() { + meta = with_tool_chain_summary_meta( + meta, + &chain_summary.summary, + chain_summary.count, + ); + } + let tool_call = pending_tool_call + .tool_call + .meta(merge_replay_message_meta(meta, message)); cx.send_notification(SessionNotification::new( args.session_id.clone(), @@ -2345,6 +2801,9 @@ impl GooseAcpAgent { agent: AgentHandle::Loading(agent_rx), internal_session_id: internal_session_id.clone(), tool_requests: replay_tool_requests, + chain_membership: HashMap::new(), + responded_tool_ids: HashSet::new(), + summarized_chains: HashSet::new(), cancel_token: None, pending_working_dir: None, }; @@ -2459,6 +2918,17 @@ impl GooseAcpAgent { let mut was_cancelled = false; let mut first_event_logged = false; let mut event_count: u32 = 0; + // Streaming chain buffer: tracks consecutive tool requests across + // `AgentEvent::Message` events so chains that span multiple rows are + // still registered. Sequential tool use (Bedrock/Anthropic) yields + // request → response → request → response across separate + // assistant/user messages, so tool responses are chain-neutral; only + // non-tool content (text, thinking, image, etc.) breaks the run. + // Holds `(tool_call_id, message_id_of_owning_row)` in arrival order; + // re-registered eagerly each time a request arrives so + // `handle_tool_response` finds the chain when subsequent responses + // are processed. + let mut chain_buffer: Vec<(String, String)> = Vec::new(); while let Some(event) = stream.next().await { if cancel_token.is_cancelled() { @@ -2478,10 +2948,12 @@ impl GooseAcpAgent { match event { Ok(crate::agents::AgentEvent::Message(message)) => { - self.thread_manager + let stored_message = self + .thread_manager .append_message(&thread_id, Some(&internal_session_id), &message) .await .internal_err_ctx("Failed to persist message")?; + let stored_message_id = stored_message.id.clone(); let mut sessions = self.sessions.lock().await; let session = sessions.get_mut(&thread_id).ok_or_else(|| { @@ -2489,10 +2961,39 @@ impl GooseAcpAgent { .data(format!("Session not found: {}", thread_id)) })?; - for content_item in &message.content { + for content_item in &stored_message.content { + match content_item { + MessageContent::ToolRequest(tr) => { + if let Some(msg_id) = stored_message_id.as_deref() { + chain_buffer.push((tr.id.clone(), msg_id.to_string())); + // Re-register eagerly so the chain is in + // place by the time the matching + // `tool_response` triggers + // `maybe_summarize_chain` (sequential + // tool use interleaves request/response + // events). + extend_chain_membership( + &chain_buffer, + &mut session.chain_membership, + ); + } + } + MessageContent::ToolResponse(_) => { + // Chain-neutral: a response between two + // requests doesn't break the run, matching + // the frontend's `groupContentSections`. + } + _ => { + // Text, thinking, image, etc. end the run. + chain_buffer.clear(); + } + } + self.handle_message_content( content_item, &args.session_id, + &thread_id, + stored_message_id.as_deref(), &agent, session, cx, @@ -2511,6 +3012,11 @@ impl GooseAcpAgent { { let mut sessions = self.sessions.lock().await; if let Some(session) = sessions.get_mut(&thread_id) { + // Final safety net: in case the stream ended without any + // chain-breaking content, make sure a multi-tool buffer is + // registered. (Eager registration during the loop usually + // covers this.) + extend_chain_membership(&chain_buffer, &mut session.chain_membership); session.cancel_token = None; } } @@ -2849,6 +3355,9 @@ impl GooseAcpAgent { agent: AgentHandle::Loading(agent_rx), internal_session_id: internal_session_id.clone(), tool_requests: HashMap::new(), + chain_membership: HashMap::new(), + responded_tool_ids: HashSet::new(), + summarized_chains: HashSet::new(), cancel_token: None, pending_working_dir: None, }; @@ -3117,6 +3626,277 @@ print(\"hello, world\") ); } + fn tool_request_block(id: &str) -> crate::conversation::message::MessageContent { + crate::conversation::message::MessageContent::ToolRequest(ToolRequest { + id: id.to_string(), + tool_call: Ok(CallToolRequestParams::new("dummy")), + metadata: None, + tool_meta: None, + }) + } + + fn text_block(text: &str) -> crate::conversation::message::MessageContent { + crate::conversation::message::MessageContent::text(text) + } + + #[test] + fn extract_tool_chains_returns_empty_for_no_tool_blocks() { + let content = vec![text_block("hello"), text_block("world")]; + assert!(extract_tool_chains(&content).is_empty()); + } + + #[test] + fn extract_tool_chains_returns_single_chain_when_only_tools() { + let content = vec![ + tool_request_block("a"), + tool_request_block("b"), + tool_request_block("c"), + ]; + let chains = extract_tool_chains(&content); + assert_eq!( + chains, + vec![vec!["a".to_string(), "b".to_string(), "c".to_string()]] + ); + } + + #[test] + fn extract_tool_chains_breaks_on_text_block() { + let content = vec![ + tool_request_block("a"), + tool_request_block("b"), + text_block("interlude"), + tool_request_block("c"), + tool_request_block("d"), + ]; + let chains = extract_tool_chains(&content); + assert_eq!( + chains, + vec![ + vec!["a".to_string(), "b".to_string()], + vec!["c".to_string(), "d".to_string()], + ] + ); + } + + #[test] + fn extract_tool_chains_includes_singletons() { + let content = vec![ + tool_request_block("a"), + text_block("split"), + tool_request_block("b"), + text_block("split"), + tool_request_block("c"), + ]; + let chains = extract_tool_chains(&content); + assert_eq!( + chains, + vec![ + vec!["a".to_string()], + vec!["b".to_string()], + vec!["c".to_string()], + ] + ); + } + + #[test] + fn extract_tool_chains_keeps_run_when_text_leads_or_trails() { + let content = vec![ + text_block("intro"), + tool_request_block("a"), + tool_request_block("b"), + text_block("outro"), + ]; + let chains = extract_tool_chains(&content); + assert_eq!(chains, vec![vec!["a".to_string(), "b".to_string()]]); + } + + 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.message_id, "row_first"); + 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.message_id, "row_for_tool_1"); + 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.message_id, "row_1"); + 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 { + id: "req_first".to_string(), + tool_call: Ok(CallToolRequestParams::new("developer__shell")), + metadata: None, + tool_meta: Some(serde_json::json!({ + crate::conversation::message::TOOL_META_CHAIN_SUMMARY_KEY: { + "summary": "applied dark mode polish", + "count": 3, + }, + })), + }; + + let pending_tool_call = pending_tool_call_from_request(&tool_request); + let mut meta = pending_tool_call.identity_meta; + let chain_summary = tool_request + .persisted_chain_summary() + .expect("chain summary should be present"); + meta = with_tool_chain_summary_meta(meta, &chain_summary.summary, chain_summary.count); + + let goose = meta + .as_ref() + .and_then(|m| m.get("goose")) + .expect("replay meta must include a goose namespace"); + assert_eq!( + goose.get("toolCall"), + Some( + &serde_json::json!({ "toolName": "developer__shell", "extensionName": "developer" }) + ), + "replay must preserve identity meta alongside the chain summary", + ); + assert_eq!( + goose.get("toolChainSummary"), + Some(&serde_json::json!({ "summary": "applied dark mode polish", "count": 3 })), + "replay must attach toolChainSummary so the chain header renders on first paint", + ); + } + + #[test] + fn replay_does_not_attach_chain_summary_for_tool_requests_without_persisted_summary() { + let tool_request = ToolRequest { + id: "req_second".to_string(), + tool_call: Ok(CallToolRequestParams::new("developer__shell")), + metadata: None, + tool_meta: None, + }; + + let chain_summary = tool_request.persisted_chain_summary(); + assert!( + chain_summary.is_none(), + "non-first tool requests must not carry chain summaries", + ); + } + #[test] fn test_summarize_tool_call_long_value_truncated() { let long_path = "a".repeat(80); diff --git a/crates/goose/src/conversation/message.rs b/crates/goose/src/conversation/message.rs index 3490ca9c..9f86d3af 100644 --- a/crates/goose/src/conversation/message.rs +++ b/crates/goose/src/conversation/message.rs @@ -116,12 +116,60 @@ impl ToolRequest { .and_then(|v| v.as_bool()) .unwrap_or(false) } + + /// Returns the persisted LLM-generated title for this tool call, if any. + /// Set asynchronously by [`crate::acp::server`] after `provider.complete_fast` + /// resolves; survives session reload via SQLite. Falls back to `None` for + /// older sessions that predate persistence — callers should use a deterministic + /// title in that case. + pub fn persisted_title(&self) -> Option<&str> { + self.tool_meta + .as_ref() + .and_then(|v| v.get(TOOL_META_TITLE_KEY)) + .and_then(|v| v.as_str()) + } + + /// Returns the persisted per-chain summary anchored on this tool request, + /// if any. Only the FIRST tool request in a chain (a run of consecutive + /// tool blocks within one assistant message) carries this. See + /// [`crate::acp::server`] for how chains are detected and summarized. + pub fn persisted_chain_summary(&self) -> Option { + let obj = self + .tool_meta + .as_ref() + .and_then(|v| v.get(TOOL_META_CHAIN_SUMMARY_KEY))?; + let summary = obj.get("summary").and_then(|v| v.as_str())?.to_string(); + let count = obj.get("count").and_then(|v| v.as_u64())?; + if count == 0 { + return None; + } + Some(PersistedChainSummary { + summary, + count: count as usize, + }) + } +} + +/// A chain summary persisted on the first tool request of a chain. +#[derive(Debug, Clone, PartialEq)] +pub struct PersistedChainSummary { + pub summary: String, + pub count: usize, } /// Marker key under `ToolRequest.tool_meta` indicating the tool was already /// executed externally; the agent loop must skip redispatch. pub const TOOL_META_EXTERNAL_DISPATCH_KEY: &str = "goose.external_dispatch"; +/// Key under `ToolRequest.tool_meta` storing the LLM-generated short title +/// for this tool call. Used to make the title survive session reload. +pub const TOOL_META_TITLE_KEY: &str = "goose.toolSummary.title"; + +/// Key under `ToolRequest.tool_meta` storing the LLM-generated chain summary +/// for the chain that starts at this tool request. Shape: `{ "summary": String, +/// "count": u64 }`. Only attached to the FIRST tool request in a chain. +pub const TOOL_META_CHAIN_SUMMARY_KEY: &str = "goose.toolChain.summary"; + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] #[derive(ToSchema)] @@ -1635,4 +1683,78 @@ mod tests { } } } + + fn make_tool_request(meta: Option) -> super::ToolRequest { + super::ToolRequest { + id: "id-1".to_string(), + tool_call: Ok(CallToolRequestParams::new("test_tool")), + metadata: None, + tool_meta: meta, + } + } + + #[test] + fn persisted_title_returns_none_when_meta_missing() { + let req = make_tool_request(None); + assert_eq!(req.persisted_title(), None); + } + + #[test] + fn persisted_title_returns_value_when_present() { + let meta = serde_json::json!({ + super::TOOL_META_TITLE_KEY: "reading project configuration", + }); + let req = make_tool_request(Some(meta)); + assert_eq!(req.persisted_title(), Some("reading project configuration")); + } + + #[test] + fn persisted_title_returns_none_for_non_string_value() { + let meta = serde_json::json!({ super::TOOL_META_TITLE_KEY: 42 }); + let req = make_tool_request(Some(meta)); + assert_eq!(req.persisted_title(), None); + } + + #[test] + fn persisted_title_does_not_collide_with_external_dispatch() { + let meta = serde_json::json!({ + super::TOOL_META_EXTERNAL_DISPATCH_KEY: true, + super::TOOL_META_TITLE_KEY: "running commands", + }); + let req = make_tool_request(Some(meta)); + assert!(req.is_externally_dispatched()); + assert_eq!(req.persisted_title(), Some("running commands")); + } + + #[test] + fn persisted_chain_summary_round_trips() { + let meta = serde_json::json!({ + super::TOOL_META_CHAIN_SUMMARY_KEY: { + "summary": "applied dark mode polish", + "count": 4, + }, + }); + let req = make_tool_request(Some(meta)); + let summary = req.persisted_chain_summary().expect("summary present"); + assert_eq!(summary.summary, "applied dark mode polish"); + assert_eq!(summary.count, 4); + } + + #[test] + fn persisted_chain_summary_returns_none_for_missing_or_zero_count() { + let req = make_tool_request(None); + assert!(req.persisted_chain_summary().is_none()); + + let meta_zero = serde_json::json!({ + super::TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "x", "count": 0 }, + }); + let req_zero = make_tool_request(Some(meta_zero)); + assert!(req_zero.persisted_chain_summary().is_none()); + + let meta_no_summary = serde_json::json!({ + super::TOOL_META_CHAIN_SUMMARY_KEY: { "count": 3 }, + }); + let req_no_summary = make_tool_request(Some(meta_no_summary)); + assert!(req_no_summary.persisted_chain_summary().is_none()); + } } diff --git a/crates/goose/src/session/thread_manager.rs b/crates/goose/src/session/thread_manager.rs index 08638188..20106037 100644 --- a/crates/goose/src/session/thread_manager.rs +++ b/crates/goose/src/session/thread_manager.rs @@ -1,5 +1,5 @@ use super::session_manager::{role_to_string, SessionStorage}; -use crate::conversation::message::Message; +use crate::conversation::message::{Message, MessageContent}; use anyhow::Result; use chrono::{DateTime, Utc}; use rmcp::model::Role; @@ -378,6 +378,73 @@ impl ThreadManager { self.get_thread(&new_id).await } + /// Merge a JSON object patch into the `tool_meta` of the `ToolRequest` whose + /// `id == tool_call_id` inside the message identified by `(thread_id, + /// message_id)`. Existing keys in `tool_meta` are preserved. + /// + /// No-ops (returns `Ok(())`) if the row containing the tool request can't + /// be found — callers (e.g. async title tasks) treat persistence as + /// best-effort. + /// + /// `message_id` is used as a coarse filter, but multiple `thread_messages` + /// rows can share the same `message_id` when the agent splits a single + /// LLM response (e.g. text + tool_request) into separate + /// `AgentEvent::Message` events. We disambiguate by walking the matching + /// rows and picking the one whose content actually contains a + /// `ToolRequest` with `tool_call_id`, then update only that row by its + /// auto-incremented primary key. Without this, the title for the first + /// tool in such a split message never persists, because `fetch_optional` + /// returns the text-only row first and finds no matching tool call. + pub async fn update_tool_request_meta( + &self, + thread_id: &str, + message_id: &str, + tool_call_id: &str, + patch: serde_json::Value, + ) -> Result<()> { + let pool = self.storage.pool().await?; + let mut tx = pool.begin_with("BEGIN IMMEDIATE").await?; + + let rows = sqlx::query_as::<_, (i64, String)>( + "SELECT id, content_json FROM thread_messages \ + WHERE thread_id = ? AND message_id = ? \ + ORDER BY id ASC", + ) + .bind(thread_id) + .bind(message_id) + .fetch_all(&mut *tx) + .await?; + + for (row_id, content_json) in rows { + let mut content: Vec = serde_json::from_str(&content_json)?; + let mut found = false; + for block in &mut content { + if let MessageContent::ToolRequest(tr) = block { + if tr.id == tool_call_id { + tr.tool_meta = Some(merge_tool_meta(tr.tool_meta.take(), &patch)); + found = true; + break; + } + } + } + if !found { + continue; + } + + let updated_json = serde_json::to_string(&content)?; + sqlx::query("UPDATE thread_messages SET content_json = ? WHERE id = ?") + .bind(updated_json) + .bind(row_id) + .execute(&mut *tx) + .await?; + tx.commit().await?; + return Ok(()); + } + + tx.commit().await?; + Ok(()) + } + pub async fn list_messages(&self, thread_id: &str) -> Result> { let pool = self.storage.pool().await?; let rows = sqlx::query_as::<_, (Option, String, Option, String, i64, String)>( @@ -431,3 +498,450 @@ fn append_text_json(content_json: &str, new_text: &str) -> anyhow::Result, + patch: &serde_json::Value, +) -> serde_json::Value { + let mut base = match existing { + Some(serde_json::Value::Object(map)) => map, + _ => serde_json::Map::new(), + }; + if let serde_json::Value::Object(patch_map) = patch { + for (k, v) in patch_map { + base.insert(k.clone(), v.clone()); + } + } + serde_json::Value::Object(base) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::conversation::message::{ + Message, MessageContent, ToolRequest, TOOL_META_CHAIN_SUMMARY_KEY, + TOOL_META_EXTERNAL_DISPATCH_KEY, TOOL_META_TITLE_KEY, + }; + use crate::session::SessionManager; + use rmcp::model::CallToolRequestParams; + use tempfile::TempDir; + + fn assistant_message_with_tool_request( + tool_id: &str, + tool_meta: Option, + ) -> Message { + let tool_request = ToolRequest { + id: tool_id.to_string(), + tool_call: Ok(CallToolRequestParams::new("developer__shell")), + metadata: None, + tool_meta, + }; + Message::new( + Role::Assistant, + chrono::Utc::now().timestamp_millis(), + vec![MessageContent::ToolRequest(tool_request)], + ) + } + + async fn fresh_thread_manager(temp: &TempDir) -> Arc { + let session_manager = SessionManager::new(temp.path().to_path_buf()); + Arc::new(ThreadManager::new(session_manager.storage().clone())) + } + + #[tokio::test] + async fn update_tool_request_meta_sets_title_when_missing() { + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let stored = mgr + .append_message( + &thread.id, + None, + &assistant_message_with_tool_request("tc-1", None), + ) + .await + .unwrap(); + let message_id = stored.id.clone().unwrap(); + + mgr.update_tool_request_meta( + &thread.id, + &message_id, + "tc-1", + serde_json::json!({ TOOL_META_TITLE_KEY: "reading config" }), + ) + .await + .unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + let req = match &messages[0].content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + assert_eq!(req.persisted_title(), Some("reading config")); + } + + #[tokio::test] + async fn update_tool_request_meta_preserves_existing_keys() { + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let stored = mgr + .append_message( + &thread.id, + None, + &assistant_message_with_tool_request( + "tc-1", + Some(serde_json::json!({ TOOL_META_EXTERNAL_DISPATCH_KEY: true })), + ), + ) + .await + .unwrap(); + let message_id = stored.id.clone().unwrap(); + + mgr.update_tool_request_meta( + &thread.id, + &message_id, + "tc-1", + serde_json::json!({ TOOL_META_TITLE_KEY: "running commands" }), + ) + .await + .unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + let req = match &messages[0].content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + assert!( + req.is_externally_dispatched(), + "external_dispatch key should be preserved across the merge" + ); + assert_eq!(req.persisted_title(), Some("running commands")); + } + + #[tokio::test] + async fn update_tool_request_meta_overwrites_existing_value() { + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let stored = mgr + .append_message( + &thread.id, + None, + &assistant_message_with_tool_request( + "tc-1", + Some(serde_json::json!({ TOOL_META_TITLE_KEY: "old" })), + ), + ) + .await + .unwrap(); + let message_id = stored.id.clone().unwrap(); + + mgr.update_tool_request_meta( + &thread.id, + &message_id, + "tc-1", + serde_json::json!({ TOOL_META_TITLE_KEY: "new" }), + ) + .await + .unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + let req = match &messages[0].content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + assert_eq!(req.persisted_title(), Some("new")); + } + + #[tokio::test] + async fn update_tool_request_meta_no_op_for_unknown_message() { + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + mgr.update_tool_request_meta( + &thread.id, + "missing-message-id", + "tc-1", + serde_json::json!({ TOOL_META_TITLE_KEY: "x" }), + ) + .await + .expect("missing message must be a no-op, not an error"); + } + + #[tokio::test] + async fn update_tool_request_meta_no_op_for_unknown_tool_call() { + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let stored = mgr + .append_message( + &thread.id, + None, + &assistant_message_with_tool_request("tc-1", None), + ) + .await + .unwrap(); + let message_id = stored.id.clone().unwrap(); + + mgr.update_tool_request_meta( + &thread.id, + &message_id, + "tc-other", + serde_json::json!({ TOOL_META_TITLE_KEY: "x" }), + ) + .await + .unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + let req = match &messages[0].content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + assert!( + req.persisted_title().is_none(), + "no-match must leave tool_meta untouched" + ); + } + + #[tokio::test] + async fn update_tool_request_meta_targets_correct_row_when_message_id_is_shared() { + // Regression for "first tool call in a chain consistently shows the + // deterministic title on reload." Bedrock/Anthropic-style streaming + // produces a single LLM message id (e.g. `msg_bdrk_…`) but the agent + // splits it across multiple `AgentEvent::Message` events — one for + // text, one for the trailing tool_request — and `append_message` + // writes a separate row per event. Both rows end up with the SAME + // `message_id`. `fetch_optional` returned the text-only row first and + // the title never persisted. + use crate::conversation::message::ToolRequest; + use rmcp::model::CallToolRequestParams; + + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let shared_id = "msg_bdrk_shared".to_string(); + + let mut text_only = Message::new( + Role::Assistant, + chrono::Utc::now().timestamp_millis(), + vec![MessageContent::text( + "Let me look at the project structure.", + )], + ); + text_only.id = Some(shared_id.clone()); + let stored_text = mgr + .append_message(&thread.id, None, &text_only) + .await + .unwrap(); + assert_eq!(stored_text.id.as_deref(), Some(shared_id.as_str())); + + let mut tool_message = Message::new( + Role::Assistant, + chrono::Utc::now().timestamp_millis(), + vec![MessageContent::ToolRequest(ToolRequest { + id: "toolu_tree".to_string(), + tool_call: Ok(CallToolRequestParams::new("tree")), + metadata: None, + tool_meta: None, + })], + ); + tool_message.id = Some(shared_id.clone()); + let stored_tool = mgr + .append_message(&thread.id, None, &tool_message) + .await + .unwrap(); + assert_eq!(stored_tool.id.as_deref(), Some(shared_id.as_str())); + + mgr.update_tool_request_meta( + &thread.id, + &shared_id, + "toolu_tree", + serde_json::json!({ TOOL_META_TITLE_KEY: "exploring project structure" }), + ) + .await + .unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + assert_eq!(messages.len(), 2, "two distinct rows must be preserved"); + let text_msg = &messages[0]; + let tool_msg = &messages[1]; + assert!( + matches!(&text_msg.content[0], MessageContent::Text(_)), + "first row must remain text-only and untouched", + ); + let tr = match &tool_msg.content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request in second row"), + }; + assert_eq!( + tr.persisted_title(), + Some("exploring project structure"), + "title must land on the row that actually contains the tool call", + ); + } + + #[tokio::test] + async fn update_tool_request_meta_serializes_concurrent_writes_preserving_all_keys() { + // Regression for "occasional bad replay" when multiple persist tasks + // (per-tool title for tc-1, per-tool title for tc-2, chain summary on + // tc-1) race against each other for the same row's tool_meta. They + // must serialize via BEGIN IMMEDIATE and merge rather than clobber. + use crate::conversation::message::ToolRequest; + use rmcp::model::CallToolRequestParams; + + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let message = Message::new( + Role::Assistant, + chrono::Utc::now().timestamp_millis(), + vec![ + MessageContent::ToolRequest(ToolRequest { + id: "tc-1".to_string(), + tool_call: Ok(CallToolRequestParams::new("developer__shell")), + metadata: None, + tool_meta: None, + }), + MessageContent::ToolRequest(ToolRequest { + id: "tc-2".to_string(), + tool_call: Ok(CallToolRequestParams::new("developer__shell")), + metadata: None, + tool_meta: None, + }), + ], + ); + let stored = mgr + .append_message(&thread.id, None, &message) + .await + .unwrap(); + let message_id = stored.id.clone().unwrap(); + + let m1 = mgr.clone(); + let t1 = thread.id.clone(); + let mid1 = message_id.clone(); + let h1 = tokio::spawn(async move { + m1.update_tool_request_meta( + &t1, + &mid1, + "tc-1", + serde_json::json!({ TOOL_META_TITLE_KEY: "ran shell command" }), + ) + .await + .unwrap(); + }); + + let m2 = mgr.clone(); + let t2 = thread.id.clone(); + let mid2 = message_id.clone(); + let h2 = tokio::spawn(async move { + m2.update_tool_request_meta( + &t2, + &mid2, + "tc-2", + serde_json::json!({ TOOL_META_TITLE_KEY: "ran another shell command" }), + ) + .await + .unwrap(); + }); + + let m3 = mgr.clone(); + let t3 = thread.id.clone(); + let mid3 = message_id.clone(); + let h3 = tokio::spawn(async move { + m3.update_tool_request_meta( + &t3, + &mid3, + "tc-1", + serde_json::json!({ + TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "inspected codebase", "count": 2 }, + }), + ) + .await + .unwrap(); + }); + + h1.await.unwrap(); + h2.await.unwrap(); + h3.await.unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + let content = &messages[0].content; + let tc1 = match &content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + let tc2 = match &content[1] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + + assert_eq!( + tc1.persisted_title(), + Some("ran shell command"), + "concurrent writes must not drop tc-1's title", + ); + let chain_summary = tc1 + .persisted_chain_summary() + .expect("tc-1 must keep its chain summary"); + assert_eq!(chain_summary.summary, "inspected codebase"); + assert_eq!(chain_summary.count, 2); + assert_eq!( + tc2.persisted_title(), + Some("ran another shell command"), + "concurrent writes must not drop tc-2's title", + ); + } + + #[tokio::test] + async fn update_tool_request_meta_persists_chain_summary_object() { + let temp = TempDir::new().unwrap(); + let mgr = fresh_thread_manager(&temp).await; + let thread = mgr.create_thread(None, None, None).await.unwrap(); + + let stored = mgr + .append_message( + &thread.id, + None, + &assistant_message_with_tool_request( + "tc-1", + Some(serde_json::json!({ TOOL_META_TITLE_KEY: "first step" })), + ), + ) + .await + .unwrap(); + let message_id = stored.id.clone().unwrap(); + + mgr.update_tool_request_meta( + &thread.id, + &message_id, + "tc-1", + serde_json::json!({ + TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "applied dark mode polish", "count": 4 }, + }), + ) + .await + .unwrap(); + + let messages = mgr.list_messages(&thread.id).await.unwrap(); + let req = match &messages[0].content[0] { + MessageContent::ToolRequest(r) => r, + _ => panic!("expected tool request"), + }; + let chain = req + .persisted_chain_summary() + .expect("chain summary should be present"); + assert_eq!(chain.summary, "applied dark mode polish"); + assert_eq!(chain.count, 4); + assert_eq!(req.persisted_title(), Some("first step")); + } +} diff --git a/ui/goose2/src/features/chat/hooks/replayBuffer.ts b/ui/goose2/src/features/chat/hooks/replayBuffer.ts index 967f93ec..31163490 100644 --- a/ui/goose2/src/features/chat/hooks/replayBuffer.ts +++ b/ui/goose2/src/features/chat/hooks/replayBuffer.ts @@ -10,12 +10,7 @@ * When the session finishes loading (loadingSessionIds removes the id), the * buffer is flushed as a single store.setMessages() call — O(1) re-render. */ -import type { - Message, - MessageContent, - ToolRequestContent, - ToolResponseContent, -} from "@/shared/types/messages"; +import type { Message } from "@/shared/types/messages"; const replayBuffers = new Map(); @@ -51,25 +46,3 @@ export function getAndDeleteReplayBuffer( export function clearReplayBuffer(sessionId: string): void { replayBuffers.delete(sessionId); } - -export function findLatestUnpairedToolRequest( - content: MessageContent[], -): ToolRequestContent | null { - for (let index = content.length - 1; index >= 0; index -= 1) { - const block = content[index]; - if (block?.type !== "toolRequest") { - continue; - } - - const alreadyHasResponse = content.some( - (candidate): candidate is ToolResponseContent => - candidate.type === "toolResponse" && candidate.id === block.id, - ); - - if (!alreadyHasResponse) { - return block; - } - } - - return null; -} diff --git a/ui/goose2/src/features/chat/lib/__tests__/toolCallPresentation.test.ts b/ui/goose2/src/features/chat/lib/__tests__/toolCallPresentation.test.ts new file mode 100644 index 00000000..4b933be4 --- /dev/null +++ b/ui/goose2/src/features/chat/lib/__tests__/toolCallPresentation.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it } from "vitest"; +import { + getToolInputSummaryRows, + isHoistableText, + isStringifiedCopyOfStructured, +} from "../toolCallPresentation"; + +describe("getToolInputSummaryRows", () => { + it("returns command + working directory rows for shell-style args", () => { + const rows = getToolInputSummaryRows({ + name: "developer__shell", + arguments: { command: "npm test", cwd: "/repo" }, + }); + expect(rows).toEqual([ + { + kind: "command", + value: "npm test", + monospace: true, + renderAs: "bash", + }, + { kind: "workingDirectory", value: "/repo", monospace: true }, + ]); + }); + + it("returns a resource row for url args", () => { + const rows = getToolInputSummaryRows({ + name: "fetch", + arguments: { url: "https://example.com" }, + }); + expect(rows).toEqual([ + { kind: "resource", value: "https://example.com", monospace: true }, + ]); + }); + + it("returns query + path rows for search-style args", () => { + const rows = getToolInputSummaryRows({ + name: "developer__grep", + arguments: { query: "TODO", path: "/repo/src" }, + }); + expect(rows).toEqual([ + { kind: "query", value: "TODO", monospace: true }, + { kind: "path", value: "/repo/src", monospace: true }, + ]); + }); + + it("collapses long file paths to basenames and preserves the full path in title", () => { + const rows = getToolInputSummaryRows({ + name: "developer__edit", + arguments: { path: "/Users/tho/repo/src/lib/index.ts" }, + }); + expect(rows).toEqual([ + { + kind: "path", + value: "index.ts", + monospace: true, + title: "/Users/tho/repo/src/lib/index.ts", + }, + ]); + }); + + it("includes line row when present alongside a path", () => { + const rows = getToolInputSummaryRows({ + name: "developer__read", + arguments: { path: "/repo/foo.ts", line: 42 }, + }); + expect(rows).toEqual([ + { + kind: "path", + value: "foo.ts", + monospace: true, + title: "/repo/foo.ts", + }, + { kind: "line", value: "42" }, + ]); + }); + + it("falls back to a tool row with the tool name when no familiar arg keys are present", () => { + const rows = getToolInputSummaryRows({ + name: "custom-extension", + arguments: { foo: 1 }, + }); + expect(rows).toEqual([{ kind: "tool", value: "custom-extension" }]); + }); + + it("returns an empty list when args and name are both empty", () => { + expect(getToolInputSummaryRows({ name: "", arguments: {} })).toEqual([]); + }); + + it("ignores empty string values when scanning args", () => { + const rows = getToolInputSummaryRows({ + name: "developer__shell", + arguments: { command: " ", cwd: "/repo" }, + }); + expect(rows).toEqual([ + { + kind: "path", + value: "repo", + monospace: true, + title: "/repo", + }, + ]); + }); +}); + +describe("isHoistableText", () => { + it("accepts a short single-line string", () => { + expect(isHoistableText("Found 3 matches")).toBe(true); + }); + + it("rejects multi-line strings", () => { + expect(isHoistableText("line one\nline two")).toBe(false); + expect(isHoistableText("line one\rline two")).toBe(false); + }); + + it("rejects empty / whitespace-only strings", () => { + expect(isHoistableText("")).toBe(false); + expect(isHoistableText(" ")).toBe(false); + expect(isHoistableText(undefined)).toBe(false); + }); + + it("rejects strings longer than the max length budget", () => { + expect(isHoistableText("x".repeat(80))).toBe(true); + expect(isHoistableText("x".repeat(81))).toBe(false); + }); + + it("trims before measuring length and line count", () => { + expect(isHoistableText(" Found 3 matches ")).toBe(true); + expect(isHoistableText(` ${"x".repeat(81)} `)).toBe(false); + }); +}); + +describe("isStringifiedCopyOfStructured", () => { + it("returns true when text is a compact JSON stringification of structured", () => { + const structured = { kind: "summary", count: 3 }; + expect( + isStringifiedCopyOfStructured(JSON.stringify(structured), structured), + ).toBe(true); + }); + + it("returns true when text is a pretty-printed JSON stringification", () => { + const structured = { kind: "summary", count: 3 }; + expect( + isStringifiedCopyOfStructured( + JSON.stringify(structured, null, 2), + structured, + ), + ).toBe(true); + }); + + it("returns false when text is not valid JSON", () => { + expect(isStringifiedCopyOfStructured("Found 3 matches", { count: 3 })).toBe( + false, + ); + }); + + it("returns false when parsed text differs structurally from structured", () => { + expect( + isStringifiedCopyOfStructured(JSON.stringify({ count: 3 }), { count: 4 }), + ).toBe(false); + }); + + it("returns false when either side is missing", () => { + expect(isStringifiedCopyOfStructured(undefined, { count: 3 })).toBe(false); + expect(isStringifiedCopyOfStructured("{}", undefined)).toBe(false); + }); + + it("treats null structured as a valid comparison target", () => { + expect(isStringifiedCopyOfStructured("null", null)).toBe(true); + }); +}); diff --git a/ui/goose2/src/features/chat/lib/__tests__/toolChainGrouping.test.ts b/ui/goose2/src/features/chat/lib/__tests__/toolChainGrouping.test.ts new file mode 100644 index 00000000..426daefb --- /dev/null +++ b/ui/goose2/src/features/chat/lib/__tests__/toolChainGrouping.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, it } from "vitest"; +import { + getChainAggregateStatus, + getToolItemName, + getToolItemStatus, + shouldRenderAsGroupedChain, + type ToolChainItem, +} from "../toolChainGrouping"; +import type { + ToolRequestContent, + ToolResponseContent, +} from "@/shared/types/messages"; + +function makeRequest( + overrides: Partial = {}, +): ToolRequestContent { + return { + type: "toolRequest", + id: "req-1", + name: "tool name", + arguments: {}, + status: "completed", + ...overrides, + }; +} + +function makeResponse( + overrides: Partial = {}, +): ToolResponseContent { + return { + type: "toolResponse", + id: "req-1", + name: "tool name", + result: "ok", + isError: false, + ...overrides, + }; +} + +function pair( + key: string, + request?: Partial, + response?: Partial, +): ToolChainItem { + return { + key, + request: request ? makeRequest(request) : undefined, + response: response ? makeResponse(response) : undefined, + }; +} + +describe("getToolItemName", () => { + it("uses request name when present", () => { + expect(getToolItemName(pair("a", { name: "edit foo" }))).toBe("edit foo"); + }); + + it("falls back to response name", () => { + expect(getToolItemName(pair("a", undefined, { name: "ran sh" }))).toBe( + "ran sh", + ); + }); + + it("falls back to a generic label when neither has a name", () => { + expect(getToolItemName(pair("a", { name: "" }, { name: "" }))).toBe( + "Tool result", + ); + }); +}); + +describe("getToolItemStatus", () => { + it("treats response presence as completed", () => { + expect(getToolItemStatus(pair("a", { name: "x" }, {}))).toBe("completed"); + }); + + it("treats response.isError as error", () => { + expect(getToolItemStatus(pair("a", { name: "x" }, { isError: true }))).toBe( + "error", + ); + }); + + it("uses request status when no response yet", () => { + expect(getToolItemStatus(pair("a", { status: "executing" }))).toBe( + "executing", + ); + }); +}); + +describe("getChainAggregateStatus", () => { + it("prefers error over pending and executing", () => { + expect( + getChainAggregateStatus([ + pair("a", { status: "executing" }), + pair("b", { name: "x" }, { isError: true }), + pair("c", { status: "pending" }), + ]), + ).toBe("error"); + }); + + it("prefers stopped over executing/pending when there is no error", () => { + expect( + getChainAggregateStatus([ + pair("a", { status: "executing" }), + pair("b", { status: "stopped" }), + ]), + ).toBe("stopped"); + }); + + it("returns executing when any request is executing and none failed", () => { + expect( + getChainAggregateStatus([ + pair("a", { name: "x" }, {}), + pair("b", { status: "executing" }), + ]), + ).toBe("executing"); + }); + + it("returns pending when only pending is present", () => { + expect( + getChainAggregateStatus([ + pair("a", { status: "pending" }), + pair("b", { status: "pending" }), + ]), + ).toBe("pending"); + }); + + it("returns completed when every step finished cleanly", () => { + expect( + getChainAggregateStatus([ + pair("a", { name: "x" }, {}), + pair("b", { name: "y" }, {}), + ]), + ).toBe("completed"); + }); +}); + +describe("shouldRenderAsGroupedChain", () => { + it("is false for single-item sections", () => { + expect(shouldRenderAsGroupedChain([pair("a", { name: "x" })])).toBe(false); + }); + + it("is true once there are 2+ items", () => { + expect( + shouldRenderAsGroupedChain([ + pair("a", { name: "x" }), + pair("b", { name: "y" }), + ]), + ).toBe(true); + }); +}); diff --git a/ui/goose2/src/features/chat/lib/__tests__/toolChainSummary.test.ts b/ui/goose2/src/features/chat/lib/__tests__/toolChainSummary.test.ts new file mode 100644 index 00000000..acbf0d90 --- /dev/null +++ b/ui/goose2/src/features/chat/lib/__tests__/toolChainSummary.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "vitest"; +import { summarizeToolChainSteps } from "../toolChainSummary"; +import type { ToolChainItem } from "../toolChainGrouping"; + +let nextId = 0; + +function step(name: string): ToolChainItem { + const id = `step-${++nextId}`; + return { + key: id, + request: { + type: "toolRequest", + id, + name, + arguments: {}, + status: "completed", + }, + }; +} + +describe("summarizeToolChainSteps", () => { + it("classifies file reads as reviewing_files", () => { + const summary = summarizeToolChainSteps([ + step("Read · src/index.ts"), + step("List · src/"), + ]); + expect(summary.kind).toBe("reviewing_files"); + expect(summary.titleKey).toBe("tool_chain.summary.reviewing_files"); + expect(summary.count).toBe(2); + }); + + it("classifies shell/bash steps as running_commands", () => { + const summary = summarizeToolChainSteps([ + step("Shell · ls -la"), + step("Shell · cargo test"), + step("Read · Cargo.toml"), + ]); + expect(summary.kind).toBe("running_commands"); + expect(summary.titleKey).toBe("tool_chain.summary.running_commands"); + }); + + it("classifies write/edit-heavy chains as updating_files", () => { + const summary = summarizeToolChainSteps([ + step("Edit · src/lib.rs"), + step("Write · src/new.rs"), + step("Read · src/lib.rs"), + ]); + expect(summary.kind).toBe("updating_files"); + expect(summary.titleKey).toBe("tool_chain.summary.updating_files"); + }); + + it("classifies fetch/url steps as checking_resources", () => { + const summary = summarizeToolChainSteps([ + step("Fetch · https://example.com"), + step("Fetch · https://example.com/api"), + ]); + expect(summary.kind).toBe("checking_resources"); + expect(summary.titleKey).toBe("tool_chain.summary.checking_resources"); + }); + + it("uses the detail to detect URLs even when the prefix is generic", () => { + const summary = summarizeToolChainSteps([ + step("Tool · https://example.com"), + step("Tool · https://example.com/api"), + ]); + expect(summary.kind).toBe("checking_resources"); + }); + + it("falls back to reviewing_files when nothing dominates", () => { + const summary = summarizeToolChainSteps([ + step("Read · src/a.ts"), + step("Read · src/b.ts"), + step("Read · src/c.ts"), + ]); + expect(summary.kind).toBe("reviewing_files"); + }); + + it("returns a sane default for empty chains", () => { + const summary = summarizeToolChainSteps([]); + expect(summary.count).toBe(0); + expect(summary.kind).toBe("reviewing_files"); + }); + + it("breaks ties between updating and reviewing in favor of updating only when strictly greater", () => { + const summary = summarizeToolChainSteps([ + step("Edit · src/a.ts"), + step("Read · src/b.ts"), + ]); + expect(summary.kind).toBe("reviewing_files"); + }); + + it("treats command tokens correctly when prefix contains 'execute'", () => { + const summary = summarizeToolChainSteps([ + step("Execute · npm test"), + step("Execute · npm build"), + ]); + expect(summary.kind).toBe("running_commands"); + }); +}); diff --git a/ui/goose2/src/features/chat/lib/toolCallPresentation.ts b/ui/goose2/src/features/chat/lib/toolCallPresentation.ts new file mode 100644 index 00000000..b66cdc99 --- /dev/null +++ b/ui/goose2/src/features/chat/lib/toolCallPresentation.ts @@ -0,0 +1,201 @@ +const COMMAND_KEYS = ["command", "cmd", "script"]; +const SEARCH_KEYS = ["query", "pattern", "search", "needle", "text"]; +const PATH_KEYS = [ + "path", + "file", + "filePath", + "filepath", + "targetPath", + "directory", + "dir", + "cwd", + "folder", +]; +const URL_KEYS = ["url", "uri", "href"]; + +/** + * Stable, locale-independent identifier for a row's label. The renderer + * resolves this to a translated string at draw time via + * `chat.tools.inputSummary.`. Keeping the identifier on the row + * (rather than baking the English label in here) lets downstream code + * branch on row identity (e.g. headers that pull the path row) without + * snapping to a particular locale. + */ +export type ToolInputSummaryRowKind = + | "command" + | "workingDirectory" + | "query" + | "path" + | "resource" + | "line" + | "tool"; + +export interface ToolInputSummaryRow { + kind: ToolInputSummaryRowKind; + value: string; + monospace?: boolean; + /** Full path/value for hover tooltip when `value` was shortened. */ + title?: string; + /** Hint for syntax-highlighting downstream renderers. */ + renderAs?: "text" | "bash"; +} + +interface ToolCallPresentationInput { + name: string; + arguments: Record; +} + +function getStringArgument( + args: Record, + keys: string[], +): string | undefined { + for (const key of keys) { + const value = args[key]; + if (typeof value === "string" && value.trim().length > 0) { + return value.trim(); + } + } + + return undefined; +} + +function getNumericArgument( + args: Record, + keys: string[], +): number | undefined { + for (const key of keys) { + const value = args[key]; + if (typeof value === "number" && Number.isFinite(value)) { + return value; + } + } + + return undefined; +} + +function basenameOf(path: string): string { + const normalized = path.replace(/\\/g, "/"); + const parts = normalized.split("/").filter(Boolean); + return parts[parts.length - 1] ?? path; +} + +/** + * Translate raw tool arguments into a small set of labeled rows for the + * expanded tool card. Falls back to an empty list when no familiar shape is + * found, leaving the JSON dump as the canonical representation. + * + * Slim port of `toolCallPresentation.ts` from PR #8773 — that version also + * leaned on `kind` / `locations` on the wire. The current main does not carry + * those fields, so this version is args-only. + */ +export function getToolInputSummaryRows({ + name, + arguments: args, +}: ToolCallPresentationInput): ToolInputSummaryRow[] { + const command = getStringArgument(args, COMMAND_KEYS); + if (command) { + const cwd = getStringArgument(args, ["cwd"]); + return [ + { + kind: "command", + value: command, + monospace: true, + renderAs: "bash", + }, + ...(cwd + ? [ + { + kind: "workingDirectory" as const, + value: cwd, + monospace: true, + }, + ] + : []), + ]; + } + + const query = getStringArgument(args, SEARCH_KEYS); + if (query) { + const path = getStringArgument(args, PATH_KEYS); + return [ + { kind: "query", value: query, monospace: true }, + ...(path + ? [{ kind: "path" as const, value: path, monospace: true }] + : []), + ]; + } + + const url = getStringArgument(args, URL_KEYS); + if (url) { + return [{ kind: "resource", value: url, monospace: true }]; + } + + const path = getStringArgument(args, PATH_KEYS); + if (path) { + const line = getNumericArgument(args, ["line", "startLine"]); + const displayPath = basenameOf(path); + return [ + { + kind: "path", + value: displayPath, + monospace: true, + title: path, + }, + ...(line ? [{ kind: "line" as const, value: String(line) }] : []), + ]; + } + + if (name.trim().length > 0) { + return [{ kind: "tool", value: name }]; + } + + return []; +} + +/** + * Maximum length (in characters, after trim) for a text result to be eligible + * for hoisting into the tool header subtitle. Longer values stay in the body. + */ +const HOISTABLE_TEXT_MAX_LENGTH = 80; + +/** + * Returns true when a tool's text `result` is short enough and simple enough + * to render as a header subtitle alongside the tool name. Multi-line and + * empty/whitespace-only strings are never hoistable. + */ +export function isHoistableText(text: string | undefined): text is string { + if (typeof text !== "string") return false; + const trimmed = text.trim(); + if (trimmed.length === 0) return false; + if (trimmed.includes("\n") || trimmed.includes("\r")) return false; + return trimmed.length <= HOISTABLE_TEXT_MAX_LENGTH; +} + +/** + * Returns true when a text `result` is just a stringified copy of the + * `structured` content — e.g. a server that emits `result = JSON.stringify(x)` + * alongside `structuredContent = x`. Whitespace- and indent-insensitive: the + * comparison normalizes both sides via JSON parse + compact stringify. + * + * Used by the de-dupe matrix in ToolCallAdapter to suppress the redundant + * text result when the structured form will already be rendered. + */ +export function isStringifiedCopyOfStructured( + text: string | undefined, + structured: unknown, +): boolean { + if (typeof text !== "string" || structured === undefined) return false; + + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return false; + } + + try { + return JSON.stringify(parsed) === JSON.stringify(structured); + } catch { + return false; + } +} diff --git a/ui/goose2/src/features/chat/lib/toolChainGrouping.ts b/ui/goose2/src/features/chat/lib/toolChainGrouping.ts new file mode 100644 index 00000000..8c975e80 --- /dev/null +++ b/ui/goose2/src/features/chat/lib/toolChainGrouping.ts @@ -0,0 +1,52 @@ +import type { + ToolCallStatus, + ToolRequestContent, + ToolResponseContent, +} from "@/shared/types/messages"; + +/** + * A pairing of one tool request and (optionally) its matching response, + * preserving the order they appeared in the assistant message. + * + * Chains are derived from this order — see `groupAdjacentToolItems` and + * `MessageBubble.groupContentSections` — so the server emits no chain metadata. + */ +export interface ToolChainItem { + key: string; + request?: ToolRequestContent; + response?: ToolResponseContent; +} + +export function getToolItemName(item: ToolChainItem): string { + return item.request?.name || item.response?.name || "Tool result"; +} + +export function getToolItemStatus(item: ToolChainItem): ToolCallStatus { + if (item.response) { + return item.response.isError ? "error" : "completed"; + } + return item.request?.status ?? "completed"; +} + +/** + * Aggregate status across a chain. Failure-leaning so collapsed parents don't + * mask a failed step behind a still-pending sibling. + */ +export function getChainAggregateStatus( + items: ToolChainItem[], +): ToolCallStatus { + if (items.some((i) => getToolItemStatus(i) === "error")) return "error"; + if (items.some((i) => getToolItemStatus(i) === "stopped")) return "stopped"; + if (items.some((i) => getToolItemStatus(i) === "executing")) + return "executing"; + if (items.some((i) => getToolItemStatus(i) === "pending")) return "pending"; + return "completed"; +} + +/** + * Whether the section should render as a grouped parent card. Single-item + * sections render inline (no parent wrapper), matching prior UX. + */ +export function shouldRenderAsGroupedChain(items: ToolChainItem[]): boolean { + return items.length >= 2; +} diff --git a/ui/goose2/src/features/chat/lib/toolChainSummary.ts b/ui/goose2/src/features/chat/lib/toolChainSummary.ts new file mode 100644 index 00000000..dc166c8f --- /dev/null +++ b/ui/goose2/src/features/chat/lib/toolChainSummary.ts @@ -0,0 +1,153 @@ +import { getToolItemName, type ToolChainItem } from "./toolChainGrouping"; + +/** + * Buckets used to classify a single step inside a tool chain. Ported from the + * Rust `classify_tool_chain_step` / `summarize_tool_chain` that previously + * lived in `crates/goose/src/acp/server.rs` so the wire stays free of any + * `_goose/tool-chain-*` metadata. + */ +export type ToolChainStepKind = + | "reviewing_files" + | "running_commands" + | "checking_resources" + | "updating_files"; + +const STEP_TITLE_KEYS: Record = { + reviewing_files: "tool_chain.summary.reviewing_files", + running_commands: "tool_chain.summary.running_commands", + checking_resources: "tool_chain.summary.checking_resources", + updating_files: "tool_chain.summary.updating_files", +}; + +const ACTIVE_TITLE_KEY = "tool_chain.summary.active"; + +const HTTP_PREFIXES = ["http://", "https://"]; +const RESOURCE_TOKENS = ["fetch", "http", "url", "uri", "download"]; +const UPDATE_TOKENS = [ + "edit", + "write", + "create", + "update", + "replace", + "rename", + "move", + "delete", +]; +const COMMAND_TOKENS = ["shell", "command", "bash", "terminal", "execute"]; + +function classifyStepLabel(label: string): ToolChainStepKind { + const lower = label.toLowerCase(); + const sepIndex = lower.indexOf(" · "); + const prefix = sepIndex === -1 ? lower : lower.slice(0, sepIndex); + const detail = sepIndex === -1 ? "" : lower.slice(sepIndex + 3); + + if ( + HTTP_PREFIXES.some((p) => detail.startsWith(p)) || + RESOURCE_TOKENS.some((t) => prefix.includes(t)) + ) { + return "checking_resources"; + } + + if (UPDATE_TOKENS.some((t) => prefix.includes(t))) { + return "updating_files"; + } + + if (COMMAND_TOKENS.some((t) => prefix.includes(t))) { + return "running_commands"; + } + + return "reviewing_files"; +} + +interface BucketCounts { + reviewing_files: number; + running_commands: number; + checking_resources: number; + updating_files: number; +} + +function countBuckets(items: ToolChainItem[]): BucketCounts { + const counts: BucketCounts = { + reviewing_files: 0, + running_commands: 0, + checking_resources: 0, + updating_files: 0, + }; + for (const item of items) { + counts[classifyStepLabel(getToolItemName(item))] += 1; + } + return counts; +} + +function pickDominantBucket(counts: BucketCounts): ToolChainStepKind { + const { + reviewing_files, + running_commands, + checking_resources, + updating_files, + } = counts; + + if ( + updating_files > reviewing_files && + updating_files >= running_commands && + updating_files >= checking_resources + ) { + return "updating_files"; + } + + if ( + checking_resources > reviewing_files && + checking_resources >= running_commands + ) { + return "checking_resources"; + } + + if (running_commands > reviewing_files) { + return "running_commands"; + } + + return "reviewing_files"; +} + +export interface ToolChainSummary { + /** i18n key for the chain title (e.g. "running commands"). */ + titleKey: string; + /** i18n key for the count suffix; consumers add count via `t(suffixKey, { count })`. */ + countKey: string; + /** Number of steps the parent card represents. */ + count: number; + /** Bucket the chain falls into; useful for icons/styling. */ + kind: ToolChainStepKind; +} + +/** + * Derive a localized chain summary from the per-step tool names. + * + * Note: when the chain is "active" (any non-completed step) callers may prefer + * the live `tool_chain.summary.active` label over the kind-based title. This + * function returns the deterministic kind regardless; the caller decides how + * to render based on aggregate status. + */ +export function summarizeToolChainSteps( + items: ToolChainItem[], +): ToolChainSummary { + if (items.length === 0) { + return { + titleKey: ACTIVE_TITLE_KEY, + countKey: "tool_chain.summary.steps", + count: 0, + kind: "reviewing_files", + }; + } + + const counts = countBuckets(items); + const kind = pickDominantBucket(counts); + return { + titleKey: STEP_TITLE_KEYS[kind], + countKey: "tool_chain.summary.steps", + count: items.length, + kind, + }; +} + +export const TOOL_CHAIN_ACTIVE_TITLE_KEY = ACTIVE_TITLE_KEY; diff --git a/ui/goose2/src/features/chat/ui/ToolCallAdapter.tsx b/ui/goose2/src/features/chat/ui/ToolCallAdapter.tsx index 99d50471..77dcbddd 100644 --- a/ui/goose2/src/features/chat/ui/ToolCallAdapter.tsx +++ b/ui/goose2/src/features/chat/ui/ToolCallAdapter.tsx @@ -1,16 +1,24 @@ -import { useState, useEffect } from "react"; +import { useEffect, useMemo, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; -import { FolderOpen, ChevronRight } from "lucide-react"; +import { ChevronRight, FolderOpen } from "lucide-react"; import { cn } from "@/shared/lib/cn"; import { Button } from "@/shared/ui/button"; +import { CodeBlock } from "@/shared/ui/ai-elements/code-block"; import { Tool, - ToolHeader, ToolContent, + ToolHeader, ToolInput, ToolOutput, + ToolSurface, } from "@/shared/ui/ai-elements/tool"; import { toolStatusMap } from "../lib/toolStatusMap"; +import { + getToolInputSummaryRows, + isHoistableText, + isStringifiedCopyOfStructured, + type ToolInputSummaryRow, +} from "@/features/chat/lib/toolCallPresentation"; import type { ToolCallLocation, ToolCallStatus } from "@/shared/types/messages"; import { useArtifactPolicyContext } from "@/features/chat/hooks/ArtifactPolicyContext"; @@ -26,6 +34,12 @@ interface ToolCallAdapterProps { startedAt?: number; open?: boolean; onOpenChange?: (open: boolean) => void; + /** When false, the chevron-side status badge is hidden (used inside chains). */ + showStatusBadge?: boolean; + /** When false, hides the trailing disclosure chevron in the header. */ + showChevron?: boolean; + /** When true, the card sizes to its content rather than filling its parent. */ + fitWidth?: boolean; } function useElapsedTime(status: ToolCallStatus, startedAt?: number) { @@ -34,7 +48,6 @@ function useElapsedTime(status: ToolCallStatus, startedAt?: number) { useEffect(() => { if (status === "executing") { const origin = startedAt ?? Date.now(); - // Compute initial elapsed immediately so the first render is accurate. setElapsed(Math.floor((Date.now() - origin) / 1000)); const interval = setInterval(() => { setElapsed(Math.floor((Date.now() - origin) / 1000)); @@ -177,6 +190,75 @@ function ArtifactActions({ locations }: { locations?: ToolCallLocation[] }) { ); } +const COMMAND_PREVIEW_CODEBLOCK_CLASSES = + "rounded-none border-0 bg-transparent shadow-none [&>div]:overflow-hidden [&_pre]:m-0 [&_pre]:bg-transparent [&_pre]:p-0 [&_pre]:whitespace-pre-wrap [&_pre]:break-words [&_pre]:text-[12px] [&_pre]:leading-5 [&_code]:font-mono [&_code]:text-[12px] [&_code]:leading-5"; + +function InputSummary({ + rows, + isOpen, +}: { + rows: ToolInputSummaryRow[]; + isOpen: boolean; +}) { + const { t } = useTranslation("chat"); + if (rows.length === 0) return null; + + return ( +
+ {rows.map((row) => { + const label = t(`tools.inputSummary.${row.kind}`); + const key = `${row.kind}:${row.value}`; + if (row.renderAs === "bash") { + return ( +
+
+ {label} +
+
+ +
+
+ ); + } + return ( +
+
+ {label} +
+
+ {row.value} +
+
+ ); + })} +
+ ); +} + +function splitHeaderTitleByPath(name: string, fileLabel: string) { + const index = name.toLowerCase().lastIndexOf(fileLabel.toLowerCase()); + if (index === -1) return null; + return { + prefix: name.slice(0, index), + fileLabel: name.slice(index, index + fileLabel.length), + suffix: name.slice(index + fileLabel.length), + }; +} + export function ToolCallAdapter({ name, arguments: args, @@ -188,19 +270,102 @@ export function ToolCallAdapter({ startedAt, open, onOpenChange, + showStatusBadge = true, + showChevron = true, + fitWidth = false, }: ToolCallAdapterProps) { const { t } = useTranslation("chat"); const elapsed = useElapsedTime(status, startedAt); const state = toolStatusMap[status]; - + const summaryRows = useMemo( + () => getToolInputSummaryRows({ name, arguments: args }), + [args, name], + ); const elapsedSeconds = status === "executing" && elapsed >= 3 ? elapsed : undefined; - const outputViewportClassName = cn( - "max-h-[28rem] overflow-auto", - "[scrollbar-color:hsl(var(--muted-foreground))_transparent] [scrollbar-width:thin]", - "[&::-webkit-scrollbar]:h-2 [&::-webkit-scrollbar]:w-2 [&::-webkit-scrollbar-track]:bg-transparent", - "[&::-webkit-scrollbar-thumb]:rounded-full [&::-webkit-scrollbar-thumb]:bg-muted-foreground/50", + + const { resolveMarkdownHref, openResolvedPath } = useArtifactPolicyContext(); + + const pathRow = summaryRows.find((row) => row.kind === "path"); + const headerFileLabel = pathRow?.value; + const headerFilePath = pathRow?.title ?? pathRow?.value; + const headerTitleParts = + headerFileLabel && headerFilePath + ? splitHeaderTitleByPath(name, headerFileLabel) + : null; + const headerFileCandidate = useMemo( + () => (headerFilePath ? resolveMarkdownHref(headerFilePath) : null), + [headerFilePath, resolveMarkdownHref], ); + const canOpenHeaderFile = Boolean(headerTitleParts && headerFileCandidate); + + const hasStructuredArgs = Object.keys(args).length > 0; + const hasOutput = Boolean(result); + const hasStructuredContent = !isError && structuredContent !== undefined; + + // De-dupe + title-hoisting matrix: when both a text result and structured + // content are present, decide whether the text is a redundant stringified + // copy of the structured payload (hide), short enough to hoist into the + // header subtitle (lift), or worth rendering in the body alongside the + // structured block (keep). + const textIsStringifiedCopy = + hasOutput && + hasStructuredContent && + isStringifiedCopyOfStructured(result, structuredContent); + const canHoistResultIntoHeader = + hasOutput && + hasStructuredContent && + !textIsStringifiedCopy && + !headerTitleParts && + isHoistableText(result); + const showResultBody = + hasOutput && !textIsStringifiedCopy && !canHoistResultIntoHeader; + + const headerTitle: ReactNode = headerTitleParts ? ( + <> + {headerTitleParts.prefix} + {canOpenHeaderFile ? ( + + ) : ( + {headerTitleParts.fileLabel} + )} + {headerTitleParts.suffix} + + ) : canHoistResultIntoHeader ? ( + <> + {name} + + + {(result ?? "").trim()} + + + ) : ( + name + ); + + const showCombinedSurface = summaryRows.length > 0 || hasStructuredArgs; return (
@@ -208,27 +373,63 @@ export function ToolCallAdapter({ - {Object.keys(args).length > 0 && } - - {!isError && structuredContent !== undefined && ( - + {showCombinedSurface ? ( + + ( + + )} + /> + {showResultBody && ( + + )} + {hasStructuredContent && ( + + )} + + ) : ( + <> + {showResultBody && ( + + )} + {hasStructuredContent && ( + + )} + )} diff --git a/ui/goose2/src/features/chat/ui/ToolChainCards.tsx b/ui/goose2/src/features/chat/ui/ToolChainCards.tsx index 1a47ed88..fa3df783 100644 --- a/ui/goose2/src/features/chat/ui/ToolChainCards.tsx +++ b/ui/goose2/src/features/chat/ui/ToolChainCards.tsx @@ -1,16 +1,87 @@ -import { useState } from "react"; -import { ChevronRight } from "lucide-react"; +import { useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Check, ChevronRight, CircleIcon, ClockIcon } from "lucide-react"; +import type { LucideIcon } from "lucide-react"; import { cn } from "@/shared/lib/cn"; import { ToolCallAdapter } from "./ToolCallAdapter"; -import type { - ToolRequestContent, - ToolResponseContent, -} from "@/shared/types/messages"; +import { + getChainAggregateStatus, + getToolItemName, + getToolItemStatus, + shouldRenderAsGroupedChain, + type ToolChainItem, +} from "@/features/chat/lib/toolChainGrouping"; +import { summarizeToolChainSteps } from "@/features/chat/lib/toolChainSummary"; +import type { ToolCallStatus } from "@/shared/types/messages"; -export interface ToolChainItem { - key: string; - request?: ToolRequestContent; - response?: ToolResponseContent; +export type { ToolChainItem }; + +const STEP_BULLET_ICON: Record< + Exclude, + LucideIcon +> = { + pending: CircleIcon, + executing: ClockIcon, + completed: Check, +}; + +const STEP_BULLET_CLASS: Record< + Exclude, + string +> = { + pending: "text-muted-foreground/70", + executing: "text-muted-foreground animate-pulse", + completed: "text-muted-foreground", +}; + +function ChainStepBullet({ status }: { status: ToolCallStatus }) { + if (status === "error") { + return ( + + ); + } + if (status === "stopped") { + return ( + + ); + } + const Icon = STEP_BULLET_ICON[status]; + return ( + + ); +} + +function ChainStepRail({ + status, + isLast = false, + lineTailVisible = true, +}: { + status: ToolCallStatus; + /** Last row in the expanded chain: hides the spine stub below the bullet until `lineTailVisible`. */ + isLast?: boolean; + lineTailVisible?: boolean; +}) { + return ( + ); }; + if (!grouped) { + return ( +
+ {primaryItems.map((item) => renderToolItem(item, { withRail: false }))} +
+ ); + } + + // Prefer the server-generated LLM chain summary (anchored on the first tool + // request of the chain) over the deterministic bucket phrase. The summary is + // attached after every step in the chain has completed, so it's only + // available for finished chains; while the chain is still active, fall back + // to the deterministic phrase. + const firstChainSummary = toolItems.find((item) => item.request?.chainSummary) + ?.request?.chainSummary; + const labelText = + !isActiveChain && firstChainSummary + ? firstChainSummary.summary + : isActiveChain + ? t("tool_chain.summary.active") + : t(summary.titleKey); + const headerText = isActiveChain + ? t("tool_chain.title.active", { count: toolItems.length }) + : t("tool_chain.title.labeled", { + label: labelText, + count: toolItems.length, + }); + + const hasHiddenDisclosure = hiddenItems.length > 0; + return ( -
- {primaryItems.map((item) => renderToolItem(item))} +
+ - {hiddenItems.length > 0 && ( -
- + {chainExpanded && ( +
+ )} -
+
); } diff --git a/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx index ae336885..7698c4d4 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/MessageBubble.test.tsx @@ -501,14 +501,19 @@ describe("MessageBubble", () => { }, ]); - render(); + const { container } = render(); + + // Completed-on-mount chains render collapsed; expand the parent card first. + const chainHeader = container.querySelector( + '[data-role="tool-chain-card"] > button[aria-expanded]', + ); + if (!chainHeader) throw new Error("expected tool-chain-card header"); + await user.click(chainHeader); expect(screen.getByText("Create PDF about whales")).toBeInTheDocument(); expect(screen.getByText("Write whales.pdf")).toBeInTheDocument(); - expect( - screen.queryByText("python3 create_whales.py"), - ).not.toBeInTheDocument(); - expect(screen.queryByText("ls -lh whales.pdf")).not.toBeInTheDocument(); + expect(screen.queryByText("python3 create_whales.py")).toBeNull(); + expect(screen.queryByText("ls -lh whales.pdf")).toBeNull(); expect(screen.getByText("Show internal steps (2)")).toBeInTheDocument(); await user.click(screen.getByText("Show internal steps (2)")); diff --git a/ui/goose2/src/features/chat/ui/__tests__/ToolCallAdapter.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ToolCallAdapter.test.tsx index 9c6a8b24..71e48015 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/ToolCallAdapter.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/ToolCallAdapter.test.tsx @@ -1,19 +1,32 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { describe, expect, it, vi, beforeEach } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ArtifactLinkCandidate } from "@/features/chat/hooks/ArtifactPolicyContext"; +import type { ToolCallLocation } from "@/shared/types/messages"; import { ToolCallAdapter } from "../ToolCallAdapter"; +const mockResolveMarkdownHref = + vi.fn<(href: string) => ArtifactLinkCandidate | null>(); +const mockPathExists = vi.fn<(path: string) => Promise>(); const mockOpenResolvedPath = vi.fn<(path: string) => Promise>(); vi.mock("@/features/chat/hooks/ArtifactPolicyContext", () => ({ useArtifactPolicyContext: () => ({ - resolveMarkdownHref: () => null, - pathExists: async () => false, + resolveMarkdownHref: mockResolveMarkdownHref, + pathExists: mockPathExists, openResolvedPath: mockOpenResolvedPath, getAllSessionArtifacts: () => [], }), })); +beforeEach(() => { + mockResolveMarkdownHref.mockReturnValue(null); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + function renderAdapter( overrides: Partial[0]> = {}, ) { @@ -28,55 +41,13 @@ function renderAdapter( ); } -describe("ToolCallAdapter — output", () => { - beforeEach(() => { - mockOpenResolvedPath.mockReset(); - }); - - it("shows content and structured content together in the parent tool accordion", () => { - renderAdapter({ - open: true, - result: JSON.stringify({ - restaurants: [{ name: "Content Coffee" }], - }), - structuredContent: { - restaurants: [{ name: "Structured Coffee" }], - }, - }); - - expect(screen.getByText("Content")).toBeInTheDocument(); - expect(screen.getByText("Structured content")).toBeInTheDocument(); - expect(screen.getByText(/Content Coffee/)).toBeInTheDocument(); - expect(screen.getByText(/Structured Coffee/)).toBeInTheDocument(); - expect( - screen.getByText(/"restaurants":\[\{"name":"Content Coffee"\}\]/), - ).toBeInTheDocument(); - expect( - screen.queryByText(/structured output .*lines/i), - ).not.toBeInTheDocument(); - }); - - it("shows falsy primitive structured content", () => { - renderAdapter({ - open: true, - result: "Completed", - structuredContent: false, - }); - - expect(screen.getByText("Structured content")).toBeInTheDocument(); - expect(screen.getByText("false")).toBeInTheDocument(); - }); -}); - describe("ToolCallAdapter — ArtifactActions", () => { - beforeEach(() => { - mockOpenResolvedPath.mockReset(); - }); + it('renders "Open file" button when a location is provided', () => { + const locations: ToolCallLocation[] = [ + { path: "/Users/test/project/output.md" }, + ]; - it('renders "Open file" button for reported ACP locations', () => { - renderAdapter({ - locations: [{ path: "/Users/test/project/output.md" }], - }); + renderAdapter({ locations }); expect(screen.getByRole("button", { name: /open file/i })).toBeEnabled(); expect( @@ -84,7 +55,7 @@ describe("ToolCallAdapter — ArtifactActions", () => { ).toBeInTheDocument(); }); - it("does NOT render artifact actions when no locations are reported", () => { + it("does NOT render artifact actions when no locations are provided", () => { renderAdapter(); expect( @@ -92,17 +63,18 @@ describe("ToolCallAdapter — ArtifactActions", () => { ).not.toBeInTheDocument(); }); - it('shows "More outputs" toggle for secondary locations', async () => { + it('shows "More outputs" toggle when there are multiple locations', async () => { const user = userEvent.setup(); - renderAdapter({ - locations: [ - { path: "/Users/test/project/output.md" }, - { path: "/Users/test/project/notes.md" }, - ], - }); + const locations: ToolCallLocation[] = [ + { path: "/Users/test/project/output.md" }, + { path: "/Users/test/project/notes.md" }, + ]; + + renderAdapter({ locations }); const toggle = screen.getByText(/more outputs/i); expect(toggle).toBeInTheDocument(); + expect( screen.queryByText("/Users/test/project/notes.md"), ).not.toBeInTheDocument(); @@ -114,33 +86,117 @@ describe("ToolCallAdapter — ArtifactActions", () => { ).toBeInTheDocument(); }); - it("opens reported location when primary button is clicked", async () => { + it("invokes openResolvedPath when an artifact button is clicked", async () => { const user = userEvent.setup(); mockOpenResolvedPath.mockResolvedValue(undefined); + const locations: ToolCallLocation[] = [ + { path: "/Users/test/project/output.md" }, + ]; + + renderAdapter({ locations }); - renderAdapter({ - locations: [{ path: "/Users/test/project/output.md" }], - }); await user.click(screen.getByRole("button", { name: /open file/i })); expect(mockOpenResolvedPath).toHaveBeenCalledWith( "/Users/test/project/output.md", ); }); +}); - it("shows opener errors", async () => { - const user = userEvent.setup(); - mockOpenResolvedPath.mockRejectedValue( - new Error("File not found: /Users/test/project/output.md"), - ); - - renderAdapter({ - locations: [{ path: "/Users/test/project/output.md" }], - }); - await user.click(screen.getByRole("button", { name: /open file/i })); - +describe("ToolCallAdapter — expanded body", () => { + it("renders the tool name and status badge in the header", () => { + renderAdapter(); expect( - await screen.findByText("File not found: /Users/test/project/output.md"), + screen.getByRole("button", { name: /write_file/i }), ).toBeInTheDocument(); }); + + it("shows the text result when expanded", () => { + renderAdapter({ open: true, structuredContent: undefined }); + expect(screen.getByText(/created \/project\/output\.md/i)).toBeVisible(); + }); + + it("renders structured content when present without a text result", () => { + renderAdapter({ + open: true, + result: undefined, + structuredContent: { kind: "summary", count: 3 }, + }); + + expect(screen.getByText(/"kind"/)).toBeInTheDocument(); + expect(screen.getByText(/"summary"/)).toBeInTheDocument(); + }); + + it("renders the error result when isError is true", () => { + renderAdapter({ open: true, isError: true, result: "Boom" }); + expect(screen.getByText("Boom")).toBeInTheDocument(); + }); +}); + +describe("ToolCallAdapter — text + structured de-dupe matrix", () => { + it("hides redundant text when result is a stringified copy of structured", () => { + const structured = { kind: "summary", count: 3 }; + renderAdapter({ + open: true, + arguments: {}, + result: JSON.stringify(structured), + structuredContent: structured, + }); + + // The structured payload renders exactly once, not twice. + const summaryMatches = screen.getAllByText(/"summary"/); + expect(summaryMatches).toHaveLength(1); + }); + + it("hoists short single-line text into the header when both differ", () => { + renderAdapter({ + open: true, + arguments: {}, + result: "Found 3 matches", + structuredContent: { matches: 3 }, + }); + + // The hoisted text renders inside the header subtitle slot. + const hoisted = document.querySelector("[data-tool-title-hoisted]"); + expect(hoisted).not.toBeNull(); + expect(hoisted?.textContent).toContain("Found 3 matches"); + + // Body shows the structured payload but does NOT duplicate the hoisted + // text — i.e. "Found 3 matches" appears exactly once across the card. + const allMatches = screen.getAllByText(/Found 3 matches/); + expect(allMatches).toHaveLength(1); + expect(screen.getByText(/"matches"/)).toBeInTheDocument(); + }); + + it("renders both text and structured in the body when text is multi-line", () => { + renderAdapter({ + open: true, + arguments: {}, + result: "line one\nline two\nline three", + structuredContent: { matches: 3 }, + }); + + // No header hoisting for multi-line text. + expect(document.querySelector("[data-tool-title-hoisted]")).toBeNull(); + + // Both blocks render in the body. + expect(screen.getByText(/line one/)).toBeInTheDocument(); + expect(screen.getByText(/"matches"/)).toBeInTheDocument(); + }); + + it("does not hoist text when path-based header hoisting takes precedence", () => { + // Tool name contains the basename → path-based hoisting activates. + renderAdapter({ + open: true, + name: "Write file output.md", + arguments: { path: "/project/output.md" }, + result: "Wrote file", + structuredContent: { bytes: 42 }, + }); + + // Path-based hoisting wins; result text stays in the body. + expect(document.querySelector("[data-tool-title-hoisted]")).toBeNull(); + expect(screen.getByText(/wrote file/i)).toBeInTheDocument(); + expect(screen.getByText(/"bytes"/)).toBeInTheDocument(); + }); }); diff --git a/ui/goose2/src/features/chat/ui/__tests__/ToolChainCards.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ToolChainCards.test.tsx new file mode 100644 index 00000000..fc69abcd --- /dev/null +++ b/ui/goose2/src/features/chat/ui/__tests__/ToolChainCards.test.tsx @@ -0,0 +1,337 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import { ToolChainCards } from "../ToolChainCards"; +import type { ToolChainItem } from "@/features/chat/lib/toolChainGrouping"; + +vi.mock("@/features/chat/hooks/ArtifactPolicyContext", () => ({ + useArtifactPolicyContext: () => ({ + resolveToolCardDisplay: () => ({ + role: "none", + primaryCandidate: null, + secondaryCandidates: [], + }), + resolveMarkdownHref: () => null, + pathExists: vi.fn().mockResolvedValue(false), + openResolvedPath: vi.fn().mockResolvedValue(undefined), + }), +})); + +let nextId = 0; + +function pair( + name: string, + options: { + isError?: boolean; + status?: ToolChainItem["request"] extends infer R + ? R extends { status: infer S } + ? S + : never + : never; + completed?: boolean; + } = {}, +): ToolChainItem { + const id = `tool-${++nextId}`; + const completed = options.completed !== false; + return { + key: id, + request: { + type: "toolRequest", + id, + name, + arguments: {}, + status: options.status ?? "completed", + }, + response: completed + ? { + type: "toolResponse", + id, + name, + result: "ok", + isError: options.isError ?? false, + } + : undefined, + }; +} + +describe("ToolChainCards", () => { + it("renders without a parent header for a single tool item", () => { + render(); + expect( + screen.queryByRole("button", { name: /reviewing files|step/i }), + ).not.toBeInTheDocument(); + }); + + it("renders a deterministic chain header for multi-tool chains", () => { + render( + , + ); + expect( + screen.getByRole("button", { name: /running commands.*2 step/i }), + ).toBeInTheDocument(); + }); + + it("uses the active label while any step is still in progress", () => { + render( + , + ); + expect( + screen.getByRole("button", { name: /working through 2 steps/i }), + ).toBeInTheDocument(); + }); + + it("collapses and re-expands an active chain when the header is clicked", async () => { + const user = userEvent.setup(); + render( + , + ); + const header = screen.getByRole("button", { + name: /working through 2 steps/i, + }); + expect(header).toHaveAttribute("aria-expanded", "true"); + await user.click(header); + expect(header).toHaveAttribute("aria-expanded", "false"); + }); + + it("starts collapsed when the chain mounts already complete (replay)", async () => { + const user = userEvent.setup(); + render( + , + ); + const header = screen.getByRole("button", { + name: /updating files.*2 steps/i, + }); + expect(header).toHaveAttribute("aria-expanded", "false"); + await user.click(header); + expect(header).toHaveAttribute("aria-expanded", "true"); + }); + + it("auto-collapses a live chain once every step has completed", () => { + const a = pair("Edit · src/a.ts"); + const bRequest = pair("Edit · src/b.ts", { + status: "executing", + completed: false, + }); + const { rerender } = render(); + const activeHeader = screen.getByRole("button", { + name: /working through 2 steps/i, + }); + expect(activeHeader).toHaveAttribute("aria-expanded", "true"); + + // Same chain identity, but the second step now has a response — i.e. the + // chain has just completed in realtime. + const bComplete: typeof bRequest = { + ...bRequest, + request: bRequest.request + ? { ...bRequest.request, status: "completed" } + : bRequest.request, + response: { + type: "toolResponse", + id: bRequest.request?.id ?? "tool-x", + name: "Edit · src/b.ts", + result: "ok", + isError: false, + }, + }; + rerender(); + + const completedHeader = screen.getByRole("button", { + name: /updating files.*2 steps/i, + }); + expect(completedHeader).toHaveAttribute("aria-expanded", "false"); + }); + + it("surfaces error status as a data attribute on the chain wrapper", () => { + const { container } = render( + , + ); + const wrapper = container.querySelector('[data-role="tool-chain-card"]'); + expect(wrapper?.getAttribute("data-status")).toBe("error"); + }); + + it("renders a step rail row for each child inside a chain", () => { + const { container } = render( + , + ); + const rows = container.querySelectorAll('[data-role="tool-chain-step"]'); + expect(rows).toHaveLength(3); + }); + + it("does not wrap a single tool call in a rail row", () => { + const { container } = render( + , + ); + const rows = container.querySelectorAll('[data-role="tool-chain-step"]'); + expect(rows).toHaveLength(0); + }); + + it("renders a left caret button on a single tool call that toggles its open state", async () => { + const user = userEvent.setup(); + const { container } = render( + , + ); + const wrapper = container.querySelector('[data-role="tool-single"]'); + expect(wrapper).not.toBeNull(); + + const caret = wrapper?.querySelector( + ":scope > button", + ) as HTMLButtonElement; + expect(caret).toBeTruthy(); + expect(caret).toHaveAttribute("aria-expanded", "false"); + await user.click(caret); + expect(caret).toHaveAttribute("aria-expanded", "true"); + }); + + it("hides the trailing right-side chevron on a single tool call", () => { + const { container } = render( + , + ); + // The shared ToolHeader's trailing chevron is a CollapsibleTrigger + // styled with the group-data-[state=closed]:-rotate-90 class. With + // showChevron={false} the icon should not render at all inside the + // single-tool wrapper. + const wrapper = container.querySelector('[data-role="tool-single"]'); + expect(wrapper).not.toBeNull(); + const trailingChevron = wrapper?.querySelector( + ".group-data-\\[state\\=closed\\]\\:-rotate-90", + ); + expect(trailingChevron).toBeNull(); + }); + + it("counts the internal-steps disclosure as part of the rail", async () => { + const user = userEvent.setup(); + const { container } = render( + , + ); + // The chain mounts as already-complete (default test pair → completed), + // so the rail starts collapsed during replay; expand it first. + await user.click( + screen.getByRole("button", { name: /updating files.*4 steps/i }), + ); + const disclosure = container.querySelector( + '[data-role="tool-chain-internal-disclosure"]', + ); + expect(disclosure).not.toBeNull(); + + const beforeRows = container.querySelectorAll( + '[data-role="tool-chain-step"]', + ); + expect(beforeRows.length).toBeGreaterThanOrEqual(1); + + const showButton = screen.getByRole("button", { + name: /show internal steps \(2\)/i, + }); + await user.click(showButton); + + const afterRows = container.querySelectorAll( + '[data-role="tool-chain-step"]', + ); + expect(afterRows.length).toBe(beforeRows.length + 2); + }); + + it("removes the heavy parent card chrome around the chain wrapper", () => { + const { container } = render( + , + ); + const wrapper = container.querySelector('[data-role="tool-chain-card"]'); + expect(wrapper).not.toBeNull(); + const className = wrapper?.getAttribute("class") ?? ""; + expect(className).not.toMatch(/border-/); + expect(className).not.toMatch(/bg-muted/); + }); + + it("prefers the LLM chain summary over the deterministic phrase when present", () => { + const a = pair("Edit · src/a.ts"); + const b = pair("Edit · src/b.ts"); + if (a.request) { + a.request.chainSummary = { + summary: "applied dark mode polish", + count: 2, + }; + } + render(); + expect( + screen.getByRole("button", { + name: /applied dark mode polish.*2 steps/i, + }), + ).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /updating files/i }), + ).not.toBeInTheDocument(); + }); + + it("falls back to the deterministic phrase when no chain summary is present", () => { + render( + , + ); + expect( + screen.getByRole("button", { name: /updating files.*2 steps/i }), + ).toBeInTheDocument(); + }); + + it("does not surface the chain summary while the chain is still active", () => { + const a = pair("Edit · src/a.ts"); + const b = pair("Edit · src/b.ts", { + status: "executing", + completed: false, + }); + if (a.request) { + a.request.chainSummary = { + summary: "applied dark mode polish", + count: 2, + }; + } + render(); + expect( + screen.getByRole("button", { name: /working through 2 steps/i }), + ).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /applied dark mode polish/i }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/ui/goose2/src/shared/api/__tests__/acpNotificationHandler.test.ts b/ui/goose2/src/shared/api/__tests__/acpNotificationHandler.test.ts index 9eff0953..8c43c44a 100644 --- a/ui/goose2/src/shared/api/__tests__/acpNotificationHandler.test.ts +++ b/ui/goose2/src/shared/api/__tests__/acpNotificationHandler.test.ts @@ -180,6 +180,158 @@ describe("acpNotificationHandler", () => { }); }); + it("attributes a completed live tool response to the matching request when a sibling is still executing", async () => { + // Regression: with two sibling tool requests, completing the first + // while the second is still unpaired must label the response with the + // first request's name. Previously the live path used the latest + // unpaired request, which could swap names across siblings. + registerPreparedSession("acp-session", "goose", "/Users/test"); + setActiveMessageId("acp-session", "assistant-1"); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tool-a", + title: "read_file", + rawInput: { path: "/tmp/notes.md" }, + }, + } as never); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tool-b", + title: "grep", + rawInput: { pattern: "TODO" }, + }, + } as never); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tool-a", + status: "completed", + content: [ + { + type: "content", + content: { type: "text", text: "file contents" }, + }, + ], + }, + } as never); + + const [message] = useChatStore.getState().messagesBySession["acp-session"]; + expect(message.content.map((block) => block.type)).toEqual([ + "toolRequest", + "toolRequest", + "toolResponse", + ]); + expect(message.content[0]).toMatchObject({ + type: "toolRequest", + id: "tool-a", + name: "read_file", + status: "completed", + }); + expect(message.content[1]).toMatchObject({ + type: "toolRequest", + id: "tool-b", + name: "grep", + status: "executing", + }); + expect(message.content[2]).toMatchObject({ + type: "toolResponse", + id: "tool-a", + name: "read_file", + result: "file contents", + isError: false, + }); + }); + + it("keeps a late live tool response from moving the streaming pointer back to its owner message", async () => { + registerPreparedSession("acp-session", "goose", "/Users/test"); + setActiveMessageId("acp-session", "assistant-1"); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tool-a", + title: "read_file", + rawInput: { path: "/tmp/notes.md" }, + }, + } as never); + + const beforeMessages = + useChatStore.getState().messagesBySession["acp-session"] ?? []; + useChatStore.setState((state) => ({ + ...state, + messagesBySession: { + ...state.messagesBySession, + "acp-session": [ + ...beforeMessages, + { + id: "assistant-2", + role: "assistant", + created: Date.now(), + content: [], + metadata: { + userVisible: true, + agentVisible: true, + completionStatus: "inProgress", + }, + }, + ], + }, + })); + useChatStore.getState().setStreamingMessageId("acp-session", "assistant-2"); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tool-a", + status: "completed", + content: [ + { + type: "content", + content: { type: "text", text: "file contents" }, + }, + ], + }, + } as never); + + expect( + useChatStore.getState().getSessionRuntime("acp-session") + .streamingMessageId, + ).toBe("assistant-2"); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: "Continuing with the answer.", + }, + }, + } as never); + + const messages = useChatStore.getState().messagesBySession["acp-session"]; + const ownerMessage = messages.find((m) => m.id === "assistant-1"); + const currentMessage = messages.find((m) => m.id === "assistant-2"); + + expect(ownerMessage?.content.map((block) => block.type)).toEqual([ + "toolRequest", + "toolResponse", + ]); + expect(currentMessage?.content).toEqual([ + { type: "text", text: "Continuing with the answer." }, + ]); + }); + it("preserves structured tool output when ACP provides rawOutput", async () => { registerPreparedSession( "acp-session", @@ -555,4 +707,183 @@ describe("acpNotificationHandler", () => { isError: false, }); }); + + it("threads tool chain summary onto the streaming tool request (live)", async () => { + registerPreparedSession("acp-session", "goose", "/tmp"); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tc-1", + title: "running ls", + }, + } as never); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tc-2", + title: "running pwd", + }, + } as never); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tc-1", + _meta: { + goose: { + toolChainSummary: { + summary: "inspected working directory", + count: 2, + }, + }, + }, + }, + } as never); + + const messages = useChatStore.getState().messagesBySession["acp-session"]; + expect(messages).toBeTruthy(); + const toolReqs = + messages?.flatMap((m) => + m.content.filter((c) => c.type === "toolRequest"), + ) ?? []; + const first = toolReqs.find( + (c) => c.type === "toolRequest" && c.id === "tc-1", + ); + const second = toolReqs.find( + (c) => c.type === "toolRequest" && c.id === "tc-2", + ); + expect(first?.type === "toolRequest" && first.chainSummary).toEqual({ + summary: "inspected working directory", + count: 2, + }); + expect( + second?.type === "toolRequest" && second.chainSummary, + ).toBeUndefined(); + }); + + it("threads tool chain summary onto the first tool call even when the agent has moved to the next assistant message (live)", async () => { + registerPreparedSession("acp-session", "goose", "/tmp"); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tc-1", + title: "running ls", + }, + } as never); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call", + toolCallId: "tc-2", + title: "running pwd", + }, + } as never); + + // Simulate the agent moving on to the next assistant message: the + // streamingMessageId now points to a brand-new message that does not + // contain the original tool requests. This is what happens in practice + // by the time the chain summary task fires (after all tool responses + // have been emitted and the next agent turn has begun). + const beforeMessages = + useChatStore.getState().messagesBySession["acp-session"] ?? []; + const newAssistantId = "next-assistant-msg"; + useChatStore.setState((state) => ({ + ...state, + messagesBySession: { + ...state.messagesBySession, + "acp-session": [ + ...beforeMessages, + { + id: newAssistantId, + role: "assistant", + created: Date.now(), + content: [{ type: "text", text: "ok" }], + metadata: { + userVisible: true, + agentVisible: true, + completionStatus: "inProgress", + }, + }, + ], + }, + })); + useChatStore + .getState() + .setStreamingMessageId("acp-session", newAssistantId); + + await handleSessionNotification({ + sessionId: "acp-session", + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tc-1", + _meta: { + goose: { + toolChainSummary: { + summary: "inspected working directory", + count: 2, + }, + }, + }, + }, + } as never); + + const messages = useChatStore.getState().messagesBySession["acp-session"]; + const toolReqs = + messages?.flatMap((m) => + m.content.filter((c) => c.type === "toolRequest"), + ) ?? []; + const first = toolReqs.find( + (c) => c.type === "toolRequest" && c.id === "tc-1", + ); + expect(first?.type === "toolRequest" && first.chainSummary).toEqual({ + summary: "inspected working directory", + count: 2, + }); + // The new assistant message must not have been mutated to absorb the + // chain summary (regression guard: it doesn't own the tool request). + const nextMsg = messages?.find((m) => m.id === newAssistantId); + expect(nextMsg?.content.some((c) => c.type === "toolRequest")).toBe(false); + }); + + it("attaches tool chain summary on initial tool_call during replay", async () => { + const replaySessionId = "replay-chain-summary-session"; + useChatStore.setState({ + loadingSessionIds: new Set([replaySessionId]), + }); + + await handleSessionNotification({ + sessionId: replaySessionId, + update: { + sessionUpdate: "tool_call", + toolCallId: "tc-1", + title: "ran two things", + _meta: { + goose: { + toolChainSummary: { + summary: "applied dark mode polish", + count: 4, + }, + }, + }, + }, + } as never); + + const buffer = getReplayBuffer(replaySessionId); + expect(buffer).toBeTruthy(); + const tc = buffer + ?.flatMap((m) => m.content) + .find((c) => c.type === "toolRequest" && c.id === "tc-1"); + expect(tc?.type === "toolRequest" && tc.chainSummary).toEqual({ + summary: "applied dark mode polish", + count: 4, + }); + }); }); diff --git a/ui/goose2/src/shared/api/acpNotificationHandler.ts b/ui/goose2/src/shared/api/acpNotificationHandler.ts index 42c350b2..2f60a914 100644 --- a/ui/goose2/src/shared/api/acpNotificationHandler.ts +++ b/ui/goose2/src/shared/api/acpNotificationHandler.ts @@ -4,10 +4,7 @@ import type { } from "@agentclientprotocol/sdk"; import { useChatStore } from "@/features/chat/stores/chatStore"; import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore"; -import { - getBufferedMessage, - findLatestUnpairedToolRequest, -} from "@/features/chat/hooks/replayBuffer"; +import { getBufferedMessage } from "@/features/chat/hooks/replayBuffer"; import type { ToolCallLocation, ToolCallStatus, @@ -31,7 +28,10 @@ import { } from "./acpReplayAssistant"; import { getReplayCreated, getReplayMessageId } from "./acpReplayMetadata"; import { handleSessionInfoUpdate } from "./acpSessionInfoUpdate"; -import { getToolCallIdentity } from "./acpToolCallIdentity"; +import { + getToolCallIdentity, + getToolChainSummary, +} from "./acpToolCallIdentity"; import { perfLog } from "@/shared/lib/perfLog"; // Pre-set message ID for the next live stream per session. @@ -214,6 +214,7 @@ function handleReplay(sessionId: string, update: SessionUpdate): void { case "tool_call": { const created = getReplayCreated(update); const identity = getToolCallIdentity(update); + const chainSummary = getToolChainSummary(update); const msg = ensureReplayAssistantMessage( sessionId, getReplayMessageId(update), @@ -228,6 +229,7 @@ function handleReplay(sessionId: string, update: SessionUpdate): void { status: "executing", ...toolCallUpdatePatch(update), startedAt: created ?? Date.now(), + ...(chainSummary ? { chainSummary } : {}), }); break; } @@ -236,6 +238,7 @@ function handleReplay(sessionId: string, update: SessionUpdate): void { const created = getReplayCreated(update); const replayMessageId = getReplayMessageId(update); const identity = getToolCallIdentity(update); + const chainSummary = getToolChainSummary(update); const trackedMessageId = getTrackedReplayAssistantMessageId(sessionId); const replayMsg = replayMessageId ? getBufferedMessage(sessionId, replayMessageId) @@ -257,7 +260,8 @@ function handleReplay(sessionId: string, update: SessionUpdate): void { if ( update.title || Object.keys(identity).length > 0 || - Object.keys(patch).length > 0 + Object.keys(patch).length > 0 || + chainSummary ) { const tc = msg.content.find( (c) => c.type === "toolRequest" && c.id === update.toolCallId, @@ -267,6 +271,7 @@ function handleReplay(sessionId: string, update: SessionUpdate): void { ...(update.title ? { name: update.title } : {}), ...identity, ...patch, + ...(chainSummary ? { chainSummary } : {}), }); } } @@ -343,6 +348,7 @@ function handleLive(sessionId: string, update: SessionUpdate): void { case "tool_call": { const messageId = ensureLiveAssistantMessage(sessionId); const identity = getToolCallIdentity(update); + const chainSummary = getToolChainSummary(update); const toolRequest: ToolRequestContent = { type: "toolRequest", @@ -353,6 +359,7 @@ function handleLive(sessionId: string, update: SessionUpdate): void { status: "executing", ...toolCallUpdatePatch(update), startedAt: Date.now(), + ...(chainSummary ? { chainSummary } : {}), }; store.setStreamingMessageId(sessionId, messageId); store.appendToStreamingMessage(sessionId, toolRequest); @@ -360,14 +367,24 @@ function handleLive(sessionId: string, update: SessionUpdate): void { } case "tool_call_update": { - const messageId = ensureLiveAssistantMessage(sessionId); const identity = getToolCallIdentity(update); + const chainSummary = getToolChainSummary(update); + // Late-arriving updates (chain summaries, async titles) can target a + // tool call whose request lives in an older message than the currently + // streaming one. Patch the message that actually owns the tool call, + // falling back to ensureLiveAssistantMessage only if we can't find it. + const ownerMessageId = findLiveMessageIdWithToolCall( + sessionId, + update.toolCallId, + ); + const messageId = ownerMessageId ?? ensureLiveAssistantMessage(sessionId); const patch = toolCallUpdatePatch(update); if ( update.title || Object.keys(identity).length > 0 || - Object.keys(patch).length > 0 + Object.keys(patch).length > 0 || + chainSummary ) { store.updateMessage(sessionId, messageId, (msg) => ({ ...msg, @@ -378,6 +395,7 @@ function handleLive(sessionId: string, update: SessionUpdate): void { ...(update.title ? { name: update.title } : {}), ...identity, ...patch, + ...(chainSummary ? { chainSummary } : {}), } : c, ), @@ -386,12 +404,18 @@ function handleLive(sessionId: string, update: SessionUpdate): void { if (update.status === "completed" || update.status === "failed") { const toolCallStatus = toolCallStatusFromUpdate(update.status); - const streamingMessage = store.messagesBySession[sessionId]?.find( + const ownerMessage = store.messagesBySession[sessionId]?.find( (m) => m.id === messageId, ); - const toolRequest = streamingMessage - ? findLatestUnpairedToolRequest(streamingMessage.content) - : null; + // Look up the request that this update belongs to by exact id — + // sibling tools can complete out of order, so the latest unpaired + // request isn't necessarily the one we're updating. Mirrors the + // replay branch above. + const toolRequest = + ownerMessage?.content.find( + (block): block is ToolRequestContent => + block.type === "toolRequest" && block.id === update.toolCallId, + ) ?? null; store.updateMessage(sessionId, messageId, (msg) => ({ ...msg, @@ -411,13 +435,15 @@ function handleLive(sessionId: string, update: SessionUpdate): void { const toolResponse: ToolResponseContent = { type: "toolResponse", id: update.toolCallId, - name: toolRequest?.name ?? "", + name: toolRequest?.name ?? update.title ?? "", result: resultText, structuredContent: extractToolStructuredContent(update), isError: update.status === "failed", }; - store.setStreamingMessageId(sessionId, messageId); - store.appendToStreamingMessage(sessionId, toolResponse); + store.updateMessage(sessionId, messageId, (msg) => ({ + ...msg, + content: [...msg.content, toolResponse], + })); if (update.status === "completed") { attachMcpAppPayload( sessionId, @@ -509,6 +535,31 @@ function findStreamingMessageId(sessionId: string): string | null { .streamingMessageId; } +/** + * Locate the live message that owns a given tool call id by scanning + * `messagesBySession` from the most recent message backwards. Used by + * `tool_call_update` to keep late-arriving updates (chain summaries, async + * titles, status flips) anchored on the request's original message even when + * the streaming pointer has moved on to the next assistant turn. + */ +function findLiveMessageIdWithToolCall( + sessionId: string, + toolCallId: string, +): string | null { + const messages = useChatStore.getState().messagesBySession[sessionId]; + if (!messages) return null; + for (let i = messages.length - 1; i >= 0; i -= 1) { + if ( + messages[i].content.some( + (c) => c.type === "toolRequest" && c.id === toolCallId, + ) + ) { + return messages[i].id; + } + } + return null; +} + function ensureLiveAssistantMessage( sessionId: string, preferredMessageId?: string | null, diff --git a/ui/goose2/src/shared/api/acpToolCallIdentity.ts b/ui/goose2/src/shared/api/acpToolCallIdentity.ts index 6d512f12..01abee68 100644 --- a/ui/goose2/src/shared/api/acpToolCallIdentity.ts +++ b/ui/goose2/src/shared/api/acpToolCallIdentity.ts @@ -5,6 +5,11 @@ export interface ToolCallIdentity { extensionName?: string; } +export interface ToolChainSummary { + summary: string; + count: number; +} + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null; } @@ -34,3 +39,30 @@ export function getToolCallIdentity(update: SessionUpdate): ToolCallIdentity { : {}), }; } + +/** + * Extract a chain summary from `_meta.goose.toolChainSummary` of a tool-call + * SessionUpdate. Returns `undefined` when the meta is missing, malformed, or + * carries a non-positive count. + * + * The server attaches this to the FIRST tool call in a multi-tool chain once + * every step has completed; replays after reload re-emit it on the initial + * `ToolCall` notification so the chain header is correct on first paint. + */ +export function getToolChainSummary( + update: SessionUpdate, +): ToolChainSummary | undefined { + if (!isRecord(update._meta)) return undefined; + const goose = update._meta.goose; + if (!isRecord(goose)) return undefined; + const chain = goose.toolChainSummary; + if (!isRecord(chain)) return undefined; + + const summary = chain.summary; + const count = chain.count; + if (typeof summary !== "string" || summary.length === 0) return undefined; + if (typeof count !== "number" || !Number.isFinite(count) || count <= 0) { + return undefined; + } + return { summary, count: Math.trunc(count) }; +} diff --git a/ui/goose2/src/shared/i18n/locales/en/chat.json b/ui/goose2/src/shared/i18n/locales/en/chat.json index 763a0df9..58e9a3f0 100644 --- a/ui/goose2/src/shared/i18n/locales/en/chat.json +++ b/ui/goose2/src/shared/i18n/locales/en/chat.json @@ -193,13 +193,44 @@ "tools": { "content": "Content", "fileNotFound": "File not found: {{path}}", + "inputSummary": { + "command": "Command", + "line": "Line", + "path": "Path", + "query": "Query", + "resource": "Resource", + "tool": "Tool", + "workingDirectory": "Working directory" + }, "moreOutputs": "More outputs ({{count}})", "openFile": "Open file", "openFolder": "Open folder", + "openNamed": "Open {{name}}", "openPath": "Open path", "pathOutsideRoots": "Path is outside allowed roots", "structuredContent": "Structured content", "structuredOutput": "Structured output", "structuredOutputLines": "{{count}} lines" + }, + "tool_chain": { + "summary": { + "active": "working", + "reviewing_files": "reviewing files", + "running_commands": "running commands", + "checking_resources": "checking resources", + "updating_files": "updating files", + "steps_one": "{{count}} step", + "steps_other": "{{count}} steps" + }, + "internalSteps": { + "show": "Show internal steps ({{count}})", + "hide": "Hide internal steps ({{count}})" + }, + "title": { + "active": "working through {{count}} step", + "active_other": "working through {{count}} steps", + "labeled": "{{label}} ({{count}} step)", + "labeled_other": "{{label}} ({{count}} steps)" + } } } diff --git a/ui/goose2/src/shared/i18n/locales/es/chat.json b/ui/goose2/src/shared/i18n/locales/es/chat.json index 2bd7612b..e710e116 100644 --- a/ui/goose2/src/shared/i18n/locales/es/chat.json +++ b/ui/goose2/src/shared/i18n/locales/es/chat.json @@ -193,13 +193,44 @@ "tools": { "content": "Contenido", "fileNotFound": "Archivo no encontrado: {{path}}", + "inputSummary": { + "command": "Comando", + "line": "Línea", + "path": "Ruta", + "query": "Búsqueda", + "resource": "Recurso", + "tool": "Herramienta", + "workingDirectory": "Directorio de trabajo" + }, "moreOutputs": "Más salidas ({{count}})", "openFile": "Abrir archivo", "openFolder": "Abrir carpeta", + "openNamed": "Abrir {{name}}", "openPath": "Abrir ruta", "pathOutsideRoots": "La ruta está fuera de las raíces permitidas de proyecto/artefactos.", "structuredContent": "Contenido estructurado", "structuredOutput": "Salida estructurada", "structuredOutputLines": "{{count}} líneas" + }, + "tool_chain": { + "summary": { + "active": "trabajando", + "reviewing_files": "revisando archivos", + "running_commands": "ejecutando comandos", + "checking_resources": "consultando recursos", + "updating_files": "actualizando archivos", + "steps_one": "{{count}} paso", + "steps_other": "{{count}} pasos" + }, + "internalSteps": { + "show": "Mostrar pasos internos ({{count}})", + "hide": "Ocultar pasos internos ({{count}})" + }, + "title": { + "active": "trabajando en {{count}} paso", + "active_other": "trabajando en {{count}} pasos", + "labeled": "{{label}} ({{count}} paso)", + "labeled_other": "{{label}} ({{count}} pasos)" + } } } diff --git a/ui/goose2/src/shared/types/messages.ts b/ui/goose2/src/shared/types/messages.ts index a859d1c6..973aaefa 100644 --- a/ui/goose2/src/shared/types/messages.ts +++ b/ui/goose2/src/shared/types/messages.ts @@ -91,6 +91,13 @@ export type MessageCompletionStatus = | "error" | "stopped"; +export interface ToolChainSummary { + /** Lowercase phrase covering the chain's tool calls (e.g. "applied dark mode polish"). */ + summary: string; + /** Number of tool calls the summary covers. */ + count: number; +} + export interface ToolRequestContent { type: "toolRequest"; id: string; @@ -104,6 +111,12 @@ export interface ToolRequestContent { /** Epoch ms when the tool call started executing (set on event receipt). */ startedAt?: number; annotations?: ContentAnnotations; + /** + * Server-generated summary of a multi-tool chain that starts at this tool + * call. Only set on the FIRST tool call of a chain (>= 2 tools); the rest of + * the chain has this field undefined. + */ + chainSummary?: ToolChainSummary; } export interface ToolResponseContent { diff --git a/ui/goose2/src/shared/ui/ai-elements/tool.tsx b/ui/goose2/src/shared/ui/ai-elements/tool.tsx index 82713363..c294cd1b 100644 --- a/ui/goose2/src/shared/ui/ai-elements/tool.tsx +++ b/ui/goose2/src/shared/ui/ai-elements/tool.tsx @@ -1,3 +1,4 @@ +import { useControllableState } from "@radix-ui/react-use-controllable-state"; import { Collapsible, CollapsibleContent, @@ -13,26 +14,66 @@ import { WrenchIcon, XCircleIcon, } from "lucide-react"; +import type { LucideIcon } from "lucide-react"; import type { ComponentProps, ReactNode } from "react"; -import { isValidElement } from "react"; +import { + createContext, + isValidElement, + useContext, + useEffect, + useMemo, + useRef, + useState, +} from "react"; import { CodeBlock } from "./code-block"; export type ToolProps = ComponentProps; -export const Tool = ({ className, ...props }: ToolProps) => ( - -); +interface ToolContextValue { + isOpen: boolean; + setIsOpen: (open: boolean) => void; +} + +const ToolContext = createContext(null); + +export const Tool = ({ + className, + open, + defaultOpen = false, + onOpenChange, + ...props +}: ToolProps) => { + const [isOpen, setIsOpen] = useControllableState({ + defaultProp: defaultOpen, + onChange: onOpenChange, + prop: open, + }); + const value = useMemo(() => ({ isOpen, setIsOpen }), [isOpen, setIsOpen]); + + return ( + + + + ); +}; export type ToolPart = ToolUIPart | DynamicToolUIPart; export type ToolHeaderProps = { - title?: string; + title?: ReactNode; className?: string; showIcon?: boolean; + showStatusBadge?: boolean; + /** When false, hides the trailing disclosure chevron in the header. */ + showChevron?: boolean; + splitTrigger?: boolean; + layout?: "fill" | "fit"; elapsedSeconds?: number; } & ( | { type: ToolUIPart["type"]; state: ToolUIPart["state"]; toolName?: never } @@ -53,21 +94,55 @@ const statusLabels: Record = { "output-error": "Error", }; -const statusIcons: Record = { - "approval-requested": , - "approval-responded": , - "input-available": , - "input-streaming": , - "output-available": , - "output-denied": , - "output-error": , +const statusIconComponents: Record = { + "approval-requested": ClockIcon, + "approval-responded": CheckCircleIcon, + "input-available": ClockIcon, + "input-streaming": CircleIcon, + "output-available": CheckCircleIcon, + "output-denied": XCircleIcon, + "output-error": XCircleIcon, }; -export const getStatusBadge = (status: ToolPart["state"]) => { +const statusIconClasses: Record = { + "approval-requested": "text-yellow-600", + "approval-responded": "text-blue-600", + "input-available": "animate-pulse", + "input-streaming": "", + "output-available": "text-green-600", + "output-denied": "text-orange-600", + "output-error": "text-red-600", +}; + +export const ToolStatusIcon = ({ + status, + className, +}: { + status: ToolPart["state"]; + className?: string; +}) => { + const Icon = statusIconComponents[status]; + return ( +