mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-16 20:02:45 +03:00
fix(codex): whitelist reasoning object keys before the wire (#13643)
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.<key>'`. 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!
This commit is contained in:
1
changelog.d/fixes/codex-reasoning-object-whitelist.md
Normal file
1
changelog.d/fixes/codex-reasoning-object-whitelist.md
Normal file
@@ -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.
|
||||
@@ -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,
|
||||
|
||||
@@ -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.<key>'", 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<string, unknown>)
|
||||
: 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;
|
||||
|
||||
96
tests/unit/codex-reasoning-wire-whitelist.test.ts
Normal file
96
tests/unit/codex-reasoning-wire-whitelist.test.ts
Normal file
@@ -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.<key>'", 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<string, unknown>, model = "gpt-6-astra") {
|
||||
const executor = new CodexExecutor();
|
||||
return executor.transformRequest(model, body, false, CTX) as Record<string, unknown>;
|
||||
}
|
||||
|
||||
function reasoningOf(result: Record<string, unknown>): Record<string, unknown> | null {
|
||||
const r = result.reasoning;
|
||||
if (r && typeof r === "object" && !Array.isArray(r)) return r as Record<string, unknown>;
|
||||
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<string, unknown>;
|
||||
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);
|
||||
});
|
||||
Reference in New Issue
Block a user