Skip to content

fix(chat): return gateway request-history IDs - #7261

Merged
lidge-jun merged 2 commits into
devfrom
codex/7257-chat-request-history-id
Oct 17, 2026
Merged

lidge-jun merged 2 commits into
devfrom
codex/7257-chat-request-history-id

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

Chat Completions records gateway usage history but omitted the ID needed to join a reply to its exact row. Return x-opencodex-request-id through the existing Responses helper when the Chat handler or a logged pre-handler refusal owns that row, including JSON/SSE replies and handler errors. Auth, origin, drain and active-turn refusals without a log owner keep no receipt header. Caller/upstream IDs cannot override the gateway ID; CORS exposes it without changing the response body.

Adds real-listener regressions for both listener policies, concurrent calls, usage correlation, cancellation and logged/unlogged refusals. Documents the header contract. Reuses the reporter's proposed fix with attribution.

Closes #7257

Verification

  • bun install — passed.

  • Regression before fix: 3 passed / 9 failed (the nine expected Chat header cases failed on absent IDs).

  • bun test tests/responses/chat-request-id-endpoint.test.ts — 13 passed / 0 failed.

  • Pinned Bun 1.4.3: bun test --isolate tests/responses/chat-request-id-endpoint.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/claude-integration/messages-request-id-endpoint.test.ts tests/claude-integration/messages-request-id-headers.test.ts — 71 passed / 0 failed. Initial Bun 1.4.0 invocation failed the tooling test's required-runner check; rerunning under the repository pin passed.

  • bun run typecheck — passed, including after the source-oracle update.

  • bun run structure:check — passed.

  • bun test tests/ci-workflows/file-size-ratchet.test.ts under pinned Bun — 9 passed / 0 failed.

  • Pinned Bun: bun test --isolate tests/server/loopback-listener-admission.test.ts tests/responses/chat-request-id-endpoint.test.ts — 44 passed / 0 failed. The first CI head exposed an indentation-specific source-oracle assertion; it now matches the Chat handler’s actual CORS arguments and bounds its scan to the Chat route.

  • bun run privacy:scan — passed.

  • Docs: bun install --frozen-lockfile and bun run build — passed; 577 pages and 81,420 internal links checked.

  • bun run test:changed — passed: 3,419 passed / 30 skipped / 0 failed across 159 files, exit 0. Acquired the shared runner after a 930-second queue wait; suite execution took 64.8 seconds.

  • Full local suite not run: concurrent release-train worktrees contend for the shared test runner; focused regressions cover the changed HTTP boundary and unchanged sibling header contracts. Broader Ubuntu validation is left to PR CI.

  • Hosted PR CI on head 8fddf05b77b4f238465eb20928f63e5f95528f11: run 38992364720 succeeded, including all four Ubuntu test shards and aggregate ci. Checkout used merge commit 48715ecb4ecd14fad06b391b16fbc5769b3f10d7 against dev at f190fc40906323a93a16729209d00414f8b602fe. macOS/Windows jobs were skipped under the repository PR policy. Initial run 38991666022 failed on the CORS source-oracle assertion described above; the corrected head passed its replacement run.

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.

Co-authored-by: daemons2000 1758467+daemons2000@users.noreply.github.com

Return the existing gateway request ID on Chat replies owned by a handler or logged admission refusal, preserving unlogged rejection behavior.

Co-authored-by: daemons2000 <1758467+daemons2000@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 47eee7c3-52de-4ff2-b70d-4554ffb48590
📥 Commits

Reviewing files that changed from the base of the PR and between 97a12fd and 8fddf05.

📒 Files selected for processing: 7
  • docs-site/src/content/docs/reference/proxy-formats.md
  • scripts/test-layout/layout.json
  • src/server/index/serve-options.ts
  • structure/data-planes/inbound-compat.md
  • tests/fixtures/test-layout-expected.json
  • tests/responses/chat-request-id-endpoint.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; 3 remain after this review.

📝 Walkthrough

The Chat Completions route now attaches the gateway request-history ID to responses when the request owns a history entry. Tests cover JSON and streaming responses, logged refusals, unlogged refusals, and matching usage-history rows. Documentation describes the header and its scope.

