[luvs01]

Summary

Closes #7462.

When the upstream WebSocket sends x-reasoning-included only in its HTTP 101 upgrade, the relay currently loses that signal while synthesizing the downstream HTTP/SSE response. Preserve the actual value from Bun's handshake event, make it available only after a successful open, and seed each canonical ChatGPT Responses exchange from that validated connection default.

• Retain only x-reasoning-included, with bounded parsing and HTTP header validation. Do not replay credentials, cookies, framing, request IDs, model/turn state, quota, or credit headers from a reusable connection.
• Preserve per-response metadata precedence, the existing header-commit boundary, connection reuse, terminal usage, and the original HTTP fallback on failed upgrades. Missing signals remain missing; literal false and empty values are preserved.
• Add real Bun loopback and malformed-upgrade regression tests, register both test files, and update the transport contract plus English/Korean integration docs.

This fixes the upstream-WebSocket-to-downstream-HTTP/SSE projection. It does not change the already-established downstream WebSocket handshake, infer a missing upstream signal, or claim to resolve all native Codex early-compaction/accounting cases.

Verification

Validation is on Linux, with the pinned application runtime Bun 1.4.2 and test-runner Bun 1.4.3.

Check	Result
Bun 1.4.2: bun test tests/responses/codex-ws-handshake.test.ts	8 passed; 85 assertions
Bun 1.4.3: bun test tests/responses/codex-ws-handshake.test.ts	8 passed; 85 assertions
Bun 1.4.3: bun test tests/responses/codex-ws-handshake-validation.test.ts	20 passed; 126 assertions
bun run typecheck	Passed
bun run structure:check	Passed
bun run privacy:scan	Passed
git diff --cached --check	Passed
Explicit layout, fixture-oracle, file-size and Lab boundary regressions (command below)	61 passed; 0 failed; 744 assertions
cd docs-site && bun install --frozen-lockfile && bun run build	Passed; generated English/Korean changes verified

The main real-loopback regression was also run against an isolated copy of base 12025981e5a304cdb6b1b54f525ba8554f623977: it failed exactly at the missing header (null instead of true), after response completion and usage assertions had passed. The fixed source passes on both Bun versions. The suite includes an actual invalid Sec-WebSocket-Accept response to verify that a handshake event alone cannot contaminate HTTP fallback.

The docs build exited 0 but its captured log ended before the final summaries. All 584 sitemap routes exist, all 585 generated HTML files are nonempty, and the build's internal-link hook separately passed against that output: 83,065 links checked.

The explicit read-as-data checks were run with inherited proxy variables removed:

bun run test --parallel=2 \
  ./tests/test-layout.test.ts \
  ./tests/test-layout-tooling.test.ts \
  ./tests/ci-workflows/file-size-ratchet.test.ts \
  ./tests/lab/core-lab-boundary.test.ts

Broad local result is not green: OCX_TEST_RUNNER_BUN=<Bun-1.4.3> bun run test:changed --parallel=2 reached the runner's 900-second limit (exit 124). Its partial output recorded 11,281 passing and 141 failing test records, with 215 files not started. Several failures show unexpected HTTP 400/503 results with the environment's inherited outbound proxy; there are also timeouts. Representative comparisons against the unchanged base are complete on Bun 1.4.3. The two network cases use the same escalated environment as the broad run; the steering case uses sandboxed execution with proxies removed.

Representative case	Execution condition	Unchanged base	Patched code
reasoning-retention-v2.test.ts: canonical native-v2 case	Escalated, inherited proxies	HTTP 400 failure	HTTP 400 failure
Same retention case	Escalated, proxies removed before startup	Passed; 7 assertions	Passed; 7 assertions
management-auto-review-auth.test.ts: listener-auth case	Escalated, inherited proxies	HTTP 503 failure	HTTP 503 failure
Same management case	Escalated, proxies removed before startup	Passed; 16 assertions	Passed; 16 assertions
ws-native-steering.test.ts: large-output replay case	Sandboxed, proxies removed, default 5-second limit	Timeout	Timeout

The large-output case calls NativeSteeringReplay directly and does not call the modified transport modules. These are baseline controls, not a performance benchmark. Selected commands were run in both the current checkout and an isolated copy whose three modified runtime files were verified byte-for-byte against base 12025981e5a304cdb6b1b54f525ba8554f623977:

