From 9167ddfc49da20e962facf60526fc3a1461c06d1 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 18 Sep 2026 03:45:07 +0900 Subject: [PATCH] fix(codex): decide flagship model availability by roster and refusal evidence (#4906) A pool holding Plus and Free Codex accounts keeps sending gpt-5.6-sol and gpt-6-astra to a Free account and taking the upstream unsupported-model 400, after a quota refresh and a catalog sync, with no alternate attempt. The ordering rules #4797 added are present and correct; they just have no evidence to act on. Both read cachedDeniedCodexAccountIdsForModel, which is cache-only by contract, and the roster cache it reads expires five minutes after a catalog sync fills it. Nothing on the flagship request path refills it, because 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, withoutModelDeniedAccounts and preferModelEntitledAccount are the identity function, and the pool selects on quota alone. The refusal itself was the missing evidence. A 400 whose body is exactly "The '' model is not supported when using Codex with a ChatGPT account." is authenticated, account-specific and model-specific. It was spent on one retry and discarded, so the next request repeated the same selection. It is now recorded per account and model in a bounded six-hour store and unioned into cachedDeniedCodexAccountIdsForModel. It stays evidence rather than a gate: consumers treat it exactly like a roster denial, so restore-on-empty and the pin exemption still hold, no model is hidden from any catalog, and nothing is refused before dispatch. A confirmed roster grant for the same pair outranks it, a successful response clears it, and a credential identity change discards it. Availability is never inferred from a plan name or from remaining quota. Detection now reads the model upstream actually named instead of rebuilding the sentence from route.modelId. applyCodexAccountGatedWireNormalization rewrites Daybreak to gpt-5.6-sol before dispatch, so upstream names Sol while the route still says Daybreak; the comparison never matched, which silently disabled both the alternate-account retry and the eight-rung same-account ladder that exists for exactly that model. --- .../260918_lane_a_bug_train/010_roadmap.md | 127 ++++++++++++ scripts/test-layout/layout.json | 1 + src/codex/model-entitlements.ts | 61 +++++- src/codex/observed-model-denials.ts | 137 +++++++++++++ src/server/responses/core-codex-account.ts | 105 +++++++--- src/server/responses/passthrough-dispatch.ts | 24 ++- structure/providers/openai-tiers.md | 17 ++ .../codex-model-denial-evidence.test.ts | 181 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 1 + 9 files changed, 628 insertions(+), 26 deletions(-) create mode 100644 devlog/_plan/260918_lane_a_bug_train/010_roadmap.md create mode 100644 src/codex/observed-model-denials.ts create mode 100644 tests/codex-integration/codex-model-denial-evidence.test.ts 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",