From 994c0aea1151ccd9a56ac7279cc2fd20588594a5 Mon Sep 17 00:00:00 2001 From: AayushGupta16 Date: Sun, 5 Jul 2026 20:41:00 -0700 Subject: [PATCH] fix(providers): resolve tool_use name/args from Function on reloaded history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ToolCall.Name and .Arguments are json:"-" (runtime-only), so after chat history round-trips through the session store only ToolCall.Function survives. The anthropic-messages and anthropic (SDK) providers emitted tool_use blocks from tc.Name alone, silently skipping every historical tool call while still emitting the matching tool_result — orphaned tool_results 400 at the API ("unexpected tool_use_id found in tool_result blocks"), killing every turn on agents with tool history. Fall back to Function.Name / json-parsed Function.Arguments (the same pattern the bedrock and openai_compat providers already use), and cover the deserialized-history shape with table tests in both providers. Co-Authored-By: Claude Fable 5 --- pkg/providers/anthropic/provider.go | 11 +- pkg/providers/anthropic/provider_test.go | 119 +++++++++++++++++ pkg/providers/anthropic_messages/provider.go | 20 ++- .../anthropic_messages/provider_test.go | 120 ++++++++++++++++++ 4 files changed, 265 insertions(+), 5 deletions(-) diff --git a/pkg/providers/anthropic/provider.go b/pkg/providers/anthropic/provider.go index 9685fe598..7f0fe53cc 100644 --- a/pkg/providers/anthropic/provider.go +++ b/pkg/providers/anthropic/provider.go @@ -181,8 +181,15 @@ func buildParams( blocks = append(blocks, anthropic.NewTextBlock(msg.Content)) } for _, tc := range msg.ToolCalls { + // Resolve tool name: prefer tc.Name, fallback to tc.Function.Name + // (tc.Name/tc.Arguments are json:"-" and may be empty when + // history is reloaded from the session store) + toolName := tc.Name + if toolName == "" && tc.Function != nil { + toolName = tc.Function.Name + } // Skip tool calls with empty names to avoid API errors - if tc.Name == "" { + if toolName == "" { continue } args := tc.Arguments @@ -194,7 +201,7 @@ func buildParams( if args == nil { args = map[string]any{} } - blocks = append(blocks, anthropic.NewToolUseBlock(tc.ID, args, tc.Name)) + blocks = append(blocks, anthropic.NewToolUseBlock(tc.ID, args, toolName)) } anthropicMessages = append(anthropicMessages, anthropic.NewAssistantMessage(blocks...)) } else { diff --git a/pkg/providers/anthropic/provider_test.go b/pkg/providers/anthropic/provider_test.go index b53d3d76c..0cb1a1132 100644 --- a/pkg/providers/anthropic/provider_test.go +++ b/pkg/providers/anthropic/provider_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "reflect" "sync/atomic" "testing" @@ -77,6 +78,124 @@ func TestBuildParams_ToolCallMessage(t *testing.T) { } } +// TestBuildParams_ToolCallFunctionFallback verifies that tool calls whose +// runtime-only fields were lost in a JSON round-trip through the session store +// (ToolCall.Name/Arguments are json:"-"; only ToolCall.Function survives) fall +// back to Function.Name / Function.Arguments, so the tool_use block is still +// emitted and its tool_result pair stays intact. Without the fallback the +// tool_use is skipped and the orphaned tool_result 400s at the API +// ("unexpected tool_use_id found in tool_result blocks"). +func TestBuildParams_ToolCallFunctionFallback(t *testing.T) { + tests := []struct { + name string + toolCall ToolCall + wantSkipped bool + wantToolName string + wantInput map[string]any + }{ + { + name: "deserialized history shape falls back to Function fields", + toolCall: ToolCall{ + ID: "toolu-fallback-1", + Name: "", + Arguments: nil, + Function: &FunctionCall{Name: "x", Arguments: `{"a":1}`}, + }, + wantToolName: "x", + wantInput: map[string]any{"a": float64(1)}, + }, + { + name: "runtime shape with Name set and Function nil still works", + toolCall: ToolCall{ + ID: "toolu-runtime-1", + Name: "y", + Arguments: map[string]any{"b": 2}, + }, + wantToolName: "y", + wantInput: map[string]any{"b": 2}, + }, + { + name: "both Name and Function.Name empty is skipped", + toolCall: ToolCall{ + ID: "toolu-empty-1", + Name: "", + Function: &FunctionCall{Name: "", Arguments: `{"c":3}`}, + }, + wantSkipped: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + messages := []Message{ + {Role: "user", Content: "run the tool"}, + {Role: "assistant", Content: "", ToolCalls: []ToolCall{tt.toolCall}}, + {Role: "tool", ToolCallID: tt.toolCall.ID, Content: "result"}, + } + + params, err := buildParams(messages, nil, "claude-sonnet-4.6", map[string]any{}) + if err != nil { + t.Fatalf("buildParams() error: %v", err) + } + if len(params.Messages) != 3 { + t.Fatalf("len(Messages) = %d, want 3", len(params.Messages)) + } + + assistantMsg := params.Messages[1] + var toolUses []*anthropic.ToolUseBlockParam + for _, block := range assistantMsg.Content { + if block.OfToolUse != nil { + toolUses = append(toolUses, block.OfToolUse) + } + } + + // The tool_result in the following user message always carries the + // original ID; look it up once for both branches. + toolResultMsg := params.Messages[2] + if len(toolResultMsg.Content) != 1 || toolResultMsg.Content[0].OfToolResult == nil { + t.Fatalf("message after assistant = %+v, want single tool_result block", toolResultMsg.Content) + } + toolResult := toolResultMsg.Content[0].OfToolResult + + if tt.wantSkipped { + if len(toolUses) != 0 { + t.Fatalf("tool_use blocks = %d, want 0 (tool call skipped)", len(toolUses)) + } + // Note: matching current behavior, the orphaned tool_result is + // still emitted even though its tool_use block was skipped. + if toolResult.ToolUseID != tt.toolCall.ID { + t.Fatalf("orphaned tool_result ToolUseID = %q, want %q", + toolResult.ToolUseID, tt.toolCall.ID) + } + return + } + + // (a) tool_use block emitted with resolved name, id, and parsed input. + if len(toolUses) != 1 { + t.Fatalf("tool_use blocks = %d, want 1", len(toolUses)) + } + toolUse := toolUses[0] + if toolUse.Name != tt.wantToolName { + t.Errorf("tool_use Name = %q, want %q", toolUse.Name, tt.wantToolName) + } + if toolUse.ID != tt.toolCall.ID { + t.Errorf("tool_use ID = %q, want %q", toolUse.ID, tt.toolCall.ID) + } + gotInput, ok := toolUse.Input.(map[string]any) + if !ok || !reflect.DeepEqual(gotInput, tt.wantInput) { + t.Errorf("tool_use Input = %#v, want %#v", toolUse.Input, tt.wantInput) + } + + // (b) the following user message's tool_result references the same + // id as the tool_use block — the pair is intact. + if toolResult.ToolUseID != toolUse.ID { + t.Errorf("tool_result ToolUseID = %q, want %q (paired with tool_use)", + toolResult.ToolUseID, toolUse.ID) + } + }) + } +} + func TestBuildParams_WithTools(t *testing.T) { tools := []ToolDefinition{ { diff --git a/pkg/providers/anthropic_messages/provider.go b/pkg/providers/anthropic_messages/provider.go index 672fb9324..3f1684956 100644 --- a/pkg/providers/anthropic_messages/provider.go +++ b/pkg/providers/anthropic_messages/provider.go @@ -233,12 +233,26 @@ func buildRequestBody( // Add tool_use blocks for _, tc := range msg.ToolCalls { - if strings.TrimSpace(tc.Name) == "" { + // Resolve tool name: prefer tc.Name, fallback to tc.Function.Name + // (tc.Name/tc.Arguments are json:"-" and may be empty when + // history is reloaded from the session store) + toolName := tc.Name + if toolName == "" && tc.Function != nil { + toolName = tc.Function.Name + } + if strings.TrimSpace(toolName) == "" { continue } - // Handle nil Arguments (GLM-4 may return null input) + // Resolve arguments: prefer tc.Arguments, fallback to parsing + // tc.Function.Arguments input := tc.Arguments + if input == nil && tc.Function != nil && tc.Function.Arguments != "" { + if err := json.Unmarshal([]byte(tc.Function.Arguments), &input); err != nil { + input = map[string]any{} + } + } + // Handle nil Arguments (GLM-4 may return null input) if input == nil { input = map[string]any{} } @@ -246,7 +260,7 @@ func buildRequestBody( toolUse := map[string]any{ "type": "tool_use", "id": tc.ID, - "name": tc.Name, + "name": toolName, "input": input, } content = append(content, toolUse) diff --git a/pkg/providers/anthropic_messages/provider_test.go b/pkg/providers/anthropic_messages/provider_test.go index 6401d84bd..f62fe3d18 100644 --- a/pkg/providers/anthropic_messages/provider_test.go +++ b/pkg/providers/anthropic_messages/provider_test.go @@ -614,6 +614,126 @@ func TestBuildRequestBody_UserToolResultsMerged(t *testing.T) { } } +// TestBuildRequestBody_ToolCallFunctionFallback verifies that tool calls whose +// runtime-only fields were lost in a JSON round-trip through the session store +// (ToolCall.Name/Arguments are json:"-"; only ToolCall.Function survives) fall +// back to Function.Name / Function.Arguments, so the tool_use block is still +// emitted and its tool_result pair stays intact. Without the fallback the +// tool_use is skipped and the orphaned tool_result 400s at the API +// ("unexpected tool_use_id found in tool_result blocks"). +func TestBuildRequestBody_ToolCallFunctionFallback(t *testing.T) { + tests := []struct { + name string + toolCall ToolCall + wantSkipped bool + wantToolName string + wantInput map[string]any + }{ + { + name: "deserialized history shape falls back to Function fields", + toolCall: ToolCall{ + ID: "toolu-fallback-1", + Name: "", + Arguments: nil, + Function: &FunctionCall{Name: "x", Arguments: `{"a":1}`}, + }, + wantToolName: "x", + wantInput: map[string]any{"a": float64(1)}, + }, + { + name: "runtime shape with Name set and Function nil still works", + toolCall: ToolCall{ + ID: "toolu-runtime-1", + Name: "y", + Arguments: map[string]any{"b": 2}, + }, + wantToolName: "y", + wantInput: map[string]any{"b": 2}, + }, + { + name: "both Name and Function.Name empty is skipped", + toolCall: ToolCall{ + ID: "toolu-empty-1", + Name: "", + Function: &FunctionCall{Name: "", Arguments: `{"c":3}`}, + }, + wantSkipped: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + messages := []Message{ + {Role: "user", Content: "run the tool"}, + {Role: "assistant", Content: "", ToolCalls: []ToolCall{tt.toolCall}}, + {Role: "tool", ToolCallID: tt.toolCall.ID, Content: "result"}, + } + + got, err := buildRequestBody(messages, nil, "test-model", map[string]any{"max_tokens": 8192}) + if err != nil { + t.Fatalf("buildRequestBody() error: %v", err) + } + + apiMessages := got["messages"].([]any) + if len(apiMessages) != 3 { + t.Fatalf("expected 3 API messages, got %d", len(apiMessages)) + } + + assistantMsg := apiMessages[1].(map[string]any) + content := assistantMsg["content"].([]any) + + if tt.wantSkipped { + if len(content) != 0 { + t.Fatalf("assistant content = %#v, want empty (tool call skipped)", content) + } + // Note: matching current behavior, the orphaned tool_result is + // still emitted in the following user message even though its + // tool_use block was skipped. + toolResultMsg := apiMessages[2].(map[string]any) + blocks := toolResultMsg["content"].([]map[string]any) + if len(blocks) != 1 || blocks[0]["tool_use_id"] != tt.toolCall.ID { + t.Fatalf("orphaned tool_result = %#v, want single block with tool_use_id %q", + blocks, tt.toolCall.ID) + } + return + } + + // (a) tool_use block emitted with resolved name, id, and parsed input. + if len(content) != 1 { + t.Fatalf("assistant content length = %d, want 1 tool_use block", len(content)) + } + toolUse := content[0].(map[string]any) + if toolUse["type"] != "tool_use" { + t.Fatalf("block type = %v, want tool_use", toolUse["type"]) + } + if toolUse["name"] != tt.wantToolName { + t.Errorf("tool_use name = %v, want %q", toolUse["name"], tt.wantToolName) + } + if toolUse["id"] != tt.toolCall.ID { + t.Errorf("tool_use id = %v, want %q", toolUse["id"], tt.toolCall.ID) + } + if !reflect.DeepEqual(toolUse["input"], tt.wantInput) { + t.Errorf("tool_use input = %#v, want %#v", toolUse["input"], tt.wantInput) + } + + // (b) the following user message's tool_result references the same + // id as the tool_use block — the pair is intact. + toolResultMsg := apiMessages[2].(map[string]any) + if toolResultMsg["role"] != "user" { + t.Fatalf("message after assistant role = %v, want user", toolResultMsg["role"]) + } + blocks := toolResultMsg["content"].([]map[string]any) + if len(blocks) != 1 { + t.Fatalf("tool_result blocks = %d, want 1", len(blocks)) + } + if blocks[0]["tool_use_id"] != toolUse["id"] { + t.Errorf("tool_result tool_use_id = %v, want %v (paired with tool_use)", + blocks[0]["tool_use_id"], toolUse["id"]) + } + }) + } +} + // TestParseResponseBodyEdgeCases tests edge cases for parseResponseBody. func TestParseResponseBodyEdgeCases(t *testing.T) { tests := []struct {