Skip to content

fix: two-pool quota model, account-add OAuth flow, quota refresh on open#8

Open
iceteaSA wants to merge 24 commits into
cortexkit:mainfrom
iceteaSA:fix/quota-two-pool
Open

fix: two-pool quota model, account-add OAuth flow, quota refresh on open#8
iceteaSA wants to merge 24 commits into
cortexkit:mainfrom
iceteaSA:fix/quota-two-pool

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #6. Four commits: two user-facing bug fixes and a quota-model correction backed by live API evidence.

The quota model was wrong — there are exactly two pools

We probed the quota API raw (fetchAvailableModels + retrieveUserQuota) on two live accounts and then ran controlled burns to watch which buckets move:

  • Burning only gemini-3.6-flash tokens dropped every gemini model's remainingFraction identically — gemini-3.1-pro included (0.9664 → 0.9571) — while claude and gpt-oss stayed untouched.
  • Burning only claude-sonnet-4-6 tokens dropped claude, opus, and gpt-oss together (1.0 → 0.9556) while gemini stayed put.

So the API has two quota pools per account: gemini (every gemini model, tab_* autocomplete included) and non-gemini (claude + gpt-oss). The per-model entries carry only {remainingFraction, resetTime, tokenType, modelId} — no window field. The previous claude / gemini-pro / gemini-flash / gpt-oss split rendered four bars where two of them could never differ and one was silently misattributed.

d7e7f81 collapses classification, aggregation, killswitch scoping, sidebar schema, and the TUI to the two real pools (Gm / NG gutter labels, resetTime countdowns, no invented window labels). Old multi-key sidebar snapshots are read tolerantly — unknown keys are ignored.

Quota snapshots are now identity-bound

Adding an account shifts array indices, and index-keyed quota attribution could paint account A's cached numbers onto account B. Each cached snapshot now carries a sha256(refreshToken)[:16] identity stamp, checked at projection: a known mismatch drops the snapshot, a missing stamp fails open (legacy snapshots keep rendering). This composes with the write-time refresh-token routing from 4ca327b — write-time routing puts quota on the right account, the read-time stamp catches stale attribution after pool mutations. Antigravity refresh tokens don't rotate (verified by refreshing repeatedly and comparing), which is what makes the token a safe identity key; the assumption is documented at each stamp site in case that ever changes.

Account dialog: add accounts from the TUI

38ac287 wires the previously stubbed "Add account…" action to a real two-phase OAuth flow (add-oauth-start returns the auth URL, the pasted code completes the exchange), with per-session pending state and locked storage persistence. The dialog also now lists existing accounts when opened (the open payload never populated them before).

Quota dialog refreshes on open

0141af6 makes /antigravity-quota trigger a forced quota refresh for all accounts when the dialog opens, and fixes the post-add sidebar write that cleared existing accounts' cached quota (the dialog previously showed "No accounts configured" until a manual refresh).

Verification

unit           1840 pass, 0 fail
e2e            28 pass, 0 fail, zero leaked temp roots
typecheck      clean
format + lint  clean, lint enforcing (0 warnings)
smoke          packed-tarball TUI install resolves

