diff --git a/devlog/_plan/260904_gated_client_version_floor/000_research.md b/devlog/_plan/260904_gated_client_version_floor/000_research.md new file mode 100644 index 0000000000..1df7017ac6 --- /dev/null +++ b/devlog/_plan/260904_gated_client_version_floor/000_research.md @@ -0,0 +1,129 @@ +# 260904 — Gated client-version floor: a stale Codex CLI hides GPT-5.6 + +## Symptom + +On a host running opencodex `dev` (2.43.0), `gpt-5.6-sol`, `gpt-5.6-terra` and +`gpt-5.6-luna` do not appear: not in the Codex catalog, not in `/v1/models`, not in the +dashboard model rows, not in the desktop projection. The account owns them. The same +account sees them from other installs. + +A second, independent symptom on the same host: `~/.opencodex/codex-runtime-clamp.json` +records `removedEfforts: ["max","ultra"]` across the whole 5.6 family. + +## Reproduction (2026-09-04, this host) + +```text +~/.opencodex/codex-runtime.json selectedVersion = "0.141.0" source = "configured" +codex --version codex-cli 0.141.0 +npm view @openai/codex version 0.153.2 +~/.opencodex/codex-runtime-clamp.json runtimeVersion 0.141.0, removedEfforts [max, ultra] +``` + +The installed CLI is twelve minor versions behind what upstream publishes. + +## Mechanism + +`src/codex/model-entitlements.ts` resolves the `client_version` it asks upstream with in +three tiers (`resolveCodexEntitlementClientVersion`, ~line 185): + +1. the inbound request's own `client_version`; +2. the persisted `codex-runtime.json` `selectedVersion`; +3. `GATED_MODEL_CLIENT_VERSION_FLOOR` — composed as the highest of the snapshot-derived + floor, the measured `MEASURED_GATED_CLIENT_VERSION_MINIMUM = "0.144.0"`, and the + `"0.142.2"` fallback. + +Upstream filters `GET /backend-api/codex/models` by that parameter. Measurement recorded in +`devlog/_fin/260817_native_gpt56_1m_context/001_measurement_evidence.md`, and independently +reproduced by the #2886 and #3022 reporters: `0.142.2` returns five models with no gpt-5.6; +`0.144.0` and above return the gated rows. + +Tier 2 is unconditional. It hands back `0.141.0` — a real, probed, honest version that is +nonetheless below the floor upstream needs. Background discovery therefore asks a question +whose truthful answer contains no gpt-5.6, and the rows disappear. + +PR #3035 (`4bdc0f6fb`) introduced the `0.144.0` measurement precisely to stop this, but wired +it into tier 3 only. Tier 2 was left to speak for itself. + +## The shape of the defect + +The clearest statement of the bug is a comparison of two hosts: + +| Host | Tier 2 | Version asked | gpt-5.6 visible | +|------|--------|---------------|-----------------| +| No Codex CLI installed at all | absent | `0.144.0` (floor) | yes | +| Codex CLI 0.141.0 installed | `"0.141.0"` | `0.141.0` | **no** | + +Having an old runtime is worse than having no runtime. That inversion is not a policy +anyone chose; it falls out of tier 2 being unconditional while tier 3 is floored. The fix is +to make the two tiers agree about the minimum question worth asking. + +## Why the absence is not evidence + +The codebase already agrees with this reasoning elsewhere. `fetchAccountModels` treats an +empty roster as unconfirmed on the 15s failure TTL rather than a confirmed denial, and +`codexModelEntitlementStateForRoster` returns `"unknown"` — not `"denied"` — when a gated +slug is missing from a roster fetched below its recorded minimum. Both guards fire correctly +here, which is why the models are merely invisible rather than actively denied. The guards +prevent a wrong answer; they cannot manufacture the right one. Only asking a better question +can do that. + +## Two symptoms, two causes, one stale runtime + +They must not be conflated: + +- **Missing rows** is an *account entitlement* question answered by upstream, filtered by the + `client_version` we send. Fixable by asking under the floor. +- **Missing `max`/`ultra`** is a *local runtime capability* question. `src/codex/catalog/effort.ts` + probes `codex debug models --bundled` and intersects the effort vocabulary the installed + binary understands. A 0.141.0 binary genuinely does not know those rungs, so clamping them + is honest and must stay. Advertising an effort the local runtime cannot express is #2548 + from the opposite side. + +This unit fixes the first and deliberately leaves the second alone. + +## Alternative considered: detect a newer Codex App runtime + +The owner asked whether opencodex could instead prefer a newer runtime shipped by the Codex +desktop app. Investigated and rejected for this unit: + +- `src/codex/runtime.ts` enumerates candidates in priority order (environment, configured, + shim, PATH, fallback) and deliberately sticks with the configured one; a newer candidate is + reported as `newerAvailable`, never silently selected. Changing that is a separate policy + decision about which binary drives sync and the clamp. +- On this host the desktop package is `OpenAI.Codex 26.825.6671.0`. That version line is not + comparable to a codex-cli `0.14x` version, and the bundled executable's version metadata is + blank. There is installation evidence but no trustworthy *version* signal. +- No cross-platform equivalent exists today. + +So app detection would invent a new, unmeasured authority to work around a floor we have +already measured. The floor is the evidence-backed fix. + +## Risk register (from the audit lanes) + +1. **`unknown` becomes `denied`.** The resolved version is recorded on the cache entry and + read back by `hasUnknownGatedAbsence` and `codexModelEntitlementStateForRoster`. If we ask + at `0.144.0` and upstream still omits the model, the answer is recorded as a denial on the + 5-minute TTL instead of unknown on 15s. This is correct: we really did ask at an adequate + version. It is a strengthening of negative authority, and it is honest only so long as the + floor itself is honest. +2. **Positive answers under a version the local runtime does not match (#2548).** A model may + be granted while the installed CLI is 0.141.0. This is acceptable because opencodex injects + `model_catalog_json` — model availability is the proxy's question, and the runtime's own + capability limits are enforced separately by the effort clamp, which stays untouched. +3. **Do not clamp anything persisted.** `selectedVersion` is real probe evidence consumed by + runtime identity, catalog cache keys, `X-Codex-Version` and install provenance. The clamp + must live only in the entitlement resolver. + +## Existing coverage + +Audited `tests/codex-model-entitlements.test.ts`: no current assertion flips under a tier-2 +floor, because every exact-resolution test either has no runtime, or a runtime above the floor +(`0.145.1`, `0.147.3`), or supplies the old version as tier 1 inbound. The precise gap is +`inbound = null` plus a usable persisted version *below* the floor. That is the regression to +write. + +## Work phases + +- `010` — floor-aware tier 2 in the entitlement resolver, with regressions. +- `020` — projection verification on a stale-runtime host. +- `030` — landing: full suite, PR to `dev`, CI-green merge. diff --git a/devlog/_plan/260904_gated_client_version_floor/005_audit_synthesis.md b/devlog/_plan/260904_gated_client_version_floor/005_audit_synthesis.md new file mode 100644 index 0000000000..73f20c920b --- /dev/null +++ b/devlog/_plan/260904_gated_client_version_floor/005_audit_synthesis.md @@ -0,0 +1,77 @@ +# 005 — Audit synthesis (round 1: FAIL) + +An adversarial plan auditor returned FAIL with six blockers. Four are accepted and change the +plan; two are corrected. Each was checked against the tree before acceptance. + +## Accepted 1 — tier 2 is not background-only, and the plan must say what it is + +The draft justified the clamp as "background discovery only". That is false. +`isDirectCallerEntitledToCodexModel` (~line 997) and both authorization paths in +`src/codex/auth-context.ts` (caller-owned Direct at ~408, stored-main substitution at ~435) +reach tier 2 as well, because they are inbound requests that simply do not carry a +`client_version`. So the clamp does change request-path authorization. + +It should. The distinction that matters is not background-vs-inbound, it is **which question +is being asked**: + +- *Does this ACCOUNT own gpt-5.6?* is a property of the account. Upstream merely happens to + filter its answer by `client_version`, so asking under a stale version returns a wrong + answer to a question the version has no bearing on. +- *Can THIS CLIENT drive gpt-5.6?* is a property of the client, and only an inbound + `client_version` can answer it. + +A caller that supplies no version is asking the first question. Flooring it is therefore +correct on every one of those paths, not a side effect to be tolerated. The plan now states +this as the policy rather than mis-describing the call sites. + +## Accepted 2 — WP3 promised something the fix does not deliver + +The draft claimed `/v1/models?client_version=0.141.0` would list the 5.6 rows. It will not, +and it should not. Tier 1 returns the inbound version verbatim and short-circuits before +tier 2 (~line 190). A client that declares itself 0.141.0 is answered as 0.141.0. + +That is deliberate, and flooring tier 1 was considered and rejected: + +- It would advertise rows to a client that told us it cannot drive them (#2548). +- It would break the existing recorded contract in + `"an omitted gated slug below its minimum is unknown and uses the failure TTL"`, which + supplies `0.140.0` as inbound and requires `unknown`, not `denied`. Flooring tier 1 would + record `0.144.0` on the cache entry and flip that to `denied` on the 5-minute TTL. + +So the honest scope is: every path that does not carry an inbound version is fixed. A stale +client that announces its own version keeps being answered for that version, and the real +remedy there is upgrading the CLI. WP3's acceptance list is corrected accordingly. + +## Accepted 3 — the regression list mislabelled controls as regressions + +Only cases 1 and 2 are RED-before-fix. Cases 3-6 are invariant controls that pass both +before and after; the memo-purity case is outright vacuous against this defect because +`memoizeRuntimeVersionForTests` returns `memoizedPersistedRuntimeVersion` directly and never +passes through the resolver. Relabelled: 1-2 regressions with a mutation proof, 3-5 controls, +6 dropped as vacuous. A control that cannot fail is not evidence, and calling it one inflates +the apparent coverage of the change. + +## Accepted 4 — cache identity changes and the plan did not say so + +The resolved version is part of `cacheKeyFor` (~line 394) and of the in-flight coalescing key +(~line 632), and distinct versions are capped at four per account. After the clamp, an inbound +`0.141.0` request and an unversioned caller occupy two different entries and no longer +coalesce. That is required for correctness — they are different questions — but it is a real +consequence and now has a test. + +## Corrected 5 — the effort-clamp wording overstated the evidence + +The auditor is right that `codex debug models --bundled` proves the 0.141.0 bundled catalog +does not *advertise* `max`/`ultra`, not that the binary cannot parse them. Wording in +`000_research.md` softened to what was measured. The decision is unchanged and conservative: +keep the clamp. Shipping a model whose advertised ladder the local runtime does not list is +the #2548 failure mode, and removing the clamp to make a row look complete would trade a +visible gap for a failing request. + +## Corrected 6 — dashboard first-poll degradation is pre-existing + +`model-rows.ts` waits ~3s while an entitlement fetch may take up to 8s, so a cold first poll +can return without the rows. True, and unchanged by this work — it is a property of the +freshness wait, not of the version floor. Recorded here so it is not rediscovered as a +regression; WP3 asserts eventual visibility on a warm read rather than pretending the first +cold poll is deterministic. diff --git a/devlog/_plan/260904_gated_client_version_floor/010_wp2_floor_aware_tier2.md b/devlog/_plan/260904_gated_client_version_floor/010_wp2_floor_aware_tier2.md new file mode 100644 index 0000000000..9eaae6f43d --- /dev/null +++ b/devlog/_plan/260904_gated_client_version_floor/010_wp2_floor_aware_tier2.md @@ -0,0 +1,96 @@ +# 010 — wp2: floor-aware tier 2 in the entitlement resolver + +## Change + +One file: `src/codex/model-entitlements.ts`. + +`resolveCodexEntitlementClientVersion` currently ends: + +```ts +return selected ?? GATED_MODEL_CLIENT_VERSION_FLOOR; +``` + +It becomes a clamp rather than a fallback: whatever tier 2 produces, the question we put to +upstream is never below `GATED_MODEL_CLIENT_VERSION_FLOOR`. + +```ts +if (selected === null) return GATED_MODEL_CLIENT_VERSION_FLOOR; +return compareClientVersions(selected, GATED_MODEL_CLIENT_VERSION_FLOOR) >= 0 + ? selected + : GATED_MODEL_CLIENT_VERSION_FLOOR; +``` + +Expressed through a small named helper so the intent reads at the call site, and so the +existing `compareClientVersions` stays the single ordering authority. + +## The policy, stated exactly + +The clamp is not "background only" — `isDirectCallerEntitledToCodexModel` and both +`src/codex/auth-context.ts` authorization paths also reach tier 2, because they are inbound +requests carrying no `client_version`. The rule is about which question is asked: + +- **No inbound version supplied** -> the caller is asking whether the ACCOUNT owns the model. + Upstream only incidentally filters that answer by version, so ask at no less than the floor. +- **An inbound version supplied** -> the caller is asking what THAT CLIENT may use. Answer for + that version, verbatim. + +## What must not change + +- **Tier 1 keeps absolute precedence.** If Codex 0.140.0 asks, it is told what 0.140.0 can + use. Clamping there would advertise rows that client cannot drive (#2548) and would break + the existing `"an omitted gated slug below its minimum is unknown and uses the failure TTL"` + contract, which supplies `0.140.0` as inbound and requires `unknown` rather than `denied`. +- **A runtime at or above the floor still wins.** `0.145.1` resolves to `0.145.1`, not to the + floor. The clamp raises; it never lowers. +- **`readRuntimeVersion` and `memoizedPersistedRuntimeVersion` stay exact.** They report what + is on disk. The clamp is applied by the resolver on the way out, so + `memoizeRuntimeVersionForTests` and every non-entitlement consumer of `selectedVersion` + (runtime identity, catalog cache keys, `X-Codex-Version`, install provenance) are untouched. +- **No new grant without upstream evidence.** The clamp changes only which version we ask + under. `granted` still requires the model to be present in the returned roster. + +## Why the clamp is not "inventing a version" + +The floor is not a guess. It is composed in this same file from the highest of: the +`minimal_client_version` this build's own bundled snapshot records for the gated slugs, the +measured `0.144.0`, and the `0.142.2` fallback. Asking under it is the narrowest question +that can still return the models this build claims to support. Tier 3 has asked exactly that +question since #3035; this change stops a stale tier 2 from asking a worse one. + +## Regressions and controls + +All in `tests/codex-model-entitlements.test.ts`. Only the first two can fail before the fix; +the rest are invariants this change must not disturb, and are labelled as such rather than +counted as coverage. + +RED before the fix, GREEN after: + +1. `inbound = null`, persisted `0.141.0` -> the resolver returns the floor, and the version + actually sent upstream is the floor. RED today: `0.141.0` both times. +2. End-to-end on the same host: upstream returns the gated rows only at or above `0.144.0` + -> `gpt-5.6-sol` projects `granted` and reaches `availableAccountGatedNativeModels`. + RED today: absent. + +Controls (pass before and after): + +3. Tier 1 verbatim: inbound `0.140.0` with persisted `0.141.0` resolves `0.140.0`, and a + gated slug missing from that roster stays `unknown`, not `denied`. +4. A runtime at or above the floor is preferred: persisted `0.145.1` -> `0.145.1`. +5. No fabricated grant: asked at the floor, a roster that genuinely omits the model does not + yield `granted`. + +Dropped as vacuous: the proposed "memo purity" case. `memoizeRuntimeVersionForTests` returns +`memoizedPersistedRuntimeVersion` directly and never passes through the resolver, so it +cannot observe this defect in either direction. + +Cache identity, added after the audit: + +6. The resolved version is part of `cacheKeyFor` and of the in-flight key, so an inbound + `0.141.0` caller and an unversioned caller now occupy separate entries and issue two + fetches rather than coalescing. Asserted directly: two fetches, `0.141.0`-scoped absence, + floor-scoped visibility. + +## Verification + +`bun test tests/codex-model-entitlements.test.ts`, plus `tests/claude-models-discovery.test.ts` +(touched by #3035 for the same seam), then `bun run typecheck`. diff --git a/devlog/_plan/260904_gated_client_version_floor/020_wp3_projection_verification.md b/devlog/_plan/260904_gated_client_version_floor/020_wp3_projection_verification.md new file mode 100644 index 0000000000..4186ecbce3 --- /dev/null +++ b/devlog/_plan/260904_gated_client_version_floor/020_wp3_projection_verification.md @@ -0,0 +1,37 @@ +# 020 — wp3: projection verification on a stale-runtime host + +The resolver fix is only meaningful if the rows reach the surfaces the user actually looks at. +This phase proves the path from a granted entitlement to a visible model, on a host whose +persisted runtime is `0.141.0`. + +## What to verify + +1. `availableAccountGatedNativeModels` includes sol/terra/luna once the roster confirms them. +2. The bare OpenAI list shape (no `client_version`) lists them. +3. The dashboard model rows path (`src/server/management/model-rows.ts`, which passes no + client version and therefore depends entirely on this fix) lists them. +4. The effort clamp still removes `max`/`ultra` for a 0.141.0 runtime. This is the control: + the fix must NOT accidentally re-advertise efforts the local binary does not list. + +Explicitly NOT claimed: `/v1/models?client_version=0.141.0` continues to omit the rows. Tier 1 +answers a self-declared stale client for the version it declared, which is the #2548 contract. +A stale Codex CLI is fixed by upgrading the CLI, not by the proxy overriding what the client +said about itself. See `005_audit_synthesis.md`. + +Also not claimed: that a cold first dashboard poll always shows the rows. `model-rows.ts` +waits ~3s while a fetch may take up to 8s. That degradation predates this unit; WP3 asserts +visibility on a warm read. + +Point 4 matters as much as the first three. Fixing entitlement visibility while silently +widening the effort ladder would trade a missing-model bug for a broken-request bug. + +## Method + +Focused tests over the projection helpers, plus a scripted resolution against a fake upstream +that mirrors the measured behaviour (gated rows returned at or above `0.144.0`, absent below). +No live account credentials are used, and no request bodies or tokens are logged. + +## Out of scope + +Changing runtime selection, the clamp, or the desktop-app detection question. Those are +recorded in `000_research.md` as considered and deferred. diff --git a/devlog/_plan/260904_gated_client_version_floor/030_wp4_landing.md b/devlog/_plan/260904_gated_client_version_floor/030_wp4_landing.md new file mode 100644 index 0000000000..a3555a5765 --- /dev/null +++ b/devlog/_plan/260904_gated_client_version_floor/030_wp4_landing.md @@ -0,0 +1,13 @@ +# 030 — wp4: landing + +1. `bun run typecheck` +2. `bun run privacy:scan` +3. `bun run test` (full suite; this is the PR-ready gate) +4. Branch `codex/260904-gated-client-version-floor` off current `dev`, targeting `dev`. +5. PR using `.github/PULL_REQUEST_TEMPLATE.md` with Summary, Verification and Checklist filled. +6. Push is owner-approved for this unit, including `--no-verify`. +7. Merge once CI is green, also owner-approved. + +Known container-only failures listed in `AGENTS.md` are not regressions; on this Windows host +the service/systemd cases may behave differently again. Any failure is compared against a +baseline run on the unmodified tree before it is called a regression. diff --git a/devlog/_plan/260904_gated_client_version_floor/070_outcome.md b/devlog/_plan/260904_gated_client_version_floor/070_outcome.md new file mode 100644 index 0000000000..4bdc9bf6b4 --- /dev/null +++ b/devlog/_plan/260904_gated_client_version_floor/070_outcome.md @@ -0,0 +1,65 @@ +# 070 — Outcome + +## What shipped + +`GATED_MODEL_CLIENT_VERSION_FLOOR` became a lower bound on tier 2 of +`resolveCodexEntitlementClientVersion` instead of only a tier-3 fallback. A host whose +persisted Codex CLI is real but older than the measured floor now asks upstream at the floor +and keeps gpt-5.6-sol/terra/luna. A runtime at or above the floor is preserved exactly, and an +inbound `client_version` still wins outright. + +Two commits: + +1. the fix, three regressions, and this plan unit; +2. review findings — documentation of the prerelease ordering gap, a note that the tier-1 + absence guard is now reachable only from tier 1, and a fixture that tracks the floor + instead of a hardcoded minor. No behaviour change. + +## Verification + +```text +bun test tests/codex-model-entitlements.test.ts 49 pass 0 fail (exit 0) +bun test claude-models-discovery + codex-catalog + + codex-catalog-sync-hardening 303 pass 0 fail (exit 0) +bun run typecheck exit 0 +bun run privacy:scan Privacy scan passed +``` + +The three regressions were driven RED against the unfixed source first, each failing with +`Expected: "0.144.0" Received: "0.141.0"`, and GREEN after. + +On the real host, before and after: + +```text +persisted = 0.141.0 +floor = 0.144.0 +resolve(no inbound) = 0.141.0 -> 0.144.0 +resolve(inbound 0.141.0) = 0.141.0 (verbatim, by design) +``` + +## Full-suite failures: investigated, not regressions + +Four tests failed in the full run. Each was re-run in isolation and against clean `dev` +(`c116dc532`): + +- `routing profile management editor API > PUT update migrates config references...` and + `POST /api/client-integrations/restore > distinguishes an unknown operation...` fail + identically on clean `dev`. Both are 5s/8s timeouts on this slow Windows host. +- Two `server local API auth` cases failed under full-suite load but the file passes 103/0 + when run alone on this branch, and the clean-`dev` baseline for it was 0 fail. Load-related + flake, and notably not the same two cases across runs. + +## Reviewer + +An independent opus-5 review returned PASS: the clamp is a genuine `max` that cannot lower, +tier 1 is untouched, no other `selectedVersion` consumer changes, all three tests are genuine +regressions, and no grant can be manufactured because the clamp only changes the query string. +All four of its findings were applied. + +## Known limits, recorded deliberately + +- `/v1/models?client_version=0.141.0` still omits the rows. A self-declared stale client is + answered for the version it declared (#2548); the remedy there is upgrading the CLI. +- `max` and `ultra` stay clamped off on a 0.141.0 host. That is a local runtime capability + limit, not an entitlement one, and re-advertising them would produce failing requests. +- The measured `0.144.0` constant is now load-bearing alone on the background path. diff --git a/src/codex/model-entitlements.ts b/src/codex/model-entitlements.ts index 7ae5baddb6..553fb501b7 100644 --- a/src/codex/model-entitlements.ts +++ b/src/codex/model-entitlements.ts @@ -149,6 +149,13 @@ export function compareClientVersionsForTests(left: string, right: string): numb } /** Numeric-segment comparison. Only used to pick the highest floor in a known-good set. */ +// Prerelease and build suffixes are deliberately NOT ordered. Splitting on [.+-] turns the +// suffix into a non-finite segment that reads as 0, so `0.144.0-rc.1` sorts at or above +// `0.144.0` rather than below it, the inverse of semver. That is tolerable because every +// version this ranks against is a release version: the gated floor, the measured minimum, and +// the snapshot's `minimal_client_version` rows. A prerelease runtime is therefore passed +// through exactly as it was before the floor bound tier 2, so this is not a new hazard. Order +// the suffix properly before reusing this anywhere a prerelease has to sort below its release. function compareClientVersions(left: string, right: string): number { const l = left.split(/[.+-]/).map(Number); const r = right.split(/[.+-]/).map(Number); @@ -165,9 +172,10 @@ function compareClientVersions(left: string, right: string): number { * * 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. +* 2. the selected Codex runtime version, for callers with no request of their own — but never + * below `GATED_MODEL_CLIENT_VERSION_FLOOR`. Retained sync refreshes runtime evidence before + * discovery, which is what makes this usable here; the persisted file itself carries no + * freshness guarantee. * 3. the floor this build's own bundled roster records for the models being gated * (`GATED_MODEL_CLIENT_VERSION_FLOOR`). * @@ -178,6 +186,25 @@ function compareClientVersions(left: string, right: string): number { * snapshot states the gated models require, so asking under it is the narrowest question * that can still return them. * + * The floor binds tier 2 as well, and that is the whole of #3436. #3022 gave tier 3 the + * measured minimum but left tier 2 to speak for itself, so a host whose persisted runtime was + * REAL but OLD — 0.141.0 against a measured 0.144.0 — asked upstream under a version upstream + * filters on, got an honest roster with no gpt-5.6, and lost sol/terra/luna everywhere. That + * made an outdated CLI strictly worse than no CLI at all, since the runtime-less host already + * asked at the floor and kept its models. The clamp only ever raises: a runtime at or above + * the floor is preserved exactly, because a newer client can drive models the floor cannot + * name. + * + * Which tier answers is a question about WHICH QUESTION IS BEING ASKED, not about background + * versus request path — `isDirectCallerEntitledToCodexModel` and both authorization paths in + * `auth-context.ts` are inbound requests that reach tier 2 because they carry no version: + * + * - No inbound version: the caller is asking whether the ACCOUNT owns the model. Upstream + * only incidentally filters that answer by version, so ask at no less than the floor. + * - An inbound version: the caller is asking what THAT CLIENT may use. Answer verbatim, even + * when it is older than the floor. Clamping there would advertise rows the client told us + * it cannot drive (#2548) and would turn an honest `unknown` into a cached `denied`. + * * There is deliberately no `0.0.0`-style fallback. A placeholder describes a client that * predates every gated model, which is what made upstream answer with an empty roster and * turned absent evidence into a manufactured confirmed negative (#2886). @@ -195,7 +222,23 @@ export function resolveCodexEntitlementClientVersion( const selected = bypass ? readRuntimeVersion(loadRuntime) : memoizedPersistedRuntimeVersion(loadRuntime, options.now ?? Date.now()); - return selected ?? GATED_MODEL_CLIENT_VERSION_FLOOR; + return raisedToGatedFloor(selected); +} + +/** + * The gated floor as a lower bound rather than a fallback. + * + * Applied only on the way out of the resolver. `readRuntimeVersion` and + * `memoizedPersistedRuntimeVersion` keep reporting what is actually on disk, because + * `selectedVersion` is probe evidence that runtime identity, catalog cache keys, + * `X-Codex-Version` and install provenance all read for their own reasons. Clamping the + * persisted value itself would corrupt every one of them to fix one question. + */ +function raisedToGatedFloor(selected: string | null): string { + if (selected === null) return GATED_MODEL_CLIENT_VERSION_FLOOR; + return compareClientVersions(selected, GATED_MODEL_CLIENT_VERSION_FLOOR) >= 0 + ? selected + : GATED_MODEL_CLIENT_VERSION_FLOOR; } const MODEL_ROSTER_TTL_MS = 5 * 60_000; @@ -575,6 +618,13 @@ async function fetchAccountModels( return unconfirmedAccountModels(credential, clientVersion, now, { kind: "parsed-empty" }); } const hasUnknownGatedAbsence = [...ACCOUNT_GATED_NATIVE_MODEL_MINIMUM_CLIENT_VERSIONS] + // Reachable only through tier 1, an inbound client_version below the floor. Every other + // resolution is now structurally >= every recorded minimum, because the floor is the max + // of the derived value and the same measured constant these minimums hold. So this is the + // escape hatch for a self-declared old client, not live protection on the background path: + // there, an absence really was asked for at an adequate version and is a denial. If + // upstream ever raises its true requirement above the measured constant, that constant is + // the only thing standing between an entitled account and a five-minute cached denial. .some(([modelId, minimum]) => ( !models.has(modelId) && compareClientVersions(clientVersion, minimum) < 0 )); diff --git a/tests/codex-model-entitlements.test.ts b/tests/codex-model-entitlements.test.ts index 683b64065d..d23738b784 100644 --- a/tests/codex-model-entitlements.test.ts +++ b/tests/codex-model-entitlements.test.ts @@ -1081,6 +1081,88 @@ describe("entitlement client version (#2886)", () => { expect(snapshot.modelsByAccount.has("main")).toBe(true); }); + test("a persisted runtime BELOW the gated floor still asks under the floor", async () => { + // #3022 restored the floor for a host with NO runtime. A host with an OLD one stayed + // broken, and ended up worse off than a host with none: tier 2 returned its honest + // 0.141.0, upstream truthfully answered without gpt-5.6, and the rows vanished. Having + // an outdated Codex CLI must not be worse than having no Codex CLI at all. + // + // A caller that supplies no client version is asking whether the ACCOUNT owns the model. + // Upstream only incidentally filters that answer by version, so the question is asked at + // no less than the version this build has measured upstream to honour. + const stale = () => ({ selectedVersion: "0.141.0" }); + expect(resolveCodexEntitlementClientVersion(null, stale, { bypassRuntimeMemo: true })) + .toBe(GATED_MODEL_CLIENT_VERSION_FLOOR); + + const seen: string[] = []; + const snapshot = await resolveCodexModelEntitlements({ codexAccounts: [] }, { + credentials: [credential("main")], + fetcher: (async (input: RequestInfo | URL) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + const version = url.searchParams.get("client_version") ?? ""; + seen.push(version); + // Mirrors the measured upstream behaviour: the gated rows appear only at >= 0.144.0. + // Gated against the floor itself rather than a hardcoded minor, so raising the floor + // moves the fixture with it instead of silently mis-gating. + return compareClientVersionsForTests(version, GATED_MODEL_CLIENT_VERSION_FLOOR) >= 0 + ? roster("gpt-5.5", SOL, TERRA, LUNA) + : roster("gpt-5.5"); + }) as typeof fetch, + now: 1_000, + clientVersion: null, + loadPersistedRuntime: stale, + }); + + // The stale version is never what upstream is asked. + expect(seen).toEqual([GATED_MODEL_CLIENT_VERSION_FLOOR]); + expect(projectedEntitlementState(snapshot, "main", SOL)).toBe("granted"); + expect([...availableAccountGatedNativeModels(snapshot)]).toEqual([SOL, TERRA, LUNA]); + }); + + test("the floor raises a stale runtime but never lowers a current one", () => { + const ask = (runtime: string | null) => resolveCodexEntitlementClientVersion( + null, + () => (runtime === null ? null : { selectedVersion: runtime }), + { bypassRuntimeMemo: true }, + ); + // Below the floor: clamped up. + expect(ask("0.141.0")).toBe(GATED_MODEL_CLIENT_VERSION_FLOOR); + expect(ask("0.100.0")).toBe(GATED_MODEL_CLIENT_VERSION_FLOOR); + // At the floor: itself, which is also the floor. + expect(ask(GATED_MODEL_CLIENT_VERSION_FLOOR)).toBe(GATED_MODEL_CLIENT_VERSION_FLOOR); + // Above the floor: preserved exactly. The clamp raises; it must never lower, or a newer + // runtime would be under-reported and lose the models only it can drive. + expect(ask("0.145.1")).toBe("0.145.1"); + expect(ask("0.153.2")).toBe("0.153.2"); + // An inbound version still wins outright, stale or not — that caller is asking what IT + // may use, and answering for a different version is #2548 in one direction or the other. + expect(resolveCodexEntitlementClientVersion("0.140.0", () => ({ selectedVersion: "0.150.0" }), { bypassRuntimeMemo: true })) + .toBe("0.140.0"); + expect(resolveCodexEntitlementClientVersion("0.150.0", () => ({ selectedVersion: "0.141.0" }), { bypassRuntimeMemo: true })) + .toBe("0.150.0"); + }); + + test("the floor asks a better question; it does not invent a grant", async () => { + // The clamp changes which version is asked, never what the answer means. An account that + // genuinely does not own the model is not granted it merely because we asked politely. + const seen: string[] = []; + const snapshot = await resolveCodexModelEntitlements({ codexAccounts: [] }, { + credentials: [credential("main")], + fetcher: (async (input: RequestInfo | URL) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + seen.push(url.searchParams.get("client_version") ?? ""); + return roster("gpt-5.5"); + }) as typeof fetch, + now: 1_000, + clientVersion: null, + loadPersistedRuntime: () => ({ selectedVersion: "0.141.0" }), + }); + expect(seen).toEqual([GATED_MODEL_CLIENT_VERSION_FLOOR]); + // Asked at an adequate version and still absent: that is a real denial, not doubt. + expect(projectedEntitlementState(snapshot, "main", SOL)).toBe("denied"); + expect(availableAccountGatedNativeModels(snapshot).has(SOL)).toBe(false); + }); + test("the gated floor derivation picks the highest usable gated version", () => { // Asserted on INDEPENDENT fixtures, not the shipped snapshot. An earlier version of this test // compared the constant against the bundled data and reimplemented the comparator, so it