diff --git a/open-sse/services/combo/quotaScoring.ts b/open-sse/services/combo/quotaScoring.ts index 8f9dbbca92..4a853b1455 100644 --- a/open-sse/services/combo/quotaScoring.ts +++ b/open-sse/services/combo/quotaScoring.ts @@ -177,46 +177,113 @@ function normalizeWindowPercentUsed(value: unknown): number | null { return clamp01(numericValue); } +type QuotaWindowSnapshot = { percentUsed: number | null; resetAt: string | null }; + +/** + * Pick the first candidate that actually carries a reset instant, falling back + * to the first present candidate. A window can be structurally present but + * carry `resetAt: null` (e.g. Codex's `window7d` placeholder when the upstream + * only reported the primary limit); a plain `a || b` short-circuit would let + * that empty window shadow a sibling that does know when it resets — #9330. + */ +function pickWindowWithResetAt( + ...candidates: Array +): QuotaWindowSnapshot | null { + return candidates.find((candidate) => candidate?.resetAt) ?? candidates.find(Boolean) ?? null; +} + function getNamedQuotaWindow( quota: unknown, windowName: ResetWindowName -): { percentUsed: number | null; resetAt: string | null } | null { +): QuotaWindowSnapshot | null { if (!quota || !isRecord(quota)) return null; if (windowName === "session") return getQuotaWindow(quota, "window5h"); if (windowName === "weekly") { - return getQuotaWindow(quota, "window7d") || getQuotaWindow(quota, "windowWeekly"); + return pickWindowWithResetAt( + getQuotaWindow(quota, "window7d"), + getQuotaWindow(quota, "windowWeekly") + ); } if (windowName === "monthly") return getQuotaWindow(quota, "windowMonthly"); return null; } -function getWindowsMapQuotaWindow( - quota: unknown, - windowName: ResetWindowName -): { percentUsed: number | null; resetAt: string | null } | null { - if (!quota || !isRecord(quota) || !isRecord(quota.windows)) return null; - const candidates = Object.entries(quota.windows) - .map(([key, value]) => ({ key: key.toLowerCase(), value })) - .filter(({ key }) => key === windowName || key.startsWith(`${windowName} `)); - - if (candidates.length === 0) return null; - candidates.sort((a, b) => a.key.localeCompare(b.key)); - const window = candidates[0].value; +function toWindowSnapshot(window: unknown): QuotaWindowSnapshot | null { if (!isRecord(window)) return null; - return { percentUsed: normalizeWindowPercentUsed(window.percentUsed), resetAt: normalizeResetAt(window.resetAt), }; } +/** + * Every entry of the snapshot's `windows` map, name lower-cased. + * + * Deliberately reads `windows` only, never Codex's wider `allWindows`: for a + * Spark request `fetchCodexQuota` narrows `windows` to the Spark scope on + * purpose, and pulling the normal-scope entries back in would rank a request + * against a window it cannot spend. + */ +function getQuotaWindowEntries( + quota: unknown +): Array<{ key: string; window: QuotaWindowSnapshot }> { + if (!quota || !isRecord(quota) || !isRecord(quota.windows)) return []; + const entries: Array<{ key: string; window: QuotaWindowSnapshot }> = []; + for (const [key, value] of Object.entries(quota.windows)) { + const window = toWindowSnapshot(value); + if (window) entries.push({ key: key.toLowerCase(), window }); + } + return entries; +} + +function getWindowsMapQuotaWindow( + quota: unknown, + windowName: ResetWindowName +): QuotaWindowSnapshot | null { + const candidates = getQuotaWindowEntries(quota).filter( + ({ key }) => key === windowName || key.startsWith(`${windowName} `) + ); + + if (candidates.length === 0) return null; + candidates.sort((a, b) => a.key.localeCompare(b.key)); + // Prefer a candidate that knows when it resets (e.g. "weekly" vs a scoped + // "weekly (spark)" placeholder without a resetAt) — #9330. + return pickWindowWithResetAt( + ...candidates.filter(({ window }) => window.resetAt).map(({ window }) => window), + candidates[0].window + ); +} + function resolveQuotaWindowByName( quota: unknown, windowName: ResetWindowName -): { percentUsed: number | null; resetAt: string | null } | null { - return getNamedQuotaWindow(quota, windowName) || getWindowsMapQuotaWindow(quota, windowName); +): QuotaWindowSnapshot | null { + return pickWindowWithResetAt( + getNamedQuotaWindow(quota, windowName), + getWindowsMapQuotaWindow(quota, windowName) + ); +} + +/** + * Earliest reset instant across EVERY window a snapshot exposes, regardless of + * how the provider named it. + * + * Last-resort normalizer for #9330: providers routed through + * `genericQuotaFetcher.convertUsageToQuotaInfo` key their `windows` map by + * MODEL ID (Antigravity: "gemini-3-flash", "claude-sonnet-5", …), so none of + * the canonical "weekly" | "session" | "monthly" lookups match. Without this + * those accounts resolved to `Infinity` ("never resets") and were sorted behind + * a Codex account whose secondary window was 26 days out. + */ +function getEarliestWindowResetMs(quota: unknown): number { + let earliest = Infinity; + for (const { window } of getQuotaWindowEntries(quota)) { + const resetMs = parseResetTimeMs(window.resetAt); + if (Number.isFinite(resetMs)) earliest = Math.min(earliest, resetMs); + } + return earliest; } function getResetUrgency(resetAt: string | null | undefined, windowMs: number): number { @@ -276,6 +343,23 @@ export function scoreResetAwareQuota( return { score }; } +/** + * Absolute epoch-ms instant at which the configured quota window next resets, + * or `Infinity` when the snapshot exposes no parseable reset (which sorts the + * target last under the `reset-window` strategy). + * + * Resolution order — each step only runs when the previous one found nothing: + * 1. the configured windows, by canonical name (structural `window5h` / + * `window7d` / `windowWeekly` / `windowMonthly` fields, then a `windows` + * map keyed by "weekly" | "session" | "monthly"); + * 2. the earliest reset across every entry of the `windows` map, whatever the + * provider named them (Antigravity keys its map by model id — #9330); + * 3. the single-signal top-level `quota.resetAt`. + * + * Step 2 sits ahead of step 3 deliberately: `quota.resetAt` is populated from + * the most-USED window, which is not necessarily the one resetting soonest, and + * is left null entirely while every window is still at 0% used. + */ export function getResetWindowTimestampMs(quota: unknown, windows: ResetWindowName[]): number { if (!quota || !isRecord(quota) || quota.limitReached === true) return Infinity; @@ -288,6 +372,10 @@ export function getResetWindowTimestampMs(quota: unknown, windows: ResetWindowNa } } + if (!Number.isFinite(selectedResetMs)) { + selectedResetMs = getEarliestWindowResetMs(quota); + } + if (!Number.isFinite(selectedResetMs)) { selectedResetMs = parseResetTimeMs(normalizeResetAt(quota.resetAt)); } @@ -295,6 +383,26 @@ export function getResetWindowTimestampMs(quota: unknown, windows: ResetWindowNa return Number.isFinite(selectedResetMs) ? selectedResetMs : Infinity; } +/** + * Milliseconds remaining until the configured window resets — the uniform + * metric the `reset-window` strategy sorts on (ascending: soonest first). + * + * Normalizing to a duration (rather than comparing raw epoch timestamps) keeps + * every provider on one scale and collapses already-elapsed resets to 0, so a + * snapshot that is stale by three days ties with one that reset a second ago + * instead of jumping the queue by virtue of being older. `Infinity` means "no + * known reset" and sorts last. + */ +export function getResetWindowRemainingMs( + quota: unknown, + windows: ResetWindowName[], + now: number = Date.now() +): number { + const resetMs = getResetWindowTimestampMs(quota, windows); + if (!Number.isFinite(resetMs)) return Infinity; + return Math.max(0, resetMs - now); +} + function getResetWindowHorizonMs(windows: ResetWindowName[]): number { if (windows.includes("monthly")) return 30 * 24 * 60 * 60 * 1000; if (windows.includes("weekly")) return RESET_AWARE_WEEKLY_WINDOW_MS; diff --git a/open-sse/services/combo/quotaStrategies.ts b/open-sse/services/combo/quotaStrategies.ts index cff82c1369..4234b29433 100644 --- a/open-sse/services/combo/quotaStrategies.ts +++ b/open-sse/services/combo/quotaStrategies.ts @@ -41,7 +41,7 @@ import { resolveResetWindowConfig, getResetAwareProvider, scoreResetAwareQuota, - getResetWindowTimestampMs, + getResetWindowRemainingMs, type QuotaFetchCacheConfig, } from "./quotaScoring.ts"; import { rankByHeadroom, type HeadroomSaturation } from "./headroomRanking.ts"; @@ -536,27 +536,35 @@ export async function orderTargetsByResetWindow( apiKeyAllowedConnectionIds ); + // One `now` snapshot for the whole ranking: quota fetches run concurrently and + // can take seconds, so re-reading the clock per target would compare remaining + // times measured against different instants (#9330). + const now = Date.now(); const scoredTargets = await scoreQuotaAwareTargets({ comboName, config, connectionById, expandedTargets, log, - scoreQuota: (quota) => ({ resetMs: getResetWindowTimestampMs(quota, config.windows) }), + scoreQuota: (quota) => ({ + remainingMs: getResetWindowRemainingMs(quota, config.windows, now), + }), }); + // Ascending: the account whose quota resets SOONEST goes first. Targets with + // no known reset (Infinity) fall to the back, ordered by combo priority. scoredTargets.sort((a, b) => { - if (a.resetMs !== b.resetMs) return a.resetMs - b.resetMs; + if (a.remainingMs !== b.remainingMs) return a.remainingMs - b.remainingMs; return a.index - b.index; }); - const bestResetMs = scoredTargets[0]?.resetMs ?? Infinity; - if (!Number.isFinite(bestResetMs) || config.tieBandMs <= 0) { + const bestRemainingMs = scoredTargets[0]?.remainingMs ?? Infinity; + if (!Number.isFinite(bestRemainingMs) || config.tieBandMs <= 0) { return scoredTargets.map((entry) => entry.target); } const tiedTargets = scoredTargets.filter( - (entry) => entry.resetMs - bestResetMs <= config.tieBandMs + (entry) => entry.remainingMs - bestRemainingMs <= config.tieBandMs ); if (tiedTargets.length <= 1) return scoredTargets.map((entry) => entry.target); diff --git a/tests/unit/combo/reset-window-strategy-9330.test.ts b/tests/unit/combo/reset-window-strategy-9330.test.ts new file mode 100644 index 0000000000..30bfcd43d0 --- /dev/null +++ b/tests/unit/combo/reset-window-strategy-9330.test.ts @@ -0,0 +1,239 @@ +/** + * Regression suite for issue #9330 — "reset-window strategy is not working properly". + * + * Reported scenario: a combo of Claude Sonnet 5 + Gemini 3.6 Flash (Antigravity, + * weekly windows, < 7 days to reset) + GPT-5.5 Medium (Codex free tier, 26 days + * to reset) under the `reset-window` strategy kept dispatching to the 26-day + * Codex account instead of the accounts resetting soonest. + * + * Root cause: `getResetWindowTimestampMs` only recognised a reset instant when + * the quota snapshot exposed a *canonically named* window (`window7d` / + * `windowWeekly` / `windowMonthly` / `window5h`, or a `windows` map keyed by + * "weekly" | "session" | "monthly"). Antigravity's snapshot comes from + * `genericQuotaFetcher.convertUsageToQuotaInfo`, whose `windows` map is keyed by + * MODEL ID ("gemini-3-flash", "claude-sonnet-5", ...). No key matched, so the + * helper fell through to the single-signal `quota.resetAt` — which + * `convertUsageToQuotaInfo` only populates from the *most-used* window and + * leaves `null` when every window is still at 0% used. Those accounts therefore + * scored `Infinity` (== "never resets") and were sorted BEHIND the Codex account + * whose `window7d` did carry a parseable 26-day reset. + * + * The fix normalises every provider shape to a comparable "milliseconds until + * reset" scalar and sorts ascending. + */ + +import test, { after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-reset-window-9330-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; + +const dbCore = await import("../../../src/lib/db/core.ts"); +const { getResetWindowRemainingMs, getResetWindowTimestampMs, resolveResetWindowConfig } = + await import("../../../open-sse/services/combo/quotaScoring.ts"); +const { orderTargetsByResetWindow } = + await import("../../../open-sse/services/combo/quotaStrategies.ts"); +const { registerQuotaFetcher } = await import("../../../open-sse/services/quotaPreflight.ts"); + +after(() => { + dbCore.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) { + delete process.env.DATA_DIR; + } else { + process.env.DATA_DIR = ORIGINAL_DATA_DIR; + } +}); + +const DAY_MS = 24 * 60 * 60 * 1000; +const HOUR_MS = 60 * 60 * 1000; +const NOW = Date.now(); +const iso = (offsetMs: number) => new Date(NOW + offsetMs).toISOString(); + +const DEFAULT_CONFIG = resolveResetWindowConfig({}); + +/** + * Codex free tier, as reshaped by `codexQuotaFetcher.fetchCodexQuota`: the + * "secondary" window is always surfaced under the `window7d` / "weekly" names + * regardless of its real duration, so a free-tier monthly limit shows up here + * as a 26-day weekly window (exactly what #9330 reported). + */ +const codexQuota26Days = { + used: 30, + total: 100, + percentUsed: 0.3, + resetAt: iso(26 * DAY_MS), + window5h: { percentUsed: 0.1, resetAt: iso(3 * HOUR_MS) }, + window7d: { percentUsed: 0.3, resetAt: iso(26 * DAY_MS) }, + windows: { + session: { percentUsed: 0.1, resetAt: iso(3 * HOUR_MS) }, + weekly: { percentUsed: 0.3, resetAt: iso(26 * DAY_MS) }, + }, + limitReached: false, +}; + +/** + * Antigravity, as reshaped by `genericQuotaFetcher.convertUsageToQuotaInfo`: + * `windows` is keyed by MODEL ID, and `resetAt` stays null while every window + * is still at 0% used. + */ +const antigravityQuotaFresh = { + used: 0, + total: 0, + percentUsed: 0, + resetAt: null, + windows: { + "gemini-3-flash": { percentUsed: 0, resetAt: iso(5 * DAY_MS) }, + "claude-sonnet-5": { percentUsed: 0, resetAt: iso(6 * DAY_MS) }, + }, + limitReached: false, +}; + +test("#9330 model-keyed quota windows resolve to a finite reset instead of Infinity", () => { + const resetMs = getResetWindowTimestampMs(antigravityQuotaFresh, DEFAULT_CONFIG.windows); + + assert.equal( + Number.isFinite(resetMs), + true, + "an Antigravity snapshot whose windows are keyed by model id must still yield a reset " + + "instant — returning Infinity is what demoted those accounts behind the 26-day Codex one" + ); + assert.equal( + Math.round((resetMs - NOW) / DAY_MS), + 5, + "the EARLIEST of the per-model windows (5 days) must win" + ); +}); + +test("#9330 remaining-time normalization ranks a 5-day reset ahead of a 26-day reset", () => { + const antigravity = getResetWindowRemainingMs(antigravityQuotaFresh, DEFAULT_CONFIG.windows, NOW); + const codex = getResetWindowRemainingMs(codexQuota26Days, DEFAULT_CONFIG.windows, NOW); + + assert.equal(Math.round(antigravity / DAY_MS), 5); + assert.equal(Math.round(codex / DAY_MS), 26); + assert.equal( + antigravity < codex, + true, + "the weekly-window account must sort before the 26-day one" + ); +}); + +test("#9330 an already-elapsed reset normalizes to 0 remaining rather than a negative age", () => { + const stale = { percentUsed: 0.5, window7d: { percentUsed: 0.5, resetAt: iso(-3 * DAY_MS) } }; + const justElapsed = { percentUsed: 0.5, window7d: { percentUsed: 0.5, resetAt: iso(-1000) } }; + + assert.equal(getResetWindowRemainingMs(stale, DEFAULT_CONFIG.windows, NOW), 0); + assert.equal(getResetWindowRemainingMs(justElapsed, DEFAULT_CONFIG.windows, NOW), 0); +}); + +test("#9330 the earliest window wins over the most-used window", () => { + // `convertUsageToQuotaInfo` sets the top-level resetAt from the most-USED + // window (6 days here), which is not necessarily the one resetting soonest. + const quota = { + percentUsed: 0.4, + resetAt: iso(6 * DAY_MS), + windows: { + "gemini-3-flash": { percentUsed: 0.1, resetAt: iso(2 * DAY_MS) }, + "claude-sonnet-5": { percentUsed: 0.4, resetAt: iso(6 * DAY_MS) }, + }, + }; + + assert.equal( + Math.round((getResetWindowTimestampMs(quota, DEFAULT_CONFIG.windows) - NOW) / DAY_MS), + 2 + ); +}); + +test("#9330 a named window without a resetAt does not shadow a sibling that has one", () => { + const quota = { + percentUsed: 0.5, + // window7d is structurally present but carries no reset instant; the + // windowWeekly sibling does. The `a || b` short-circuit used to pick the + // resetAt-less window7d and report Infinity. + window7d: { percentUsed: 0.5, resetAt: null }, + windowWeekly: { percentUsed: 0.5, resetAt: iso(2 * DAY_MS) }, + }; + + assert.equal( + Math.round((getResetWindowTimestampMs(quota, DEFAULT_CONFIG.windows) - NOW) / DAY_MS), + 2 + ); +}); + +test("#9330 exhausted (limitReached) accounts stay demoted to Infinity", () => { + assert.equal( + getResetWindowTimestampMs( + { ...antigravityQuotaFresh, limitReached: true }, + DEFAULT_CONFIG.windows + ), + Infinity + ); + assert.equal(getResetWindowTimestampMs(null, DEFAULT_CONFIG.windows), Infinity); + assert.equal( + getResetWindowRemainingMs({ percentUsed: 0.1 }, DEFAULT_CONFIG.windows, NOW), + Infinity + ); +}); + +test("#9330 canonically named windows keep their existing resolution (no regression)", () => { + assert.equal( + Math.round( + (getResetWindowTimestampMs(codexQuota26Days, DEFAULT_CONFIG.windows) - NOW) / DAY_MS + ), + 26, + "config windows = ['weekly'] must still read window7d, not the 3h session window" + ); + + const withSession = resolveResetWindowConfig({ resetWindowIncludeSession: true }); + assert.equal( + Math.round((getResetWindowTimestampMs(codexQuota26Days, withSession.windows) - NOW) / HOUR_MS), + 3, + "opting session in must still pull the 5h window forward" + ); +}); + +test("#9330 orderTargetsByResetWindow dispatches the soonest-resetting account first", async () => { + const antigravity = `agy-9330-${randomUUID()}`; + const codex = `codex-9330-${randomUUID()}`; + const antigravityConnection = `agy-conn-${randomUUID()}`; + const codexConnection = `codex-conn-${randomUUID()}`; + + registerQuotaFetcher(antigravity, async () => antigravityQuotaFresh); + registerQuotaFetcher(codex, async () => codexQuota26Days); + + const target = (provider: string, connectionId: string, stepId: string) => ({ + kind: "model" as const, + stepId, + executionKey: `${stepId}@${connectionId}`, + modelStr: `${provider}/model`, + provider, + providerId: provider, + connectionId, + weight: 1, + label: null, + }); + + // Codex is FIRST in the combo definition — exactly the reported layout. + const ordered = await orderTargetsByResetWindow( + [ + target(codex, codexConnection, "gpt-5.5-medium"), + target(antigravity, antigravityConnection, "claude-sonnet-5"), + ], + `reset-window-9330-${randomUUID()}`, + {}, + { warn: () => {} }, + null + ); + + assert.equal( + ordered[0]?.provider, + antigravity, + "the Antigravity account (~5 days to reset) must be dispatched before the Codex account " + + "(~26 days to reset), despite Codex being first in the combo definition" + ); +});