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 79d974eab8..030599c063 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -1,4 +1,5 @@ { + "_rebaseline_2026_09_15_13643_combined_growth": "Combined growth of the 2026-09-15 maxmad64bis uplift batch (each PR rebaselined its own growth; the merged sum is larger): open-sse/executors/codex.ts->1528. Every hunk is flag-gated or a verified fix covered by that PR's tests; see the batch report.", "_rebaseline_2026_09_15_13609_combined_growth": "Combined growth of the 2026-09-15 maxmad64bis uplift batch (each PR rebaselined its own growth; the merged sum is larger): open-sse/services/accountFallback.ts->2507. Every hunk is flag-gated or a verified fix covered by that PR's tests; see the batch report.", "_rebaseline_2026_09_15_13602_combined_growth": "Combined growth of the 2026-09-15 maxmad64bis uplift batch (each PR rebaselined its own growth; the merged sum is larger): src/sse/handlers/chatHelpers.ts->1214. Every hunk is flag-gated or a verified fix covered by that PR's tests; see the batch report.", "_rebaseline_2026_09_15_13580_combined_growth": "Combined growth of the 2026-09-15 maxmad64bis uplift batch (each PR rebaselined its own growth; the merged sum is larger): src/sse/handlers/chatHelpers.ts->1202. Every hunk is flag-gated or a verified fix covered by that PR's tests; see the batch report.", @@ -450,7 +451,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": 1505, + "open-sse/executors/codex.ts": 1528, "open-sse/executors/cursor.ts": 1759, "open-sse/executors/muse-spark-web.ts": 1405, "open-sse/handlers/chatCore.ts": 6146, diff --git a/open-sse/executors/codex.ts b/open-sse/executors/codex.ts index 25b316df7e..8e478cb833 100644 --- a/open-sse/executors/codex.ts +++ b/open-sse/executors/codex.ts @@ -1379,8 +1379,16 @@ export class CodexExecutor extends BaseExecutor { // Issue #2331: model suffix aliases (for example gpt-5.5-xhigh) represent an // explicit model selection, so they must override client-injected defaults such // as OpenCode's automatic reasoning.effort=medium for GPT-5-family requests. + // 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 = - modelEffort || explicitReasoning || requestReasoningEffort || fallbackReasoningEffort; + modelEffort || + explicitReasoning || + requestReasoningEffort || + (clientDisabledReasoning ? "none" : fallbackReasoningEffort); if (rawEffort) { const clampedEffort = clampEffort(cleanModel, rawEffort); @@ -1390,6 +1398,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); +});