From 4a43761b806e94ece913d59e0fb17a1ee6ac758e Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 04:10:36 -0400 Subject: [PATCH 1/8] fix(forecast): gate availability on the requested model's family evaluateForecastAccount checked per-family rate-limit records with a hardwired "codex" family, so the forecast's --model never reached the record check and the runtime-overlay staleness cross-check inherited the same family. A forecast for a general-family model reported an account ready while its active record had the runtime proxy refusing every request for that family, and the persisted rate-limited overlay reason backed by that record was judged stale against the codex family and dropped. ForecastAccountInput gains an optional family (default codex, so model-less surfaces keep their exact behavior); forecast, best, and report resolve it from their model via getModelProfile. The staleness cross-check becomes family-aware through the same value. --- lib/codex-manager/commands/best.ts | 6 +- lib/codex-manager/commands/forecast.ts | 7 ++- lib/codex-manager/commands/report.ts | 1 + lib/forecast.ts | 14 ++++- test/forecast.test.ts | 78 ++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 4 deletions(-) diff --git a/lib/codex-manager/commands/best.ts b/lib/codex-manager/commands/best.ts index ddba0d7ec..56efa44ce 100644 --- a/lib/codex-manager/commands/best.ts +++ b/lib/codex-manager/commands/best.ts @@ -1,6 +1,9 @@ import type { ForecastAccountResult } from "../../forecast.js"; import { type CodexQuotaSnapshot, describeCodexProbeFailure } from "../../quota-probe.js"; -import { resolveNormalizedModel } from "../../request/helpers/model-map.js"; +import { + getModelProfile, + resolveNormalizedModel, +} from "../../request/helpers/model-map.js"; import type { AccountStorageV3 } from "../../storage.js"; import type { TokenFailure, TokenResult } from "../../types.js"; import { DEFAULT_LIVE_PROBE_MODEL } from "../quota-cache-helpers.js"; @@ -289,6 +292,7 @@ export async function runBestCommand( now, refreshFailure: refreshFailures.get(index), liveQuota: liveQuotaByIndex.get(index), + family: getModelProfile(probeModel).promptFamily, })); const forecastResults = deps.evaluateForecastAccounts(forecastInputs); const recommendation = deps.recommendForecastAccount(forecastResults); diff --git a/lib/codex-manager/commands/forecast.ts b/lib/codex-manager/commands/forecast.ts index f454cb03e..c448d964b 100644 --- a/lib/codex-manager/commands/forecast.ts +++ b/lib/codex-manager/commands/forecast.ts @@ -14,7 +14,11 @@ import { } from "../forecast-report-shared.js"; import type { QuotaCacheData } from "../../quota-cache.js"; import { type CodexQuotaSnapshot, describeCodexProbeFailure } from "../../quota-probe.js"; -import { DEFAULT_PROBE_MODEL, resolveNormalizedModel } from "../../request/helpers/model-map.js"; +import { + DEFAULT_PROBE_MODEL, + getModelProfile, + resolveNormalizedModel, +} from "../../request/helpers/model-map.js"; import { type AccountMetadataV3, type AccountStorageV3 } from "../../storage.js"; import type { TokenFailure, TokenResult } from "../../types.js"; @@ -368,6 +372,7 @@ export async function runForecastCommand( quotaCache, allAccounts: storage.accounts, runtimeOverlay, + family: getModelProfile(requestedModel).promptFamily, })); const forecastResults = deps.evaluateForecastAccounts(forecastInputs); const summary = deps.summarizeForecast(forecastResults); diff --git a/lib/codex-manager/commands/report.ts b/lib/codex-manager/commands/report.ts index 0ad7d6ff5..dc9ce7141 100644 --- a/lib/codex-manager/commands/report.ts +++ b/lib/codex-manager/commands/report.ts @@ -474,6 +474,7 @@ export async function runReportCommand( quotaCache, allAccounts: storage.accounts, runtimeOverlay: runtimeSnapshot, + family: getModelProfile(modelInspection.normalized).promptFamily, })), ) : []; diff --git a/lib/forecast.ts b/lib/forecast.ts index 3e55f3e5f..c19089836 100644 --- a/lib/forecast.ts +++ b/lib/forecast.ts @@ -10,6 +10,7 @@ import { isQuotaCacheEntryExhausted, quotaUsedPercentIsExhausted, } from "./quota-readiness.js"; +import type { ModelFamily } from "./request/helpers/model-map.js"; import { getRateLimitResetTimeForFamily } from "./runtime/account-status.js"; import type { AccountMetadataV3 } from "./storage.js"; import type { TokenFailure } from "./types.js"; @@ -27,6 +28,12 @@ export interface ForecastAccountInput { quotaCache?: QuotaCacheData | null; allAccounts?: readonly AccountMetadataV3[]; runtimeOverlay?: RuntimeForecastOverlay | null; + /** + * Prompt family whose per-family rate-limit records gate this forecast. + * Callers with a model in hand resolve it via getModelProfile; the codex + * default preserves the historical behavior for model-less surfaces. + */ + family?: ModelFamily; } export interface RuntimeForecastOverlay { @@ -245,7 +252,7 @@ export function evaluateForecastAccount( const rateLimitResetAt = getRateLimitResetTimeForFamily( account, now, - "codex", + input.family ?? "codex", ); if (typeof rateLimitResetAt === "number") { const remaining = Math.max(0, rateLimitResetAt - now); @@ -298,7 +305,10 @@ export function evaluateForecastAccount( // drop the overlay reason when the condition it describes is no longer // active. Each reason validates only against its own backing disk state // ("rate-limited" -> rateLimitResetTimes, "cooling-down" -> coolingDownUntil) - // so we never substitute a misleading reason string. Non-time-bounded + // so we never substitute a misleading reason string. The rate-limited + // cross-check is family-aware through rateLimitResetAt above: a record for + // the forecast's own family keeps the reason, while a record for another + // family neither sustains it nor gates this model's availability. Non-time-bounded // reasons ("circuit-open", "token-exhausted", "policy-blocked") have no disk // expiry to check and are always applied. const coolingDownActive = diff --git a/test/forecast.test.ts b/test/forecast.test.ts index 4d858f2f3..6ba3f263f 100644 --- a/test/forecast.test.ts +++ b/test/forecast.test.ts @@ -387,6 +387,84 @@ describe("forecast helpers", () => { expect(result.reasons).toContain("runtime skip: rate-limited"); }); + it("gates availability on the requested family's record, not the codex family", () => { + const now = 1_700_000_000_000; + const account = { + refreshToken: "refresh-1", + addedAt: now - 10_000, + lastUsed: now - 10_000, + rateLimitResetTimes: { "gpt-5.2": now + 30_000 }, + }; + + const general = evaluateForecastAccount({ + index: 0, + now, + isCurrent: false, + account, + family: "gpt-5.2", + }); + expect(general.availability).toBe("delayed"); + expect(general.waitMs).toBe(30_000); + expect( + general.reasons.some((reason) => reason.startsWith("rate limit resets in")), + ).toBe(true); + + const codex = evaluateForecastAccount({ + index: 0, + now, + isCurrent: false, + account, + family: "codex", + }); + expect(codex.availability).toBe("ready"); + }); + + it("keeps a rate-limited overlay alive when the record matches the requested family", () => { + const now = 1_700_000_000_000; + const result = evaluateForecastAccount({ + index: 0, + now, + isCurrent: false, + account: { + refreshToken: "refresh-1", + addedAt: now - 10_000, + lastUsed: now - 10_000, + rateLimitResetTimes: { "gpt-5.2": now + 30_000 }, + }, + runtimeOverlay: { + lastPoolExhaustionSkipReasons: { "0": "rate-limited" }, + }, + family: "gpt-5.2", + }); + + // Before family threading this overlay was cross-checked against the + // codex family, judged stale, and dropped - the account read "ready" + // while the runtime proxy refused every request for the family. + expect(result.availability).toBe("unavailable"); + expect(result.reasons).toContain("runtime skip: rate-limited"); + }); + + it("drops a rate-limited overlay backed only by another family's record", () => { + const now = 1_700_000_000_000; + const result = evaluateForecastAccount({ + index: 0, + now, + isCurrent: false, + account: { + refreshToken: "refresh-1", + addedAt: now - 10_000, + lastUsed: now - 10_000, + rateLimitResetTimes: { "gpt-5.2": now + 30_000 }, + }, + runtimeOverlay: { + lastPoolExhaustionSkipReasons: { "0": "rate-limited" }, + }, + family: "codex", + }); + + expect(result.availability).toBe("ready"); + }); + it("ignores a stale cooling-down overlay when cooldown has elapsed on disk", () => { const now = 1_700_000_000_000; const result = evaluateForecastAccount({ From fec9988911e856f7c4505d8e975a6b1392293bd7 Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 16:08:22 -0400 Subject: [PATCH 2/8] test(forecast): assert the family reaches evaluation, and name it in the deps contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: the injected evaluateForecastAccounts contracts in best and forecast declared their input shape inline without the new family field, so a typed test fake could silently drop it. Both contracts now name family?: ModelFamily. Command-level coverage asserts the resolved family reaches evaluation: best and forecast capture the injected evaluator's inputs (default and explicit --model), and report — which calls the real evaluator — proves it end to end with a gpt-5.2 record that delays a gpt-5.6-sol report and leaves a gpt-5.3-codex report ready. --- lib/codex-manager/commands/best.ts | 2 + lib/codex-manager/commands/forecast.ts | 2 + test/codex-manager-best-command.test.ts | 60 +++++++++++++++++++++ test/codex-manager-forecast-command.test.ts | 40 +++++++++++++- test/codex-manager-report-command.test.ts | 51 ++++++++++++++++++ 5 files changed, 154 insertions(+), 1 deletion(-) diff --git a/lib/codex-manager/commands/best.ts b/lib/codex-manager/commands/best.ts index 56efa44ce..10c4a5301 100644 --- a/lib/codex-manager/commands/best.ts +++ b/lib/codex-manager/commands/best.ts @@ -2,6 +2,7 @@ import type { ForecastAccountResult } from "../../forecast.js"; import { type CodexQuotaSnapshot, describeCodexProbeFailure } from "../../quota-probe.js"; import { getModelProfile, + type ModelFamily, resolveNormalizedModel, } from "../../request/helpers/model-map.js"; import type { AccountStorageV3 } from "../../storage.js"; @@ -126,6 +127,7 @@ export interface BestCommandDeps { now: number; refreshFailure?: TokenFailure; liveQuota?: CodexQuotaSnapshot; + family?: ModelFamily; }>, ) => ForecastAccountResult[]; recommendForecastAccount: (results: ForecastAccountResult[]) => { diff --git a/lib/codex-manager/commands/forecast.ts b/lib/codex-manager/commands/forecast.ts index c448d964b..3bd0ec286 100644 --- a/lib/codex-manager/commands/forecast.ts +++ b/lib/codex-manager/commands/forecast.ts @@ -17,6 +17,7 @@ import { type CodexQuotaSnapshot, describeCodexProbeFailure } from "../../quota- import { DEFAULT_PROBE_MODEL, getModelProfile, + type ModelFamily, resolveNormalizedModel, } from "../../request/helpers/model-map.js"; import { type AccountMetadataV3, type AccountStorageV3 } from "../../storage.js"; @@ -89,6 +90,7 @@ export interface ForecastCommandDeps { quotaCache?: QuotaCacheData | null; allAccounts?: readonly AccountMetadataV3[]; runtimeOverlay?: RuntimeForecastOverlay | null; + family?: ModelFamily; }>, ) => ForecastAccountResult[]; summarizeForecast: (results: ForecastAccountResult[]) => { diff --git a/test/codex-manager-best-command.test.ts b/test/codex-manager-best-command.test.ts index 4f8307e13..a6b702be2 100644 --- a/test/codex-manager-best-command.test.ts +++ b/test/codex-manager-best-command.test.ts @@ -6,6 +6,8 @@ import { } from "../lib/codex-manager/commands/best.js"; import { CodexUnavailableError } from "../lib/errors.js"; import { CODEX_UNAVAILABLE_PROBE_NOTE } from "../lib/quota-probe.js"; +import { DEFAULT_LIVE_PROBE_MODEL } from "../lib/codex-manager/quota-cache-helpers.js"; +import { getModelProfile } from "../lib/request/helpers/model-map.js"; import type { AccountStorageV3 } from "../lib/storage.js"; function createAccount( @@ -137,6 +139,64 @@ describe("runBestCommand", () => { ); }); + it("threads the probe model's family into forecast evaluation", async () => { + const evaluateForecastAccounts = vi.fn((inputs) => { + void inputs; + return [ + { + index: 0, + label: "1. best@example.com", + isCurrent: true, + availability: "ready", + riskScore: 0, + riskLevel: "low", + waitMs: 0, + reasons: [], + }, + ] as const; + }); + const deps = createDeps({ + evaluateForecastAccounts, + parseBestArgs: vi.fn(() => ({ + ok: true as const, + options: { + live: false, + json: true, + model: DEFAULT_LIVE_PROBE_MODEL, + modelProvided: false, + } satisfies BestCliOptions, + })), + }); + + await expect(runBestCommand(["--json"], deps)).resolves.toBe(0); + const defaulted = evaluateForecastAccounts.mock.calls.at(-1)?.[0] as + | Array<{ family?: string }> + | undefined; + expect(defaulted?.[0]?.family).toBe( + getModelProfile(DEFAULT_LIVE_PROBE_MODEL).promptFamily, + ); + + const explicitDeps = createDeps({ + evaluateForecastAccounts, + parseBestArgs: vi.fn(() => ({ + ok: true as const, + options: { + live: true, + json: true, + model: "gpt-5.6-sol", + modelProvided: true, + } satisfies BestCliOptions, + })), + }); + await expect( + runBestCommand(["--json", "--live", "--model", "gpt-5.6-sol"], explicitDeps), + ).resolves.toBe(0); + const explicit = evaluateForecastAccounts.mock.calls.at(-1)?.[0] as + | Array<{ family?: string }> + | undefined; + expect(explicit?.[0]?.family).toBe(getModelProfile("gpt-5.6-sol").promptFamily); + }); + it("emits json output when no accounts are configured", async () => { const deps = createDeps({ loadAccounts: vi.fn(async () => ({ diff --git a/test/codex-manager-forecast-command.test.ts b/test/codex-manager-forecast-command.test.ts index 9846e773d..ac83590df 100644 --- a/test/codex-manager-forecast-command.test.ts +++ b/test/codex-manager-forecast-command.test.ts @@ -5,7 +5,10 @@ import { } from "../lib/codex-manager/commands/forecast.js"; import { CodexUnavailableError } from "../lib/errors.js"; import { CODEX_UNAVAILABLE_PROBE_NOTE } from "../lib/quota-probe.js"; -import { DEFAULT_PROBE_MODEL } from "../lib/request/helpers/model-map.js"; +import { + DEFAULT_PROBE_MODEL, + getModelProfile, +} from "../lib/request/helpers/model-map.js"; import type { AccountStorageV3 } from "../lib/storage.js"; function createStorage(): AccountStorageV3 { @@ -169,6 +172,41 @@ describe("runForecastCommand", () => { ); }); + it("threads the requested model's family into forecast evaluation", async () => { + const evaluateForecastAccounts = vi.fn((inputs) => { + void inputs; + return [ + { + index: 0, + label: "1. forecast@example.com", + isCurrent: true, + availability: "ready", + riskScore: 0, + riskLevel: "low", + waitMs: 0, + reasons: [], + }, + ] as const; + }); + const deps = createDeps({ evaluateForecastAccounts }); + + await expect( + runForecastCommand(["--json", "--model", "gpt-5.6-sol"], deps), + ).resolves.toBe(0); + const explicit = evaluateForecastAccounts.mock.calls.at(-1)?.[0] as + | Array<{ family?: string }> + | undefined; + expect(explicit?.[0]?.family).toBe(getModelProfile("gpt-5.6-sol").promptFamily); + + await expect(runForecastCommand(["--json"], deps)).resolves.toBe(0); + const defaulted = evaluateForecastAccounts.mock.calls.at(-1)?.[0] as + | Array<{ family?: string }> + | undefined; + expect(defaulted?.[0]?.family).toBe( + getModelProfile(DEFAULT_PROBE_MODEL).promptFamily, + ); + }); + it("honors --no-runtime-overlay in json forecast output", async () => { const evaluateForecastAccounts = vi.fn((inputs) => { const overlay = inputs[0]?.runtimeOverlay as diff --git a/test/codex-manager-report-command.test.ts b/test/codex-manager-report-command.test.ts index 3df81a704..f39df66e5 100644 --- a/test/codex-manager-report-command.test.ts +++ b/test/codex-manager-report-command.test.ts @@ -93,6 +93,57 @@ describe("runReportCommand", () => { expect(deps.logError).toHaveBeenCalledWith("Unknown option: --bogus"); }); + it("gates the forecast on the requested model's family record", async () => { + const storage = createStorage([ + { + email: "one@example.com", + refreshToken: "refresh-token-1", + accessToken: "access-token-1", + expiresAt: 10, + addedAt: 1, + lastUsed: 1, + enabled: true, + rateLimitResetTimes: { "gpt-5.2": 31_000 }, + }, + ]); + const deps = createDeps({ loadAccounts: vi.fn(async () => storage) }); + + const readForecast = (): { + accounts: Array<{ availability: string; reasons: string[] }>; + } => + ( + JSON.parse( + String( + (deps.logInfo as ReturnType).mock.calls.at(-1)?.[0] ?? + "{}", + ), + ) as { + forecast: { + accounts: Array<{ availability: string; reasons: string[] }>; + }; + } + ).forecast; + + // The record is under the gpt-5.2 family, which gpt-5.6-sol belongs to. + await expect( + runReportCommand(["--json", "--model", "gpt-5.6-sol"], deps), + ).resolves.toBe(0); + const general = readForecast(); + expect(general.accounts[0]?.availability).toBe("delayed"); + expect( + general.accounts[0]?.reasons.some((reason) => + reason.startsWith("rate limit resets in"), + ), + ).toBe(true); + + // A codex-family model is not gated by that record. + await expect( + runReportCommand(["--json", "--model", "gpt-5.3-codex"], deps), + ).resolves.toBe(0); + const codex = readForecast(); + expect(codex.accounts[0]?.availability).toBe("ready"); + }); + it("rejects a flag-like or whitespace-only --model value instead of consuming it", async () => { // Split-arg form trims before validating, so " -x" / " " can't slip // through and silently fall back to the default model. From 75d3fc9d1f699435e853f448c3db68261858b38e Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 16:03:53 -0400 Subject: [PATCH 3/8] fix(runtime): tell the pinned-503 truth about pin source and reset time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pinned-account 503 always advised `codex-multi-auth unpin`, but the pin honored there is state.forcedAccountIndex ?? the persisted switch pin — and for a forced pin (--account / CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX) unpin clears nothing, so the advice was wrong exactly where the message appears most: launcher-managed sessions that pin per invocation. The body also carried no recovery time even when the skip reason was a time-bounded record whose reset moment sat in the store. buildPinnedUnavailableErrorBody now takes optional context: pin_source ("forced" pins get a relaunch remedy instead of unpin), and reset_at/retry_after_ms threaded from the blocking record — the family's rate-limit record for a rate-limited skip, coolingDownUntil for a cooldown — with the message naming the reset moment when one is known. The proxy call site distinguishes the pin source it already tracks and resolves the reset for the request's family. Absent context, the body and message are byte-identical to before (the issue-474 expectations pass unchanged). --- lib/request/rate-limit-decision.ts | 42 +++++++++++++++++++++++++++++- lib/runtime-rotation-proxy.ts | 32 +++++++++++++++++++++++ test/rate-limit-decision.test.ts | 34 ++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/lib/request/rate-limit-decision.ts b/lib/request/rate-limit-decision.ts index 88cddb962..6f775e5f1 100644 --- a/lib/request/rate-limit-decision.ts +++ b/lib/request/rate-limit-decision.ts @@ -185,12 +185,31 @@ export interface PinnedUnavailableErrorBody { code: "codex_pinned_account_unavailable"; pinnedAccountIndex: number | null; reason: string | null; + /** How the pin was set; forced pins are not cleared by `unpin`. */ + pin_source: "forced" | "manual" | null; + /** When the blocking record ends, when the skip reason is time-bounded. */ + reset_at: string | null; + retry_after_ms: number | null; account_skip_reasons: Record; } +export interface PinnedUnavailableContext { + /** + * "forced" when the pin came from the wrapper's forced-account mode + * (`--account` / CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX), "manual" when it + * came from `switch`. `unpin` clears only the manual kind, so the remedy + * line must not suggest it for a forced pin. + */ + pinSource?: "forced" | "manual" | null; + /** Epoch ms when the blocking record ends (rate limit or cooldown). */ + resetAtMs?: number | null; + now?: number; +} + export function buildPinnedUnavailableErrorBody( pinnedIndex: number | null | undefined, accountSkipReasons: ReadonlyMap, + context?: PinnedUnavailableContext, ): PinnedUnavailableErrorBody { const normalizedPinnedIndex = typeof pinnedIndex === "number" ? pinnedIndex : null; @@ -205,11 +224,32 @@ export function buildPinnedUnavailableErrorBody( normalizedPinnedIndex === null ? "The pinned account" : `Pinned account ${normalizedPinnedIndex + 1}`; + const pinSource = context?.pinSource ?? null; + const resetAtMs = + typeof context?.resetAtMs === "number" && + Number.isFinite(context.resetAtMs) && + context.resetAtMs > 0 + ? context.resetAtMs + : null; + const now = context?.now ?? Date.now(); + const retryAfterMs = resetAtMs !== null ? Math.max(0, resetAtMs - now) : null; + const resetAt = resetAtMs !== null ? new Date(resetAtMs).toISOString() : null; + const waitSuffix = + resetAt !== null ? `; the recorded limit resets at ${resetAt}` : ""; + // A forced pin belongs to the launching process, not to ndy's persisted + // pin state, so `unpin` would clear nothing — say what actually helps. + const remedy = + pinSource === "forced" + ? "the pin was set by this session's launcher, so relaunch to select a different account" + : "run `codex-multi-auth status` for details, or `codex-multi-auth unpin` to allow rotation"; return { - message: `${accountPhrase} is currently unavailable${reasonSuffix}; run \`codex-multi-auth status\` for details, or \`codex-multi-auth unpin\` to allow rotation.`, + message: `${accountPhrase} is currently unavailable${reasonSuffix}${waitSuffix}; ${remedy}.`, code: "codex_pinned_account_unavailable", pinnedAccountIndex: normalizedPinnedIndex, reason: skipReason, + pin_source: pinSource, + reset_at: resetAt, + retry_after_ms: retryAfterMs, account_skip_reasons: Object.fromEntries( [...accountSkipReasons.entries()].map(([index, reason]) => [ String(index), diff --git a/lib/runtime-rotation-proxy.ts b/lib/runtime-rotation-proxy.ts index f447b985e..bc8cc0dca 100644 --- a/lib/runtime-rotation-proxy.ts +++ b/lib/runtime-rotation-proxy.ts @@ -72,6 +72,7 @@ import { responseHeadersForClient, withTimeout, } from "./request/stream-failover-runtime.js"; +import { getRateLimitResetTimeForFamily } from "./runtime/account-status.js"; import { chooseAccount } from "./runtime/rotation-account-selection.js"; import { createRotationProxyState, @@ -1573,9 +1574,40 @@ async function handleRequestInner( // null reason indicates a forecast/runtime state desync (the pinned // account was selected but no skip reason was recorded) — see #486. if (isPinned) { + const pinnedAccount = + typeof pinnedIndex === "number" + ? accountManager.getAccountByIndex(pinnedIndex) + : null; + const pinnedSkipReason = + typeof pinnedIndex === "number" + ? accountSkipReasons.get(pinnedIndex) ?? null + : null; + // A rate-limited skip is bounded by the family record and a cooldown + // by coolingDownUntil; anything else has no known recovery moment. + const pinnedResetAtMs = + pinnedAccount === null || pinnedSkipReason === null + ? null + : pinnedSkipReason === "rate-limited" + ? getRateLimitResetTimeForFamily( + pinnedAccount, + state.now(), + context.family, + ) + : pinnedSkipReason.startsWith("cooling-down") && + typeof pinnedAccount.coolingDownUntil === "number" && + pinnedAccount.coolingDownUntil > state.now() + ? pinnedAccount.coolingDownUntil + : null; const errorBody = buildPinnedUnavailableErrorBody( pinnedIndex, accountSkipReasons, + { + // typeof check so a forced index of 0 still reads as forced. + pinSource: + typeof state.forcedAccountIndex === "number" ? "forced" : "manual", + resetAtMs: pinnedResetAtMs, + now: state.now(), + }, ); if (errorBody.reason === null) { state.status.lastError = `pinned-503 missing skip reason (pinnedIndex=${pinnedIndex})`; diff --git a/test/rate-limit-decision.test.ts b/test/rate-limit-decision.test.ts index 40d6cf371..4410d3c42 100644 --- a/test/rate-limit-decision.test.ts +++ b/test/rate-limit-decision.test.ts @@ -295,5 +295,39 @@ describe("buildPinnedUnavailableErrorBody", () => { expect(body.message).not.toContain("Pinned account 1"); expect(body.message).not.toContain("("); expect(body.account_skip_reasons).toEqual({}); + expect(body.pin_source).toBeNull(); + expect(body.reset_at).toBeNull(); + expect(body.retry_after_ms).toBeNull(); + }); + + it("tailors the remedy to a forced pin and threads the recorded reset", () => { + const resetAtMs = 1_700_000_030_000; + const body = buildPinnedUnavailableErrorBody( + 0, + new Map([[0, "rate-limited"]]), + { pinSource: "forced", resetAtMs, now: 1_700_000_000_000 }, + ); + expect(body.pin_source).toBe("forced"); + expect(body.reset_at).toBe(new Date(resetAtMs).toISOString()); + expect(body.retry_after_ms).toBe(30_000); + expect(body.message).toContain( + `the recorded limit resets at ${new Date(resetAtMs).toISOString()}`, + ); + // A forced pin is not cleared by `unpin`; the remedy must not suggest it. + expect(body.message).toContain("set by this session's launcher"); + expect(body.message).not.toContain("unpin"); + }); + + it("keeps the unpin advice for manual pins and nulls an unknown reset", () => { + const body = buildPinnedUnavailableErrorBody( + 1, + new Map([[1, "disabled"]]), + { pinSource: "manual" }, + ); + expect(body.pin_source).toBe("manual"); + expect(body.reset_at).toBeNull(); + expect(body.retry_after_ms).toBeNull(); + expect(body.message).toContain("codex-multi-auth unpin"); + expect(body.message).not.toContain("resets at"); }); }); From 199cda59b140b7399836d8c0ee22d9c53842325f Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 16:32:38 -0400 Subject: [PATCH 4/8] fix(runtime): derive pinned-503 recovery from account state, not the skip reason Review follow-ups on both fronts of the recovery metadata: - A direct 429 or network error on the pinned account reaches the 503 with the retry loop's selection verdict (already-attempted) as its skip reason, so gating the reset lookup on rate-limited/cooling-down strings suppressed recovery info exactly where it was freshest. The reset now comes straight from the account's persisted state. - With several overlapping records for the family, the account stays skipped until the LAST one expires, so the earliest reset would send a client straight back into a 503. getAccountRecoveryTimeForFamily returns the latest matching bound (records plus active cooldown), leaving getRateLimitResetTimeForFamily's earliest-reset semantics to its wait-display callers. Covered by test/account-status.test.ts (helper semantics) and two runtime proxy regressions (test/runtime-rotation-proxy.test.ts) that force a pinned account through a direct 429 and a network-error cooldown and assert the 503 carries pin_source, reason, reset_at, and retry_after_ms. --- lib/runtime-rotation-proxy.ts | 33 +++++------ lib/runtime/account-status.ts | 34 ++++++++++++ test/account-status.test.ts | 54 ++++++++++++++++++ test/runtime-rotation-proxy.test.ts | 85 +++++++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 19 deletions(-) diff --git a/lib/runtime-rotation-proxy.ts b/lib/runtime-rotation-proxy.ts index bc8cc0dca..d7c56cc81 100644 --- a/lib/runtime-rotation-proxy.ts +++ b/lib/runtime-rotation-proxy.ts @@ -72,7 +72,7 @@ import { responseHeadersForClient, withTimeout, } from "./request/stream-failover-runtime.js"; -import { getRateLimitResetTimeForFamily } from "./runtime/account-status.js"; +import { getAccountRecoveryTimeForFamily } from "./runtime/account-status.js"; import { chooseAccount } from "./runtime/rotation-account-selection.js"; import { createRotationProxyState, @@ -1578,26 +1578,21 @@ async function handleRequestInner( typeof pinnedIndex === "number" ? accountManager.getAccountByIndex(pinnedIndex) : null; - const pinnedSkipReason = - typeof pinnedIndex === "number" - ? accountSkipReasons.get(pinnedIndex) ?? null - : null; - // A rate-limited skip is bounded by the family record and a cooldown - // by coolingDownUntil; anything else has no known recovery moment. + // Recovery comes from the pinned account's persisted state, not the + // recorded skip reason: a direct 429/cooldown on the pinned account + // reaches this 503 as "already-attempted" (the retry loop's selection + // verdict) while the record it just wrote is what actually bounds + // recovery — and with several overlapping records the account stays + // skipped until the LAST one expires, so the latest bound is the one + // worth advertising. const pinnedResetAtMs = - pinnedAccount === null || pinnedSkipReason === null + pinnedAccount === null ? null - : pinnedSkipReason === "rate-limited" - ? getRateLimitResetTimeForFamily( - pinnedAccount, - state.now(), - context.family, - ) - : pinnedSkipReason.startsWith("cooling-down") && - typeof pinnedAccount.coolingDownUntil === "number" && - pinnedAccount.coolingDownUntil > state.now() - ? pinnedAccount.coolingDownUntil - : null; + : getAccountRecoveryTimeForFamily( + pinnedAccount, + state.now(), + context.family, + ); const errorBody = buildPinnedUnavailableErrorBody( pinnedIndex, accountSkipReasons, diff --git a/lib/runtime/account-status.ts b/lib/runtime/account-status.ts index 267cf663e..0f72e8ed4 100644 --- a/lib/runtime/account-status.ts +++ b/lib/runtime/account-status.ts @@ -38,6 +38,40 @@ export function getRateLimitResetTimeForFamily( return minReset; } +/** + * The moment the account becomes usable again for `family`: the LATEST + * matching rate-limit record plus any active cooldown. Distinct from + * getRateLimitResetTimeForFamily, whose earliest-reset answer feeds wait + * displays: the account stays skipped while ANY matching record is active, + * so a retry hint built from the earliest reset would send clients back + * into a 503. Null when nothing bounds recovery. + */ +export function getAccountRecoveryTimeForFamily( + account: { + rateLimitResetTimes?: Record; + coolingDownUntil?: number; + }, + now: number, + family: ModelFamily, +): number | null { + let latest: number | null = null; + const consider = (value: number | undefined): void => { + if (typeof value !== "number" || !Number.isFinite(value)) return; + if (value <= now) return; + if (latest === null || value > latest) latest = value; + }; + const times = account.rateLimitResetTimes; + if (times) { + const prefix = `${family}:`; + for (const [key, value] of Object.entries(times)) { + if (key !== family && !key.startsWith(prefix)) continue; + consider(value); + } + } + consider(account.coolingDownUntil); + return latest; +} + export function formatRateLimitEntry( account: { rateLimitResetTimes?: Record }, now: number, diff --git a/test/account-status.test.ts b/test/account-status.test.ts index b4313d24b..7ad30f753 100644 --- a/test/account-status.test.ts +++ b/test/account-status.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { formatRateLimitEntry, + getAccountRecoveryTimeForFamily, getRateLimitResetTimeForFamily, resolveActiveIndex, } from "../lib/runtime/account-status.js"; @@ -101,3 +102,56 @@ describe("account status helpers", () => { ).toBe("resets in 4000ms"); }); }); + +describe("getAccountRecoveryTimeForFamily", () => { + it("returns the LATEST matching reset so a retry lands after real recovery", () => { + // Two overlapping records for the family: the account stays skipped + // until the last one expires, so the earliest reset would send a + // client straight back into a 503. + expect( + getAccountRecoveryTimeForFamily( + { rateLimitResetTimes: { codex: 3_000, "codex:5h": 9_000 } }, + 1_000, + "codex", + ), + ).toBe(9_000); + }); + + it("ignores other families and expired records", () => { + expect( + getAccountRecoveryTimeForFamily( + { rateLimitResetTimes: { "gpt-5.2": 9_000, codex: 500 } }, + 1_000, + "codex", + ), + ).toBeNull(); + }); + + it("folds an active cooldown into the recovery moment", () => { + expect( + getAccountRecoveryTimeForFamily( + { rateLimitResetTimes: { codex: 3_000 }, coolingDownUntil: 7_000 }, + 1_000, + "codex", + ), + ).toBe(7_000); + expect( + getAccountRecoveryTimeForFamily( + { coolingDownUntil: 2_000 }, + 1_000, + "codex", + ), + ).toBe(2_000); + }); + + it("returns null when nothing bounds recovery", () => { + expect(getAccountRecoveryTimeForFamily({}, 1_000, "codex")).toBeNull(); + expect( + getAccountRecoveryTimeForFamily( + { coolingDownUntil: 900 }, + 1_000, + "codex", + ), + ).toBeNull(); + }); +}); diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index a892bf923..83c007519 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -799,6 +799,91 @@ describe("runtime rotation proxy", () => { expect(calls).toHaveLength(0); }); + it("carries pin source and recovery metadata when the forced account 429s directly", async () => { + const now = Date.now(); + const accountManager = new AccountManager(undefined, createStorage(now, 2)); + const { calls, fetchImpl } = createRecordingFetch( + () => + new Response('{"error":{"message":"rate limited"}}', { + status: HTTP_STATUS.TOO_MANY_REQUESTS, + headers: { + "content-type": "application/json", + "retry-after": "120", + }, + }), + ); + const proxy = await startProxy({ + accountManager, + fetchImpl, + options: { forcedAccountIndex: 0 }, + }); + + const response = await postResponses(proxy, { + model: "gpt-5-codex", + stream: true, + input: [{ type: "message", role: "user", content: "hi" }], + }); + + expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + // One direct attempt on the pin, then fail-hard — never a rotation. + expect(calls).toHaveLength(1); + const payload = (await response.json()) as { + error: { + code: string; + pin_source: string | null; + reason: string | null; + reset_at: string | null; + retry_after_ms: number | null; + message: string; + }; + }; + expect(payload.error.code).toBe("codex_pinned_account_unavailable"); + expect(payload.error.pin_source).toBe("forced"); + // The retry loop's selection verdict — recovery metadata must not + // depend on this string, only on the account's persisted state. + expect(payload.error.reason).toBe("already-attempted"); + expect(payload.error.retry_after_ms).toBeGreaterThan(0); + expect(Date.parse(payload.error.reset_at ?? "")).toBeGreaterThan(now); + expect(payload.error.message).toContain("the recorded limit resets at"); + expect(payload.error.message).toContain("launcher"); + expect(payload.error.message).not.toContain("unpin"); + }); + + it("carries cooldown recovery metadata when the forced account fails with a network error", async () => { + const now = Date.now(); + const accountManager = new AccountManager(undefined, createStorage(now, 2)); + const { calls, fetchImpl } = createRecordingFetch(() => { + throw new TypeError("fetch failed"); + }); + const proxy = await startProxy({ + accountManager, + fetchImpl, + options: { forcedAccountIndex: 0 }, + }); + + const response = await postResponses(proxy, { + model: "gpt-5-codex", + stream: true, + input: [{ type: "message", role: "user", content: "hi" }], + }); + + expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + expect(calls).toHaveLength(1); + const payload = (await response.json()) as { + error: { + code: string; + pin_source: string | null; + reset_at: string | null; + retry_after_ms: number | null; + }; + }; + expect(payload.error.code).toBe("codex_pinned_account_unavailable"); + expect(payload.error.pin_source).toBe("forced"); + // The network-error cooldown bounds recovery. + expect(payload.error.retry_after_ms).toBeGreaterThan(0); + expect(Date.parse(payload.error.reset_at ?? "")).toBeGreaterThan(now); + }); + it("reads the forced account from CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX env when no option is passed (#623)", async () => { // This is the exact mechanism the pin uses to cross the launcher -> detached // app-helper process boundary: no option, env only. From 5b76c68542c4ee8c5087ede3b5d1c5ad07b400ed Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 16:53:00 -0400 Subject: [PATCH 5/8] fix(runtime): include the circuit deadline in pinned-503 recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An open circuit breaker outlives the short failure cooldowns that tripped it, so recovery derived only from the persisted account state advertised an early reset — or none at all once the cooldown lapsed — while requests kept 503ing until the breaker's own deadline. The 503 recovery is now the later of the account-state bound and the breaker's next-attempt time, exposed through AccountManager.getCircuitRecoveryTime over the breaker's existing getTimeUntilAvailable. A proxy regression trips the default breaker on a forced pin (with the network-error cooldown zeroed so every request records a failure) and asserts the advertised recovery is the circuit deadline, not the elapsed cooldown. --- lib/accounts.ts | 16 +++++++++ lib/runtime-rotation-proxy.ts | 15 ++++++++- test/runtime-rotation-proxy.test.ts | 51 +++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 8b7152e3f..5854ce9ce 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1294,6 +1294,22 @@ export class AccountManager { return getCircuitBreaker(getAccountCircuitKey(account)).isAvailable(); } + /** + * When the account's circuit breaker will admit an attempt again, as an + * epoch-ms deadline — null when it already would. Lets the pinned-503 + * recovery metadata cover circuit-open skips, whose deadline lives in the + * breaker rather than the persisted account record. + */ + getCircuitRecoveryTime( + account: ManagedAccount, + now = Date.now(), + ): number | null { + const waitMs = getCircuitBreaker( + getAccountCircuitKey(account), + ).getTimeUntilAvailable(now); + return waitMs > 0 ? now + waitMs : null; + } + incrementAuthFailures(account: ManagedAccount): number { account.consecutiveAuthFailures = (account.consecutiveAuthFailures ?? 0) + 1; diff --git a/lib/runtime-rotation-proxy.ts b/lib/runtime-rotation-proxy.ts index d7c56cc81..949ee2e94 100644 --- a/lib/runtime-rotation-proxy.ts +++ b/lib/runtime-rotation-proxy.ts @@ -1585,7 +1585,7 @@ async function handleRequestInner( // recovery — and with several overlapping records the account stays // skipped until the LAST one expires, so the latest bound is the one // worth advertising. - const pinnedResetAtMs = + const pinnedStateRecoveryAtMs = pinnedAccount === null ? null : getAccountRecoveryTimeForFamily( @@ -1593,6 +1593,19 @@ async function handleRequestInner( state.now(), context.family, ); + // An open circuit outlives the short failure cooldowns that tripped + // it; its deadline lives in the breaker, not the account record. + const pinnedCircuitRecoveryAtMs = + pinnedAccount === null + ? null + : accountManager.getCircuitRecoveryTime(pinnedAccount, state.now()); + const pinnedResetAtMs = + pinnedStateRecoveryAtMs === null && pinnedCircuitRecoveryAtMs === null + ? null + : Math.max( + pinnedStateRecoveryAtMs ?? 0, + pinnedCircuitRecoveryAtMs ?? 0, + ); const errorBody = buildPinnedUnavailableErrorBody( pinnedIndex, accountSkipReasons, diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index 83c007519..282ecde3b 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -884,6 +884,57 @@ describe("runtime rotation proxy", () => { expect(Date.parse(payload.error.reset_at ?? "")).toBeGreaterThan(now); }); + it("advertises the circuit deadline once repeated failures open the pinned account's breaker", async () => { + const now = Date.now(); + const accountManager = new AccountManager(undefined, createStorage(now, 2)); + const { calls, fetchImpl } = createRecordingFetch(() => { + throw new TypeError("fetch failed"); + }); + // Zero the network-error cooldown so every request reaches upstream + // and records a breaker failure; otherwise the cooldown absorbs the + // retries and the breaker never opens. + vi.stubEnv("CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS", "0"); + const proxy = await startProxy({ + accountManager, + fetchImpl, + options: { forcedAccountIndex: 0 }, + }); + vi.unstubAllEnvs(); + const body = { + model: "gpt-5-codex", + stream: true, + input: [{ type: "message", role: "user", content: "hi" }], + }; + + // Three failing requests trip the default breaker (threshold 3). + for (let attempt = 0; attempt < 3; attempt += 1) { + const failed = await postResponses(proxy, body); + expect(failed.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + } + expect(calls).toHaveLength(3); + + const response = await postResponses(proxy, body); + expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + // Circuit-open skips before any upstream attempt. + expect(calls).toHaveLength(3); + const payload = (await response.json()) as { + error: { + code: string; + pin_source: string | null; + reset_at: string | null; + retry_after_ms: number | null; + }; + }; + expect(payload.error.code).toBe("codex_pinned_account_unavailable"); + expect(payload.error.pin_source).toBe("forced"); + // The breaker's 30s reset outlives the short network-error cooldown + // that tripped it; the advertised recovery must be the circuit + // deadline, not the already-elapsed cooldown. + expect(payload.error.retry_after_ms).toBeGreaterThan(10_000); + expect(payload.error.retry_after_ms).toBeLessThanOrEqual(30_000); + expect(Date.parse(payload.error.reset_at ?? "")).toBeGreaterThan(now); + }); + it("reads the forced account from CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX env when no option is passed (#623)", async () => { // This is the exact mechanism the pin uses to cross the launcher -> detached // app-helper process boundary: no option, env only. From c112ddc9f7903f7fe93d330fd12eb8663500765b Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 17:15:49 -0400 Subject: [PATCH 6/8] fix(runtime): bound pinned-503 recovery by the keys that gate the request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selection consults exactly two rate-limit keys per request — the family-wide key and the requested model's key (isRateLimitedForFamily) — so another model's record in the same family never blocks the request and must not inflate its advertised recovery. getAccountRecoveryTimeForFamily now takes the model and considers only those gating keys plus the active cooldown; the proxy passes the request's model through. Unit coverage pins both directions: an unrelated model's later record is ignored, and a model-scoped record alone does not gate a model-less request. --- lib/runtime-rotation-proxy.ts | 1 + lib/runtime/account-status.ts | 22 ++++++++++-------- test/account-status.test.ts | 43 +++++++++++++++++++++++++++++++---- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/lib/runtime-rotation-proxy.ts b/lib/runtime-rotation-proxy.ts index 949ee2e94..c11bb6e1e 100644 --- a/lib/runtime-rotation-proxy.ts +++ b/lib/runtime-rotation-proxy.ts @@ -1592,6 +1592,7 @@ async function handleRequestInner( pinnedAccount, state.now(), context.family, + context.model, ); // An open circuit outlives the short failure cooldowns that tripped // it; its deadline lives in the breaker, not the account record. diff --git a/lib/runtime/account-status.ts b/lib/runtime/account-status.ts index 0f72e8ed4..afc8b312a 100644 --- a/lib/runtime/account-status.ts +++ b/lib/runtime/account-status.ts @@ -39,12 +39,16 @@ export function getRateLimitResetTimeForFamily( } /** - * The moment the account becomes usable again for `family`: the LATEST - * matching rate-limit record plus any active cooldown. Distinct from + * The moment the account becomes usable again for a `family`/`model` + * request: the LATEST bound among the records that actually gate that + * request plus any active cooldown. Two deliberate differences from * getRateLimitResetTimeForFamily, whose earliest-reset answer feeds wait - * displays: the account stays skipped while ANY matching record is active, - * so a retry hint built from the earliest reset would send clients back - * into a 503. Null when nothing bounds recovery. + * displays: the account stays skipped while ANY gating record is active, + * so the earliest reset would send clients back into a 503 — and only the + * keys selection consults (`family`, plus `family:` when a model is + * known; see isRateLimitedForFamily) may contribute, because another + * model's record does not block this request and would overstate its + * recovery. Null when nothing bounds recovery. */ export function getAccountRecoveryTimeForFamily( account: { @@ -53,6 +57,7 @@ export function getAccountRecoveryTimeForFamily( }, now: number, family: ModelFamily, + model?: string | null, ): number | null { let latest: number | null = null; const consider = (value: number | undefined): void => { @@ -62,11 +67,8 @@ export function getAccountRecoveryTimeForFamily( }; const times = account.rateLimitResetTimes; if (times) { - const prefix = `${family}:`; - for (const [key, value] of Object.entries(times)) { - if (key !== family && !key.startsWith(prefix)) continue; - consider(value); - } + consider(times[family]); + if (model) consider(times[`${family}:${model}`]); } consider(account.coolingDownUntil); return latest; diff --git a/test/account-status.test.ts b/test/account-status.test.ts index 7ad30f753..6effc24a8 100644 --- a/test/account-status.test.ts +++ b/test/account-status.test.ts @@ -104,19 +104,52 @@ describe("account status helpers", () => { }); describe("getAccountRecoveryTimeForFamily", () => { - it("returns the LATEST matching reset so a retry lands after real recovery", () => { - // Two overlapping records for the family: the account stays skipped - // until the last one expires, so the earliest reset would send a - // client straight back into a 503. + it("returns the LATEST gating reset so a retry lands after real recovery", () => { + // Family-wide and requested-model records overlap: the account stays + // skipped until the later one expires, so the earliest reset would + // send a client straight back into a 503. expect( getAccountRecoveryTimeForFamily( - { rateLimitResetTimes: { codex: 3_000, "codex:5h": 9_000 } }, + { + rateLimitResetTimes: { + codex: 3_000, + "codex:gpt-5-codex": 9_000, + }, + }, 1_000, "codex", + "gpt-5-codex", ), ).toBe(9_000); }); + it("ignores records that do not gate the request", () => { + // Another model's record in the same family does not block this + // request (selection checks only the family key and the requested + // model's key), so it must not inflate the advertised recovery. + expect( + getAccountRecoveryTimeForFamily( + { + rateLimitResetTimes: { + codex: 3_000, + "codex:gpt-5.3-codex": 9_000, + }, + }, + 1_000, + "codex", + "gpt-5-codex", + ), + ).toBe(3_000); + // Without a model only the family-wide key gates. + expect( + getAccountRecoveryTimeForFamily( + { rateLimitResetTimes: { "codex:gpt-5-codex": 9_000 } }, + 1_000, + "codex", + ), + ).toBeNull(); + }); + it("ignores other families and expired records", () => { expect( getAccountRecoveryTimeForFamily( From 2a03c2b812f90edc220e6c9510834544133c8737 Mon Sep 17 00:00:00 2001 From: Mike Bannister Date: Sat, 15 Aug 2026 17:41:36 -0400 Subject: [PATCH 7/8] fix(runtime): no timed recovery under a permanent pinned blocker, and reuse getQuotaKey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups: a disabled, workspace-disabled, auth-invalidated, policy-blocked, or out-of-range pinned account stays unselectable after any concurrent rate-limit record or cooldown expires, so the 503 no longer advertises that record's expiry — selection rejects such an account before any attempt, so the recorded skip reason is reliably the permanent one and gates the suppression. A proxy regression pins a disabled account carrying an active record and asserts reset_at and retry_after_ms stay null. The recovery helper also derives its record keys through getQuotaKey instead of a hand-rolled template, so the shape cannot drift from what markRateLimitedWithReason persists. --- lib/runtime-rotation-proxy.ts | 31 +++++++++++++++++++-- lib/runtime/account-status.ts | 5 ++-- test/runtime-rotation-proxy.test.ts | 42 +++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/lib/runtime-rotation-proxy.ts b/lib/runtime-rotation-proxy.ts index c11bb6e1e..3f04b1261 100644 --- a/lib/runtime-rotation-proxy.ts +++ b/lib/runtime-rotation-proxy.ts @@ -3,6 +3,7 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse } import type { Socket } from "node:net"; import { AccountManager, + AUTH_INVALIDATION_MARKER, extractAccountId, type ManagedAccount, } from "./accounts.js"; @@ -166,6 +167,19 @@ function toUrlHost(host: string): string { // failures surfaced only as a last-write-wins status.lastError string. Logs are // level-gated and carry the per-request correlation id set in handleRequest. const proxyLog = createLogger("runtime-proxy"); + +/** + * Pinned skip reasons that no timer clears: the account stays unselectable + * after any concurrent rate-limit record or cooldown expires, so the pinned + * 503 must not advertise that record's expiry as a recovery time. + */ +const PINNED_PERMANENT_SKIP_REASONS: ReadonlySet = new Set([ + "missing", + "disabled", + "workspace-disabled", + "policy-blocked", + AUTH_INVALIDATION_MARKER, +]); const DEFAULT_QUOTA_REMAINING_THRESHOLD = 10; /** @internal Stable identity key for in-memory quota snapshots across reloads. */ @@ -1578,7 +1592,19 @@ async function handleRequestInner( typeof pinnedIndex === "number" ? accountManager.getAccountByIndex(pinnedIndex) : null; - // Recovery comes from the pinned account's persisted state, not the + const pinnedSkipReason = + typeof pinnedIndex === "number" + ? accountSkipReasons.get(pinnedIndex) ?? null + : null; + // A permanent blocker (disabled, no enabled workspace, invalidated + // auth, policy block, out-of-range pin) outlives every timed record, + // so advertising a record's expiry would invite a retry into another + // 503. Selection rejects such an account before any attempt, so the + // recorded skip reason is the permanent one in exactly these cases. + const pinnedBlockedPermanently = + pinnedSkipReason !== null && + PINNED_PERMANENT_SKIP_REASONS.has(pinnedSkipReason); + // Otherwise recovery comes from the pinned account's persisted state, not the // recorded skip reason: a direct 429/cooldown on the pinned account // reaches this 503 as "already-attempted" (the retry loop's selection // verdict) while the record it just wrote is what actually bounds @@ -1601,7 +1627,8 @@ async function handleRequestInner( ? null : accountManager.getCircuitRecoveryTime(pinnedAccount, state.now()); const pinnedResetAtMs = - pinnedStateRecoveryAtMs === null && pinnedCircuitRecoveryAtMs === null + pinnedBlockedPermanently || + (pinnedStateRecoveryAtMs === null && pinnedCircuitRecoveryAtMs === null) ? null : Math.max( pinnedStateRecoveryAtMs ?? 0, diff --git a/lib/runtime/account-status.ts b/lib/runtime/account-status.ts index afc8b312a..c6d47bafb 100644 --- a/lib/runtime/account-status.ts +++ b/lib/runtime/account-status.ts @@ -1,3 +1,4 @@ +import { getQuotaKey } from "../accounts/rate-limits.js"; import type { ModelFamily } from "../prompts/codex.js"; export function resolveActiveIndex( @@ -67,8 +68,8 @@ export function getAccountRecoveryTimeForFamily( }; const times = account.rateLimitResetTimes; if (times) { - consider(times[family]); - if (model) consider(times[`${family}:${model}`]); + consider(times[getQuotaKey(family)]); + if (model) consider(times[getQuotaKey(family, model)]); } consider(account.coolingDownUntil); return latest; diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index 282ecde3b..21158983b 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -935,6 +935,48 @@ describe("runtime rotation proxy", () => { expect(Date.parse(payload.error.reset_at ?? "")).toBeGreaterThan(now); }); + it("suppresses timed recovery when a permanent blocker holds the pinned account", async () => { + const now = Date.now(); + const accountManager = new AccountManager(undefined, createStorage(now, 2)); + const pinned = accountManager.getAccountByIndex(0); + if (!pinned) throw new Error("setup failed"); + // Disabled outlives the record: after the rate limit expires the + // account is still unselectable, so no recovery time is honest. + pinned.enabled = false; + pinned.rateLimitResetTimes = { codex: now + 60_000 }; + const { calls, fetchImpl } = createRecordingFetch(() => + textEventStream("data: should-not-be-reached\n\n"), + ); + const proxy = await startProxy({ + accountManager, + fetchImpl, + options: { forcedAccountIndex: 0 }, + }); + + const response = await postResponses(proxy, { + model: "gpt-5-codex", + stream: true, + input: [{ type: "message", role: "user", content: "hi" }], + }); + + expect(response.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); + expect(calls).toHaveLength(0); + const payload = (await response.json()) as { + error: { + code: string; + reason: string | null; + reset_at: string | null; + retry_after_ms: number | null; + message: string; + }; + }; + expect(payload.error.code).toBe("codex_pinned_account_unavailable"); + expect(payload.error.reason).toBe("disabled"); + expect(payload.error.reset_at).toBeNull(); + expect(payload.error.retry_after_ms).toBeNull(); + expect(payload.error.message).not.toContain("resets at"); + }); + it("reads the forced account from CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX env when no option is passed (#623)", async () => { // This is the exact mechanism the pin uses to cross the launcher -> detached // app-helper process boundary: no option, env only. From 63c83bcc51548b350212fc39856c010943d489ee Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 16 Aug 2026 19:44:18 +0800 Subject: [PATCH 8/8] fix(forecast,runtime): keep the default forecast on codex, and harden the pinned-503 reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes on top of #670 and #671. #670 threaded the requested model's prompt family into `forecast`, `best`, and `report`. All three carry a DEFAULT model (`DEFAULT_PROBE_MODEL` / `DEFAULT_LIVE_PROBE_MODEL` = `gpt-5.6-sol`) whose prompt family is `gpt-5.2`, so deriving the family unconditionally silently moved every bare invocation off the codex family: - `codex-multi-auth forecast` / `report` reported an account rate-limited on the codex family as `ready`, and dropped a live `rate-limited` runtime overlay backed by a codex record as "stale" — the exact desync #670 set out to remove, aimed at the default invocation instead. - `codex-multi-auth best` recommended (and `switch` then pinned) an account the runtime proxy refuses for every codex request, since the wrapper's `/codex/responses` path always buckets into the codex family. The family now moves only when the invocation actually carried `--model`; `modelProvided` is tracked in the forecast/report parsers the way `best` already tracked it. Bare invocations keep `evaluateForecastAccount`'s codex default, exactly as #670's description promised. Also from the review: - `report` reuses `modelInspection.promptFamily`, which `inspectRequestedModel` already resolved, instead of calling `getModelProfile` again for every account; `forecast` and `best` hoist the same resolution out of their per-account `.map()`. - `buildPinnedUnavailableErrorBody` no longer feeds an out-of-range epoch to `new Date(...).toISOString()`. The deadline comes from persisted account state (`rateLimitResetTimes`, `coolingDownUntil`), which a corrupted or hand-edited storage file can carry past the ECMAScript time range; the RangeError would have replaced the pinned-503 diagnostics with the proxy's generic 500. - `docs/reference/error-contracts.md` documents the new `pin_source`, `reset_at`, and `retry_after_ms` fields and drops the claim that the pinned 503 only ever comes from a manual `switch` pin. - The circuit-breaker proxy test restores `CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS` in a `finally` instead of calling `vi.unstubAllEnvs()` mid-test, so an early assertion failure cannot leak the override into later tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WYnWb16vmd1XdS33GPtdxi --- docs/reference/error-contracts.md | 12 +++++- lib/codex-manager/commands/best.ts | 14 ++++++- lib/codex-manager/commands/forecast.ts | 25 +++++++++++- lib/codex-manager/commands/report.ts | 23 ++++++++++- lib/request/rate-limit-decision.ts | 16 ++++++-- test/codex-manager-best-command.test.ts | 9 ++++- test/codex-manager-forecast-command.test.ts | 10 +++-- test/codex-manager-report-command.test.ts | 43 +++++++++++++++++++++ test/rate-limit-decision.test.ts | 15 +++++++ test/runtime-rotation-proxy.test.ts | 30 ++++++++++---- 10 files changed, 176 insertions(+), 21 deletions(-) diff --git a/docs/reference/error-contracts.md b/docs/reference/error-contracts.md index 80a3de63e..3e0c4fcd4 100644 --- a/docs/reference/error-contracts.md +++ b/docs/reference/error-contracts.md @@ -120,11 +120,21 @@ The default-on localhost Responses proxy returns JSON error payloads with a stab | `runtime_rotation_proxy_unauthorized` | `401` | Local request did not include the per-process proxy client key | | `runtime_rotation_proxy_payload_too_large` | `413` | Request body exceeded the proxy safety cap | | `codex_runtime_rotation_pool_exhausted` | `429` or `503` | No managed account can currently service the runtime request | -| `codex_pinned_account_unavailable` | `503` | A manual pin is set (via `codex-multi-auth switch`) but the pinned account is rate-limited, cooling down, disabled, or blocked by policy. Run `codex-multi-auth status` for details, or `codex-multi-auth unpin` to allow rotation | +| `codex_pinned_account_unavailable` | `503` | A pin is in force — either a manual pin (`codex-multi-auth switch`) or a forced per-invocation pin (`--account` / `CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX`) — but the pinned account is rate-limited, cooling down, disabled, or blocked by policy. The remedy depends on `pin_source` (see below) | | `codex_runtime_rotation_proxy_error` | `500` | Proxy failed before forwarding the request | Pool exhaustion includes a `reason`, `retry_after_ms`, and a hint to run `codex-multi-auth rotation status`. Pinned-account-unavailable responses include a `pinnedAccountIndex` field identifying the pinned account, a structured `reason` field carrying the runtime skip reason (for example `rate-limited`, `cooling-down:auth-failure`, `circuit-open`, `disabled`, `workspace-disabled`, `policy-blocked`, `missing`, `already-attempted`) or `null` when no reason was recorded, and an `account_skip_reasons` map keyed by account index that mirrors the pool-exhausted response shape. The human-readable `message` appends the same reason in parentheses when present (see issue #486). +Pinned-account-unavailable responses also carry: + +| Field | Type | Meaning | +| --- | --- | --- | +| `pin_source` | `"forced"`, `"manual"`, or `null` | `"forced"` when the pin came from `--account` / `CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX`, `"manual"` when it came from `codex-multi-auth switch`. `unpin` clears only a manual pin, so the `message` tells a forced-pin caller to relaunch instead | +| `reset_at` | ISO-8601 string or `null` | When the blocking state ends — the latest of the gating rate-limit record, the account cooldown, and the circuit-breaker deadline. `null` under a permanent blocker (`disabled`, `workspace-disabled`, `policy-blocked`, `missing`, invalidated auth) or when nothing bounds recovery | +| `retry_after_ms` | number or `null` | `reset_at` expressed as a delay from the moment the response was built; `null` whenever `reset_at` is `null` | + +When `reset_at` is present the `message` appends `; the recorded limit resets at `. + Account policy pause/drain is enforced through runtime policy evaluation and contributes to selection skip reasons such as `policy-blocked`. --- diff --git a/lib/codex-manager/commands/best.ts b/lib/codex-manager/commands/best.ts index 10c4a5301..acadabd0d 100644 --- a/lib/codex-manager/commands/best.ts +++ b/lib/codex-manager/commands/best.ts @@ -287,14 +287,24 @@ export async function runBestCommand( } } + // Resolved once, not once per account. Only an EXPLICIT `--model` may move + // the availability family: DEFAULT_LIVE_PROBE_MODEL is `gpt-5.6-sol`, whose + // prompt family is `gpt-5.2`, so deriving it unconditionally would let a bare + // `best` recommend an account that is rate-limited on the codex family the + // wrapper actually routes — and every Codex request would then 503 off the + // pin that recommendation produced. + const forecastFamily = options.modelProvided + ? getModelProfile(probeModel).promptFamily + : undefined; + const forecastActiveIndex = deps.resolveActiveIndex(storage, "codex"); const forecastInputs = storage.accounts.map((account, index) => ({ index, account, - isCurrent: index === deps.resolveActiveIndex(storage, "codex"), + isCurrent: index === forecastActiveIndex, now, refreshFailure: refreshFailures.get(index), liveQuota: liveQuotaByIndex.get(index), - family: getModelProfile(probeModel).promptFamily, + family: forecastFamily, })); const forecastResults = deps.evaluateForecastAccounts(forecastInputs); const recommendation = deps.recommendForecastAccount(forecastResults); diff --git a/lib/codex-manager/commands/forecast.ts b/lib/codex-manager/commands/forecast.ts index 3bd0ec286..9aaefd086 100644 --- a/lib/codex-manager/commands/forecast.ts +++ b/lib/codex-manager/commands/forecast.ts @@ -28,6 +28,14 @@ interface ForecastCliOptions { json: boolean; explain: boolean; model: string; + /** + * True only when the invocation actually carried `--model`. `model` always + * holds a value (it falls back to DEFAULT_PROBE_MODEL so the probe and the + * header have something to name), so it cannot distinguish an explicit + * request from the default — and the availability family must never follow a + * default the user did not ask for. + */ + modelProvided: boolean; runtimeOverlay: boolean; } @@ -156,6 +164,7 @@ function parseForecastArgs( json: false, explain: false, model: DEFAULT_PROBE_MODEL, + modelProvided: false, runtimeOverlay: true, }; @@ -184,6 +193,7 @@ function parseForecastArgs( return { ok: false, message: "Missing value for --model" }; } options.model = value; + options.modelProvided = true; i += 1; continue; } @@ -193,6 +203,7 @@ function parseForecastArgs( return { ok: false, message: "Missing value for --model" }; } options.model = value; + options.modelProvided = true; continue; } return { ok: false, message: `Unknown option: ${arg}` }; @@ -223,6 +234,18 @@ export async function runForecastCommand( const options = parsedArgs.options; const requestedModel = options.model?.trim() || DEFAULT_PROBE_MODEL; const probeModel = resolveNormalizedModel(requestedModel); + // Resolved once, not once per account: getModelProfile re-parses and + // re-resolves the model string on every call. + // + // Only an EXPLICIT `--model` may move the availability family. The default + // probe model is `gpt-5.6-sol`, whose prompt family is `gpt-5.2`, so + // deriving the family unconditionally would flip a bare `forecast` off the + // codex family and report a codex-rate-limited account as `ready` — the + // exact forecast/runtime desync this threading exists to remove, aimed at + // the default invocation instead. + const forecastFamily = options.modelProvided + ? getModelProfile(requestedModel).promptFamily + : undefined; const display = deps.loadDashboardDisplaySettings ? (await deps.loadDashboardDisplaySettings().catch(() => null)) ?? deps.defaultDisplay @@ -374,7 +397,7 @@ export async function runForecastCommand( quotaCache, allAccounts: storage.accounts, runtimeOverlay, - family: getModelProfile(requestedModel).promptFamily, + family: forecastFamily, })); const forecastResults = deps.evaluateForecastAccounts(forecastInputs); const summary = deps.summarizeForecast(forecastResults); diff --git a/lib/codex-manager/commands/report.ts b/lib/codex-manager/commands/report.ts index dc9ce7141..6691cbe4c 100644 --- a/lib/codex-manager/commands/report.ts +++ b/lib/codex-manager/commands/report.ts @@ -46,6 +46,14 @@ interface ReportCliOptions { json: boolean; explain: boolean; model: string; + /** + * True only when the invocation actually carried `--model`. `model` always + * holds a value (it falls back to DEFAULT_PROBE_MODEL for the probe and the + * `modelSelection` block), so it cannot distinguish an explicit request from + * the default — and the availability family must never follow a default the + * user did not ask for. + */ + modelProvided: boolean; maxAccounts?: number; maxProbes?: number; cachedOnly: boolean; @@ -144,6 +152,7 @@ function parseReportArgs(args: string[]): ParsedArgsResult { json: false, explain: false, model: DEFAULT_PROBE_MODEL, + modelProvided: false, cachedOnly: false, }; @@ -172,6 +181,7 @@ function parseReportArgs(args: string[]): ParsedArgsResult { return { ok: false, message: "Missing value for --model" }; } options.model = value; + options.modelProvided = true; i += 1; continue; } @@ -181,6 +191,7 @@ function parseReportArgs(args: string[]): ParsedArgsResult { return { ok: false, message: "Missing value for --model" }; } options.model = value; + options.modelProvided = true; continue; } if (arg === "--max-accounts") { @@ -462,6 +473,16 @@ export async function runReportCommand( } } + // `inspectRequestedModel` already resolved the profile, so reuse its + // `promptFamily` instead of re-resolving the model once per account. + // + // Only an EXPLICIT `--model` may move the availability family: the default + // probe model is `gpt-5.6-sol`, whose prompt family is `gpt-5.2`, so + // deriving it unconditionally would make a bare `report` describe gpt-5.2 + // availability while the wrapper routes codex-family traffic. + const forecastFamily = options.modelProvided + ? modelInspection.promptFamily + : undefined; const forecastResults = storage ? evaluateForecastAccounts( storage.accounts.map((account, index) => ({ @@ -474,7 +495,7 @@ export async function runReportCommand( quotaCache, allAccounts: storage.accounts, runtimeOverlay: runtimeSnapshot, - family: getModelProfile(modelInspection.normalized).promptFamily, + family: forecastFamily, })), ) : []; diff --git a/lib/request/rate-limit-decision.ts b/lib/request/rate-limit-decision.ts index 6f775e5f1..ff10ecde9 100644 --- a/lib/request/rate-limit-decision.ts +++ b/lib/request/rate-limit-decision.ts @@ -193,6 +193,9 @@ export interface PinnedUnavailableErrorBody { account_skip_reasons: Record; } +/** Largest epoch-ms a `Date` can represent; beyond it `toISOString()` throws. */ +const MAX_ECMASCRIPT_TIME_MS = 8.64e15; + export interface PinnedUnavailableContext { /** * "forced" when the pin came from the wrapper's forced-account mode @@ -228,7 +231,14 @@ export function buildPinnedUnavailableErrorBody( const resetAtMs = typeof context?.resetAtMs === "number" && Number.isFinite(context.resetAtMs) && - context.resetAtMs > 0 + context.resetAtMs > 0 && + // `new Date(x).toISOString()` throws RangeError past the ECMAScript time + // range. The deadline is read from persisted account state + // (`rateLimitResetTimes`, `coolingDownUntil`) which a corrupted or + // hand-edited storage file can carry out of range, and this builder runs + // on the 503 path: a throw here would drop the pin diagnostics entirely + // and surface the proxy's generic 500 instead. + context.resetAtMs <= MAX_ECMASCRIPT_TIME_MS ? context.resetAtMs : null; const now = context?.now ?? Date.now(); @@ -236,8 +246,8 @@ export function buildPinnedUnavailableErrorBody( const resetAt = resetAtMs !== null ? new Date(resetAtMs).toISOString() : null; const waitSuffix = resetAt !== null ? `; the recorded limit resets at ${resetAt}` : ""; - // A forced pin belongs to the launching process, not to ndy's persisted - // pin state, so `unpin` would clear nothing — say what actually helps. + // A forced pin belongs to the launching process, not to the persisted pin + // state, so `unpin` would clear nothing — say what actually helps. const remedy = pinSource === "forced" ? "the pin was set by this session's launcher, so relaunch to select a different account" diff --git a/test/codex-manager-best-command.test.ts b/test/codex-manager-best-command.test.ts index a6b702be2..2b2ae403a 100644 --- a/test/codex-manager-best-command.test.ts +++ b/test/codex-manager-best-command.test.ts @@ -168,12 +168,17 @@ describe("runBestCommand", () => { })), }); + // No `--model` means no family: DEFAULT_LIVE_PROBE_MODEL is `gpt-5.6-sol`, + // whose prompt family is `gpt-5.2`. Threading it would let a bare `best` + // recommend an account that is rate-limited on the codex family, and the + // resulting pin would 503 on every Codex request. await expect(runBestCommand(["--json"], deps)).resolves.toBe(0); const defaulted = evaluateForecastAccounts.mock.calls.at(-1)?.[0] as | Array<{ family?: string }> | undefined; - expect(defaulted?.[0]?.family).toBe( - getModelProfile(DEFAULT_LIVE_PROBE_MODEL).promptFamily, + expect(defaulted?.[0]?.family).toBeUndefined(); + expect(getModelProfile(DEFAULT_LIVE_PROBE_MODEL).promptFamily).not.toBe( + "codex", ); const explicitDeps = createDeps({ diff --git a/test/codex-manager-forecast-command.test.ts b/test/codex-manager-forecast-command.test.ts index ac83590df..dd75c841f 100644 --- a/test/codex-manager-forecast-command.test.ts +++ b/test/codex-manager-forecast-command.test.ts @@ -198,13 +198,17 @@ describe("runForecastCommand", () => { | undefined; expect(explicit?.[0]?.family).toBe(getModelProfile("gpt-5.6-sol").promptFamily); + // No `--model` means no family: DEFAULT_PROBE_MODEL is `gpt-5.6-sol`, + // whose prompt family is `gpt-5.2`, so threading it here would move a bare + // `forecast` off the codex family that the wrapper actually routes and + // report a codex-rate-limited account as `ready`. Leaving it undefined + // keeps evaluateForecastAccount on its codex default. await expect(runForecastCommand(["--json"], deps)).resolves.toBe(0); const defaulted = evaluateForecastAccounts.mock.calls.at(-1)?.[0] as | Array<{ family?: string }> | undefined; - expect(defaulted?.[0]?.family).toBe( - getModelProfile(DEFAULT_PROBE_MODEL).promptFamily, - ); + expect(defaulted?.[0]?.family).toBeUndefined(); + expect(getModelProfile(DEFAULT_PROBE_MODEL).promptFamily).not.toBe("codex"); }); it("honors --no-runtime-overlay in json forecast output", async () => { diff --git a/test/codex-manager-report-command.test.ts b/test/codex-manager-report-command.test.ts index f39df66e5..1ddb00663 100644 --- a/test/codex-manager-report-command.test.ts +++ b/test/codex-manager-report-command.test.ts @@ -144,6 +144,49 @@ describe("runReportCommand", () => { expect(codex.accounts[0]?.availability).toBe("ready"); }); + it("keeps the default (no --model) report on the codex family", async () => { + // DEFAULT_PROBE_MODEL is `gpt-5.6-sol`, whose prompt family is `gpt-5.2`. + // A bare `report` must NOT inherit that family: the wrapper routes + // codex-family traffic, so a codex record has to keep gating the default + // report and a gpt-5.2 record must not. + const readForecast = ( + deps: ReportCommandDeps, + ): { accounts: Array<{ availability: string }> } => + ( + JSON.parse( + String( + (deps.logInfo as ReturnType).mock.calls.at(-1)?.[0] ?? + "{}", + ), + ) as { forecast: { accounts: Array<{ availability: string }> } } + ).forecast; + const withRecord = ( + rateLimitResetTimes: Record, + ): ReportCommandDeps => { + const storage = createStorage([ + { + email: "one@example.com", + refreshToken: "refresh-token-1", + accessToken: "access-token-1", + expiresAt: 10, + addedAt: 1, + lastUsed: 1, + enabled: true, + rateLimitResetTimes, + }, + ]); + return createDeps({ loadAccounts: vi.fn(async () => storage) }); + }; + + const codexDeps = withRecord({ codex: 31_000 }); + await expect(runReportCommand(["--json"], codexDeps)).resolves.toBe(0); + expect(readForecast(codexDeps).accounts[0]?.availability).toBe("delayed"); + + const generalDeps = withRecord({ "gpt-5.2": 31_000 }); + await expect(runReportCommand(["--json"], generalDeps)).resolves.toBe(0); + expect(readForecast(generalDeps).accounts[0]?.availability).toBe("ready"); + }); + it("rejects a flag-like or whitespace-only --model value instead of consuming it", async () => { // Split-arg form trims before validating, so " -x" / " " can't slip // through and silently fall back to the default model. diff --git a/test/rate-limit-decision.test.ts b/test/rate-limit-decision.test.ts index 4410d3c42..75a411625 100644 --- a/test/rate-limit-decision.test.ts +++ b/test/rate-limit-decision.test.ts @@ -318,6 +318,21 @@ describe("buildPinnedUnavailableErrorBody", () => { expect(body.message).not.toContain("unpin"); }); + it("drops an out-of-range reset instead of throwing on toISOString", () => { + // A corrupted / hand-edited storage file can carry a `coolingDownUntil` + // or rate-limit reset past the ECMAScript time range. `new Date(x) + // .toISOString()` throws RangeError there, and this builder runs on the + // 503 path — a throw would replace the pin diagnostics with a generic 500. + const body = buildPinnedUnavailableErrorBody( + 0, + new Map([[0, "rate-limited"]]), + { pinSource: "manual", resetAtMs: 8.64e15 + 1, now: 1_700_000_000_000 }, + ); + expect(body.reset_at).toBeNull(); + expect(body.retry_after_ms).toBeNull(); + expect(body.message).not.toContain("resets at"); + }); + it("keeps the unpin advice for manual pins and nulls an unknown reset", () => { const body = buildPinnedUnavailableErrorBody( 1, diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index 21158983b..1dd087b4c 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -892,14 +892,28 @@ describe("runtime rotation proxy", () => { }); // Zero the network-error cooldown so every request reaches upstream // and records a breaker failure; otherwise the cooldown absorbs the - // retries and the breaker never opens. - vi.stubEnv("CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS", "0"); - const proxy = await startProxy({ - accountManager, - fetchImpl, - options: { forcedAccountIndex: 0 }, - }); - vi.unstubAllEnvs(); + // retries and the breaker never opens. The proxy reads the value once at + // startup, so restore the exact prior value right after — `unstubAllEnvs` + // would clear every other stub too, and an assertion failure before it ran + // would leak the override into every later test (this file's afterEach + // only clears CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX). + const previousCooldown = + process.env.CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS; + process.env.CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS = "0"; + let proxy: Awaited>; + try { + proxy = await startProxy({ + accountManager, + fetchImpl, + options: { forcedAccountIndex: 0 }, + }); + } finally { + if (previousCooldown === undefined) { + delete process.env.CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS; + } else { + process.env.CODEX_AUTH_NETWORK_ERROR_COOLDOWN_MS = previousCooldown; + } + } const body = { model: "gpt-5-codex", stream: true,