diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 631974337..54d21d775 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -738,7 +738,54 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message final = append(final, msg) } - return final + // Third pass: ensure valid message alternation. + // Dropping messages in earlier passes can create consecutive same-role + // messages (e.g. two user messages with a dropped assistant between them). + // LLM APIs (Anthropic, DeepSeek, etc.) require strict user/assistant + // alternation and reject requests with consecutive same-role messages. + merged := make([]providers.Message, 0, len(final)) + for _, msg := range final { + // Skip empty user messages (no text content, no tool result) + if msg.Role == "user" && strings.TrimSpace(msg.Content) == "" && msg.ToolCallID == "" { + logger.DebugCF("agent", "Dropping empty user message", nil) + continue + } + + if len(merged) == 0 { + // First message must be a user message (after system is stripped). + // Drop leading assistant/tool messages. + if msg.Role != "user" { + logger.DebugCF("agent", "Dropping leading non-user message", map[string]any{ + "role": msg.Role, + }) + continue + } + merged = append(merged, msg) + continue + } + + prev := &merged[len(merged)-1] + + // Merge consecutive user text messages (not tool results). + if msg.Role == "user" && msg.ToolCallID == "" && + prev.Role == "user" && prev.ToolCallID == "" { + if prev.Content != "" && msg.Content != "" { + prev.Content = prev.Content + "\n" + msg.Content + } else if msg.Content != "" { + prev.Content = msg.Content + } + logger.DebugCF("agent", "Merged consecutive user messages", nil) + continue + } + + // Tool results (role "tool" or "user" with ToolCallID) count as user + // messages in the Anthropic API. If the previous message is also a + // tool/user, that's fine — but if the previous is a plain user text + // message and this is also a plain user text message, we merged above. + merged = append(merged, msg) + } + + return merged } func (cb *ContextBuilder) AddToolResult( diff --git a/pkg/agent/context_test.go b/pkg/agent/context_test.go index f307a637b..a84783eca 100644 --- a/pkg/agent/context_test.go +++ b/pkg/agent/context_test.go @@ -233,12 +233,13 @@ func TestSanitizeHistoryForProvider_IncompleteToolResults(t *testing.T) { result := sanitizeHistoryForProvider(history) // The assistant message with incomplete tool results should be dropped, - // along with its partial tool result. The remaining messages are: - // user ("do two things"), user ("next question"), assistant ("answer") - if len(result) != 3 { - t.Fatalf("expected 3 messages, got %d: %+v", len(result), roles(result)) + // along with its partial tool result. The two consecutive user messages + // are then merged by the third pass. + // Remaining: user ("do two things\nnext question"), assistant ("answer") + if len(result) != 2 { + t.Fatalf("expected 2 messages, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "user", "assistant") + assertRoles(t, result, "user", "assistant") } // TestSanitizeHistoryForProvider_MissingAllToolResults tests the case where @@ -254,11 +255,12 @@ func TestSanitizeHistoryForProvider_MissingAllToolResults(t *testing.T) { result := sanitizeHistoryForProvider(history) // The assistant message with no tool results should be dropped. - // Remaining: user ("do something"), user ("hello"), assistant ("hi") - if len(result) != 3 { - t.Fatalf("expected 3 messages, got %d: %+v", len(result), roles(result)) + // The two consecutive user messages are merged by the third pass. + // Remaining: user ("do something\nhello"), assistant ("hi") + if len(result) != 2 { + t.Fatalf("expected 2 messages, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "user", "assistant") + assertRoles(t, result, "user", "assistant") } // TestSanitizeHistoryForProvider_PartialToolResultsInMiddle tests that @@ -283,11 +285,12 @@ func TestSanitizeHistoryForProvider_PartialToolResultsInMiddle(t *testing.T) { // First round is complete (user, assistant+tools, tool, assistant), // second round is incomplete and dropped (assistant+tools, partial tool), // third round is complete (user, assistant+tools, tool, assistant). - // Remaining: user, assistant, tool, assistant, user, user, assistant, tool, assistant - if len(result) != 9 { - t.Fatalf("expected 9 messages, got %d: %+v", len(result), roles(result)) + // The two consecutive user messages ("second", "third") are merged. + // Remaining: user, assistant, tool, assistant, user, assistant, tool, assistant + if len(result) != 8 { + t.Fatalf("expected 8 messages, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "user", "assistant", "tool", "assistant") + assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "assistant", "tool", "assistant") } // TestSanitizeHistoryForProvider_EmptyToolCallName tests that tool calls with @@ -379,9 +382,10 @@ func TestSanitizeHistoryForProvider_AllToolCallsDroppedEmptyContent(t *testing.T result := sanitizeHistoryForProvider(history) // The assistant message has no valid tool calls and no content → dropped. // Its tool result is orphaned and also dropped. - // Remaining: user ("hello"), user ("follow up") - if len(result) != 2 { - t.Fatalf("expected 2 messages, got %d: %+v", len(result), roles(result)) + // The two consecutive user messages are merged by the third pass. + // Remaining: user ("hello\nfollow up") + if len(result) != 1 { + t.Fatalf("expected 1 message, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "user") + assertRoles(t, result, "user") } diff --git a/pkg/providers/anthropic/provider.go b/pkg/providers/anthropic/provider.go index 87beae99a..ac77f040a 100644 --- a/pkg/providers/anthropic/provider.go +++ b/pkg/providers/anthropic/provider.go @@ -146,7 +146,8 @@ func buildParams( var system []anthropic.TextBlockParam var anthropicMessages []anthropic.MessageParam - for _, msg := range messages { + for i := 0; i < len(messages); i++ { + msg := messages[i] switch msg.Role { case "system": // Prefer structured SystemParts for per-block cache_control. @@ -165,9 +166,16 @@ func buildParams( } case "user": if msg.ToolCallID != "" { - anthropicMessages = append(anthropicMessages, - anthropic.NewUserMessage(anthropic.NewToolResultBlock(msg.ToolCallID, msg.Content, false)), - ) + // Tool result — group with any following tool results into + // a single user message to avoid consecutive user messages. + var blocks []anthropic.ContentBlockParamUnion + blocks = append(blocks, anthropic.NewToolResultBlock(msg.ToolCallID, msg.Content, false)) + for i+1 < len(messages) && (messages[i+1].Role == "tool" || + (messages[i+1].Role == "user" && messages[i+1].ToolCallID != "")) { + i++ + blocks = append(blocks, anthropic.NewToolResultBlock(messages[i].ToolCallID, messages[i].Content, false)) + } + anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(blocks...)) } else { anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(anthropic.NewTextBlock(msg.Content)), @@ -198,9 +206,16 @@ func buildParams( ) } case "tool": - anthropicMessages = append(anthropicMessages, - anthropic.NewUserMessage(anthropic.NewToolResultBlock(msg.ToolCallID, msg.Content, false)), - ) + // Group consecutive tool results into a single user message. + // The Anthropic API requires alternating user/assistant messages; + // sending each tool result as a separate user message can violate this. + var blocks []anthropic.ContentBlockParamUnion + blocks = append(blocks, anthropic.NewToolResultBlock(msg.ToolCallID, msg.Content, false)) + for i+1 < len(messages) && messages[i+1].Role == "tool" { + i++ + blocks = append(blocks, anthropic.NewToolResultBlock(messages[i].ToolCallID, messages[i].Content, false)) + } + anthropicMessages = append(anthropicMessages, anthropic.NewUserMessage(blocks...)) } } diff --git a/pkg/providers/anthropic_messages/provider.go b/pkg/providers/anthropic_messages/provider.go index 8a83a7058..aeba3e11b 100644 --- a/pkg/providers/anthropic_messages/provider.go +++ b/pkg/providers/anthropic_messages/provider.go @@ -176,7 +176,8 @@ func buildRequestBody( var systemPrompt string var apiMessages []any - for _, msg := range messages { + for i := 0; i < len(messages); i++ { + msg := messages[i] switch msg.Role { case "system": // Accumulate system messages @@ -188,7 +189,8 @@ func buildRequestBody( case "user": if msg.ToolCallID != "" { - // Tool result message + // Tool result message — group consecutive tool results into + // a single user message to avoid consecutive user messages. content := []map[string]any{ { "type": "tool_result", @@ -196,6 +198,15 @@ func buildRequestBody( "content": msg.Content, }, } + for i+1 < len(messages) && (messages[i+1].Role == "tool" || + (messages[i+1].Role == "user" && messages[i+1].ToolCallID != "")) { + i++ + content = append(content, map[string]any{ + "type": "tool_result", + "tool_use_id": messages[i].ToolCallID, + "content": messages[i].Content, + }) + } apiMessages = append(apiMessages, map[string]any{ "role": "user", "content": content, @@ -236,7 +247,7 @@ func buildRequestBody( }) case "tool": - // Tool result (alternative format) + // Group consecutive tool results into a single user message. content := []map[string]any{ { "type": "tool_result", @@ -244,6 +255,14 @@ func buildRequestBody( "content": msg.Content, }, } + for i+1 < len(messages) && messages[i+1].Role == "tool" { + i++ + content = append(content, map[string]any{ + "type": "tool_result", + "tool_use_id": messages[i].ToolCallID, + "content": messages[i].Content, + }) + } apiMessages = append(apiMessages, map[string]any{ "role": "user", "content": content,