fix(codex): bound completed entitlement version misses per account

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<string, number>;
+}
+
+/**
+ * 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<string, AccountVersionMissBudget>();
 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<CachedAccountModels> {
   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<string, number>() };
+  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<OcxConfig, "codexAccounts">): 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;

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

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<void>(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