Skip to content

fix(routing): skip known-exhausted accounts at admission - #57

Merged
ualtinok merged 1 commit into
cortexkit:mainfrom
iceteaSA:fix/routing-skip-exhausted
Aug 10, 2026
Merged

fix(routing): skip known-exhausted accounts at admission#57
ualtinok merged 1 commit into
cortexkit:mainfrom
iceteaSA:fix/routing-skip-exhausted

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Stacked on #54 and #56 — the base merge (145c489) brings isQuotaExhausted (#54) and the getSidebarState(path) overload + snapshot-level quota.checkedAt (#56). Only the top commit (d5f1fdd) is this PR's change; merge those two first and this reduces to one commit.

Why

Rate-limit marks are per-process and in-memory. With several concurrent opencode processes and fallback-first routing pinned at an exhausted account, every process independently pays one doomed admission probe (admission-time usage_limit_reached → mark → retryable error → reroute) before learning what the machine-global sidebar file already knows: the account is at 100% with a reset days away. The operator sees each discovery as a visible retry/error flash.

What

Admission-time candidate selection now consults quota before probing:

  • Dual source with freshness precedence: in-memory QuotaManager peek vs the shared sidebar file row (compared by primary.checkedAt, then snapshot checkedAt, then entry checkedAt). The fresher source is selected — the file wins only when strictly newer, memory wins ties, and an empty in-memory cache (fresh process) defers to a valid file row. isQuotaExhausted (type-safe, fail-open) is applied only to the selected source.
  • Fallback filter: exhausted candidates are dropped after the existing killswitch/rate-limit filters; skipped accounts are never probed, so their backoff/mark state is untouched. Applied to both the proactive (fallback-first) gate and the reactive iterator via a shared memoized selection.
  • Exhausted primary: synthesizes the existing killswitch-style 429 (reason quota-exhausted, Retry-After from the account's own resetsAt) so the reroute happens without the doomed probe — only when a non-exhausted fallback survives.
  • Safety valves: unknown/missing/malformed/past-reset quota is never exhausted (fail-open); if filtering would remove the last admission path, the current wire-probe order is fully restored — a stale or corrupt file can never brick routing, the wire stays the final authority. Sidebar file read at most once per request, tolerant reader, no-throw.
  • Each skip logs on the quota channel: admission skip: exhausted account {accountId, source, resetsAt}.

Verification

  • RED-first: exhausted first fallback (expected client-alt, got work-alt), fresh-process file exhaustion, newer-exhausted-file vs stale-healthy-memory, exhausted primary reroute — all fail pre-fix; 7 fail-open characterizations proven non-vacuous by reverse-applying the src diff (they pass on reverted source, i.e. they pin non-interference).
  • Review (gemini-3.1-pro): APPROVE 0 must / 0 should — adversarial-file surface (block/steer/brick), empty-cache tie-break, filter ordering (no state mutation for skipped accounts), once-per-request memoization, proactive+reactive coverage, and reverse-apply RED all verified.
  • Gates: build ✓ · tsc ✓ · full suite green ✓ · biome clean.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Skip probes for accounts already known to be quota‑exhausted by consulting the freshest, identity‑matched quota from memory or the shared sidebar. When main is exhausted and a healthy fallback exists, short‑circuit with a precise 429 (quota-exhausted) using the earliest future reset.

  • New Features

    • Admission: choose the fresher source (memory vs getSidebarState, once per request) by window checkedAt → snapshot checkedAt → entry checkedAt; require identity match (mainAccountId/per‑fallback accountId); unstamped file rows fail open when the live identity is known; drop exhausted candidates; memoize for proactive/reactive paths; log skips; if filtering would remove the last path, keep the original probe order.
    • Exhausted primary: synthesize a 429 with Retry-After from the earliest exhausted window; only when the request is replayable and a non‑exhausted fallback survives.
    • Quota/state: bind snapshots to account identity in QuotaManager (peekFallbackForPolicy(id, identity?)); setFallback(..., identity) clears mid‑stream marks on re‑login; machine writes include checkedAt, mainAccountId, and per‑fallback accountId, merge quotas by freshness per account, and keep identity tied to the winning snapshot.
  • Bug Fixes

    • Exhaustion check considers both windows and returns the earliest future reset; fail‑open on unknown/malformed usage/reset or past‑reset; an absent window never blocks.
    • Prevent stale overwrites and cross‑identity leaks in setSidebarMachineState; a stale write can’t replace fresher on‑disk quota, and identity always follows the snapshot that wins the merge.
    • Only block main by quota when a healthy fallback remains and the request can be replayed; single tolerant getSidebarState read per request.

Written for commit 2c772a4. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 10 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/opencode/src/index.ts
Comment thread packages/opencode/src/core/refresh-all-quota.ts Outdated
Comment thread packages/opencode/src/index.ts Outdated
Comment thread packages/opencode/src/tests/integration.test.ts
@iceteaSA
iceteaSA force-pushed the fix/routing-skip-exhausted branch from d5f1fdd to 8dfde55 Compare July 22, 2026 10:03
@ualtinok

Copy link
Copy Markdown
Contributor

Thanks. The candidate-loop replay safety work is useful, but this stacked branch needs revision before merge:

  1. Rebase onto current main (v0.4.2) as a clean delta. Do not carry the older background-refresh/session-routing stack or discard authoritative reset propagation from the WebSocket work.
  2. Require main-account identity agreement before treating shared quota as fresh, and pass the identity when storing refreshed main quota. Do the equivalent token-bound check for fallbacks.
  3. Admission exhaustion currently inspects only the primary window. Either evaluate every present quota window, matching policy semantics, or document and test an intentional provider-specific primary-only contract.
  4. Harden cross-identity sidebar merge behavior with a two-writer account-switch test so an older writer cannot replace newer identity/quota state.
  5. Test the real request selector in index.ts, not only the TUI resolver. Also explicitly confirm the intended fail-open behavior when filtering would otherwise remove every route.

Please rerun the full gate on the rebased branch.

@iceteaSA
iceteaSA force-pushed the fix/routing-skip-exhausted branch 2 times, most recently from c03687c to 15d11bb Compare July 22, 2026 20:10
@ualtinok

Copy link
Copy Markdown
Contributor

Thanks for the revision. The clean rebase, main/sidebar identity handling, reset expiry, both-window evaluation, and production-selector coverage are now in place.

One blocker remains: fallback quota is not identity-bound across re-login. getFallback(id, token) invalidates a mismatched token, but persisted account.quota is immediately reseeded under the new token and admission reads it by internal ID without checking the live fallback accountId. A replacement identity can therefore inherit the prior identity's exhausted quota and then write that stale quota under the replacement identity.

Bind fallback cached and persisted quota to the stable fallback identity, then add production-loader regressions for fallback re-login identity isolation and secondary-only admission exhaustion.

@iceteaSA
iceteaSA force-pushed the fix/routing-skip-exhausted branch from 15d11bb to e8fe6d5 Compare July 23, 2026 05:55
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Fixed in e8fe6d5 — fallback quota is now identity-bound across re-login, mirroring the main-account pattern.

  • QuotaEntry.accountId records the ChatGPT identity a snapshot was captured under. setFallback(…, identity?) binds it; an unknown incoming identity preserves the prior binding (same as setMain).
  • peekFallbackForPolicy(id, identity?) returns null on a known-identity mismatch, fail-open when either side is unknown — so a replacement identity can no longer inherit the prior identity's exhausted quota.
  • Both seed paths (seedFallbacksFromAccounts, seedFallbackQuota) pass account.accountId, so the reseed under a new token carries the new identity, not the old quota's.
  • admissionQuotaDecision judges the sidebar file row's accountId against the live candidate; a mismatched row is treated as absent.
  • pushQuota derives the fallback identity from the token and buildSidebarMachineState stamps the quota's own identity, so the shared file never pairs a stale identity with new quota.

Production-loader regressions added: fallback re-login identity isolation on both the setFallback and seed paths, a stale-identity sidebar row treated as absent, and secondary-only admission exhaustion. Full gate green (750/0).

Note for cross-PR consistency: #56 has been converged onto this same entry-based identity mechanism, so the two branches now share a byte-identical identity surface (QuotaManager internals + sidebar stamping) — the eventual merge of both is conflict-free.

@ualtinok

ualtinok commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Thanks for the revision. The fallback re-login isolation is now wired through persisted quota, push, policy/admission reads, sidebar state, and regression coverage, so the prior stale-identity blocker is resolved.

This is now a merge candidate. Because the PR also adds substantial shared-file admission routing, I am putting the integrated behavior through an adversarial review before merging.

@ualtinok

Copy link
Copy Markdown
Contributor

I ran an adversarial pass over the integrated behavior, focused on whether the shared file can deny a request the wire would have served. The last-path safety valve holds up: with every account marked exhausted, retained.length === 0 makes quotaBlocksMain false and applyAdmissionQuotaSafety restores the original order, so a stale or corrupt file cannot brick routing. Corrupt input fails open at every layer. Two things need fixing before merge.

1. An unstamped file row is trusted for an account whose identity is known.

index.ts:1761-1763 admits the file when fileAccountId === undefined || fileAccountId === currentAccountId. The mismatch case is right, but the missing case is inverted: a row with no accountId is applied to whatever identity currently occupies that internal id. That is the same re-login class we just closed on the memory path — and the asymmetry runs the wrong way, since memory fails open on unknown identity while the file fails closed on unknown identity by trusting its exhaustion. Any pre-upgrade file, or a partially written row, resurrects the bug.

Require agreement when the live identity is known, and treat an unstamped row as absent rather than authoritative. Add a regression with a file row carrying quota but no accountId against a known live identity, asserting the account is still probed.

2. quotaBlocksMain is not gated on replayability.

index.ts:2343-2345 blocks main whenever the file says exhausted and any fallback is retained. But both fallback paths require a replayable request — proactive at index.ts:2297, reactive inside tryFallbackAccounts. So for a non-replayable request (non-POST, non-string body, or a path that is not /responses) with an exhausted main and a retained fallback, the synthetic 429 is returned, main is never probed, and no fallback can run. That is a hard denial with no wire check anywhere in the request.

Adding isReplayableRequest(requestInput, init) to that condition closes it. Please add the non-replayable regression.

Two notes, neither blocking:

  • The claim that the wire remains the final authority holds only when filtering would remove the last path. Whenever any fallback is retained on paper, a stale main exhaustion skips main entirely. I am fine with that tradeoff, but the comment at index.ts:2340-2342 should say so plainly.
  • quotaCheckedAt (index.ts:1727-1740) reads primary.checkedAt, then the snapshot stamp, then the entry — it never consults secondary.checkedAt. An account whose only fresh window is the secondary is judged on an older key than it should be.

Sidebar read latency on the hot path is acceptable as-is, since it is memoized once per request and failures fail open. Worth revisiting only if a hung filesystem shows up in practice, as readFile has no timeout.

Each process learned an account was exhausted only by spending a request
on it, because rate-limit marks live in per-process memory while the
shared sidebar file already knows. With several sessions running, every
one of them paid its own doomed probe.

Admission now consults the shared file alongside memory, skipping an
account whose quota is exhausted there and synthesizing a retryable 429
for an exhausted primary. Filtering never removes the last path: if it
would leave no candidate, the original order is restored and the wire
decides, so a stale or corrupt file cannot brick routing.

A file row is trusted only when its identity matches the live caller's.
An unstamped row - written by an older build, or caught mid-write - is
treated as absent rather than authoritative, since applying it would let
one account's exhaustion follow the internal slot onto whoever logs in
next. That is the re-login bug already closed on the memory path, and
the file must not reintroduce it. Unknown identity on either side still
fails open.

Blocking is also gated on the request being replayable. Both fallback
paths require replay, so a non-replayable request with an exhausted main
and a healthy fallback would have been denied outright with no wire
check anywhere in it.

Freshness now considers both windows. An account whose only fresh window
is the secondary was judged on the older primary timestamp.

The wire remains the authority only where filtering would strip the last
candidate. While any fallback survives on paper, a stale exhaustion does
skip main - the tradeoff that buys the saved probes.
@iceteaSA
iceteaSA force-pushed the fix/routing-skip-exhausted branch from 11264c1 to 2c772a4 Compare August 10, 2026 17:53
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Both blockers fixed in 2c772a4. 756 pass / 0 fail. Your framing on the first one was sharper than mine and changed the fix.

1. Unstamped row trusted for a known identity.

- (fileAccountId === undefined || fileAccountId === currentAccountId) &&
+ fileAccountId === currentAccountId &&

The asymmetry you named is the part I had missed: memory fails open on unknown identity while the file was failing closed by trusting its exhaustion. Same input, opposite defaults, and the file's default is the dangerous one because it denies rather than probes. An unstamped row is now treated as absent when the live identity is known; unknown identity on either side still fails open.

Regression added: a file row carrying quota but no accountId against a known live identity — the account is still probed.

2. quotaBlocksMain not gated on replayability. Correct, and it was a hard denial with no wire check anywhere in the request. isReplayableRequest(requestInput, init) added to the condition, with a non-replayable regression asserting main is probed rather than 429'd.

Both notes folded in.

The comment at index.ts:2340-2342 now states the tradeoff plainly rather than implying the wire always decides — the wire is authoritative only where filtering would strip the last candidate; while any fallback survives on paper, a stale main exhaustion does skip main. That is the cost of the saved probes and it should read that way.

quotaCheckedAt now consults secondary.checkedAt as well. No existing expectation changed as a result — the only secondary-window test short-circuits on memoryQuota === undefined, so the extra field is not the deciding factor there. Worth stating since a silent expectation change would have been the thing to look at.

On the sidebar read latency: agreed it is fine as-is, memoized per request and failing open. readFile having no timeout only matters on a hung filesystem, which would break considerably more than routing.

One process note, since it is the kind of thing worth surfacing rather than burying: the fix touched five existing test expectations. I checked the diff specifically for weakened assertions before accepting it — 100 lines added, zero removed, and the updates were inside the new tests rather than relaxations of old ones. Changing existing expectations to make a change pass is the failure mode I would want caught here, so I would rather say that I looked than have you assume it.

@ualtinok

Copy link
Copy Markdown
Contributor

Verified both fixes at 2c772a4c.

admissionQuotaDecision now requires fileAccountId === currentAccountId (index.ts:1769), so an unstamped row no longer misattributes its exhaustion to whatever identity holds the slot. quotaBlocksMain now carries isReplayableRequest (index.ts:2354), so a non-replayable request can no longer be denied on a path where no fallback could have run. Both regressions are present and target the right paths. 756 tests pass locally on the branch.

One residual, non-blocking: when the live identity is also unknown, undefined === undefined still admits an unstamped row. That is the both-sides-unknown case with no better signal available, and it fails toward trusting exhaustion rather than spending. Fine as-is; worth a comment if you touch that line again.

Ready to merge from my side.

@iceteaSA

Copy link
Copy Markdown
Contributor Author

Thanks — and agreed on the residual, with one clarification worth recording since it is the third time identity handling has been the subtle part of this file.

Both-sides-unknown admitting the row is deliberate. The asymmetry you found last round was that memory failed open while the file failed closed on the same signal; that is now aligned for the case that matters, where the live identity is known and the row is not. When neither side has an identity there is genuinely no discriminator, and the two options are: trust the exhaustion and skip a probe that may have been fine, or ignore it and spend a request that may be doomed. Skipping is the cheaper error, and it is self-correcting — the next response stamps an identity and the ambiguity disappears.

I will add the comment when that line is next touched rather than pushing a comment-only change to a branch you are ready to merge.

Ready on my side too.

@ualtinok
ualtinok merged commit 40b706a into cortexkit:main Aug 10, 2026
5 checks passed
ualtinok added a commit that referenced this pull request Aug 10, 2026
feat(routing): sticky-balanced placement, session reset, and cachekeep sustain

Rebased onto the revised #57 so the admission identity rule and the
replayability gate survive; freshestQuotaSnapshot keeps both windows'
timestamps as the freshness key.
@iceteaSA
iceteaSA deleted the fix/routing-skip-exhausted branch August 11, 2026 09:12
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