Merge pull request #1498 from justintruong29/fix/gemini-tool-result-name

fix(providers): carry tool name on tool results for Gemini

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++
 				}

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

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,
 	}
 

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,
 			})
 		}
 	}

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

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 {

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
 			}
 		}
 

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"])
+	}
+}

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.

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,
 			})
 		}
 	}