From 9627fe2d611fb16d1ee8ede4f79750395bc0e918 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 11 Sep 2026 08:41:27 +0900 Subject: [PATCH 1/2] fix(client): refuse a hub catalog the local Codex CLI cannot parse A connected client reported connected, catalog present, token owned and rotation clean, while its installed Codex CLI exited before making a single request: failed to parse model_catalog_json ... unknown variant `max`, expected one of `none`, `minimal`, `low`, `medium`, `high`, `xhigh` The connection state answered a different question from the one the operator was asking. It proved the hub was reachable and the credential worked; nothing on that path proved the selected local runtime could consume what was written. validateRemoteCatalog checks JSON shape only, and connect wrote the hub's bytes verbatim, so the effort clamp that already exists for local catalog sync never saw the downloaded file. The client path now establishes compatibility before it materialises anything. catalogEffortCompatibility reports which reasoning levels the observed local ladder would reject, without mutating the catalog - the clamp beside it is mutate-and-continue, which is right when this process owns the file it is about to write and wrong for a catalog that must keep agreeing with hub truth. Both hub-download writes, connect and sync, are gated on it and fail closed: the download is refused before the write, so the previous known-good catalog is still there, no connection is committed, and no success is printed. The two restore paths stay ungated, since refusing to restore a catalog this machine already accepted would strand the client with none at all. The refusal names the level, what was preserved, and both ways out - upgrade the Codex CLI, or point CODEX_CLI_PATH at a build that supports it and run `ocx sync`. It never suggests editing the hub catalog and it does not touch running Codex processes, both of which the issue rules out. An unobservable local ladder is reported as unverified rather than incompatible, and does not block. A client machine may legitimately have no Codex CLI to observe, and refusing on absent evidence would break a working configuration; the issue asks to preserve the prior catalog when compatibility cannot be established, which is the incompatible case, not the inconclusive one. That call is mine and is recorded here. Closes #4207 --- scripts/test-layout/layout.json | 1 + src/client/catalog-compatibility.ts | 107 ++++++++++++++ src/client/connect.ts | 12 ++ src/codex/catalog/effort.ts | 41 ++++++ .../client-catalog-compatibility.test.ts | 139 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 1 + 6 files changed, 301 insertions(+) create mode 100644 src/client/catalog-compatibility.ts create mode 100644 tests/clients/client-catalog-compatibility.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 135aeaba5c..83057fdd24 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -360,6 +360,7 @@ "cli-transport-honesty.test.ts": "cli", "cli-usage-report.test.ts": "cli", "cli-version-skew.test.ts": "cli", + "client-catalog-compatibility.test.ts": "clients", "client-config-export-new-clients.test.ts": "config", "client-config-export.test.ts": "config", "client-config-new-clients.test.ts": "config", diff --git a/src/client/catalog-compatibility.ts b/src/client/catalog-compatibility.ts new file mode 100644 index 0000000000..8b4b9011cf --- /dev/null +++ b/src/client/catalog-compatibility.ts @@ -0,0 +1,107 @@ +/** + * #4207: a connected client reported `connected` with a present, freshly synced catalog while + * its installed Codex CLI exited before making a request, because the hub's catalog contained a + * reasoning level that CLI does not know: + * + * failed to parse model_catalog_json ... unknown variant `max`, + * expected one of `none`, `minimal`, `low`, `medium`, `high`, `xhigh` + * + * The connection state answered a different question from the one the operator was asking. It + * proved the hub was reachable and the credential worked; it never proved the selected local + * runtime could consume what was downloaded. This module supplies the missing half, and the + * connect path fails closed on it: an incompatible catalog is refused before it is written, so + * the previous known-good file survives and no success is reported. + * + * What it deliberately does not do: rewrite the hub's catalog into a locally compatible + * projection (the client would then silently disagree with hub truth) and terminate running + * Codex processes. Both are ruled out by the issue. + */ +import { catalogEffortCompatibility, codexSupportedReasoningEfforts } from "../codex/catalog/effort"; +import type { RawEntry } from "../codex/catalog/parsing"; + +export type ClientCatalogCompatibility = + | { kind: "compatible" } + /** The runtime ladder could not be observed, so incompatibility cannot be established. */ + | { kind: "unverified"; reason: string } + | { + kind: "incompatible"; + unsupportedEfforts: readonly string[]; + affectedModels: readonly string[]; + }; + +export interface CatalogCompatibilityDeps { + /** Injected in tests; defaults to observing the selected local Codex runtime. */ + supportedEfforts?: () => ReadonlySet | null; +} + +function parseModels(body: string): RawEntry[] | null { + try { + const parsed = JSON.parse(body) as { models?: unknown }; + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null; + return Array.isArray(parsed.models) ? parsed.models as RawEntry[] : []; + } catch { + return null; + } +} + +/** + * Assess a downloaded catalog against the reasoning efforts the selected local Codex runtime + * accepts. Unreadable bytes are reported as unverified rather than incompatible: the hub + * client already rejects a malformed body, and inventing a second cause for it here would + * repeat the mistake #4169 was filed for. + */ +export function assessClientCatalogCompatibility( + body: string, + deps: CatalogCompatibilityDeps = {}, +): ClientCatalogCompatibility { + const models = parseModels(body); + if (!models) return { kind: "unverified", reason: "the downloaded catalog could not be read" }; + const supported = (deps.supportedEfforts ?? (() => codexSupportedReasoningEfforts()))(); + if (!supported) { + return { + kind: "unverified", + reason: "the selected local Codex runtime did not report the reasoning levels it supports", + }; + } + const result = catalogEffortCompatibility(models, supported); + if (result.compatible) return { kind: "compatible" }; + return { + kind: "incompatible", + unsupportedEfforts: result.unsupportedEfforts, + affectedModels: result.affectedModels, + }; +} + +/** + * Raised instead of writing an incompatible catalog. It names both remedies the issue asks + * for, because the operator cannot act on "incompatible" alone, and it never suggests editing + * the hub. + */ +export class ClientCatalogIncompatibleError extends Error { + readonly unsupportedEfforts: readonly string[]; + readonly affectedModels: readonly string[]; + + constructor(unsupportedEfforts: readonly string[], affectedModels: readonly string[]) { + const efforts = unsupportedEfforts.join(", "); + const models = affectedModels.length > 3 + ? `${affectedModels.slice(0, 3).join(", ")} and ${affectedModels.length - 3} more` + : affectedModels.join(", "); + super( + `catalog_incompatible: the hub catalog uses reasoning ${unsupportedEfforts.length === 1 ? "level" : "levels"} ` + + `${efforts}, which the selected local Codex CLI rejects${models ? ` (${models})` : ""}. ` + + "The previous catalog was kept and nothing was changed. Upgrade the Codex CLI to a " + + "version that supports those levels, or point CODEX_CLI_PATH at one that does and run " + + "`ocx sync`, then retry. `ocx doctor` reports which runtime is selected.", + ); + this.name = "ClientCatalogIncompatibleError"; + this.unsupportedEfforts = unsupportedEfforts; + this.affectedModels = affectedModels; + } +} + +/** Fail closed: refuse an incompatible catalog before anything is written. */ +export function assertClientCatalogCompatible(body: string, deps: CatalogCompatibilityDeps = {}): void { + const assessment = assessClientCatalogCompatibility(body, deps); + if (assessment.kind !== "incompatible") return; + throw new ClientCatalogIncompatibleError(assessment.unsupportedEfforts, assessment.affectedModels); +} diff --git a/src/client/connect.ts b/src/client/connect.ts index 2b0e8efe08..3569f83c83 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -67,6 +67,7 @@ import { readClientConnectionState, assertNoClientDisconnectPending, assertClientConnectionUnchanged, sameClientConnectionOwner, } from "./state"; +import { assertClientCatalogCompatible, type CatalogCompatibilityDeps } from "./catalog-compatibility"; class RotationRecoveryRequiredError extends Error { constructor(message: string, options?: ErrorOptions) { @@ -89,6 +90,7 @@ export interface ClientConnectDeps { fetchImpl?: typeof fetch; now?: () => Date; lifecycleLockDeps?: ClientLifecycleLockDeps; + catalogCompatibility?: CatalogCompatibilityDeps; } export interface RotateClientOptions { @@ -543,6 +545,12 @@ export async function connectClient( fetchImpl: deps.fetchImpl, timeoutMs: options.catalogTimeoutMs, }); + // Fail closed BEFORE the write (#4207). The hub being reachable and the credential working + // does not mean the selected local Codex runtime can consume what arrived: an older CLI + // exits on an unknown reasoning level before making a single request, while connect + // reports success. Refusing here leaves the previous catalog in place untouched, rather + // than writing one and restoring it afterwards. + assertClientCatalogCompatible(catalog.body, deps.catalogCompatibility); writtenCatalogFingerprint = withClientLifecycleSync(() => withConfigMutationLockSync(() => { assertConnectingState(persisted.fingerprint); atomicWriteFile(DEFAULT_CATALOG_PATH, catalog.body); @@ -659,6 +667,10 @@ export async function syncConnectedClient( if (!transient) throw error; stale = true; } + // Same gate as connect (#4207): a sync must never replace a catalog the local CLI can parse + // with one it cannot. Refusing leaves the connection and the existing catalog exactly as + // they were, which is the known-good state. + if (downloaded) assertClientCatalogCompatible(downloaded.body, deps.catalogCompatibility); const next = withClientLifecycleSync(() => withConfigMutationLockSync(() => { assertClientConnectionUnchanged(initial.connection); const token = readServiceApiTokenState(); diff --git a/src/codex/catalog/effort.ts b/src/codex/catalog/effort.ts index 491518cda6..ba324f12a3 100644 --- a/src/codex/catalog/effort.ts +++ b/src/codex/catalog/effort.ts @@ -389,6 +389,47 @@ export interface ObservedCatalogEffortClamp { readonly affectedModels: readonly string[]; } +export interface CatalogEffortCompatibility { + readonly compatible: boolean; + readonly unsupportedEfforts: readonly string[]; + readonly affectedModels: readonly string[]; +} + +/** + * Report which reasoning efforts in a catalog the local Codex runtime would reject, without + * changing anything. + * + * The clamp above is mutate-and-continue, which is right when this process owns the file it + * is about to write. It is wrong for a catalog downloaded from a hub: rewriting it locally + * would make the client disagree with hub truth, and #4207 asks for the opposite — establish + * compatibility first, and refuse rather than materialise a catalog the local CLI cannot + * parse. `supported` of null means the runtime ladder could not be observed, which is not + * evidence of incompatibility, so nothing is reported. + */ +export function catalogEffortCompatibility( + models: readonly RawEntry[], + supported: ReadonlySet | null, +): CatalogEffortCompatibility { + if (!supported) return { compatible: true, unsupportedEfforts: [], affectedModels: [] }; + const unsupported = new Set(); + const affected: string[] = []; + for (const entry of models) { + const rejected = catalogEntryEfforts(entry).filter(effort => !supported.has(effort)); + const fallback = typeof entry.default_reasoning_level === "string" + && !supported.has(entry.default_reasoning_level) + ? [entry.default_reasoning_level] + : []; + if (rejected.length === 0 && fallback.length === 0) continue; + for (const effort of [...rejected, ...fallback]) unsupported.add(effort); + if (typeof entry.slug === "string") affected.push(entry.slug); + } + return { + compatible: unsupported.size === 0, + unsupportedEfforts: [...unsupported].sort(), + affectedModels: affected, + }; +} + /** Apply an already-observed runtime ladder without probing, logging, or writing diagnostics. */ export function clampCatalogModelsToObservedCodexSupport( models: RawEntry[], diff --git a/tests/clients/client-catalog-compatibility.test.ts b/tests/clients/client-catalog-compatibility.test.ts new file mode 100644 index 0000000000..f01f43d071 --- /dev/null +++ b/tests/clients/client-catalog-compatibility.test.ts @@ -0,0 +1,139 @@ +/** + * #4207: `ocx connect status` reported connected, catalog present and freshly synced, while + * the installed Codex CLI exited before its first request because the downloaded catalog used + * a reasoning level it does not know. Connection state proved the hub and the credential; it + * never proved the selected local runtime could consume what was written. + * + * The gate fails closed: an incompatible catalog is refused before the write, so the previous + * known-good file survives. It does not rewrite the hub's catalog into a local projection and + * it does not touch running Codex processes. + */ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { catalogEffortCompatibility } from "../../src/codex/catalog/effort"; +import { + assertClientCatalogCompatible, + assessClientCatalogCompatibility, + ClientCatalogIncompatibleError, +} from "../../src/client/catalog-compatibility"; +import { repoPath } from "../helpers/repo-root"; + +/** The shape the hub publishes: a model row with a reasoning ladder. */ +function catalogBody(levels: string[], slug = "gpt-5.6-sol", defaultLevel?: string): string { + return JSON.stringify({ + models: [{ + slug, + supported_reasoning_levels: levels.map(effort => ({ effort })), + ...(defaultLevel ? { default_reasoning_level: defaultLevel } : {}), + }], + }); +} + +/** Codex CLI 0.135.0's ladder, verbatim from the parse error in the issue. */ +const OLD_CLI = new Set(["none", "minimal", "low", "medium", "high", "xhigh"]); +const NEW_CLI = new Set([...OLD_CLI, "max", "ultra"]); + +describe("#4207 catalog effort compatibility", () => { + test("reports the levels an older runtime would reject, without changing the catalog", () => { + const models = JSON.parse(catalogBody(["low", "high", "max"])).models; + const before = JSON.stringify(models); + + const result = catalogEffortCompatibility(models, OLD_CLI); + + expect(result.compatible).toBe(false); + expect(result.unsupportedEfforts).toEqual(["max"]); + expect(result.affectedModels).toEqual(["gpt-5.6-sol"]); + // The clamp beside it mutates; this one must not, or the client would silently disagree + // with hub truth. + expect(JSON.stringify(models)).toBe(before); + }); + + test("a default level the runtime does not know is an incompatibility too", () => { + // The CLI parses default_reasoning_level with the same enum, so a ladder that survives + // the filter can still fail on the default alone. + const models = JSON.parse(catalogBody(["low", "high"], "gpt-5.6-sol", "ultra")).models; + const result = catalogEffortCompatibility(models, OLD_CLI); + expect(result.compatible).toBe(false); + expect(result.unsupportedEfforts).toEqual(["ultra"]); + }); + + test("a catalog the runtime fully supports is compatible", () => { + const models = JSON.parse(catalogBody(["low", "high", "max"])).models; + expect(catalogEffortCompatibility(models, NEW_CLI)).toEqual({ + compatible: true, + unsupportedEfforts: [], + affectedModels: [], + }); + }); + + test("an unobservable runtime ladder is not evidence of incompatibility", () => { + const models = JSON.parse(catalogBody(["low", "max"])).models; + expect(catalogEffortCompatibility(models, null).compatible).toBe(true); + }); +}); + +describe("#4207 client catalog gate", () => { + test("the exact ladder from the report is refused", () => { + const body = catalogBody(["low", "medium", "high", "xhigh", "max"]); + + expect(() => assertClientCatalogCompatible(body, { supportedEfforts: () => OLD_CLI })) + .toThrow(ClientCatalogIncompatibleError); + }); + + test("the refusal names the level, both remedies, and what was preserved", () => { + let thrown: unknown; + try { + assertClientCatalogCompatible(catalogBody(["low", "max"]), { supportedEfforts: () => OLD_CLI }); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(ClientCatalogIncompatibleError); + const message = (thrown as Error).message; + expect(message).toContain("max"); + // "Incompatible" alone is not actionable: the operator has to know which way out exists. + expect(message).toContain("Upgrade the Codex CLI"); + expect(message).toContain("CODEX_CLI_PATH"); + expect(message).toContain("The previous catalog was kept"); + expect((thrown as ClientCatalogIncompatibleError).unsupportedEfforts).toEqual(["max"]); + }); + + test("a compatible catalog passes the gate", () => { + expect(() => assertClientCatalogCompatible( + catalogBody(["low", "high", "max"]), + { supportedEfforts: () => NEW_CLI }, + )).not.toThrow(); + }); + + test("an unverifiable runtime does not block the connection", () => { + // A client machine may legitimately have no Codex CLI to observe. Refusing then would + // block a working configuration on absent evidence rather than on an incompatibility. + const assessment = assessClientCatalogCompatibility(catalogBody(["max"]), { supportedEfforts: () => null }); + expect(assessment.kind).toBe("unverified"); + expect(() => assertClientCatalogCompatible(catalogBody(["max"]), { supportedEfforts: () => null })) + .not.toThrow(); + }); + + test("an unreadable body is unverified, not blamed on the runtime", () => { + // The hub client already rejects a malformed body with its own cause. Inventing a second + // one here would repeat #4169, where a refusal named a cause the server never reported. + const assessment = assessClientCatalogCompatibility("not json", { supportedEfforts: () => OLD_CLI }); + expect(assessment.kind).toBe("unverified"); + }); + + test("both catalog downloads are gated, and the restore paths are not", () => { + // connect and sync each write the hub's bytes to the same path; a gate on only one of them + // still lets a sync replace a parseable catalog with an unparseable one. + const source = readFileSync(repoPath("src", "client", "connect.ts"), "utf8"); + for (const written of ["catalog.body", "downloaded.body"]) { + const write = source.indexOf(`atomicWriteFile(DEFAULT_CATALOG_PATH, ${written})`); + expect(write).toBeGreaterThan(0); + const gate = source.indexOf(`assertClientCatalogCompatible(${written}`); + expect(gate).toBeGreaterThan(0); + expect(gate).toBeLessThan(write); + } + // Restoring a catalog this machine previously accepted must not be gated on a runtime that + // may since have changed — that would strand the client with no catalog at all. + expect(source).toContain("atomicWriteFile(DEFAULT_CATALOG_PATH, snapshot.body)"); + expect(source.match(/assertClientCatalogCompatible\(/g)).toHaveLength(2); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index a9784f67e1..5ecabf4665 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -195,6 +195,7 @@ "cli-transport-honesty.test.ts": "cli", "cli-usage-report.test.ts": "cli", "cli-version-skew.test.ts": "cli", + "client-catalog-compatibility.test.ts": "clients", "client-config-export-new-clients.test.ts": "config", "client-config-export.test.ts": "config", "client-config-new-clients.test.ts": "config", From 8f3e0b890d494c4b11ba7fc9f6637ced04695c4e Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 11 Sep 2026 08:42:41 +0900 Subject: [PATCH 2/2] docs(devlog): record the wp3 client-catalog work-phase --- .../030_wp3_client_catalog.md | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 devlog/_plan/260911_l4_service_cli/030_wp3_client_catalog.md diff --git a/devlog/_plan/260911_l4_service_cli/030_wp3_client_catalog.md b/devlog/_plan/260911_l4_service_cli/030_wp3_client_catalog.md new file mode 100644 index 0000000000..be2d964772 --- /dev/null +++ b/devlog/_plan/260911_l4_service_cli/030_wp3_client_catalog.md @@ -0,0 +1,66 @@ +# wp3 — #4207: connected catalog reports success while the local Codex CLI rejects it + +Work-phase 3 of the L4 lane, stacked on wp2. No carried PR: this issue had none. + +## The gap + +Subagent Bernoulli mapped the client path. `connectClient` downloads at `connect.ts:542` and +writes the hub's bytes verbatim at `:545-549`; `syncConnectedClient` writes the same way at +`:667`. The only validation in between is `validateRemoteCatalog` +(`hub-client.ts:145`), which checks JSON shape — object, `models` array, unique non-empty +slugs — and nothing about reasoning levels. `src/client` never imports the effort clamp. + +So the connection state proves the hub is reachable and the credential works, and is then +reported as readiness. The reporter's Codex CLI 0.135.0 exited before its first request on +`unknown variant \`max\``, while `ocx connect status --json` said `connected` with the catalog +present. `ocx status` even reported an active effort clamp for that same older runtime: the +local machinery already knew the ladder, and the client path simply never consulted it. + +## Decision + +The packet records it: **fail closed — block local readiness rather than reporting success.** +Not a locally clamped projection, which would make the client silently disagree with hub truth. + +## Shape + +- `catalogEffortCompatibility(models, supported)` in `src/codex/catalog/effort.ts` — pure, no + mutation, reports the rejected efforts and the models carrying them. It sits beside + `clampCatalogModelsToObservedCodexSupport`, which mutates; that is correct for a file this + process owns and wrong for one that must keep matching the hub. +- `src/client/catalog-compatibility.ts` — assesses a downloaded body against the observed local + ladder and throws `ClientCatalogIncompatibleError` when it cannot be consumed. +- Both hub-download writes are gated **before** the write. Refusing before the write is stronger + than writing and restoring: there is no window in which an unparseable catalog exists on disk, + and `writtenCatalogFingerprint` stays null so the existing rollback correctly does nothing. +- The two restore paths (`connect.ts:126`, `:729`) are deliberately **not** gated. Refusing to + restore a catalog this machine already accepted would strand the client with none at all. + +## Decisions I had to make + +**An unobservable ladder does not block.** `codexSupportedReasoningEfforts` returns null when +`codex debug models --bundled` cannot be observed. That is not evidence of incompatibility, and a +client machine may legitimately have no Codex CLI. I read the issue's *"preserve the prior +known-good catalog if compatibility cannot be established"* as the incompatible branch — the +alternative to the compatible-projection branch offered in the same sentence — not as the +inconclusive one. Recorded in the PR body too, because the other reading is defensible. + +**An invented command was caught before it shipped.** The first draft of the refusal recommended +`ocx codex-runtime`, which does not exist. `AGENTS.md` records this exact failure mode — a +documented `ocx request-history` that never existed — so every command in the message was +checked against the CLI registry. It now names `CODEX_CLI_PATH` and `ocx sync`, with `ocx doctor` +for diagnosis, matching `doctor.ts`'s existing advice. + +## Audit + +Bohr reviewed the diff adversarially and returned `SAFE_TO_PUSH`: no strict-tsc failure on the +new `src/` lines (checked by hand, since typecheck was NOT RUN), no import cycle into +`src/client` and no module-load side effect, Lab boundary untouched, all four +`atomicWriteFile(DEFAULT_CATALOG_PATH, …)` sites classified, and the gate proven to precede +`commitClientConnection`. One nit folded: a test title claimed write ordering that only the +source-scan test actually asserts, and was renamed. + +## Not run + +`bun test`, `bun run test`, `bun run test:changed`, `bun run typecheck`, `bun run build:gui` +and `bun install` are NOT RUN by operator instruction. Hosted CI on the exact pushed head is +the only product evidence this round accepts.