Rendering verified against a live OpenCode host. Raw quota probe dumps and burn-test data available if useful.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Switches quota to two pools (Gemini / Non‑Gemini) with weekly/5h windows, adds an “Add account…” OAuth flow, and refreshes quotas when the dialog opens. Also hardens quota attribution, project scoping, runtime reloads, and cooldown handling.

  • New Features

    • Windowed quotas: primary source is retrieveUserQuotaSummary; map gemini-*gemini and 3p-*non-gemini; derive remaining/reset from the most‑constrained window; tolerate unknown buckets; count models correctly; per‑endpoint failover; RUQS and Gemini‑CLI fetches run in parallel; fall back to the legacy summary on 403/failure.
    • Per‑window UI: persist windows in cached quota and project them to the sidebar; CLI/sidebar/TUI render 7d/5h rows per pool; collapsed row shows “Gm/NG” with the binding window and preserves reset times; tolerate legacy keys/windowless snapshots.
    • OAuth add‑account: two‑phase flow with pending state and label prompt; after persist, reload the live runtime so routes use the new account immediately; the quota dialog triggers a refresh on open.
  • Bug Fixes

    • Quota attribution: stamp cached snapshots with sha256(refreshToken)[:16] and preserve across save→load; drop cachedQuota on stamp mismatch; refresh writes re‑resolve the index by the captured token; centralize pool→sidebar projection and carry per‑window data and reset times through all writers.
    • Classification & routing: prioritize Claude/GPT‑OSS substrings over gemini so gemini-claude-* maps to non-gemini; ensure tab_* maps to gemini.
    • Accounts & runtime: removing a non‑current account keeps the same current index; clamp persisted index to 0 when removing the last current account; preserve per‑family active cursors and cooldowns on removal; sidebar refresher carries coolingDownUntil and now distinguishes an absent cooldown from an empty one; retire activeIndex() in favor of getActiveIndexByFamily; ManagedAccount stores record‑level projectId/managedProjectId; the auth loader’s fetch delegates to the live runtime and reloads are serialized to avoid races.
    • E2E mock server: distinguish 400 for malformed quota‑summary bodies from 403 for wrong managedProjectId.

Written for commit bfd519a. Summary will update on new commits.

Review in cubic

iceteaSA added 4 commits July 24, 2026 16:25
(cherry picked from commit 99fc9fbfceacabc4e06834cf8121ded69ed8ed02)
(cherry picked from commit d10b553882d37c2996627de447d933db65517da2)
…eal API

The Antigravity quota API exposes exactly two pools per account:
- gemini  — all Gemini models (pro, flash, every version, image, agent,
  AND tab_* autocomplete models) share one remainingFraction + resetTime.
- non-gemini — Claude (sonnet+opus) AND gpt-oss share the other.

This replaces the prior 4-key model (claude / gemini-pro / gemini-flash /
gpt-oss) with those two keys throughout the stack.

Core:
- QuotaGroup type collapsed to 'gemini' | 'non-gemini'
- ModelQuotaGroup in model-registry updated; getQuotaGroupForModel() now
  falls back to prefix matching (gemini*/tab_* → gemini, claude*/gpt-oss*
  → non-gemini)
- classifyQuotaGroup / resolveQuotaGroup / aggregateQuota updated
- ManagedAccount gains cachedQuotaAccountId (opaque refresh-token identity
  stamp) so a later projection can detect stale snapshot after index shift
- updateQuotaCache accepts expectedRefreshToken for concurrent-add safety
  and stamps the cached quota with the identity hash

OpenCode plugin:
- command-data: 2-key SUPPORTED_QUOTA_KEYS / QUOTA_GROUP_LABELS;
  toCommandAccountRow drops cachedQuota on identity mismatch (KNOWN
  mismatch → drop; UNKNOWN/no-stamp → fail open); refreshQuota persists
  cachedQuotaAccountId; writeSidebar pushes 2 keys
- Upstream 4ca327b's refresh-token-keyed identity plumbing (mutateLiveAndStorage,
  refresh-loop token-keyed persistence, accountIneligible guard) preserved
  and reconciled with the a13 identity-stamp mechanism
- SidebarState: SidebarQuotaKey → 'gemini' | 'non-gemini'; normalizeAccount
  iterates only the 2 new keys (tolerant read: legacy multi-key snapshots
  ignored, not crashed)
- killswitch: QUOTA_GROUP_BY_FAMILY + quotaGroupForModel mapped to 2 pools;
  tab_* models resolve to gemini pool
- TUI: QUOTA_LABELS ('Gm' / 'NG'), QUOTA_ORDER (gemini first), collapsed row
  picks gemini bar first; data-model comment updated
- auth-menu + quota-status formatters: 2-key layout
- oauth-methods: 2-group display in 'Check quotas' output

No invented windows: the API has no window field — resetTime countdowns only.

