mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 04:16:06 +00:00
fix(agent): handle duplicate tool call IDs in OpenAI-compatible transcripts
Simplified approach to fix duplicate tool_call_ids that cause HTTP 400 on OpenAI-compatible APIs (OpenRouter, vLLM, DeepSeek). - uniquifyToolCallIDs: appends runID+iteration+index to all IDs at response time, guaranteeing cross-turn uniqueness via UUID - sanitizeHistory: simple seen-set dedup for legacy sessions with pre-existing duplicate IDs - Anthropic guard: skips ID rewriting when RawAssistantContent is present - Unit tests for both paths + edge cases Closes #283
This commit is contained in:
1 parent
cb93559154
commit
bc16af3797
4 files changed
+198
-14
No files matched your search
@@ -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",
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
Reference in new issue
Block a user