fix(proxy): allow verified replay past dead owner route - #1604
fix(proxy): allow verified replay past dead owner route#1604jaekwonhong wants to merge 2 commits into
Conversation
|
@codex review |
![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: 58b474fe84
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| This behavior MUST cover raw HTTP/SSE, native Responses WebSocket, and the HTTP responses bridge. Before recording transient account backoff, the service MUST release response-create and stream leases held for the failed account. A request-scoped API-key reservation MUST remain singular across an internal pre-dispatch failover, MUST settle or release at the terminal request outcome before the account-health write, and MUST NOT be reacquired solely for the internal failover. If neither settlement nor fallback release can be confirmed, the service MUST leave the health write unapplied. HTTP-bridge startup cleanup MUST release only an unowned current request lifecycle, and each reservation lifecycle MUST drain only its own health writes after confirmed settlement or release. The confirmed failure MUST place the account at the existing bounded transient error-backoff floor, but MUST NOT pause, deactivate, rate-limit, or quota-penalize it. | ||
|
|
||
| The service MUST NOT replay a request when dispatch is unknown or when the request depends on hard previous-response, turn-state, uploaded-file, single-account, or other required account ownership. If no eligible replacement account exists, the service MUST preserve the original sanitized upstream-unavailable failure instead of replacing it with a generated `no_accounts` error. | ||
| When local continuity evidence verifies that a previous-response continuation contains a complete fresh replay which can be resent without its owner anchor, a confirmed pre-dispatch failure on that owner MUST remove the anchor and retry another eligible account. This exception MUST NOT apply when the request also depends on turn-state, uploaded-file, single-account, or other required account ownership, or after any output becomes downstream-visible. |
There was a problem hiding this comment.
Because this new MUST is in the same requirement that says confirmed pre-dispatch failover covers raw HTTP/SSE, native Responses WebSocket, and the HTTP bridge, it now promises verified-owner replay for those transports too. I checked the other transport paths: native WS still returns surface for confirmed pre-dispatch errors whenever require_preferred_account is true (app/modules/proxy/_service/websocket/mixin.py:3194-3195), and bridge startup still raises immediately for a required account (app/modules/proxy/_service/http_bridge/proxy_failover.py:52-53), so a verified fresh replay on either path still fails closed instead of removing the anchor and retrying. Please either implement the same anchor-clearing path there or narrow this spec to the HTTP/SSE path that changed.
AGENTS.md reference: AGENTS.md:L24-L27
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Closing this stacked PR because the repository owner has directed all further publication to XZ-Organization/Codex_Model-provider-selector_codex-lb only. |
1 participant
Summary
Addresses the current-head Codex P2 on #1542.
Validation
17 passed: verified fresh replay / confirmed proxy-connect failure / ambiguous-dispatch unit regressions1 passed: Responses integration failover regressionopenspec validate retry-account-proxy-connect-failures --strictpassedopenspec validate --specs --strict: 49 passedReview context
Addresses unresolved thread
PRRT_kwDOQ1HZ7s6WlLSJon #1542. This PR intentionally targets the feature branch so the fix can be absorbed without widening the original PR.