diff --git a/devlog/_plan/260918_lane_a_bug_train/010_roadmap.md b/devlog/_plan/260918_lane_a_bug_train/010_roadmap.md new file mode 100644 index 0000000000..380b159a1c --- /dev/null +++ b/devlog/_plan/260918_lane_a_bug_train/010_roadmap.md @@ -0,0 +1,127 @@ +# Lane A bug train — roadmap + +Base: `origin/dev` at `2f025814f3`, package 2.59.0. + +Three reported bugs are in scope. They share no source files, so each lands as an +independent pull request against `dev` rather than a serial stack. + +| Issue | Area | Files | PR | +| --- | --- | --- | --- | +| #4906 | Codex account/model entitlement routing | `src/codex/model-entitlements.ts`, `src/server/responses/core-codex-account.ts`, `src/server/responses/passthrough-dispatch.ts` | 020 | +| #4893 | Responses passthrough transient-retry policy | `src/providers/key-failover.ts`, `src/server/responses/passthrough-dispatch.ts` | 030 | +| #4903 | Combo failover classification of a `response_format` refusal | `src/combos/failover.ts` | 040 | + +#4893 and #4906 both touch `passthrough-dispatch.ts`, in disjoint regions: #4906 edits the +pool-retry block near the end of the recovery loop, #4893 edits the four +`fetchWithTransientRetry` option literals. Whichever lands second rebases onto the first. + +## Why each report needed re-judging at the current head + +Every report names 2.57.0 or 2.58.0. The findings below are re-derived from the `dev` tip, and +two of the three reports are accurate about the symptom while wrong about the cause. + +### #4906 — the roster preference is real but almost never has evidence + +`#4797` (`f35fbe8ef6`) added two ordering rules, and both are present at the tip: +`withoutModelDeniedAccounts` narrows the eligible list and `preferModelEntitledAccount` +corrects an active cursor. Both read `selectionOptions.deniedModelAccountIds`, which comes +from `cachedDeniedCodexAccountIdsForModel`. + +That reader is cache-only by contract, and the cache it reads expires in five minutes +(`MODEL_ROSTER_TTL_MS = 5 * 60_000`). Entries past `expiresAt` are skipped, so the reader +returns `undefined`. Nothing on the flagship request path refills it: `resolveCodexModelEntitlements` +is awaited only for `ACCOUNT_GATED_NATIVE_OPENAI_MODELS`, which since the 2026-09-04 owner +decision holds `gpt-daybreak-blue-latest` alone. The remaining writers are proxy startup, +catalog sync, convergence, and the catalog endpoint. + +So for a flagship request more than five minutes after the last sync, `deniedModelAccountIds` +is `undefined`, both ordering rules are the identity function, and the pool selects on quota +alone — which is exactly the Free account the reporter sees. The report's own guess ("the +refresh did not produce usable denial evidence, or it is not being consumed") is right about +the outcome and reaches the wrong half: the evidence is produced, and then it expires. + +The second half is that the upstream refusal teaches the pool nothing. A 400 whose body is +exactly `The '' model is not supported when using Codex with a ChatGPT account.` is an +authenticated, account-specific, model-specific denial — strictly better evidence than an +absent roster row. It is currently used once, to trigger one alternate-account retry, and then +discarded. The next request repeats the same selection and takes the same 400. + +A third defect sits in the detector itself. `isAllowListedCodexAccountModel400` builds its +expected string from `route.modelId`, but `applyCodexAccountGatedWireNormalization` rewrites +the wire model for `gpt-daybreak-blue-latest` to `gpt-5.6-sol` before dispatch. Upstream +therefore names `gpt-5.6-sol` in the refusal while the comparison expects the Daybreak slug, +the match fails, and the one model that is still account-gated gets neither the +alternate-account retry nor the eight-rung same-account ladder built for it. + +Fix: record the confirmed 400 as durable per-account denial evidence, union it into +`cachedDeniedCodexAccountIdsForModel` beneath roster-positive evidence, and compare the +refusal against the normalized wire model as well as the route model. + +Bounds this keeps. It stays an ordering preference: `withoutModelDeniedAccounts` still +restores denied members when filtering would empty the list, `preferModelEntitledAccount` +still returns the active account unchanged when no entitled alternative exists, no model is +hidden from any catalog, and nothing is refused before dispatch. The 2026-09-04 decision that +the flagships fail open is untouched. Availability is decided by the authenticated roster and +by upstream error evidence — never by a plan name and never by remaining quota. + +### #4893 — the passthrough lane cannot read the policy it is configured with + +The three source facts in the report hold at the tip. `transientRetryPolicyFor` rejects every +adapter but `openai-chat`; `createResponsesPassthroughAdapter` sets `passthrough: true` and +`core.ts` returns into `executePassthroughResponse` on that flag, before the three call sites +that would read the policy are constructed; and `passthrough-dispatch.ts` does not import the +function at all, passing the constant `TRANSIENT_RETRY_MAX_ATTEMPTS` at four call sites — +the initial send plus the OAuth-401, rate-limit-429, and rebuild recovery legs. + +PR #4800 widens the adapter gate only. That is necessary and not sufficient: the lane never +calls the gated function, so with #4800 alone the reproduction is unchanged at three sends. +This PR carries #4800's gate change with a `Co-authored-by` trailer and wires the lane. + +The two boundaries the constant currently insulates: + +- **Budget accounting.** `remainingTransientSendBudget(cap)` resolves to + `RequestExecutionBudget.remainingBaseSends(cap)`, so the provider value is a per-leg + ceiling intersected with what the logical request has left, not an independent allowance. + Every leg must read the same resolved cap, including `sendBudgetExhausted()`, which today + asks the question at the constant and would otherwise declare a request with configured + headroom exhausted. +- **The non-replayable boundary.** `isNonReplayableResponse` is checked inside + `fetchWithTransientRetry` and at each recovery branch, and is unaffected by `attempts`. + Raising the configured value must not become a way to obtain a resend that marker forbids; + the regression asserts that explicitly. + +Closure criterion: total physical sends equal the configured value intersected with the +request budget — `attempts: 1` sends once, `attempts: 5` sends at most five, no policy sends +three. + +### #4903 — a capability refusal classified as a request-shape refusal + +The combo chain stops because `comboFailureDecision` reaches +`["origin_rejected", "context_length_exceeded", "invalid_request_error"].includes(error.code)` +and returns `stop`. `isRequestLocalTargetIncompatibility` runs first and could return `hop`, +but its envelope admits three shapes only: `Unsupported parameter: user`, an +`unsupported_value` on `reasoning.effort`, and a model-scoped image-input rejection. A +`response_format` refusal matches none of them, and the gateway's `invalid_parameter_error` +is not in the accepted code set either, so the function returns at its first guard. + +Neither rejected option is taken. Hopping on every 400 would replay a genuinely malformed +request against every remaining target. Dropping `response_format` would change the output +contract the caller asked for, silently, on a path whose whole purpose is a structured title. + +Fix: extend the bounded envelope to the exact shape of a `response_format` capability +refusal — HTTP 400, intact provider JSON, `type: "invalid_request_error"`, a code in the +accepted set widened by `invalid_parameter_error`, and a message that names +`response_format` as unavailable or unsupported. A target that cannot honour the contract is +a target-local capability gap; the next target keeps the same request and either honours it or +is skipped in turn. Traversal stays finite because each candidate is tried once. + +## Operating constraints for this lane + +- No local verification of any kind. Correctness is argued from source and proven by hosted CI + at the exact head. +- Push with `git push --no-verify`; the pre-push hook runs the local suite. +- The lane does not merge, does not push to `dev`, does not rebase unasked, and does not close + issues or pull requests. Each item ends with an open PR and exact-head CI evidence. +- No flake management: no widened timeouts, no added retries, no platform skips. +- New test files need byte-identical entries in `scripts/test-layout/layout.json` and + `tests/fixtures/test-layout-expected.json`. diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index adebdb38bb..e9599ad03c 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -506,6 +506,7 @@ "codex-management-convergence.test.ts": "codex-integration", "codex-metadata-integrity.test.ts": "codex-integration", "codex-model-entitlements.test.ts": "codex-integration", + "codex-model-denial-evidence.test.ts": "codex-integration", "codex-model-availability-error.test.ts": "codex-integration", "codex-models-cache-invalidate.test.ts": "codex-integration", "codex-native-residue.test.ts": "codex-integration", diff --git a/src/codex/model-entitlements.ts b/src/codex/model-entitlements.ts index a770925f1d..39ed12a7b3 100644 --- a/src/codex/model-entitlements.ts +++ b/src/codex/model-entitlements.ts @@ -17,6 +17,13 @@ import { loadPersistedCodexRuntime } from "./runtime"; import { codexRuntimeStateEpoch } from "./runtime"; import upstreamModelsSnapshot from "./data/upstream-models.json"; import { codexCredentialMutationEpoch } from "./credential-mutation-epoch"; +import { + clearObservedCodexModelDenial, + forgetObservedCodexModelDenialsForAccount, + observedDeniedCodexAccountIdsForModel, + recordObservedCodexModelDenial, + resetObservedCodexModelDenialsForTests, +} from "./observed-model-denials"; const CODEX_MODELS_ENDPOINT = "https://chatgpt.com/backend-api/codex/models"; @@ -870,6 +877,11 @@ function needsEntitlementRefresh( const cached = accountModelsCache.get(cacheKeyFor(accountId, clientVersion)); if (cached && cached.credentialIdentity !== credentialIdentity) { invalidateCodexModelEntitlementsForAccount(accountId); + // The credential itself changed, so evidence gathered under the previous one answers for a + // different subscription. This is the only call site that knows that: the two gated-model + // sites in `core-codex-account.ts` invalidate a STALE roster for an unchanged credential, + // and clearing observed refusals there would discard the very evidence #4906 is about. + forgetObservedCodexModelDenialsForAccount(accountId); } else if (cached && cached.expiresAt > now) { return false; } @@ -1280,17 +1292,63 @@ export function cachedDeniedCodexAccountIdsForModel( if (state === "granted") granted.add(accountId); else if (state === "denied") denied.add(accountId); } + // The roster is not the only evidence, and on this path it is usually the weaker one. A cached + // roster expires in five minutes and nothing on the flagship request path refetches it, so + // absent an ongoing catalog sync the loop above contributes nothing at all. An upstream + // refusal does not expire on that schedule and is not a snapshot of a pending answer: it is + // the account's own Codex surface naming this model and declining it (#4906). + for (const accountId of observedDeniedCodexAccountIdsForModel(modelId, now) ?? []) { + // Under the caller's read fence, like the roster loop above. Nothing here reads account + // storage, but an excluded account must stay UNKNOWN rather than denied so a profile switch + // or a request-owned credential produces the same selection it does today. + if (options.excludeAccountIds?.has(accountId)) continue; + denied.add(accountId); + } // One account holds one entry per client version, and upstream filters the roster by that // version. So the same account can legitimately carry a granted entry under a current client // and a denied one under an older client that predates the model. Positive evidence is // authoritative regardless of which version asked for it -- the same rule // `codexModelEntitlementStateForRoster` applies within a single entry -- so a grant anywhere // clears the denial rather than being outvoted by whichever entry the map happened to yield - // last. + // last. It outranks an observed refusal for the same reason: a confirmed roster that lists the + // model is the newer answer, and a rollout that reaches an account must not be held back by a + // refusal it has already superseded. for (const accountId of granted) denied.delete(accountId); return denied.size > 0 ? denied : undefined; } +/** + * Record an authenticated upstream refusal as this account's own evidence about `modelId`. + * + * Scoped to {@link ENTITLEMENT_PREFERRED_NATIVE_OPENAI_MODELS} because that is the set whose + * availability varies per account while the row stays visible, and it is the set + * {@link cachedDeniedCodexAccountIdsForModel} will read back. A model outside it either has no + * per-account variance or is gated by the fail-closed roster path, where an ordering preference + * would change nothing. + * + * The caller must have matched the exact allow-listed refusal body first. A status alone is not + * admissible here: 400 covers every malformed request too, and remembering one of those as an + * entitlement fact would steer routing away from a perfectly capable account. + */ +export function recordCodexModelDenialEvidence( + accountId: string | null | undefined, + modelId: string | undefined, + now = Date.now(), +): void { + if (!accountId || !modelId) return; + if (!ENTITLEMENT_PREFERRED_NATIVE_OPENAI_MODELS.has(modelId)) return; + recordObservedCodexModelDenial(accountId, modelId, now); +} + +/** Drop the refusal evidence for a pair the account has just served successfully. */ +export function clearCodexModelDenialEvidence( + accountId: string | null | undefined, + modelId: string | undefined, +): void { + if (!accountId || !modelId) return; + clearObservedCodexModelDenial(accountId, modelId); +} + /** Synchronous projection for management/catalog readers after a discovery pass. */ export function cachedAvailableAccountGatedNativeModels( now = Date.now(), @@ -1351,6 +1409,7 @@ export function resetCodexModelEntitlementCacheForTests(): void { negativeCredentialMemo.clear(); entitlementEnsureFlights.clear(); runtimeVersionMemo = null; + resetObservedCodexModelDenialsForTests(); } /** Test-only snapshot for proving publication fences, which cache lookup intentionally masks. */ diff --git a/src/codex/observed-model-denials.ts b/src/codex/observed-model-denials.ts new file mode 100644 index 0000000000..97b0297a88 --- /dev/null +++ b/src/codex/observed-model-denials.ts @@ -0,0 +1,137 @@ +/** + * Per-account model denials observed from an authenticated upstream refusal. + * + * [Decision Log] + * - 목적과 의도: Keep the one piece of account-specific model evidence that is never stale -- + * the upstream's own refusal -- instead of discarding it after a single retry. + * - 기존 구현 및 제약 조건: `cachedDeniedCodexAccountIdsForModel` reads authenticated `/models` + * rosters, and those entries expire five minutes after they are fetched + * (`MODEL_ROSTER_TTL_MS`). Nothing on the flagship request path refills them, because + * `resolveCodexModelEntitlements` is awaited only for `ACCOUNT_GATED_NATIVE_OPENAI_MODELS`, + * which no longer holds the flagships. So the denial set is usually absent, the #4797 + * ordering rules become the identity function, and the pool picks on quota alone (#4906). + * - 검토한 주요 대안: Fetch a roster on the flagship request path, lengthen the roster TTL, or + * infer availability from the account's plan name. + * - 선택한 방식: Record the exact upstream unsupported-model refusal per (account, model) and + * let selection read it alongside the roster. + * - 다른 대안 대신 이 방식을 선택한 이유: A roster fetch on the request path puts an + * authenticated upstream call in front of the most commonly requested models in the product, + * which is what the cache-only contract exists to prevent. A longer TTL keeps a shard's + * stale absence around for longer without adding any evidence. A plan name proves nothing + * about a grant -- `available_in_plans` for `gpt-6-astra` lists `free` while free accounts + * are refused -- and #3022 is what happened the last time availability was inferred rather + * than observed. + * - 장점, 단점 및 영향: A refusal is spent once and then remembered, so the pool stops + * re-sending a model to the account that just refused it. The evidence is confirmed rather + * than inferred, it is still only an ordering preference, and it is overridden by any + * positive roster grant for the same pair. + * + * What this deliberately is NOT: an eligibility filter. Readers treat these ids exactly like + * roster denials -- `withoutModelDeniedAccounts` restores them when filtering would empty the + * candidate list, and `preferModelEntitledAccount` leaves the active account alone when no + * entitled alternative exists. No model is hidden from any catalog and no request is refused + * before dispatch, so the 2026-09-04 owner decision that the flagships fail open is untouched. + */ + +/** + * Six hours, against a five-minute roster TTL. + * + * The asymmetry is the point. A roster entry is a snapshot of an answer that may simply not + * have arrived yet, so it expires quickly and absence means "unknown". A refusal is an answer: + * upstream named this model and this account and said no. It still expires, because a rollout + * can reach an account between two requests, and the two faster paths back are a positive + * roster grant (which overrides this outright) and a successful response from the same account + * for the same model (which clears the entry). + */ +const OBSERVED_DENIAL_TTL_MS = 6 * 60 * 60_000; + +/** Bounded like the roster cache: pool size times flagship count, with room to spare. */ +const OBSERVED_DENIAL_MAX_ENTRIES = 512; + +/** `accountId\u0000modelId` -> expiry. Insertion order is the eviction order. */ +const observedDenials = new Map(); + +function denialKey(accountId: string, modelId: string): string { + return `${accountId}\u0000${modelId}`; +} + +function accountIdOfDenialKey(key: string): string { + return key.slice(0, key.indexOf("\u0000")); +} + +function modelIdOfDenialKey(key: string): string { + return key.slice(key.indexOf("\u0000") + 1); +} + +/** + * Remember that `accountId` was refused `modelId` by its own authenticated upstream. + * + * Re-recording refreshes the entry rather than extending an older one, so a pair that keeps + * being refused stays remembered and one that stops being refused ages out. + */ +export function recordObservedCodexModelDenial( + accountId: string, + modelId: string, + now = Date.now(), +): void { + const key = denialKey(accountId, modelId); + // Delete before set so the refreshed entry moves to the back of the eviction order. + observedDenials.delete(key); + observedDenials.set(key, now + OBSERVED_DENIAL_TTL_MS); + while (observedDenials.size > OBSERVED_DENIAL_MAX_ENTRIES) { + const oldest = observedDenials.keys().next(); + if (oldest.done) break; + observedDenials.delete(oldest.value); + } +} + +/** + * Forget one pair, because the account just served the model. + * + * A success is newer and stronger evidence than the refusal that preceded it: whatever the + * entitlement was when upstream refused, it is not that now. + */ +export function clearObservedCodexModelDenial(accountId: string, modelId: string): void { + observedDenials.delete(denialKey(accountId, modelId)); +} + +/** + * Forget every pair for one account, because its credential changed. + * + * A reauthenticated account can be a different subscription entirely, so evidence gathered + * under the previous credential says nothing about this one -- the same reasoning + * `invalidateCodexModelEntitlementsForAccount` applies to cached rosters. + */ +export function forgetObservedCodexModelDenialsForAccount(accountId: string | null | undefined): void { + if (!accountId) return; + for (const key of [...observedDenials.keys()]) { + if (accountIdOfDenialKey(key) === accountId) observedDenials.delete(key); + } +} + +/** + * Accounts refused `modelId` within the retention window. + * + * Returns `undefined` rather than an empty set when nothing is recorded, matching + * `cachedDeniedCodexAccountIdsForModel`: a caller must not be able to read "no evidence" as + * "nobody is denied". + */ +export function observedDeniedCodexAccountIdsForModel( + modelId: string | undefined, + now = Date.now(), +): ReadonlySet | undefined { + if (!modelId) return undefined; + const denied = new Set(); + for (const [key, expiresAt] of [...observedDenials]) { + if (expiresAt <= now) { + observedDenials.delete(key); + continue; + } + if (modelIdOfDenialKey(key) === modelId) denied.add(accountIdOfDenialKey(key)); + } + return denied.size > 0 ? denied : undefined; +} + +export function resetObservedCodexModelDenialsForTests(): void { + observedDenials.clear(); +} diff --git a/src/server/responses/core-codex-account.ts b/src/server/responses/core-codex-account.ts index 7d25b86501..c201e7acf3 100644 --- a/src/server/responses/core-codex-account.ts +++ b/src/server/responses/core-codex-account.ts @@ -32,6 +32,7 @@ import { resolveCodexModelEntitlements, invalidateCodexModelEntitlementsForAccount, entitledCodexAccountIdsForModel, + recordCodexModelDenialEvidence, } from "../../codex/model-entitlements"; import type { TransientSendBudget } from "../../lib/upstream-retry"; import { resolveAdapter, resolveWireProtocolOverride } from "../adapter-resolve"; @@ -161,43 +162,93 @@ export function normalizeCodexUnsupportedModelDetail(value: string): string { } -export function isAllowListedCodexAccountModel400( +/** + * The model id an authenticated Codex refusal names, or `undefined` when the body is not that + * refusal. + * + * Extracted rather than string-compared so the caller can learn WHICH model was refused. The + * route and the wire can legitimately disagree: `applyCodexAccountGatedWireNormalization` + * rewrites `gpt-daybreak-blue-latest` to `gpt-5.6-sol` before dispatch, so upstream names the + * model it was actually sent. Building the expected sentence from `route.modelId` alone made + * that comparison fail for the one model that is still account-gated, which silently disabled + * both the alternate-account retry and the same-account ladder built for exactly that case. + * + * The envelope is unchanged and stays exact: a top-level `detail` string, whitespace-collapsed + * and case-folded, matching the whole sentence with nothing before or after it. No prose is + * inferred and no other 400 shape is admitted, because a 400 is also what a malformed request + * earns and that must never read as an entitlement fact. + */ +export function codexUnsupportedModelFromDetail( status: number, bodyText: string, - modelId: string, -): boolean { - if (status !== 400) return false; +): string | undefined { + if (status !== 400) return undefined; try { const payload = JSON.parse(bodyText) as unknown; - if (!payload || typeof payload !== "object" || Array.isArray(payload)) return false; + if (!payload || typeof payload !== "object" || Array.isArray(payload)) return undefined; const detail = (payload as { detail?: unknown }).detail; - if (typeof detail !== "string") return false; - const expected = `The '${modelId}' model is not supported when using Codex with a ChatGPT account.`; - return normalizeCodexUnsupportedModelDetail(detail) - === normalizeCodexUnsupportedModelDetail(expected); + if (typeof detail !== "string") return undefined; + const matched = /^the '([^']{1,256})' model is not supported when using codex with a chatgpt account\.$/u + .exec(normalizeCodexUnsupportedModelDetail(detail)); + return matched?.[1]; } catch { - return false; + return undefined; } } -export async function shouldRetryCodexPoolAccountModel400( +/** + * The refused model id when this response is the exact unsupported-model refusal for this + * request, read from a bounded clone. + * + * Same admission rules {@link shouldRetryCodexPoolAccountModel400} always applied, which is now + * a predicate over this: a truncated or non-display-safe body proves nothing and is refused. + * Returning the id lets the caller record the denial against the model upstream actually named. + */ +export async function codexPoolAccountModel400Denial( response: Response, modelId: string, signal?: AbortSignal, -): Promise { - if (response.status !== 400) return false; + wireModelId?: string, +): Promise { + if (response.status !== 400) return undefined; try { const body = await readBoundedResponseBody(response.clone(), { signal }); - return body.displaySafe - && !body.truncated - && isAllowListedCodexAccountModel400(response.status, body.text, modelId); + if (!body.displaySafe || body.truncated) return undefined; + return isAllowListedCodexAccountModel400(response.status, body.text, modelId, wireModelId) + ? codexUnsupportedModelFromDetail(response.status, body.text) + : undefined; } catch { - return false; + return undefined; } } +export function isAllowListedCodexAccountModel400( + status: number, + bodyText: string, + modelId: string, + wireModelId?: string, +): boolean { + const refused = codexUnsupportedModelFromDetail(status, bodyText); + if (refused === undefined) return false; + return [modelId, wireModelId].some(candidate => ( + candidate !== undefined + && refused === normalizeCodexUnsupportedModelDetail(candidate) + )); +} + + +export async function shouldRetryCodexPoolAccountModel400( + response: Response, + modelId: string, + signal?: AbortSignal, + wireModelId?: string, +): Promise { + return await codexPoolAccountModel400Denial(response, modelId, signal, wireModelId) !== undefined; +} + + /** Pre-stream quota/billing rejections that warrant one alternate-account attempt (#584). */ export function codexQuotaFailureMessage(body: string): string | undefined { try { @@ -778,15 +829,25 @@ export async function retryCodexPoolOnAlternateAccount( } retrySendCount += 1; args.onResponse?.(upstreamResponse, retryAuthCtx, request); + // The alternate account can refuse the same model, and that refusal is evidence about the + // account that produced it. Read BEFORE the ladder's own break, so the ordinary + // single-retry path -- every flagship model, which is the #4906 case -- records it too + // rather than only the gated ladder below. Without this the pool learns nothing from a + // refusal and the next request repeats the same selection. + const retryModelDenial = await codexPoolAccountModel400Denial( + upstreamResponse, + route.modelId, + options.abortSignal, + parsed.modelId, + ); + if (retryModelDenial !== undefined) { + recordCodexModelDenialEvidence(retryAuthCtx.accountId, retryModelDenial); + } if (!retrySameConfirmedAccount || retrySendCount >= maxRetrySends) break; // Caller-owned main is an alternate-account replay and can never enter the bounded // same-stored-account 400 loop above. Keep that invariant explicit for the account-id reads. if (retryAuthCtx.kind === "main") break; - if (!await shouldRetryCodexPoolAccountModel400( - upstreamResponse, - route.modelId, - options.abortSignal, - )) break; + if (retryModelDenial === undefined) break; invalidateCodexModelEntitlementsForAccount(retryAuthCtx.accountId); let refreshed: Awaited>; try { diff --git a/src/server/responses/passthrough-dispatch.ts b/src/server/responses/passthrough-dispatch.ts index 56bca0310d..8b6fdf0467 100644 --- a/src/server/responses/passthrough-dispatch.ts +++ b/src/server/responses/passthrough-dispatch.ts @@ -91,11 +91,15 @@ import { usesCodexForwardPoolAuth, codexWsQuotaObserver, isFixedCodexAccount, - shouldRetryCodexPoolAccountModel400, + codexPoolAccountModel400Denial, shouldRetryCodexPoolAccountQuota, shouldRetryCodexPoolAccountTransient, retryCodexPoolOnAlternateAccount, } from "./core-codex-account"; +import { + clearCodexModelDenialEvidence, + recordCodexModelDenialEvidence, +} from "../../codex/model-entitlements"; import { readCodexWsStage } from "./codex-ws-wire"; import { linkAbortSignal } from "./core-lifetime"; import type { CodexAuthContext } from "../../codex/auth-context"; @@ -1291,11 +1295,25 @@ export async function preparePassthroughExchange( if (usesCodexForwardPoolAuth(admissionState.authCtx, route.provider)) { let poolRetryOutcome: number | undefined; - if (await shouldRetryCodexPoolAccountModel400( + // A success is the freshest evidence there is about this pair, and it outranks any earlier + // refusal: whatever the entitlement was when upstream declined, it is not that now. Both + // ids are cleared because the wire model can differ from the routed one. + if (upstreamResponse.ok) { + clearCodexModelDenialEvidence(admissionState.authCtx.accountId, route.modelId); + clearCodexModelDenialEvidence(admissionState.authCtx.accountId, parsed.modelId); + } + const model400Denial = await codexPoolAccountModel400Denial( upstreamResponse, route.modelId, options.abortSignal, - )) { + parsed.modelId, + ); + if (model400Denial !== undefined) { + // Spend this refusal on more than one retry. It is the account's own authenticated + // answer about this model, and the roster cache that selection otherwise reads expires + // five minutes after a catalog sync fills it -- so without remembering this, the next + // request selects the same account on quota alone and takes the same 400 (#4906). + recordCodexModelDenialEvidence(admissionState.authCtx.accountId, model400Denial); poolRetryOutcome = 400; } else if (!admissionState.authCtx.fixedAccount && await shouldRetryCodexPoolAccountQuota( upstreamResponse, diff --git a/structure/providers/openai-tiers.md b/structure/providers/openai-tiers.md index 38f72878ab..f10d28fe1e 100644 --- a/structure/providers/openai-tiers.md +++ b/structure/providers/openai-tiers.md @@ -402,6 +402,23 @@ Native Spark membership and its model-specific request/tool exceptions are remov unconfirmed, expired and too-old-client rosters stay unknown and change nothing; a grant under any client version clears a denial recorded under another. Nothing refuses before dispatch, and the bounded alternate-account retry on an exact unsupported-model 400 remains the safety net (#4768). + A cached roster lives five minutes and nothing on the flagship request path refetches it, so the + roster alone left that evidence absent for most requests and both ordering rules became the + identity function — the pool then selected on quota, which is #4906. The refusal itself is + therefore the second source: an exact pre-stream unsupported-model 400 from a Pool account is + recorded per (account, model) in `src/codex/observed-model-denials.ts` and unioned into + `cachedDeniedCodexAccountIdsForModel`. It is confirmed, authenticated evidence, never a plan + name and never remaining quota. It is bounded and retained for six hours, it is outranked by any + confirmed roster grant for the same pair, it is cleared when that account successfully serves + that model, and it is discarded when the account's credential identity changes. Recording is + scoped to the always-visible flagships, so a 400 anywhere else cannot steer routing. Every + consumer treats it exactly like a roster denial, so the restore-on-empty and pin-exempt rules + above continue to hold and no request is refused before dispatch. + Detection reads the model upstream actually named rather than rebuilding the sentence from + `route.modelId`, because `applyCodexAccountGatedWireNormalization` rewrites Daybreak to + `gpt-5.6-sol` before dispatch; comparing against the route model alone never matched for the + one model that is still account-gated, which disabled both its alternate-account retry and its + same-account ladder. `getEligiblePoolAccounts` is not the only door, so `preferModelEntitledAccount` applies the same evidence to an already-active shared cursor: the replacement is drawn from the eligible list, the active account is returned unchanged when no entitled alternative exists, and the correction is diff --git a/tests/codex-integration/codex-model-denial-evidence.test.ts b/tests/codex-integration/codex-model-denial-evidence.test.ts new file mode 100644 index 0000000000..bc1f7067de --- /dev/null +++ b/tests/codex-integration/codex-model-denial-evidence.test.ts @@ -0,0 +1,181 @@ +import { beforeEach, describe, expect, test } from "bun:test"; +import { + cachedDeniedCodexAccountIdsForModel, + clearCodexModelDenialEvidence, + recordCodexModelDenialEvidence, + resetCodexModelEntitlementCacheForTests, + seedCodexModelEntitlementsForTests, +} from "../../src/codex/model-entitlements"; +import { + codexUnsupportedModelFromDetail, + isAllowListedCodexAccountModel400, + shouldRetryCodexPoolAccountModel400, +} from "../../src/server/responses/core-codex-account"; + +const TEST_CLIENT_VERSION = "0.146.0"; +const DAYBREAK = "gpt-daybreak-blue-latest"; +const SOL = "gpt-5.6-sol"; +const ASTRA = "gpt-6-astra"; + +/** The exact refusal body the ChatGPT Codex backend returns for an unentitled model. */ +function refusalBody(modelId: string): string { + return JSON.stringify({ + detail: `The '${modelId}' model is not supported when using Codex with a ChatGPT account.`, + }); +} + +function refusalResponse(modelId: string): Response { + return new Response(refusalBody(modelId), { + status: 400, + headers: { "content-type": "application/json" }, + }); +} + +beforeEach(() => resetCodexModelEntitlementCacheForTests()); + +/** + * #4906. `#4797` taught selection to order by roster denial, and the reporter still lands on a + * Free account for Sol and Astra after a refresh and a catalog sync. + * + * The reason is that the roster is the only evidence the reader had, and a roster entry lives + * five minutes (`MODEL_ROSTER_TTL_MS`). Nothing on the flagship request path refetches it -- + * `resolveCodexModelEntitlements` is awaited only for `ACCOUNT_GATED_NATIVE_OPENAI_MODELS`, + * which holds Daybreak alone since the 2026-09-04 owner decision. So for most requests the + * denial set is absent, both ordering rules are the identity function, and the pool chooses on + * quota alone. + * + * These tests pin the second source of evidence: the upstream refusal itself. It is + * account-specific, model-specific, authenticated, and it does not expire on the roster's + * schedule. + */ +describe("upstream refusal as per-account model denial evidence", () => { + test("a recorded refusal denies the account with no roster cached at all", () => { + const now = 1_800_000_000_000; + // Precondition, and the whole of #4906: with no roster evidence the reader is silent, so + // selection sees nothing and picks the Free account on quota. + expect(cachedDeniedCodexAccountIdsForModel(SOL, now)).toBeUndefined(); + + recordCodexModelDenialEvidence("free", SOL, now); + + expect([...(cachedDeniedCodexAccountIdsForModel(SOL, now) ?? [])]).toEqual(["free"]); + // Model-scoped: refusing Sol says nothing about Astra on the same account. + expect(cachedDeniedCodexAccountIdsForModel(ASTRA, now)).toBeUndefined(); + }); + + test("a confirmed roster grant outranks an earlier refusal", () => { + const now = 1_800_000_000_000; + recordCodexModelDenialEvidence("plus", ASTRA, now); + expect([...(cachedDeniedCodexAccountIdsForModel(ASTRA, now) ?? [])]).toEqual(["plus"]); + + // A rollout reached the account. The newer answer wins, so a refusal cannot strand an + // account that has since been granted the model. + seedCodexModelEntitlementsForTests("plus", [ASTRA], now, TEST_CLIENT_VERSION); + expect(cachedDeniedCodexAccountIdsForModel(ASTRA, now)).toBeUndefined(); + }); + + test("a success clears the refusal for that pair only", () => { + const now = 1_800_000_000_000; + recordCodexModelDenialEvidence("free", SOL, now); + recordCodexModelDenialEvidence("free", ASTRA, now); + + clearCodexModelDenialEvidence("free", SOL); + + expect(cachedDeniedCodexAccountIdsForModel(SOL, now)).toBeUndefined(); + expect([...(cachedDeniedCodexAccountIdsForModel(ASTRA, now) ?? [])]).toEqual(["free"]); + }); + + test("refusal evidence expires, and outlives the five-minute roster window", () => { + const now = 1_800_000_000_000; + recordCodexModelDenialEvidence("free", SOL, now); + + // The roster TTL is where #4797's evidence disappeared. This must still be answering there. + expect([...(cachedDeniedCodexAccountIdsForModel(SOL, now + 5 * 60_000 + 1) ?? [])]) + .toEqual(["free"]); + expect([...(cachedDeniedCodexAccountIdsForModel(SOL, now + 6 * 60 * 60_000 - 1) ?? [])]) + .toEqual(["free"]); + // It is still evidence about a moment, not a permanent verdict. + expect(cachedDeniedCodexAccountIdsForModel(SOL, now + 6 * 60 * 60_000 + 1)).toBeUndefined(); + }); + + test("only always-visible natives are recorded, so a 400 elsewhere cannot steer routing", () => { + const now = 1_800_000_000_000; + recordCodexModelDenialEvidence("free", "gpt-5.5", now); + recordCodexModelDenialEvidence("free", DAYBREAK, now); + + expect(cachedDeniedCodexAccountIdsForModel("gpt-5.5", now)).toBeUndefined(); + // Daybreak is account-gated and fails closed through the eligibility path instead. + expect(cachedDeniedCodexAccountIdsForModel(DAYBREAK, now)).toBeUndefined(); + }); + + test("an excluded account stays unknown rather than denied", () => { + const now = 1_800_000_000_000; + recordCodexModelDenialEvidence("free", SOL, now); + + // The native-main read fence: an excluded account must produce the selection it does today. + expect(cachedDeniedCodexAccountIdsForModel(SOL, now, { + excludeAccountIds: new Set(["free"]), + })).toBeUndefined(); + expect([...(cachedDeniedCodexAccountIdsForModel(SOL, now, { + excludeAccountIds: new Set(["other"]), + }) ?? [])]).toEqual(["free"]); + }); +}); + +/** + * The detector that decides whether a 400 IS that refusal. + * + * It gained the wire model because `applyCodexAccountGatedWireNormalization` rewrites Daybreak + * to `gpt-5.6-sol` before dispatch, so upstream names Sol while `route.modelId` is still + * Daybreak. Comparing against the route model alone made the match fail for the only model that + * is still account-gated, which disabled both its alternate-account retry and the eight-rung + * same-account ladder that exists specifically for it. + */ +describe("unsupported-model refusal detection", () => { + test("extracts the model upstream named", () => { + expect(codexUnsupportedModelFromDetail(400, refusalBody(SOL))).toBe(SOL); + // Case and whitespace are normalized exactly as before. + expect(codexUnsupportedModelFromDetail(400, JSON.stringify({ + detail: `The '${SOL}' model is NOT supported when using Codex with a ChatGPT account.`, + }))).toBe(SOL); + }); + + test("admits nothing but that exact envelope", () => { + expect(codexUnsupportedModelFromDetail(400, JSON.stringify({ detail: "Bad request" }))) + .toBeUndefined(); + // Prose around the sentence is not the sentence. + expect(codexUnsupportedModelFromDetail(400, JSON.stringify({ + detail: `note: The '${SOL}' model is not supported when using Codex with a ChatGPT account.`, + }))).toBeUndefined(); + expect(codexUnsupportedModelFromDetail(400, JSON.stringify({ error: refusalBody(SOL) }))) + .toBeUndefined(); + expect(codexUnsupportedModelFromDetail(400, "not json")).toBeUndefined(); + // A different status is a different fact, whatever the body says. + expect(codexUnsupportedModelFromDetail(403, refusalBody(SOL))).toBeUndefined(); + }); + + test("matches the wire model when normalization rewrote it", () => { + // Before the fix this was `false`: upstream names Sol, the route still says Daybreak. + expect(isAllowListedCodexAccountModel400(400, refusalBody(SOL), DAYBREAK, SOL)).toBe(true); + expect(isAllowListedCodexAccountModel400(400, refusalBody(SOL), DAYBREAK)).toBe(false); + // The route model still matches on its own, so the unnormalized path is unchanged. + expect(isAllowListedCodexAccountModel400(400, refusalBody(SOL), SOL)).toBe(true); + // And an unrelated model is still not a match under either id. + expect(isAllowListedCodexAccountModel400(400, refusalBody(ASTRA), DAYBREAK, SOL)).toBe(false); + }); + + test("the response-level predicate carries the wire model through", async () => { + expect(await shouldRetryCodexPoolAccountModel400(refusalResponse(SOL), DAYBREAK, undefined, SOL)) + .toBe(true); + expect(await shouldRetryCodexPoolAccountModel400(refusalResponse(SOL), DAYBREAK)) + .toBe(false); + expect(await shouldRetryCodexPoolAccountModel400(refusalResponse(SOL), SOL)).toBe(true); + expect(await shouldRetryCodexPoolAccountModel400( + new Response("{}", { status: 400 }), + SOL, + )).toBe(false); + expect(await shouldRetryCodexPoolAccountModel400( + new Response(refusalBody(SOL), { status: 200 }), + SOL, + )).toBe(false); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 5ba90d98db..a9b39d63ab 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -338,6 +338,7 @@ "codex-management-convergence.test.ts": "codex-integration", "codex-metadata-integrity.test.ts": "codex-integration", "codex-model-entitlements.test.ts": "codex-integration", + "codex-model-denial-evidence.test.ts": "codex-integration", "codex-model-availability-error.test.ts": "codex-integration", "codex-models-cache-invalidate.test.ts": "codex-integration", "codex-native-residue.test.ts": "codex-integration",