fix(agent): sanitize invalid tool-call history
This commit is contained in:
parent
4a8a2e9c23
commit
6a0f388730
2 changed files with 114 additions and 12 deletions
|
|
@ -615,18 +615,26 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||||
|
|
||||||
case "assistant":
|
case "assistant":
|
||||||
if len(msg.ToolCalls) > 0 {
|
if len(msg.ToolCalls) > 0 {
|
||||||
if len(sanitized) == 0 {
|
var keep bool
|
||||||
logger.DebugCF("agent", "Dropping assistant tool-call turn at history start", map[string]any{})
|
msg, keep = sanitizeAssistantToolCalls(msg)
|
||||||
|
if !keep {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
prev := sanitized[len(sanitized)-1]
|
|
||||||
if prev.Role != "user" && prev.Role != "tool" {
|
if len(msg.ToolCalls) > 0 {
|
||||||
logger.DebugCF(
|
if len(sanitized) == 0 {
|
||||||
"agent",
|
logger.DebugCF("agent", "Dropping assistant tool-call turn at history start", map[string]any{})
|
||||||
"Dropping assistant tool-call turn with invalid predecessor",
|
continue
|
||||||
map[string]any{"prev_role": prev.Role},
|
}
|
||||||
)
|
prev := sanitized[len(sanitized)-1]
|
||||||
continue
|
if prev.Role != "user" && prev.Role != "tool" {
|
||||||
|
logger.DebugCF(
|
||||||
|
"agent",
|
||||||
|
"Dropping assistant tool-call turn with invalid predecessor",
|
||||||
|
map[string]any{"prev_role": prev.Role},
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
sanitized = append(sanitized, msg)
|
sanitized = append(sanitized, msg)
|
||||||
|
|
@ -650,8 +658,10 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||||
expected[tc.ID] = false
|
expected[tc.ID] = false
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check following messages for matching tool results
|
// Collect following tool results, keeping only those that still
|
||||||
|
// match a valid tool_call from the assistant message.
|
||||||
toolMsgCount := 0
|
toolMsgCount := 0
|
||||||
|
matchingToolResults := make([]providers.Message, 0, len(expected))
|
||||||
for j := i + 1; j < len(sanitized); j++ {
|
for j := i + 1; j < len(sanitized); j++ {
|
||||||
if sanitized[j].Role != "tool" {
|
if sanitized[j].Role != "tool" {
|
||||||
break
|
break
|
||||||
|
|
@ -659,7 +669,12 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||||
toolMsgCount++
|
toolMsgCount++
|
||||||
if _, exists := expected[sanitized[j].ToolCallID]; exists {
|
if _, exists := expected[sanitized[j].ToolCallID]; exists {
|
||||||
expected[sanitized[j].ToolCallID] = true
|
expected[sanitized[j].ToolCallID] = true
|
||||||
|
matchingToolResults = append(matchingToolResults, sanitized[j])
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
logger.DebugCF("agent", "Dropping unexpected tool result from history", map[string]any{
|
||||||
|
"tool_call_id": sanitized[j].ToolCallID,
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
// If any tool_call_id is missing, drop this assistant message and its partial tool messages
|
// If any tool_call_id is missing, drop this assistant message and its partial tool messages
|
||||||
|
|
@ -685,6 +700,11 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||||
i += toolMsgCount
|
i += toolMsgCount
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final = append(final, msg)
|
||||||
|
final = append(final, matchingToolResults...)
|
||||||
|
i += toolMsgCount
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
final = append(final, msg)
|
final = append(final, msg)
|
||||||
}
|
}
|
||||||
|
|
@ -692,6 +712,33 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message
|
||||||
return final
|
return final
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func sanitizeAssistantToolCalls(msg providers.Message) (providers.Message, bool) {
|
||||||
|
filtered := make([]providers.ToolCall, 0, len(msg.ToolCalls))
|
||||||
|
for _, tc := range msg.ToolCalls {
|
||||||
|
normalized := providers.NormalizeToolCall(tc)
|
||||||
|
if strings.TrimSpace(normalized.ID) == "" || strings.TrimSpace(normalized.Name) == "" {
|
||||||
|
logger.DebugCF("agent", "Dropping invalid tool call from history", map[string]any{
|
||||||
|
"tool_call_id": normalized.ID,
|
||||||
|
"tool_name": normalized.Name,
|
||||||
|
})
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
filtered = append(filtered, normalized)
|
||||||
|
}
|
||||||
|
|
||||||
|
msg.ToolCalls = filtered
|
||||||
|
if len(filtered) > 0 {
|
||||||
|
return msg, true
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.TrimSpace(msg.Content) != "" || strings.TrimSpace(msg.ReasoningContent) != "" {
|
||||||
|
return msg, true
|
||||||
|
}
|
||||||
|
|
||||||
|
logger.DebugCF("agent", "Dropping assistant message with only invalid tool calls", map[string]any{})
|
||||||
|
return providers.Message{}, false
|
||||||
|
}
|
||||||
|
|
||||||
func (cb *ContextBuilder) AddToolResult(
|
func (cb *ContextBuilder) AddToolResult(
|
||||||
messages []providers.Message,
|
messages []providers.Message,
|
||||||
toolCallID, toolName, result string,
|
toolCallID, toolName, result string,
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ func msg(role, content string) providers.Message {
|
||||||
func assistantWithTools(toolIDs ...string) providers.Message {
|
func assistantWithTools(toolIDs ...string) providers.Message {
|
||||||
calls := make([]providers.ToolCall, len(toolIDs))
|
calls := make([]providers.ToolCall, len(toolIDs))
|
||||||
for i, id := range toolIDs {
|
for i, id := range toolIDs {
|
||||||
calls[i] = providers.ToolCall{ID: id, Type: "function"}
|
calls[i] = providers.ToolCall{ID: id, Type: "function", Name: "tool_" + id}
|
||||||
}
|
}
|
||||||
return providers.Message{Role: "assistant", ToolCalls: calls}
|
return providers.Message{Role: "assistant", ToolCalls: calls}
|
||||||
}
|
}
|
||||||
|
|
@ -188,6 +188,61 @@ func TestSanitizeHistoryForProvider_PlainConversation(t *testing.T) {
|
||||||
assertRoles(t, result, "user", "assistant", "user", "assistant")
|
assertRoles(t, result, "user", "assistant", "user", "assistant")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSanitizeHistoryForProvider_DropsInvalidToolCallsAndUnexpectedToolResults(t *testing.T) {
|
||||||
|
history := []providers.Message{
|
||||||
|
msg("user", "check"),
|
||||||
|
{
|
||||||
|
Role: "assistant",
|
||||||
|
ToolCalls: []providers.ToolCall{
|
||||||
|
{ID: "call_ok", Name: "read_file"},
|
||||||
|
{ID: "call_blank_name", Name: " "},
|
||||||
|
{ID: "", Name: "exec"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
toolResult("call_ok"),
|
||||||
|
toolResult("call_blank_name"),
|
||||||
|
toolResult(""),
|
||||||
|
msg("assistant", "done"),
|
||||||
|
}
|
||||||
|
|
||||||
|
result := sanitizeHistoryForProvider(history)
|
||||||
|
if len(result) != 4 {
|
||||||
|
t.Fatalf("expected 4 messages, got %d: %+v", len(result), result)
|
||||||
|
}
|
||||||
|
assertRoles(t, result, "user", "assistant", "tool", "assistant")
|
||||||
|
if got := len(result[1].ToolCalls); got != 1 {
|
||||||
|
t.Fatalf("assistant tool call count = %d, want 1", got)
|
||||||
|
}
|
||||||
|
if result[1].ToolCalls[0].ID != "call_ok" {
|
||||||
|
t.Fatalf("assistant tool call id = %q, want %q", result[1].ToolCalls[0].ID, "call_ok")
|
||||||
|
}
|
||||||
|
if result[2].ToolCallID != "call_ok" {
|
||||||
|
t.Fatalf("tool result id = %q, want %q", result[2].ToolCallID, "call_ok")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSanitizeHistoryForProvider_DropsAssistantWithOnlyInvalidToolCalls(t *testing.T) {
|
||||||
|
history := []providers.Message{
|
||||||
|
msg("user", "check"),
|
||||||
|
{
|
||||||
|
Role: "assistant",
|
||||||
|
ToolCalls: []providers.ToolCall{
|
||||||
|
{ID: "call_blank_name", Name: " "},
|
||||||
|
{ID: "", Name: "exec"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
toolResult("call_blank_name"),
|
||||||
|
msg("user", "next"),
|
||||||
|
msg("assistant", "ok"),
|
||||||
|
}
|
||||||
|
|
||||||
|
result := sanitizeHistoryForProvider(history)
|
||||||
|
if len(result) != 3 {
|
||||||
|
t.Fatalf("expected 3 messages, got %d: %+v", len(result), result)
|
||||||
|
}
|
||||||
|
assertRoles(t, result, "user", "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 {
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue