Skip to content
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# 090 — Work-phase 3: pre-dispatch account selection

Doc `080` recorded kiro-lb as ahead on one axis that matters directly to the user's ask:
it picks an account *before* dispatch, while we only reordered the 429 recovery path. This
phase closes that gap. Branch `codex/kiro-pool-predispatch`, off merged `dev` `d82b3049d`.

## What changed

`preferredInitialAccount(config, provider)` answers "which account should open this turn".
The initial OAuth resolution in `src/server/responses/core.ts` consults it and, when it
names an account, resolves that account's snapshot instead of the active one.

It is a **preference, not a gate**. A null answer means "use the active account", and null
is returned for: rotation disabled, fewer than two accounts, no quota evidence anywhere on
the roster, every candidate cooled, or the ranking simply agreeing with the active account.
A provider with no per-account quota therefore behaves exactly as before.

## Five review rounds

An independent reviewer failed this four times before passing. Each finding was real, and
three of them were defects I would not have found by testing the happy path.

### Round 1 — three blockers

1. **Antigravity could pair B's bearer with A's project.** The ordinary path fills the CCA
project only when it is *empty* (`!route.provider.project`), so a preferred account
installed its own bearer beside the configured account's project — #2841 in its
original shape, at a site nobody had reason to look at.
2. **A quota-less provider could still be redirected.** Cooling the active account collapses
the eligible list to one candidate, and ranking a single candidate returns it unchanged.
That *looks* like a ranked answer while nothing was ever measured. Evidence is now
checked across the whole roster, before eligibility narrows anything.
3. **Two uncached credential-file reads per request.** `loadAuthStore` chmods the config
dir, chmods the secret, and re-parses the whole file on every call — the exact cost the
neighbouring `PRESENCE_CACHE_TTL_MS` comment exists to warn about.

### Round 2 — the fail-closed 401 was worse than the bug

My first Antigravity fix returned 401 when a preferred account had no project. But
Antigravity tolerates project discovery failing, so a project-less account is an ordinary
stored state: a *preference* had been given the power to break a request that would
otherwise have worked. It now falls back to the active account.

### Round 3 — a removed account became a 401

The roster is cached for two seconds, so an account can be deleted after being chosen.
Resolving it throws, and that throw reached the client as 401 while a healthy active
account sat unused. The reviewer reproduced it exactly. Resolution failures now drop the
stale roster and retry on the active account.

### Round 4 — the one a catch could not catch

The sharpest finding. An account newly flagged `needsReauth` **does not throw**: its
credential is still readable, so resolution succeeds and no error path fires. The request
would dispatch on an account already known to need a fresh login.

My first fix re-read the store to validate the winner — and reopened blocker 3, because the
steady state of this feature is a pool where one account consistently ranks higher, so
"validate only on redirect" is "validate on every request".

### Round 5 — atomic validation, then PASS

The check belongs where the store row is *already* being read.
`getAccountCredentialWithStatus` returns credential and `needsReauth` from one read, and
`requireUsableAccount` makes account-scoped resolution reject an unusable account from
inside it. Selection now performs no store read at all; the caller's existing fallback
handles the rejection. Zero added I/O on the redirect path, both stale classes closed.

## Verification

```text
bun x tsc --noEmit -> exit 0
bun run privacy:scan -> Privacy scan passed
bun test (11 files) -> 181 pass / 0 fail / 656 expect() calls
core-lab-boundary -> pass, no new src/lab/ reach
```

Tests worth naming, because each encodes a defect above: a redirecting selection with
`auth.json` deleted still answers (proves the cache); a reauth-flagged account resolves
plainly but rejects under `requireUsableAccount` (proves why a catch was insufficient); and
cooling the *active* account of a quota-less provider still returns null.

## Result

The "pre-request selection" row moves out of doc `080`'s "they are ahead" column. Two rows
remain there honestly: kiro-lb persists quota across restart, and it has a real operations
dashboard. Neither is in scope here.
13 changes: 13 additions & 0 deletions src/oauth/account-quota-rank.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,19 @@ export function rankAccountsByHeadroom(provider: string, ring: readonly string[]
.map(entry => entry.id);
}

