diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 5a84c45e2..1708d8a10 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -614,6 +614,26 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message sanitized = append(sanitized, msg) case "assistant": + if len(msg.ToolCalls) > 0 { + // Normalize tool calls: restores Name from Function.Name after JSON + // deserialization (Name is json:"-" and is lost when sessions are + // persisted). Drop any tool calls that still have an empty name or + // ID after normalization — these would cause Anthropic API errors + // ("tooluse.name: String should have at least 1 character"). + cleaned := make([]providers.ToolCall, 0, len(msg.ToolCalls)) + for _, tc := range msg.ToolCalls { + tc = providers.NormalizeToolCall(tc) + if strings.TrimSpace(tc.ID) == "" || strings.TrimSpace(tc.Name) == "" { + logger.DebugCF("agent", "Dropping tool call with empty id or name", map[string]any{ + "id": tc.ID, + "name": tc.Name, + }) + continue + } + cleaned = append(cleaned, tc) + } + msg.ToolCalls = cleaned + } if len(msg.ToolCalls) > 0 { if len(sanitized) == 0 { logger.DebugCF("agent", "Dropping assistant tool-call turn at history start", map[string]any{}) @@ -640,6 +660,9 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message // tool result messages following it. This is required by strict providers // like DeepSeek that enforce: "An assistant message with 'tool_calls' must // be followed by tool messages responding to each 'tool_call_id'." + // Also drops tool results whose ToolCallID has no matching tool call in the + // preceding assistant message (can occur when some tool calls were dropped + // by the normalization step above). final := make([]providers.Message, 0, len(sanitized)) for i := 0; i < len(sanitized); i++ { msg := sanitized[i] @@ -650,13 +673,13 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message expected[tc.ID] = false } - // Check following messages for matching tool results - toolMsgCount := 0 + // Collect following tool messages + var toolMsgs []providers.Message for j := i + 1; j < len(sanitized); j++ { if sanitized[j].Role != "tool" { break } - toolMsgCount++ + toolMsgs = append(toolMsgs, sanitized[j]) if _, exists := expected[sanitized[j].ToolCallID]; exists { expected[sanitized[j].ToolCallID] = true } @@ -673,7 +696,7 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message map[string]any{ "missing_tool_call_id": toolCallID, "expected_count": len(expected), - "found_count": toolMsgCount, + "found_count": len(toolMsgs), }, ) break @@ -682,9 +705,25 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message if !allFound { // Skip this assistant message and its tool messages - i += toolMsgCount + i += len(toolMsgs) continue } + + // Add the assistant message, then only tool results that match an + // expected tool call ID. Drop orphaned results (referencing a tool + // call that was removed by the normalization step above). + final = append(final, msg) + for _, tm := range toolMsgs { + if expected[tm.ToolCallID] { + final = append(final, tm) + } else { + logger.DebugCF("agent", "Dropping tool result with no matching tool call", map[string]any{ + "tool_call_id": tm.ToolCallID, + }) + } + } + i += len(toolMsgs) + continue } final = append(final, msg) } diff --git a/pkg/agent/context_test.go b/pkg/agent/context_test.go index 5756ed911..1e37e3fca 100644 --- a/pkg/agent/context_test.go +++ b/pkg/agent/context_test.go @@ -13,7 +13,15 @@ func msg(role, content string) providers.Message { func assistantWithTools(toolIDs ...string) providers.Message { calls := make([]providers.ToolCall, len(toolIDs)) for i, id := range toolIDs { - calls[i] = providers.ToolCall{ID: id, Type: "function"} + calls[i] = providers.ToolCall{ + ID: id, + Type: "function", + Name: "test_tool", + Function: &providers.FunctionCall{ + Name: "test_tool", + Arguments: "{}", + }, + } } return providers.Message{Role: "assistant", ToolCalls: calls} } @@ -281,3 +289,76 @@ func TestSanitizeHistoryForProvider_PartialToolResultsInMiddle(t *testing.T) { } assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "user", "assistant", "tool", "assistant") } + +// TestSanitizeHistoryForProvider_EmptyToolCallName tests that tool calls with +// an empty name are dropped. This can occur when sessions are deserialized from +// JSON storage: ToolCall.Name is json:"-" so it is lost, but Function.Name is +// preserved. NormalizeToolCall restores Name from Function.Name; tool calls +// where both are empty are dropped to prevent "tooluse.name: String should +// have at least 1 character" errors from the Anthropic API. +func TestSanitizeHistoryForProvider_EmptyToolCallName(t *testing.T) { + // Tool call with empty Name but Function.Name set — should be normalized and kept. + toolCallRestorable := providers.ToolCall{ + ID: "A", + Type: "function", + // Name intentionally empty (simulates post-JSON-deserialization state) + Function: &providers.FunctionCall{Name: "my_tool", Arguments: "{}"}, + } + // Tool call with both Name and Function.Name empty — truly invalid, should be dropped. + toolCallInvalid := providers.ToolCall{ + ID: "B", + Type: "function", + // Name empty, Function.Name empty + Function: &providers.FunctionCall{Name: "", Arguments: "{}"}, + } + + history := []providers.Message{ + msg("user", "hello"), + {Role: "assistant", ToolCalls: []providers.ToolCall{toolCallRestorable, toolCallInvalid}}, + toolResult("A"), + toolResult("B"), // orphaned result for the dropped tool call + } + + result := sanitizeHistoryForProvider(history) + // toolCallRestorable: Name restored from Function.Name → kept. + // toolCallInvalid: Name still empty after normalization → dropped. + // toolResult("B") references the dropped call → also dropped. + // Result: user, assistant (with only tc_A), tool_A + if len(result) != 3 { + t.Fatalf("expected 3 messages, got %d: %+v", len(result), roles(result)) + } + assertRoles(t, result, "user", "assistant", "tool") + if len(result[1].ToolCalls) != 1 { + t.Fatalf("expected 1 tool call in assistant, got %d", len(result[1].ToolCalls)) + } + if result[1].ToolCalls[0].Name != "my_tool" { + t.Errorf("expected tool call name %q, got %q", "my_tool", result[1].ToolCalls[0].Name) + } + if result[2].ToolCallID != "A" { + t.Errorf("expected surviving tool result id %q, got %q", "A", result[2].ToolCallID) + } +} + +// TestSanitizeHistoryForProvider_EmptyToolCallID tests that tool calls with an +// empty ID are dropped (an empty ID also causes Anthropic API errors). +func TestSanitizeHistoryForProvider_EmptyToolCallID(t *testing.T) { + history := []providers.Message{ + msg("user", "hello"), + {Role: "assistant", ToolCalls: []providers.ToolCall{ + {ID: "", Name: "my_tool", Type: "function", Function: &providers.FunctionCall{Name: "my_tool"}}, + }}, + // No tool result since the call has no ID to reference + } + + result := sanitizeHistoryForProvider(history) + // The tool call with empty ID is dropped; the assistant message has no tool + // calls after filtering, so it passes through as a plain assistant message. + // user + assistant (plain) = 2 + if len(result) != 2 { + t.Fatalf("expected 2 messages, got %d: %+v", len(result), roles(result)) + } + assertRoles(t, result, "user", "assistant") + if len(result[1].ToolCalls) != 0 { + t.Fatalf("expected 0 tool calls in assistant, got %d", len(result[1].ToolCalls)) + } +}