bun test ./tests/responses/reasoning-retention-v2.test.ts -t 'canonical native v2 retains its original protocol and creates no local archive'
bun test ./tests/server/management-auto-review-auth.test.ts
bun test ./tests/responses/ws-native-steering.test.ts -t 'large output arrays stay ordered below the replay byte limit'

The proxy control removes HTTP_PROXY, HTTPS_PROXY, ALL_PROXY, NO_PROXY and their lowercase equivalents before the selected bun test process starts. The remaining local failures were not individually classified; the local broad run remains recorded as unsuccessful.

Within that run, the existing directly relevant files completed with 193 passing tests and one pre-existing skip: ws-upstream.test.ts (127 pass, 1 skip), ws-upstream-reuse.test.ts (37 pass), and ws-pre-output-refusal.test.ts (29 pass). No passing file was rerun solely for confidence.

Hosted validation completed successfully on exact head 4e9212662e3e173ab7e5372b712da6c1fdecd54d: all four full-suite shards and aggregate ci passed in run 39555730150. The new validation file's 20 cases passed in shard 3, and the real-handshake file's 8 cases passed in shard 4. Typecheck/GUI/privacy gates, docs build, structure, packaging, Docker, keyring, catalog contracts, React Doctor, PR target and hygiene checks also passed.

An independent maintainer review reports additional Bun 1.4.2 coverage (250 passed, 1 intentional skip) and Bun 1.4.3 coverage (64 passed), plus a manual response-header boundary review. That is a COMMENTED review, not an approval; no unresolved inline review findings were present when readiness was assessed.

The full local suite is deferred under the documented resource exception: the import-graph run alone exceeded 15 minutes with two workers in this environment. The focused real-transport tests above cover the changed behavior. The repository-wide Linux suite completed in hosted CI as recorded above. Windows/macOS jobs were not selected by this PR's automatic scope and are not claimed as tested. The new handshake regressions use only loopback sockets and synthetic payloads, with no real credentials.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

• New Features

  • Preserved upstream reasoning-inclusion metadata for HTTP and SSE clients using the canonical ChatGPT route.
  • Retained this metadata across reused WebSocket connections.
  • Applied response-level metadata before connection defaults, including explicit “false” values.

• Bug Fixes

  • Rejected malformed, oversized, or invalid handshake metadata without affecting fallback behavior.
  • Prevented metadata from leaking between connections or changing after a response is committed.

• Documentation

  • Documented reasoning metadata behavior and the limitation that completed client-facing WebSocket upgrades cannot receive headers retroactively.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 57d2c198-9b8c-47fc-92f7-b9d757a30702

📥 Commits

Reviewing files that changed from the base of the PR and between 1202598 and 4e92126.

📒 Files selected for processing: 12

• docs-site/src/content/docs/guides/codex-integration.md
• docs-site/src/content/docs/ko/guides/codex-integration.md
• scripts/test-layout/layout.json
• src/server/responses/codex-ws-exchange.ts
• src/server/responses/codex-ws-metadata.ts
• src/server/responses/codex-ws-session.ts
• structure/transports/responses-wire-shapes.md
• structure/transports/responses.md
• structure/transports/streaming-health.md
• tests/fixtures/test-layout-expected.json
• tests/responses/codex-ws-handshake-validation.test.ts
• tests/responses/codex-ws-handshake.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The canonical Responses WebSocket relay now captures a validated upstream x-reasoning-included upgrade header and uses it to seed exchange metadata for HTTP/SSE responses. Per-exchange metadata precedence and response-header commit behavior remain covered by tests. Documentation describes the signal’s limits.

Changes: Canonical Responses WebSocket header forwarding