Coexistence with upstream 4ca327b:
Upstream rewired command-data's refresh loop and mutateLiveAndStorage to key
by refresh token (canonical identity) rather than array index, for safety
against concurrent OAuth adds. This rework keeps that mechanism intact and
adds the a13 cachedQuotaAccountId stamp on top: identity is computed once
(sha256(refreshToken)[:16]), stamped at persist time and in the live
AccountManager, and checked at projection time in toCommandAccountRow. One
coherent identity mechanism, not two competing ones.
Add two focused unit tests for the cachedQuotaAccountId identity-stamp
mechanism introduced in the 2-pool quota rework:

1. KNOWN mismatch: a cached quota stamped under account A's identity must
   NOT render on account B — the stale snapshot is dropped and the row
   shows empty quota.
2. UNKNOWN (no stamp): a legacy snapshot without any identity stamp must
   still render (fail open) — the stamp was added in a later version and
   its absence must not prevent a cold-start account from showing quota.

Also fix the AccountModelFamily doc comment which was wrongly updated
during the rework: it carries the ROUTING families ('claude' /
'gemini-flash' / 'gemini-pro'), not the quota-pool groups ('gemini' /
'non-gemini'). Restored to match transform/types.ts ModelFamily.

@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 34 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread packages/core/src/account-manager.ts
Comment thread packages/opencode/src/plugin/commands.ts
Comment thread packages/core/src/account-manager.ts
Comment thread packages/core/src/account-types.ts
Comment thread packages/opencode/src/sidebar-state.ts
Comment thread packages/opencode/src/tui/command-dialogs.test.tsx
Comment thread packages/opencode/src/tui/command-dialogs.test.tsx
Comment thread packages/opencode/src/plugin/account-command-oauth.ts
Comment thread packages/opencode/src/plugin/commands.test.ts
Comment thread packages/opencode/src/plugin/command-data.test.ts
The collapsed compact row now renders both quota pools on one line,
middle-dot separated, in fixed order Gm then NG:

  Gm: 70% · NG: 25% ●

When a pool has no cached data, its slot renders an em dash rather
than disappearing, so the two-pool shape stays stable:

  Gm: 4% · NG: — ●

The single-pool usedLabel() helper is removed; poolText() handles
per-pool formatting with the existing QUOTA_LABELS mapping.
PR8 Fix Pass added 4 commits July 24, 2026 19:00
- P1#1: AccountManager.loadFromDisk and buildStorageSnapshot now
  propagate cachedQuotaAccountId so the stamp survives a save->load
  roundtrip. The previous buildStorageSnapshot omission meant
  persisted snapshots looked legacy/unstamped after every debounced
  save, opening the door to cross-account misattribution when the
  stamp check fell back to fail-open.
- P1#3: triggerAsyncQuotaRefreshForAccount now captures the refresh
  token BEFORE awaiting quotaManager.refreshAccount, resolves the
  live index for that token on resolve, and passes the captured
  token as expectedRefreshToken so the write cannot land on a
  different account that shifted into the same slot while the
  refresh was in flight.
- P2#5: getQuotaGroupForModel, resolveQuotaGroup, classifyQuotaGroup,
  and quotaGroupForModel all now check Claude / GPT-OSS substrings
  BEFORE the gemini substring so a 'gemini-claude-*' alias (a Claude
  route exposed under a 'gemini-' namespace) attributes to the
  non-gemini pool rather than the gemini pool.
- P2#6: redactAccountForSidebar carries cachedQuotaAccountId and
  currentQuotaAccountId; when the snapshot stamp does not match the
  current account identity, the cachedQuota is dropped at projection
  rather than rendered onto the wrong account. All live quota->sidebar
  writers (quota wrapper, refreshSidebar, auth-loader install) now
  pass both stamps.
- Adds a save->loadFromDisk stamp roundtrip test, a
  remove-during-refresh race test, a gemini-claude classification
  test, three stamp-mismatch sidebar tests, and a contrasting
  non-gemini value in the killswitch family-max/model-scoping
  fixtures so a regression that ignores 'model' would no longer pass.
