From f81b44bf194927e28f2a87a0b2f091bdb8d78a4c Mon Sep 17 00:00:00 2001 From: Liqiang Liu Date: Mon, 23 Mar 2026 17:24:46 +0800 Subject: [PATCH] fix(provider): deduplicate tool results and merge consecutive tool_result blocks for Anthropic API (#1793) Anthropic API returns 400 when multiple tool_result blocks share the same tool_use_id, or when consecutive tool results are sent as separate user messages. This fix: 1. Adds ToolCallID deduplication in sanitizeHistoryForProvider (context.go) to drop duplicate tool results before sending to any provider. 2. Merges consecutive tool result messages into a single user message with multiple tool_result content blocks in Anthropic's buildRequestBody, for both "user" (with ToolCallID) and "tool" role messages. 3. Adds tests for both behaviors. Co-authored-by: Claude Opus 4.6 (1M context) --- pkg/agent/context.go | 13 +++ pkg/agent/context_test.go | 25 ++++++ pkg/providers/anthropic_messages/provider.go | 44 +++++---- .../anthropic_messages/provider_test.go | 90 +++++++++++++++++++ 4 files changed, 156 insertions(+), 16 deletions(-) diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 4f5d75a50..12e3cdd4d 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -678,8 +678,21 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message // like DeepSeek that enforce: "An assistant message with 'tool_calls' must // be followed by tool messages responding to each 'tool_call_id'." final := make([]providers.Message, 0, len(sanitized)) + seenToolCallID := make(map[string]bool) for i := 0; i < len(sanitized); i++ { msg := sanitized[i] + + // Deduplicate tool results by ToolCallID + if msg.Role == "tool" && msg.ToolCallID != "" { + if seenToolCallID[msg.ToolCallID] { + logger.DebugCF("agent", "Dropping duplicate tool result", map[string]any{ + "tool_call_id": msg.ToolCallID, + }) + continue + } + seenToolCallID[msg.ToolCallID] = true + } + if msg.Role == "assistant" && len(msg.ToolCalls) > 0 { // Collect expected tool_call IDs expected := make(map[string]bool, len(msg.ToolCalls)) diff --git a/pkg/agent/context_test.go b/pkg/agent/context_test.go index 5756ed911..0d7948eef 100644 --- a/pkg/agent/context_test.go +++ b/pkg/agent/context_test.go @@ -188,6 +188,31 @@ func TestSanitizeHistoryForProvider_PlainConversation(t *testing.T) { assertRoles(t, result, "user", "assistant", "user", "assistant") } +func TestSanitizeHistoryForProvider_DuplicateToolResults(t *testing.T) { + history := []providers.Message{ + msg("user", "do something"), + assistantWithTools("A", "B"), + toolResult("A"), + toolResult("B"), + toolResult("A"), // duplicate + toolResult("B"), // duplicate + msg("assistant", "done"), + } + + result := sanitizeHistoryForProvider(history) + if len(result) != 5 { + t.Fatalf("expected 5 messages, got %d: %+v", len(result), roles(result)) + } + assertRoles(t, result, "user", "assistant", "tool", "tool", "assistant") + // Verify the kept tool results have the correct IDs + if result[2].ToolCallID != "A" { + t.Errorf("expected tool result A, got %q", result[2].ToolCallID) + } + if result[3].ToolCallID != "B" { + t.Errorf("expected tool result B, got %q", result[3].ToolCallID) + } +} + func roles(msgs []providers.Message) []string { r := make([]string, len(msgs)) for i, m := range msgs { diff --git a/pkg/providers/anthropic_messages/provider.go b/pkg/providers/anthropic_messages/provider.go index 2b19e941a..6a1c473dd 100644 --- a/pkg/providers/anthropic_messages/provider.go +++ b/pkg/providers/anthropic_messages/provider.go @@ -188,17 +188,23 @@ func buildRequestBody( case "user": if msg.ToolCallID != "" { - // Tool result message - content := []map[string]any{ - { - "type": "tool_result", - "tool_use_id": msg.ToolCallID, - "content": msg.Content, - }, + // Tool result message — merge into previous user message if it contains tool_results + toolResultBlock := map[string]any{ + "type": "tool_result", + "tool_use_id": msg.ToolCallID, + "content": msg.Content, + } + if len(apiMessages) > 0 { + if prev, ok := apiMessages[len(apiMessages)-1].(map[string]any); ok && prev["role"] == "user" { + if content, ok := prev["content"].([]map[string]any); ok { + prev["content"] = append(content, toolResultBlock) + continue + } + } } apiMessages = append(apiMessages, map[string]any{ "role": "user", - "content": content, + "content": []map[string]any{toolResultBlock}, }) } else { // Regular user message @@ -246,17 +252,23 @@ func buildRequestBody( }) case "tool": - // Tool result (alternative format) - content := []map[string]any{ - { - "type": "tool_result", - "tool_use_id": msg.ToolCallID, - "content": msg.Content, - }, + // Tool result (alternative format) — merge into previous user message if it contains tool_results + toolResultBlock := map[string]any{ + "type": "tool_result", + "tool_use_id": msg.ToolCallID, + "content": msg.Content, + } + if len(apiMessages) > 0 { + if prev, ok := apiMessages[len(apiMessages)-1].(map[string]any); ok && prev["role"] == "user" { + if content, ok := prev["content"].([]map[string]any); ok { + prev["content"] = append(content, toolResultBlock) + continue + } + } } apiMessages = append(apiMessages, map[string]any{ "role": "user", - "content": content, + "content": []map[string]any{toolResultBlock}, }) } } diff --git a/pkg/providers/anthropic_messages/provider_test.go b/pkg/providers/anthropic_messages/provider_test.go index 8eabc15fa..39bc48117 100644 --- a/pkg/providers/anthropic_messages/provider_test.go +++ b/pkg/providers/anthropic_messages/provider_test.go @@ -562,6 +562,96 @@ func TestBuildRequestBodyEdgeCases(t *testing.T) { } } +func TestBuildRequestBody_ConsecutiveToolResultsMerged(t *testing.T) { + // Consecutive tool results (role "tool") should be merged into a single "user" message + messages := []Message{ + {Role: "user", Content: "Use tools"}, + {Role: "assistant", Content: "", ToolCalls: []ToolCall{ + {ID: "t1", Name: "tool_a", Arguments: map[string]any{"x": 1}}, + {ID: "t2", Name: "tool_b", Arguments: map[string]any{"y": 2}}, + }}, + {Role: "tool", ToolCallID: "t1", Content: "result1"}, + {Role: "tool", ToolCallID: "t2", Content: "result2"}, + } + + got, err := buildRequestBody(messages, nil, "test-model", map[string]any{"max_tokens": 8192}) + if err != nil { + t.Fatalf("buildRequestBody() error: %v", err) + } + + apiMessages, ok := got["messages"].([]any) + if !ok { + t.Fatalf("messages is not []any") + } + + // Expect: user, assistant, user (merged tool results) + if len(apiMessages) != 3 { + for i, m := range apiMessages { + t.Logf("message[%d]: %+v", i, m) + } + t.Fatalf("expected 3 API messages, got %d", len(apiMessages)) + } + + // The third message should be a user message with 2 tool_result blocks + toolResultMsg, ok := apiMessages[2].(map[string]any) + if !ok { + t.Fatalf("tool result message is not map[string]any") + } + if toolResultMsg["role"] != "user" { + t.Errorf("expected role 'user', got %v", toolResultMsg["role"]) + } + content, ok := toolResultMsg["content"].([]map[string]any) + if !ok { + t.Fatalf("content is not []map[string]any: %T", toolResultMsg["content"]) + } + if len(content) != 2 { + t.Fatalf("expected 2 tool_result blocks, got %d", len(content)) + } + if content[0]["tool_use_id"] != "t1" { + t.Errorf("first tool_result tool_use_id = %v, want t1", content[0]["tool_use_id"]) + } + if content[1]["tool_use_id"] != "t2" { + t.Errorf("second tool_result tool_use_id = %v, want t2", content[1]["tool_use_id"]) + } +} + +func TestBuildRequestBody_UserToolResultsMerged(t *testing.T) { + // Consecutive tool results using role "user" with ToolCallID should also be merged + messages := []Message{ + {Role: "user", Content: "Use tools"}, + {Role: "assistant", Content: "", ToolCalls: []ToolCall{ + {ID: "t1", Name: "tool_a", Arguments: map[string]any{"x": 1}}, + {ID: "t2", Name: "tool_b", Arguments: map[string]any{"y": 2}}, + }}, + {Role: "user", ToolCallID: "t1", Content: "result1"}, + {Role: "user", ToolCallID: "t2", Content: "result2"}, + } + + got, err := buildRequestBody(messages, nil, "test-model", map[string]any{"max_tokens": 8192}) + if err != nil { + t.Fatalf("buildRequestBody() error: %v", err) + } + + apiMessages, ok := got["messages"].([]any) + if !ok { + t.Fatalf("messages is not []any") + } + + // Expect: user, assistant, user (merged tool results) + if len(apiMessages) != 3 { + t.Fatalf("expected 3 API messages, got %d", len(apiMessages)) + } + + toolResultMsg := apiMessages[2].(map[string]any) + content, ok := toolResultMsg["content"].([]map[string]any) + if !ok { + t.Fatalf("content is not []map[string]any: %T", toolResultMsg["content"]) + } + if len(content) != 2 { + t.Fatalf("expected 2 tool_result blocks, got %d", len(content)) + } +} + // TestParseResponseBodyEdgeCases tests edge cases for parseResponseBody. func TestParseResponseBodyEdgeCases(t *testing.T) { tests := []struct {