fix(responses): keep the reserved functions group intact for codex-spark (#3217) - #3224
Conversation
…ark (#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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21b73c22b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| (value.type === "custom_tool_call" || value.type === "function_call") | ||
| && typeof value.name === "string" | ||
| && value.name.length > 0 | ||
| && value.namespace === value.name |
There was a problem hiding this comment.
Restrict scrubbing to malformed Spark calls
When a caller legitimately declares a namespace and child with the same name (for example, namespace mcp__worker containing function mcp__worker), this unconditional equality check removes the namespace from every provider/model's streamed and bounded passthrough response. The client then receives a different bare-tool identity; depending on the catalog, the undeclared-tool guard either rejects the response or permits dispatch to the wrong bare tool. Gate this repair on the affected Spark route and verify that the bare tool was actually declared instead of assuming every self-named pair is malformed.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| 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); |
There was a problem hiding this comment.
Normalize function schemas inside the preserved group
When a functions group contains a function whose parameters is absent or not an object schema, preserving the group here prevents the later normalizeToolSchemas pass from repairing it because that pass only examines direct entries in body.tools and additional_tools.tools. Before this change the group was flattened, so the child received the required { type: "object" } normalization; now the malformed nested declaration reaches Spark and can make the upstream reject the entire request. Apply the same schema normalization recursively to retained group children.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
리뷰 · 우선순위 77 / 80이 PR은 Codex 0.147+ Responses Lite에서 원인은 벨트 역할로 검증은 탄탄합니다. 라인 476 ( 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
…sion audit (#3218) * docs(devlog): open the bug/PR closeout stack roadmap * docs(devlog): fold the A-gate import-boundary finding into phase 5 * docs(devlog): record the #3163 and #3166 landings * docs(devlog): record why #2986 does not land in this train * docs(devlog): close out the bug/PR closeout stack * docs(devlog): record the final green CI verdict on dev * docs(devlog): open the bug-label drawdown roadmap with audit corrections * docs(devlog): record the Batch A landings and first rebase carry * docs(devlog): record the Batch B rebase carries * docs(devlog): record why the rebase service earned its keep * docs(devlog): record the Batch C rebases and the one real review finding * docs(devlog): record the #2999 scope boundary that survived execution * docs(devlog): record Batch D - every bug PR closed * docs(devlog): record what the PR half of the campaign cost * docs(devlog): replan the remaining issues to one per cycle * docs(devlog): carry the i3141 evidence into the replan * docs(devlog): diagnose i3141 - fix predates the reported version * docs(devlog): retire the second bundle * docs(devlog): record the i3141 re-triage action and outcome * docs(devlog): diagnose i3152 log table jitter * docs(devlog): i3152 - measurement disproved the layout diagnosis * docs(devlog): diagnose i3136 slashed-id price lookup * docs(devlog): diagnose i3150 citation marker passthrough * docs(devlog): diagnose i3155 capacity plan allowlist * docs(devlog): i1419 stays open pending crash frames * docs(devlog): record the i1419 re-triage ask * docs(devlog): diagnose i2999 publication overwrite race * docs(devlog): record the i2999 outcome and remaining scope * docs(devlog): diagnose i2813 as a client-side reserve gate * docs(devlog): diagnose i1527 residuals as trace-blocked * docs(devlog): correct i1527 envelope-cap wording (192 blobs, HTTP 400) * docs(devlog): plan p3193 loopback alpha-search reimplementation * docs(devlog): record p3193 landing (#3205 -> 53c09a2) * docs(devlog): plan the main->dev regression audit * docs(devlog): pin regaudit counts, add tests-only/security passes and the exact-head dispatch * docs(devlog): record regaudit reviewer verdicts * docs(devlog): record the exact-head dev CI verdict and Windows classification * docs(devlog): record the main control run proving the Windows failures predate the range * docs(devlog): record the pass-1 recount and the #3217 root cause * docs(devlog): plan i3217 (Spark functions-namespace flattening) * docs(devlog): record i3217 landing (#3224 -> d23eab4) * docs(devlog): regaudit2 recount and disposition table * docs(devlog): regaudit2 CI verdict on d23eab4 and the four PR arrivals * docs(devlog): plan p3226 (scoped namespace scrub) * docs(devlog): p3226 audit finding and carry plan * docs(devlog): record p3226 landing (#3234 -> b732b0d) * docs(devlog): plan p3227 (combo zero-output incomplete failover) * docs(devlog): record p3227 landing * docs(devlog): plan p3228 (encrypted V2 spawn native fallback) * docs(devlog): record p3228 landing * docs(devlog): plan p3229 (Codexless originator in task recovery) * docs(devlog): record p3229 landing and the #3239 regression repair * docs(devlog): r3239 regression repair record * docs(devlog): r3239 audit note * docs(devlog): record p3232 (merged by maintainer) * docs(devlog): p3232 verification result * docs(devlog): regaudit3 recount and landing table * docs(devlog): record the #3239/#3240 revert and correct the #3228 disposition * docs(devlog): rv3239 revert record * docs(devlog): rv3239 audit note * docs(devlog): regaudit3 second-dispatch verdict * docs(devlog): regaudit3 recount refreshed (#1419 closed by maintainer; count 4) * docs(devlog): regaudit3 final CI verdict and c-7 --------- Co-authored-by: jun <jun@lidge.dev>
…ark (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 <jun@lidge.dev>
…sion audit (lidge-jun#3218) * docs(devlog): open the bug/PR closeout stack roadmap * docs(devlog): fold the A-gate import-boundary finding into phase 5 * docs(devlog): record the lidge-jun#3163 and lidge-jun#3166 landings * docs(devlog): record why lidge-jun#2986 does not land in this train * docs(devlog): close out the bug/PR closeout stack * docs(devlog): record the final green CI verdict on dev * docs(devlog): open the bug-label drawdown roadmap with audit corrections * docs(devlog): record the Batch A landings and first rebase carry * docs(devlog): record the Batch B rebase carries * docs(devlog): record why the rebase service earned its keep * docs(devlog): record the Batch C rebases and the one real review finding * docs(devlog): record the lidge-jun#2999 scope boundary that survived execution * docs(devlog): record Batch D - every bug PR closed * docs(devlog): record what the PR half of the campaign cost * docs(devlog): replan the remaining issues to one per cycle * docs(devlog): carry the i3141 evidence into the replan * docs(devlog): diagnose i3141 - fix predates the reported version * docs(devlog): retire the second bundle * docs(devlog): record the i3141 re-triage action and outcome * docs(devlog): diagnose i3152 log table jitter * docs(devlog): i3152 - measurement disproved the layout diagnosis * docs(devlog): diagnose i3136 slashed-id price lookup * docs(devlog): diagnose i3150 citation marker passthrough * docs(devlog): diagnose i3155 capacity plan allowlist * docs(devlog): i1419 stays open pending crash frames * docs(devlog): record the i1419 re-triage ask * docs(devlog): diagnose i2999 publication overwrite race * docs(devlog): record the i2999 outcome and remaining scope * docs(devlog): diagnose i2813 as a client-side reserve gate * docs(devlog): diagnose i1527 residuals as trace-blocked * docs(devlog): correct i1527 envelope-cap wording (192 blobs, HTTP 400) * docs(devlog): plan p3193 loopback alpha-search reimplementation * docs(devlog): record p3193 landing (lidge-jun#3205 -> 53c09a2) * docs(devlog): plan the main->dev regression audit * docs(devlog): pin regaudit counts, add tests-only/security passes and the exact-head dispatch * docs(devlog): record regaudit reviewer verdicts * docs(devlog): record the exact-head dev CI verdict and Windows classification * docs(devlog): record the main control run proving the Windows failures predate the range * docs(devlog): record the pass-1 recount and the lidge-jun#3217 root cause * docs(devlog): plan i3217 (Spark functions-namespace flattening) * docs(devlog): record i3217 landing (lidge-jun#3224 -> d23eab4) * docs(devlog): regaudit2 recount and disposition table * docs(devlog): regaudit2 CI verdict on d23eab4 and the four PR arrivals * docs(devlog): plan p3226 (scoped namespace scrub) * docs(devlog): p3226 audit finding and carry plan * docs(devlog): record p3226 landing (lidge-jun#3234 -> b732b0d) * docs(devlog): plan p3227 (combo zero-output incomplete failover) * docs(devlog): record p3227 landing * docs(devlog): plan p3228 (encrypted V2 spawn native fallback) * docs(devlog): record p3228 landing * docs(devlog): plan p3229 (Codexless originator in task recovery) * docs(devlog): record p3229 landing and the lidge-jun#3239 regression repair * docs(devlog): r3239 regression repair record * docs(devlog): r3239 audit note * docs(devlog): record p3232 (merged by maintainer) * docs(devlog): p3232 verification result * docs(devlog): regaudit3 recount and landing table * docs(devlog): record the lidge-jun#3239/lidge-jun#3240 revert and correct the lidge-jun#3228 disposition * docs(devlog): rv3239 revert record * docs(devlog): rv3239 audit note * docs(devlog): regaudit3 second-dispatch verdict * docs(devlog): regaudit3 recount refreshed (lidge-jun#1419 closed by maintainer; count 4) * docs(devlog): regaudit3 final CI verdict and c-7 --------- Co-authored-by: jun <jun@lidge.dev>
…ark (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 <jun@lidge.dev>
…sion audit (lidge-jun#3218) * docs(devlog): open the bug/PR closeout stack roadmap * docs(devlog): fold the A-gate import-boundary finding into phase 5 * docs(devlog): record the lidge-jun#3163 and lidge-jun#3166 landings * docs(devlog): record why lidge-jun#2986 does not land in this train * docs(devlog): close out the bug/PR closeout stack * docs(devlog): record the final green CI verdict on dev * docs(devlog): open the bug-label drawdown roadmap with audit corrections * docs(devlog): record the Batch A landings and first rebase carry * docs(devlog): record the Batch B rebase carries * docs(devlog): record why the rebase service earned its keep * docs(devlog): record the Batch C rebases and the one real review finding * docs(devlog): record the lidge-jun#2999 scope boundary that survived execution * docs(devlog): record Batch D - every bug PR closed * docs(devlog): record what the PR half of the campaign cost * docs(devlog): replan the remaining issues to one per cycle * docs(devlog): carry the i3141 evidence into the replan * docs(devlog): diagnose i3141 - fix predates the reported version * docs(devlog): retire the second bundle * docs(devlog): record the i3141 re-triage action and outcome * docs(devlog): diagnose i3152 log table jitter * docs(devlog): i3152 - measurement disproved the layout diagnosis * docs(devlog): diagnose i3136 slashed-id price lookup * docs(devlog): diagnose i3150 citation marker passthrough * docs(devlog): diagnose i3155 capacity plan allowlist * docs(devlog): i1419 stays open pending crash frames * docs(devlog): record the i1419 re-triage ask * docs(devlog): diagnose i2999 publication overwrite race * docs(devlog): record the i2999 outcome and remaining scope * docs(devlog): diagnose i2813 as a client-side reserve gate * docs(devlog): diagnose i1527 residuals as trace-blocked * docs(devlog): correct i1527 envelope-cap wording (192 blobs, HTTP 400) * docs(devlog): plan p3193 loopback alpha-search reimplementation * docs(devlog): record p3193 landing (lidge-jun#3205 -> 144ddf4) * docs(devlog): plan the main->dev regression audit * docs(devlog): pin regaudit counts, add tests-only/security passes and the exact-head dispatch * docs(devlog): record regaudit reviewer verdicts * docs(devlog): record the exact-head dev CI verdict and Windows classification * docs(devlog): record the main control run proving the Windows failures predate the range * docs(devlog): record the pass-1 recount and the lidge-jun#3217 root cause * docs(devlog): plan i3217 (Spark functions-namespace flattening) * docs(devlog): record i3217 landing (lidge-jun#3224 -> fe855b3) * docs(devlog): regaudit2 recount and disposition table * docs(devlog): regaudit2 CI verdict on fe855b3 and the four PR arrivals * docs(devlog): plan p3226 (scoped namespace scrub) * docs(devlog): p3226 audit finding and carry plan * docs(devlog): record p3226 landing (lidge-jun#3234 -> 827456e) * docs(devlog): plan p3227 (combo zero-output incomplete failover) * docs(devlog): record p3227 landing * docs(devlog): plan p3228 (encrypted V2 spawn native fallback) * docs(devlog): record p3228 landing * docs(devlog): plan p3229 (Codexless originator in task recovery) * docs(devlog): record p3229 landing and the lidge-jun#3239 regression repair * docs(devlog): r3239 regression repair record * docs(devlog): r3239 audit note * docs(devlog): record p3232 (merged by maintainer) * docs(devlog): p3232 verification result * docs(devlog): regaudit3 recount and landing table * docs(devlog): record the lidge-jun#3239/lidge-jun#3240 revert and correct the lidge-jun#3228 disposition * docs(devlog): rv3239 revert record * docs(devlog): rv3239 audit note * docs(devlog): regaudit3 second-dispatch verdict * docs(devlog): regaudit3 recount refreshed (lidge-jun#1419 closed by maintainer; count 4) * docs(devlog): regaudit3 final CI verdict and c-7 --------- Co-authored-by: jun <jun@lidge.dev>
Summary
Codex 0.147+ on Responses Lite ships every ordinary client tool inside the reserved
functionsnamespace group, carried in anadditional_toolsinput 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 ascustom_tool_call { name: "exec", namespace: "exec" }; codex-rs treats onlyNone/""/"functions"as the default namespace, concatenates that into the unroutableexecexec, 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: group flattened →
namespace:"exec"back, turn loops (25execexecin 60 s); group intact → bareexecback,pwdruns, turn completes with 0 errors.stripSparkCompatibilitykeeps afunctionsgroup as a group on bothbody.toolsandadditional_tools, still filtering its children (tool_searchdropped,defer_loadingstripped) and admittingcustominside it — exactly whatcreate_tools_json_for_responses_litesends direct. MCP-style groups flatten as before. Non-Spark models untouched.src/server/responses-self-named-namespace-scrub.tsdrops a tool-callnamespacethat repeats the call's ownnameon the client-facing passthrough (SSE payload rewrites and the bounded-JSON path). That shape is never a legitimate identity, so no catalog lookup; a genuinemcp__*namespace is untouched.Fixes #3217.
Verification
bun test tests/openai-responses-passthrough.test.ts— new case "keeps the reserved functions group intact for codex-spark, flattens MCP groups ([Bug][2.39.0] Responses Lite exec is returned with namespace "exec", causing an execexec tool-call loop #3217)"; red without the adapter change.bun test tests/responses-self-named-namespace-scrub.test.ts— SSE scrub via livehandleResponses,stream:falsebounded-JSON scrub, MCP namespace preserved, recursive unit case; red without the scrub module.bun run test:changed: 5794 pass / 0 fail across 301 files.bun run typecheckclean;bun run privacy:scanpassed.codex exec --model gpt-5.3-codex-sparkagainst a dev proxy from this branch completedpwd(before: 25×unsupported custom tool call: execexec).Checklist
dev