Skip to content

fix(codex): strip include from compact responses requests - #6805

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.47from
yinaoxiong:fix/codex-compact-include-clean
Jul 10, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.47from
yinaoxiong:fix/codex-compact-include-clean

Conversation

@yinaoxiong

@yinaoxiong yinaoxiong commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Strip include from Codex /responses/compact requests.
  • Apply the strip both before normal request shaping and again after ensureCodexReasoningSummary(), because that helper can re-add include: ["reasoning.encrypted_content"] when reasoning is enabled.
  • Keep regular Codex /responses behavior unchanged, including encrypted reasoning content replay support.

Related Issues

Context / Root Cause

/responses/compact is stricter than normal Codex /responses. OmniRoute already treats it specially by removing fields such as stream, stream_options, client_metadata, and store. The remaining gap is include.

The failure path is:

  1. Compact requests enter CodexExecutor.transformRequest() with requestEndpointPath: "/responses/compact".
  2. The compact branch removes compact-incompatible fields early.
  3. Later, ensureCodexReasoningSummary() sees a non-none reasoning effort and re-adds include: ["reasoning.encrypted_content"].
  4. Upstream Codex rejects the compact request with 400 Unknown parameter: 'include'.

This PR deletes include in the compact branch, and deletes it again after reasoning summary injection so the final compact payload stays compatible.

Upstream Codex has the same split between normal Responses payload construction and compact payload construction:

Validation

  • npm run lint
  • npm run test:unit
  • npm run test:coverage
  • Coverage is still >= 60% for statements, lines, functions, and branches
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below

Focused validation run locally:

  • node --import tsx/esm --test tests/unit/executor-codex.test.ts — 40/40 passing
  • GITHUB_BASE_REF=main node scripts/check/check-pr-test-policy.mjs — PASS after adding the regression assertion
  • GITHUB_BASE_REF=main npm run check:test-masking — PASS
  • Commit hook: Prettier + ESLint on the touched TS files, docs sync, check:any-budget:t11, check:tracked-artifacts
  • Push hook: check:any-budget:t11, check:tracked-artifacts

Test Output

✔ CodexExecutor.transformRequest preserves compact requests and native passthrough semantics
ℹ tests 40
ℹ pass 40
ℹ fail 0
## PR Test Policy
Changed production files: 1
Changed automated test files: 1
Result: PASS

Tests Added Or Updated

  • tests/unit/executor-codex.test.ts now asserts compact requests strip include while preserving existing compact passthrough behavior.

Coverage Notes

The production change is limited to open-sse/executors/codex.ts. It only affects requests detected as Codex /responses/compact; normal /responses requests still keep include when reasoning summary support requires encrypted reasoning content.

Reviewer Notes

  • The two delete body.include calls are intentional:
    • the first removes any client-provided include from compact requests;
    • the second removes the value that ensureCodexReasoningSummary() can synthesize later.
  • This PR deliberately excludes Docker, compose, docs, and unrelated deployment changes.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants