Skip to content

[v3.8.50] fix(api): retry Codex image generation by account - #8307

Closed
fenix007 wants to merge 20 commits into
cheaper-inference:release/v3.8.50from
fenix007:fix/codex-image-account-fallback
Closed

fenix007 wants to merge 20 commits into
cheaper-inference:release/v3.8.50from
fenix007:fix/codex-image-account-fallback

Conversation

@fenix007

@fenix007 fenix007 commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Pass the resolved image model into credential selection so Codex account exclusions and model lockouts are respected.
  • Retry the exact ChatGPT-account model-access 400 once on an eligible sibling account, with a short model-scoped lockout and per-attempt proxy resolution.
  • Attribute Codex image-generation success and error call logs to the selected provider connection/account.
  • Preserve the image API-key authentication and call-log API-key context introduced by fix(api): enforce image generation API key auth #8306 on every initial and fallback attempt.
  • Keep release-PR production builds reliable: trusted release runners use Turbopack, while memory-constrained GitHub-hosted runners use the documented webpack fallback.

Related Issues

  • Related to a production Codex image-generation incident where the same model succeeded or failed depending on the selected ChatGPT account.

Validation

  • Image route/auth/fallback suites: 29/29
  • Image handler and Adobe Firefly suites: 84/84
  • npm run typecheck:core
  • ESLint on all changed TypeScript files
  • npm run check:complexity-ratchets
  • npm run check:file-size
  • npm run check:docs-all
  • Workflow regression suite: 32/32
  • actionlint .github/workflows/quality.yml
  • Local production webpack build
  • git diff --check
  • GitHub Actions is green on the latest head

Tests Added

  • tests/unit/image-generation-codex-account-fallback.test.ts
    • skips accounts that exclude the requested Codex model
    • retries one eligible sibling for the exact ChatGPT-account entitlement error
    • applies a temporary model lockout so the failing account is skipped on the next request
    • does not retry ordinary 400 responses
    • stops after one sibling retry
    • records the selected account and connection in call logs

Current Release Integration

The branch merges the current release/v3.8.49 tip (3b515d90b), including merged #8306. The application diff remains limited to six files: the image route, image handler, extracted call-log result helper, focused fallback tests, changelog fragment, and one-line Stryker test registration. Historical release-base CI repair commits no longer appear in the PR diff.

Four additional CI-maintenance files make the new advisory production build reliable for fork PRs: quality.yml, its regression test, and the two release-quality docs. The final GitHub-hosted webpack build passed in 11 minutes after two unrelated Turbopack runs were terminated by hosted-runner shutdown signals.

Reviewer Notes

  • Retry classification is intentionally narrow: Codex only, status 400, and the exact upstream ChatGPT-account unsupported-model message for the requested model.
  • Retry is bounded to one sibling account. The first account receives a two-minute in-memory model-scope lockout.
  • Each attempt resolves its own connection proxy and writes its own connection/account-attributed call log while retaining the validated API-key context.
  • response_format behavior is unchanged.
  • The build fallback preserves the fork security boundary: fork code stays on GitHub-hosted runners and never executes on the trusted omni-release runner.

@fenix007
fenix007 requested a review from diegosouzapw as a code owner July 23, 2026 15:43
@fenix007

Copy link
Copy Markdown
Contributor Author

CI follow-up (2026-07-23): reproduced the red gates against the clean base commit release/v3.8.49@27f0ad0db and the PR head. Both fail identically on the stale open-sse/executors/muse-spark-web.ts error-helper allowlist, the same env/docs drift, and tests/unit/autoCombo/provider-family-combos.test.ts expecting the removed auggie catalog entry. All four unit shards also fail on neighboring PR #8306 against the same base. None of these paths is changed by #8307.

The image fallback patch remains focused and its route suite passes 21/21. I am not copying unrelated base-red repairs into this PR. #8306 overlaps one route hunk and should land first; after the release baseline repairs land, #8307 should be updated from the new base and CI rerun.

alexey.nazarov@softmg.ru and others added 13 commits July 23, 2026 20:29
…der maps

cheaper-inference#6533 guard: every registered catalog engine must have an aggressivenessOf()
rank above "off" and a non-generic expectedReductionFactor(), or floor-mode
escalation ranking silently treats it as inert.

The `codex-responses` engine (Responses Tool Output, stackPriority 12, added with
the 12-engine catalog) was never added to AGGRESSIVENESS / REDUCTION_FACTOR in
adaptiveCompression/ladder.ts, so aggressivenessOf() returned 0 (== off) and
expectedReductionFactor() fell back to the generic 0.9. Placed it between rtk (20)
and ionizer (25) with a conservative, lossless-first reduction factor of 0.85 — it
stays a single-mode engine, not part of the automatic DEFAULT_LADDER (like ionizer
/ relevance / omniglyph).

Validated: ladder-engine-maps-6533 now 3/3; the compression/adaptive ladder test
files stay green (24/0).
…cheaper-inference#8123 model realignment

Base-red slice 3. Two test files referenced antigravity model IDs that
cheaper-inference#8013 (align official clients / callable catalog) and cheaper-inference#8123 (discover live AGY
models) intentionally renamed/retired — the candidate builder and static catalog
are correct; the tests were stale.

- auto-combo-credentialed-model-pool: claude-sonnet-5 -> claude-sonnet-4-6 and the
  gemini-3.5-flash-{low,medium,high} tier -> gemini-3.6-flash-{low,medium,high}
  (verified against the live createVirtualAutoCombo candidate set); the exclusion
  wildcard and per-account transparency assertions are unchanged in intent.
- T31 static-catalog: gemini-3-pro-preview was retired by cheaper-inference#8013, so assert the
  current client-visible top flash tier (gemini-3.6-flash-high) plus its absence.

Test-only. Validated: auto-combo-credentialed-model-pool 4/4, the T31/T33/T34/T38
model-specs file 8/8.
… agy alias

Same cheaper-inference#8013 realignment as this PR: `getModelInfo` resolves the stored `agy/…` alias
target to its canonical provider id `antigravity` (ALIAS_TO_PROVIDER_ID). The stored
alias STRING stays `agy/gemini-pro-agent`; only the resolved `provider` is now
`antigravity`, not `agy`. Test updated to match. 6/0.
…hrowing ReferenceError

Base-red slice 4 (single root cause across the whole handleChat cluster).

src/sse/handlers/chat.ts:1273 references PROVIDER_BREAKER_FAILURE_STATUSES to decide
whether an all-rate-limited provider result should trip the provider breaker, but the
constant was only a FILE-LOCAL const in chatPredicates.ts (a refactor extracted it out
of chat.ts and never re-exported it). Every request that reached that branch threw
`ReferenceError: PROVIDER_BREAKER_FAILURE_STATUSES is not defined`, so the global
fallback and breaker-gate paths blew up — surfacing as "All models failed |
PROVIDER_BREAKER_FAILURE_STATUSES is not defined" and breaking the handleChat coverage
tests (combo-error passthrough, 503 for cooled-down/open-breaker, budget-error, model
cooldown, body-derived retry-after, non-JSON rate-limit bodies).

Fix: export the const from chatPredicates.ts and import it in chat.ts (one canonical
definition, restoring the pre-refactor behavior).

Validated: chat-route-coverage 15/0 (was 12/3), chat-cooldown-aware-retry 6/0,
chat-rate-limit-body-lock 2/0; breaker guards (7907, combo-breaker-429, openrouter-6842)
unchanged; typecheck:core clean.
…tes.ts

Same root cause as the chat.ts import fix in this PR: the const was extracted out
of chat.ts into chatPredicates.ts, so the nvidia-quota Phase-1 guard (which greps
the source for the `= new Set([...])` declaration to prove 429 was not added to the
breaker classification) must read chatPredicates.ts, not chat.ts. Now 13/0.
…e was added

