Skip to content

Commit fe855b3

Browse files
lidge-junjun
andauthored
fix(responses): keep the reserved functions group intact for codex-spark (lidge-jun#3217) (lidge-jun#3224)
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 lidge-jun#3217. Co-authored-by: jun <[email protected]>
1 parent 0a3ca98 commit fe855b3

5 files changed

Lines changed: 299 additions & 4 deletions

File tree

‎src/adapters/openai-responses.ts‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,12 @@ function normalizeConfiguredReasoningSummaryDelivery(
467467
* namespace, tool_search, web_search, custom) plus extensions (defer_loading,
468468
* parallel_tool_calls, tool_search_call/output items). Spark's serving path only
469469
* supports flat function tools and hosted web_search. This function:
470-
* - Flattens namespace tools → promotes inner functions to top level
470+
* - Flattens MCP-style namespace tools → promotes inner functions to top level. The reserved
471+
* `functions` group is kept as a group (#3217): Codex 0.147+ sends every ordinary client tool
472+
* inside it on Responses Lite, the backend accepts the group as-is, and flattening it changes
473+
* what the backend answers with — a `custom_tool_call` carrying `namespace: "exec"`, which
474+
* codex-rs concatenates into the unroutable `execexec`. Traced on a live proxy: with the
475+
* group intact the same backend returns the bare `exec` call and the turn completes.
471476
* - Drops unsupported tool types (tool_search, custom)
472477
* - Strips defer_loading from function tools
473478
* - Strips namespace from input items
@@ -482,12 +487,39 @@ function stripSparkCompatibility(body: unknown): unknown {
482487
let changed = false;
483488

484489
const SPARK_SAFE_TOOL_TYPES = new Set(["function", "web_search", "web_search_preview"]);
490+
// Inside the reserved group Codex sends freeform `custom` tools (code-mode `exec`) and the
491+
// backend accepts them there; the top-level "drop custom" rule stays for flattened groups.
492+
const SPARK_SAFE_FUNCTIONS_GROUP_CHILD_TYPES = new Set(["function", "custom"]);
493+
const filterSparkFunctionsGroup = (group: Record<string, unknown>): Record<string, unknown> | undefined => {
494+
if (!Array.isArray(group.tools)) return undefined;
495+
let groupChanged = false;
496+
const children: unknown[] = [];
497+
for (const child of group.tools) {
498+
if (!isPlainObject(child) || typeof child.type !== "string" || !SPARK_SAFE_FUNCTIONS_GROUP_CHILD_TYPES.has(child.type)) {
499+
groupChanged = true;
500+
continue;
501+
}
502+
if (child.type === "function" && "defer_loading" in child) {
503+
const { defer_loading: _, ...rest } = child;
504+
groupChanged = true;
505+
children.push(rest);
506+
continue;
507+
}
508+
children.push(child);
509+
}
510+
if (children.length === 0) return undefined;
511+
return groupChanged ? { ...group, tools: children } : group;
512+
};
485513

486514
let tools = body.tools;
487515
if (Array.isArray(tools)) {
488516
const flattened: unknown[] = [];
489517
for (const t of tools) {
490-
if (isPlainObject(t) && t.type === "namespace") {
518+
if (isPlainObject(t) && t.type === "namespace" && t.name === SPARK_RESERVED_FUNCTIONS_NAMESPACE) {
519+
const kept = filterSparkFunctionsGroup(t);
520+
if (kept !== t) changed = true;
521+
if (kept) flattened.push(kept);
522+
} else if (isPlainObject(t) && t.type === "namespace") {
491523
changed = true;
492524
if (Array.isArray(t.tools)) {
493525
for (const inner of t.tools) flattened.push(inner);
@@ -527,7 +559,11 @@ function stripSparkCompatibility(body: unknown): unknown {
527559
const innerTools = item.tools as unknown[];
528560
const filteredInner: unknown[] = [];
529561
for (const t of innerTools) {
530-
if (isPlainObject(t) && t.type === "namespace") {
562+
if (isPlainObject(t) && t.type === "namespace" && t.name === SPARK_RESERVED_FUNCTIONS_NAMESPACE) {
563+
const kept = filterSparkFunctionsGroup(t);
564+
if (kept !== t) changed = true;
565+
if (kept) filteredInner.push(kept);
566+
} else if (isPlainObject(t) && t.type === "namespace") {
531567
changed = true;
532568
if (Array.isArray(t.tools)) {
533569
for (const fn of t.tools) filteredInner.push(fn);
@@ -574,6 +610,9 @@ function isPlainObject(v: unknown): v is Record<string, unknown> {
574610
return !!v && typeof v === "object" && !Array.isArray(v);
575611
}
576612

613+
/** Codex's reserved client-tool group on Responses Lite; carries no wire prefix. */
614+
const SPARK_RESERVED_FUNCTIONS_NAMESPACE = "functions";
615+
577616
/**
578617
* Apply the routed provider's real effort ladder to an existing Responses reasoning field.
579618
* Native forward requests keep the server-owned native clamp; unknown third-party ladders stay
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import type { SsePayloadRewrite } from "./sse-payload-rewrite";
2+
3+
function isPlainObject(value: unknown): value is Record<string, unknown> {
4+
return !!value && typeof value === "object" && !Array.isArray(value);
5+
}
6+
7+
/**
8+
* Drop a tool-call `namespace` that merely repeats the call's own `name` (#3217).
9+
*
10+
* codex-rs resolves a client tool call as `ToolName::new(namespace, name)` and treats only
11+
* `None | "" | "functions"` as the default namespace; anything else is concatenated into a flat
12+
* name before routing. A backend answer of `{ name: "exec", namespace: "exec" }` therefore
13+
* becomes `execexec`, which no client tool matches, and Codex re-issues the same call forever.
14+
* That shape is never a legitimate identity — an MCP namespace is a server name, not the tool —
15+
* so it is safe to scrub without consulting the declared catalog. The adapter fix that stops
16+
* provoking the answer lives in `stripSparkCompatibility`; this is the belt to that suspender.
17+
*/
18+
export function scrubSelfNamedToolCallNamespace(value: unknown): { value: unknown; changed: boolean } {
19+
if (Array.isArray(value)) {
20+
let changed = false;
21+
const out = value.map(entry => {
22+
const result = scrubSelfNamedToolCallNamespace(entry);
23+
changed ||= result.changed;
24+
return result.value;
25+
});
26+
return changed ? { value: out, changed: true } : { value, changed: false };
27+
}
28+
if (!isPlainObject(value)) return { value, changed: false };
29+
let changed = false;
30+
const out: Record<string, unknown> = {};
31+
for (const [key, entry] of Object.entries(value)) {
32+
const result = scrubSelfNamedToolCallNamespace(entry);
33+
out[key] = result.value;
34+
changed ||= result.changed;
35+
}
36+
if (
37+
(value.type === "custom_tool_call" || value.type === "function_call")
38+
&& typeof value.name === "string"
39+
&& value.name.length > 0
40+
&& value.namespace === value.name
41+
) {
42+
delete out.namespace;
43+
changed = true;
44+
}
45+
return changed ? { value: out, changed: true } : { value, changed: false };
46+
}
47+
48+
export function scrubSelfNamedToolCallNamespaceInJson(text: string): string {
49+
if (!text.includes("\"namespace\"")) return text;
50+
let payload: unknown;
51+
try {
52+
payload = JSON.parse(text);
53+
} catch {
54+
return text;
55+
}
56+
const result = scrubSelfNamedToolCallNamespace(payload);
57+
return result.changed ? JSON.stringify(result.value) : text;
58+
}
59+
60+
export function createSelfNamedToolCallNamespaceScrubRewrite(): SsePayloadRewrite {
61+
return payload => scrubSelfNamedToolCallNamespaceInJson(payload);
62+
}
63+

‎src/server/responses/core.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,10 @@ import {
326326
restoreImageGenCallsInJson,
327327
} from "../responses-image-gen-repair";
328328
import { createResponsesModelPayloadRewrite, rewriteResponsesModelJson } from "../responses-model-rewrite";
329+
import {
330+
createSelfNamedToolCallNamespaceScrubRewrite,
331+
scrubSelfNamedToolCallNamespaceInJson,
332+
} from "../responses-self-named-namespace-scrub";
329333
import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog";
330334

331335
import { buildToolBridgeMaps, collabSurface, injectDeveloperMessage, multiAgentGuidanceText } from "./collaboration";
@@ -4641,6 +4645,8 @@ async function handleResponsesInner(
46414645
// Compose opt-in payload rewrites into one parse/stringify pass (image-gen restore first).
46424646
const payloadRewrites = [
46434647
createImageGenCallRestoreRewrite(imageGenCallAliases),
4648+
// #3217: a call whose namespace repeats its own name is unroutable in codex-rs.
4649+
createSelfNamedToolCallNamespaceScrubRewrite(),
46444650
routedNamespaceToolAliases.size > 0
46454651
? createRoutedNamespaceCallRestoreRewrite(routedNamespaceToolAliases)
46464652
: undefined,
@@ -4873,7 +4879,7 @@ async function handleResponsesInner(
48734879
inspectResponseLogJson(logCtx, text);
48744880
const clientJson = (() => {
48754881
const restoredNamespace = restoreRoutedNamespaceCallsInJson(
4876-
restoreImageGenCallsInJson(text, imageGenCallAliases),
4882+
scrubSelfNamedToolCallNamespaceInJson(restoreImageGenCallsInJson(text, imageGenCallAliases)),
48774883
routedNamespaceToolAliases,
48784884
);
48794885
const restoredAuthorizedBareNamespace = restoreRoutedNamespaceCallsInJson(

‎tests/openai-responses-passthrough.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1717,6 +1717,61 @@ describe("OpenAI Responses passthrough sanitization", () => {
17171717
});
17181718
});
17191719

1720+
test("keeps the reserved functions group intact for codex-spark, flattens MCP groups (#3217)", () => {
1721+
// Codex 0.147+ on Responses Lite ships every ordinary client tool inside the reserved
1722+
// `functions` namespace group, carried in an `additional_tools` input item. Flattening that
1723+
// group made the backend answer `custom_tool_call { name: "exec", namespace: "exec" }`,
1724+
// which codex-rs concatenates into the unroutable `execexec` and loops on.
1725+
const adapter = createResponsesPassthroughAdapter(provider);
1726+
const functionsGroup = {
1727+
type: "namespace",
1728+
name: "functions",
1729+
description: "client tools",
1730+
tools: [
1731+
{ type: "custom", name: "exec", description: "shell" },
1732+
{ type: "function", name: "wait", parameters: { type: "object", properties: {} }, defer_loading: true },
1733+
{ type: "tool_search", name: "tool_search" },
1734+
],
1735+
};
1736+
const mcpGroup = {
1737+
type: "namespace",
1738+
name: "mcp__docs",
1739+
tools: [{ type: "function", name: "search", parameters: { type: "object", properties: {} } }],
1740+
};
1741+
const request = adapter.buildRequest({
1742+
modelId: "gpt-5.3-codex-spark",
1743+
context: { messages: [] },
1744+
stream: true,
1745+
options: {},
1746+
_rawBody: {
1747+
model: "gpt-5.3-codex-spark",
1748+
input: [
1749+
{ type: "additional_tools", role: "developer", tools: [functionsGroup, mcpGroup] },
1750+
{ type: "message", role: "user", content: [{ type: "input_text", text: "run pwd" }] },
1751+
],
1752+
tools: [functionsGroup, mcpGroup],
1753+
},
1754+
}, { headers: new Headers({ authorization: "Bearer token" }) });
1755+
const body = JSON.parse(request.body) as {
1756+
tools: Array<Record<string, unknown>>;
1757+
input: Array<{ type: string; tools?: Array<Record<string, unknown>> }>;
1758+
};
1759+
const expectedGroup = {
1760+
type: "namespace",
1761+
name: "functions",
1762+
description: "client tools",
1763+
tools: [
1764+
{ type: "custom", name: "exec", description: "shell" },
1765+
{ type: "function", name: "wait", parameters: { type: "object", properties: {} } },
1766+
],
1767+
};
1768+
// The reserved group survives as a group with its custom child; tool_search is still dropped
1769+
// and defer_loading still stripped inside it. The MCP group is still flattened.
1770+
expect(body.tools).toEqual([expectedGroup, { type: "function", name: "search", parameters: { type: "object", properties: {} } }]);
1771+
const additional = body.input.find(item => item.type === "additional_tools");
1772+
expect(additional?.tools).toEqual([expectedGroup, { type: "function", name: "search", parameters: { type: "object", properties: {} } }]);
1773+
});
1774+
17201775
test("strips image_generation hosted tool for codex-spark passthrough", () => {
17211776
const adapter = createResponsesPassthroughAdapter(provider);
17221777
const request = adapter.buildRequest({
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* #3217 — a custom_tool_call whose `namespace` repeats its own `name` must not reach Codex.
3+
*
4+
* codex-rs resolves `ToolName::new(namespace, name)` and only treats None/""/"functions" as the
5+
* default namespace; `{ name: "exec", namespace: "exec" }` becomes the flat name `execexec`,
6+
* which no client tool matches, and Codex re-issues the call every turn. The adapter fix keeps
7+
* the reserved `functions` group intact so the backend stops answering that way; this scrub is
8+
* the belt to that suspender on the client-facing passthrough (SSE and bounded JSON).
9+
*/
10+
import { afterEach, expect, test } from "bun:test";
11+
import { handleResponses } from "../src/server/responses";
12+
import { scrubSelfNamedToolCallNamespace } from "../src/server/responses-self-named-namespace-scrub";
13+
import type { OcxConfig } from "../src/types";
14+
15+
const originalFetch = globalThis.fetch;
16+
afterEach(() => { globalThis.fetch = originalFetch; });
17+
18+
function forwardConfig(): OcxConfig {
19+
return {
20+
port: 0,
21+
defaultProvider: "openai",
22+
providers: {
23+
openai: {
24+
adapter: "openai-responses",
25+
baseUrl: "https://chatgpt.com/backend-api/codex",
26+
authMode: "forward",
27+
codexAccountMode: "direct",
28+
},
29+
},
30+
} as unknown as OcxConfig;
31+
}
32+
33+
const requestBody = {
34+
model: "gpt-5.3-codex-spark",
35+
stream: true,
36+
store: false,
37+
instructions: "x",
38+
input: [
39+
{
40+
type: "additional_tools",
41+
role: "developer",
42+
tools: [{
43+
type: "namespace",
44+
name: "functions",
45+
tools: [
46+
{ type: "custom", name: "exec", description: "shell" },
47+
{ type: "function", name: "wait", parameters: { type: "object", properties: {} } },
48+
],
49+
}],
50+
},
51+
{ type: "message", role: "user", content: [{ type: "input_text", text: "run pwd" }] },
52+
],
53+
};
54+
55+
function sseFrom(items: Array<Record<string, unknown>>): string {
56+
const events = [
57+
{ type: "response.output_item.added", output_index: 0, item: { ...items[0], input: "", status: "in_progress" } },
58+
{ type: "response.output_item.done", output_index: 0, item: items[0] },
59+
{ type: "response.completed", response: { id: "r1", status: "completed", output: items } },
60+
];
61+
return events.map(e => `event: ${e.type}\ndata: ${JSON.stringify(e)}\n\n`).join("");
62+
}
63+
64+
function request(): Request {
65+
return new Request("http://localhost/v1/responses", {
66+
method: "POST",
67+
headers: { "content-type": "application/json", authorization: "Bearer test", "chatgpt-account-id": "acct" },
68+
body: JSON.stringify(requestBody),
69+
});
70+
}
71+
72+
test("a self-named namespace on a passthrough custom_tool_call is scrubbed before the client (#3217)", async () => {
73+
const call = { type: "custom_tool_call", id: "ctc_1", call_id: "call_1", name: "exec", namespace: "exec", input: "pwd", status: "completed" };
74+
globalThis.fetch = (async () => new Response(sseFrom([call]), {
75+
status: 200, headers: { "content-type": "text/event-stream" },
76+
})) as typeof fetch;
77+
78+
const res = await handleResponses(request(), forwardConfig(), { model: "", provider: "" });
79+
expect(res.status).toBe(200);
80+
const text = await res.text();
81+
const payloads = text.split("\n").filter(l => l.startsWith("data: ") && l !== "data: [DONE]").map(l => JSON.parse(l.slice(6)) as Record<string, unknown>);
82+
const callItems = payloads.flatMap(p => {
83+
const item = p.item as Record<string, unknown> | undefined;
84+
const output = (p.response as { output?: Array<Record<string, unknown>> } | undefined)?.output ?? [];
85+
return [...(item ? [item] : []), ...output];
86+
}).filter(i => i.type === "custom_tool_call");
87+
expect(callItems.length).toBeGreaterThanOrEqual(3);
88+
for (const item of callItems) {
89+
expect(item.name).toBe("exec");
90+
expect("namespace" in item).toBe(false);
91+
}
92+
});
93+
94+
test("a genuine MCP namespace on a passthrough call is left alone", async () => {
95+
const call = { type: "function_call", id: "fc_1", call_id: "call_1", name: "search", namespace: "mcp__docs", arguments: "{}", status: "completed" };
96+
globalThis.fetch = (async () => new Response(sseFrom([call]), {
97+
status: 200, headers: { "content-type": "text/event-stream" },
98+
})) as typeof fetch;
99+
100+
const res = await handleResponses(request(), forwardConfig(), { model: "", provider: "" });
101+
const text = await res.text();
102+
expect(text).toContain('"namespace":"mcp__docs"');
103+
});
104+
105+
test("the bounded JSON (stream:false) passthrough path scrubs the same shape (#3217)", async () => {
106+
const call = { type: "custom_tool_call", id: "ctc_1", call_id: "call_1", name: "exec", namespace: "exec", input: "pwd", status: "completed" };
107+
globalThis.fetch = (async () => new Response(JSON.stringify({ id: "r1", object: "response", status: "completed", output: [call] }), {
108+
status: 200, headers: { "content-type": "application/json" },
109+
})) as typeof fetch;
110+
111+
const req = new Request("http://localhost/v1/responses", {
112+
method: "POST",
113+
headers: { "content-type": "application/json", authorization: "Bearer test", "chatgpt-account-id": "acct" },
114+
body: JSON.stringify({ ...requestBody, stream: false }),
115+
});
116+
const res = await handleResponses(req, forwardConfig(), { model: "", provider: "" });
117+
expect(res.status).toBe(200);
118+
const body = await res.json() as { output: Array<Record<string, unknown>> };
119+
expect(body.output[0]).toMatchObject({ type: "custom_tool_call", name: "exec" });
120+
expect("namespace" in body.output[0]).toBe(false);
121+
});
122+
123+
test("scrub is recursive, shape-preserving, and a no-op on clean payloads", () => {
124+
const clean = { type: "response.completed", response: { output: [{ type: "custom_tool_call", name: "exec", input: "" }] } };
125+
expect(scrubSelfNamedToolCallNamespace(clean)).toEqual({ value: clean, changed: false });
126+
const dirty = { response: { output: [{ type: "function_call", name: "wait", namespace: "wait", arguments: "{}" }, { type: "message" }] } };
127+
const result = scrubSelfNamedToolCallNamespace(dirty);
128+
expect(result.changed).toBe(true);
129+
expect(result.value).toEqual({ response: { output: [{ type: "function_call", name: "wait", arguments: "{}" }, { type: "message" }] } });
130+
// An empty name never matches: a namespace equal to "" is not the self-named shape.
131+
expect(scrubSelfNamedToolCallNamespace({ type: "custom_tool_call", name: "", namespace: "" }).changed).toBe(false);
132+
});

0 commit comments

Comments
 (0)