- Regenerates src/tui-compiled/sidebar-state.ts from the source.
- P1#2: accountOAuth now accepts an onAfterPersist hook the plugin
  entry wires to authLoader.reload, so a successful OAuth add
  refreshes the live AccountManager + fetch interceptor immediately
  instead of waiting for the next auth reload. Reuses the
  auth-loader install path (load -> replaceAccountRuntime -> fetch
  rebuild -> sidebar publish) rather than inventing a second reload
  path. authLoader exposes .reload alongside the callable loader
  shape the host contract requires (auth.auth.loader).
- P2#4: takePending now consumes the entry atomically with the
  take (deleted inside takePending BEFORE the async exchange starts)
  so two concurrent add-oauth-finish calls for the same session
  cannot both exchange the same auth code. The previous peek-then-
  finally-del pattern was the long-deferred 'Should' from a11.
- P2#7: accountOAuth.authorize / .exchange now go through
  dependencies.oauth.* (the same seam every other sub-factory uses)
  rather than the concrete authorizeAntigravity / exchangeAntigravity
  imports, so injected overrides (e2e harness, custom hosts) reach
  the OAuth add path.
- P2#10: account-command-oauth.finish separates the four error
  stages - callback parse, exchange, persist, listAccounts - so a
  persistence failure no longer reports as 'OAuth exchange failed
  due to a network error'. The post-persist listAccounts failure
  still reports success with an empty accounts payload (the
  on-disk write already landed).
- P3: commands.ts now imports the shared AccountCommandOAuthService
  type from account-command-oauth instead of declaring its own.
  Adds onAfterPersist invocation tests, the concurrent finish()
  race test, the persistence-vs-exchange error stage tests, and
  asserts exchange is NOT called when no pending session exists.
- P2#8: writeSidebar now preserves each pool's q.resetAt as an ISO
  resetTime string so the sidebar can render a reset countdown for
  the gemini + non-gemini pools. The previous toFraction only
  carried remainingPercent, so every dialog re-render dropped the
  freshest reset deadline.
- P2#9: mutateLiveAndStorage.remove now captures the live current
  account's refresh token per family BEFORE the storage mutation and
  resolves the post-removal storage index by token, so a
  non-current account removal persists the same current the live
  AccountManager.removeAccountByIndex preserves. The previous
  hardcoded activeIndex: 0 re-elected whichever account shifted
  into slot 0 on every restart.
- Pins both fixes with new tests: the refreshed-account stamp
  assertion (command-data.test.ts) and the non-current-removal
  remap test that asserts the persisted activeIndex follows the
  shift instead of resetting to 0.
- Regenerates src/tui-compiled/plugin/command-data.ts from source.
- accounts.test.ts 'model takes precedence over family' now uses a
  cross-family pair (claude family + gemini model, gemini family +
  claude/gpt-oss model) so a regression that drops the model-vs-
  family check would no longer pass.
- quota-status.test.ts gains a legacy/unknown-key fixture asserting
  that the 'claude', 'gemini-antigravity', and 'gemini-cli' keys
  left over from the 3/4-key model are dropped by the collapsed
  two-pool projection while the supported keys still render.
- command-dialogs.tsx empty OAuth code-prompt submit now toasts a
  validation message and reopens the prompt instead of silently
  dropping the user back to the account list.
- command-dialogs.test.tsx asserts the OAuth URL surfaces in the
  Copy-URL option's description so the user can paste the mocked
  authorize URL into a browser.
- Regenerates src/tui-compiled/tui/command-dialogs.tsx from source.
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — the persistence and concurrency findings were real and our own review had missed them. Pushed four commits (db2d762..50340de) addressing all P1/P2 and the in-scope P3s. Dispositions:

P1

  • Identity stamp lost on reload (account-manager.ts) — cachedQuotaAccountId now flows through both loadFromDisk and buildStorageSnapshot, and is declared on the persisted record. Without this the whole identity guard was a no-op after the debounced save — good catch. Added a save→reload roundtrip test (stamp survives; a tampered stamp is still dropped at projection).
  • Added account not in live runtime (commands.ts:574) — OAuth add now triggers the auth-loader reload path (same replaceAccountRuntime/createFetch/sidebar-publish as a normal load), so routing sees the new account without waiting for a plugin reload.
  • Cross-account cache on concurrent refresh (account-manager.ts) — the refresh token is captured before the await and re-resolved after; updateQuotaCache drops the result if the token's account is gone or moved.