Base-red slice 5. cheaper-inference#8034/cheaper-inference#8064 (per-model/endpoint compression exclusion filter)
appended the new `compression-exclusions` sidebar item to the END of the compression
group, after `compression-studio`. That broke the deliberate "Compression Studio is
the last item of the compression group" invariant asserted by two tests
(sidebar-engine-items "Studio must be last" and sidebar-visibility's exact-order array,
whose expected list places compression-studio immediately before cli-code).

Rather than weaken the invariant, moved `compression-exclusions` ABOVE
`compression-studio` in sections.ts (new pages go above the Studio showcase) and added
the new item to sidebar-visibility's expected order. Both tests, plus the other four
sidebar suites (reorg / back-compat / tools / engine-items), stay green.
… URL

Base-red slice 6, rebased onto the advanced release/v3.8.49 (91fd5f9). The oauth
grok-cli cheaper-inference#7610 guard was already fixed on the base by cheaper-inference#8027 (it reads the warning
from grokCliAuthJson.ts) — dropped from this slice to avoid a conflicting duplicate.
Remaining two, still red on the current base:

- i18n cheaper-inference#7258: the "focused repro" asserted zh-TW.json STILL carries raw __MISSING__:
  placeholders. That backlog was filled (the "no locale has a raw __MISSING__: leaf"
  invariant is the durable guard); retired the now-inverted repro.
- qianfan: Baidu renamed the product page (product/wenxinworkshop -> product-s/
  qianfan_home); updated the expected website URL.

Validated (clean env): i18n 4/0, qianfan 5/0; oauth-modal-grok 2/0 already green on base.
…s + eslint baseline

The release branch accumulated deterministic unit-test failures (fast-path red on
every open PR). These are the ones with a clear, surgical root cause:

1. cheaper-inference#6863 combo model-lockout — the cheaper-inference#7940/cheaper-inference#7980 "cap exactCooldownMs against
   maxCooldownMs" clamp was also clamping an AUTHORITATIVE parsed upstream quota
   reset (e.g. "Resets in 92h27m28s") down to maxCooldownMs, so an exhausted model
   was retried far too early. recordModelLockoutFailure now takes
   exactCooldownIsUpstreamReset — set by the combo callers when the exact cooldown
   is a real upstream reset — which exempts it from the cap. The cheaper-inference#7980 computed
   until-midnight cap is unchanged (flag absent → still capped).

2. cheaper-inference#5786 streaming claude←codex — stripInternalReasoningPlaceholder (cheaper-inference#8081/cheaper-inference#8162)
   unconditionally .trim()'d every value. On the per-delta streaming path this ate
   the meaningful edge spaces of each delta ("Hello, " + "world." + " Bye." glued to
   "Hello,world.Bye."). It now only collapses to "" when whitespace is all that
   remains after removing the placeholder, preserving real content verbatim.

3. SPAWN_CAPABLE_PREFIXES test — cheaper-inference#7892 added /api/vnc-session (11th spawn-capable
   prefix, spawns Docker) but the client-safe guard test still expected 10 and did
   not list it. Aligned to 11 + added the entry to the checklist.

4. ESLint baseline — cheaper-inference#8008/cheaper-inference#8062 merged new test files with no-explicit-any without
   refreshing the frozen suppressions, so "No new ESLint warnings" went red for the
   whole branch. Regenerated the two affected entries
   (combo-routing-engine.test.ts 269→271, oauth-refresh-connection-dedup-8059.test.ts +1).

Validated: the three failing tests now pass; the sibling guards they interact with
stay green (cheaper-inference#7980 exact-cooldown-cap 4/4, cheaper-inference#8162 placeholder suites 17+12+41,
account-fallback 77); typecheck:core clean; lint:json --max-warnings 0 exits 0.

