Repository navigation
fix(proxy): keep serving briefly after SIGTERM so the gateway moves off a draining pod - #5474
Conversation
/_proxy/health is a static 200 and is used for readiness as well as liveness, so a draining proxy stays in Service endpoints while it answers every new request with 503. Add /_proxy/ready, which returns the draining 503 once shutdown starts, so Kubernetes stops routing to the replica. /_proxy/health stays static so liveness never restarts a pod mid-drain.
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
|
@codex review |
|
Score: 90/100. Small, correct change that adds a readiness endpoint which fails while draining.
Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The gateway reaches the proxy over pooled keep-alive connections through the Service. Requests already queued on those connections got the draining 503 the moment shutdown started, and the gateway passed it to users. For SHUTDOWN_ADMISSION_GRACE_MS (default 10s, within the drain budget) a draining proxy still serves new requests and closes each connection after the response, so the gateway reconnects through the Service, which no longer routes to the terminating pod. New WebSocket upgrades and requests after the window still get the 503. The request drain then waits for the remaining budget.
|
@codex review |
kojiwakayama
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0913cdc2d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Refuse new SSE streams (Accept: text/event-stream and the control-plane run stream POST) as well as WebSocket upgrades from the start of the admission grace window; they could outlive the remaining drain budget. - Grant the admission grace only for SIGTERM; Ctrl-C (SIGINT) shuts down without the 10s wait and keeps the full drain budget. - Wait out the grace window, and poll the drain, on a setTimeout captured before extensions load, so a replaced timer cannot skip cleanup. - Document SHUTDOWN_ADMISSION_GRACE_MS (default, max, interaction with SHUTDOWN_DRAIN_TIMEOUT_MS) beside the other proxy settings. - Give the memory smoke a 1s grace so the idle proxy exits inside `docker stop --time 5` instead of being SIGKILLed.
|
@codex review |
|
@codex review |
Moves the draining Connection: close decision out of the router into ProxyDrainAdmission.closeIfDraining, which brings the router back under Sonar's cognitive-complexity limit and puts the branch under unit test.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a301557b2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Capture performance.now (and Reflect.apply) at module evaluation so a replaced Performance.prototype.now cannot make the signal handler throw in drainAdmission.begin before draining and cleanup run. - Run the five SIGTERMed binary smoke probes with SHUTDOWN_ADMISSION_GRACE_MS=0 so each idle proxy exits at once.
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Bun defines now on the performance instance, not Performance.prototype, so the captured-clock test patches whichever object owns it.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31af7efc96
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…hile draining - A GET/HEAD admitted during the drain whose response is an SSE stream (negotiated by the response, without Accept: text/event-stream) gets the draining 503 so the client reconnects to a replica that stays; its upstream body is cancelled. Non-idempotent streams are kept. - Router error responses (the Host 400 and, while draining, unexpected failures as a reported 500) also close their connection, so the gateway does not stay pinned to the terminating pod. - Capture Math.max/Math.ceil before extensions load and move the drain budget arithmetic into ProxyDrainAdmission.remainingDrainBudgetMs.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 866cd8f9bf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
useChat and useAgent POST /api/ag-ui and /api/agents/:id without Accept: text/event-stream, so admission now classifies those routes as long-lived and refuses them before their run starts.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cac5aba06
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
POST /api/agents/:id returns JSON (useAgent parses response.json()), so only the AG-UI SSE endpoint is refused before it starts.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85e7f15cd0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
waitForDrain and getInFlightRequests now use Math.min, Number.isSafeInteger, Array.from and Map.prototype.values captured before extensions load, so a replaced intrinsic cannot cut the drain short or skip cleanup.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6193627cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…t mode - useChat sends Accept: text/event-stream on every chat POST (callers can still override it), so a draining proxy refuses new chat streams on any configured `api` path, not only /api/ag-ui. - Local `serve --split` starts the proxy child with SHUTDOWN_ADMISSION_GRACE_MS=0: it has no gateway to wait for and its signal handler forwards SIGTERM.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d4bf1d1c6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Both hooks POST and read a streamed body; they now send an Accept that lists text/event-stream (with text/plain and */* fallbacks), so a draining proxy refuses them before the work starts. Swept the other framework React hooks: useChat already declares it, useAgent reads JSON.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a03cd9547
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
useCompletion and useStreaming decode raw text, so an endpoint that negotiates on Accept must keep choosing plain text. SSE stays listed at q=0.9 so a draining proxy still recognises the request as a stream.
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Requests failed with 503 during proxy rollouts and node drains.
The gateway reaches the proxy over pooled keep-alive connections through the Service. When a proxy pod got SIGTERM it returned the draining 503 to every new request at once, including requests that were already arriving on the gateway's open connections, and the gateway passed those 503s to users. The Service itself stops routing new connections to a terminating pod, so the gap is the reused connections.
Changes
SHUTDOWN_ADMISSION_GRACE_MSafter SIGTERM (default 10s, capped at the drain budget), the proxy keeps serving new requests and sendsConnection: closeon each response. The gateway reconnects through the Service and lands on a pod that stays. After the window, new requests get the draining 503 as before. A new WebSocket upgrade gets the 503 right away.SHUTDOWN_DRAIN_TIMEOUT_MS - grace), so the total stays within the Kubernetes grace period. The routing invalidation subscription stays open throughout, as before./_proxy/ready. A readiness endpoint that returns 503 once shutdown starts./_proxy/healthis unchanged and stays the liveness and startup target. The chart can switch readiness to it once a server image contains it (an older image would answer 404); with this PR it is optional.I checked that Deno.serve closes an HTTP/1.1 keep-alive connection after a response carrying
Connection: close(one response, then EOF; without the header the same connection serves both requests).Tests
request-drain.test.ts: the admission window opens on the first signal and isn't extended by a second;withConnectionCloseworks on mutable and immutable (fetched) responses; the env name in parse errors; readiness before and after shutdown.main.test.ts: the ordering of readiness, refusal and tracking;Connection: closeon admitted drain responses; grace before drain wait; the reduced drain budget.deno test src/proxy/main.test.ts src/proxy/request-drain.test.ts: 4 passed (33 steps).deno check src/proxy/main.tspasses.(drafted by Claude for Koji)