From 1aa07bb63a9b7cd5725fe590886fc423cf654cff Mon Sep 17 00:00:00 2001 From: Kevin Lam Date: Thu, 2 Apr 2026 06:48:31 -0700 Subject: [PATCH] fix(acp): avoid duplicate thought chunks around tool calls (#8215) Signed-off-by: Kevin Lam --- crates/goose/src/agents/agent.rs | 29 +++++++++++-- crates/goose/src/agents/reply_parts.rs | 60 +++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 06b1d00f..ac8c6295 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -395,10 +395,12 @@ impl Agent { &self, response: &Message, tools: &[rmcp::model::Tool], + suppress_replayed_thinking: bool, ) -> ToolCategorizeResult { // Categorize tool requests - let (frontend_requests, remaining_requests, filtered_response) = - self.categorize_tool_requests(response, tools).await; + let (frontend_requests, remaining_requests, filtered_response) = self + .categorize_tool_requests(response, tools, suppress_replayed_thinking) + .await; ToolCategorizeResult { frontend_requests, @@ -1224,6 +1226,11 @@ impl Agent { let mut did_recovery_compact_this_iteration = false; let mut exit_chat = false; + // Track whether this provider turn has already emitted visible + // thinking so a later tool-call chunk can suppress replayed + // reasoning without hiding final-only non-streaming thoughts. + let mut surfaced_thinking_in_turn = false; + while let Some(next) = stream.next().await { if is_token_cancelled(&cancel_token) || exit_chat { break; @@ -1242,7 +1249,23 @@ impl Agent { frontend_requests, remaining_requests, filtered_response, - } = self.categorize_tools(&response, &tools).await; + } = self + .categorize_tools( + &response, + &tools, + surfaced_thinking_in_turn, + ) + .await; + + surfaced_thinking_in_turn |= filtered_response.content.iter().any( + |content| { + matches!( + content, + MessageContent::Thinking(_) + | MessageContent::RedactedThinking(_) + ) + }, + ); yield AgentEvent::Message(filtered_response.clone()); tokio::task::yield_now().await; diff --git a/crates/goose/src/agents/reply_parts.rs b/crates/goose/src/agents/reply_parts.rs index f7a96157..f6805262 100644 --- a/crates/goose/src/agents/reply_parts.rs +++ b/crates/goose/src/agents/reply_parts.rs @@ -339,6 +339,7 @@ impl Agent { &self, response: &Message, tools: &[Tool], + suppress_replayed_thinking: bool, ) -> (Vec, Vec, Message) { // First collect all tool requests with coercion applied let tool_requests: Vec = response @@ -367,7 +368,16 @@ impl Agent { }) .collect(); - // Create a filtered message with frontend tool requests removed + let has_tool_requests = !tool_requests.is_empty(); + let should_suppress_replayed_thinking = suppress_replayed_thinking && has_tool_requests; + + // Create a filtered message with frontend tool requests removed. + // When a response contains tool calls, keep reasoning in the original + // message for provider/state purposes but only suppress it from the + // user-visible filtered message if the caller already surfaced + // thinking earlier in this provider turn. That avoids replaying full + // accumulated reasoning after streamed thought chunks while still + // preserving final-only non-streaming thoughts. let mut filtered_content = Vec::new(); let mut tool_request_index = 0; @@ -389,6 +399,8 @@ impl Agent { } } } + MessageContent::Thinking(_) | MessageContent::RedactedThinking(_) + if should_suppress_replayed_thinking => {} _ => { filtered_content.push(content.clone()); } @@ -662,6 +674,52 @@ mod tests { ); } + #[tokio::test] + async fn categorize_tool_requests_keeps_thinking_when_not_previously_streamed() { + let agent = crate::agents::Agent::new(); + let response = Message::assistant() + .with_thinking("final-only reasoning", "") + .with_tool_request( + "tool-1", + Ok(rmcp::model::CallToolRequestParams::new("test_tool")), + ); + + let (_frontend_requests, other_requests, filtered_message) = + agent.categorize_tool_requests(&response, &[], false).await; + + assert_eq!(other_requests.len(), 1); + assert_eq!(filtered_message.content.len(), 2); + assert!(matches!( + filtered_message.content[0], + MessageContent::Thinking(_) + )); + assert!(matches!( + filtered_message.content[1], + MessageContent::ToolRequest(_) + )); + } + + #[tokio::test] + async fn categorize_tool_requests_drops_replayed_thinking_after_streaming() { + let agent = crate::agents::Agent::new(); + let response = Message::assistant() + .with_thinking("replayed reasoning", "") + .with_tool_request( + "tool-1", + Ok(rmcp::model::CallToolRequestParams::new("test_tool")), + ); + + let (_frontend_requests, other_requests, filtered_message) = + agent.categorize_tool_requests(&response, &[], true).await; + + assert_eq!(other_requests.len(), 1); + assert_eq!(filtered_message.content.len(), 1); + assert!(matches!( + filtered_message.content[0], + MessageContent::ToolRequest(_) + )); + } + fn make_tool_with_meta(meta_json: Option) -> Tool { let mut tool = Tool::new("test_tool", "a test tool", object!({ "type": "object" })); if let Some(v) = meta_json {