diff --git a/docs/reference/error-contracts.md b/docs/reference/error-contracts.md index 80a3de63..3e0c4fcd 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/accounts.ts b/lib/accounts.ts index 8b7152e3..5854ce9c 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/codex-manager/commands/best.ts b/lib/codex-manager/commands/best.ts index ddba0d7e..acadabd0 100644 --- a/lib/codex-manager/commands/best.ts +++ b/lib/codex-manager/commands/best.ts @@ -1,6 +1,10 @@ import type { ForecastAccountResult } from "../../forecast.js"; import { type CodexQuotaSnapshot, describeCodexProbeFailure } from "../../quota-probe.js"; -import { resolveNormalizedModel } from "../../request/helpers/model-map.js"; +import { + getModelProfile, + type ModelFamily, + 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"; @@ -123,6 +127,7 @@ export interface BestCommandDeps { now: number; refreshFailure?: TokenFailure; liveQuota?: CodexQuotaSnapshot; + family?: ModelFamily; }>, ) => ForecastAccountResult[]; recommendForecastAccount: (results: ForecastAccountResult[]) => { @@ -282,13 +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: 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 f454cb03..9aaefd08 100644 --- a/lib/codex-manager/commands/forecast.ts +++ b/lib/codex-manager/commands/forecast.ts @@ -14,7 +14,12 @@ 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, + type ModelFamily, + resolveNormalizedModel, +} from "../../request/helpers/model-map.js"; import { type AccountMetadataV3, type AccountStorageV3 } from "../../storage.js"; import type { TokenFailure, TokenResult } from "../../types.js"; @@ -23,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; } @@ -85,6 +98,7 @@ export interface ForecastCommandDeps { quotaCache?: QuotaCacheData | null; allAccounts?: readonly AccountMetadataV3[]; runtimeOverlay?: RuntimeForecastOverlay | null; + family?: ModelFamily; }>, ) => ForecastAccountResult[]; summarizeForecast: (results: ForecastAccountResult[]) => { @@ -150,6 +164,7 @@ function parseForecastArgs( json: false, explain: false, model: DEFAULT_PROBE_MODEL, + modelProvided: false, runtimeOverlay: true, }; @@ -178,6 +193,7 @@ function parseForecastArgs( return { ok: false, message: "Missing value for --model" }; } options.model = value; + options.modelProvided = true; i += 1; continue; } @@ -187,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}` }; @@ -217,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 @@ -368,6 +397,7 @@ export async function runForecastCommand( quotaCache, allAccounts: storage.accounts, runtimeOverlay, + 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 0ad7d6ff..6691cbe4 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,6 +495,7 @@ export async function runReportCommand( quotaCache, allAccounts: storage.accounts, runtimeOverlay: runtimeSnapshot, + family: forecastFamily, })), ) : []; diff --git a/lib/forecast.ts b/lib/forecast.ts index 3e55f3e5..c1908983 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/lib/request/rate-limit-decision.ts b/lib/request/rate-limit-decision.ts index 88cddb96..ff10ecde 100644 --- a/lib/request/rate-limit-decision.ts +++ b/lib/request/rate-limit-decision.ts @@ -185,12 +185,34 @@ 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; } +/** 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 + * (`--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 +227,39 @@ 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 && + // `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(); + 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 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" + : "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 f447b985..3f04b126 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"; @@ -72,6 +73,7 @@ import { responseHeadersForClient, withTimeout, } from "./request/stream-failover-runtime.js"; +import { getAccountRecoveryTimeForFamily } from "./runtime/account-status.js"; import { chooseAccount } from "./runtime/rotation-account-selection.js"; import { createRotationProxyState, @@ -165,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. */ @@ -1573,9 +1588,62 @@ 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 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 + // 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 pinnedStateRecoveryAtMs = + pinnedAccount === null + ? null + : getAccountRecoveryTimeForFamily( + 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. + const pinnedCircuitRecoveryAtMs = + pinnedAccount === null + ? null + : accountManager.getCircuitRecoveryTime(pinnedAccount, state.now()); + const pinnedResetAtMs = + pinnedBlockedPermanently || + (pinnedStateRecoveryAtMs === null && pinnedCircuitRecoveryAtMs === null) + ? null + : Math.max( + pinnedStateRecoveryAtMs ?? 0, + pinnedCircuitRecoveryAtMs ?? 0, + ); 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/lib/runtime/account-status.ts b/lib/runtime/account-status.ts index 267cf663..c6d47baf 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( @@ -38,6 +39,42 @@ export function getRateLimitResetTimeForFamily( return minReset; } +/** + * 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 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: { + rateLimitResetTimes?: Record; + coolingDownUntil?: number; + }, + now: number, + family: ModelFamily, + model?: string | null, +): 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) { + consider(times[getQuotaKey(family)]); + if (model) consider(times[getQuotaKey(family, model)]); + } + 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 b4313d24..6effc24a 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,89 @@ describe("account status helpers", () => { ).toBe("resets in 4000ms"); }); }); + +describe("getAccountRecoveryTimeForFamily", () => { + 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: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( + { 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/codex-manager-best-command.test.ts b/test/codex-manager-best-command.test.ts index 4f8307e1..2b2ae403 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,69 @@ 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, + })), + }); + + // 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).toBeUndefined(); + expect(getModelProfile(DEFAULT_LIVE_PROBE_MODEL).promptFamily).not.toBe( + "codex", + ); + + 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 9846e773..dd75c841 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,45 @@ 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); + + // 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).toBeUndefined(); + expect(getModelProfile(DEFAULT_PROBE_MODEL).promptFamily).not.toBe("codex"); + }); + 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 3df81a70..1ddb0066 100644 --- a/test/codex-manager-report-command.test.ts +++ b/test/codex-manager-report-command.test.ts @@ -93,6 +93,100 @@ 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("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/forecast.test.ts b/test/forecast.test.ts index 4d858f2f..6ba3f263 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({ diff --git a/test/rate-limit-decision.test.ts b/test/rate-limit-decision.test.ts index 40d6cf37..75a41162 100644 --- a/test/rate-limit-decision.test.ts +++ b/test/rate-limit-decision.test.ts @@ -295,5 +295,54 @@ 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("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, + 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"); }); }); diff --git a/test/runtime-rotation-proxy.test.ts b/test/runtime-rotation-proxy.test.ts index a892bf92..1dd087b4 100644 --- a/test/runtime-rotation-proxy.test.ts +++ b/test/runtime-rotation-proxy.test.ts @@ -799,6 +799,198 @@ 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("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. 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, + 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("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.