diff --git a/gui/src/codex-quota-utils.ts b/gui/src/codex-quota-utils.ts index c836aa9ede..57feab153f 100644 --- a/gui/src/codex-quota-utils.ts +++ b/gui/src/codex-quota-utils.ts @@ -1,4 +1,4 @@ -import { CODEX_EXHAUSTED_USAGE_PERCENT, TERMINAL_SHORT_WINDOW_FRESHNESS_MS } from "../../src/codex/quota-types"; +import { isTerminalShortWindow } from "../../src/codex/quota-types"; export interface AccountQuota { weeklyPercent?: number; @@ -83,17 +83,24 @@ export function computeCodexUsageScore( : [quota.weeklyPercent, quota.monthlyPercent]; const knownLong = longWindows.filter(finite); if (knownLong.length === 0) { - const shortReset = quota.fiveHourResetAt ?? quota.shortResetAt; - const shortObservationAge = typeof quota.shortObservedAt === "number" - ? now - quota.shortObservedAt - : undefined; - const isExhausted = finite(shortPercent) && shortPercent >= CODEX_EXHAUSTED_USAGE_PERCENT && ( - (typeof shortReset === "number" && shortReset > now) || - (typeof shortObservationAge === "number" - && shortObservationAge >= 0 - && shortObservationAge <= TERMINAL_SHORT_WINDOW_FRESHNESS_MS) - ); - return isExhausted ? 100 : null; + // The same decision the router makes, made by the same function rather than by a second + // copy of the rule. The copy that used to live here differed twice: it compared a stored + // reset against `now` without normalizing seconds to milliseconds, so a seconds-form + // future reset read as expired; and it accepted a fresh observation even when an ELAPSED + // reset was present, where routing treats a reset as authoritative once it exists. Either + // difference reports an account the router will refuse as usable (#5045). + // + // The alias collapse happens here because it is a wire concern of this DTO: the account + // API spells the same burst window `fiveHour*` and the stored snapshot spells it `short*`. + return isTerminalShortWindow({ + ...(finite(shortPercent) ? { shortPercent } : {}), + ...(finite(quota.fiveHourResetAt ?? quota.shortResetAt) + ? { shortResetAt: quota.fiveHourResetAt ?? quota.shortResetAt } + : {}), + ...(finite(quota.shortObservedAt) ? { shortObservedAt: quota.shortObservedAt } : {}), + }, now) + ? 100 + : null; } const values = finite(shortPercent) ? [...knownLong, shortPercent] : knownLong; return values.length ? Math.max(...values) : null; diff --git a/src/codex/quota-types.ts b/src/codex/quota-types.ts index 626bd24aac..ca7574bc6b 100644 --- a/src/codex/quota-types.ts +++ b/src/codex/quota-types.ts @@ -19,6 +19,51 @@ export const TERMINAL_SHORT_WINDOW_FRESHNESS_MS = 5 * 60_000; */ export const CODEX_EXHAUSTED_USAGE_PERCENT = 100; +/** + * Above this a value is already milliseconds; at or below it, it is Unix seconds. + * + * Both reach storage, so the split has to live somewhere every reader can see. It lives on this + * leaf rather than beside the merge that uses it because the dashboard reads the same stored + * value and cannot import the disk-cache owner. + */ +const RESET_AT_SECONDS_MAX = 10_000_000_000; + +/** Normalize a stored reset instant to milliseconds. */ +export function resetAtToMs(resetAt: number): number { + return resetAt < RESET_AT_SECONDS_MAX ? resetAt * 1000 : resetAt; +} + +/** + * A short-only reading that proves the account is blocked NOW. + * + * Routing and the dashboard's account-switch warning must answer this identically for the same + * snapshot, or the warning tells an operator an account is usable while the router refuses it. + * They did not: the dashboard compared a stored reset against `Date.now()` without normalizing + * units, so a seconds-form future reset looked expired there and live here, and it treated a + * fresh observation as sufficient even when an ELAPSED reset was present, where routing treats + * the reset as authoritative once it exists (#5045). + * + * Freshness is not optional in the reset-less branch. `getAccountQuota` performs no expiry + * check, partial updates carry a still-open short tuple forward, and disk hydration accepts a + * persisted reading for hours, so scoring exhausted from `shortPercent` alone would keep + * excluding an account whose burst window has since reset. + */ +export function isTerminalShortWindow( + quota: Pick, + now: number, +): boolean { + const finite = (value: unknown): value is number => typeof value === "number" && Number.isFinite(value); + if (!finite(quota.shortPercent) || quota.shortPercent < CODEX_EXHAUSTED_USAGE_PERCENT) return false; + const resetAt = quota.shortResetAt; + if (!finite(resetAt) || resetAt <= 0) { + const observedAt = quota.shortObservedAt; + if (!finite(observedAt)) return false; + const age = now - observedAt; + return age >= 0 && age <= TERMINAL_SHORT_WINDOW_FRESHNESS_MS; + } + return resetAtToMs(resetAt) > now; +} + export type StoredAccountQuota = { weeklyPercent?: number; monthlyPercent?: number; diff --git a/src/codex/quota.ts b/src/codex/quota.ts index a8c6a0a6ab..56ba7abdd3 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -8,7 +8,7 @@ import { getObservedMainQuotaIdentityKey, isMainQuotaWriterLive, type MainQuotaW import { CodexQuotaHistory, QUOTA_HISTORY_LIMITS, type QuotaHistoryWindow } from "./quota-history"; import { isPoolQuotaWriterLive, poolQuotaHistoryIdentity } from "./account-store"; -import { CODEX_EXHAUSTED_USAGE_PERCENT, MAIN_ACCOUNT_HARD_LOCK_PERCENT } from "./quota-types"; +import { CODEX_EXHAUSTED_USAGE_PERCENT, MAIN_ACCOUNT_HARD_LOCK_PERCENT, resetAtToMs } from "./quota-types"; import type { PoolQuotaWriter, StoredAccountQuota, WhamUsageResponse, WhamUsageWindow } from "./quota-types"; export type { StoredAccountQuota, WhamUsageResponse } from "./quota-types"; @@ -57,14 +57,13 @@ const WEEKLY_WINDOW_MIN_MINUTES = WEEKLY_WINDOW_MIN_SECONDS / 60; * Both units reach storage — `normalizeResetAt` does not scale, and the GUI disambiguates by * magnitude at read time — so a comparison written against one assumption is off by 1000x * against the other. In the seconds-read-as-milliseconds direction every reading looks like it - * elapsed in 1970, which is a check that passes its own test and does nothing. Exported so - * `isTerminalShortWindow` in routing.ts shares this one split instead of repeating the literal. + * elapsed in 1970, which is a check that passes its own test and does nothing. + * + * The split now lives on `./quota-types`, the leaf the dashboard can also import, because the + * dashboard was the reader that did not have it (#5045). Re-exported here so the existing + * callers of this module keep their import path. */ -const RESET_AT_SECONDS_MAX = 10_000_000_000; - -export function resetAtToMs(resetAt: number): number { - return resetAt < RESET_AT_SECONDS_MAX ? resetAt * 1000 : resetAt; -} +export { resetAtToMs }; const accountQuota = new Map(); const quotaHistory = new CodexQuotaHistory(); diff --git a/src/codex/routing/cooldown-math.ts b/src/codex/routing/cooldown-math.ts index 61d6df875d..00f85abac7 100644 --- a/src/codex/routing/cooldown-math.ts +++ b/src/codex/routing/cooldown-math.ts @@ -1,10 +1,9 @@ import { CODEX_EXHAUSTED_USAGE_PERCENT, CODEX_UNKNOWN_USAGE_SCORE, - resetAtToMs, } from "../quota"; import { isThirtyDayOnlyCodexPlan } from "../plan"; -import { TERMINAL_SHORT_WINDOW_FRESHNESS_MS } from "../quota-types"; +import { isTerminalShortWindow } from "../quota-types"; import type { CodexQuotaScope } from "./health-store"; import type { TransientProbeGrant } from "./thread-affinity"; @@ -133,44 +132,11 @@ export function computeCodexUsageScore(quota: { 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 a still-open 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. Merge no longer carries an elapsed shortResetAt, but an - * explicit incoming elapsed tuple is still stored, and a missing reset cannot be aged there. - * 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. - * - * A missing reset can instead be aged by shortObservedAt (#3425). General updatedAt is not - * sufficient: credit-only updates preserve the old short tuple but advance that timestamp. - * Old disk snapshots without short-window provenance remain unknown. - */ -function isTerminalShortWindow( - quota: { shortPercent?: number; shortResetAt?: number; shortObservedAt?: 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) { - const observedAt = quota.shortObservedAt; - if (typeof observedAt !== "number" || !Number.isFinite(observedAt)) return false; - const age = now - observedAt; - return age >= 0 && age <= TERMINAL_SHORT_WINDOW_FRESHNESS_MS; - } - // Seconds and milliseconds both reach storage, so the split lives in one place next to the - // merge that also ages a stored reset instant (`resetAtToMs`, src/codex/quota.ts). - return resetAtToMs(resetAt) > now; -} +// `isTerminalShortWindow` moved to ../quota-types, the leaf the dashboard can import. Routing +// and the account-switch warning have to answer this identically for the same snapshot, and +// they did not: see the note on the shared function (#5045). Its #3029 and #3425 reasoning — +// why freshness is not optional, and why a reading with no reset and no observation stays +// unknown rather than exhausted — moved with it. export function classifyCodexUpstreamOutcome( outcome: CodexUpstreamOutcome, diff --git a/src/providers/codex-capacity.ts b/src/providers/codex-capacity.ts index 175620def9..604176f8e1 100644 --- a/src/providers/codex-capacity.ts +++ b/src/providers/codex-capacity.ts @@ -36,6 +36,15 @@ export const CODEX_CAPACITY_MAX_QUOTA_AGE_MS = 30 * 60_000; export type CodexCapacityQuota = { fiveHourPercent?: number; fiveHourResetAt?: number; + /** + * Local observation time for the burst-window percentage. + * + * Carried because the terminal-short-window rule needs it whenever the reading has no reset + * instant, and the dashboard's account-switch warning evaluates that rule against this DTO. + * Dropping it here made the warning return "no opinion" for a snapshot routing was already + * refusing on (#5045). + */ + shortObservedAt?: number; weeklyPercent?: number; weeklyResetAt?: number; monthlyPercent?: number; diff --git a/src/providers/quota/report-cache.ts b/src/providers/quota/report-cache.ts index 44010ebbf7..cae46ccb9c 100644 --- a/src/providers/quota/report-cache.ts +++ b/src/providers/quota/report-cache.ts @@ -151,6 +151,9 @@ export function providerQuotaFromCodexQuota( const projected: CodexCapacityQuota = { ...(quota.shortPercent !== undefined ? { fiveHourPercent: quota.shortPercent } : {}), ...(quota.shortResetAt !== undefined ? { fiveHourResetAt: quota.shortResetAt } : {}), + // Freshness for the reset-less terminal rule. Without it the dashboard evaluates that rule + // with no evidence and returns null while routing refuses the same account (#5045). + ...(quota.shortObservedAt !== undefined ? { shortObservedAt: quota.shortObservedAt } : {}), ...(quota.weeklyPercent !== undefined ? { weeklyPercent: quota.weeklyPercent } : {}), ...(quota.weeklyResetAt !== undefined ? { weeklyResetAt: quota.weeklyResetAt } : {}), ...(quota.monthlyPercent !== undefined ? { monthlyPercent: quota.monthlyPercent } : {}), diff --git a/tests/gui/gui-codex-usage-score-parity.test.ts b/tests/gui/gui-codex-usage-score-parity.test.ts new file mode 100644 index 0000000000..e72f550d9a --- /dev/null +++ b/tests/gui/gui-codex-usage-score-parity.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, test } from "bun:test"; +import { computeCodexUsageScore as guiScore } from "../../gui/src/codex-quota-utils"; +import type { AccountQuota } from "../../gui/src/codex-quota-utils"; +import { computeCodexUsageScore as routerScore } from "../../src/codex/routing/cooldown-math"; +import { providerQuotaFromCodexQuota } from "../../src/providers/quota/report-cache"; +import { CODEX_UNKNOWN_USAGE_SCORE } from "../../src/codex/quota"; +import { TERMINAL_SHORT_WINDOW_FRESHNESS_MS } from "../../src/codex/quota-types"; + +/** + * The account-switch warning and the router must reach the same verdict for one snapshot, or + * the dashboard reports an account as usable while the router refuses it (#5045). + * + * The two disagreed in three places, each of which looked local and correct: the dashboard + * compared a stored reset against `now` without normalizing Unix seconds to milliseconds; it + * accepted a fresh observation even when an ELAPSED reset was present, where the router treats + * a reset as authoritative once it exists; and the Free/Go projection dropped the burst window + * that the router counts on every plan. + * + * So this compares the two implementations on shared fixtures rather than asserting either + * one against a literal. A future edit that moves one side alone fails here. + */ +const NOW = 1_800_000_000_000; + +/** The router spells unknown as a sentinel above the 0..100 domain; the dashboard spells it null. */ +type RouterQuota = Parameters[0]; +const asRouterQuota = (quota: AccountQuota): RouterQuota => quota as unknown as RouterQuota; + +function agree(quota: AccountQuota, plan?: string | null): { gui: number | null; router: number } { + return { + gui: guiScore(quota, plan ?? null, NOW), + router: routerScore(asRouterQuota(quota), plan ?? null, NOW), + }; +} + +function expectSame(quota: AccountQuota, plan?: string | null): void { + const { gui, router } = agree(quota, plan); + const guiAsRouter = gui === null ? CODEX_UNKNOWN_USAGE_SCORE : gui; + expect({ quota, plan: plan ?? null, gui: guiAsRouter }).toEqual({ quota, plan: plan ?? null, gui: router }); +} + +describe("account-switch warning agrees with routing (#5045)", () => { + test("a terminal burst window reads the same in seconds and in milliseconds", () => { + // Both units reach storage. Read as milliseconds, a seconds-form instant lands in 1970 and + // every future reset looks elapsed — a check that passes its own test and does nothing. + const futureMs = NOW + 60_000; + for (const shortResetAt of [futureMs, Math.floor(futureMs / 1000)]) { + const quota: AccountQuota = { shortPercent: 100, shortResetAt, updatedAt: NOW }; + expectSame(quota); + expect(guiScore(quota, null, NOW)).toBe(100); + } + }); + + test("an elapsed reset is authoritative even with a fresh observation", () => { + // The reset says the window is over. Freshness is the fallback for a reading that has no + // reset at all, not a second opinion that can override one. + expectSame({ + shortPercent: 100, + shortResetAt: Math.floor((NOW - 60_000) / 1000), + shortObservedAt: NOW - 1_000, + updatedAt: NOW, + }); + }); + + test("a reset-less reading follows its observation freshness on both sides", () => { + for (const age of [0, TERMINAL_SHORT_WINDOW_FRESHNESS_MS, TERMINAL_SHORT_WINDOW_FRESHNESS_MS + 1]) { + expectSame({ shortPercent: 100, shortObservedAt: NOW - age, updatedAt: NOW }); + } + // Neither reset nor observation is still unknown, not exhausted: a wrongly-excluded account + // is invisible until someone reads the pool by hand. + expectSame({ shortPercent: 100, updatedAt: NOW }); + }); + + test("a known governing window still wins over the burst refinement", () => { + expectSame({ weeklyPercent: 42, monthlyPercent: 7, shortPercent: 100, updatedAt: NOW }); + expectSame({ monthlyPercent: 90, updatedAt: NOW }, "plus"); + }); + + test("the DTO the dashboard receives carries the freshness the rule needs", () => { + // The warning scores whatever `providerQuotaFromCodexQuota` delivered. That projection + // mapped short -> fiveHour but dropped `shortObservedAt`, so a reset-less terminal reading + // arrived with no freshness evidence and the dashboard returned "no opinion" for an account + // the router was already refusing. The two are compared on the SAME stored snapshot, one + // through the DTO and one directly, which is the shape of the divergence. + const stored = { shortPercent: 100, shortObservedAt: NOW - 1_000, updatedAt: NOW }; + const dto = providerQuotaFromCodexQuota(stored); + expect(dto?.shortObservedAt).toBe(NOW - 1_000); + expect(guiScore(dto as AccountQuota, null, NOW)).toBe(100); + expect(guiScore(dto as AccountQuota, null, NOW)).toBe(routerScore(stored, null, NOW)); + }); +});