From de428cef867eceb207475db1c0bc96fb37f08ff4 Mon Sep 17 00:00:00 2001 From: "Bob.Hou" Date: Fri, 18 Sep 2026 09:21:44 -0400 Subject: [PATCH] fix(codex): whitelist reasoning object keys before the wire (#13643) (#14065) The Codex executor now whitelists the wire `reasoning` object to `effort`/`summary` before dispatch instead of spreading whatever the client sent, and maps `reasoning.enabled === false` to `effort: "none"` when no more specific effort was requested. OpenRouter-style keys (`enabled`, `max_tokens`, `exclude`) were reaching the Responses API and 400-ing the whole combo target with `Unknown parameter: 'reasoning.'`. The precedence chain keeps an explicit per-request effort ahead of `enabled: false`, and the strip matches the siblings already removed in the same function (`truncation`, `user`, `prompt_cache_retention`). Validated as a combined board first (this PR merged with the 11 siblings of the same batch on the release tip): eslint on every changed file with the suppressions file, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 275 passing / 0 failing focused node:test cases across the 28 test files the batch touches. Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thanks @HouMinXi! Co-authored-by: Diego Rodrigues de Sa e Souza --- .../fixes/codex-reasoning-object-whitelist.md | 1 + config/quality/file-size-baseline.json | 5 +- open-sse/executors/codex.ts | 25 ++++- .../codex-reasoning-wire-whitelist.test.ts | 96 +++++++++++++++++++ 4 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 changelog.d/fixes/codex-reasoning-object-whitelist.md create mode 100644 tests/unit/codex-reasoning-wire-whitelist.test.ts diff --git a/changelog.d/fixes/codex-reasoning-object-whitelist.md b/changelog.d/fixes/codex-reasoning-object-whitelist.md new file mode 100644 index 0000000000..139d470bae --- /dev/null +++ b/changelog.d/fixes/codex-reasoning-object-whitelist.md @@ -0,0 +1 @@ +- Fixed Codex executor forwarding client `reasoning` sub-fields (`enabled`, `max_tokens`, `exclude`) that the Codex Responses API rejects with HTTP 400, taking down every combo target with a deterministic client error. The reasoning object is now whitelisted to `effort`/`summary`, and `enabled: false` maps to effort `none` when no more specific effort was requested. diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 223c0faa40..d53ff3b863 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -1,4 +1,5 @@ { + "_rebaseline_2026_09_18_restore_13643_after_clobber": "Restore of #13643 after e7999c477b clobbered the whitelist at the release tip (see #14062). open-sse/executors/codex.ts 1530->1552 (+22): keep the tip force-rule precedence and restore OpenRouter-style enabled:false plus the reasoning-object key whitelist before the wire. Covered by tests/unit/codex-reasoning-wire-whitelist.test.ts.", "_rebaseline_2026_09_03_12648_xkiro_provider": "PR #12648 (feat/provider-xkiro) own growth: src/shared/constants/providers/apikey/gateways.ts +18 lines on top of #12649 (the xkiro APIKEY_PROVIDERS_GATEWAYS catalog entry with hasFree/freeNote/authHint/apiHint documenting the 5M tokens/day free plan, plus the Prettier reflow of two pre-existing >100-col authHint lines (oneminai, freebuff) that lint-staged enforces on any touch of the file; additive data at the existing registry chokepoint, same god-file no-split rationale as prior gateways.ts rebaselines: #11786 seekai, #10987 logfare, #10531 freebuff). Covered by tests/unit/free-provider-xkiro.test.ts (4/4).", "_rebaseline_2026_09_15_12643_messages_entry_guard": "#12643 own growth: src/sse/handlers/chat.ts +10 (2490->2500 after syncing the 09-16 base, which itself moved the frozen value). A `messages` array containing a null/non-object entry (e.g. `[null]`) passed every existing entry guard (#5110/#6402/#6407/#6412) and crashed downstream translators/session helpers (openai-to-claude.ts, sessionManager.ts, contextManager.ts's fixToolPairs) reading `.role`/`.content` off the raw entry, surfacing as an HTTP 500 instead of a clean 400. Adds one more entry-shape guard clause to the same chokepoint, extending the existing guard family — same pattern, irreducible call-site wiring (the check itself is a one-line `.some()` predicate, not extractable into its own leaf without hiding the chokepoint). Covered by tests/unit/chat-messages-entry-objects-12643.test.ts (3/3) plus the sibling guard suites (chat-messages-validation-6402.test.ts, chat-non-string-model-6407.test.ts, 22/22, no regression). ATUALIZADO 2026-09-17: o teto foi refixado em 2519 ao mergear o tip atual. O tip sozinho ja esta em 2509 (acima do teto 2500 que esta PR havia fixado contra um tip anterior); o +10 desta PR e o proprio guard de entrada. O excedente do tip (2509>2500) e base-red herdado, nao introduzido aqui.", "_rebaseline_2026_09_17_13185_claude_oauth_sticky_refresh": "PR #13185 (@RaviTharuma): soft-fail do refresh do Claude para CredentialHealth nao ficar sticky-dead. src/lib/tokenHealthCheck.ts 1214 (tip) -> 1220 na branch e 1221 na arvore combinada com #13426; teto fixado em 1221. O teto anterior (1218) tinha apenas 4 linhas de folga. O crescimento e o proprio fix: preservar o refresh_token e distinguir falha transitoria de credencial morta exige estado extra no caminho de sweep, que nao pode sair do modulo sem quebrar a API interna. Coberto por tests/unit/tokenHealthCheck-claude-refresh-token-preserved.test.ts; os 14 arquivos irmaos de tokenHealthCheck/credentialHealth foram rodados juntos (72/72).", @@ -467,7 +468,7 @@ "open-sse/executors/antigravity.ts": 1665, "open-sse/executors/base.ts": 1753, "open-sse/executors/chatgpt-web.ts": 5056, - "open-sse/executors/codex.ts": 1530, + "open-sse/executors/codex.ts": 1552, "open-sse/executors/cursor.ts": 1847, "open-sse/executors/muse-spark-web.ts": 1405, "open-sse/handlers/chatCore.ts": 6287, @@ -598,7 +599,7 @@ "_rebaseline_pr4592_exclude_exhausted_auto": "Reconcile #4592 already-merged growth: combo.ts 2991->3036 (+45, terminal-status quota-cutoff exclusion in buildAutoCandidates + opt-in gate). Fast-gate PR->release does not run check:file-size.", "open-sse/executors/antigravity.ts": "1528", "open-sse/executors/base.ts": "1640", - "open-sse/executors/codex.ts": "1562", + "open-sse/executors/codex.ts": 1552, "open-sse/executors/cursor.ts": "1563", "open-sse/executors/deepseek-web.ts": "1148", "open-sse/executors/grok-web.ts": "1044", diff --git a/open-sse/executors/codex.ts b/open-sse/executors/codex.ts index 2c9d98d25e..8d5162eb0a 100644 --- a/open-sse/executors/codex.ts +++ b/open-sse/executors/codex.ts @@ -1412,12 +1412,17 @@ export class CodexExecutor extends BaseExecutor { // explicit model selection, so they must override client-injected defaults such // as OpenCode's automatic reasoning.effort=medium for GPT-5-family requests. // A server-selected force rule is stronger than either source. + // OpenRouter-style `enabled: false` asks for reasoning to be off. It + // wins over the connection default but still loses to any per-request + // effort selection (model suffix, reasoning.effort, or flat + // reasoning_effort). + const clientDisabledReasoning = reasoningRecord?.enabled === false; const rawEffort = getForcedReasoningEffort(credentials) || modelEffort || explicitReasoning || requestReasoningEffort || - fallbackReasoningEffort; + (clientDisabledReasoning ? "none" : fallbackReasoningEffort); if (rawEffort) { const clampedEffort = clampEffort(cleanModel, rawEffort); @@ -1427,6 +1432,24 @@ export class CodexExecutor extends BaseExecutor { effort: clampedEffort === "ultra" ? "max" : clampedEffort, }; } + + // The Codex Responses API accepts only `effort` and `summary` inside + // `reasoning`. Client ecosystems send OpenRouter-style keys (`enabled`, + // `max_tokens`, `exclude`, ...) that the upstream rejects with HTTP 400 + // "Unknown parameter: 'reasoning.'", so whitelist the object before + // it reaches the wire. This must run even when no effort was resolved, + // because the client's original object is forwarded unchanged in that + // case. + const wireReasoning = + body.reasoning && typeof body.reasoning === "object" && !Array.isArray(body.reasoning) + ? (body.reasoning as Record) + : null; + if (wireReasoning) { + for (const key of Object.keys(wireReasoning)) { + if (key !== "effort" && key !== "summary") delete wireReasoning[key]; + } + if (Object.keys(wireReasoning).length === 0) delete body.reasoning; + } ensureCodexReasoningSummary(body); if (isCompactRequest) { delete body.include; diff --git a/tests/unit/codex-reasoning-wire-whitelist.test.ts b/tests/unit/codex-reasoning-wire-whitelist.test.ts new file mode 100644 index 0000000000..6b6cb84eaa --- /dev/null +++ b/tests/unit/codex-reasoning-wire-whitelist.test.ts @@ -0,0 +1,96 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { CodexExecutor } from "../../open-sse/executors/codex.ts"; +import { setThinkingBudgetConfig, ThinkingMode } from "../../open-sse/services/thinkingBudget.ts"; + +// The Codex Responses API accepts only `effort` and `summary` inside +// `reasoning`. Client ecosystems send OpenRouter-style keys (`enabled`, +// `max_tokens`, `exclude`, ...) that the upstream rejects with HTTP 400 +// "Unknown parameter: 'reasoning.'", taking down every combo target +// with the same deterministic client error. The executor must whitelist the +// object before it reaches the wire; `enabled: false` maps to effort "none" +// when no more specific effort was requested. + +const CTX = { requestEndpointPath: "/responses" }; + +function transform(body: Record, model = "gpt-6-astra") { + const executor = new CodexExecutor(); + return executor.transformRequest(model, body, false, CTX) as Record; +} + +function reasoningOf(result: Record): Record | null { + const r = result.reasoning; + if (r && typeof r === "object" && !Array.isArray(r)) return r as Record; + return null; +} + +test("reasoning.enabled is stripped; explicit effort survives", () => { + const r = reasoningOf(transform({ reasoning: { enabled: true, effort: "high" } })); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.effort, "high"); + assert.equal("enabled" in r, false); +}); + +test("reasoning.enabled:false maps to effort none when nothing more specific is set", () => { + const r = reasoningOf(transform({ reasoning: { enabled: false } })); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.effort, "none"); + assert.equal("enabled" in r, false); + assert.equal("summary" in r, false, "no summary for disabled reasoning"); +}); + +test("OpenRouter-style reasoning.max_tokens never reaches the wire", () => { + const r = reasoningOf(transform({ reasoning: { max_tokens: 2048 } })); + assert.ok(!r || !("max_tokens" in r), "max_tokens must be stripped"); +}); + +test("reasoning.exclude is stripped; sibling effort survives", () => { + const r = reasoningOf(transform({ reasoning: { exclude: true, effort: "low" } })); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.effort, "low"); + assert.equal("exclude" in r, false); +}); + +test("client-provided summary is preserved", () => { + const r = reasoningOf(transform({ reasoning: { summary: "detailed", effort: "medium" } })); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.summary, "detailed"); + assert.equal(r.effort, "medium"); +}); + +test("model suffix effort still wins over enabled:false", () => { + const r = reasoningOf(transform({ reasoning: { enabled: false } }, "gpt-6-astra-high")); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.effort, "high"); +}); + +test("enabled:false wins over an explicit connection reasoning default", () => { + setThinkingBudgetConfig({ mode: ThinkingMode.PASSTHROUGH }); + try { + const executor = new CodexExecutor(); + const result = executor.transformRequest( + "gpt-6-astra", + { reasoning: { enabled: false } }, + false, + { + requestEndpointPath: "/responses", + providerSpecificData: { requestDefaults: { reasoningEffort: "high" } }, + } + ) as Record; + const r = reasoningOf(result); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.effort, "none", "client disable must beat the connection default"); + } finally { + setThinkingBudgetConfig({}); + } +}); + +test("flat reasoning_effort path stays clean of extra keys", () => { + const result = transform({ reasoning_effort: "low", reasoning: { enabled: true } }); + assert.equal("reasoning_effort" in result, false, "flat key must never reach the wire"); + const r = reasoningOf(result); + assert.ok(r, "reasoning object should be present"); + assert.equal(r.effort, "low"); + assert.equal("enabled" in r, false); +});