Repository navigation
fix(codex): strip include from compact responses requests #6805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - **fix(codex): strip include from compact responses requests** (#6805 — thanks @yinaoxiong). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -894,7 +894,9 @@ export class CodexExecutor extends BaseExecutor { | |
| headers["chatgpt-account-id"] = workspaceId; | ||
| } | ||
| const clientIdentity = credentials?.providerSpecificData?.codexClientIdentity as | ||
| CodexClientIdentity | null | undefined; | ||
| | CodexClientIdentity | ||
| | null | ||
| | undefined; | ||
|
|
||
| // Originator header — identifies the client type to the Codex backend. | ||
| // Ref: openai/codex login/src/auth/default_client.rs DEFAULT_ORIGINATOR = "codex_cli_rs" | ||
|
|
@@ -1001,6 +1003,7 @@ export class CodexExecutor extends BaseExecutor { | |
| delete body.stream; | ||
| delete body.stream_options; | ||
| delete body.client_metadata; | ||
| delete body.include; | ||
| } else { | ||
| body.stream = true; | ||
| } | ||
|
|
@@ -1174,6 +1177,9 @@ export class CodexExecutor extends BaseExecutor { | |
| }; | ||
| } | ||
| ensureCodexReasoningSummary(body); | ||
| if (isCompactRequest) { | ||
| delete body.include; | ||
| } | ||
|
Comment on lines
+1180
to
+1182
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the Repository Style Guide (Hard Rule 9), you must always include or update tests when changing production code in Please add or update unit tests in References
|
||
| delete body.reasoning_effort; | ||
|
|
||
| // Remove unsupported token limit parameters BEFORE the passthrough return. | ||
|
|
@@ -1214,7 +1220,9 @@ export class CodexExecutor extends BaseExecutor { | |
| applyCodexClientMetadata( | ||
| body, | ||
| credentials?.providerSpecificData?.codexClientIdentity as | ||
| CodexClientIdentity | null | undefined | ||
| | CodexClientIdentity | ||
| | null | ||
| | undefined | ||
| ); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
|
|
||
| import { CodexExecutor } from "../../open-sse/executors/codex.ts"; | ||
|
|
||
| // #6805: compact Codex requests must not forward `include` (e.g. | ||
| // "reasoning.encrypted_content") — the compact endpoint rejects it. Kept in a | ||
| // standalone file so the frozen executor-codex.test.ts does not grow past its cap. | ||
| test("CodexExecutor.transformRequest strips include from compact requests (#6805)", () => { | ||
| const executor = new CodexExecutor(); | ||
| const result = executor.transformRequest( | ||
| "gpt-5.3-codex", | ||
| { | ||
| _nativeCodexPassthrough: true, | ||
| include: ["reasoning.encrypted_content"], | ||
| instructions: "keep this", | ||
| stream: false, | ||
| }, | ||
| false, | ||
| { | ||
| requestEndpointPath: "/responses/compact", | ||
| providerSpecificData: { requestDefaults: { serviceTier: "priority" } }, | ||
| } | ||
| ); | ||
| assert.equal(result.include, undefined); | ||
| assert.equal(result._nativeCodexPassthrough, undefined); | ||
| assert.equal(result.instructions, "keep this"); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
delete body.include;statement here is redundant becausebody.includeis unconditionally deleted for compact requests later in the function (at line 1181, right afterensureCodexReasoningSummary(body)is called).Whether
ensureCodexReasoningSummarymodifies/addsincludeor returns early, the second deletion at line 1181 ensures thatincludeis completely stripped from the final payload. Removing this first deletion simplifies the code and avoids potential confusion about double-deletion.