/**
* Do we hold any measurement at all for these accounts?
*
* Ranking a single candidate is trivially the identity, which makes it useless as an
* evidence test: a caller that has already filtered its list down to one account would be
* told "ranked" when nothing was measured. Pre-dispatch selection asks this first so it
* can decline to act on a roster it knows nothing about.
*/
export function hasHeadroomEvidence(provider: string, ids: readonly string[]): boolean {
return ids.some(id =>
headroomOf(provider, id) !== null
|| (provider === "kiro" && getKiroAccountExhaustion(`${provider}\u0000${id}`) !== null));
Comment on lines +86 to +89

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 Badge Expire stale quota evidence before selecting

When a Kiro quota probe ages past ACCOUNT_QUOTA_TTL_MS, getKiroAccountExhaustion deliberately returns null, but getCachedProviderAccountQuota continues returning the percentage row because sweepExpiredProviderAccountQuotaRows is not registered in STATE_STORE_REGISTRATIONS. This check therefore keeps accepting stale evidence; for example, a previously exhausted 100%-used account becomes “measured healthy” once its exhaustion verdict expires and can beat an unknown active account, proactively sending later requests back to the exhausted account. Make the evidence/ranking path enforce the quota TTL before redirecting.

Useful? React with 👍 / 👎.

}
/**
* How long to cool an account that just 429'd, when we know its allowance is spent.
*
Expand Down
102 changes: 101 additions & 1 deletion src/oauth/generic-account-failover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
import { getAccountSet } from "./store";
import { getValidAccessSnapshotForAccount, type OAuthAccessSnapshot } from "./index";
import { exhaustedCooldownMs, rankAccountsByHeadroom } from "./account-quota-rank";
import { exhaustedCooldownMs, hasHeadroomEvidence, rankAccountsByHeadroom } from "./account-quota-rank";
import { parseRetryAfterMs } from "../combos/failover";
import { sweepExpiredOnWrite } from "../lib/state-store-sweeper";
import type { OcxConfig, OcxProviderConfig } from "../types";
Expand Down Expand Up @@ -61,12 +61,29 @@ interface PresenceEntry {
readAt: number;
}

/**
* Ordered roster plus the active id, for the pre-dispatch preference.
*
* Same reasoning as the presence cache: `getAccountSet` reads through `loadAuthStore`,
* which chmods and re-parses the whole credential file on every call. Selection needs the
* ORDER and the active id, which the presence count cannot supply, so it gets its own
* TTL-bounded row. Ids and an active pointer only — never a credential.
*/
interface RosterEntry {
ids: string[];
activeId: string | null;
readAt: number;
}

/** Process-local, like the Anthropic pool's: a restart is allowed to forget a cooldown. */
const health = new Map<string, AccountHealth>();

/** Provider -> recent eligible-account count. TTL-bounded; never holds credential material. */
const presence = new Map<string, PresenceEntry>();

/** Provider -> recently read roster. TTL-bounded; never holds credential material. */
const roster = new Map<string, RosterEntry>();

const healthKey = (provider: string, accountId: string) => `${provider}\u0000${accountId}`;

