diff --git a/src/codex/model-entitlements.ts b/src/codex/model-entitlements.ts index 553fb501b7..790d598eaf 100644 --- a/src/codex/model-entitlements.ts +++ b/src/codex/model-entitlements.ts @@ -317,6 +317,37 @@ const MODEL_ROSTER_VERSIONS_PER_ACCOUNT_MAX = 4; * roster. */ const MODEL_ROSTER_FLIGHTS_PER_ACCOUNT_MAX = 4; + +/** + * Distinct caller-selected roster versions admitted per account in one roster window. + * + * The cache budget and the flight budget both bound STATE, not WORK. A caller that cycles + * `client_version` and waits for each answer misses the cache by design and misses the flight + * key by design, so it can renew an authenticated upstream request under EVERY stored account + * token as often as it likes, and the gated-model checks it displaces fail closed while it does. + * + * DISTINCT VERSIONS are counted, never attempts. One legitimate client retrying a single version + * through an upstream outage comes back every 15s on the failure TTL; charging each attempt would + * spend the whole allowance on that one version and then refuse it for the rest of the 5-minute + * window, turning a recovered upstream into several more minutes without gated models. + */ +const MODEL_ROSTER_VERSION_MISSES_PER_ACCOUNT_MAX = 4; + +interface AccountVersionMissBudget { + credentialIdentity: string; + /** Version -> when this version stops occupying the allowance. */ + versions: Map; +} + +/** + * One row per ACCOUNT, not per credential identity. + * + * A Pool access-token refresh increments the generation, so an identity-keyed map would gain a + * permanent row per generation for the lifetime of the process: a protection against renewable + * work would have introduced an unbounded cache. A generation change replaces the row instead, + * which is also the right budget semantics — new credential, new allowance. + */ +const accountModelsMisses = new Map(); const DIRECT_CALLER_ACCOUNT_PREFIX = "__direct_codex__:"; export interface CodexModelEntitlementCredentialSnapshot { @@ -670,6 +701,7 @@ async function modelsForCredential( fetcher: typeof fetch, now: number, clientVersion: string, + trustedClientVersion: string, credentialMutationEpoch?: number, ): Promise { const cached = accountModelsCache.get(cacheKeyFor(credential.accountId, clientVersion)); @@ -698,6 +730,21 @@ async function modelsForCredential( confirmed: false, }; } + // Cache hits, joined flights and capacity refusals start no upstream request. Charge only + // after capacity admission; the locally selected runtime version remains exempt. + if ( + !credential.accountId.startsWith(DIRECT_CALLER_ACCOUNT_PREFIX) + && clientVersion !== trustedClientVersion + && !admitVersionMiss(credential, clientVersion, now) + ) { + return { + credentialIdentity: credential.credentialIdentity, + clientVersion, + expiresAt: now, + models: new Set(), + confirmed: false, + }; + } const flight = fetchAccountModels(credential, fetcher, now, clientVersion) .then(result => { if ( @@ -716,6 +763,30 @@ async function modelsForCredential( return flight; } +/** Whether this caller-selected version may open a new upstream request for the account. */ +function admitVersionMiss( + credential: CodexModelEntitlementCredentialSnapshot, + clientVersion: string, + now: number, +): boolean { + const stored = accountModelsMisses.get(credential.accountId); + const budget = stored && stored.credentialIdentity === credential.credentialIdentity + ? stored + : { credentialIdentity: credential.credentialIdentity, versions: new Map() }; + for (const [version, expiresAt] of budget.versions) { + if (expiresAt <= now) budget.versions.delete(version); + } + const alreadyCharged = budget.versions.has(clientVersion); + const admitted = alreadyCharged + || budget.versions.size < MODEL_ROSTER_VERSION_MISSES_PER_ACCOUNT_MAX; + // A repeat keeps its ORIGINAL expiry. Refreshing it here would let a caller hold one version + // open indefinitely, and it is the retry case this distinction exists to protect. + if (admitted && !alreadyCharged) budget.versions.set(clientVersion, now + MODEL_ROSTER_TTL_MS); + if (budget.versions.size === 0) accountModelsMisses.delete(credential.accountId); + else accountModelsMisses.set(credential.accountId, budget); + return admitted; +} + function candidateAccountIds(config: Pick): string[] { return [ MAIN_CODEX_ACCOUNT_ID, @@ -992,6 +1063,10 @@ export async function resolveCodexModelEntitlements( fetcher, now, clientVersion, + resolveCodexEntitlementClientVersion( + null, + options.loadPersistedRuntime ?? loadPersistedCodexRuntime, + ), options.credentialMutationEpoch, ), }))); @@ -1050,6 +1125,7 @@ export async function isDirectCallerEntitledToCodexModel( options.fetcher ?? fetch, options.now ?? Date.now(), clientVersion, + clientVersion, ); return codexModelEntitlementStateForRoster( result.models, @@ -1128,11 +1204,13 @@ export function invalidateCodexModelEntitlementsForAccount(accountId: string | n for (const key of [...accountModelsCache.keys()]) { if (accountIdOfCacheKey(key) === accountId) accountModelsCache.delete(key); } + accountModelsMisses.delete(accountId); } export function resetCodexModelEntitlementCacheForTests(): void { accountModelsCache.clear(); accountModelsFlights.clear(); + accountModelsMisses.clear(); negativeCredentialMemo.clear(); entitlementEnsureFlights.clear(); runtimeVersionMemo = null; diff --git a/structure/catalog.md b/structure/catalog.md index 0f995ec4fa..47a827426b 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -81,6 +81,14 @@ mapped account reports it. A failed or malformed discovery is not positive evide hides the gated row until a later refresh. The same snapshot gates Pool selection, so the catalog and runtime cannot disagree by advertising through one account and dispatching through another. +`client_version` arrives on the inbound request and is part of that cache identity, so +`src/codex/model-entitlements.ts` bounds the work as well as the state: stored versions per account, concurrent +roster flights per account, and distinct caller-selected versions admitted per account in one roster +window. Repeating a version already charged still retries on the failure TTL, and the locally +selected runtime version is never charged, so a legitimate refresh survives. Over the bound the +answer is unconfirmed, which hides the gated row rather than confirming a denial. Flight capacity +is checked before charging a distinct version, so a capacity refusal consumes no miss allowance. + The app-server's model list comes from this shared catalog, not from patching the App. Codex Desktop may still apply its remote native-only allowlist after `model/list`; an explicitly configured combo `nativeAlias` is the bounded compatibility path. It replaces one supported bare native row with a diff --git a/tests/codex-integration/codex-model-entitlements.test.ts b/tests/codex-integration/codex-model-entitlements.test.ts index 219ec7dcc8..926f1994a7 100644 --- a/tests/codex-integration/codex-model-entitlements.test.ts +++ b/tests/codex-integration/codex-model-entitlements.test.ts @@ -1544,6 +1544,81 @@ describe("entitlement client version (#2886)", () => { expect(fetches).toBe(afterFill + 1); }); + test("capacity-rejected versions do not consume the miss allowance", async () => { + let fetches = 0; + let hold = true; + const release: Array<() => void> = []; + const backend = (async () => { + fetches += 1; + if (hold) await new Promise(resolve => release.push(resolve)); + return roster(SOL); + }) as typeof fetch; + const credentials = [credential("pool-capacity-budget")]; + const ask = (clientVersion: string) => resolveCodexModelEntitlements({ codexAccounts: [] }, { + fetcher: backend, now: 1_000, clientVersion, credentials, + loadPersistedRuntime: () => ({ selectedVersion: "0.300.0" }), + }); + const pending = ["0.300.0", "0.400.0", "0.401.0", "0.402.0"].map(ask); + try { + for (let i = 0; i < 100 && release.length < 4; i += 1) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + expect(fetches).toBe(4); + const rejected = await ask("0.403.0"); + expect(rejected.confirmedAccountIds.has("pool-capacity-budget")).toBe(false); + expect(fetches).toBe(4); + } finally { + hold = false; + for (const resolve of release) resolve(); + await Promise.all(pending); + } + // Only three caller-selected versions opened a flight. A different fourth version + // must still be admitted once capacity is free; the rejected attempt spent nothing. + const admitted = await ask("0.404.0"); + expect(fetches).toBe(5); + expect(admitted.confirmedAccountIds.has("pool-capacity-budget")).toBe(true); + }); + + test("completed caller-selected version misses are bounded per account", async () => { + // The cache budget bounds stored state and the flight budget bounds concurrency. Neither + // bounds COMPLETED work, so a caller cycling client_version and waiting for each answer could + // renew an authenticated upstream request under the account token as often as it liked. + let fetches = 0; + const backend = (async () => { fetches += 1; return roster(SOL); }) as typeof fetch; + const credentials = [{ + accountId: "pool-miss-budget", + accessToken: "tok-miss", + chatgptAccountId: "acct-miss", + credentialIdentity: "pool:1:acct-miss", + }]; + const ask = (clientVersion: string) => resolveCodexModelEntitlements({ codexAccounts: [] }, { + fetcher: backend, + now: 1_000, + clientVersion, + credentials, + loadPersistedRuntime: () => ({ selectedVersion: "0.300.0" }), + }); + + for (let i = 0; i < 8; i += 1) await ask(`0.${400 + i}.0`); + expect(fetches).toBe(4); + + // Over the allowance the answer is UNCONFIRMED - the same fail-closed shape a discovery + // failure produces, never a confirmed denial assembled from a roster nobody fetched. + const refused = await ask("0.499.0"); + expect(refused.confirmedAccountIds.has("pool-miss-budget")).toBe(false); + expect(fetches).toBe(4); + + // A version already charged keeps retrying. One client coming back every 15s on the failure + // TTL must not spend the allowance and lock itself out for the rest of the roster window. + await ask("0.400.0"); + expect(fetches).toBe(5); + + // The locally selected runtime version is never charged, so its legitimate refresh survives + // an untrusted caller spending everything else. + await ask("0.300.0"); + expect(fetches).toBe(6); + }); + test("the class budget counts accounts, not cached keys", async () => { // The documented budget is 64 ACCOUNTS per class. Counting keys instead would silently divide // that by the per-account version bound, so a deployment well inside the intended limit would