Repository navigation
feat(server): return trusted Responses request ids #2827
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 |
|---|---|---|
|
|
@@ -442,6 +442,37 @@ function attachLiveSidebandUpstream( | |
| // trackSseForRequestLog( | ||
| // export function relaySseWithHeartbeat | ||
|
|
||
| const REQUEST_LOG_ID_RESPONSE_HEADER = "x-opencodex-request-id"; | ||
|
|
||
| function withRequestLogId(response: Response, requestId: string): Response { | ||
| const headers = new Headers(response.headers); | ||
| headers.set(REQUEST_LOG_ID_RESPONSE_HEADER, requestId); | ||
| // A custom `x-` header is not CORS-safelisted, so cross-origin JavaScript gets null from | ||
| // `response.headers.get()` even though the header is on the wire. Naming it here is what | ||
| // makes the id readable by a browser client — the only caller that needs a correlation id | ||
| // it did not send itself. | ||
| // | ||
| // Appending to whatever `withCors` already set, rather than overwriting, keeps this | ||
| // independent of the CORS layer: if the data plane later exposes another header, both | ||
| // survive. Duplicate names are harmless, and the header stays absent from responses that | ||
| // never reach this wrapper, so no management or rejected-origin response is widened. | ||
| const exposed = headers.get("Access-Control-Expose-Headers"); | ||
| const already = (exposed ?? "") | ||
| .split(",") | ||
| .some(name => name.trim().toLowerCase() === REQUEST_LOG_ID_RESPONSE_HEADER); | ||
| if (!already) { | ||
| headers.set( | ||
| "Access-Control-Expose-Headers", | ||
| exposed ? `${exposed}, ${REQUEST_LOG_ID_RESPONSE_HEADER}` : REQUEST_LOG_ID_RESPONSE_HEADER, | ||
| ); | ||
| } | ||
| return new Response(response.body, { | ||
| status: response.status, | ||
| statusText: response.statusText, | ||
| headers, | ||
| }); | ||
| } | ||
|
|
||
| export interface StartServerDeps { | ||
| /** Test-only seam; production always initializes its own management credential state. */ | ||
| managementAuthState?: ManagementAuthState; | ||
|
|
@@ -1426,7 +1457,10 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server<W | |
| finalizeNativePassthroughLog(499, { closeReason: "client_cancel" }); | ||
| }, | ||
| }); | ||
| return withCors(responseWithDeferredRequestLog(response, requestId, start, logCtx), req, policy); | ||
| return withRequestLogId( | ||
| withCors(responseWithDeferredRequestLog(response, requestId, start, logCtx), req, policy), | ||
| requestId, | ||
| ); | ||
|
Comment on lines
+1460
to
+1463
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.
When the active-turn limit is reached, Useful? React with 👍 / 👎. |
||
| }); | ||
| } | ||
|
|
||
|
|
||
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.
This adds a user-visible wire contract, but the commit does not update
docs-site/, so API clients have no reference describing the header name, its authentication/origin-admission boundary, or that browser callers may read it through CORS. Add it to the canonical Responses API documentation and keep translations non-contradictory.AGENTS.md reference: src/AGENTS.md:L28-L29
Useful? React with 👍 / 👎.