Layer / File(s)	Summary
Capture and validate upgrade headers src/server/responses/codex-ws-metadata.ts, src/server/responses/codex-ws-session.ts, tests/responses/codex-ws-handshake-validation.test.ts	The session parses bounded upgrade metadata and retains the allowlisted reasoning-inclusion header after a successful connection open. Tests cover malformed headers, header limits, post-open events, and connection isolation.
Project connection metadata into exchanges src/server/responses/codex-ws-exchange.ts, tests/responses/codex-ws-handshake.test.ts, structure/transports/*, docs-site/src/content/docs/guides/codex-integration.md, docs-site/src/content/docs/ko/guides/codex-integration.md, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json	Each exchange seeds metadata from the opened session’s upgrade headers. Integration tests cover connection reuse, metadata precedence, response-header commit boundaries, and fallback behavior. Transport and integration documentation describes the forwarding behavior and its limits.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant UpstreamBunWebSocket
  participant CodexWsSession
  participant codexWsExchange
  participant HTTP_SSE_Client
  UpstreamBunWebSocket->>CodexWsSession: handshake event with upgrade response
  CodexWsSession->>CodexWsSession: validate and retain reasoning-inclusion header
  UpstreamBunWebSocket->>CodexWsSession: open event
  CodexWsSession->>codexWsExchange: provide opened connection headers
  codexWsExchange->>codexWsExchange: seed exchange metadata
  codexWsExchange->>HTTP_SSE_Client: return response with committed headers and SSE body

Loading

Possibly related PRs

• lidge-jun/opencodex#3664: Introduced the reusable CodexWsSession and codexWsExchange flow that this change extends.

Merge Risk: ⚪ Minimal · up to 4e921

The upgrade-only reasoning header is forwarded in the exercised loopback path. No demonstrated issue requires a fix before merging.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (7 skipped: 7…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving reasoning-inclusion metadata that appears only in the WebSocket upgrade response.
Linked Issues check		Issue #7462 is directly linked and has coding requirements. The implementation in src/server/responses/codex-ws-session.ts captures Bun's handshake event, accepts the value only after open, expo…
Out of Scope Changes check		The changed runtime files implement the #7462 handshake-to-HTTP/SSE projection. The new tests directly verify that behavior and its failure boundaries. Test-layout updates register those tests. The tr…

Full details: Linked Issues check

Explanation

Issue #7462 is directly linked and has coding requirements. The implementation in src/server/responses/codex-ws-session.ts captures Bun's handshake event, accepts the value only after open, exposes it only for an opened non-closed session, and clears it on detach. src/server/responses/codex-ws-metadata.ts accepts only a bounded, validated HTTP 101 header set and retains only x-reasoning-included; it preserves empty and false values and rejects malformed input. src/server/responses/codex-ws-exchange.ts seeds the canonical Responses metadata projection from the validated connection headers. The seed does not replace an existing per-response value, and the existing commit path still freezes headers before later events. The added tests/responses/codex-ws-handshake.test.ts and tests/responses/codex-ws-handshake-validation.test.ts cover loopback handshakes, reuse, precedence, malformed upgrades, bounds, fallback isolation, filtering, and terminal behavior. The test-layout files register both suites. The transport contract and English/Korean integration documents state the new behavior and its limits. The supplied validation reports the focused suites and hosted full suite as passing. No requirement in #7462 remains unmet.

Full details: Out of Scope Changes check

Explanation

The changed runtime files implement the #7462 handshake-to-HTTP/SSE projection. The new tests directly verify that behavior and its failure boundaries. Test-layout updates register those tests. The transport contract and English/Korean documentation describe the retained signal, precedence, commit boundary, connection reuse, and WebSocket limitation. No changed file shown in the supplied whole-PR summary adds unrelated runtime behavior, credentials, usage rewriting, downstream WebSocket changes, or signal inference. The reported local broad-run timeout does not identify an unrelated code change, and hosted validation passed.

Full details: Docstring Coverage

Explanation

Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (7 skipped: 7 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[Ingwannu]

Independent review of exact head 4e9212662e3e173ab7e5372b712da6c1fdecd54d against 12025981e5a3:

• Production Bun 1.4.2, isolated homes and hard cgroup limits: 250 passed, 1 intentional older-runtime-only skip, 0 failed across your two new files, my six independently written real-loopback controls, and existing WS upstream/reuse/pre-output-refusal/stage suites. The independent controls cover upgrade-only true/false/absent, prelude override, late-frame commit, retained connection, private snapshots and disposal. They were temporary validation files, removed afterward, not changes to your branch.
• Test Bun 1.4.3: 64 passed, 0 failed across your two new files and both layout guards/file-size ratchet.
• Manual response-header boundary review: only the bounded actual inclusion signal is retained; cookies, credentials, framing and per-turn quota/request IDs are excluded. Candidate metadata is promoted only after a successful open, failed upgrades preserve the original HTTP fallback, and per-exchange metadata/usage retain their owner. No Codex Security scan or live provider call was used.

Your PR arrived while I was independently preparing the same repair. I am withdrawing my redundant #7465 in favor of this earlier contributor PR; my regression evidence supports this head. No source from your branch was copied into my alternative, and no changes were pushed to your branch. The full diff and unchanged workflow/install surfaces were reviewed; hosted functional CI is already executing, so no pending fork-execution approval was needed when checked. Readiness remains yours; actual completed CI and remaining review findings still govern final approval.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-21T06:48:06.443121Z	4e92126	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[luvs01]

@Ingwannu Thank you for the independent verification of #7464 and for closing #7465 as a duplicate. I've assigned you as the assignee because I want to settle the remaining design choices with the maintainer rather than treat my earlier implementation—or green CI—as the final architectural decision. I place particular weight on the maintainers' preferred transport contract and long-term maintenance cost.

I compared the actual patches, not just their test results. Both correctly target only the observed x-reasoning-included on a successful upstream WS upgrade, leave request-scoped metadata authoritative, and do not synthesize usage or alter retries.

Design	Advantages	Trade-offs
#7464 (current PR)	Explicit pending → successful-open promotion; validates every raw header name/value, budgets and Connection-nominated hop-by-hop fields; a dedicated metadata.seed() does not charge a synthetic control frame against the prelude byte budget; extensive real-upgrade/failure/reuse tests and EN/KO docs.	Larger patch and documentation surface; duplicate inclusion headers are combined into e.g. true, false, which is not a meaningful single-valued signal.
#7465 (your alternative)	Smaller, simpler patch; reuses the established metadata.consume() path and its checks; rejects duplicate inclusion fields as ambiguous; has a focused contract document and ADR.	A fabricated metadata event consumes part of the prelude-frame budget despite not arriving as a frame; the raw parser does not check unrelated header-name syntax or Connection: x-reasoning-included nomination.

Decisions where I would value your design preference before modifying #7464:

1. Duplicate inclusion headers. I lean toward your fail-closed behavior (omit an ambiguous duplicate), rather than forwarding a comma-combined value. It would also drop identical duplicate lines; please advise whether that is the intended contract.
2. Metadata ownership. I lean toward keeping fix(responses): preserve upgrade-only reasoning inclusion #7464's dedicated, connection-only seed(): an HTTP 101 header is not a codex.response.metadata frame and should not spend its prelude budget. If consistency with the existing consume() path is more important, I can use your approach.
3. Documentation structure and validation strictness. fix(responses): preserve WS upgrade reasoning signal #7465's focused ADR/contract is easier to locate; fix(responses): preserve upgrade-only reasoning inclusion #7464 retains stricter hop-by-hop/header-name checks and bilingual user documentation. Should we adopt the ADR layout while retaining those checks, or keep the current structure to minimize churn?

One shared upstream caveat: current Codex readers treat x-reasoning-included largely as a presence signal, even if its value is false. Both PRs intentionally relay the actual value rather than invent semantics. I would keep that transport-fidelity boundary unless we agree on a verified client contract.

I have not changed the passing #7464 implementation based on these preferences alone. My goal is to converge on whichever design you consider best for this repository, then make any agreed changes in #7464, without reviving a competing PR.

[Ingwannu]

Approved exact head 4e9212662e3e173ab7e5372b712da6c1fdecd54d against current dev 12025981e5a3, following the independent transport/header-boundary review recorded above. Production Bun 1.4.2: 250 passed, 1 intentional older-runtime-only skip, 0 failed, including six independently written loopback controls; Bun 1.4.3: 64 passed, 0 failed with layout/ratchet guards. Actual current-head Cross-platform CI, Catalog contracts and React Doctor now completed successfully, including gates/typecheck, four selected test shards and docs build. The author has completed readiness on this unchanged head, with no open review threads or maintainer change request. Capture is actual-signal-only, promoted after successful open, bounded and cleared at disposal, with per-response metadata precedence and unchanged fallback/usage. Client-upgrade/accounting limitations remain explicitly documented. My duplicate #7465 is closed in favor of this contributor implementation. No Codex Security scan, live account request, self-approval, merge or release was performed.