fix(auth): treat empty upstream completions as retriable failures - #4881
fix(auth): treat empty upstream completions as retriable failures#4881warelik wants to merge 10 commits into
Conversation
|
This pull request targeted The base branch has been automatically changed to |
![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: d82c51cf12
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 426e5b5e03
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d447213f05
ℹ️ 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".
|
Thanks for the review — all four points addressed in the latest commits:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f6e81b5a5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d0ea62260
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 301b7b5026
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe5b7c96e8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a5cee57cc
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c873518b30
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4485f59a2f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a2c546c3
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 691f2a9f29
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5420c1211f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 635c340033
ℹ️ 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".
Current-head summary (@27667c9d) — CPA follow-up review contextThis revision adds four accepted logical commits on top of the existing empty-completion retry feature, now merged with latest Route-exhaustion header preservation (P2 — now fixed)The wrapper no longer drops upstream header carriers. Out-of-scope threads (empty_completion.go / selector.go)Two remaining P2 threads target the empty-completion feature ( New fixes in this revision
Verification (head
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27667c9da8
ℹ️ 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".
Current-head summary (@e59758d) — surface errors before stream DONE
ChangeBoth OpenAI initial streaming peek loops now consume a buffered pending error before committing SSE headers:
When Deterministic actual-handler regressions (real executor + registered auth)
Verification (head
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e59758d601
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12c609f78c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7b1c2aff8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22deb3eb23
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89e87a859f
ℹ️ 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".
89e87a8 to
5c6295e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c6295e5a1
ℹ️ 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".
067ec47 to
a3bd120
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3bd120da6
ℹ️ 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".
0953456 to
c982b65
Compare
c982b65 to
bbccdfa
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04ae4ef71e
ℹ️ 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".
The previous revert of sdk/cliproxy/auth to origin/dev removed changes that belong to PR router-for-me#5150. Restore only the replay-alias/session-cache parts from 9031522: - sdk/cliproxy/auth/session_cache.go: all router-for-me#5150 additions (RestoreAliasesIfAbsent, SetAliasesIfAllAbsent, SetAliasesIfNoConflict, ReplaceAliasesIfUnchanged, compact helpers) used for replay alias binding. - sdk/cliproxy/auth/selector_test.go: only the session-cache unit tests (SetAliasesIfAllAbsent, SetAliasesIfNoConflict, ReplaceAliasesIfUnchanged). Left the SessionAffinitySelector.Pick alias-rebind tests and selector.go changes out because they overlap with router-for-me#4881. Build and go test ./... pass.
13dd4c9 to
2e37241
Compare
Finding from router-for-me#4881 review: internal/pluginhost/executor_route.go:248 `discardStreamChunks` started a goroutine that ranged over the source channel forever. If a plugin left the channel open after an empty terminal frame, the drainer goroutine never exited, causing a cumulative leak in a long-lived proxy. - Pass the request context into `discardStreamChunks`. - Add `streamDrainTimeout` (5s default); the drainer exits on context cancellation, on timeout, or when the source channel closes. - Reset the drain timeout on each received chunk so slow trailing chunks are still drained, but the goroutine is always bounded. - Add `TestDiscardStreamChunksExitsOnContextCancel` and `TestDiscardStreamChunksExitsOnOpenUnclosedChannel` proving the goroutine exits even when the source channel is never closed. - Update reports/4881-rebuild.md with the take/no-take findings and router-for-me#5130 status.
`discardStreamChunks` started a goroutine that ranged over the source channel forever. If a plugin left the channel open after an empty terminal frame, the drainer never exited, causing a cumulative leak in a long-lived proxy. - Pass the request context into `discardStreamChunks`. - Add `streamDrainTimeout` (5s default); the drainer exits on context cancellation, on timeout, or when the source channel closes. - Reset the drain timeout on each received chunk so slow trailing chunks are still drained, but the goroutine is always bounded. - Add `TestDiscardStreamChunksExitsOnContextCancel` and `TestDiscardStreamChunksExitsOnOpenUnclosedChannel` proving the goroutine exits even when the source channel is never closed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e37241300
ℹ️ 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".
5d35e9d to
a995f7b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a995f7b18a
ℹ️ 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".
… and timeout `sdk/cliproxy/auth/conductor_stream.go` had its own `discardStreamChunks` variant that started a goroutine ranging over the source channel forever. Without a context, a plugin that left the channel open leaked a goroutine per stream. - Add `ctx` parameter and `streamDrainTimeout` (5s default) to the auth `discardStreamChunks`, mirroring the pluginhost fix. - The drainer exits on context cancellation, on timeout, or when the source channel closes; the timeout resets on each received chunk so slow trailing chunks are still drained. - Update all call sites in `conductor_stream.go` to pass the request context. - Add `TestDiscardStreamChunksExitsOnContextCancel` and `TestDiscardStreamChunksExitsOnOpenUnclosedChannel` in `conductor_stream_test.go`. Also removes the transient-429 changes from the router-for-me#4881 branch; that finding belongs on router-for-me#5130 (fix/quota-backoff-hint-floor).
Keep router-for-me#4881 TTFT attempt context. Apply router-for-me#5211 newUpstreamAttemptContext on each stream model attempt and unauthorized refresh retry.
There was a problem hiding this comment.
💡 Codex Review
CLIProxyAPI/sdk/cliproxy/auth/conductor_stream.go
Lines 278 to 282 in be85813
Sanitize post-bootstrap error payloads before forwarding
When an in-band provider error arrives after meaningful output has committed the stream, this branch sanitizes only the parsed streamErr; chunk.Payload remains unchanged and is subsequently forwarded to the downstream client. An error such as {"error":{"message":"Incorrect API key provided: sk-live-secret"}} therefore exposes the proxy's upstream credential to the caller. Fresh evidence beyond the earlier result-recording fix is that the post-bootstrap path passes the original payload through rewriteForceMappedStreamChunk; sanitize or reconstruct the error frame before emitting it.
ℹ️ 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".
Error-path stream chunks still forwarded raw credentials after the parsed *Error was sanitized. Apply the same recognition set to payloads, and match prose "API key provided" plus vendor prefixes, not only sk-.
P1-A (review body 5011224530 — no thread,
|
|
Pushed the split in-band error frame fix:
|
S3 Claude empty detector treated the failover stream as empty_completion. Parent router-for-me#4881 fixture already carries content_block_start text:ok.
1 participant
Summary
Providers occasionally return HTTP 200 with a semantically empty completion —
finish_reason: stop, zero content, zero tool calls, zero completion tokens (observed on Gemini AI Studio-path providers with long tool-call conversations). Today this counts as success: the client gets a useless empty reply, no retry happens, and the auth is never cooled down.This change treats a genuinely empty terminal completion as a retriable failure:
empty_completion.go): conservative predicates for OpenAI-style SSE, Claude, Gemini, and Interactions. Empty iff terminal AND zero non-whitespace content AND zero tool calls AND zero/absent completion tokens. A lone signature (thoughtSignature, Claudesignature/signature_delta, InteractionshasSignature()) is not content.conductor_stream.go): bootstrap detector, drain on terminal emptiness, first-chunk/establishment timeout. In-band provider errors are finalized at EOF (finish()).conductor_execution.go, Home, Antigravity credits): same judgment; empty is recorded only as failure (markEmptyCompletion).invalid_api_key/api_key_invalid/ "API key not valid" is credential-scoped, not a request fault.summarizeErrorForLogredactssk-…/ Bearer / labeled secrets.No selector, session affinity, pluginhost, HTTP handler, or registry-resume changes in this PR.
History
Previous head
239f5fb9also carried selector/affinity/pluginhost/registry/handler scope (~61 files, +15953). That volume was force-pushed out of #4881; it lands in a follow-up PR after this one (serialized with overlapping cooldown/selector work).Test plan
go build ./...go test -count=1 ./sdk/cliproxy/auth/— 1417 passed, includingTestExecuteSignatureOnlyRotatesAuth(HTTP 200, signature-only, 0 tokens → rotate)