NOTE: the release branch has ~20 further real base-red failures (compression-engine
catalog, handleChat fallback, provider candidate transparency, i18n, misc). Those are
tracked separately, one focused PR per root-cause cluster; this PR is the first slice.
@fenix007
fenix007 force-pushed the fix/codex-image-account-fallback branch from dfd35ea to 41582b9 Compare July 23, 2026 17:30
diegosouzapw added a commit to herjarsa/OmniRoute that referenced this pull request Jul 27, 2026
…ests into dedicated file

The Codex account-fallback tests from PR cheaper-inference#8477 (rebase of cheaper-inference#8307) push the
shared image-generation-route test file above the 800-line testCap from
config/quality/file-size-baseline.json. Extract the 5 new tests
(skips-excluded-model, retries-one-sibling, no-retry-ordinary-400,
stops-after-one-fallback, attributes-call-logs) plus their shared
fixtures into tests/unit/image-generation-route-codex-account-fallback.test.ts
to keep both files well under the cap. No production code touched;
all 21 tests still pass.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw diegosouzapw changed the title fix(api): retry Codex image generation by account [v3.8.50] fix(api): retry Codex image generation by account Jul 28, 2026
@diegosouzapw diegosouzapw added the deferred-v3.8.50 Adiada para o ciclo v3.8.50 (validacao VPS, refactor, ou escopo grande) label Jul 28, 2026
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.49 to release/v3.8.50 July 28, 2026 18:43
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Re-homed to release/v3.8.50: v3.8.49 entered its release freeze, so the branch now belongs to the release captain and development continues on the next cycle. Nothing is wrong with this PR — it just needed a live base. No action needed from you; CI will re-run against the new base.

@diegosouzapw diegosouzapw added merge-train-deferred PR ejetada do merge-train — triagem separada and removed merge-train-deferred PR ejetada do merge-train — triagem separada deferred-v3.8.50 Adiada para o ciclo v3.8.50 (validacao VPS, refactor, ou escopo grande) labels Aug 17, 2026
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for tracking down this production incident and writing the original repro/test — the account-fallback diagnosis and the exact upstream error signature are all yours.

Since this PR was opened, a general credential-fallback mechanism landed for image generation (executeImageWithCredentialFallback + the retryable opt-in, #10494) that already retries account failures for every image provider — your inline retry loop now collides with it (4 file conflicts against the current release tip). Rather than resolve those, I reimplemented the same fix through that existing mechanism, reusing your bug diagnosis and error classifier: #10838.

Closing this one in favor of #10838, which carries your co-authorship on the commit. Appreciate the original catch!

fenix007 pushed a commit to fenix007/OmniRoute that referenced this pull request Aug 20, 2026
check:complexity-ratchets has been red on stable since fork.3, which rebaselined
check:file-size for its ported fixes but left the two complexity ratchets behind.

Measured per tag with one ESLint walk each, so the drift is attributed rather
than assumed:

- cyclomatic + max-lines: 2056 baseline, but pristine v3.8.48 (fork.1, which
  carries only docker/CI commits) already measures 2058 — inherited from
  upstream, which does not run this gate on the fast-path PR->release. fork.2
  adds +1 (peekCodexSseTransientError, cheaper-inference#7570/cheaper-inference#8043) and fork.3 adds +1
  (recordProviderFailure, cheaper-inference#10116).
- cognitive: 890 baseline matches fork.1 exactly. fork.2 adds +3 (two functions
  in codex.ts plus imageCredentialRetry.ts, cheaper-inference#9231/cheaper-inference#8307) and fork.3 adds +2
  (accountFallback.ts cheaper-inference#10116, providerLimits.ts cheaper-inference#10534).

fork.4 measures the same 2060 / 895: the error-status patch set added zero
violations, verified by diffing the per-violation lists between the two trees.
No fork-owned code sits over either threshold — every flagged function comes
from an upstream commit upstream rebaselined on its own branch, so the ratchets
are moved to the measured values instead of refactoring ported code away from
its upstream shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.

2 participants