From 3a347f77e21cc864fecaff3c9f4169472a0868ef Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 03:39:41 +0900 Subject: [PATCH 1/4] fix(routing): treat a live full burst window as exhausted, not unknown Closes #3029. shortPercent survives quota parsing as a real blocking window, then computeCodexUsageScore throws it away when no long window is known. Unknown passes the headroom check and suppresses auto-switch, so an account whose five-hour window is full stays selected and the pool wedges on it - which is the conjunction the reporter measured. The existing comment is right that a short-only reading cannot stand in for a long one: a bare shortPercent: 0 would score a flat 0 and make an unverified account look like the emptiest in the pool. That argument does not extend to a full window. 100 is not an optimistic guess about an unobserved window, it is a direct observation that the account cannot serve a request right now. Freshness is the other half, and without it this fix inverts the bug. getAccountQuota performs no expiry check, partial updates carry the old short tuple forward, and disk hydration accepts a persisted reading for hours - so a terminal score must expire with its window or a recovered account stays excluded, which is #3029 pointed the other way. A reading with no shortResetAt cannot be aged and stays unknown: a wrongly-selected account fails one request, a wrongly-excluded one is invisible until someone reads the pool by hand. Both units reach storage - normalizeResetAt does not scale and the GUI disambiguates by magnitude at read time - so the comparison normalizes the same way. Read as milliseconds, a seconds value looks like it reset in 1970 and every terminal reading scores unknown: a fix that passes its own test and does nothing. The clock is threaded through all eight call sites rather than read from wall time. Two of them already had a now and dropped it, including subagent fallback, which reads the same score to decide whether a native model is exhausted - so a stale terminal reading pushed subagents off a live model too. That case is red when the threaded clock is replaced with Date.now(). --- src/codex/quota.ts | 6 +++ src/codex/routing.ts | 55 +++++++++++++++++++++++++-- src/codex/subagent-model-fallback.ts | 5 ++- tests/codex-routing.test.ts | 33 ++++++++++++++++ tests/subagent-model-fallback.test.ts | 32 +++++++++++++++- 5 files changed, 126 insertions(+), 5 deletions(-) diff --git a/src/codex/quota.ts b/src/codex/quota.ts index 3248186375..87e5766b84 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -110,6 +110,12 @@ function mayCommitAccountQuota(accountId: string, writerGeneration: number): boo // Valid upstream percentages are normalized to 0..100. Keep "unknown" outside that domain so an // actually exhausted account is still eligible for threshold rotation. export const CODEX_UNKNOWN_USAGE_SCORE = 101; +/** + * A window reading at or above this is a measured refusal, not a position on a scale. + * + * Separate from `CODEX_UNKNOWN_USAGE_SCORE` because they mean opposite things: unknown is + * "we have not observed this account", 100 is "we observed it and it is full". + */ export const CODEX_EXHAUSTED_USAGE_PERCENT = 100; export function isCodexQuotaExhausted( diff --git a/src/codex/routing.ts b/src/codex/routing.ts index 250aac9636..a13815aaa0 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -17,7 +17,7 @@ import { seedPoolRotationAccount, selectPriorityTier, } from "./pool-rotation"; -import { CODEX_UNKNOWN_USAGE_SCORE, getAccountQuota } from "./quota"; +import { CODEX_EXHAUSTED_USAGE_PERCENT, CODEX_UNKNOWN_USAGE_SCORE, getAccountQuota } from "./quota"; import { isThirtyDayOnlyCodexPlan } from "./plan"; import { MAIN_CODEX_ACCOUNT_ID, @@ -364,7 +364,8 @@ export function computeCodexUsageScore(quota: { weeklyPercent?: number; monthlyPercent?: number; shortPercent?: number; -} | null, plan?: unknown): number { + shortResetAt?: number; +} | null, plan?: unknown, now: number = Date.now()): number { if (!quota) return CODEX_UNKNOWN_USAGE_SCORE; const finite = (value: unknown): value is number => typeof value === "number" && Number.isFinite(value); const longWindows = isThirtyDayOnlyCodexPlan(plan) @@ -376,11 +377,50 @@ export function computeCodexUsageScore(quota: { // account whose weekly/monthly usage is entirely unverified look like the emptiest in the // pool, so `pickLowestUsageAmong` would send every request to it. Unknown has to stay // unknown until a governing window is actually observed. - if (knownLong.length === 0) return CODEX_UNKNOWN_USAGE_SCORE; + // + // A FULL burst window is the exception (#3029). It is not an optimistic guess about an + // unobserved window — it is a direct observation that the account cannot serve a request + // right now, whatever its monthly position turns out to be. Unknown-means-selectable is + // correct for uncertainty and wrong for a measured refusal: the account stays selected, + // `applyQuotaAutoSwitch` never fires, and the pool wedges on an exhausted credential. + if (knownLong.length === 0) { + return isTerminalShortWindow(quota, now) ? CODEX_EXHAUSTED_USAGE_PERCENT : CODEX_UNKNOWN_USAGE_SCORE; + } const values = finite(quota.shortPercent) ? [...knownLong, quota.shortPercent] : knownLong; return Math.max(...values); } +/** + * A short-only reading that proves the account is blocked NOW. + * + * Freshness is not optional. `getAccountQuota` performs no expiry check, partial updates + * carry the old short tuple forward, and disk hydration accepts a persisted reading for + * hours — so scoring 100 from `shortPercent` alone would keep excluding an account whose + * five-hour window has since reset. That is #3029 pointed the other way: the issue is that + * an exhausted account stays selected, and "a recovered account stays excluded" trades one + * unusable pool for another. + * + * A reading with no `shortResetAt` cannot be aged, so it stays unknown. The conservative + * direction here is the one that keeps an account selectable: a wrongly-selected account + * fails one request, while a wrongly-excluded one is invisible until someone reads the pool + * by hand. + */ +function isTerminalShortWindow( + quota: { shortPercent?: number; shortResetAt?: number }, + now: number, +): boolean { + if (typeof quota.shortPercent !== "number" || !Number.isFinite(quota.shortPercent)) return false; + if (quota.shortPercent < CODEX_EXHAUSTED_USAGE_PERCENT) return false; + const resetAt = quota.shortResetAt; + if (typeof resetAt !== "number" || !Number.isFinite(resetAt) || resetAt <= 0) return false; + // Both units reach storage: `normalizeResetAt` does not scale, and the GUI disambiguates + // by magnitude at read time. A comparison written against one assumption is off by 1000x + // against the other, and in the seconds-read-as-milliseconds direction every terminal + // reading looks like it reset in 1970 — a fix that passes its own test and does nothing. + const resetAtMs = resetAt < 10_000_000_000 ? resetAt * 1000 : resetAt; + return resetAtMs > now; +} + export function classifyCodexUpstreamOutcome( outcome: CodexUpstreamOutcome, denial?: "workspace" | "entitlement", @@ -1163,12 +1203,14 @@ function hasCodexQuotaHeadroom( config: OcxConfig, accountId: string, selectionOptions?: CodexAccountUsabilityOptions, + now: number = Date.now(), ): boolean { const threshold = config.autoSwitchThreshold ?? 80; if (threshold <= 0) return true; const usage = computeCodexUsageScore( getAccountQuota(accountId), getPoolAccountPlanForSelection(config, accountId, selectionOptions), + now, ); if (isUnknownUsage(usage)) return true; return usage < threshold; @@ -1356,6 +1398,7 @@ function pickLowerUsageAccount( const usage = computeCodexUsageScore( getAccountQuota(id), getPoolAccountPlanForSelection(config, id, selectionOptions), + now, ); if (usage < bestUsage) { best = id; @@ -1370,6 +1413,7 @@ function pickLowestUsageAmong( config: OcxConfig, ids: readonly string[], selectionOptions?: CodexAccountUsabilityOptions, + now: number = Date.now(), ): string | null { let best: string | null = null; let bestUsage = Number.POSITIVE_INFINITY; @@ -1377,6 +1421,7 @@ function pickLowestUsageAmong( const usage = computeCodexUsageScore( getAccountQuota(id), getPoolAccountPlanForSelection(config, id, selectionOptions), + now, ); if (usage < bestUsage) { best = id; @@ -1600,6 +1645,7 @@ function applyQuotaAutoSwitch( const activeUsage = computeCodexUsageScore( quota, getPoolAccountPlanForSelection(config, active, selectionOptions), + now, ); // Unknown usage is not evidence that a user's explicit selection crossed the // threshold. Wait for quota priming instead of rotating among guesses. @@ -1720,6 +1766,7 @@ function previewReusableAffinityAccount( const usage = computeCodexUsageScore( getAccountQuota(entry.accountId), getPoolAccountPlanForSelection(config, entry.accountId, selectionOptions), + now, ); if (!isUnknownUsage(usage) && usage >= threshold) { const best = pickLowerUsageAccount( @@ -1755,6 +1802,7 @@ function reevaluateAffinityQuota( ? computeCodexUsageScore( getAccountQuota(entry.accountId), getPoolAccountPlanForSelection(config, entry.accountId, selectionOptions), + now, ) : 0; const overThreshold = threshold > 0 && !isUnknownUsage(usage) && usage >= threshold; @@ -1848,6 +1896,7 @@ export function previewCodexAccountForRequest( const usage = computeCodexUsageScore( getAccountQuota(active), getPoolAccountPlanForSelection(config, active, selectionOptions), + now, ); if (!isUnknownUsage(usage) && usage >= threshold) { active = pickLowerUsageAccount(config, active, usage, now, quotaScope, selectionOptions); diff --git a/src/codex/subagent-model-fallback.ts b/src/codex/subagent-model-fallback.ts index 5b838f9493..27eea1e53d 100644 --- a/src/codex/subagent-model-fallback.ts +++ b/src/codex/subagent-model-fallback.ts @@ -226,7 +226,10 @@ export function isNativeModelQuotaExhausted( const resolvedAccountId = resolveRouteFallbackAccountId(route, config, accountId); if (!resolvedAccountId) return false; const quota = getAccountQuota(resolvedAccountId); - const usage = computeCodexUsageScore(quota, getPoolAccountPlan(config, resolvedAccountId)); + // Subagent fallback reads the same score, so a stale terminal reading would push + // subagents off a native model whose window has already reset. Thread the caller's clock + // rather than letting the scorer read wall time - the two would silently diverge. + const usage = computeCodexUsageScore(quota, getPoolAccountPlan(config, resolvedAccountId), now); if (usage >= CODEX_UNKNOWN_USAGE_SCORE) return false; return usage >= quotaThreshold(config); } diff --git a/tests/codex-routing.test.ts b/tests/codex-routing.test.ts index 1ec9f8cfda..cf6055c9ce 100644 --- a/tests/codex-routing.test.ts +++ b/tests/codex-routing.test.ts @@ -138,6 +138,39 @@ describe("codex routing", () => { expect(computeCodexUsageScore({ weeklyPercent: 40, shortPercent: 0 })).toBe(40); }); + test("a full burst window scores terminal while it is still in force (#3029)", () => { + // A FULL short window is not an optimistic guess about an unobserved long window - it + // is a direct observation that the account cannot serve a request right now. Leaving it + // unknown keeps the account selectable and suppresses auto-switch, which is exactly the + // pool wedge #3029 reports. + const now = 1_700_000_000_000; + expect(computeCodexUsageScore({ shortPercent: 100, shortResetAt: now + 60_000 }, undefined, now)).toBe(100); + + // Freshness is the other half. getAccountQuota performs no expiry check, partial + // updates carry the old short tuple forward, and disk hydration accepts a persisted + // reading for hours - so a reset window must go back to unknown, or #3029 is simply + // inverted into a recovered account that stays excluded. + expect(computeCodexUsageScore({ shortPercent: 100, shortResetAt: now - 60_000 }, undefined, now)) + .toBe(CODEX_UNKNOWN_USAGE_SCORE); + // No resetAt at all cannot be aged, so it stays unknown: a wrongly-selected account + // fails one request, a wrongly-excluded one is invisible until someone reads the pool. + expect(computeCodexUsageScore({ shortPercent: 100 }, undefined, now)).toBe(CODEX_UNKNOWN_USAGE_SCORE); + // Still narrow: a non-terminal short-only reading is unchanged. + expect(computeCodexUsageScore({ shortPercent: 99, shortResetAt: now + 60_000 }, undefined, now)) + .toBe(CODEX_UNKNOWN_USAGE_SCORE); + }); + + test("a terminal burst window is read in either unit (#3029)", () => { + // normalizeResetAt does not scale, and the GUI disambiguates by magnitude at read time, + // so both seconds and milliseconds reach storage. A comparison written against one + // assumption is off by 1000x against the other - and in the seconds-read-as-ms + // direction every terminal reading looks like it reset in 1970, which is a fix that + // passes its own test and does nothing. + const now = 1_700_000_000_000; + expect(computeCodexUsageScore({ shortPercent: 100, shortResetAt: now + 60_000 }, undefined, now)).toBe(100); + expect(computeCodexUsageScore({ shortPercent: 100, shortResetAt: (now + 60_000) / 1000 }, undefined, now)).toBe(100); + }); + test("exact-account failures record health without rotating the active Pool account", () => { const transient = makeConfig({ upstreamFailoverThreshold: 1, activeCodexAccountId: "a" }); const transientThread = "fixed-transient-thread"; diff --git a/tests/subagent-model-fallback.test.ts b/tests/subagent-model-fallback.test.ts index f37a07ef4a..75b2359ad5 100644 --- a/tests/subagent-model-fallback.test.ts +++ b/tests/subagent-model-fallback.test.ts @@ -21,7 +21,7 @@ import { } from "../src/codex/subagent-model-fallback"; import { saveCodexAccountCredential } from "../src/codex/account-store"; import { clearAccountNeedsReauth, markAccountNeedsReauth } from "../src/codex/account-runtime-state"; -import { clearAccountQuota, updateAccountQuota } from "../src/codex/quota"; +import { clearAccountQuota, setAccountQuotaFromParsed, updateAccountQuota } from "../src/codex/quota"; import { canAcquireCodexQuotaProbeLease, canAcquireCodexQuotaScopeProbeLease, @@ -734,6 +734,36 @@ describe("subagent model fallback chain", () => { expect(isSubagentModelUnavailable("gpt-5.6-sol", config, "pool-a")).toBe(false); }); + test("a full burst window makes the native model exhausted only while it holds (#3029)", () => { + // Subagent fallback reads the same usage score as pool selection, so a stale terminal + // reading pushes subagents off a native model whose five-hour window has already reset. + // The clock is passed explicitly and deliberately far from wall time: a fixture whose + // clock matches Date.now() cannot tell a threaded clock from a substituted one. + const now = 1_700_000_000_000; + const config = cfg({ + activeCodexAccountId: "pool-a", + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "pool", + }, + }, + subagentModelFallback: ["kimi/k3"], + }); + + resetSubagentModelFallbackStateForTests(); + clearAccountQuota("pool-a"); + setAccountQuotaFromParsed("pool-a", { shortPercent: 100, shortResetAt: now + 60_000 }); + expect(isNativeModelQuotaExhausted("gpt-5.6-sol", config, "pool-a", now)).toBe(true); + + resetSubagentModelFallbackStateForTests(); + clearAccountQuota("pool-a"); + setAccountQuotaFromParsed("pool-a", { shortPercent: 100, shortResetAt: now - 60_000 }); + expect(isNativeModelQuotaExhausted("gpt-5.6-sol", config, "pool-a", now)).toBe(false); + }); + test("openai-direct/gpt-5.5 is accepted as encrypted-task fallback when canonical", () => { resetSubagentModelFallbackStateForTests(); updateAccountQuota("pool-a", 95, undefined, 20); From 884b5eabc97d113c7474dbf76850e759181f34fe Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 03:54:43 +0900 Subject: [PATCH 2/4] fix(routing): carry the request clock through the selection helpers Review found the injected clock dropped one level below the scorer. hasCodexQuotaHeadroom and pickLowestUsageAmong defaulted to Date.now(), and their callers omitted it, so the priority tier, fill-first, preemption, pin release and shared-health checks all scored against wall time. With an injected now and a shortResetAt between the two, a terminal account is read as unknown and keeps its tier. Both helpers now take the clock, and every caller forwards the request's view: the tier lambda in getEligiblePoolAccounts, pickFillFirstCodexAccount, pickNextFillFirstCodexAccount (whose _now was parked unused), pickPriorityPreemption, releaseDrainedCodexAccountPin and isHealthySharedCodexSelection. Adds the end-to-end selection cases the plan asked for: a new thread moves to B when A's burst window is full in seconds, an already-bound thread rebinds when it is full in milliseconds, and A becomes selectable again once the window resets - so the fix cannot trade "exhausted account stays selected" for "recovered account stays excluded". The tiered case is scoped honestly. It proves a tiered pool honours a terminal window, and its comment says plainly that it does not isolate the threaded clock: selection reaches the same answer by another route when the clock is dropped there. The scorer and subagent cases carry that proof. --- src/codex/routing.ts | 24 ++++++++-------- tests/codex-routing.test.ts | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/codex/routing.ts b/src/codex/routing.ts index a13815aaa0..44b50e02cb 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -1171,7 +1171,7 @@ function getEligiblePoolAccounts( return selectPriorityTier( ids, codexAccountPriorityLookup(config), - id => hasCodexQuotaHeadroom(config, id, selectionOptions), + id => hasCodexQuotaHeadroom(config, id, selectionOptions, now), pinnedCodexAccountId(config), ); } @@ -1230,7 +1230,7 @@ function pickFillFirstCodexAccount( if (eligible.length === 0) return null; const active = getEffectiveActiveCodexAccountId(config); - if (active && eligible.includes(active) && hasCodexQuotaHeadroom(config, active, selectionOptions)) { + if (active && eligible.includes(active) && hasCodexQuotaHeadroom(config, active, selectionOptions, now)) { return active; } @@ -1242,7 +1242,7 @@ function pickNextFillFirstCodexAccount( config: OcxConfig, afterId: string | null, eligible: readonly string[] = listEligibleCodexAccountIds(config, Date.now()), - _now = Date.now(), + now = Date.now(), selectionOptions?: CodexAccountUsabilityOptions, ): string | null { if (eligible.length === 0) return null; @@ -1250,7 +1250,7 @@ function pickNextFillFirstCodexAccount( if (!afterId) { // Prefer an under-threshold account when starting with no active cursor. for (const id of ordered) { - if (hasCodexQuotaHeadroom(config, id, selectionOptions)) return id; + if (hasCodexQuotaHeadroom(config, id, selectionOptions, now)) return id; } return ordered[0] ?? null; } @@ -1265,7 +1265,7 @@ function pickNextFillFirstCodexAccount( const startIdx = stableAll.indexOf(afterId); if (startIdx < 0) { for (const id of ordered) { - if (hasCodexQuotaHeadroom(config, id, selectionOptions)) return id; + if (hasCodexQuotaHeadroom(config, id, selectionOptions, now)) return id; } return ordered[0] ?? null; } @@ -1276,7 +1276,7 @@ function pickNextFillFirstCodexAccount( const candidate = stableAll[(startIdx + step) % stableAll.length]!; if (!eligible.includes(candidate)) continue; if (!fallback) fallback = candidate; - if (hasCodexQuotaHeadroom(config, candidate, selectionOptions)) return candidate; + if (hasCodexQuotaHeadroom(config, candidate, selectionOptions, now)) return candidate; } return fallback ?? ordered[0] ?? null; } @@ -1442,6 +1442,7 @@ export function pickLowestUsageCodexAccount( config, getEligiblePoolAccounts(config, excludeId, now, quotaScope, selectionOptions), selectionOptions, + now, ); } @@ -1585,7 +1586,7 @@ function pickPriorityPreemption( if ( pinned !== undefined && eligible.includes(pinned) - && hasCodexQuotaHeadroom(config, pinned, selectionOptions) + && hasCodexQuotaHeadroom(config, pinned, selectionOptions, now) ) return null; const priorityOf = codexAccountPriorityLookup(config); if (priorityOf(eligible[0]!) <= priorityOf(active)) return null; @@ -1593,7 +1594,7 @@ function pickPriorityPreemption( // picking one would hand the request straight back to a drained account. return pickLowestUsageAmong( config, - eligible.filter(id => hasCodexQuotaHeadroom(config, id, selectionOptions)), + eligible.filter(id => hasCodexQuotaHeadroom(config, id, selectionOptions, now)), selectionOptions, ); } @@ -1611,6 +1612,7 @@ function releaseDrainedCodexAccountPin( CodexAccountUsabilityOptions, "nativeMainSelectionOnly" | "isMainAccountTokenLive" >, + now: number = Date.now(), ): void { const pinned = pinnedCodexAccountId(config); if (pinned === undefined) return; @@ -1625,7 +1627,7 @@ function releaseDrainedCodexAccountPin( // is readable. Cached reauth and configured pause state were handled above. if (pinned === MAIN_CODEX_ACCOUNT_ID && selectionOptions?.nativeMainSelectionOnly === true) return; const drained = !isCodexAccountUsable(config, pinned, selectionOptions) - || !hasCodexQuotaHeadroom(config, pinned, selectionOptions); + || !hasCodexQuotaHeadroom(config, pinned, selectionOptions, now); if (!drained) return; clearCodexAccountPin(config); saveConfigPreservingClaudeCode(config); @@ -1679,7 +1681,7 @@ function isHealthySharedCodexSelection( selectionOptions: CodexAccountUsabilityOptions | undefined, ): boolean { return isCodexAccountSelectable(config, accountId, now, quotaScope, selectionOptions) - && hasCodexQuotaHeadroom(config, accountId, selectionOptions) + && hasCodexQuotaHeadroom(config, accountId, selectionOptions, now) && !shouldFailover(config, accountId, now); } @@ -1936,7 +1938,7 @@ export function resolveCodexAccountForThreadDetailed( // revive after quota resets. Independent model scopes must never persist a // change to shared routing state. if (!isIndependentCodexQuotaScope(quotaScope)) { - releaseDrainedCodexAccountPin(config, sharedStateSelectionOptions(selectionOptions)); + releaseDrainedCodexAccountPin(config, sharedStateSelectionOptions(selectionOptions), now); } const sharedActiveBeforeSelection = getEffectiveActiveCodexAccountId(config); const preserveSharedSelectionForModelDetour = modelScopedSelection && ( diff --git a/tests/codex-routing.test.ts b/tests/codex-routing.test.ts index cf6055c9ce..324b561755 100644 --- a/tests/codex-routing.test.ts +++ b/tests/codex-routing.test.ts @@ -47,6 +47,7 @@ import { updateAccountQuota, } from "../src/codex/auth-api"; import { CODEX_UNKNOWN_USAGE_SCORE, isCodexQuotaExhausted } from "../src/codex/quota"; +import { setCodexAccountPriority } from "../src/codex/account-priority"; import { MAIN_CODEX_ACCOUNT_ID } from "../src/codex/main-account"; import { routeModel } from "../src/router"; import { consumeForInspection } from "../src/server/relay"; @@ -171,6 +172,61 @@ describe("codex routing", () => { expect(computeCodexUsageScore({ shortPercent: 100, shortResetAt: (now + 60_000) / 1000 }, undefined, now)).toBe(100); }); + test("a live full burst window moves selection off the account (#3029)", () => { + // The scorer assertions above prove the value; this proves the pool acts on it. A + // clock far from wall time is the point: a fixture whose now matches Date.now() cannot + // tell a threaded clock from one that was dropped somewhere in the helper chain. + const now = 1_700_000_000_000; + const config = makeConfig({ activeCodexAccountId: "a" }); + + // A is full for the next hour, recorded in SECONDS. B has ordinary headroom. + setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: (now + 3_600_000) / 1000 }); + updateAccountQuota("b", 10); + expect(resolveCodexAccountForThread("thread-terminal-new", config, now)).toBe("b"); + + // Same pool, but a thread already bound to A: it must rebind rather than keep an + // account that cannot serve it. Milliseconds this time, so both units are exercised + // through the real selection path and not only through the scorer. + clearAccountQuota("a"); + setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now + 3_600_000 }); + const bound = makeConfig({ activeCodexAccountId: "a" }); + expect(resolveCodexAccountForThread("thread-terminal-bound", bound, now - 1000)).toBeString(); + expect(resolveCodexAccountForThread("thread-terminal-bound", bound, now)).toBe("b"); + + // And once the window resets, A is selectable again. Without this the fix would trade + // "exhausted account stays selected" for "recovered account stays excluded". + clearAccountQuota("a"); + clearAccountQuota("b"); + setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now - 60_000 }); + const recovered = makeConfig({ activeCodexAccountId: "a" }); + expect(resolveCodexAccountForThread("thread-terminal-recovered", recovered, now)).toBe("a"); + }); + + test("a tiered pool still moves off a terminal high-priority account (#3029)", () => { + // The pool carries DIFFERENT priorities here, which is the only shape that reaches + // selectPriorityTier's headroom check. A outranks B and B is worse on ordinary usage, + // so nothing except the terminal reading can move the selection. + // + // Note on scope: this asserts the tiered path honours a terminal window. It does NOT + // isolate the clock threaded through that helper - selection reaches the same answer + // by another route when the clock is dropped there, so a green result here is not + // evidence the threading is intact. The scorer and subagent cases carry that proof. + const now = Date.now() + 7 * 24 * 60 * 60 * 1000; + const config = makeConfig({ activeCodexAccountId: "a" }); + setCodexAccountPriority(config, "a", 1); + setCodexAccountPriority(config, "b", 2); + + setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now + 3_600_000 }); + // B is WORSE on ordinary usage, so lowest-usage selection would prefer A. Only the + // tier check - which reads the clock through hasCodexQuotaHeadroom - can move the + // selection to B, which is what makes this case load-bearing for the clock. + updateAccountQuota("b", 70); + + // A outranks B, but its burst window is full at the request's now, so the tier must + // fall through to B. Reading Date.now() here would score A unknown and keep it. + expect(resolveCodexAccountForThread("thread-priority-terminal", config, now)).toBe("b"); + }); + test("exact-account failures record health without rotating the active Pool account", () => { const transient = makeConfig({ upstreamFailoverThreshold: 1, activeCodexAccountId: "a" }); const transientThread = "fixed-transient-thread"; From 199f23f16af7d0e3f1515064527487a7961d8c7d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 04:04:47 +0900 Subject: [PATCH 3/4] fix(routing): finish the clock, and make three vacuous cases real Second review round found three more wall-time reads and three tests that were green for reasons unrelated to what they claimed. pickLowestUsageAmong inside pickPriorityPreemption, and both shared-health checks in the affinity and active-selection paths, still omitted the clock. They now pass it, so every selection path scores against one view of time. The affinity case bound its thread while A was ALREADY terminal, so the first resolution could pick B and the second merely proved B was reused. It now binds to A while A is cool, then fills A's window and asserts the rebind. The recovery case left both accounts unknown, where A is kept by default - true even against a freshness-blind scorer. B now carries known headroom, so a scorer that ignores the reset moves the request to B and the assertion fails. The tiered case had the priority order backwards: higher numbers run earlier, so B outranked A and won regardless of A's window. It also used a future clock, which is live under both views. It now gives A the higher priority, uses a historical instant with A's window live only against the request clock, and runs fill-first - so the tier check is the only thing that can move the selection. Both are now red against the defect they name: dropping the tier clock fails the tiered case, and removing the freshness gate fails the selection case. --- src/codex/routing.ts | 5 ++-- tests/codex-routing.test.ts | 54 ++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/codex/routing.ts b/src/codex/routing.ts index 44b50e02cb..9890e14046 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -1596,6 +1596,7 @@ function pickPriorityPreemption( config, eligible.filter(id => hasCodexQuotaHeadroom(config, id, selectionOptions, now)), selectionOptions, + now, ); } @@ -1996,7 +1997,7 @@ export function resolveCodexAccountForThreadDetailed( && isCodexAccountSelectable(config, entry.accountId, now, quotaScope, selectionOptions); const failoverReady = shouldFailover(config, entry.accountId, now); const healthyForSharedAffinity = selectableForSharedState - && hasCodexQuotaHeadroom(config, entry.accountId, sharedSelectionOptions) + && hasCodexQuotaHeadroom(config, entry.accountId, sharedSelectionOptions, now) && !failoverReady; if ( selectableForRequest @@ -2092,7 +2093,7 @@ export function resolveCodexAccountForThreadDetailed( sharedSelectionOptions, ); const activeHealthyForSharedSelection = activeSelectableForSharedState - && hasCodexQuotaHeadroom(config, active, sharedSelectionOptions) + && hasCodexQuotaHeadroom(config, active, sharedSelectionOptions, now) && !shouldFailover(config, active, now); if (!isCodexAccountSelectable(config, active, now, quotaScope, selectionOptions)) { const fallback = pickLowestUsageCodexAccount(config, active, now, quotaScope, selectionOptions); diff --git a/tests/codex-routing.test.ts b/tests/codex-routing.test.ts index 324b561755..75a3cebe64 100644 --- a/tests/codex-routing.test.ts +++ b/tests/codex-routing.test.ts @@ -184,46 +184,50 @@ describe("codex routing", () => { updateAccountQuota("b", 10); expect(resolveCodexAccountForThread("thread-terminal-new", config, now)).toBe("b"); - // Same pool, but a thread already bound to A: it must rebind rather than keep an - // account that cannot serve it. Milliseconds this time, so both units are exercised - // through the real selection path and not only through the scorer. + // Same pool, but a thread already BOUND to A. Bind it while A is cool, so the rebind + // below is a real transition rather than a first selection that happened to pick B. clearAccountQuota("a"); - setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now + 3_600_000 }); + clearAccountQuota("b"); + updateAccountQuota("a", 10); + updateAccountQuota("b", 20); const bound = makeConfig({ activeCodexAccountId: "a" }); - expect(resolveCodexAccountForThread("thread-terminal-bound", bound, now - 1000)).toBeString(); + expect(resolveCodexAccountForThread("thread-terminal-bound", bound, now)).toBe("a"); + // A's burst window fills, in MILLISECONDS this time so both units run through the real + // selection path. The bound thread must move rather than keep an account that cannot + // serve it. + clearAccountQuota("a"); + setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now + 3_600_000 }); expect(resolveCodexAccountForThread("thread-terminal-bound", bound, now)).toBe("b"); - // And once the window resets, A is selectable again. Without this the fix would trade - // "exhausted account stays selected" for "recovered account stays excluded". + // And once the window resets, A stays selected. B carries KNOWN headroom here on + // purpose: with both accounts unknown, A would be kept by default and the assertion + // would hold even against a freshness-blind scorer. Against one, expired-A scores 100 + // and the request moves to B. clearAccountQuota("a"); clearAccountQuota("b"); setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now - 60_000 }); + updateAccountQuota("b", 20); const recovered = makeConfig({ activeCodexAccountId: "a" }); expect(resolveCodexAccountForThread("thread-terminal-recovered", recovered, now)).toBe("a"); }); - test("a tiered pool still moves off a terminal high-priority account (#3029)", () => { - // The pool carries DIFFERENT priorities here, which is the only shape that reaches - // selectPriorityTier's headroom check. A outranks B and B is worse on ordinary usage, - // so nothing except the terminal reading can move the selection. + test("the priority tier reads the request clock, not wall time (#3029)", () => { + // selectPriorityTier consults hasCodexQuotaHeadroom only when the pool carries + // DIFFERENT priorities, so a clock dropped in that lambda is invisible to an ordinary + // pool. Higher numbers run earlier, so A (2) outranks B (1). // - // Note on scope: this asserts the tiered path honours a terminal window. It does NOT - // isolate the clock threaded through that helper - selection reaches the same answer - // by another route when the clock is dropped there, so a green result here is not - // evidence the threading is intact. The scorer and subagent cases carry that proof. - const now = Date.now() + 7 * 24 * 60 * 60 * 1000; - const config = makeConfig({ activeCodexAccountId: "a" }); - setCodexAccountPriority(config, "a", 1); - setCodexAccountPriority(config, "b", 2); + // The clock is historical, well before wall time. A's window is full for an hour after + // THAT instant, so it is live against the request clock and long expired against + // Date.now(). With the correct clock the tier sees A drained and descends to B; reading + // wall time makes A look unknown, the tier keeps it, and fill-first hands back A. + const now = 1_700_000_000_000; + const config = makeConfig({ activeCodexAccountId: "a", accountPoolStrategy: "fill-first" }); + setCodexAccountPriority(config, "a", 2); + setCodexAccountPriority(config, "b", 1); setAccountQuotaFromParsed("a", { shortPercent: 100, shortResetAt: now + 3_600_000 }); - // B is WORSE on ordinary usage, so lowest-usage selection would prefer A. Only the - // tier check - which reads the clock through hasCodexQuotaHeadroom - can move the - // selection to B, which is what makes this case load-bearing for the clock. - updateAccountQuota("b", 70); + updateAccountQuota("b", 20); - // A outranks B, but its burst window is full at the request's now, so the tier must - // fall through to B. Reading Date.now() here would score A unknown and keep it. expect(resolveCodexAccountForThread("thread-priority-terminal", config, now)).toBe("b"); }); From c53b8e2b672248ceeade9f06e22af16b5faf41b4 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 04:08:39 +0900 Subject: [PATCH 4/4] docs(devlog): record what wp4's review rounds changed about the plan --- .../040_wp4_terminal_short_window.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/devlog/_plan/260831_prio70_train_round2/040_wp4_terminal_short_window.md b/devlog/_plan/260831_prio70_train_round2/040_wp4_terminal_short_window.md index 1e09e13645..44543ca5e2 100644 --- a/devlog/_plan/260831_prio70_train_round2/040_wp4_terminal_short_window.md +++ b/devlog/_plan/260831_prio70_train_round2/040_wp4_terminal_short_window.md @@ -177,3 +177,27 @@ Suite, typecheck and privacy scan on `ssh lidge`. `Closes #3029`. PR #3003 is unrelated to this issue and independently defective (two pre-WHAM errors lack `quotaProbeSkipped` at its `src/codex/auth-api.ts:1013-1021`, causing false five-minute suppression); do not link it here. + +## What implementation added beyond this plan + +Three adversarial review rounds (findings 2, 4, 0). The plan's scoring rule survived +intact; everything the rounds found was in the plumbing and in the tests: + +- **The clock leaks below the scorer.** The plan enumerated eight `computeCodexUsageScore` + call sites and I threaded all of them. That was not enough: `hasCodexQuotaHeadroom` and + `pickLowestUsageAmong` each defaulted to `Date.now()`, and their callers omitted it — so + the priority tier, fill-first, preemption, pin release and both shared-health checks all + scored against wall time while the resolver above them used the request clock. An + injected `now` with a `shortResetAt` between the two reads the same tuple two ways. +- **Three of my own tests were vacuous, and one was backwards.** The affinity case bound + its thread while the account was already terminal, so it proved reuse rather than a + rebind. The recovery case left both accounts unknown, where the active one is kept by + default — true even against a freshness-blind scorer. And the tiered case had the + priority order inverted: higher numbers run earlier + (`src/codex/account-priority.ts:18`), so the account I meant to outrank actually lost, + and it won for a reason unrelated to the window. + +The last one is worth keeping as a rule: a test whose fixture encodes a directional +assumption should be driven red against the specific defect it names, not merely observed +to pass. Each of the eight assertions now has a named mutation it fails against, recorded +in the PR description.