fix(proxy): promote caller x-session-id to session_id so non-Codex prefixes stay warm by mayigululu-hash · Pull Request #6521 · lidge-jun/opencodex

mayigululu-hash · 2026-10-03T15:31:39Z

Summary

Non-Codex clients of the public endpoints (/v1/responses, /v1/messages) get zero prompt-cache reuse on ChatGPT-backed OpenAI models: with byte-identical prefixes every turn reports cached_tokens: 0 and — via usage.attribution — cache_write_tokens: 0, i.e. the backend never warms the prefix. In [Bug]: Non-Codex clients on public endpoints get zero prompt-cache reuse on ChatGPT-backed OpenAI models (no synthesized session_id/originator) #6520 one 25-turn agent task billed 1.92M uncached input tokens, while the same account and model from Codex clients reused 97%+ the same evening.
Cause: the ChatGPT Codex backend only reuses a warmed prefix when session_id is present ([Bug]: Grok Build turns on ChatGPT-backed OpenAI models get almost no prompt-cache reuse (missing session_id) #6481). fix(grok): forward the Grok conversation as session_id so OpenAI prefixes stay warm #6482 fixed that for the managed Grok surface only; every other non-Codex caller (third-party agents, plain HTTP clients) still carries no session identity, and claudeNativeSessionId only synthesizes one for Claude-Code-style metadata.user_id callers.
Fix: new src/server/caller-session-identity.ts. On the two public routes, when the caller sent no session_id/session-id/thread-id but did send a safe x-session-id (a per-conversation id agents like ZCode already emit; the proxy's own adapters treat the same header as a session marker), promote it to session_id — the same promotion shape as the Grok surface, with an explicit caller session header always winning. It is applied after withGrokSessionIdentity, so Grok-promoted requests are left untouched.

Evidence

Verified against the live ChatGPT backend (single ChatGPT account, gpt-6.1-sol): two identical ~2.3K-token requests without identity headers → both cached_tokens: 0; the same pair with the caller's x-session-id promoted into session_id → second request cached_tokens: 2176/2341 (93%). The promoted header rides the existing FORWARD_HEADERS relay, so the end-to-end test asserts it reaches the ChatGPT backend wire and that the request log gains a stable conversationId.

Tests

tests/server/caller-session-identity.test.ts: unit cases (explicit session headers win, unsafe/empty values ignored, body + abort signal preserved, composition after the Grok promotion) plus an end-to-end handleResponses case asserting the upstream request carries the promoted session_id, and wiring pins for both public routes.
Updated the two existing pins that assert the exact serve-options.ts call lines (grok-session-identity.test.ts, loopback-listener-admission.test.ts).
Layout registry and expected fixture updated for the new test file.
bun x tsc --noEmit clean; the four touched test files pass together (68 tests).

Fixes #6520.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.

Summary by CodeRabbit

New Features
- Session identity is now preserved across Responses and Claude Messages requests when a valid caller-provided session ID is available and no explicit session or thread ID takes precedence.

coderabbitai · 2026-10-03T15:32:06Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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: 4c933c6b-b127-42b5-ac9f-9be1e218341e

📥 Commits

Reviewing files that changed from the base of the PR and between 36330ae and c26edb9.

📒 Files selected for processing (7)

scripts/test-layout/layout.json
src/server/caller-session-identity.ts
src/server/index/serve-options.ts
tests/fixtures/test-layout-expected.json
tests/providers/xai/grok-session-identity.test.ts
tests/server/caller-session-identity.test.ts
tests/server/loopback-listener-admission.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

Walkthrough

The change adds caller session identity promotion from x-session-id and applies it to public Responses and Claude Messages requests. Explicit session headers take precedence. Tests cover validation, request preservation, route wiring, and propagation to the ChatGPT Codex backend.

Changes

Caller Session Identity

Layer / File(s)	Summary
Caller identity extraction and request wrapping `src/server/caller-session-identity.ts`, `tests/server/caller-session-identity.test.ts`	`callerSessionId` returns a trimmed `x-session-id` only when no recognized explicit session header is present and the value matches the allowed pattern. `withCallerSessionIdentity` returns the original request when no eligible ID exists, or clones it with `session_id` while retaining its body stream and abort signal. Tests cover these conditions and composition with Grok identity promotion.
Public route wiring and integration coverage `src/server/index/serve-options.ts`, `tests/server/caller-session-identity.test.ts`, `tests/providers/xai/grok-session-identity.test.ts`, `tests/server/loopback-listener-admission.test.ts`, `scripts/test-layout/layout.json`, `tests/fixtures/test-layout-expected.json`	The Responses handler applies caller identity after Grok identity. The Claude Messages handler applies caller identity to the request. Tests check route wiring and backend propagation, and the test-layout files map the new test to the `server` group.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c26ed

Caller session identity reaches both public request paths without an identified merge-blocking issue.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to c26ed

Conversation IDs now affect shared request-routing state, not only caching. Clients with matching IDs can influence another client's later routing. Authentication and existing destination controls limit the exposure; unauthorized data disclosure has not been established.

Retained concerns

Medium · security · inferred: Automatic promotion also makes x-session-id a key for shared combo-routing recall without authenticated-caller scoping. An admitted caller completing a configured combo request on a matching lane can influence another caller's subsequent compaction request when its bare model matches the remembered response model. The lane-only design already affected explicit session headers; this PR broadens the exposed client population. Existing target checks bound routing to configured targets, and no authorization bypass or data disclosure has been established.

Security review details

Security Blast Radius

inferred — The locally demonstrated interference boundary is shared routing recall within one proxy process. It requires an admitted caller, a matching normalized lane, a completed configured combo response and a victim request satisfying the compaction/model predicates. Recall contains routing identifiers, not retained credentials or prompt bodies.

Security Findings and Attack Paths

inferred — An admitted caller knowing or guessing another client's conversation ID can complete a combo request under that lane. A subsequent matching compaction request from an x-session-id-only client can then recall the seeded combo. The PR newly exposes that client to an existing lane-collision mechanism; the inspected path establishes routing influence, not unauthorized disclosure.

Trust Boundaries and Controls

observed — Local routing-state ownership is inconsistent: compact handoff requires an authenticated principal plus lane and declines identity-less or loopback admission, while combo recall reads and writes using the lane alone. Header validation does not resolve this ownership difference.
observed — Recall rejects expired, disabled or invalid configured targets. Combo processing also carries the current admission into child processing and uses admission-model scope for destination decisions. These controls constrain routing influence rather than authenticating the owner of a recalled lane.

Resilience and Maintainability Implications

observed — Recall is written through a response-completion callback and checks configuration generation and target validity. Its entry count, retained model bytes and lifetime are bounded, with expiry sweeping and generation reconciliation. These controls contain stale-state and resource growth, but do not prevent equal lanes from different callers sharing routing state.

Hardening Proposals

proposed — Separate local routing ownership from the upstream cache identifier: key shared recall by authenticated principal plus conversation lane while preserving the intended session_id wire value. Validate isolation using two independently admitted callers with the same conversation ID, including completion-order and compaction cases.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: promoting the caller's x-session-id to session_id to keep non-Codex prefixes warm.
Linked Issues check	✅ Passed	Issue `#6520` is closed and provides historical context only. No active directly linked target remains, so no linked-issue coding requirements apply.
Out of Scope Changes check	✅ Passed	The reported changes support the PR’s stated caller-session promotion: `src/server/caller-session-identity.ts` promotes eligible `x-session-id` values, and `src/server/index/serve-options.ts` applies …

Full details: Docstring Coverage

Explanation

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · 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

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

github-actions · 2026-10-03T15:32:06Z

✅ Deterministic PR hygiene checks passed.

github-actions · 2026-10-03T15:32:21Z

⏳ DRAFT

review readiness checklist open (0/4 boxes ticked).

What to do

Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
⬜ I resolved all correct Codex and CodeRabbit findings.
⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

✅ Deterministic PR hygiene checks passed.

…efixes stay warm

lidge-jun · 2026-10-04T02:19:50Z

Superseded by #6554 at c20c9e7, carrying source head 011ab50.

The replacement carries the full source helper, endpoint forwarding, precedence, body/abort preservation, wiring tests and test registrations. It scopes authenticated conversation markers by principal and preserves loopback marker behavior, with additional ingress/admission/isolation tests. No useful source behavior was dropped. mayigululu-hash is credited in commit and PR Co-authored-by trailers.

Independent current-head code/security review and source coverage are verified. The focused 59-test receipt and hosted structure/gates passed. Broader CI remains incomplete; the replacement is an unmerged draft. This is supersession, not a shipped-cache-improvement claim. Issue #6520 remains open until landing and its acceptance are established.

Refines #6521 with trusted-principal scoping and admission-time propagation. Co-authored-by: mayigululu-hash <[email protected]>

mayigululu-hash requested review from Ingwannu and lidge-jun as code owners October 3, 2026 15:31

github-actions Bot marked this pull request as draft October 3, 2026 15:32

github-actions Bot added the bug Something isn't working label Oct 3, 2026

fix(proxy): promote caller x-session-id to session_id so non-Codex pr…

011ab50

…efixes stay warm

mayigululu-hash force-pushed the fix/proxy-caller-session-identity branch from c26edb9 to 011ab50 Compare October 3, 2026 17:27

lidge-jun mentioned this pull request Oct 4, 2026

fix(server): promote scoped caller conversation headers #6554

Merged

3 tasks done

lidge-jun added the superseded label Oct 4, 2026

lidge-jun closed this Oct 4, 2026

lidge-jun added a commit that referenced this pull request Oct 4, 2026

fix(server): promote scoped caller conversation headers

89ae31b

Refines #6521 with trusted-principal scoping and admission-time propagation. Co-authored-by: mayigululu-hash <[email protected]>

Navigation Menu

Repository navigation

fix(proxy): promote caller x-session-id to session_id so non-Codex prefixes stay warm - #6521

fix(proxy): promote caller x-session-id to session_id so non-Codex prefixes stay warm#6521
mayigululu-hash wants to merge 1 commit into
lidge-jun:devfrom
mayigululu-hash:fix/proxy-caller-session-identity

mayigululu-hash commented Oct 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading

Draft PR not reviewed

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Oct 3, 2026

Uh oh!

github-actions Bot commented Oct 3, 2026 •

edited

Loading

Uh oh!

lidge-jun commented Oct 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mayigululu-hash commented Oct 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Evidence

Tests

Review readiness checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Draft PR not reviewed

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Oct 3, 2026

Uh oh!

github-actions Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏳ DRAFT

What to do

Review readiness checklist

Hygiene

Uh oh!

lidge-jun commented Oct 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mayigululu-hash commented Oct 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading

github-actions Bot commented Oct 3, 2026 •

edited

Loading