tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

fix: Dedupe duplicate signed thinking blocks to fix Anthropic 400 (#10080) (#10083)

Browse Source 
Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
Michael Neale
2026-07-08 16:48:18 -07:00
committed by GitHub
parent e6be2e9392
commit e09289cbac
3 changed files with 428 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose-provider-types/src/conversation.rs
+301
View File
@@ -135,6 +135,10 @@ impl Conversation {
self.0.pop()
}
pub fn remove(&mut self, index: usize) -> Message {
self.0.remove(index)
}
pub fn truncate(&mut self, len: usize) {
self.0.truncate(len);
}
@@ -247,6 +251,7 @@ fn fix_messages(messages: Vec<Message>) -> (Vec<Message>, Vec<String>) {
fix_empty_tool_results,
fix_tool_calling,
merge_consecutive_messages,
dedupe_signed_thinking,
fix_lead_trail,
populate_if_empty,
]
@@ -502,6 +507,71 @@ pub fn merge_consecutive_messages(messages: Vec<Message>) -> (Vec<Message>, Vec<
(merged_messages, issues)
}
/// Signed thinking carries a signature; redacted thinking is always signed.
/// Signed blocks must be replayed exactly; unsigned reasoning summaries need not.
fn is_signed_thinking(content: &MessageContent) -> bool {
match content {
MessageContent::Thinking(t) => !t.signature.is_empty(),
MessageContent::RedactedThinking(_) => true,
_ => false,
}
}
/// Drops duplicate signed thinking blocks, keeping the first occurrence. Some
/// signed-replay APIs (like Anthropic) reject a request that repeats the same
/// signed block more than once.
///
/// Duplicates arise two ways, both handled here:
/// - Within one assistant message, when a standalone thinking message is
/// merged with a tool-call message that re-embedded the same thinking.
/// - Across assistant messages, when the agent splits one provider turn into
/// several tool-call messages (interleaved with tool results) that each
/// carry a copy of the turn's signed thinking.
///
/// The `seen` set spans the whole conversation. A signed block carries a
/// cryptographic signature unique to its generation, so an exact (text +
/// signature) match can only be the same turn's thinking copied onto split
/// messages — never two genuinely distinct thoughts. Unsigned reasoning
/// summaries are left untouched, since providers like Kimi/DeepSeek require
/// them echoed on every tool-call message.
///
/// This runs before any provider formatter, so it covers every Claude transport
/// (direct Anthropic, Bedrock, Databricks, Vertex) in one place.
fn dedupe_signed_thinking(messages: Vec<Message>) -> (Vec<Message>, Vec<String>) {
let mut issues = Vec::new();
let mut seen: Vec<MessageContent> = Vec::new();
let fixed_messages = messages
.into_iter()
.map(|mut message| {
if message.role != Role::Assistant {
return message;
}
let original_len = message.content.len();
let mut deduped: Vec<MessageContent> = Vec::with_capacity(original_len);
for content in &message.content {
let is_signed = is_signed_thinking(content);
if is_signed && seen.contains(content) {
continue;
}
if is_signed {
seen.push(content.clone());
}
deduped.push(content.clone());
}
if deduped.len() != original_len {
issues.push("Removed duplicate signed thinking block".to_string());
message.content = deduped;
}
message
})
.collect();
(fixed_messages, issues)
}
fn has_tool_response(message: &Message) -> bool {
message
.content
@@ -1304,6 +1374,237 @@ mod tests {
assert!(!fixed_messages[5].metadata.agent_visible);
}
#[test]
fn test_dedupes_duplicate_signed_thinking_around_tool_call() {
use crate::conversation::message::MessageContent;
use rmcp::model::Content;
// Reproduces the Anthropic 400 scenario: a standalone signed thinking
// message immediately followed by an assistant message that repeats the
// same signed thinking plus a tool_use. After merging consecutive
// assistant messages these become two adjacent identical thinking blocks.
let messages = vec![
Message::user().with_text("Do the thing"),
Message::assistant().with_thinking("Let me think about this", "sig-1"),
Message::assistant()
.with_thinking("Let me think about this", "sig-1")
.with_tool_request(
"tool_1",
Ok(CallToolRequestParams::new("do_thing").with_arguments(object!({"x": 1}))),
),
Message::user().with_tool_response(
"tool_1",
Ok(rmcp::model::CallToolResult::success(vec![Content::text(
"done",
)])),
),
Message::user().with_text("Now continue"),
];
let (fixed, issues) = fix_conversation(Conversation::new_unvalidated(messages));
assert!(
issues
.iter()
.any(|i| i == "Removed duplicate signed thinking block"),
"expected dedupe issue, got: {:?}",
issues
);
let fixed_messages = fixed.messages();
let assistant = fixed_messages
.iter()
.find(|m| {
m.role == Role::Assistant
&& m.content
.iter()
.any(|c| matches!(c, MessageContent::ToolRequest(_)))
})
.expect("assistant tool-call message should exist");
let thinking_count = assistant
.content
.iter()
.filter(|c| {
matches!(
c,
MessageContent::Thinking(_) | MessageContent::RedactedThinking(_)
)
})
.count();
assert_eq!(
thinking_count, 1,
"duplicate signed thinking should be collapsed to one block"
);
}
#[test]
fn test_keeps_distinct_signed_thinking_blocks_in_assistant_message() {
use crate::conversation::message::MessageContent;
use rmcp::model::Content;
// Distinct signed thinking blocks (different signatures) must be preserved.
let messages = vec![
Message::user().with_text("Do the thing"),
Message::assistant()
.with_thinking("First thought", "sig-A")
.with_thinking("Second thought", "sig-B")
.with_tool_request(
"tool_1",
Ok(CallToolRequestParams::new("do_thing").with_arguments(object!({"x": 1}))),
),
Message::user().with_tool_response(
"tool_1",
Ok(rmcp::model::CallToolResult::success(vec![Content::text(
"done",
)])),
),
Message::user().with_text("Now continue"),
];
let (fixed, issues) = fix_conversation(Conversation::new_unvalidated(messages));
assert!(
!issues
.iter()
.any(|i| i == "Removed duplicate signed thinking block"),
"should not dedupe distinct thinking blocks, got: {:?}",
issues
);
let fixed_messages = fixed.messages();
let thinking_count = fixed_messages[1]
.content
.iter()
.filter(|c| {
matches!(
c,
MessageContent::Thinking(_) | MessageContent::RedactedThinking(_)
)
})
.count();
assert_eq!(thinking_count, 2, "distinct thinking blocks must be kept");
}
#[test]
fn test_keeps_duplicate_unsigned_thinking_blocks() {
use crate::conversation::message::MessageContent;
// Unsigned thinking (reasoning summaries from non-Anthropic providers)
// can legitimately repeat and must not be dropped, since only signed
// blocks trigger the Anthropic exact-replay 400.
let messages = vec![
Message::user().with_text("Do the thing"),
Message::assistant()
.with_thinking("same reasoning", "")
.with_thinking("same reasoning", ""),
Message::user().with_text("Now continue"),
];
let (fixed, issues) = fix_conversation(Conversation::new_unvalidated(messages));
assert!(
!issues
.iter()
.any(|i| i == "Removed duplicate signed thinking block"),
"unsigned thinking must not be deduped, got: {:?}",
issues
);
let fixed_messages = fixed.messages();
let thinking_count = fixed_messages
.iter()
.flat_map(|m| m.content.iter())
.filter(|c| matches!(c, MessageContent::Thinking(_)))
.count();
assert_eq!(
thinking_count, 2,
"duplicate unsigned thinking blocks must be kept"
);
}
#[test]
fn test_dedupes_signed_thinking_across_split_tool_messages() {
use crate::conversation::message::MessageContent;
use rmcp::model::Content;
// The agent splits one provider turn with multiple tool calls into one
// assistant message per call (interleaved with tool results), each
// carrying the same signed thinking. merge_consecutive_messages cannot
// merge them because tool results sit between, so the dedupe must span
// messages and keep the signed block only on the first.
let messages = vec![
Message::user().with_text("Use both tools"),
Message::assistant()
.with_thinking("multi-tool reasoning", "sig-1")
.with_tool_request(
"call_1",
Ok(CallToolRequestParams::new("tool_a").with_arguments(object!({"p": 1}))),
),
Message::user().with_tool_response(
"call_1",
Ok(rmcp::model::CallToolResult::success(vec![Content::text(
"ok",
)])),
),
Message::assistant()
.with_thinking("multi-tool reasoning", "sig-1")
.with_tool_request(
"call_2",
Ok(CallToolRequestParams::new("tool_b").with_arguments(object!({"p": 2}))),
),
Message::user().with_tool_response(
"call_2",
Ok(rmcp::model::CallToolResult::success(vec![Content::text(
"ok",
)])),
),
Message::user().with_text("Now continue"),
];
let (fixed, issues) = fix_conversation(Conversation::new_unvalidated(messages));
assert!(
issues
.iter()
.any(|i| i == "Removed duplicate signed thinking block"),
"expected cross-message dedupe issue, got: {:?}",
issues
);
let fixed_messages = fixed.messages();
let total_thinking = fixed_messages
.iter()
.flat_map(|m| m.content.iter())
.filter(|c| {
matches!(
c,
MessageContent::Thinking(_) | MessageContent::RedactedThinking(_)
)
})
.count();
assert_eq!(
total_thinking, 1,
"the repeated signed block must survive only once across the turn"
);
let total_tool_requests = fixed_messages
.iter()
.flat_map(|m| m.content.iter())
.filter(|c| matches!(c, MessageContent::ToolRequest(_)))
.count();
assert_eq!(total_tool_requests, 2, "both tool calls must be preserved");
// The first split message keeps the thinking; the second loses it.
assert!(
fixed_messages[1]
.content
.iter()
.any(|c| matches!(c, MessageContent::Thinking(_))),
"first split message keeps signed thinking"
);
}
#[test]
fn test_push_coalesces_thinking_deltas() {
use crate::conversation::message::MessageContent;
crates/goose/src/agents/agent.rs
+28 -32
View File
@@ -2291,9 +2291,16 @@ impl Agent {
}
}
// Preserve thinking/reasoning content from the original response.
// Gemini (and other thinking models) require thinking to be echoed back.
// Kimi/DeepSeek require reasoning_content on assistant tool call messages.
// Thinking/reasoning belongs on the tool-call messages, not also
// as a separate standalone message: Gemini and Kimi/DeepSeek
// require it echoed on each assistant tool-call message, and the
// provider formatters reconstruct per-provider shape from there.
// Storing it both standalone AND on the tool-call message
// duplicates it; once merge_consecutive_messages glues the adjacent
// standalone and tool-call messages together, the duplicate signed
// blocks make Anthropic reject the turn with a 400. So the thinking
// is carried onto the split request messages below and never kept
// as a redundant standalone message.
let direct_thinking: Vec<MessageContent> = response
.content
.iter()
@@ -2306,36 +2313,25 @@ impl Agent {
})
.cloned()
.collect();
if !direct_thinking.is_empty() {
let thinking_msg = Message::new(
response.role.clone(),
response.created,
direct_thinking.clone(),
)
.with_id(format!("msg_{}", Uuid::new_v4()));
messages_to_add.push(thinking_msg);
}
// When thinking arrived in an earlier stream chunk (stored as a
// thinking-only message) and this chunk has only tool calls,
// reuse that thinking so each split request_msg carries it.
// When thinking arrived in an earlier stream chunk it was stored as
// a standalone thinking-only message; reuse that thinking on the
// tool-call messages and drop the standalone so it isn't duplicated.
let response_thinking = if direct_thinking.is_empty() {
messages_to_add
.messages()
.iter()
.rev()
.find(|m| {
m.role == response.role
&& !m.content.is_empty()
&& m.content.iter().all(|c| {
matches!(
c,
MessageContent::Thinking(_)
| MessageContent::RedactedThinking(_)
)
})
})
.map(|m| m.content.clone())
.unwrap_or_default()
let prior = messages_to_add.messages().iter().rposition(|m| {
m.role == response.role
&& !m.content.is_empty()
&& m.content.iter().all(|c| {
matches!(
c,
MessageContent::Thinking(_)
| MessageContent::RedactedThinking(_)
)
})
});
match prior {
Some(idx) => messages_to_add.remove(idx).content,
None => Vec::new(),
}
} else {
direct_thinking
};
crates/goose/tests/agent.rs
+99
View File
@@ -1988,6 +1988,105 @@ mod tests {
Ok(())
}
/// Regression for the Anthropic 400: signed thinking arriving in a
/// separate chunk before the tool calls must be stored once per
/// tool-call message and never as an extra standalone message. When the
/// Anthropic formatter serializes the persisted history, each assistant
/// turn must carry exactly one thinking block — a duplicate signed block
/// is rejected with `thinking blocks ... cannot be modified`.
#[tokio::test]
async fn test_signed_thinking_not_duplicated_for_anthropic() -> Result<()> {
use goose_providers::formats::anthropic::format_messages as anthropic_format;
let temp_dir = tempfile::tempdir()?;
let session_manager = Arc::new(SessionManager::new(temp_dir.path().to_path_buf()));
let config = AgentConfig::new(
session_manager.clone(),
PermissionManager::instance(),
None,
GooseMode::Auto,
true,
GoosePlatform::GooseCli,
);
let agent = Agent::with_config(config);
let provider = Arc::new(MultiToolThinkingProvider::new());
let session = session_manager
.create_session(
PathBuf::default(),
"anthropic-signed-thinking-test".to_string(),
SessionType::Hidden,
GooseMode::default(),
)
.await?;
let session_id = session.id.clone();
agent
.update_provider(provider, ModelConfig::new("mock-model"), &session_id)
.await?;
let session_config = SessionConfig {
id: session_id.clone(),
schedule_id: None,
max_turns: Some(2),
retry_config: None,
};
let reply_stream = agent
.reply(
Message::user().with_text("Use both tools"),
session_config,
None,
)
.await?;
tokio::pin!(reply_stream);
while let Some(event) = reply_stream.next().await {
event?;
}
let reloaded = session_manager.get_session(&session_id, true).await?;
let messages = reloaded
.conversation
.expect("should have conversation")
.messages()
.to_vec();
// No standalone thinking-only assistant message should be persisted —
// thinking lives on the tool-call messages.
let standalone_thinking = messages.iter().any(|m| {
m.role == rmcp::model::Role::Assistant
&& !m.content.is_empty()
&& m.content
.iter()
.all(|c| matches!(c, MessageContent::Thinking(_)))
});
assert!(
!standalone_thinking,
"thinking must not be persisted as a standalone message: {messages:#?}"
);
// Every serialized Anthropic assistant message must contain at most
// one thinking block; a duplicate is what triggers the 400.
let spec = anthropic_format(&messages);
for msg in &spec {
if msg.get("role") == Some(&serde_json::json!("assistant")) {
if let Some(content) = msg.get("content").and_then(|c| c.as_array()) {
let thinking_blocks = content
.iter()
.filter(|c| c.get("type") == Some(&serde_json::json!("thinking")))
.count();
assert!(
thinking_blocks <= 1,
"assistant message has {thinking_blocks} thinking blocks, \
Anthropic rejects duplicates: {msg}"
);
}
}
}
Ok(())
}
}
#[cfg(test)]