Skip to content

fix(resilience): retry Codex pre-output transport failures on the same account (#9708) - #10792

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
Prudhvivuda:fix/9708-codex-same-account-retry
Aug 20, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
Prudhvivuda:fix/9708-codex-same-account-retry

Conversation

@Prudhvivuda

@Prudhvivuda Prudhvivuda commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Retry a retryable pre-output transport failure (502/503/504/507, connection reset, retry-buffer overflow) once on the same account after a 2–3s jittered delay, without marking the connection unavailable on the first blip.
  • Keeps Codex session/account affinity so a brief proxy reset does not drop the warm prompt-cache partition.
  • When the last eligible account is only temporarily unavailable and siblings are quota-filtered, surface a mixed-cause 503 instead of 429 All codex accounts reached configured quota threshold.

Closes #9708

⚠️ base-red inherited: #9985

Test plan

  • node --import tsx/esm --test tests/unit/codex-same-account-transport-retry-9708.test.ts
  • node --import tsx/esm --test tests/unit/sse-auth.test.ts --test-name-pattern quota
  • Confirm a first 503/507 on the last eligible Codex account retries the same connection and succeeds
  • Confirm a second consecutive transport failure then cools the account and rotates
  • Confirm a quota-filtered pool plus one transport-cooled account does not return the all-quota 429

@diegosouzapw

Copy link
Copy Markdown
Owner

Reviewed and validated in an isolated worktree off release/v3.8.50.

Tests: codex-same-account-transport-retry-9708.test.ts — 7/7 pass, including the getProviderCredentials mixed-cause (quota + transport-cooldown) 503 case. Existing sse-auth.test.ts --test-name-pattern quota — 67/67 pass, no regression. eslint clean on all touched files; tsc --noEmit clean.

Circuit-breaker/cooldown interaction: traced this carefully since it touches the resilience layers described in AGENTS.md — no conflict. The same-account retry only exempts the first pre-output 502/503/504/507 blip; a second consecutive failure on the same connection falls through unchanged into the existing markAccountUnavailable() / breaker._onFailure() path, so provider-breaker and connection-cooldown accounting behave exactly as before for genuinely repeated failures. Nice, surgical addition.

Two things worth a quick look before/after merge, neither blocking:

  1. Scope: shouldRetrySameAccountTransport / isRetryablePreOutputTransportError and the new mixed-availability 503 branch in auth.ts aren't gated on provider === "codex" anywhere — as written this changes retry/error behavior for every provider, not just Codex, even though the title/issue/changelog all frame it as Codex-specific. Was that intentional (a general resilience improvement, generalized past the original issue), or should it be scoped to Codex? Either is fine, just want to make sure it's a deliberate call rather than an oversight, since it's a behavior change for the other 340 providers with no dedicated test coverage for them.
  2. hasEmittedOutput: sameAccountTransportRetry.ts documents and unit-tests a guard against retrying after output has already streamed to the client, but it's never passed from the real call site in chat.ts (confirmed via repo-wide grep). It's harmless today — result.success only goes false pre-stream in the current control flow (the STREAM_EARLY_EOF/stream_timeout branch that would hit mid-stream always exits before reaching this block) — but as written it's a safety param that looks load-bearing and isn't. Worth either wiring it up or dropping it from the signature so it doesn't mislead the next person touching this file.

Also noticed 3 unchecked boxes in the test plan (live-account 503/507 retry, second-failure rotation, mixed-pool 503) — the automated coverage looks solid to me as a substitute, but flagging in case you want to close the loop with a live check per the repo's bug-fix protocol.

Overall: solid, well-scoped fix mechanically. ⭐⭐⭐⭐

@diegosouzapw
diegosouzapw merged commit aa32d2e into diegosouzapw:release/v3.8.50 Aug 20, 2026
5 checks passed
diegosouzapw added a commit that referenced this pull request Aug 20, 2026
…fallback and combo hops

#10792 (#9708) added a same-account retry for retryable 502/503/504/507 transport
failures, applied uniformly inside handleSingleModelChat. Two other paths call
into the same function recursively/iteratively and each carries its own
documented single-call guarantee that the retry silently broke:

- Emergency fallback (#1731): exactly one hop to the free fallback model, no
  extra calls against an already-exhausted provider. The retry was doubling
  that call whenever the fallback model itself returned a transient-looking
  status.
- Combo routing: target-level fallback is the combo's own policy (next target,
  not same-account retry). The retry delayed that policy and could surface the
  wrong terminal status when a later combo/global-fallback hop threw.

Both regressions were already covered by existing tests in
chat-route-coverage.test.ts (asserting exact call counts / preserved status) —
confirmed red on the release tip before this fix, green after.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
fenix007 pushed a commit to fenix007/OmniRoute that referenced this pull request Aug 20, 2026
…e account (diegosouzapw#9708) (diegosouzapw#10792)

Merged via merge-train (release/v3.8.50, batch1 2026-08-20) — static gates (typecheck/file-size/complexity/cognitive/changelog) green on the combined tree; test:unit reds observed in the boarded run were verified pre-existing on the pure release tip (unrelated flake), not caused by this PR. Thanks for the contribution!

(cherry picked from commit aa32d2e)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…e account (diegosouzapw#9708) (diegosouzapw#10792)

Merged via merge-train (release/v3.8.50, batch1 2026-08-20) — static gates (typecheck/file-size/complexity/cognitive/changelog) green on the combined tree; test:unit reds observed in the boarded run were verified pre-existing on the pure release tip (unrelated flake), not caused by this PR. Thanks for the contribution!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…fallback and combo hops

diegosouzapw#10792 (diegosouzapw#9708) added a same-account retry for retryable 502/503/504/507 transport
failures, applied uniformly inside handleSingleModelChat. Two other paths call
into the same function recursively/iteratively and each carries its own
documented single-call guarantee that the retry silently broke:

- Emergency fallback (diegosouzapw#1731): exactly one hop to the free fallback model, no
  extra calls against an already-exhausted provider. The retry was doubling
  that call whenever the fallback model itself returned a transient-looking
  status.
- Combo routing: target-level fallback is the combo's own policy (next target,
  not same-account retry). The retry delayed that policy and could surface the
  wrong terminal status when a later combo/global-fallback hop threw.

Both regressions were already covered by existing tests in
chat-route-coverage.test.ts (asserting exact call counts / preserved status) —
confirmed red on the release tip before this fix, green after.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(resilience): retry transient Codex transport failures on the same account before fallback

2 participants