function isCooled(provider: string, accountId: string, now: number): boolean {
Expand Down Expand Up @@ -101,6 +118,24 @@ function eligibleAccountCount(providerName: string, now: number): number {
return eligible;
}

/**
* Roster ids and the active pointer, read at most once per TTL window.
*
* `needsReauth` accounts are excluded for the same reason the presence count excludes
* them: a revoked credential cannot serve the request we are about to send.
*/
function cachedRoster(providerName: string, now: number): { ids: string[]; activeId: string | null } {
const cached = roster.get(providerName);
if (cached && now >= cached.readAt && now - cached.readAt < PRESENCE_CACHE_TTL_MS) {
return { ids: cached.ids, activeId: cached.activeId };
}
const set = getAccountSet(providerName);
const ids = set ? set.accounts.filter(a => a.needsReauth !== true).map(a => a.id) : [];
const activeId = set?.activeAccountId ?? null;
roster.set(providerName, { ids, activeId, readAt: now });
return { ids, activeId };
}

/**
* Presence IS consent (#2568d).
*
Expand Down Expand Up @@ -184,6 +219,8 @@ export function rotateGenericOAuthAccountOn429(
// A rotation means the roster in use just changed; do not answer the next activation question
// from a count read before the failure.
presence.delete(providerName);
// Same for the selection roster: the next request must not pick from a pre-failure read.
roster.delete(providerName);
// Deterministic: start after the failed account so repeated 429s walk the roster instead of
// hammering whichever id happens to sort first. The ring is built BEFORE ranking — ranking
// the store's own order would change which account a quota-less provider rotates to.
Expand Down Expand Up @@ -211,6 +248,61 @@ export async function failoverAccountSnapshot(
return getValidAccessSnapshotForAccount(providerName, accountId);
}

/**
* Which account should serve the FIRST attempt of a request.
*
* Rotation only ever ran after a 429, so a turn still opened on whichever account happened
* to be active — including one a previous probe already measured as spent. That costs a
* full upstream round trip and one of three rotations to rediscover what the cache knew.
*
* Returns null whenever the ordinary active-account path should be used unchanged: no
* quorum, rotation disabled, a single account, or no quota evidence to act on. This is a
* preference, never a gate — a cooled or unmeasured account is still perfectly usable, so
* an empty answer means "carry on", not "refuse".
*/
export function preferredInitialAccount(
config: OcxConfig,
providerName: string,
now = Date.now(),
): string | null {
if (!isGenericOAuthFailoverEnabled(config, providerName)) return null;
// This runs on the initial resolution of EVERY request, and `loadAuthStore` has no
// cache: each call chmods the config dir, chmods the secret, reads the whole file and
// normalizes it (store.ts:136-151). So the store is consulted at most ONCE here, behind
// the same TTL the presence check uses, and never at all for a single-account provider.
const { ids: order, activeId: active } = cachedRoster(providerName, now);
if (order.length < 2) return null;

// Evidence is required BEFORE eligibility narrows the field. Without this, a provider
// with no quota data at all could still be redirected: cool the active account with a
// 429 and the eligible list collapses to one candidate, which any ranking returns
// unchanged — an answer that looks ranked but was never measured. The no-op guarantee
// for quota-less providers has to be checked on the full roster.
if (!hasHeadroomEvidence(providerName, order)) return null;

// Cooldowns are respected here, unlike in the presence count: this picks the account to
// send to right now, and one inside its 429 window is the single candidate we hold
// positive evidence against.
const eligible = order.filter(id => !isCooled(providerName, id, now));
if (eligible.length === 0) return null;

// Start the ring at the active account so an unranked outcome reproduces today's choice.
const start = active ? order.indexOf(active) : -1;
const ring = start >= 0 ? [...order.slice(start), ...order.slice(0, start)] : order;
const candidates = ring.filter(id => eligible.includes(id));
if (candidates.length === 0) return null;

const best = rankAccountsByHeadroom(providerName, candidates)[0] ?? null;
// Nothing to do when the ranking agrees with the account we would have used anyway.
//
// The roster may be up to PRESENCE_CACHE_TTL_MS old, so this answer is a PREFERENCE the
// caller must be able to abandon: it resolves the account with `requireUsableAccount`,
// which rejects a removed or reauth-flagged account inside the store read it was already
// performing, and falls back to the active account. Validating here instead would mean a
// second uncached read of the credential file on every redirected request.
return best && best !== active ? best : null;
}

/** Earliest remaining cooldown, for a client-facing Retry-After when every account is cooled. */
export function genericFailoverRetryAfterSeconds(providerName: string, now = Date.now()): number | null {
const set = getAccountSet(providerName);
Expand All @@ -224,14 +316,22 @@ export function genericFailoverRetryAfterSeconds(providerName: string, now = Dat
return earliest === null ? null : Math.max(1, Math.ceil((earliest - now) / 1000));
}

/** Test seam and manual-recovery hook. */
export function forgetGenericFailoverRoster(providerName: string): void {
roster.delete(providerName);
presence.delete(providerName);
}

/** Test seam and manual-recovery hook. */
export function clearGenericFailoverHealth(providerName?: string): void {
if (!providerName) {
health.clear();
presence.clear();
roster.clear();
return;
}
presence.delete(providerName);
roster.delete(providerName);
for (const key of [...health.keys()]) {
if (key.startsWith(`${providerName}\u0000`)) health.delete(key);
}
Expand Down
16 changes: 12 additions & 4 deletions src/oauth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types";
import { loadConfig, resolveEnvValue, saveConfig } from "../config";
import { maskEmail } from "../lib/privacy";
import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro";
import { getAccountCredential, getAccountSet, removeAccount, saveAccountCredential, saveCredential, setActiveAccount, getCredential, credentialGeneration, createOAuthRefreshIntentLock, mergeAccountCredential, markAccountNeedsReauthIfGeneration, readOAuthRefreshIntent, writeOAuthRefreshIntent, markOAuthRefreshIntentStaleOwner, clearOAuthRefreshIntent, normalizeAuthStoreBuffer, OAuthMutationBusyError } from "./store";
import { getAccountCredential, getAccountCredentialWithStatus, getAccountSet, removeAccount, saveAccountCredential, saveCredential, setActiveAccount, getCredential, credentialGeneration, createOAuthRefreshIntentLock, mergeAccountCredential, markAccountNeedsReauthIfGeneration, readOAuthRefreshIntent, writeOAuthRefreshIntent, markOAuthRefreshIntentStaleOwner, clearOAuthRefreshIntent, normalizeAuthStoreBuffer, OAuthMutationBusyError } from "./store";
import { loginXai, refreshXaiToken, XAI_LOCAL_CLI_DETACH_WARNING, XaiTokenRequestError } from "./xai";
import { ANTHROPIC_OAUTH_BETA, AnthropicTokenError, loginAnthropic, refreshAnthropicToken } from "./anthropic";
import { loginKimi, refreshKimiToken } from "./kimi";
Expand Down Expand Up @@ -436,11 +436,18 @@ async function resolveAccessSnapshotForAccount(
provider: string,
accountId: string,
rejectedGeneration?: string,
requireUsableAccount = false,
): Promise<OAuthAccessSnapshot> {
const def = OAUTH_PROVIDERS[provider];
if (!def) throw new UnsupportedOAuthProviderError(provider);
const cred = getAccountCredential(provider, accountId);
if (!cred) throw new OAuthLoginRequiredError(provider);
// One store read answers both questions. A caller that opts in gets the account REJECTED
// when it needs reauthentication, which a bare credential read cannot detect: a revoked
// account keeps a readable credential, so resolution would otherwise succeed and the
// request would dispatch on an account already known to need a fresh login.
const row = getAccountCredentialWithStatus(provider, accountId);
if (!row) throw new OAuthLoginRequiredError(provider);
if (requireUsableAccount && row.needsReauth) throw new OAuthLoginRequiredError(provider);
const cred = row.credential;
const current = accessSnapshot(provider, accountId, cred);
if (rejectedGeneration !== undefined && current.generation !== rejectedGeneration) return current;
if (rejectedGeneration === undefined && cred.expires > Date.now() + REFRESH_SKEW_MS) return current;
Expand Down Expand Up @@ -529,8 +536,9 @@ export async function getValidAccessTokenForAccount(provider: string, accountId:
export async function getValidAccessSnapshotForAccount(
provider: string,
accountId: string,
opts: { requireUsableAccount?: boolean } = {},
): Promise<OAuthAccessSnapshot> {
return resolveAccessSnapshotForAccount(provider, accountId);
return resolveAccessSnapshotForAccount(provider, accountId, undefined, opts.requireUsableAccount === true);
}

/** Terminal refresh failures (revoked/rotated-away grants) — retrying cannot succeed. */
Expand Down
16 changes: 16 additions & 0 deletions src/oauth/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,22 @@ export function getAccountCredential(provider: string, accountId: string): OAuth
return loadAuthStore()[provider]?.accounts.find(a => a.id === accountId)?.credential ?? null;
}

/**
* Credential plus the account's reauth flag, from ONE store read.
*
* A caller that checks `needsReauth` separately pays a second `loadAuthStore`, which
* chmods and re-parses the whole credential file. Returning both together lets an
* account-scoped resolver reject a revoked account without that extra read.
*/
export function getAccountCredentialWithStatus(
provider: string,
accountId: string,
): { credential: OAuthCredentials; needsReauth: boolean } | null {
const account = loadAuthStore()[provider]?.accounts.find(a => a.id === accountId);
if (!account?.credential) return null;
return { credential: account.credential, needsReauth: account.needsReauth === true };
}

/** Persist a refreshed credential for a SPECIFIC account without touching activeAccountId. */
export async function saveAccountCredential(
provider: string,
Expand Down
Loading
Loading