From 5b2089c5a35fd4c1f83b53fd12e83efab45a7a9a Mon Sep 17 00:00:00 2001 From: Nai Long 7th Date: Thu, 20 Aug 2026 06:55:47 +0800 Subject: [PATCH] fix(grok): lower Codex tool-search discovery outputs --- .../pkg/apicompat/responses_client_tools.go | 42 ++++- .../apicompat/responses_client_tools_test.go | 160 ++++++++++++++++++ .../openai_gateway_grok_tool_protocol_test.go | 33 ++++ 3 files changed, 234 insertions(+), 1 deletion(-) diff --git a/backend/internal/pkg/apicompat/responses_client_tools.go b/backend/internal/pkg/apicompat/responses_client_tools.go index 7c529fc8cb..4237a95ff0 100644 --- a/backend/internal/pkg/apicompat/responses_client_tools.go +++ b/backend/internal/pkg/apicompat/responses_client_tools.go @@ -212,7 +212,7 @@ func rewriteClientToolHistory(value any, adapter *ResponsesClientToolMapping) bo if adapter.ToolSearch { typed["type"] = "function_call_output" dropInvalidLoweredFunctionItemID(typed) - normalizeClientToolOutput(typed) + normalizeToolSearchOutput(typed) changed = true } } @@ -256,6 +256,46 @@ func normalizeClientToolOutput(item map[string]any) { item["output"] = string(encoded) } +// normalizeToolSearchOutput converts both tool_search output wire shapes into +// the string output required by function_call_output. Older clients send an +// output field directly; newer Codex clients return discovered definitions in +// a top-level tools field. Codex treats that field's value as the tool output, +// so serialize the value directly rather than wrapping it in another object. +func normalizeToolSearchOutput(item map[string]any) { + if output, hasOutput := item["output"]; hasOutput { + switch typed := output.(type) { + case string: + item["output"] = typed + case nil: + item["output"] = "" + default: + encoded, err := json.Marshal(typed) + if err != nil { + return + } + item["output"] = string(encoded) + } + dropToolSearchOutputPrivateFields(item) + return + } + tools, hasTools := item["tools"] + if !hasTools { + return + } + encoded, err := json.Marshal(tools) + if err != nil { + return + } + item["output"] = string(encoded) + dropToolSearchOutputPrivateFields(item) +} + +func dropToolSearchOutputPrivateFields(item map[string]any) { + delete(item, "tools") + delete(item, "status") + delete(item, "execution") +} + func rewriteClientToolChoice(req map[string]any, adapter *ResponsesClientToolMapping) bool { choice, ok := req["tool_choice"].(map[string]any) if !ok { diff --git a/backend/internal/pkg/apicompat/responses_client_tools_test.go b/backend/internal/pkg/apicompat/responses_client_tools_test.go index 0836e22d64..41c436374a 100644 --- a/backend/internal/pkg/apicompat/responses_client_tools_test.go +++ b/backend/internal/pkg/apicompat/responses_client_tools_test.go @@ -66,6 +66,166 @@ func TestAdaptResponsesClientTools_LowersDeclarationsHistoryChoiceAndNamespaces( require.Equal(t, "team__send", namespaceCall["name"]) } +func TestAdaptResponsesClientTools_LowersDiscoveredToolSearchOutput(t *testing.T) { + requestJSON := `{ + "tools":[{"type":"tool_search"}], + "input":[ + {"type":"tool_search_call","id":"tsc_client","call_id":"call_search","arguments":{"query":"codex app"},"execution":"client","status":"completed"}, + {"type":"tool_search_output","id":"tso_client","call_id":"call_search","execution":"client","status":"completed","tools":[ + {"type":"namespace","name":"codex_app","tools":[{"type":"function","name":"load_workspace_dependencies","description":"Load workspace dependencies","parameters":{"type":"object","properties":{},"additionalProperties":false}}]}, + {"type":"namespace","name":"multi_agent_v1","tools":[{"type":"function","name":"spawn_agent","description":"Spawn an agent","parameters":{"type":"object","properties":{"message":{"type":"string"}},"required":["message"],"additionalProperties":false}}]} + ]} + ] + }` + + adapt := func() map[string]any { + var req map[string]any + require.NoError(t, json.Unmarshal([]byte(requestJSON), &req)) + _, changed, err := AdaptResponsesClientTools(req) + require.NoError(t, err) + require.True(t, changed) + return req + } + + first := adapt() + second := adapt() + firstInput := requireResponsesClientToolValue[[]any](t, first["input"]) + secondInput := requireResponsesClientToolValue[[]any](t, second["input"]) + + call := requireResponsesClientToolValue[map[string]any](t, firstInput[0]) + require.Equal(t, "function_call", call["type"]) + require.Equal(t, toolSearchProxyName, call["name"]) + require.JSONEq(t, `{"query":"codex app"}`, requireResponsesClientToolValue[string](t, call["arguments"])) + require.NotContains(t, call, "execution") + + output := requireResponsesClientToolValue[map[string]any](t, firstInput[1]) + require.Equal(t, map[string]any{ + "type": "function_call_output", + "call_id": "call_search", + "output": output["output"], + }, output) + outputText := requireResponsesClientToolValue[string](t, output["output"]) + require.JSONEq(t, `[ + {"type":"namespace","name":"codex_app","tools":[{"type":"function","name":"load_workspace_dependencies","description":"Load workspace dependencies","parameters":{"type":"object","properties":{},"additionalProperties":false}}]}, + {"type":"namespace","name":"multi_agent_v1","tools":[{"type":"function","name":"spawn_agent","description":"Spawn an agent","parameters":{"type":"object","properties":{"message":{"type":"string"}},"required":["message"],"additionalProperties":false}}]} + ]`, outputText) + secondOutput := requireResponsesClientToolValue[map[string]any](t, secondInput[1]) + require.Equal(t, outputText, secondOutput["output"], "tool discovery output encoding must be deterministic") +} + +func TestAdaptResponsesClientTools_ToolSearchOutputEdgeCases(t *testing.T) { + unencodableOutput := make(chan struct{}) + tests := []struct { + name string + item map[string]any + wantOutput any + wantOutputExists bool + wantPrivateKeys []string + wantExactOutput bool + }{ + { + name: "absent tools and output remains visibly malformed", + item: map[string]any{"type": "tool_search_output", "call_id": "call_empty", "status": "completed"}, + wantOutputExists: false, + wantPrivateKeys: []string{"status"}, + }, + { + name: "preexisting string output wins", + item: map[string]any{ + "type": "tool_search_output", "call_id": "call_legacy", "output": "legacy", + "tools": []any{map[string]any{"type": "function", "name": "ignored"}}, "execution": "client", + }, + wantOutput: "legacy", + wantOutputExists: true, + wantExactOutput: true, + }, + { + name: "preexisting object output remains legacy representation", + item: map[string]any{ + "type": "tool_search_output", "call_id": "call_object", "output": map[string]any{"groups": []any{"github"}}, + "tools": []any{map[string]any{"type": "function", "name": "ignored"}}, + }, + wantOutput: `{"groups":["github"]}`, + wantOutputExists: true, + wantExactOutput: true, + }, + { + name: "unencodable preexisting output remains visibly malformed", + item: map[string]any{ + "type": "tool_search_output", "call_id": "call_bad_output", "output": unencodableOutput, + "tools": []any{map[string]any{"type": "function", "name": "retained"}}, "status": "completed", "execution": "client", + }, + wantOutput: unencodableOutput, + wantOutputExists: true, + wantPrivateKeys: []string{"tools", "status", "execution"}, + }, + { + name: "empty tools array is a valid empty output", + item: map[string]any{ + "type": "tool_search_output", "call_id": "call_empty_tools", + "tools": []any{}, "status": "completed", "execution": "client", + }, + wantOutput: `[]`, + wantOutputExists: true, + wantExactOutput: true, + }, + { + name: "non-array tools value is serialized directly", + item: map[string]any{ + "type": "tool_search_output", "call_id": "call_malformed", + "tools": map[string]any{"unexpected": true}, "status": "completed", "execution": "client", + }, + wantOutput: `{"unexpected":true}`, + wantOutputExists: true, + wantExactOutput: true, + }, + { + name: "unencodable tools remains visibly malformed", + item: map[string]any{ + "type": "tool_search_output", "call_id": "call_unencodable", "tools": make(chan struct{}), "status": "completed", + }, + wantOutputExists: false, + wantPrivateKeys: []string{"tools", "status"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := map[string]any{ + "tools": []any{map[string]any{"type": "tool_search"}}, + "input": []any{tt.item}, + } + _, changed, err := AdaptResponsesClientTools(req) + require.NoError(t, err) + require.True(t, changed) + input := requireResponsesClientToolValue[[]any](t, req["input"]) + output := requireResponsesClientToolValue[map[string]any](t, input[0]) + require.Equal(t, "function_call_output", output["type"]) + actualOutput, outputExists := output["output"] + require.Equal(t, tt.wantOutputExists, outputExists) + if tt.wantOutputExists { + require.Equal(t, tt.wantOutput, actualOutput) + } + if tt.wantExactOutput { + require.Equal(t, map[string]any{ + "type": "function_call_output", + "call_id": output["call_id"], + "output": tt.wantOutput, + }, output) + } + if len(tt.wantPrivateKeys) > 0 { + for _, key := range tt.wantPrivateKeys { + require.Contains(t, output, key) + } + } else { + require.NotContains(t, output, "tools") + require.NotContains(t, output, "status") + require.NotContains(t, output, "execution") + } + }) + } +} + func requireResponsesClientToolValue[T any](t *testing.T, value any) T { t.Helper() typed, ok := value.(T) diff --git a/backend/internal/service/openai_gateway_grok_tool_protocol_test.go b/backend/internal/service/openai_gateway_grok_tool_protocol_test.go index bca3d3f2bd..50efdbfce8 100644 --- a/backend/internal/service/openai_gateway_grok_tool_protocol_test.go +++ b/backend/internal/service/openai_gateway_grok_tool_protocol_test.go @@ -65,6 +65,39 @@ func TestPatchGrokResponsesBodyWithClientToolsLowersCodexProtocol(t *testing.T) require.False(t, gjson.GetBytes(patched, "input.4.namespace").Exists()) } +func TestPatchGrokResponsesBodyWithClientToolsLowersDiscoveredToolsOutput(t *testing.T) { + t.Parallel() + + body := []byte(`{ + "model":"grok-4.5", + "tools":[{"type":"tool_search"}], + "input":[ + {"type":"tool_search_call","id":"tsc_request_74547","call_id":"call_request_74547","arguments":{"query":"subagent"},"execution":"client","status":"completed"}, + {"type":"tool_search_output","id":"tso_request_74547","call_id":"call_request_74547","execution":"client","status":"completed","tools":[ + {"type":"namespace","name":"codex_app","tools":[{"type":"function","name":"load_workspace_dependencies","parameters":{"type":"object","properties":{},"additionalProperties":false}}]}, + {"type":"namespace","name":"multi_agent_v1","tools":[{"type":"function","name":"spawn_agent","parameters":{"type":"object","properties":{"message":{"type":"string"}},"required":["message"],"additionalProperties":false}}]} + ]} + ] + }`) + + patched, mapping, err := patchGrokResponsesBodyWithClientTools(body, "grok-4.5") + require.NoError(t, err) + require.True(t, mapping.ToolSearch) + require.Equal(t, "function_call", gjson.GetBytes(patched, "input.0.type").String()) + require.Equal(t, "tool_search", gjson.GetBytes(patched, "input.0.name").String()) + require.Equal(t, "function_call_output", gjson.GetBytes(patched, "input.1.type").String()) + require.Equal(t, "call_request_74547", gjson.GetBytes(patched, "input.1.call_id").String()) + require.Len(t, gjson.GetBytes(patched, "input.1").Map(), 3) + require.False(t, gjson.GetBytes(patched, "input.1.tools").Exists()) + require.False(t, gjson.GetBytes(patched, "input.1.status").Exists()) + require.False(t, gjson.GetBytes(patched, "input.1.execution").Exists()) + output := gjson.GetBytes(patched, "input.1.output").String() + require.JSONEq(t, `[ + {"type":"namespace","name":"codex_app","tools":[{"type":"function","name":"load_workspace_dependencies","parameters":{"type":"object","properties":{},"additionalProperties":false}}]}, + {"type":"namespace","name":"multi_agent_v1","tools":[{"type":"function","name":"spawn_agent","parameters":{"type":"object","properties":{"message":{"type":"string"}},"required":["message"],"additionalProperties":false}}]} + ]`, output) +} + func TestPatchGrokResponsesBodyWithClientToolsRewritesEveryToolChoice(t *testing.T) { t.Parallel()