diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 3c2f38e6..83d2d3e7 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -808,6 +808,12 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) break } + // Ensure globally unique tool call IDs (OpenAI-compatible APIs return 400 on duplicates). + // Skip if raw content is present (Anthropic thinking passback) to avoid desync. + if resp.RawAssistantContent == nil { + resp.ToolCalls = uniquifyToolCallIDs(resp.ToolCalls, req.RunID, iteration) + } + // Build assistant message with tool calls assistantMsg := providers.Message{ Role: "assistant", diff --git a/internal/agent/loop_history.go b/internal/agent/loop_history.go index 71c447ee..08cee423 100644 --- a/internal/agent/loop_history.go +++ b/internal/agent/loop_history.go @@ -370,7 +370,8 @@ func limitHistoryTurns(msgs []providers.Message, limit int) []providers.Message // - Orphaned tool messages at start of history (after truncation) // - tool_result without matching tool_use in preceding assistant message // - assistant with tool_calls but missing tool_results -// sanitizeHistory repairs tool_use/tool_result pairing in session history. +// - Duplicate tool call IDs across turns (legacy sessions before uniquifyToolCallIDs) +// // Returns the cleaned messages and the number of messages that were dropped or synthesized. func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { if len(msgs) == 0 { @@ -393,15 +394,35 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { } // 2. Walk through messages ensuring tool_result follows matching tool_use. + // Also dedup tool call IDs across the transcript for legacy sessions that + // may have persisted duplicates before the live uniquify fix was deployed. var result []providers.Message + globalSeen := make(map[string]bool) // tracks IDs seen across entire transcript + for i := start; i < len(msgs); i++ { msg := msgs[i] if msg.Role == "assistant" && len(msg.ToolCalls) > 0 { - // Collect expected tool call IDs + // Deep-copy ToolCalls to avoid mutating the original session history. + oldCalls := msg.ToolCalls + msg.ToolCalls = make([]providers.ToolCall, len(oldCalls)) + copy(msg.ToolCalls, oldCalls) + + // Dedup: rewrite any ID that was already seen in an earlier turn. + // Maps original ID → rewritten ID for pairing with tool results below. + idRemap := make(map[string]string, len(msg.ToolCalls)) expectedIDs := make(map[string]bool, len(msg.ToolCalls)) - for _, tc := range msg.ToolCalls { - expectedIDs[tc.ID] = true + for j := range msg.ToolCalls { + origID := msg.ToolCalls[j].ID + newID := origID + if globalSeen[origID] { + newID = fmt.Sprintf("%s_dedup_%d", origID, j) + slog.Debug("sanitizeHistory: dedup tool call ID", "orig", origID, "new", newID) + } + msg.ToolCalls[j].ID = newID + globalSeen[newID] = true + idRemap[origID] = newID + expectedIDs[newID] = true } result = append(result, msg) @@ -410,9 +431,10 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { for i+1 < len(msgs) && msgs[i+1].Role == "tool" { i++ toolMsg := msgs[i] - if expectedIDs[toolMsg.ToolCallID] { + if newID, ok := idRemap[toolMsg.ToolCallID]; ok && expectedIDs[newID] { + toolMsg.ToolCallID = newID result = append(result, toolMsg) - delete(expectedIDs, toolMsg.ToolCallID) + delete(expectedIDs, newID) } else { slog.Debug("sanitizeHistory: dropping mismatched tool result", "tool_call_id", toolMsg.ToolCallID) @@ -421,14 +443,16 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { } // Synthesize missing tool results - for id := range expectedIDs { - slog.Debug("sanitizeHistory: synthesizing missing tool result", "tool_call_id", id) - result = append(result, providers.Message{ - Role: "tool", - Content: "[Tool result missing — session was compacted]", - ToolCallID: id, - }) - dropped++ + for _, tc := range msg.ToolCalls { + if expectedIDs[tc.ID] { + slog.Debug("sanitizeHistory: synthesizing missing tool result", "tool_call_id", tc.ID) + result = append(result, providers.Message{ + Role: "tool", + Content: "[Tool result missing — session was compacted]", + ToolCallID: tc.ID, + }) + dropped++ + } } } else if msg.Role == "tool" { // Orphaned tool message mid-history (no preceding assistant with matching tool_calls) diff --git a/internal/agent/loop_history_test.go b/internal/agent/loop_history_test.go index 0bedcc13..7e863d82 100644 --- a/internal/agent/loop_history_test.go +++ b/internal/agent/loop_history_test.go @@ -204,6 +204,133 @@ func TestSanitizeHistory_DropsOrphanedToolMidHistory(t *testing.T) { } } +func TestSanitizeHistory_DedupsDuplicateIDsAcrossTurns(t *testing.T) { + msgs := []providers.Message{ + {Role: "user", Content: "turn 1"}, + {Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{ + {ID: "call_abc", Name: "read_file"}, + }}, + {Role: "tool", Content: "result1", ToolCallID: "call_abc"}, + {Role: "user", Content: "turn 2"}, + {Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{ + {ID: "call_abc", Name: "write_file"}, // duplicate ID from earlier turn + }}, + {Role: "tool", Content: "result2", ToolCallID: "call_abc"}, + {Role: "assistant", Content: "done"}, + } + got, _ := sanitizeHistory(msgs) + + // Collect all tool call IDs (from assistant messages) + seen := make(map[string]bool) + for _, m := range got { + for _, tc := range m.ToolCalls { + if seen[tc.ID] { + t.Errorf("duplicate tool call ID in sanitized output: %s", tc.ID) + } + seen[tc.ID] = true + } + } + + // Both tool results should be present (paired correctly) + toolResults := 0 + for _, m := range got { + if m.Role == "tool" { + toolResults++ + } + } + if toolResults != 2 { + t.Errorf("expected 2 tool results, got %d", toolResults) + } +} + +func TestSanitizeHistory_NoDedupWhenIDsUnique(t *testing.T) { + msgs := []providers.Message{ + {Role: "user", Content: "hello"}, + {Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{ + {ID: "tc1", Name: "read_file"}, + }}, + {Role: "tool", Content: "ok", ToolCallID: "tc1"}, + {Role: "user", Content: "next"}, + {Role: "assistant", Content: "", ToolCalls: []providers.ToolCall{ + {ID: "tc2", Name: "write_file"}, + }}, + {Role: "tool", Content: "ok", ToolCallID: "tc2"}, + } + got, dropped := sanitizeHistory(msgs) + if dropped != 0 { + t.Errorf("expected 0 dropped, got %d", dropped) + } + if len(got) != 6 { + t.Errorf("expected 6 messages, got %d", len(got)) + } + // IDs should be unchanged + if got[1].ToolCalls[0].ID != "tc1" { + t.Errorf("expected tc1, got %s", got[1].ToolCalls[0].ID) + } + if got[4].ToolCalls[0].ID != "tc2" { + t.Errorf("expected tc2, got %s", got[4].ToolCalls[0].ID) + } +} + +func TestUniquifyToolCallIDs(t *testing.T) { + runID := "abcdef12-3456-7890-abcd-ef1234567890" + + t.Run("empty calls", func(t *testing.T) { + got := uniquifyToolCallIDs(nil, runID, 0) + if len(got) != 0 { + t.Errorf("expected empty, got %d", len(got)) + } + }) + + t.Run("appends run prefix", func(t *testing.T) { + calls := []providers.ToolCall{ + {ID: "call_123", Name: "read_file"}, + {ID: "call_456", Name: "write_file"}, + } + got := uniquifyToolCallIDs(calls, runID, 2) + if got[0].ID != "call_123_abcdef12_2_0" { + t.Errorf("unexpected ID: %s", got[0].ID) + } + if got[1].ID != "call_456_abcdef12_2_1" { + t.Errorf("unexpected ID: %s", got[1].ID) + } + }) + + t.Run("handles empty ID", func(t *testing.T) { + calls := []providers.ToolCall{ + {ID: "", Name: "read_file"}, + } + got := uniquifyToolCallIDs(calls, runID, 0) + if got[0].ID != "call_abcdef12_0_0" { + t.Errorf("unexpected ID for empty: %s", got[0].ID) + } + }) + + t.Run("does not mutate input", func(t *testing.T) { + calls := []providers.ToolCall{ + {ID: "original", Name: "test"}, + } + got := uniquifyToolCallIDs(calls, runID, 0) + if calls[0].ID != "original" { + t.Error("input was mutated") + } + if got[0].ID == "original" { + t.Error("output should differ from input") + } + }) + + t.Run("duplicate IDs become unique", func(t *testing.T) { + calls := []providers.ToolCall{ + {ID: "same_id", Name: "a"}, + {ID: "same_id", Name: "b"}, + } + got := uniquifyToolCallIDs(calls, runID, 0) + if got[0].ID == got[1].ID { + t.Errorf("IDs should be unique, both are: %s", got[0].ID) + } + }) +} + func TestEstimateTokens(t *testing.T) { msgs := []providers.Message{ {Role: "user", Content: "Hello world!"}, // 12 chars → ~4 tokens diff --git a/internal/agent/loop_utils.go b/internal/agent/loop_utils.go index f43fe20b..e168f29e 100644 --- a/internal/agent/loop_utils.go +++ b/internal/agent/loop_utils.go @@ -82,3 +82,30 @@ func (l *Loop) ProviderName() string { } return l.provider.Name() } + +// uniquifyToolCallIDs ensures all tool call IDs are globally unique across the +// transcript by appending a short run-ID prefix and iteration index. +// Returns a new slice (does not mutate the input). +// +// Some OpenAI-compatible APIs (OpenRouter, vLLM, DeepSeek) return duplicate IDs +// within a single response or reuse IDs from earlier turns, causing HTTP 400. +// Using the run UUID guarantees cross-turn uniqueness without history rewriting. +func uniquifyToolCallIDs(calls []providers.ToolCall, runID string, iteration int) []providers.ToolCall { + if len(calls) == 0 { + return calls + } + short := runID + if len(short) > 8 { + short = short[:8] + } + out := make([]providers.ToolCall, len(calls)) + copy(out, calls) + for i := range out { + if out[i].ID == "" { + out[i].ID = fmt.Sprintf("call_%s_%d_%d", short, iteration, i) + } else { + out[i].ID = fmt.Sprintf("%s_%s_%d_%d", out[i].ID, short, iteration, i) + } + } + return out +}