Skip to content

fix(proxy): fail over dead account proxy routes - #1322

Closed
luawl wants to merge 1 commit into
Soju06:mainfrom
luawl:fix/1314-account-bound-proxy-failover
Closed

fix(proxy): fail over dead account proxy routes#1322
luawl wants to merge 1 commit into
Soju06:mainfrom
luawl:fix/1314-account-bound-proxy-failover

Conversation

@luawl

@luawl luawl commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix account-bound proxy connection failures that occurred before upstream dispatch so movable Responses requests can fail over transparently instead of repeatedly selecting the dead route. The change keeps ambiguous POST failures and hard account ownership fail-closed.

Type of change

  • fix: - bug fix (no behavior change beyond the bug)
  • feat: - new user-facing feature or capability
  • refactor: - internal refactor (no behavior change, no API change)
  • docs: - documentation only
  • chore: / ci: / build: - tooling, CI, packaging
  • test: - test-only change
  • Breaking change

Linked issue: Fixes #1314

OpenSpec

  • This PR includes / updates an OpenSpec change
  • Not applicable - bug fix that matches the existing spec
  • Not applicable - docs / CI / chore only
  • This PR touches a codex-faithful path and preserves upstream-equivalent behavior

Change directory: openspec/changes/retry-account-proxy-connect-failures/

Changes

  • Add sanitized unknown / not_dispatched provenance to routed transport failures. Only explicit HTTP or SOCKS proxy-connect failures authorize a non-idempotent same-pool fallback.
  • Retry a movable request on another eligible account across raw HTTP/SSE, native Responses WebSocket, and HTTP bridge startup after every same-pool endpoint fails pre-dispatch.
  • Release response-create and stream leases before applying the existing bounded transient backoff floor (30 seconds, capped at 300 seconds), while retaining one request-scoped API-key reservation.
  • Preserve the original sanitized 502 when no replacement exists. Previous-response, turn-state, uploaded-file, required-account, and single-account contracts remain fail-closed.
  • Surface ambiguous POST dispatch outcomes without replaying them.
  • Keep the HTTP bridge architecture ratchet green by extracting the small failover state helper instead of increasing the 2,400-line mixin limit.

Diff composition

The headline diff is test/spec heavy:

  • production: 314 additions / 45 deletions across 11 files (269 net lines)
  • regression tests: 1,055 additions / 4 deletions across 9 files
  • OpenSpec: 146 additions

Test plan

pytest tests/unit/test_proxy_utils.py tests/unit/test_codex_client.py tests/unit/test_codex_upstream_paths.py tests/unit/test_load_balancer_concurrency.py tests/unit/test_proxy_websocket_client.py ...
778 passed, 4 deselected

pytest tests/unit/test_proxy_http_bridge.py -q
269 passed

pytest tests/integration/test_proxy_responses.py tests/integration/test_proxy_websocket_responses.py tests/integration/test_http_responses_bridge.py -q
213 passed

focused proxy-connect / dispatch-provenance matrix
18 passed

ruff check + ruff format --check
passed

ty check
passed

python scripts/check_proxy_architecture.py
passed

openspec validate retry-account-proxy-connect-failures --strict
passed

The four deselected tests are unrelated Windows environment-proxy precedence cases; the new core/service regressions and all affected endpoint suites ran.

Checklist

  • Title is in Conventional Commits format (<type>(<scope>)?: <subject>).
  • Linked the related issue / discussion above.
  • Added or updated tests covering the change.
  • Ran the relevant local CI subsets.
  • Strict OpenSpec validation passes and the task checklist is complete.
  • CHANGELOG is not edited by hand.

@Soju06

