Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions devlog/_plan/260829_kiro_quota_pool/080_head_to_head_result.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,30 @@ AGPL-3.0 reference clone (commit `474df2b`). Behaviour was studied; no code was

Stating this plainly, because a comparison that only lists our wins is worthless.

1. **Pre-request selection.** kiro-lb picks an account *before* dispatch with a weighted
race (`kiro/account_manager.py:1183-1208`). Ours ranks only on the 429 recovery path
(`src/oauth/generic-account-failover.ts`), so the first request of a turn can still
land on a spent account. Deferred by design (doc `060`), not solved.
2. **Persistence across restart.** Its quota rows live in SQLite and seed routing at
1. **Persistence across restart.** Its quota rows live in SQLite and seed routing at
startup (`kiro/store.py:206-289`). Our caches are process-local, so a restart forgets
every measurement until the next probe.
3. **Operations dashboard.** Request-rate charts, per-model token panels, Prometheus
2. **Operations dashboard.** Request-rate charts, per-model token panels, Prometheus
export (`kiro/metrics.py`). We render quota bars and a CLI column.
4. **Account onboarding.** Device login for Builder ID, Google and GitHub straight from
3. **Account onboarding.** Device login for Builder ID, Google and GitHub straight from
the dashboard (`kiro/device_login.py`). Ours hands off to the Kiro CLI one account at
a time.

Item 1 is the one that matters most for the user's "pool 기반 자동 탑재" ask, and it is
the first follow-up work-phase.
**Closed since this was written:** pre-request selection. kiro-lb picks an account before
dispatch with a weighted race (`kiro/account_manager.py:1183-1208`) and this document
originally recorded that as their lead. Doc `090` implements it on our side
(`preferredInitialAccount`), and ours is model-agnostic but evidence-gated and
deterministic where theirs is a random race over stale-by-up-to-15-minutes headroom.

## Honest summary

On *correctness of the quota reading* and *safety of the pool machinery* we are ahead:
resource selection, overage semantics, trial balances, region validation, credential/route
pairing, and stale-state handling are each demonstrably stricter, with tests. On *routing
sophistication* kiro-lb is still ahead on the pre-dispatch axis, and on *operational
surface* it is ahead outright.
sophistication* the two are now comparable — both select before dispatch; ours refuses to
act without evidence, theirs always ranks. On *operational surface* kiro-lb is ahead
outright.

"Better than kiro-lb" is therefore true for the two things this unit set out to do —
display Kiro quota, and make the pool quota-aware — and not yet true as a blanket claim
about the whole gateway.
"Better than kiro-lb" is therefore true for what this unit set out to do — display Kiro
quota, and make the pool quota-aware in both directions — and not a blanket claim about the
whole gateway, which still has a dashboard and restart persistence we do not.
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.
59 changes: 59 additions & 0 deletions devlog/_plan/260829_kiro_quota_pool/100_quota_persistence.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# 100 — Work-phase 4: quota persistence across restart

Doc `080` listed three axes where kiro-lb was ahead. Doc `090` closed pre-request
selection. This closes the second: kiro-lb persists quota rows in SQLite and seeds
routing from them at startup (`kiro/store.py:206-289`), while our caches were
process-local — a restart forgot every measurement.

## Why it matters more now than it did before

Before pre-dispatch selection, forgetting quota only meant an empty dashboard until the
next probe. Now it means the pool opens its first turn after every restart with no idea
which account has room — precisely the blindness `090` exists to remove. Persistence is
what makes that feature survive a restart rather than warm up from scratch.

## Design

`src/providers/account-quota-disk.ts`, modelled directly on the Codex pool's own
snapshot (`src/codex/quota.ts`) rather than inventing a second shape:

- A single JSON file under `OPENCODEX_HOME`, written atomically, debounced 250ms.
- Keyed exactly like the in-memory cache, so hydration is a direct fill.
- Six-hour maximum age on load. A stale bar is still useful for ORDERING — a wrong
guess costs one 429 that rotation already handles — but a day-old reading of a
monthly window should not outrank a fresh probe.
- Percentages and reset timestamps only. No token, no email, no label; the account id
is the store's own opaque id, which already keys the in-memory cache.
- Corrupt, missing, or future-version files load as empty. A cache must never be able
to break startup.

Hydration is lazy and once-only, on the first cached read. `clearAccountQuotaCache()`
resets the hydration flag and cancels any pending write, so a cleared cache cannot be
re-seeded from the file it was just cleared of.

## Accept criteria

| # | Scenario | Observable proof |
| --- | --- | --- |
| 1 | Write then read in a fresh process | the percentage survives |
| 2 | Snapshot older than six hours | discarded, not loaded |
| 3 | Corrupt JSON | loads empty, does not throw |
| 4 | `version: 2` file | ignored |
| 5 | No file | not an error |
| 6 | Written file inspected | contains percentages; contains no token, email, ARN or secret |
| 7 | Five writes in a burst | one file write, last value wins |
| 8 | Cancelled write | no file created |

## Verification

```text
bun x tsc --noEmit -> exit 0
bun run privacy:scan -> Privacy scan passed
bun test (8 files) -> 208 pass / 0 fail / 634 expect() calls
```

## What remains kiro-lb's

One axis from doc `080`: the operations dashboard — request-rate charts, per-model token
panels, Prometheus export. That is a product surface, not pool machinery, and it is
outside this unit's objective.
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));
}
/**
* 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
Loading
Loading