fix(codex): ask upstream for the entitlement roster under a real client version

devlog/_plan/260829_bugpr_lane_h_residual_issues/160_issue_2886_entitlement_client_version.md

@@ -0,0 +1,138 @@
+# 160 — issue #2886: entitled GPT-5.6 Sol/Terra/Luna vanish from the native catalog
+
+## What the reporter saw
+
+A healthy ChatGPT Plus account that can demonstrably use `gpt-5.6-sol` — native Codex
+routing shows it, a fresh Sol conversation completes, and OpenCodex 2.33.0 advertises all
+three — loses `gpt-5.6-sol`, `gpt-5.6-terra`, and `gpt-5.6-luna` from both
+`ocx models live` and the Codex App picker after upgrading to 2.35.0. Re-enabling them
+by hand fails with `invalid model visibility target`.
+
+The A/B is single-variable and includes a working control, so this is not a stale picker
+cache.
+
+## The filtering is upstream, and this repository already measured it
+
+OpenCodex never compares `minimal_client_version` itself — it strips the field
+(`src/codex/catalog/metadata.ts:502`, `src/codex/catalog/parsing.ts:486`), with a
+regression pinning that at `tests/codex-catalog.test.ts:2737`. So no local filter is
+dropping these rows; the roster arrives without them.
+
+A prior unit measured the endpoint directly
+(`devlog/_fin/260817_native_gpt56_1m_context/001_measurement_evidence.md`):
+`client_version` is a required query parameter, and the model count returned depends on
+it — `0.60.0` yields **0** models, `0.142.2` yields **5**.
+
+Entitlement discovery asks for exactly `client_version=0.0.0`
+(`src/codex/model-entitlements.ts:14`). It is asking upstream to describe what a
+prehistoric client may use, and then treating the answer as what this account owns.
+
+`#2550` added the three slugs to `ACCOUNT_GATED_NATIVE_OPENAI_MODELS`
+(`src/codex/catalog/native-models.ts:5-10`), which is correct policy for the inverse
+report in `#2548`. Fail-closed is right when entitlement is unknown; the defect is that
+the input was never a real entitlement answer. A valid `{models:[...]}` response sets
+`confirmed: true` regardless of contents (`:133-143`, `:172-180`), availability requires
+`confirmed && models.has(id)` (`:317-327`), and catalog sync then drops the rows
+(`src/codex/catalog/sync.ts:1579-1596`).
+
+The account's plan is never consulted, and WHAM is a separate request
+(`src/codex/auth-api.ts:767-785`) — `plan=plus, status=200` proves authentication, not
+roster contents. The reporter's evidence and the code were measuring different things.
+
+## Where the real version comes from
+
+There is no existing outbound precedent to copy: native forwarding uses
+`FORWARD_HEADERS` (`src/adapters/openai-responses.ts:35`), which carries neither
+`user-agent` nor any version header, and the other `client_version: "0.0.0"` sites
+(`src/codex/convergence.ts:476`, `src/codex/catalog/sync.ts:1988`) are local Codex cache
+wrappers, not upstream requests.
+
+But the best source is already in hand for the path that matters. A live catalog request
+arrives **from Codex**, carrying its own `client_version` query parameter, and the handler
+already detects it (`src/server/index.ts:1173`) while calling entitlement discovery
+without it (`:1073`). The value is right there and is thrown away.
+
+So version authority is a precedence chain, not a single lookup:
+
+1. **The inbound request's `client_version`**, when the caller supplied one. This is the
+   only value that is certainly the version of the client being answered.
+2. **The selected Codex runtime version** for background sync, where there is no inbound
+   request. `loadPersistedCodexRuntime()?.selectedVersion` (`src/codex/runtime.ts:256`)
+   performs no freshness validation and the file is written only by runtime-selection paths
+   (`:621`), so it can be absent after a persist failure, stale before selection runs, or
+   describe the binary OpenCodex chose rather than an externally launched client. Retained
+   sync does refresh runtime evidence first (`src/codex/catalog/sync.ts:1828`), which is
+   what makes it usable here and not elsewhere.
+3. **Neither available → do not ask.** Sending `0.0.0` manufactures a confirmed negative,
+   which is the whole defect. No trustworthy version means unconfirmed, and unconfirmed
+   already suppresses (`:321`) and routing already rejects
+   (`src/codex/auth-context.ts:458`). Fail closed on absent evidence, never on invented
+   evidence.
+
+This needs a real seam. `fetcher` (`:43`) can observe the URL but cannot choose the
+version, so `resolveCodexModelEntitlements` and `isDirectCallerEntitledToCodexModel`
+both take an explicit client version.
+
+## The cache has to be version-scoped
+
+`accountModelsCache` is keyed by account ID alone, with credential identity stored as a
+discriminator (`:30`, `:216`); the flight key is account plus credential identity
+(`:223`). Version must join both, or a roster fetched under one version keeps answering
+for another until the TTL expires.
+
+`cachedAvailableAccountGatedNativeModels` scans every cache entry (`:331`). Once two
+versions can be retained at once, that scan will leak a newer roster into an older
+client's projection — the `#2548` failure, arrived at from the opposite direction. It has
+to filter by the version being projected.
+
+`isCodexModelEntitlementSnapshotCurrent` validates credentials only (`:346`); a runtime
+version change during a gather needs the same stale-result protection.
+
+## Sub-defect B, correctly scoped
+
+`ocx models enable gpt-5.6-sol` fails because `/api/model-visibility` builds
+`supportedNative` from `nativeModelRows(config)`
+(`src/server/management/model-routes.ts:461-468`), which has already dropped the
+suppressed rows, so validation rejects at `:477-478`.
+
+Validating bare native IDs against the static `NATIVE_OPENAI_MODELS` set
+(`src/codex/catalog/native-models.ts:69`) fixes that, **unioned with** the existing
+account-qualified targets rather than replacing them.
+
+Being precise about what this buys: acceptance only clears `disabledModels` (`:532`).
+Entitlement still filters `nativeModelRows` (`src/codex/catalog/metadata.ts:424`) and
+routing stays gated. So B is **not** a manual escape from a false negative — the earlier
+draft of this page claimed that and was wrong. B removes a misleading 400 and lets an
+operator pre-clear an independent disable key. If no disable key exists, B changes nothing
+the user can see. A is the fix; B is a UX and configuration repair that stops the CLI from
+lying about why.
+
+## Verification
+
+**A** in `tests/codex-model-entitlements.test.ts` (fetch seam already exercised at
+`tests/codex-model-entitlements.test.ts:38`): a mock backend that returns a legacy-only
+roster below the threshold and the full roster at `0.146.0`. The wrong behavior asserted
+is the real one — *an entitled account is classified as denying GPT-5.6 because OpenCodex
+under-reports its own client version*. Named mutation: restore the `0.0.0` literal.
+
+A second case pins the precedence chain: with no trustworthy version, discovery must be
+**unconfirmed** rather than a confirmed negative. Named mutation: fall back to `0.0.0`;
+the account is then reported as positively denying the models.
+
+Cache identity gets its own case — fetch under one version, ask under another, assert a
+re-fetch. Named mutation: drop version from the cache key.
+
+**B** in `tests/model-visibility-management-api.test.ts`: with `disabledModels:
+["gpt-5.6-sol"]` and no entitlement cache, the PUT must be accepted and clear the entry,
+specifically not returning `invalid model visibility target`. Named mutation: derive
+`supportedNative` from `nativeModelRows` again.
+
+## What this does not claim
+
+The reporter supplied no captured `/codex/models` response, so I cannot prove their
+machine took the confirmed-negative branch rather than a transient failure. Both produce
+the same symptom. The version-filter explanation is what the source, the version boundary,
+and this repository's own measurement support, and the fix is correct either way — but if
+their roster was failing for another reason the models will still be missing afterwards,
+and the issue should be reopened with a redacted capture rather than assumed fixed.
+

