From 303685c230bf76e69b2e563fdfa0005a8be4beaa Mon Sep 17 00:00:00 2001 From: lamtran Date: Sun, 31 May 2026 22:49:23 +0700 Subject: [PATCH] fix(executor/xai): drop orphaned tool_choice when Claude tools array is empty When Claude Code sends a stop-hook evaluator request (or any request without tools), the payload includes "tools": [] (empty array). The claude->codex translator unconditionally emits tools: [] + tool_choice: "auto" + parallel_tool_calls: true into the Codex Responses shape. When that payload is routed to xAI, the upstream rejects with HTTP 400: "A tool_choice was set on the request but no tools were specified." Fix entirely in the xAI executor (translator package is policy-locked): add normalizeXAIToolChoiceForTools() after normalizeXAITools() to drop tool_choice and parallel_tool_calls whenever tools end up absent or empty (covering both the empty-from-source case and the all-filtered-out case where every tool was an unsupported type such as tool_search or image_generation). Per code-review feedback: always remove parallel_tool_calls when tools are missing (not gated on tool_choice presence) and existence-check each key before sjson delete to avoid unnecessary JSON parse/copy. Verification: - go build -o test-output ./cmd/server - go test ./internal/runtime/executor/... -count=1 - 5 new regression tests cover empty / missing / present / orphaned parallel_tool_calls / no-op-when-both-absent. --- internal/runtime/executor/xai_executor.go | 23 ++++++++ .../runtime/executor/xai_executor_test.go | 54 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/internal/runtime/executor/xai_executor.go b/internal/runtime/executor/xai_executor.go index cb42f939..5cb27949 100644 --- a/internal/runtime/executor/xai_executor.go +++ b/internal/runtime/executor/xai_executor.go @@ -506,6 +506,7 @@ func (e *XAIExecutor) prepareResponsesRequest(ctx context.Context, req cliproxye body, _ = sjson.DeleteBytes(body, "safety_identifier") body, _ = sjson.DeleteBytes(body, "stream_options") body = normalizeXAITools(body) + body = normalizeXAIToolChoiceForTools(body) body = normalizeXAIInputReasoningItems(body) body = normalizeCodexInstructions(body) body = sanitizeXAIResponsesBody(body, baseModel) @@ -715,6 +716,28 @@ func normalizeXAITools(body []byte) []byte { return updated } +// normalizeXAIToolChoiceForTools drops tool_choice and parallel_tool_calls +// when tools are absent or empty (including after normalizeXAITools filtering). +// xAI rejects payloads that include tool_choice without any tools defined. +// Existence checks avoid unnecessary sjson parse/copy passes. +func normalizeXAIToolChoiceForTools(body []byte) []byte { + tools := gjson.GetBytes(body, "tools") + hasTools := tools.Exists() && tools.IsArray() && len(tools.Array()) > 0 + if hasTools { + return body + } + if tools.Exists() { + body, _ = sjson.DeleteBytes(body, "tools") + } + if gjson.GetBytes(body, "tool_choice").Exists() { + body, _ = sjson.DeleteBytes(body, "tool_choice") + } + if gjson.GetBytes(body, "parallel_tool_calls").Exists() { + body, _ = sjson.DeleteBytes(body, "parallel_tool_calls") + } + return body +} + func normalizeXAITool(tool gjson.Result) ([]byte, bool, bool) { toolType := tool.Get("type").String() changed := false diff --git a/internal/runtime/executor/xai_executor_test.go b/internal/runtime/executor/xai_executor_test.go index 5579cd90..e8c11cf6 100644 --- a/internal/runtime/executor/xai_executor_test.go +++ b/internal/runtime/executor/xai_executor_test.go @@ -592,3 +592,57 @@ func TestXAIExecutorExecuteVideosUsesNativeEndpointFromRequestPath(t *testing.T) }) } } + +func TestNormalizeXAIToolChoiceForTools_DropsWhenToolsEmpty(t *testing.T) { + body := []byte(`{"model":"grok-4","tools":[],"tool_choice":"auto","parallel_tool_calls":true,"input":"hi"}`) + out := normalizeXAIToolChoiceForTools(body) + + if gjson.GetBytes(out, "tools").Exists() { + t.Fatalf("empty tools should be removed: %s", string(out)) + } + if gjson.GetBytes(out, "tool_choice").Exists() { + t.Fatalf("tool_choice should be removed when tools empty: %s", string(out)) + } + if gjson.GetBytes(out, "parallel_tool_calls").Exists() { + t.Fatalf("parallel_tool_calls should be removed when tools empty: %s", string(out)) + } +} + +func TestNormalizeXAIToolChoiceForTools_DropsWhenToolsMissing(t *testing.T) { + body := []byte(`{"model":"grok-4","tool_choice":"auto","input":"hi"}`) + out := normalizeXAIToolChoiceForTools(body) + + if gjson.GetBytes(out, "tool_choice").Exists() { + t.Fatalf("tool_choice should be removed when tools missing: %s", string(out)) + } +} + +func TestNormalizeXAIToolChoiceForTools_DropsOrphanedParallelToolCalls(t *testing.T) { + body := []byte(`{"model":"grok-4","parallel_tool_calls":true,"input":"hi"}`) + out := normalizeXAIToolChoiceForTools(body) + + if gjson.GetBytes(out, "parallel_tool_calls").Exists() { + t.Fatalf("parallel_tool_calls should be removed when tools missing even without tool_choice: %s", string(out)) + } +} + +func TestNormalizeXAIToolChoiceForTools_KeepsWhenToolsPresent(t *testing.T) { + body := []byte(`{"model":"grok-4","tools":[{"type":"function","name":"Bash"}],"tool_choice":"auto","input":"hi"}`) + out := normalizeXAIToolChoiceForTools(body) + + if !gjson.GetBytes(out, "tools").Exists() { + t.Fatalf("tools should be kept: %s", string(out)) + } + if got := gjson.GetBytes(out, "tool_choice").String(); got != "auto" { + t.Fatalf("tool_choice = %q, want auto: %s", got, string(out)) + } +} + +func TestNormalizeXAIToolChoiceForTools_NoOpWhenBothAbsent(t *testing.T) { + body := []byte(`{"model":"grok-4","input":"hi"}`) + out := normalizeXAIToolChoiceForTools(body) + + if gjson.GetBytes(out, "tool_choice").Exists() { + t.Fatalf("tool_choice should not appear: %s", string(out)) + } +} -- 2.51.2