diff --git a/internal/agent/loop_history_sanitize.go b/internal/agent/loop_history_sanitize.go index a55b2bf9..7a009dcf 100644 --- a/internal/agent/loop_history_sanitize.go +++ b/internal/agent/loop_history_sanitize.go @@ -152,6 +152,7 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { Role: "tool", Content: "[Tool result missing — session was compacted]", ToolCallID: tc.ID, + ToolName: tc.Name, }) dropped++ } diff --git a/internal/agent/loop_pipeline_tool_callbacks.go b/internal/agent/loop_pipeline_tool_callbacks.go index a79bd26e..155f08a3 100644 --- a/internal/agent/loop_pipeline_tool_callbacks.go +++ b/internal/agent/loop_pipeline_tool_callbacks.go @@ -168,6 +168,7 @@ func (l *Loop) makeExecuteToolRaw(req *RunRequest) func(ctx context.Context, tc Role: "tool", Content: result.ForLLM, ToolCallID: tc.ID, + ToolName: tc.Name, IsError: result.IsError, } return msg, &toolRawResult{result: result, duration: dur, start: start, spanID: spanID, toolName: registryName, rawName: tc.Name}, nil diff --git a/internal/agent/loop_tools.go b/internal/agent/loop_tools.go index 7639051d..37c3c4ef 100644 --- a/internal/agent/loop_tools.go +++ b/internal/agent/loop_tools.go @@ -158,6 +158,7 @@ func (l *Loop) processToolResult( Role: "tool", Content: result.ForLLM, ToolCallID: tc.ID, + ToolName: tc.Name, IsError: result.IsError, } diff --git a/internal/agent/memoryflush.go b/internal/agent/memoryflush.go index 7db493a0..2d95b5d8 100644 --- a/internal/agent/memoryflush.go +++ b/internal/agent/memoryflush.go @@ -232,6 +232,7 @@ func (l *Loop) runMemoryFlush(ctx context.Context, sessionKey string, settings * Role: "tool", Content: result.ForLLM, ToolCallID: tc.ID, + ToolName: tc.Name, }) } } diff --git a/internal/agent/pruning.go b/internal/agent/pruning.go index 6cec5e05..07e04976 100644 --- a/internal/agent/pruning.go +++ b/internal/agent/pruning.go @@ -299,6 +299,10 @@ func pruneContextMessages(msgs []providers.Message, contextWindowTokens int, cfg Role: msg.Role, Content: trimmed, ToolCallID: msg.ToolCallID, + // Preserve ToolName: Gemini needs it to build FunctionResponse.name, + // and pruned results are exactly the ones whose assistant tool_call + // may already be out of the request window. + ToolName: msg.ToolName, } totalTokens += est.estimateTokens(trimmed) - msgTokens if stats != nil { @@ -352,6 +356,7 @@ func pruneContextMessages(msgs []providers.Message, contextWindowTokens int, cfg Role: msg.Role, Content: settings.hardClearPlaceholder, ToolCallID: msg.ToolCallID, + ToolName: msg.ToolName, } afterTokens := est.estimateTokens(settings.hardClearPlaceholder) totalTokens += afterTokens - beforeTokens diff --git a/internal/pipeline/tool_stage.go b/internal/pipeline/tool_stage.go index f6f1f0b1..b9b0c5ef 100644 --- a/internal/pipeline/tool_stage.go +++ b/internal/pipeline/tool_stage.go @@ -167,6 +167,7 @@ func (s *ToolStage) preflightToolCall(ctx context.Context, state *RunState, tc p Role: "tool", Content: reason, ToolCallID: tc.ID, + ToolName: tc.Name, IsError: true, } } @@ -191,6 +192,7 @@ func (s *ToolStage) preflightToolCall(ctx context.Context, state *RunState, tc p Role: "tool", Content: "Hook blocked: pre_tool_use", ToolCallID: tc.ID, + ToolName: tc.Name, } } if r.UpdatedToolInput != nil { diff --git a/internal/providers/openai_request.go b/internal/providers/openai_request.go index 61de8d87..8cfa6172 100644 --- a/internal/providers/openai_request.go +++ b/internal/providers/openai_request.go @@ -146,13 +146,27 @@ func (p *OpenAIProvider) buildRequestBody(model string, req ChatRequest, stream // (FunctionResponse.name). Most other OpenAI-compat hosts (Together, Groq, // vLLM) either ignore or reject unknown fields — gate to Gemini only to // avoid silent 400s on stricter proxies. - if supportsThoughtSignature { - if name := toolNameByID[m.ToolCallID]; name != "" { - msg["name"] = name - } else if m.Role == "tool" { - slog.Warn("openai: tool msg without matching tool_call", - "provider", p.name, "tool_call_id", m.ToolCallID) + if supportsThoughtSignature && m.Role == "tool" { + // Prefer the name carried on the message: it survives pruning, + // truncation and tool_call collapse. Fall back to the reverse index + // for history persisted before Message.ToolName existed. + name := m.ToolName + if name == "" { + name = toolNameByID[m.ToolCallID] } + if name == "" { + // Gemini pairs functionCall↔functionResponse by name and has no + // tool_call_id to fall back on, so an empty name is a hard 400 + // ("Name cannot be empty"). A synthetic name is not an option + // either — it would match no prior functionCall. Dropping the + // unlabelled result is the only way to keep the request valid. + // Reachable only for legacy history whose assistant tool_call is + // already out of the window, so nothing dangles by removing it. + slog.Warn("openai: dropping tool result with unresolvable tool name", + "provider", p.name, "tool_call_id", m.ToolCallID) + continue + } + msg["name"] = name } } diff --git a/internal/providers/openai_tool_message_name_test.go b/internal/providers/openai_tool_message_name_test.go index 7a0c7c22..5e88f0f7 100644 --- a/internal/providers/openai_tool_message_name_test.go +++ b/internal/providers/openai_tool_message_name_test.go @@ -82,11 +82,65 @@ func TestBuildRequestBody_ToolMessageNameLookupUsesRawID(t *testing.T) { } } -// TestBuildRequestBody_ToolMessageWithoutMatchingCallOmitsName verifies that a -// stray tool message (no preceding tool_call with matching ID) does NOT emit an -// empty name field — better to drop the field than send "" which Gemini rejects -// just the same. Logged via slog.Warn for observability. -func TestBuildRequestBody_ToolMessageWithoutMatchingCallOmitsName(t *testing.T) { +// TestBuildRequestBody_ToolNameSurvivesMissingAssistant is the core regression +// test for the Gemini 400. Message.ToolName must be used directly, so a tool +// result still serializes a valid FunctionResponse.name even when the assistant +// tool_call it originated from is no longer in the request window (pruned, +// truncated, or collapsed). The reverse id→name index cannot help here. +func TestBuildRequestBody_ToolNameSurvivesMissingAssistant(t *testing.T) { + p := NewOpenAIProvider("test-gemini", "key", + "https://generativelanguage.googleapis.com/v1beta/openai", "gemini-3-flash-preview") + + req := ChatRequest{ + Messages: []Message{ + {Role: "user", Content: "hi"}, + // No assistant tool_call in the window — only the carried name can save this. + {Role: "tool", ToolCallID: "call_gone", ToolName: "mcp__srv__lookup", Content: "ok"}, + }, + } + + body := p.buildRequestBody("gemini-3-flash-preview", req, false) + msgs := body["messages"].([]map[string]any) + if len(msgs) != 2 { + t.Fatalf("tool msg must be kept when ToolName is present, got %d msgs", len(msgs)) + } + if got := msgs[1]["name"]; got != "mcp__srv__lookup" { + t.Fatalf("msg[1] name = %v, want mcp__srv__lookup", got) + } +} + +// TestBuildRequestBody_ToolNameBeatsStaleIndex verifies precedence: the name +// carried on the message wins over the reverse index, so a rewritten/aliased +// tool_call ID cannot resurface a mismatched name. +func TestBuildRequestBody_ToolNameBeatsStaleIndex(t *testing.T) { + p := NewOpenAIProvider("test-gemini", "key", + "https://generativelanguage.googleapis.com/v1beta/openai", "gemini-3-flash-preview") + + req := ChatRequest{ + Messages: []Message{ + {Role: "user", Content: "hi"}, + {Role: "assistant", ToolCalls: []ToolCall{ + { + ID: "call_1", Name: "indexed_name", + Metadata: map[string]string{"thought_signature": "sig"}, + }, + }}, + {Role: "tool", ToolCallID: "call_1", ToolName: "carried_name", Content: "ok"}, + }, + } + + body := p.buildRequestBody("gemini-3-flash-preview", req, false) + msgs := body["messages"].([]map[string]any) + if got := msgs[2]["name"]; got != "carried_name" { + t.Fatalf("carried ToolName must win; got %v", got) + } +} + +// TestBuildRequestBody_UnresolvableToolNameDropped verifies that a tool result +// with neither a carried name nor an index match is DROPPED for Gemini. Emitting +// it with an empty (or absent) name is what produced HTTP 400 "Name cannot be +// empty" — Gemini has no tool_call_id fallback to pair on. +func TestBuildRequestBody_UnresolvableToolNameDropped(t *testing.T) { p := NewOpenAIProvider("test-gemini", "key", "https://generativelanguage.googleapis.com/v1beta/openai", "gemini-3-flash-preview") @@ -99,13 +153,41 @@ func TestBuildRequestBody_ToolMessageWithoutMatchingCallOmitsName(t *testing.T) body := p.buildRequestBody("gemini-3-flash-preview", req, false) msgs := body["messages"].([]map[string]any) - if len(msgs) < 2 { - t.Fatalf("expected at least 2 msgs after collapse, got %d", len(msgs)) - } - last := msgs[len(msgs)-1] - if last["role"] == "tool" { - if _, present := last["name"]; present { - t.Fatalf("orphan tool msg should omit name field, got %v", last["name"]) + + for i, m := range msgs { + if m["role"] == "tool" { + t.Fatalf("msg[%d]: unresolvable tool result must be dropped, got %v", i, m) + } + if name, present := m["name"]; present && name == "" { + t.Fatalf("msg[%d]: empty name must never reach Gemini", i) } } } + +// TestBuildRequestBody_UnresolvableToolNameKeptForNonGemini is the regression +// guard for every other OpenAI-compat host sharing this code path (OpenAI, Qwen, +// DeepSeek, Together, ...). They pair by tool_call_id and never need `name`, so +// the drop must not apply — silently losing tool results there would be a bug. +func TestBuildRequestBody_UnresolvableToolNameKeptForNonGemini(t *testing.T) { + p := NewOpenAIProvider("together", "key", + "https://api.together.xyz/v1", "meta-llama/Llama-3-70b") + + req := ChatRequest{ + Messages: []Message{ + {Role: "user", Content: "hi"}, + {Role: "tool", ToolCallID: "orphan_id", Content: "stale"}, + }, + } + + body := p.buildRequestBody("meta-llama/Llama-3-70b", req, false) + msgs := body["messages"].([]map[string]any) + if len(msgs) != 2 { + t.Fatalf("non-Gemini must keep tool msg, got %d msgs", len(msgs)) + } + if msgs[1]["role"] != "tool" { + t.Fatalf("msg[1] role = %v, want tool", msgs[1]["role"]) + } + if _, present := msgs[1]["name"]; present { + t.Fatalf("non-Gemini must not emit name, got %v", msgs[1]["name"]) + } +} diff --git a/internal/providers/types.go b/internal/providers/types.go index 672d4bc3..12e161dc 100644 --- a/internal/providers/types.go +++ b/internal/providers/types.go @@ -152,6 +152,16 @@ type Message struct { ToolCallID string `json:"tool_call_id,omitempty"` // for role="tool" responses IsError bool `json:"is_error,omitempty"` // for role="tool" responses + // ToolName is the originating tool's name for role="tool" messages. + // Google Gemini drops tool_call_id entirely and pairs functionCall↔functionResponse + // by function name, so its OpenAI-compat shim requires a non-empty + // FunctionResponse.name. Carrying the name on the message itself keeps it + // recoverable after pruning, truncation or tool_call collapse — all of which + // can remove the assistant tool_call that a reverse id→name lookup depends on. + // Must match the assistant ToolCall.Name that was sent on the wire (raw name, + // not the resolved registry name). Empty on history persisted before this field. + ToolName string `json:"tool_name,omitempty"` + // Phase is a Codex-specific field (gpt-5.3-codex) indicating message purpose. // Values: "commentary" (intermediate), "final_answer" (closeout), or "" (unset). // Must be persisted and passed back in subsequent requests for Codex performance. diff --git a/internal/tools/subagent_exec.go b/internal/tools/subagent_exec.go index a15efe5b..f5c6554f 100644 --- a/internal/tools/subagent_exec.go +++ b/internal/tools/subagent_exec.go @@ -369,6 +369,7 @@ func (sm *SubagentManager) executeTask(ctx context.Context, task *SubagentTask) Role: "tool", Content: result.ForLLM, ToolCallID: tc.ID, + ToolName: tc.Name, }) } }