fix(routing): skip known-exhausted accounts at admission#57
Open
iceteaSA wants to merge 4 commits into
Open
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/core/refresh-all-quota.ts">
<violation number="1" location="packages/opencode/src/core/refresh-all-quota.ts:91">
P2: A future `checkedAt` suppresses polling until that future time plus the freshness window, so clock-skewed or malformed shared state can leave quotas unrefreshed indefinitely. Treat timestamps later than `now` as stale.</violation>
</file>
<file name="packages/opencode/src/index.ts">
<violation number="1" location="packages/opencode/src/index.ts:162">
P2: Overlapping plugin lifecycles can silently disable quota polling for the active loader: a newer loader replaces this singleton's callback, then an older loader's dispose stops it. Keep the refresh controller per `CodexAuthPlugin`/loader or add ownership before stopping it.</violation>
<violation number="2" location="packages/opencode/src/index.ts:2233">
P1: Re-login can incorrectly hard-block the new main account when the shared sidebar still contains a fresher exhausted quota for the previous account. Persist/compare the main account identity with the sidebar quota, or fail open on the file source when it cannot be tied to `mainAccountIdentity`.</violation>
</file>
<file name="packages/opencode/src/tests/integration.test.ts">
<violation number="1" location="packages/opencode/src/tests/integration.test.ts:3411">
P2: Helper `writeAdmissionSidebarState` writes sidebar state with `route: 'fallback-first'` hardcoded, but the test `admission quota skips a file-exhausted main without probing it` uses `mode: 'main-first'` in `seedAdmissionAccounts` and never overrides route in the sidebar state write. This means sidebar state says `fallback-first` while config says `main-first`, creating a mismatch that may mask whether the routing mode is correctly respected by the admission logic.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const sidebarState = await requestSidebarState() | ||
| const mainQuotaDecision = admissionQuotaDecision( | ||
| quotaManager.peekMainForPolicy(mainAccountIdentity), | ||
| sidebarState.main.quota, |
There was a problem hiding this comment.
P1: Re-login can incorrectly hard-block the new main account when the shared sidebar still contains a fresher exhausted quota for the previous account. Persist/compare the main account identity with the sidebar quota, or fail open on the file source when it cannot be tied to mainAccountIdentity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/index.ts, line 2233:
<comment>Re-login can incorrectly hard-block the new main account when the shared sidebar still contains a fresher exhausted quota for the previous account. Persist/compare the main account identity with the sidebar quota, or fail open on the file source when it cannot be tied to `mainAccountIdentity`.</comment>
<file context>
@@ -2044,6 +2222,36 @@ export async function CodexAuthPlugin(
+ const sidebarState = await requestSidebarState()
+ const mainQuotaDecision = admissionQuotaDecision(
+ quotaManager.peekMainForPolicy(mainAccountIdentity),
+ sidebarState.main.quota,
+ Date.now(),
+ )
</file context>
| (checkedAt) => | ||
| typeof checkedAt === 'number' && | ||
| Number.isFinite(checkedAt) && | ||
| deps.now() - checkedAt < freshnessMs, |
There was a problem hiding this comment.
P2: A future checkedAt suppresses polling until that future time plus the freshness window, so clock-skewed or malformed shared state can leave quotas unrefreshed indefinitely. Treat timestamps later than now as stale.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/core/refresh-all-quota.ts, line 91:
<comment>A future `checkedAt` suppresses polling until that future time plus the freshness window, so clock-skewed or malformed shared state can leave quotas unrefreshed indefinitely. Treat timestamps later than `now` as stale.</comment>
<file context>
@@ -65,53 +68,86 @@ export async function refreshAllQuota(
+ (checkedAt) =>
+ typeof checkedAt === 'number' &&
+ Number.isFinite(checkedAt) &&
+ deps.now() - checkedAt < freshnessMs,
+ )
</file context>
Suggested change
| deps.now() - checkedAt < freshnessMs, | |
| checkedAt <= deps.now() && deps.now() - checkedAt < freshnessMs, |
| const HANDLED_SENTINEL = '__OPENCODE_OPENAI_AUTH_COMMAND_HANDLED__' | ||
|
|
||
| let bootQuotaSeedStarted = false | ||
| const backgroundQuotaRefresh = new BackgroundQuotaRefresh() |
There was a problem hiding this comment.
P2: Overlapping plugin lifecycles can silently disable quota polling for the active loader: a newer loader replaces this singleton's callback, then an older loader's dispose stops it. Keep the refresh controller per CodexAuthPlugin/loader or add ownership before stopping it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/index.ts, line 162:
<comment>Overlapping plugin lifecycles can silently disable quota polling for the active loader: a newer loader replaces this singleton's callback, then an older loader's dispose stops it. Keep the refresh controller per `CodexAuthPlugin`/loader or add ownership before stopping it.</comment>
<file context>
@@ -152,6 +159,7 @@ const DEFAULT_MID_STREAM_RATE_LIMIT_RESET_MS = 60_000
const HANDLED_SENTINEL = '__OPENCODE_OPENAI_AUTH_COMMAND_HANDLED__'
let bootQuotaSeedStarted = false
+const backgroundQuotaRefresh = new BackgroundQuotaRefresh()
const logModels = createLogger('models')
let loggedCostRestoration = false
</file context>
| @@ -1,5 +1,11 @@ | |||
| import { afterEach, beforeEach, describe, expect, it, test } from 'bun:test' | |||
There was a problem hiding this comment.
P2: Helper writeAdmissionSidebarState writes sidebar state with route: 'fallback-first' hardcoded, but the test admission quota skips a file-exhausted main without probing it uses mode: 'main-first' in seedAdmissionAccounts and never overrides route in the sidebar state write. This means sidebar state says fallback-first while config says main-first, creating a mismatch that may mask whether the routing mode is correctly respected by the admission logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/integration.test.ts, line 3411:
<comment>Helper `writeAdmissionSidebarState` writes sidebar state with `route: 'fallback-first'` hardcoded, but the test `admission quota skips a file-exhausted main without probing it` uses `mode: 'main-first'` in `seedAdmissionAccounts` and never overrides route in the sidebar state write. This means sidebar state says `fallback-first` while config says `main-first`, creating a mismatch that may mask whether the routing mode is correctly respected by the admission logic.</comment>
<file context>
@@ -3213,6 +3393,357 @@ describe('integration: active fallback routing', () => {
+ now + 3600_000,
+ )
+ hooks = loaded.hooks
+ writeAdmissionSidebarState({
+ fallbackIds: ['work-alt', 'client-alt'],
+ fallbackQuotas: {
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Rate-limit marks are per-process and in-memory. With several concurrent opencode processes and
fallback-firstrouting pinned at an exhausted account, every process independently pays one doomed admission probe (admission-timeusage_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:
primary.checkedAt, then snapshotcheckedAt, then entrycheckedAt). 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-first) gate and the reactive iterator via a shared memoized selection.quota-exhausted,Retry-Afterfrom the account's ownresetsAt) so the reroute happens without the doomed probe — only when a non-exhausted fallback survives.admission skip: exhausted account {accountId, source, resetsAt}.Verification
client-alt, gotwork-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).Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Skip probing accounts that are already quota‑exhausted at admission by using the freshest of in‑memory and shared sidebar data. Adds a background quota refresh to keep snapshots current, reducing error flashes and wasted retries.
New Features
getSidebarState) and drops exhausted fallbacks; if main is exhausted, synthesize a 429 with Retry-After from its ownresetsAtand reroute when a healthy fallback exists.BackgroundQuotaRefresh,refreshQuotaInBackground) runs ~every 5m with jitter, respects backoff, skips snapshots fresher than 4m, and updates the sidebar only when quota changes.checkedAtto quota,isQuotaExhausted, avoid exhausted entries inresolveSessionSidebarRouting, and record routing for a parent session viax-parent-session-id.Bug Fixes
setSidebarMachineStatenow merges quota by freshness so newer on-disk snapshots aren’t clobbered.Written for commit d5f1fdd. Summary will update on new commits.