src/codex/model-entitlements.ts

@@ -10,8 +10,60 @@ import {
   type NativeMainRefreshDependencies,
 } from "./main-account";
 import { ACCOUNT_GATED_NATIVE_OPENAI_MODELS } from "./catalog/native-models";
+import { loadPersistedCodexRuntime } from "./runtime";
 
-const CODEX_MODELS_URL = "https://chatgpt.com/backend-api/codex/models?client_version=0.0.0";
+const CODEX_MODELS_ENDPOINT = "https://chatgpt.com/backend-api/codex/models";
+
+/**
+ * Upstream filters this roster by the client version it is told, and `client_version` is a
+ * required parameter — a measured `0.60.0` returns zero models where `0.142.2` returns five
+ * (devlog/_fin/260817_native_gpt56_1m_context/001_measurement_evidence.md). Asking as
+ * `0.0.0` therefore describes what a prehistoric client may use, and treating that as the
+ * account's entitlement hides models the account genuinely owns (#2886).
+ *
+ * A version must be supplied by the caller. There is deliberately no default: inventing one
+ * either manufactures a confirmed negative (the defect) or advertises models the installed
+ * runtime cannot drive (#2548, from the opposite side).
+ */
+function codexModelsUrl(clientVersion: string): string {
+  return `${CODEX_MODELS_ENDPOINT}?client_version=${encodeURIComponent(clientVersion)}`;
+}
+
+/** A version string upstream can filter on. Rejects empty and the `0.0.0` placeholder. */
+export function isUsableCodexClientVersion(value: string | null | undefined): value is string {
+  if (typeof value !== "string") return false;
+  const trimmed = value.trim();
+  if (!trimmed || trimmed === "0.0.0") return false;
+  return /^\d+(\.\d+)*([-+][0-9A-Za-z.-]+)?$/.test(trimmed);
+}
+
+/**
+ * Version authority, in precedence order:
+ *
+ * 1. the inbound request's own `client_version` — the only value certainly describing the
+ *    client being answered;
+ * 2. the selected Codex runtime version, for background sync where no request exists.
+ *    Retained sync refreshes runtime evidence before discovery, which is what makes this
+ *    usable here; the persisted file itself carries no freshness guarantee.
+ *
+ * Neither available means the roster is not requested at all. Unconfirmed already suppresses
+ * the gated rows and routing already rejects them, so failing closed on ABSENT evidence is
+ * the existing contract; failing closed on INVENTED evidence was the bug.
+ */
+export function resolveCodexEntitlementClientVersion(
+  inbound?: string | null,
+  loadRuntime: () => { selectedVersion?: string | null } | null = loadPersistedCodexRuntime,
+): string | null {
+  if (isUsableCodexClientVersion(inbound)) return inbound.trim();
+  let persisted: { selectedVersion?: string | null } | null = null;
+  try {
+    persisted = loadRuntime();
+  } catch {
+    persisted = null;
+  }
+  const selected = persisted?.selectedVersion;
+  return isUsableCodexClientVersion(selected) ? selected.trim() : null;
+}
 const MODEL_ROSTER_TTL_MS = 5 * 60_000;
 const MODEL_ROSTER_FAILURE_TTL_MS = 15_000;
 const MODEL_ROSTER_TIMEOUT_MS = 8_000;