P2

  • takePending now deletes the session entry before the async exchange (no double-exchange of one code).
  • Claude/gpt-oss aliases are matched before the broad gemini prefix, so gemini-claude-* attributes to non-gemini.
  • Sidebar projection carries the stamp and drops cachedQuota on mismatch; account-add wired through the dependencies.oauth seam; reset times preserved as ISO on the sidebar write; non-current-account removal persists the remapped current index instead of resetting to 0.
  • Killswitch tests now carry a contrasting non-gemini value so model-scoped evaluation is actually exercised.

P3 — folded in the coverage gaps (cross-family precedence test, legacy-key tolerance fixtures, OAuth URL/finish assertions, dead tab_ branch removed, shared OAuth service type, empty-code-prompt feedback). One skipped: the non-blocking refreshQuota test — the apply call is still asserted, but making it truly deferred needs harness work I judged out of scope for this pass; noted for follow-up.

Gates: 1854 unit / 0 fail, 28/0 e2e (zero leaked temp roots), typecheck + format + lint (enforcing) clean, TUI build in sync, pack smoke green.

@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 27 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/opencode/src/plugin/index.ts
Comment thread packages/opencode/src/plugin/command-data.ts Outdated
Comment thread packages/opencode/src/plugin/auth-loader.ts Outdated
Comment thread packages/opencode/src/plugin/auth-loader.ts
Comment thread packages/opencode/src/plugin/command-data.ts Outdated
Comment thread packages/opencode/src/sidebar-state.test.ts
Comment thread packages/opencode/src/plugin/account-command-oauth.ts
Comment thread packages/core/src/account-manager.test.ts
PR8 Fix Pass added 4 commits July 24, 2026 20:47
- Add QuotaWindowEntry to QuotaGroupSummary + AccountMetadataV3 cachedQuota type
- Add fetchQuotaSummary() calling v1internal:retrieveUserQuotaSummary
- Map RUQS groups to pools by bucketId prefix (gemini-* → gemini, 3p-* → non-gemini)
- Derive pool remainingFraction/resetTime from most-constrained window
- Swap primary quota fetch to RUQS; fall back to fetchAvailableModels on 403/failure
- Use managedProjectId from refresh token parts as primary; fall back to projectId
- Fix quota.test.ts event count for the added fallback fetch call
- AccountBlock iterates per-window entries, one QuotaRow per window
- Pool prefix (Gm/NG) on first window row; indented gutter labels for rest
- Gutter labels: 7d (weekly) / 5h (5h) matching the fleet's convention
- Collapsed row shows pool + binding-window label + used%
- SidebarQuotaEntry gains windows array (redacted)
- SidebarAccountRedactionInput.cachedQuota supports windows shape
- Tolerant read: pre-windows snapshots render a single QuotaRow as before
- Copy real Pro (weekly+5h) and Free (weekly-only) RUQS response dumps
- aggregateQuotaSummary tests: pool mapping, window order, most-constrained derivation
- fetchQuotaSummary tests: managedProjectId primary, 403→projectId fallback, missing ID
- Add quotaSummaryWindow fixture to mock-antigravity-server for e2e coverage
- normalizeQuota: deserialize windows array from on-disk record (was stripping it)
- QuotaRow label width: 3→5 chars to fit pool+gutter labels (Gm 7d, Gm 5h, etc.)
- AccountBlock: pool prefix on every window row (not just first)
- Adopt tui-windows-frames.test.tsx (reviewer's render harness, 4 frame tests)
- e2e rpc-tui-flow: enqueue quotaSummaryWindow fixture instead of legacy quotaSummary

@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 15 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/core/src/quota-manager.ts Outdated
Comment thread packages/opencode/src/plugin/quota.ts Outdated
Comment thread packages/core/src/quota-manager.ts
Comment thread packages/core/src/quota-manager.ts Outdated
Comment thread packages/e2e-tests/src/mock-antigravity-server.ts
Comment thread packages/core/src/quota-manager.test.ts
PR8 Fix Pass added 3 commits July 24, 2026 21:36
Centralize pool→sidebar projection in projectQuotaPoolForSidebar (sidebar-state.ts).
redactAccountForSidebar uses it for the canonical {remainingPercent, resetAt, windows}
conversion. toCommandAccountRow now carries windows from cachedQuota entries.
writeSidebar projects them through into the sidebar input shape.
Add 5 producer-seam tests: windowed→redacted, legacy tolerance, Free weekly-only,
full write→read round-trip, identity-mismatch drops entire cachedQuota.
Bare refresh tokens have no packed project IDs — the real managedProjectId
lives on the persisted account record, not in the refresh string. Fall back
to account.managedProjectId (mirroring the existing pattern at quota.ts:511).

Add regression test: fetchQuotaSummary posts managedProjectId from options.
e2e mock: quotaSummaryWindow fixture enforces 403 when the posted project
does not match the managedProjectId — catches exactly this class of bug.
rpc-tui-flow account now has distinct projectId vs managedProjectId.
…data

Add standalone projectId/managedProjectId fields to ManagedAccount,
populated from the authoritative stored account record during construction.
These survive bare-refresh-token rotations where parts.* may be lost.

getAccountsForQuotaCheck + buildStorageSnapshot now fall back to the
record-level fields via a.parts.* ?? a.*. updateFromAuth keeps both
parts.* and record-level fields in sync.

Regression tests: getAccountsForQuotaCheck returns managedProjectId from
bare-token accounts; save→reload round-trip preserves it.

@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.

1 issue found across 11 files (changes from recent commits).

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/e2e-tests/src/rpc-tui-flow.e2e.test.ts">

<violation number="1" location="packages/e2e-tests/src/rpc-tui-flow.e2e.test.ts:131">
P2: This fixture does not enforce the behavior described by the preceding comments: the test passes both when `project` is omitted and when the wrong project triggers the fallback. Adding an assertion for the windowed quota data (and making the mock reject a missing project when `managedProjectId` is configured) would make this regression test effective.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

kind: 'quotaSummary',
models: [{ id: 'gemini-3-flash', displayName: 'Gemini 3 Flash' }],
kind: 'quotaSummaryWindow',
managedProjectId: 'managed-rpc',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This fixture does not enforce the behavior described by the preceding comments: the test passes both when project is omitted and when the wrong project triggers the fallback. Adding an assertion for the windowed quota data (and making the mock reject a missing project when managedProjectId is configured) would make this regression test effective.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/e2e-tests/src/rpc-tui-flow.e2e.test.ts, line 131:

<comment>This fixture does not enforce the behavior described by the preceding comments: the test passes both when `project` is omitted and when the wrong project triggers the fallback. Adding an assertion for the windowed quota data (and making the mock reject a missing project when `managedProjectId` is configured) would make this regression test effective.</comment>

<file context>
@@ -122,8 +122,13 @@ describe('rpc / tui flow (e2e)', () => {
+      // exercises the fallback path instead — failing the window assertion.
       h.server.enqueue({
         kind: 'quotaSummaryWindow',
+        managedProjectId: 'managed-rpc',
         groups: [
           {
</file context>

Comment thread packages/core/src/quota-manager.test.ts Outdated
Comment thread packages/core/src/account-manager.test.ts
Comment thread packages/core/src/account-manager.test.ts
PR8 Fix Pass added 5 commits July 24, 2026 22:55
P1 cortexkit#1: returned fetch captured the current runtime's interceptor by
reference, so the host kept routing requests through the OLD
AccountManager after an OAuth-driven reload. Replace with a call-time
delegating wrapper that reads the live runtime on every call.

P1 cortexkit#4: AuthLoaderHandle.load was advertised on the type but never
attached to the callable. Add load so the contract matches the
documented public surface.

P1 cortexkit#5: installRuntime's prior-runtime dispose was fire-and-forget
(void), so two overlapping reloads could interleave teardown. Await
the dispose and serialize reloads through a shared promise chain so
each install waits for the previous one to settle.
…moval

P1 cortexkit#2: removeAccount() persisted a hardcoded 0 activeIndex when the
captured current token was the one removed. The live AccountManager
keeps the cursor at the removed slot, so use the clamped removed
index instead of falling back to 0 — restart-time reload now re-elects
the same account the live manager kept current.

P2 cortexkit#3: removeAccount() collapsed both per-family active cursors onto
the same captured token, silently unelecting whichever family the
captured token didn't belong to. Expose getActiveIndexByFamily on
the AccountManager + view, capture per-family tokens in the mutator,
and persist each family's remapped index independently.

P2 cortexkit#6: the sidebar refresher built dialog rows from the post-mutation
command data, which does not carry the running cooldown. A toggle /
remove / setCurrent action momentarily cleared any rate-limited
account's cooldown. Look up the live account's coolingDownUntil by
index and preserve it on the projection.
…tches

P2 cortexkit#7: fetchQuotaSummary used only options.endpoints[0] for every
project attempt, so a 5xx on the primary endpoint permanently lost
the request. Iterate the endpoint list per project attempt so a 500
or 429 on one endpoint falls through to the next — matches the
legacy fetchers' failover convention.

P2 cortexkit#8: aggregateQuotaSummary derived the pool from group.buckets[0]
unconditionally, silently dropping the whole group when an
unrecognized bucketId led the array. Use the first RECOGNIZED
bucket for pool derivation so the recognized windows are kept.

P2 cortexkit#9: modelCount parsed the raw description prose and counted
non-comma / non-colon tokens, so the 'Models within this group:'
prefix was counted as a model. Strip the prefix before splitting.

P2 cortexkit#10: the windowed summary fetch and the gemini-CLI quota fetch
ran sequentially — two 10s timeouts back-to-back, ~20s per account
on modal open. Run them concurrently via Promise.all.
P3 #11: the e2e mock's quotaSummaryWindow fixture dropped fixture
headers before writing the body. Apply them via the shared
applyHeaders seam so cache-bust and trace-header tests have a path.

P3 #12: the quota-manager 'back-compat' test loaded a windows-shaped
fixture rather than exercising a windows-less legacy shape through
the real read path. Rewire it through a hand-rolled
RetrieveUserQuotaSummaryResponse that omits per-window data.

P3 #13: account-manager 'remove-during-refresh' test never
attempted the quota write, so the expectedRefreshToken guard wasn't
exercised. Call updateQuotaCache(0, …, refreshTokenForA) after the
removal so the assertion covers the actual cross-account guard.

P3 #14: the sidebar-state 'half-missing stamp' test covered only
the cachedQuotaAccountId-only case. Add the symmetric
currentQuotaAccountId-only case so legacy snapshots missing the
persisted stamp don't silently drop quota.

P3 #15: account-command-oauth failure messages said 'Please try
again' even though the pending entry was consumed and the same code
could not be redeemed. Update the messages to 'start a new OAuth
flow' so the operator knows the OAuth session is gone.

P3 #16: tighten the account-command-oauth test assertions — pin
the failed-exchange response, pin the full persisted success object
(not just label), assert 'Work account' renders in the
add-oauth-finish dialog, and restore an explicit unknown-key legacy
fixture for the renderer.

The tui-compiled mirror of command-data.ts is regenerated by the
build:tui gate so the bundled TUI payload matches the source.
The fences test asserts the EXACT sequence of fetch starts so a
regression that bypasses the lifecycle drain guard surfaces. With
the new concurrent quota path (summary + gemini-cli run in
parallel), each refreshAccounts call now performs 2 fetches instead
of 1, so the expected list grows by 1.
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Rounds 2 and 3 addressed in 8bff3ec..7d31887 (plus c0ae5f2..1cbcfd6 earlier — the windowed quota source). Highlights:

The stale-fetch-closure P1 was the sharpest catch of the three rounds. The loader returned fetch: fetchRuntime!.fetch — a direct reference — so the host's captured closure survived reload() and an OAuth-added account stayed invisible to routing until a full auth reload. The loader now returns a call-time delegating wrapper ((input, init) => fetchRuntime!.fetch(input, init)), reloads are serialized, and the previous runtime's dispose is awaited; a regression test captures the returned fetch, reloads onto a different manager, and proves the captured reference routes through the new one.

Remove-account persistence: removing the current middle account now persists the clamped index instead of 0, and claude/gemini active identities are captured per family (new getActiveIndexByFamily seam) instead of collapsing both to one token. Account actions no longer erase live cooldowns from the sidebar.

Windowed quota fetch: endpoint failover now actually iterates the fallback list; pool derivation uses the first recognized bucket instead of dying on a junk first entry; the model count no longer counts the description prefix as a model; the summary and gemini-CLI fetches run concurrently (a slow summary endpoint no longer doubles the modal-open latency).

Also in this range: the quota source moved to retrieveUserQuotaSummary (weekly + 5h windows per pool, variable by plan tier — free accounts return weekly-only), keyed by the managed project id with 403 fallback to the legacy path, with the per-window bars rendered in the sidebar and quota dialog. Live-verified against both a Pro and a free account.

Test batch folded in (remove-during-refresh now exercises the write, back-compat runs the real read path, OAuth failure messages tell the truth about retry). 1888/0 unit, 28/0 e2e, all gates enforcing.

@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 19 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/opencode/src/plugin/command-data.ts Outdated
Comment thread packages/opencode/src/tui-compiled/plugin/command-data.ts
Comment thread packages/opencode/src/plugin/commands.ts Outdated

@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 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/e2e-tests/src/mock-antigravity-server.ts
- persist 0 (not -1) when removing the current last account, matching
  the core's buildStorageSnapshot clamp; auth-doctor treats negative
  activeIndex as corruption
- remove dead activeIndex() from CommandDataAccountManagerView interface
  and all adapters (replaced by getActiveIndexByFamily)
- carry coolingDownUntil on CommandAccountRow so the sidebar refresher
  uses the row's own cooldown instead of matching by unstable numeric
  index
- distinguish malformed quota-summary JSON bodies (400 INVALID_ARGUMENT)
  from the missing-project 403 PERMISSION_DENIED gate in the mock server
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Latest rounds addressed in cba814d/5602193.

Removal index divergence (P2) — real, and the interesting part is which layer is authoritative. AccountManager.removeAccount() sets an in-memory -1 sentinel when the removed account was the current last one, but buildStorageSnapshot clamps with Math.max(0, …), so the core never persists a negative; the loader's clampNonNegativeInt maps negatives back to 0, and auth-doctor reports activeIndex < 0 as storage corruption. The storage-mutation path now persists 0 for that case (first/middle removals keep their index), matching the snapshot contract rather than the sentinel. Tests cover all three positions and pin the >= 0 persisted contract with the reason.

Cooldown misattribution (P3)CommandAccountRow now carries coolingDownUntil directly instead of being cross-referenced by numeric index at refresh time, so a reload between projection and refresh can't paint the wrong account as rate-limited. Test reorders the pool mid-flight to prove it.

Dead activeIndex() (P3) — removed from the view interface and both implementations; no remaining callers. Stale comment deleted.

Mock parse failures (P3) — a malformed quota-summary body now returns 400 INVALID_ARGUMENT before the project gate, so a serialization bug in a test can't masquerade as an auth fallback. The missing/wrong-project 403 behavior is unchanged.

1891 unit / 42 e2e, gates clean.

@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 8 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/opencode/src/plugin/command-data.ts
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Valid, and a genuine hole in the previous fix — bfd519a.

Carrying coolingDownUntil on the row fixed misattribution for accounts that have a cooldown, but ?? can't distinguish "row owns the field and its value is undefined" (no cooldown) from "row predates the field" (legacy). So a no-cooldown account still fell through to the index lookup and could inherit a neighbour's cooldown after a renumber — the exact bug, still live for the empty case.

Now keyed on presence rather than value:

const liveCooldown =
  'coolingDownUntil' in entry ? entry.coolingDownUntil : liveByIndex.get(entry.index)

The regression test renumbers the pool between projection and refresh so a no-cooldown account lands on a cooling-down account's index; it fails on the pre-fix code. Legacy rows without the key still take the index fallback.

1893/0 unit, 42/0 e2e, gates clean.

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.

1 participant