From 21b73c22b9c36e36ec2d70eb9832aca6e4df30b4 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 2 Sep 2026 06:38:39 +0900 Subject: [PATCH] fix(responses): keep the reserved functions group intact for codex-spark (#3217) Codex 0.147+ on Responses Lite ships every ordinary client tool inside the reserved `functions` namespace group, carried in an `additional_tools` input item. stripSparkCompatibility() flattened every namespace group for *codex-spark* models, a rule written when the only groups Codex sent were MCP-style. With the reserved group flattened the ChatGPT backend answers the code-mode call as custom_tool_call { name: "exec", namespace: "exec" }; codex-rs treats only None/""/"functions" as the default namespace, so it concatenates that into the unroutable `execexec` and re-issues the call every turn. Bypassing the proxy sends the group intact and works. Traced on a live dev proxy with a tap on both sides: flattened group -> namespace:"exec" back, turn loops; group intact -> bare `exec` back, pwd runs, turn completes. - stripSparkCompatibility keeps a `functions` group as a group, still filtering its children (tool_search dropped, defer_loading stripped) and admitting `custom` inside it, which is what the direct client sends. MCP-style groups are flattened as before. - Belt to that suspender: scrub a tool-call `namespace` that repeats the call's own `name` on the client-facing passthrough (SSE and bounded JSON). That shape is never a legitimate identity, so no catalog lookup. Fixes #3217. --- src/adapters/openai-responses.ts | 45 +++++- .../responses-self-named-namespace-scrub.ts | 63 +++++++++ src/server/responses/core.ts | 8 +- tests/openai-responses-passthrough.test.ts | 55 ++++++++ ...sponses-self-named-namespace-scrub.test.ts | 132 ++++++++++++++++++ 5 files changed, 299 insertions(+), 4 deletions(-) create mode 100644 src/server/responses-self-named-namespace-scrub.ts create mode 100644 tests/responses-self-named-namespace-scrub.test.ts diff --git a/src/adapters/openai-responses.ts b/src/adapters/openai-responses.ts index 0209bc63c3e..781bbef4145 100644 --- a/src/adapters/openai-responses.ts +++ b/src/adapters/openai-responses.ts @@ -467,7 +467,12 @@ function normalizeConfiguredReasoningSummaryDelivery( * namespace, tool_search, web_search, custom) plus extensions (defer_loading, * parallel_tool_calls, tool_search_call/output items). Spark's serving path only * supports flat function tools and hosted web_search. This function: - * - Flattens namespace tools → promotes inner functions to top level + * - Flattens MCP-style namespace tools → promotes inner functions to top level. The reserved + * `functions` group is kept as a group (#3217): Codex 0.147+ sends every ordinary client tool + * inside it on Responses Lite, the backend accepts the group as-is, and flattening it changes + * what the backend answers with — a `custom_tool_call` carrying `namespace: "exec"`, which + * codex-rs concatenates into the unroutable `execexec`. Traced on a live proxy: with the + * group intact the same backend returns the bare `exec` call and the turn completes. * - Drops unsupported tool types (tool_search, custom) * - Strips defer_loading from function tools * - Strips namespace from input items @@ -482,12 +487,39 @@ function stripSparkCompatibility(body: unknown): unknown { let changed = false; const SPARK_SAFE_TOOL_TYPES = new Set(["function", "web_search", "web_search_preview"]); + // Inside the reserved group Codex sends freeform `custom` tools (code-mode `exec`) and the + // backend accepts them there; the top-level "drop custom" rule stays for flattened groups. + const SPARK_SAFE_FUNCTIONS_GROUP_CHILD_TYPES = new Set(["function", "custom"]); + const filterSparkFunctionsGroup = (group: Record): Record | undefined => { + if (!Array.isArray(group.tools)) return undefined; + let groupChanged = false; + const children: unknown[] = []; + for (const child of group.tools) { + if (!isPlainObject(child) || typeof child.type !== "string" || !SPARK_SAFE_FUNCTIONS_GROUP_CHILD_TYPES.has(child.type)) { + groupChanged = true; + continue; + } + if (child.type === "function" && "defer_loading" in child) { + const { defer_loading: _, ...rest } = child; + groupChanged = true; + children.push(rest); + continue; + } + children.push(child); + } + if (children.length === 0) return undefined; + return groupChanged ? { ...group, tools: children } : group; + }; let tools = body.tools; if (Array.isArray(tools)) { const flattened: unknown[] = []; for (const t of tools) { - if (isPlainObject(t) && t.type === "namespace") { + if (isPlainObject(t) && t.type === "namespace" && t.name === SPARK_RESERVED_FUNCTIONS_NAMESPACE) { + const kept = filterSparkFunctionsGroup(t); + if (kept !== t) changed = true; + if (kept) flattened.push(kept); + } else if (isPlainObject(t) && t.type === "namespace") { changed = true; if (Array.isArray(t.tools)) { for (const inner of t.tools) flattened.push(inner); @@ -527,7 +559,11 @@ function stripSparkCompatibility(body: unknown): unknown { const innerTools = item.tools as unknown[]; const filteredInner: unknown[] = []; for (const t of innerTools) { - if (isPlainObject(t) && t.type === "namespace") { + if (isPlainObject(t) && t.type === "namespace" && t.name === SPARK_RESERVED_FUNCTIONS_NAMESPACE) { + const kept = filterSparkFunctionsGroup(t); + if (kept !== t) changed = true; + if (kept) filteredInner.push(kept); + } else if (isPlainObject(t) && t.type === "namespace") { changed = true; if (Array.isArray(t.tools)) { for (const fn of t.tools) filteredInner.push(fn); @@ -574,6 +610,9 @@ function isPlainObject(v: unknown): v is Record { return !!v && typeof v === "object" && !Array.isArray(v); } +/** Codex's reserved client-tool group on Responses Lite; carries no wire prefix. */ +const SPARK_RESERVED_FUNCTIONS_NAMESPACE = "functions"; + /** * Apply the routed provider's real effort ladder to an existing Responses reasoning field. * Native forward requests keep the server-owned native clamp; unknown third-party ladders stay diff --git a/src/server/responses-self-named-namespace-scrub.ts b/src/server/responses-self-named-namespace-scrub.ts new file mode 100644 index 00000000000..82be205f1bb --- /dev/null +++ b/src/server/responses-self-named-namespace-scrub.ts @@ -0,0 +1,63 @@ +import type { SsePayloadRewrite } from "./sse-payload-rewrite"; + +function isPlainObject(value: unknown): value is Record { + return !!value && typeof value === "object" && !Array.isArray(value); +} + +/** + * Drop a tool-call `namespace` that merely repeats the call's own `name` (#3217). + * + * codex-rs resolves a client tool call as `ToolName::new(namespace, name)` and treats only + * `None | "" | "functions"` as the default namespace; anything else is concatenated into a flat + * name before routing. A backend answer of `{ name: "exec", namespace: "exec" }` therefore + * becomes `execexec`, which no client tool matches, and Codex re-issues the same call forever. + * That shape is never a legitimate identity — an MCP namespace is a server name, not the tool — + * so it is safe to scrub without consulting the declared catalog. The adapter fix that stops + * provoking the answer lives in `stripSparkCompatibility`; this is the belt to that suspender. + */ +export function scrubSelfNamedToolCallNamespace(value: unknown): { value: unknown; changed: boolean } { + if (Array.isArray(value)) { + let changed = false; + const out = value.map(entry => { + const result = scrubSelfNamedToolCallNamespace(entry); + changed ||= result.changed; + return result.value; + }); + return changed ? { value: out, changed: true } : { value, changed: false }; + } + if (!isPlainObject(value)) return { value, changed: false }; + let changed = false; + const out: Record = {}; + for (const [key, entry] of Object.entries(value)) { + const result = scrubSelfNamedToolCallNamespace(entry); + out[key] = result.value; + changed ||= result.changed; + } + if ( + (value.type === "custom_tool_call" || value.type === "function_call") + && typeof value.name === "string" + && value.name.length > 0 + && value.namespace === value.name + ) { + delete out.namespace; + changed = true; + } + return changed ? { value: out, changed: true } : { value, changed: false }; +} + +export function scrubSelfNamedToolCallNamespaceInJson(text: string): string { + if (!text.includes("\"namespace\"")) return text; + let payload: unknown; + try { + payload = JSON.parse(text); + } catch { + return text; + } + const result = scrubSelfNamedToolCallNamespace(payload); + return result.changed ? JSON.stringify(result.value) : text; +} + +export function createSelfNamedToolCallNamespaceScrubRewrite(): SsePayloadRewrite { + return payload => scrubSelfNamedToolCallNamespaceInJson(payload); +} + diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 1e84aab9678..daba1cae30f 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -326,6 +326,10 @@ import { restoreImageGenCallsInJson, } from "../responses-image-gen-repair"; import { createResponsesModelPayloadRewrite, rewriteResponsesModelJson } from "../responses-model-rewrite"; +import { + createSelfNamedToolCallNamespaceScrubRewrite, + scrubSelfNamedToolCallNamespaceInJson, +} from "../responses-self-named-namespace-scrub"; import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog"; import { buildToolBridgeMaps, collabSurface, injectDeveloperMessage, multiAgentGuidanceText } from "./collaboration"; @@ -4641,6 +4645,8 @@ async function handleResponsesInner( // Compose opt-in payload rewrites into one parse/stringify pass (image-gen restore first). const payloadRewrites = [ createImageGenCallRestoreRewrite(imageGenCallAliases), + // #3217: a call whose namespace repeats its own name is unroutable in codex-rs. + createSelfNamedToolCallNamespaceScrubRewrite(), routedNamespaceToolAliases.size > 0 ? createRoutedNamespaceCallRestoreRewrite(routedNamespaceToolAliases) : undefined, @@ -4873,7 +4879,7 @@ async function handleResponsesInner( inspectResponseLogJson(logCtx, text); const clientJson = (() => { const restoredNamespace = restoreRoutedNamespaceCallsInJson( - restoreImageGenCallsInJson(text, imageGenCallAliases), + scrubSelfNamedToolCallNamespaceInJson(restoreImageGenCallsInJson(text, imageGenCallAliases)), routedNamespaceToolAliases, ); const restoredAuthorizedBareNamespace = restoreRoutedNamespaceCallsInJson( diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index a1972dc1fbe..83cf68fe454 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -1717,6 +1717,61 @@ describe("OpenAI Responses passthrough sanitization", () => { }); }); + test("keeps the reserved functions group intact for codex-spark, flattens MCP groups (#3217)", () => { + // Codex 0.147+ on Responses Lite ships every ordinary client tool inside the reserved + // `functions` namespace group, carried in an `additional_tools` input item. Flattening that + // group made the backend answer `custom_tool_call { name: "exec", namespace: "exec" }`, + // which codex-rs concatenates into the unroutable `execexec` and loops on. + const adapter = createResponsesPassthroughAdapter(provider); + const functionsGroup = { + type: "namespace", + name: "functions", + description: "client tools", + tools: [ + { type: "custom", name: "exec", description: "shell" }, + { type: "function", name: "wait", parameters: { type: "object", properties: {} }, defer_loading: true }, + { type: "tool_search", name: "tool_search" }, + ], + }; + const mcpGroup = { + type: "namespace", + name: "mcp__docs", + tools: [{ type: "function", name: "search", parameters: { type: "object", properties: {} } }], + }; + const request = adapter.buildRequest({ + modelId: "gpt-5.3-codex-spark", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "gpt-5.3-codex-spark", + input: [ + { type: "additional_tools", role: "developer", tools: [functionsGroup, mcpGroup] }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run pwd" }] }, + ], + tools: [functionsGroup, mcpGroup], + }, + }, { headers: new Headers({ authorization: "Bearer token" }) }); + const body = JSON.parse(request.body) as { + tools: Array>; + input: Array<{ type: string; tools?: Array> }>; + }; + const expectedGroup = { + type: "namespace", + name: "functions", + description: "client tools", + tools: [ + { type: "custom", name: "exec", description: "shell" }, + { type: "function", name: "wait", parameters: { type: "object", properties: {} } }, + ], + }; + // The reserved group survives as a group with its custom child; tool_search is still dropped + // and defer_loading still stripped inside it. The MCP group is still flattened. + expect(body.tools).toEqual([expectedGroup, { type: "function", name: "search", parameters: { type: "object", properties: {} } }]); + const additional = body.input.find(item => item.type === "additional_tools"); + expect(additional?.tools).toEqual([expectedGroup, { type: "function", name: "search", parameters: { type: "object", properties: {} } }]); + }); + test("strips image_generation hosted tool for codex-spark passthrough", () => { const adapter = createResponsesPassthroughAdapter(provider); const request = adapter.buildRequest({ diff --git a/tests/responses-self-named-namespace-scrub.test.ts b/tests/responses-self-named-namespace-scrub.test.ts new file mode 100644 index 00000000000..b4bd1fcebd2 --- /dev/null +++ b/tests/responses-self-named-namespace-scrub.test.ts @@ -0,0 +1,132 @@ +/** + * #3217 — a custom_tool_call whose `namespace` repeats its own `name` must not reach Codex. + * + * codex-rs resolves `ToolName::new(namespace, name)` and only treats None/""/"functions" as the + * default namespace; `{ name: "exec", namespace: "exec" }` becomes the flat name `execexec`, + * which no client tool matches, and Codex re-issues the call every turn. The adapter fix keeps + * the reserved `functions` group intact so the backend stops answering that way; this scrub is + * the belt to that suspender on the client-facing passthrough (SSE and bounded JSON). + */ +import { afterEach, expect, test } from "bun:test"; +import { handleResponses } from "../src/server/responses"; +import { scrubSelfNamedToolCallNamespace } from "../src/server/responses-self-named-namespace-scrub"; +import type { OcxConfig } from "../src/types"; + +const originalFetch = globalThis.fetch; +afterEach(() => { globalThis.fetch = originalFetch; }); + +function forwardConfig(): OcxConfig { + return { + port: 0, + defaultProvider: "openai", + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "direct", + }, + }, + } as unknown as OcxConfig; +} + +const requestBody = { + model: "gpt-5.3-codex-spark", + stream: true, + store: false, + instructions: "x", + input: [ + { + type: "additional_tools", + role: "developer", + tools: [{ + type: "namespace", + name: "functions", + tools: [ + { type: "custom", name: "exec", description: "shell" }, + { type: "function", name: "wait", parameters: { type: "object", properties: {} } }, + ], + }], + }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run pwd" }] }, + ], +}; + +function sseFrom(items: Array>): string { + const events = [ + { type: "response.output_item.added", output_index: 0, item: { ...items[0], input: "", status: "in_progress" } }, + { type: "response.output_item.done", output_index: 0, item: items[0] }, + { type: "response.completed", response: { id: "r1", status: "completed", output: items } }, + ]; + return events.map(e => `event: ${e.type}\ndata: ${JSON.stringify(e)}\n\n`).join(""); +} + +function request(): Request { + return new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json", authorization: "Bearer test", "chatgpt-account-id": "acct" }, + body: JSON.stringify(requestBody), + }); +} + +test("a self-named namespace on a passthrough custom_tool_call is scrubbed before the client (#3217)", async () => { + const call = { type: "custom_tool_call", id: "ctc_1", call_id: "call_1", name: "exec", namespace: "exec", input: "pwd", status: "completed" }; + globalThis.fetch = (async () => new Response(sseFrom([call]), { + status: 200, headers: { "content-type": "text/event-stream" }, + })) as typeof fetch; + + const res = await handleResponses(request(), forwardConfig(), { model: "", provider: "" }); + expect(res.status).toBe(200); + const text = await res.text(); + const payloads = text.split("\n").filter(l => l.startsWith("data: ") && l !== "data: [DONE]").map(l => JSON.parse(l.slice(6)) as Record); + const callItems = payloads.flatMap(p => { + const item = p.item as Record | undefined; + const output = (p.response as { output?: Array> } | undefined)?.output ?? []; + return [...(item ? [item] : []), ...output]; + }).filter(i => i.type === "custom_tool_call"); + expect(callItems.length).toBeGreaterThanOrEqual(3); + for (const item of callItems) { + expect(item.name).toBe("exec"); + expect("namespace" in item).toBe(false); + } +}); + +test("a genuine MCP namespace on a passthrough call is left alone", async () => { + const call = { type: "function_call", id: "fc_1", call_id: "call_1", name: "search", namespace: "mcp__docs", arguments: "{}", status: "completed" }; + globalThis.fetch = (async () => new Response(sseFrom([call]), { + status: 200, headers: { "content-type": "text/event-stream" }, + })) as typeof fetch; + + const res = await handleResponses(request(), forwardConfig(), { model: "", provider: "" }); + const text = await res.text(); + expect(text).toContain('"namespace":"mcp__docs"'); +}); + +test("the bounded JSON (stream:false) passthrough path scrubs the same shape (#3217)", async () => { + const call = { type: "custom_tool_call", id: "ctc_1", call_id: "call_1", name: "exec", namespace: "exec", input: "pwd", status: "completed" }; + globalThis.fetch = (async () => new Response(JSON.stringify({ id: "r1", object: "response", status: "completed", output: [call] }), { + status: 200, headers: { "content-type": "application/json" }, + })) as typeof fetch; + + const req = new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json", authorization: "Bearer test", "chatgpt-account-id": "acct" }, + body: JSON.stringify({ ...requestBody, stream: false }), + }); + const res = await handleResponses(req, forwardConfig(), { model: "", provider: "" }); + expect(res.status).toBe(200); + const body = await res.json() as { output: Array> }; + expect(body.output[0]).toMatchObject({ type: "custom_tool_call", name: "exec" }); + expect("namespace" in body.output[0]).toBe(false); +}); + +test("scrub is recursive, shape-preserving, and a no-op on clean payloads", () => { + const clean = { type: "response.completed", response: { output: [{ type: "custom_tool_call", name: "exec", input: "" }] } }; + expect(scrubSelfNamedToolCallNamespace(clean)).toEqual({ value: clean, changed: false }); + const dirty = { response: { output: [{ type: "function_call", name: "wait", namespace: "wait", arguments: "{}" }, { type: "message" }] } }; + const result = scrubSelfNamedToolCallNamespace(dirty); + expect(result.changed).toBe(true); + expect(result.value).toEqual({ response: { output: [{ type: "function_call", name: "wait", arguments: "{}" }, { type: "message" }] } }); + // An empty name never matches: a namespace equal to "" is not the self-named shape. + expect(scrubSelfNamedToolCallNamespace({ type: "custom_tool_call", name: "", namespace: "" }).changed).toBe(false); +});