Soju06 commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Triage note: this PR is conflicting with main (proxy failover code moved with #1296/#1324 landing). Please rebase onto current main — happy to re-review/trigger Codex once it's green. If you'd rather not carry it forward, let us know and we'll close it for now.

@mereyabdenbekuly-ctrl

Copy link
Copy Markdown
Contributor

Independent production corroboration from a codex-lb 1.21.0 deployment:

  • We observed account-bound proxy connection/transport failures before any upstream response headers or Responses event were received.
  • Without account exclusion/failover, retries could select the same account-bound dead route again, surfacing as repeated reconnects / stream disconnected before completion to clients.
  • Our production mitigation only retries when the failure is known pre-dispatch, releases the failed account's leases before the next selection, and keeps previous-response/file/required-account continuity fail-closed.
  • That behavior stopped the repeated dead-route loop in the affected traffic.

This matches the safety boundary and intent of this PR. I am not opening a competing failover PR; sharing the deployment evidence here for the rebase/review. No account identifiers, proxy addresses, or credentials are included.

@Komzpa Komzpa added the needs rebase Needs rebase or conflict repair against current main label Jul 22, 2026
@Soju06

Soju06 commented Jul 24, 2026

Copy link
Copy Markdown
Owner

We still want this fix for #1314 — excluding dead account-bound proxy routes from retry selection is the right behavior, and the production corroboration above supports it. The branch is CONFLICTING against current main (head f740562, the failover path moved with #1296/#1324). Please rebase onto current main; merge gates (CI rollup, review threads, mergeable state) will run right after.

@Soju06

Soju06 commented Jul 27, 2026

Copy link
Copy Markdown
Owner

There's been no activity here since the PR was opened on 07-14, and both rebase requests (07-16 and 07-24) have gone unanswered, while the underlying bug (#1314) remains one we want fixed — the independent production corroboration above reinforces that. The conflict surface has also grown: #1492 (Live Voice sideband) substantially reworked app/core/clients/proxy_websocket.py, proxy.py, and codex.py, exactly where this PR plumbs the connect-failure provenance, so a mechanical rebase is no longer realistic.

Plan: we'll adopt the design from this PR — sanitized unknown / not_dispatched provenance, proxy-connect-only authorization for non-idempotent same-pool fallback, and lease release before the bounded backoff floor — into a maintainer-owned branch rebuilt on current main, keeping your commits and attribution wherever they survive the rework. If you'd rather carry it forward yourself, reply within the next couple of days and it's yours.

@Soju06

Soju06 commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Maintainer takeover is now live as #1542 per the earlier notice — your commit is preserved there with original authorship, rebuilt against current main's dispatch-provenance layer (which absorbed part of this PR's design since it was opened). This PR will be closed when #1542 merges; if you're back and want to pick it up, comment there.

@luawl luawl closed this by deleting the head repository Aug 2, 2026
mereyabdenbekuly-ctrl pushed a commit to mereyabdenbekuly-ctrl/codex-lb that referenced this pull request Aug 3, 2026
An account-bound upstream proxy can stop accepting connections while the
account stays administratively active. Movable Responses requests selected
onto that account then failed with a terminal sanitized 502 (or a bridge
startup error) instead of failing over, and client retries could select the
same dead route again (Soju06#1314).

Rebuilt from PR Soju06#1322 onto current main, which already carries the sanitized
pre-dispatch provenance (`retryable_same_contract` + `failure_phase ==
"connect"`) this change originally introduced as a dispatch-state enum:

- `is_confirmed_pre_dispatch_transport_error` is the single predicate that
  authorizes cross-account replay; host-wide network loss keeps its
  account-neutral process recovery path and TLS verification failures stay
  non-replayable.
- A confirmed pre-dispatch connect failure may try the next endpoint in the
  same resolved proxy pool even for a non-idempotent POST.
- Raw HTTP/SSE streaming, native Responses WebSocket connects, and HTTP
  bridge session startup release the failed account's stream lease, record
  the bounded transient backoff floor (`record_error_backoff`, shared
  `ERROR_BACKOFF_THRESHOLD`), exclude the account, and retry another
  eligible account within the existing attempt and deadline budgets.
- Hard previous-response/turn-state/file/single-account ownership fails
  closed on the original sanitized failure without crossing accounts, and
  selection exhaustion preserves that failure instead of generating
  `no_accounts`.
- The API-key reservation stays request-scoped and singular across internal
  failover; ambiguous POST dispatch outcomes are surfaced without replay.

Fixes Soju06#1314
Supersedes Soju06#1322

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mereyabdenbekuly-ctrl pushed a commit to mereyabdenbekuly-ctrl/codex-lb that referenced this pull request Aug 3, 2026
…lover

Maintainer follow-up to the Soju06#1322 takeover: pin the negative boundaries of
the confirmed pre-dispatch predicate at the externally observable surfaces.

- TLS verification connect failures do not authorize same-pool POST
  fallback or cross-account websocket replay.
- Host-wide network loss (proxy_network_unavailable) and post-dispatch
  body-read failures never classify as confirmed pre-dispatch.
- An idle bridge disconnect does not exclude the account or record the
  transient backoff floor, so healthy accounts stay healthy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 pushed a commit that referenced this pull request Aug 4, 2026
An account-bound upstream proxy can stop accepting connections while the
account stays administratively active. Movable Responses requests selected
onto that account then failed with a terminal sanitized 502 (or a bridge
startup error) instead of failing over, and client retries could select the
same dead route again (#1314).

Rebuilt from PR #1322 onto current main, which already carries the sanitized
pre-dispatch provenance (`retryable_same_contract` + `failure_phase ==
"connect"`) this change originally introduced as a dispatch-state enum:

- `is_confirmed_pre_dispatch_transport_error` is the single predicate that
  authorizes cross-account replay; host-wide network loss keeps its
  account-neutral process recovery path and TLS verification failures stay
  non-replayable.
- A confirmed pre-dispatch connect failure may try the next endpoint in the
  same resolved proxy pool even for a non-idempotent POST.
- Raw HTTP/SSE streaming, native Responses WebSocket connects, and HTTP
  bridge session startup release the failed account's stream lease, record
  the bounded transient backoff floor (`record_error_backoff`, shared
  `ERROR_BACKOFF_THRESHOLD`), exclude the account, and retry another
  eligible account within the existing attempt and deadline budgets.
- Hard previous-response/turn-state/file/single-account ownership fails
  closed on the original sanitized failure without crossing accounts, and
  selection exhaustion preserves that failure instead of generating
  `no_accounts`.
- The API-key reservation stays request-scoped and singular across internal
  failover; ambiguous POST dispatch outcomes are surfaced without replay.

Fixes #1314
Supersedes #1322

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 added a commit that referenced this pull request Aug 4, 2026
…lover

Maintainer follow-up to the #1322 takeover: pin the negative boundaries of
the confirmed pre-dispatch predicate at the externally observable surfaces.

- TLS verification connect failures do not authorize same-pool POST
  fallback or cross-account websocket replay.
- Host-wide network loss (proxy_network_unavailable) and post-dispatch
  body-read failures never classify as confirmed pre-dispatch.
- An idle bridge disconnect does not exclude the account or record the
  transient backoff floor, so healthy accounts stay healthy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 pushed a commit that referenced this pull request Aug 4, 2026
An account-bound upstream proxy can stop accepting connections while the
account stays administratively active. Movable Responses requests selected
onto that account then failed with a terminal sanitized 502 (or a bridge
startup error) instead of failing over, and client retries could select the
same dead route again (#1314).

Rebuilt from PR #1322 onto current main, which already carries the sanitized
pre-dispatch provenance (`retryable_same_contract` + `failure_phase ==
"connect"`) this change originally introduced as a dispatch-state enum:

- `is_confirmed_pre_dispatch_transport_error` is the single predicate that
  authorizes cross-account replay; host-wide network loss keeps its
  account-neutral process recovery path and TLS verification failures stay
  non-replayable.
- A confirmed pre-dispatch connect failure may try the next endpoint in the
  same resolved proxy pool even for a non-idempotent POST.
- Raw HTTP/SSE streaming, native Responses WebSocket connects, and HTTP
  bridge session startup release the failed account's stream lease, record
  the bounded transient backoff floor (`record_error_backoff`, shared
  `ERROR_BACKOFF_THRESHOLD`), exclude the account, and retry another
  eligible account within the existing attempt and deadline budgets.
- Hard previous-response/turn-state/file/single-account ownership fails
  closed on the original sanitized failure without crossing accounts, and
  selection exhaustion preserves that failure instead of generating
  `no_accounts`.
- The API-key reservation stays request-scoped and singular across internal
  failover; ambiguous POST dispatch outcomes are surfaced without replay.

Fixes #1314
Supersedes #1322

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 added a commit that referenced this pull request Aug 4, 2026
…lover

Maintainer follow-up to the #1322 takeover: pin the negative boundaries of
the confirmed pre-dispatch predicate at the externally observable surfaces.

- TLS verification connect failures do not authorize same-pool POST
  fallback or cross-account websocket replay.
- Host-wide network loss (proxy_network_unavailable) and post-dispatch
  body-read failures never classify as confirmed pre-dispatch.
- An idle bridge disconnect does not exclude the account or record the
  transient backoff floor, so healthy accounts stay healthy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 pushed a commit that referenced this pull request Aug 6, 2026
An account-bound upstream proxy can stop accepting connections while the
account stays administratively active. Movable Responses requests selected
onto that account then failed with a terminal sanitized 502 (or a bridge
startup error) instead of failing over, and client retries could select the
same dead route again (#1314).

Rebuilt from PR #1322 onto current main, which already carries the sanitized
pre-dispatch provenance (`retryable_same_contract` + `failure_phase ==
"connect"`) this change originally introduced as a dispatch-state enum:

- `is_confirmed_pre_dispatch_transport_error` is the single predicate that
  authorizes cross-account replay; host-wide network loss keeps its
  account-neutral process recovery path and TLS verification failures stay
  non-replayable.
- A confirmed pre-dispatch connect failure may try the next endpoint in the
  same resolved proxy pool even for a non-idempotent POST.
- Raw HTTP/SSE streaming, native Responses WebSocket connects, and HTTP
  bridge session startup release the failed account's stream lease, record
  the bounded transient backoff floor (`record_error_backoff`, shared
  `ERROR_BACKOFF_THRESHOLD`), exclude the account, and retry another
  eligible account within the existing attempt and deadline budgets.
- Hard previous-response/turn-state/file/single-account ownership fails
  closed on the original sanitized failure without crossing accounts, and
  selection exhaustion preserves that failure instead of generating
  `no_accounts`.
- The API-key reservation stays request-scoped and singular across internal
  failover; ambiguous POST dispatch outcomes are surfaced without replay.

Fixes #1314
Supersedes #1322

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 added a commit that referenced this pull request Aug 6, 2026
…lover

Maintainer follow-up to the #1322 takeover: pin the negative boundaries of
the confirmed pre-dispatch predicate at the externally observable surfaces.

- TLS verification connect failures do not authorize same-pool POST
  fallback or cross-account websocket replay.
- Host-wide network loss (proxy_network_unavailable) and post-dispatch
  body-read failures never classify as confirmed pre-dispatch.
- An idle bridge disconnect does not exclude the account or record the
  transient backoff floor, so healthy accounts stay healthy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 pushed a commit that referenced this pull request Aug 6, 2026
An account-bound upstream proxy can stop accepting connections while the
account stays administratively active. Movable Responses requests selected
onto that account then failed with a terminal sanitized 502 (or a bridge
startup error) instead of failing over, and client retries could select the
same dead route again (#1314).

Rebuilt from PR #1322 onto current main, which already carries the sanitized
pre-dispatch provenance (`retryable_same_contract` + `failure_phase ==
"connect"`) this change originally introduced as a dispatch-state enum:

- `is_confirmed_pre_dispatch_transport_error` is the single predicate that
  authorizes cross-account replay; host-wide network loss keeps its
  account-neutral process recovery path and TLS verification failures stay
  non-replayable.
- A confirmed pre-dispatch connect failure may try the next endpoint in the
  same resolved proxy pool even for a non-idempotent POST.
- Raw HTTP/SSE streaming, native Responses WebSocket connects, and HTTP
  bridge session startup release the failed account's stream lease, record
  the bounded transient backoff floor (`record_error_backoff`, shared
  `ERROR_BACKOFF_THRESHOLD`), exclude the account, and retry another
  eligible account within the existing attempt and deadline budgets.
- Hard previous-response/turn-state/file/single-account ownership fails
  closed on the original sanitized failure without crossing accounts, and
  selection exhaustion preserves that failure instead of generating
  `no_accounts`.
- The API-key reservation stays request-scoped and singular across internal
  failover; ambiguous POST dispatch outcomes are surfaced without replay.

Fixes #1314
Supersedes #1322

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 added a commit that referenced this pull request Aug 6, 2026
…lover

Maintainer follow-up to the #1322 takeover: pin the negative boundaries of
the confirmed pre-dispatch predicate at the externally observable surfaces.

- TLS verification connect failures do not authorize same-pool POST
  fallback or cross-account websocket replay.
- Host-wide network loss (proxy_network_unavailable) and post-dispatch
  body-read failures never classify as confirmed pre-dispatch.
- An idle bridge disconnect does not exclude the account or record the
  transient backoff floor, so healthy accounts stay healthy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Soju06 added a commit that referenced this pull request Aug 6, 2026
…tch (#1542)

* fix(proxy): fail over dead account proxy routes before upstream dispatch

An account-bound upstream proxy can stop accepting connections while the
account stays administratively active. Movable Responses requests selected
onto that account then failed with a terminal sanitized 502 (or a bridge
startup error) instead of failing over, and client retries could select the
same dead route again (#1314).

Rebuilt from PR #1322 onto current main, which already carries the sanitized
pre-dispatch provenance (`retryable_same_contract` + `failure_phase ==
"connect"`) this change originally introduced as a dispatch-state enum:

- `is_confirmed_pre_dispatch_transport_error` is the single predicate that
  authorizes cross-account replay; host-wide network loss keeps its
  account-neutral process recovery path and TLS verification failures stay
  non-replayable.
- A confirmed pre-dispatch connect failure may try the next endpoint in the
  same resolved proxy pool even for a non-idempotent POST.
- Raw HTTP/SSE streaming, native Responses WebSocket connects, and HTTP
  bridge session startup release the failed account's stream lease, record
  the bounded transient backoff floor (`record_error_backoff`, shared
  `ERROR_BACKOFF_THRESHOLD`), exclude the account, and retry another
  eligible account within the existing attempt and deadline budgets.
- Hard previous-response/turn-state/file/single-account ownership fails
  closed on the original sanitized failure without crossing accounts, and
  selection exhaustion preserves that failure instead of generating
  `no_accounts`.
- The API-key reservation stays request-scoped and singular across internal
  failover; ambiguous POST dispatch outcomes are surfaced without replay.

Fixes #1314
Supersedes #1322

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(proxy): guard replay-authorization boundaries for dead-route failover

Maintainer follow-up to the #1322 takeover: pin the negative boundaries of
the confirmed pre-dispatch predicate at the externally observable surfaces.

- TLS verification connect failures do not authorize same-pool POST
  fallback or cross-account websocket replay.
- Host-wide network loss (proxy_network_unavailable) and post-dispatch
  body-read failures never classify as confirmed pre-dispatch.
- An idle bridge disconnect does not exclude the account or record the
  transient backoff floor, so healthy accounts stay healthy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(proxy): settle keyed failover health writes safely

* fix(proxy): move verified fresh replay off dead pinned owner before pre-dispatch hard-ownership raise

A confirmed pre-dispatch connect failure guarantees zero upstream bytes,
so the confirmed dead-route branch can safely run the verified-owner
replay step (same file/turn-state/single-account guards as the
post-refresh path) ahead of the require_preferred_account fail-closed
raise, moving a locally verified full-resend to a fresh account instead
of erroring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: luawl <252236154+luawl@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: SSY <234955825+mereyabdenbekuly-ctrl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs rebase Needs rebase or conflict repair against current main

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: dead account-bound proxy does not fail over before visible output

4 participants