fix(agent): preserve reused tool call IDs across turns (#2528)

Scope tool result deduplication to each assistant tool-call block so providers
that reuse call IDs across separate turns do not lose valid tool results. Also
drop invalid empty tool call IDs and orphaned tool messages after validation.
This commit is contained in:
wenjie 2026-04-15 20:18:09 +08:00 committed by GitHub
parent 100e576609
commit 7bd11181a6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 95 additions and 25 deletions

View file

@ -685,43 +685,60 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
// tool result messages following it. This is required by strict providers // tool result messages following it. This is required by strict providers
// like DeepSeek that enforce: "An assistant message with 'tool_calls' must // like DeepSeek that enforce: "An assistant message with 'tool_calls' must
// be followed by tool messages responding to each 'tool_call_id'." // be followed by tool messages responding to each 'tool_call_id'."
//
// Deduplication is scoped to the contiguous tool-result block that follows a
// single assistant tool-call message. Some providers legitimately reuse call
// IDs across separate turns (for example "call_0"), so global deduplication
// would incorrectly delete later valid tool results and leave an
// assistant(tool_calls) -> assistant sequence behind.
final := make([]providers.Message, 0, len(sanitized)) final := make([]providers.Message, 0, len(sanitized))
seenToolCallID := make(map[string]bool)
for i := 0; i < len(sanitized); i++ { for i := 0; i < len(sanitized); i++ {
msg := 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 { if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
// Collect expected tool_call IDs
expected := make(map[string]bool, len(msg.ToolCalls)) expected := make(map[string]bool, len(msg.ToolCalls))
invalidToolCallID := false
for _, tc := range msg.ToolCalls { for _, tc := range msg.ToolCalls {
if tc.ID == "" {
invalidToolCallID = true
continue
}
expected[tc.ID] = false expected[tc.ID] = false
} }
// Check following messages for matching tool results block := make([]providers.Message, 0, len(expected))
toolMsgCount := 0 seenInBlock := make(map[string]bool, len(expected))
for j := i + 1; j < len(sanitized); j++ { j := i + 1
if sanitized[j].Role != "tool" { for ; j < len(sanitized); j++ {
next := sanitized[j]
if next.Role != "tool" {
break break
} }
toolMsgCount++ if next.ToolCallID == "" {
if _, exists := expected[sanitized[j].ToolCallID]; exists { logger.DebugCF("agent", "Dropping tool result without tool_call_id", map[string]any{})
expected[sanitized[j].ToolCallID] = true continue
} }
if _, ok := expected[next.ToolCallID]; !ok {
logger.DebugCF("agent", "Dropping unexpected tool result", map[string]any{
"tool_call_id": next.ToolCallID,
})
continue
}
if seenInBlock[next.ToolCallID] {
logger.DebugCF("agent", "Dropping duplicate tool result in tool block", map[string]any{
"tool_call_id": next.ToolCallID,
})
continue
}
seenInBlock[next.ToolCallID] = true
expected[next.ToolCallID] = true
block = append(block, next)
} }
// If any tool_call_id is missing, drop this assistant message and its partial tool messages allFound := !invalidToolCallID
allFound := true if invalidToolCallID {
logger.DebugCF("agent", "Dropping assistant message with empty tool_call_id", map[string]any{})
}
for toolCallID, found := range expected { for toolCallID, found := range expected {
if !found { if !found {
allFound = false allFound = false
@ -731,7 +748,7 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
map[string]any{ map[string]any{
"missing_tool_call_id": toolCallID, "missing_tool_call_id": toolCallID,
"expected_count": len(expected), "expected_count": len(expected),
"found_count": toolMsgCount, "found_count": len(block),
}, },
) )
break break
@ -739,11 +756,23 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
} }
if !allFound { if !allFound {
// Skip this assistant message and its tool messages i = j - 1
i += toolMsgCount
continue continue
} }
final = append(final, msg)
final = append(final, block...)
i = j - 1
continue
} }
if msg.Role == "tool" {
logger.DebugCF("agent", "Dropping orphaned tool message after validation", map[string]any{
"tool_call_id": msg.ToolCallID,
})
continue
}
final = append(final, msg) final = append(final, msg)
} }

View file

@ -213,6 +213,47 @@ func TestSanitizeHistoryForProvider_DuplicateToolResults(t *testing.T) {
} }
} }
func TestSanitizeHistoryForProvider_ReusedToolCallIDAcrossRounds(t *testing.T) {
history := []providers.Message{
msg("user", "first"),
assistantWithTools("call_0"),
toolResult("call_0"),
msg("assistant", "first done"),
msg("user", "second"),
assistantWithTools("call_0"),
toolResult("call_0"),
msg("assistant", "second done"),
}
result := sanitizeHistoryForProvider(history)
if len(result) != 8 {
t.Fatalf("expected 8 messages, got %d: %+v", len(result), roles(result))
}
assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "assistant", "tool", "assistant")
if result[2].ToolCallID != "call_0" || result[6].ToolCallID != "call_0" {
t.Fatalf(
"expected both tool results to be preserved, got IDs %q and %q",
result[2].ToolCallID,
result[6].ToolCallID,
)
}
}
func TestSanitizeHistoryForProvider_DropsAssistantWithEmptyToolCallID(t *testing.T) {
history := []providers.Message{
msg("user", "do something"),
assistantWithTools(""),
toolResult(""),
msg("assistant", "done"),
}
result := sanitizeHistoryForProvider(history)
if len(result) != 2 {
t.Fatalf("expected 2 messages, got %d: %+v", len(result), roles(result))
}
assertRoles(t, result, "user", "assistant")
}
func roles(msgs []providers.Message) []string { func roles(msgs []providers.Message) []string {
r := make([]string, len(msgs)) r := make([]string, len(msgs))
for i, m := range msgs { for i, m := range msgs {