diff --git a/changelog.d/fixes/12585-codex-strip-temperature.md b/changelog.d/fixes/12585-codex-strip-temperature.md new file mode 100644 index 0000000000..1e02f64225 --- /dev/null +++ b/changelog.d/fixes/12585-codex-strip-temperature.md @@ -0,0 +1 @@ +- **fix(sse):** strip `temperature`/`top_p` on native Codex Responses passthrough so combo `codex-review` traffic no longer 400s with `Unsupported parameter: temperature` ([#12585](https://github.com/diegosouzapw/OmniRoute/pull/12585)) diff --git a/open-sse/executors/codex.ts b/open-sse/executors/codex.ts index 644a67c08b..82bd7931de 100644 --- a/open-sse/executors/codex.ts +++ b/open-sse/executors/codex.ts @@ -19,6 +19,7 @@ import { } from "../config/codexInstructions.ts"; import { FETCH_BODY_TIMEOUT_MS, HTTP_STATUS, PROVIDERS } from "../config/constants.ts"; import { readCodexPeekChunk, buildCodexTimeoutSafePassthroughBody } from "./codex/bodyTimeout.ts"; +import { stripCodexPassthroughRejectedParams } from "./codex/stripPassthroughRejectedParams.ts"; import { CODEX_CLI_RS_ORIGINATOR, getCodexClientVersion, @@ -1431,16 +1432,7 @@ export class CodexExecutor extends BaseExecutor { delete body.truncation; delete body.background; // Droid CLI sends this but Codex Responses API rejects it - // Issue #3317: strip client-only fields the Codex Responses API rejects with - // 400 "Unsupported parameter" — for BOTH the native passthrough (early return - // below) and the translated path. The chat-completions path already removes - // these (base.ts prompt_cache_retention #1884; openai-responses translator - // safety_identifier #2770), but the responses->responses passthrough skips - // translation. `user` is always rejected by Codex /responses, so it is removed - // unconditionally here (unlike base.ts, which only drops it when empty). - delete body.prompt_cache_retention; - delete body.safety_identifier; - delete body.user; + stripCodexPassthroughRejectedParams(cleanModel || model, body); // Inject prompt_cache_key for Codex prompt caching. // The official Codex client sets this to conversation_id (a stable UUID per session). diff --git a/open-sse/executors/codex/stripPassthroughRejectedParams.ts b/open-sse/executors/codex/stripPassthroughRejectedParams.ts new file mode 100644 index 0000000000..fc998d32bc --- /dev/null +++ b/open-sse/executors/codex/stripPassthroughRejectedParams.ts @@ -0,0 +1,27 @@ +// Strip fields Codex /responses rejects with 400 "Unsupported parameter" +// BEFORE native passthrough returns (the Responses allowlist never runs on +// that path). Extracted from CodexExecutor.transformRequest so codex.ts stays +// under its frozen file-size cap. +// +// Client-only (#3317): prompt_cache_retention, safety_identifier, user. +// The chat-completions path already removes these (base.ts prompt_cache_retention +// #1884; openai-responses translator safety_identifier #2770), but the +// responses->responses passthrough skips translation. `user` is always rejected +// by Codex /responses, so it is removed unconditionally (unlike base.ts, which +// only drops it when empty). +// +// Sampling: temperature, top_p — FastAPI `{"detail":"Unsupported parameter: +// temperature"}`. Combo codex-review forwarded client temperature onto +// gpt-5.6-sol-xhigh / gpt-5.6-luna-max. + +import { stripUnsupportedParams } from "../../translator/paramSupport.ts"; + +export function stripCodexPassthroughRejectedParams( + model: string, + body: Record +): void { + delete body.prompt_cache_retention; + delete body.safety_identifier; + delete body.user; + stripUnsupportedParams("codex", model, body); +} diff --git a/open-sse/services/gpt5SamplingGuard.ts b/open-sse/services/gpt5SamplingGuard.ts index 729c1ce450..a258dd1b16 100644 --- a/open-sse/services/gpt5SamplingGuard.ts +++ b/open-sse/services/gpt5SamplingGuard.ts @@ -12,7 +12,9 @@ * This guard removes `temperature`/`top_p` only when the resolved effort is active * (anything other than `none`). It is scoped to the `openai` provider (raw * api.openai.com Chat Completions): the `codex` provider's Responses requests are - * already covered by the CodexExecutor allowlist (which drops both params), and + * already covered by CodexExecutor.transformRequest (STRIP_RULES drops + * temperature/top_p before native passthrough returns; the Responses allowlist + * also omits them on the translated path), and * other providers manage their own sampling rules. * * Refs: litellm#27351 (GPT-5.1 accepts temperature only when effort=none), diff --git a/open-sse/translator/paramSupport.ts b/open-sse/translator/paramSupport.ts index 85dcec35ae..79d2b5219e 100644 --- a/open-sse/translator/paramSupport.ts +++ b/open-sse/translator/paramSupport.ts @@ -34,6 +34,13 @@ const STRIP_RULES: StripRule[] = [ { match: /claude-opus-4/i, drop: ["temperature"] }, // GitHub Copilot gpt-5.4: temperature unsupported. { provider: "github", match: /gpt-5\.4/i, drop: ["temperature"] }, + // Codex /responses (chatgpt.com backend-api) rejects sampling params with + // FastAPI 400 `{"detail":"Unsupported parameter: temperature"}`. Native + // Codex passthrough returns before the Responses allowlist, so this rule + // must run from CodexExecutor.transformRequest via + // stripCodexPassthroughRejectedParams. Live: combo codex-review + // gpt-5.6-sol-xhigh / gpt-5.6-luna-max. + { provider: "codex", match: /.*/, drop: ["temperature", "top_p"] }, // GitHub Copilot Claude (except opus/sonnet 4.6): thinking + reasoning_effort rejected. #713 { provider: "github", diff --git a/tests/unit/codex-responses-passthrough-strip-3317.test.ts b/tests/unit/codex-responses-passthrough-strip-3317.test.ts index b925eb14d7..4cfcb0b0c2 100644 --- a/tests/unit/codex-responses-passthrough-strip-3317.test.ts +++ b/tests/unit/codex-responses-passthrough-strip-3317.test.ts @@ -25,6 +25,8 @@ test("codex native responses passthrough strips client-only params (#3317)", asy safety_identifier: "droid-user-123", user: "user-abc", max_output_tokens: 16, + temperature: 0.7, + top_p: 0.9, }; const result = (await executor.transformRequest("gpt-5.5", body, false, {} as never)) as Record< @@ -35,6 +37,12 @@ test("codex native responses passthrough strips client-only params (#3317)", asy assert.equal(result.prompt_cache_retention, undefined, "prompt_cache_retention must be stripped"); assert.equal(result.safety_identifier, undefined, "safety_identifier must be stripped"); assert.equal(result.user, undefined, "user must be stripped"); + assert.equal( + result.temperature, + undefined, + "temperature must be stripped before native passthrough" + ); + assert.equal(result.top_p, undefined, "top_p must be stripped before native passthrough"); // The real request payload must survive the strip. assert.ok(Array.isArray(result.input), "input array preserved"); }); diff --git a/tests/unit/executors-strip-unsupported-params.test.ts b/tests/unit/executors-strip-unsupported-params.test.ts index 9e98a9f9de..914c63be78 100644 --- a/tests/unit/executors-strip-unsupported-params.test.ts +++ b/tests/unit/executors-strip-unsupported-params.test.ts @@ -57,6 +57,25 @@ test("stripUnsupportedParams: github + gpt-5 (non-5.4) keeps temperature", () => assert.equal(body.temperature, 1); }); +test("stripUnsupportedParams: codex strips temperature and top_p (Responses 400)", () => { + const body: Record = { + temperature: 0.7, + top_p: 0.9, + model: "gpt-5.6-luna-max", + input: [], + }; + stripUnsupportedParams("codex", "gpt-5.6-sol-xhigh", body); + assert.equal(body.temperature, undefined, "Codex /responses rejects temperature"); + assert.equal(body.top_p, undefined, "Codex /responses rejects top_p"); + assert.equal(body.model, "gpt-5.6-luna-max", "other params must survive"); +}); + +test("stripUnsupportedParams: non-codex provider keeps temperature for gpt-5.6-luna-max", () => { + const body: Record = { temperature: 0.7 }; + stripUnsupportedParams("openai", "gpt-5.6-luna-max", body); + assert.equal(body.temperature, 0.7, "codex sampling strip is provider-scoped"); +}); + test("stripUnsupportedParams: github + Claude strips thinking + reasoning_effort", () => { const body: Record = { thinking: { type: "enabled" }, @@ -193,11 +212,19 @@ test("stripUnsupportedParams: volcengine kimi-k2-5-260127 also clamps max_comple test("stripUnsupportedParams: volcengine non-kimi model (glm-4-7-251222) is NOT clamped by the kimi rule", () => { const body: Record = { max_tokens: 65536 }; stripUnsupportedParams("volcengine", "glm-4-7-251222", body); - assert.equal(body.max_tokens, 65536, "kimi-specific cap must not apply to other volcengine models"); + assert.equal( + body.max_tokens, + 65536, + "kimi-specific cap must not apply to other volcengine models" + ); }); test("stripUnsupportedParams: kimi rule is provider-scoped (no-op for non-volcengine providers)", () => { const body: Record = { max_tokens: 65536 }; stripUnsupportedParams("kimi", "kimi-k2-5-260127", body); - assert.equal(body.max_tokens, 65536, "the Ark-specific cap must not leak to other kimi-hosting providers"); + assert.equal( + body.max_tokens, + 65536, + "the Ark-specific cap must not leak to other kimi-hosting providers" + ); });