@@ -29,6 +81,12 @@ export interface CodexModelEntitlementCredentialSnapshot {
 
 interface CachedAccountModels {
   readonly credentialIdentity: string;
+  /**
+   * Client version this roster was fetched under. Upstream filters by it, so a roster is
+   * only an answer for that version — an entry fetched under one must never satisfy a read
+   * for another (#2886).
+   */
+  readonly clientVersion: string;
   readonly expiresAt: number;
   readonly models: ReadonlySet<string>;
   readonly confirmed: boolean;
@@ -42,6 +100,17 @@ export interface CodexModelEntitlementSnapshot {
 
 export interface CodexModelEntitlementResolveOptions {
   readonly fetcher?: typeof fetch;
+  /**
+   * Client version to ask upstream about. Absent means no trustworthy version was
+   * available, and discovery is skipped rather than asked under a placeholder.
+   */
+  readonly clientVersion?: string | null;
+  /**
+   * Test-only seam for the persisted-runtime half of the version precedence chain, so a case
+   * can reach the no-trustworthy-version branch without depending on the host's own runtime
+   * state file.
+   */
+  readonly loadPersistedRuntime?: () => { selectedVersion?: string | null } | null;
   readonly nativeMainRefreshDependencies?: NativeMainRefreshDependencies;
   readonly now?: number;
   readonly signal?: AbortSignal;
@@ -150,6 +219,7 @@ async function fetchAccountModels(
   credential: CodexModelEntitlementCredentialSnapshot,
   fetcher: typeof fetch,
   now: number,
+  clientVersion: string,
 ): Promise<CachedAccountModels> {
   const controller = new AbortController();
   const timer = setTimeout(() => controller.abort(new DOMException("Codex model discovery timed out", "TimeoutError")), MODEL_ROSTER_TIMEOUT_MS);
@@ -159,7 +229,7 @@ async function fetchAccountModels(
       Accept: "application/json",
     });
     if (credential.chatgptAccountId) headers.set("ChatGPT-Account-Id", credential.chatgptAccountId);
-    const response = await fetcher(CODEX_MODELS_URL, {
+    const response = await fetcher(codexModelsUrl(clientVersion), {
       headers,
       redirect: "error",
       signal: controller.signal,
@@ -174,13 +244,15 @@ async function fetchAccountModels(
       : null;
     return {
       credentialIdentity: credential.credentialIdentity,
+      clientVersion,
       expiresAt: now + (models ? MODEL_ROSTER_TTL_MS : MODEL_ROSTER_FAILURE_TTL_MS),
       models: models ?? new Set(),
       confirmed: models !== null,
     };
   } catch {
     return {
       credentialIdentity: credential.credentialIdentity,
+      clientVersion,
       expiresAt: now + MODEL_ROSTER_FAILURE_TTL_MS,
       models: new Set(),
       confirmed: false,
@@ -212,18 +284,22 @@ async function modelsForCredential(
   credential: CodexModelEntitlementCredentialSnapshot,
   fetcher: typeof fetch,
   now: number,
+  clientVersion: string,
 ): Promise<CachedAccountModels> {
   const cached = accountModelsCache.get(credential.accountId);
   if (
     cached
     && cached.credentialIdentity === credential.credentialIdentity
+    // Upstream filters by client version, so a roster fetched under a different one is a
+    // different question's answer and must not be reused.
+    && cached.clientVersion === clientVersion
     && cached.expiresAt > now
   ) return cached;
 
-  const flightKey = `${credential.accountId}\u0000${credential.credentialIdentity}`;
+  const flightKey = `${credential.accountId}\u0000${credential.credentialIdentity}\u0000${clientVersion}`;
   const existing = accountModelsFlights.get(flightKey);
   if (existing) return existing;
-  const flight = fetchAccountModels(credential, fetcher, now)
+  const flight = fetchAccountModels(credential, fetcher, now, clientVersion)
     .then(result => {
       if (currentCredentialIdentity(credential.accountId) === credential.credentialIdentity) {
         boundedCacheSet(credential.accountId, result);
@@ -269,16 +345,30 @@ export async function resolveCodexModelEntitlements(
 ): Promise<CodexModelEntitlementSnapshot> {
   const now = options.now ?? Date.now();
   const fetcher = options.fetcher ?? fetch;
+  const clientVersion = resolveCodexEntitlementClientVersion(
+    options.clientVersion,
+    options.loadPersistedRuntime ?? loadPersistedCodexRuntime,
+  );
   const allowedAccountIds = candidateAccountIds(config)
     .filter(accountId => !options.excludeAccountIds?.has(accountId));
   const credentialSnapshot = options.credentialSnapshot ?? accountCredentialSnapshot;
   const credentials = options.credentials
     ? [...options.credentials].filter(credential => !options.excludeAccountIds?.has(credential.accountId))
     : (await Promise.all(allowedAccountIds.map(accountId => credentialSnapshot(accountId, options))))
       .filter((value): value is CodexModelEntitlementCredentialSnapshot => value !== null);
+  // No trustworthy client version means the roster cannot be asked for meaningfully.
+  // Report every account as UNCONFIRMED rather than fetching under a placeholder that
+  // upstream would answer with an empty or truncated roster (#2886).
+  if (clientVersion === null) {
+    return {
+      modelsByAccount: new Map(credentials.map(credential => [credential.accountId, new Set<string>()])),
+      confirmedAccountIds: new Set<string>(),
+      credentialIdentities: new Map(credentials.map(credential => [credential.accountId, credential.credentialIdentity])),
+    };
+  }
   const results = await Promise.all(credentials.map(async credential => ({
     credential,
-    result: await modelsForCredential(credential, fetcher, now),
+    result: await modelsForCredential(credential, fetcher, now, clientVersion),
   })));
   return {
     modelsByAccount: new Map(results.map(({ credential, result }) => [credential.accountId, result.models])),
@@ -291,15 +381,19 @@ export async function resolveCodexModelEntitlements(
 export async function isDirectCallerEntitledToCodexModel(
   headers: Headers,
   modelId: string,
-  options: Pick<CodexModelEntitlementResolveOptions, "fetcher" | "now"> = {},
+  options: Pick<CodexModelEntitlementResolveOptions, "fetcher" | "now" | "clientVersion"> = {},
 ): Promise<boolean> {
   if (!ACCOUNT_GATED_NATIVE_OPENAI_MODELS.has(modelId)) return true;
   const credential = directCallerCredential(headers);
   if (!credential) return false;
+  const clientVersion = resolveCodexEntitlementClientVersion(options.clientVersion);
+  // Fail closed on absent evidence, exactly as an unconfirmed roster does.
+  if (clientVersion === null) return false;
   const result = await modelsForCredential(
     credential,
     options.fetcher ?? fetch,
     options.now ?? Date.now(),
+    clientVersion,
   );
   return result.confirmed && result.models.has(modelId);
 }
@@ -331,11 +425,22 @@ export function availableAccountGatedNativeModels(
 export function cachedAvailableAccountGatedNativeModels(
   now = Date.now(),
   eligibleAccountIds?: ReadonlySet<string>,
+  clientVersion?: string | null,
 ): ReadonlySet<string> {
+  // Entries fetched under a different client version answer a different question, so a caller
+  // that knows which version it is projecting for must say so — otherwise a newer client's
+  // roster leaks into an older client's projection (#2548 from the opposite direction).
+  //
+  // Omitting the argument deliberately does NOT filter. This is a synchronous read of
+  // whatever discovery already proved, and its callers (catalog metadata) are not
+  // request-scoped: resolving a version here would silently discard every entry fetched under
+  // a different one, which is a suppression this function has no evidence to justify.
+  const version = isUsableCodexClientVersion(clientVersion) ? clientVersion.trim() : null;
   return new Set([...ACCOUNT_GATED_NATIVE_OPENAI_MODELS].filter(modelId => (
     [...accountModelsCache].some(([accountId, entry]) => (
       (!eligibleAccountIds || eligibleAccountIds.has(accountId))
       && !accountId.startsWith(DIRECT_CALLER_ACCOUNT_PREFIX)
+      && (version === null || entry.clientVersion === version)
       && entry.confirmed
       && entry.expiresAt > now
       && entry.models.has(modelId)
@@ -363,9 +468,11 @@ export function seedCodexModelEntitlementsForTests(
   accountId: string,
   models: readonly string[],
   now = Date.now(),
+  clientVersion = "0.146.0",
 ): void {
   boundedCacheSet(accountId, {
     credentialIdentity: `test:${accountId}`,
+    clientVersion,
     expiresAt: now + MODEL_ROSTER_TTL_MS,
     models: new Set(models),
     confirmed: true,

src/server/index.ts

@@ -1070,7 +1070,10 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server<W
         try {
           [goModels, modelEntitlements] = await Promise.all([
             fetchAllModels(config),
-            resolveCodexModelEntitlements(config),
+            // Codex sends its own client_version on this request, and upstream filters the
+            // entitlement roster by it. Passing it through is what stops an entitled account
+            // being told it cannot use models a newer client can (#2886).
+            resolveCodexModelEntitlements(config, { clientVersion: url.searchParams.get("client_version") }),
           ]);
         } catch (error) {
           if (error instanceof CatalogGatherBusyError) {