Changes: Chat Completions request-ID correlation

Layer / File(s) Summary
Request-ID ownership and endpoint coverage
src/server/index/serve-options.ts, tests/responses/chat-request-id-endpoint.test.ts, tests/server/loopback-listener-admission.test.ts, docs-site/src/content/docs/reference/proxy-formats.md, structure/data-planes/inbound-compat.md, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
The route adds the request-history ID only when it owns a history entry. Endpoint tests cover logged JSON and SSE responses, logged refusals, unlogged refusals, and matching usage rows. Documentation describes the header behavior, and the test layout maps the new endpoint test to the responses suite.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ChatCompletionsRoute
  participant runAdmittedHttpTurn
  participant UsageHistory
  Client->>ChatCompletionsRoute: Send Chat Completions request
  ChatCompletionsRoute->>runAdmittedHttpTurn: Process admitted request
  runAdmittedHttpTurn-->>ChatCompletionsRoute: Return result and request-log ownership
  ChatCompletionsRoute->>Client: Return response with ID when a history entry is owned
  runAdmittedHttpTurn->>UsageHistory: Record usage history row
Loading
Merge Risk: ⚪ Minimal · up to 8fddf

This change adds the gateway request-history ID header to logged Chat Completions replies without changing response bodies. No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (4 skipped: 4 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Issue #7257 is an active direct issue. The route change in src/server/index/serve-options.ts:1618-1630 applies withRequestLogId only when the handler or WorkflowRefusalLog.onLogged confirms requ…
Out of Scope Changes check Passed The changes remain within Issue #7257. The production change adds conditional Chat request-history ID response handling. The new endpoint tests cover the required response, refusal, correlation, usage…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: Chat Completions now return gateway request-history IDs.
Full details: Linked Issues check

Explanation

Issue #7257 is an active direct issue. The route change in src/server/index/serve-options.ts:1618-1630 applies withRequestLogId only when the handler or WorkflowRefusalLog.onLogged confirms request-log ownership. The existing withRequestLogId helper in src/server/index/startup-warnings.ts preserves the response body and adds x-opencodex-request-id to CORS exposure. tests/responses/chat-request-id-endpoint.test.ts covers loopback and authenticated JSON/SSE replies, handler validation errors, upstream HTTP errors, workflow and body-capacity refusals, concurrent calls, upstream and caller ID override attempts, and cancellation. The tests also verify the returned ID matches one canonical chat history row and that usage values remain correct. Auth, origin, and active-turn refusals assert no header and no new row. The focused validation reported in the PR description passed, and the documentation updates describe the header contract. No linked coding requirement remains unmet.

Full details: Out of Scope Changes check

Explanation

The changes remain within Issue #7257. The production change adds conditional Chat request-history ID response handling. The new endpoint tests cover the required response, refusal, correlation, usage, concurrency, CORS, and cancellation behavior. The test-layout entries and the listener-test regex update maintain the repository test structure for those tests. The proxy-format and inbound-compatibility documentation changes document the new contract. No provider credentials, routing rules, response bodies, or unrelated runtime behavior are changed.

Full details: Docstring Coverage

Explanation

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

Resolution

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 17, 2026
@lidge-jun
lidge-jun marked this pull request as ready for review October 17, 2026 01:53
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 17, 2026 01:53
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

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-17T01:55:24.160518Z 8fddf05 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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev (MAINTAINERS.md maintainer-integration exception) for the 2.88.0 train.

Exact head 8fddf05b77b4f238465eb20928f63e5f95528f11: PR CI run 38992364720 completed successfully (gates, test 1/4-4/4, aggregate ci); no unresolved review threads. Reviewed the diff: Chat Completions now returns the gateway history id only when the handler or a logged pre-handler refusal owns the history row; unlogged auth/origin/active-turn refusals stay without an id, and caller/upstream values cannot replace it. The only shared files with #7262 (now on dev) are the two test-layout maps, with distinct keys; GitHub reports the PR mergeable against the current dev.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant