From a71a0ca6abd5b9801f79089eb9c62d5cff75ff8e Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 8 Oct 2026 09:59:24 +0900 Subject: [PATCH 1/4] fix(catalog): keep Codex's auto-review model so reviews stop falling back to the task model The canonical native catalog dropped codex-auto-review as an unsupported bare Codex slug. Because model_catalog_json replaces Codex's entire catalog, the approval reviewer could no longer find its preferred model and fell back to the parent task model at Low effort. Share the exact control-plane id set with routing and preserve or separately backfill one hidden reviewer row whenever native OpenAI rows are included. Keep its upstream reasoning metadata and exclude it from native synthesis, account clones, picker ordering, public lists and override stamping. Preserve provider/per-model/root reviewer precedence and non-OpenAI fallback behavior. Cover all catalog writers, persisted duplicates, visibility, Low effort, HTTP model lists, shared routing and override selection. Register the new regression file in both layout inventories and document the contract. --- .../docs/reference/configuration/providers.md | 8 +- scripts/test-layout/layout.json | 2 +- src/codex/catalog/auto-review.ts | 19 +- src/codex/catalog/build-entries.ts | 21 +- src/codex/catalog/control-plane.ts | 28 ++ src/codex/catalog/effort.ts | 3 +- src/codex/control-plane-models.ts | 6 + src/router.ts | 4 +- structure/catalog.md | 2 +- structure/runtime.md | 2 +- .../catalog-auto-review-row.test.ts | 241 ++++++++++++++++++ .../catalog-full-picker-order.test.ts | 4 +- .../codex-catalog-golden.test.ts | 1 + .../codex-catalog-sync-hardening.test.ts | 5 +- tests/fixtures/test-layout-expected.json | 2 +- 15 files changed, 321 insertions(+), 27 deletions(-) create mode 100644 src/codex/catalog/control-plane.ts create mode 100644 src/codex/control-plane-models.ts create mode 100644 tests/codex-integration/catalog-auto-review-row.test.ts diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index bfc084cf0ae..42013f164d5 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -478,10 +478,16 @@ optional pin in a hand-edited file is ignored on load without discarding the res Codex reads `auto_review_model_override` from the catalog row of the current turn's model to choose the model that reviews approval requests. The root `auto_review_model` setting in -`$CODEX_HOME/config.toml` applies one reviewer to every catalog row; the provider-scoped fields +`$CODEX_HOME/config.toml` applies one reviewer to task catalog rows; the provider-scoped fields below override it per provider. The [provider guide](/guides/providers/#approval-reviewer-per-provider) has the operator workflow and a worked example. +When native OpenAI rows are included, OpenCodex keeps the hidden `codex-auto-review` row so +Codex can select its preferred approval reviewer when no override is configured. It stays out +of model pickers, subagent choices, Desktop lists and public `/v1/models` lists. Provider and +root reviewer overrides retain their precedence and do not stamp this internal row. Catalogs +without a native OpenAI path omit it, preserving Codex's fallback to the task model. + `autoReviewModel` is the provider-wide reviewer target. A value can be a bare model id of that same provider (the catalog row is normalized to the `provider/model` slug) or a full public catalog slug such as `opencode-go/deepseek-v4-flash`. A bare value resolves against that provider's rows diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 05cae70aa51..d01131fec9a 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,7 +2,7 @@ "version": 1, "root": "tests", "explicit": { - "messages-native-scope.test.ts": "claude-integration", + "messages-native-scope.test.ts": "claude-integration", "catalog-auto-review-row.test.ts": "codex-integration", "cli-log-view-filter.test.ts": "cli", "cli-usage-model-search.test.ts": "cli", "cli-companion-usage.test.ts": "cli", "cli-account-key-quota.test.ts": "cli", "cli-observe-snapshot.test.ts": "cli", "cli-access-data-plane.test.ts": "cli", "cli-access-key-guard.test.ts": "cli", "cli-access-rename.test.ts": "cli", "cli-access-audio.test.ts": "cli", "cli-access-audio-live.test.ts": "cli", "cli-log-follow.test.ts": "cli", "cli-injection-follow.test.ts": "cli", diff --git a/src/codex/catalog/auto-review.ts b/src/codex/catalog/auto-review.ts index 3487272b848..be416776b87 100644 --- a/src/codex/catalog/auto-review.ts +++ b/src/codex/catalog/auto-review.ts @@ -1,3 +1,4 @@ +import { isCodexControlPlaneModel } from "../control-plane-models"; import { redactSecretString } from "../../lib/redact"; import type { OcxConfig } from "../../types"; import { encodeRoutedModelId } from "../../providers/slug-codec"; @@ -34,6 +35,7 @@ export type AutoReviewModelOverrideResult = "absent" | "applied" | "invalid" | " /** True when a catalog row was synthesized by opencodex instead of coming from upstream. */ function isRoutedCatalogEntry(entry: RawEntry): boolean { + if (isCodexControlPlaneModel(entry.slug)) return false; const slug = typeof entry.slug === "string" ? entry.slug : ""; return slug.includes("/") || (typeof entry.description === "string" && entry.description.startsWith("Routed via opencodex → ")); @@ -57,6 +59,7 @@ function clearAutoReviewOverrideValue(entry: RawEntry): void { * Returns the stamped values when the observed rows match that shape. */ function legacyRootStampValues(observedModels: readonly RawEntry[]): ReadonlySet | undefined { + observedModels = observedModels.filter(entry => !isCodexControlPlaneModel(entry?.slug)); if (observedModels.some(entry => entry?.[AUTO_REVIEW_ROOT_MARKER] !== undefined)) return undefined; const configuredValues = new Set(observedModels.flatMap(entry => { const value = entry?.auto_review_model_override; @@ -90,7 +93,7 @@ function clearLegacyRootStamps(models: readonly RawEntry[], sourceModels: readon const legacyStamp = legacyRootStampValues([...models, ...sourceModels]); if (legacyStamp === undefined) return; for (const entry of models) { - if (!entry || typeof entry !== "object") continue; + if (!entry || typeof entry !== "object" || isCodexControlPlaneModel(entry.slug)) continue; const current = entry.auto_review_model_override; if (entry[AUTO_REVIEW_ROOT_MARKER] === undefined && typeof current === "string" && legacyStamp.has(current)) clearAutoReviewOverrideValue(entry); @@ -107,7 +110,7 @@ function clearAutoReviewModelOverride( ): void { const legacyStamp = legacyRootStampValues([...models, ...sourceModels]); for (const entry of models) { - if (!entry || typeof entry !== "object") continue; + if (!entry || typeof entry !== "object" || isCodexControlPlaneModel(entry.slug)) continue; const current = entry.auto_review_model_override; if (isRoutedCatalogEntry(entry) || (entry[AUTO_REVIEW_ROOT_MARKER] === true || rootAutoReviewStamp(entry) !== undefined) @@ -188,6 +191,7 @@ function preserveNativeAutoReviewModelOverrides( /** Stamp a root-derived override and mark native rows so later root removal is durable. */ function stampRootAutoReviewOverride(entry: RawEntry, target: string): void { + if (isCodexControlPlaneModel(entry.slug)) return; if (!isRoutedCatalogEntry(entry)) { const previous = rootAutoReviewStamp(entry); const current = entry.auto_review_model_override; @@ -399,7 +403,7 @@ function applyRootSelectorToRemaining( ): AutoReviewModelOverrideResult { const clearRemaining = (): void => { for (const entry of models) { - if (!entry || providerStamped.has(entry)) continue; + if (!entry || providerStamped.has(entry) || isCodexControlPlaneModel(entry.slug)) continue; // Native rows written by releases before the root marker cannot be told apart from upstream // values once provider stamps diverge. clearLegacyRootStamps sweeps the ones the legacy // uniform signature still recognizes before provider plans land, because provider stamping @@ -427,7 +431,7 @@ function applyRootSelectorToRemaining( return "unresolved"; } for (const entry of models) { - if (!entry || providerStamped.has(entry)) continue; + if (!entry || providerStamped.has(entry) || isCodexControlPlaneModel(entry.slug)) continue; stampRootAutoReviewOverride(entry, trimmed); } return "applied"; @@ -449,7 +453,7 @@ export function applyConfiguredAutoReviewModelOverride( const { plans, failure } = buildProviderReviewPlans(models, config); const providerStamped = new Set(); for (const entry of models) { - if (!entry || typeof entry !== "object") continue; + if (!entry || typeof entry !== "object" || isCodexControlPlaneModel(entry.slug)) continue; const provider = catalogEntryProviderName(entry); if (!provider) continue; const plan = plans.get(provider); @@ -482,7 +486,10 @@ export function finalizeAutoReviewModelOverride( sourceModels: readonly RawEntry[] = [], config?: Pick, ): AutoReviewModelOverrideResult { - if (models && sourceModels.length > 0) preserveNativeAutoReviewModelOverrides(models, sourceModels); + if (models && sourceModels.length > 0) preserveNativeAutoReviewModelOverrides( + models.filter(entry => !isCodexControlPlaneModel(entry.slug)), + sourceModels.filter(entry => !isCodexControlPlaneModel(entry.slug)), + ); if (config && configHasProviderAutoReview(config)) { return applyConfiguredAutoReviewModelOverride(models, readConfiguredAutoReviewModel(), config, sourceModels); } diff --git a/src/codex/catalog/build-entries.ts b/src/codex/catalog/build-entries.ts index 40c5807d382..26ef3931eb8 100644 --- a/src/codex/catalog/build-entries.ts +++ b/src/codex/catalog/build-entries.ts @@ -1,3 +1,5 @@ +import { isCodexControlPlaneModel } from "../control-plane-models"; +import { withCodexControlPlaneRows } from "./control-plane"; import { CODEX_REASONING_LEVELS, type CodexReasoningLevel } from "../../reasoning-effort"; import { clearModelCache } from "../model-cache"; import { routedSlug, slugEquivalenceKey } from "../../providers/slug-codec"; @@ -194,7 +196,7 @@ export function buildCatalogEntriesFromObservedState({ const comboPublicSlugs = new Set(goModels .filter(model => model.provider === COMBO_NAMESPACE) .map(catalogModelSlug)); - for (const slug of gptSlugs) { + for (const slug of gptSlugs.filter(slug => !isCodexControlPlaneModel(slug))) { const native = deriveEntry(template, slug, "OpenAI native model (Codex OAuth passthrough).", 9, undefined, new Set(), openaiContextCap); if (rank.has(slug)) native.priority = rank.get(slug)!; nativeEntries.push(native); @@ -224,7 +226,7 @@ export function buildCatalogEntriesFromObservedState({ const selectorNativeSlugs = accountNativeSlugsBySelector?.get(selector) ?? accountNativeSlugs ?? gptSlugs; - const accountNativeEntries = selectorNativeSlugs.filter(slug => slug !== NATIVE_RESERVE_MODEL).map(slug => ( + const accountNativeEntries = selectorNativeSlugs.filter(slug => slug !== NATIVE_RESERVE_MODEL && !isCodexControlPlaneModel(slug)).map(slug => ( nativeEntriesBySlug.get(slug) ?? deriveEntry(template, slug, "OpenAI native model (Codex OAuth passthrough).", 9, undefined, new Set(), openaiContextCap) )); @@ -306,10 +308,10 @@ export function buildCatalogEntriesFromObservedState({ delete entry.prefer_websockets; } } - return applyMultiAgentMode(out, multiAgentMode, multiAgentV2Enabled, { + return withCodexControlPlaneRows(applyMultiAgentMode(out, multiAgentMode, multiAgentV2Enabled, { keepNativeChatGptOnV1, preserveDefaultMultiAgentVersion: isReserveCatalogProjection, - }); + }), template ? [template] : [], gptSlugs.length > 0, wsEnabled); } export function resetCatalogRuntimeStateForTests(): void { @@ -409,12 +411,12 @@ export function mergeCatalogModelsWithNativeRecovery( ): RawEntry[] { const merged = [...primaryCatalogModels]; const recoveredNativeSlugs = new Set(primaryCatalogModels.flatMap(entry => { - const slug = recoverableNativeSlug(entry); + const slug = isCodexControlPlaneModel(entry.slug) ? String(entry.slug) : recoverableNativeSlug(entry); return slug === null ? [] : [slug]; })); for (const source of nativeRecoverySources) { for (const entry of source) { - const slug = recoverableNativeSlug(entry); + const slug = isCodexControlPlaneModel(entry.slug) ? String(entry.slug) : recoverableNativeSlug(entry); if (slug === null || recoveredNativeSlugs.has(slug)) continue; merged.push(structuredClone(entry) as RawEntry); recoveredNativeSlugs.add(slug); @@ -460,6 +462,7 @@ export function applyFullModelPickerOrder(entries: RawEntry[], order: readonly s if (!pickerOrder.some(slug => !slug.includes("/"))) return; const rankOf = modelPickerRank(pickerOrder); for (const entry of entries) { + if (isCodexControlPlaneModel(entry.slug)) continue; const natural = entry[SPAWN_PRIORITY_FIELD] ?? entry.priority ?? 9; entry[SPAWN_PRIORITY_FIELD] = natural; entry.priority = rankOf(String(entry.slug)) ?? pickerOrder.length + Number(natural); @@ -539,10 +542,12 @@ export function mergeCatalogEntriesFromObservedState({ // Raw catalog rows contain nested arrays/objects that normalization mutates. Detach every row at // the observed-core boundary so callers can safely retain evidence objects or repeat the merge. const detachedCatalogModels = catalogModels + .filter(entry => !isCodexControlPlaneModel(entry.slug)) .map(entry => restoreNativeDisplayName(structuredClone(entry) as RawEntry)); const detachedBaselineCatalogModels = baselineCatalogModels + .filter(entry => !isCodexControlPlaneModel(entry.slug)) .map(entry => restoreNativeDisplayName(structuredClone(entry) as RawEntry)); - const detachedRoutedEntries = routedEntries.map(entry => structuredClone(entry) as RawEntry); + const detachedRoutedEntries = routedEntries.filter(entry => !isCodexControlPlaneModel(entry.slug)).map(entry => structuredClone(entry) as RawEntry); // Track this invocation's generated custom rows, not ownership markers read from disk. // Their builder already finalized exact native ladders and ordinary routed mock tiers. const freshCustomEntries = new Set(detachedRoutedEntries.filter(entry => @@ -948,7 +953,7 @@ export function mergeCatalogEntriesFromObservedState({ delete entry.opencodex_catalog_kind; entry.opencodex_catalog_kind = kind; } - return versionedEntries; + return withCodexControlPlaneRows(versionedEntries, [...catalogModels, ...baselineCatalogModels, ...routedEntries], includeNativeOpenAi, wsEnabled); } /** Merge retained-sync rows using the process-observed Codex feature state. */ diff --git a/src/codex/catalog/control-plane.ts b/src/codex/catalog/control-plane.ts new file mode 100644 index 00000000000..90dde448c47 --- /dev/null +++ b/src/codex/catalog/control-plane.ts @@ -0,0 +1,28 @@ +import { CODEX_INTERNAL_OPENAI_MODELS, isCodexControlPlaneModel } from "../control-plane-models"; +import { pinnedNativeModelRows } from "./pinned-models"; +import type { RawEntry } from "./parsing"; + +/** Keep internal reviewer metadata separate from native synthesis, picker ordering and account clones. */ +export function withCodexControlPlaneRows( + entries: RawEntry[], + sourceRows: readonly RawEntry[], + includeNativeOpenAi: boolean, + wsEnabled: boolean, +): RawEntry[] { + const models = entries.filter(entry => !isCodexControlPlaneModel(entry.slug)); + if (!includeNativeOpenAi) return models; + for (const slug of CODEX_INTERNAL_OPENAI_MODELS) { + const source = sourceRows.find(entry => entry.slug === slug) + ?? pinnedNativeModelRows().find(entry => entry.slug === slug); + if (!source) throw new Error(`Pinned Codex control-plane model is missing: ${slug}`); + const row: RawEntry = structuredClone(source); + row.visibility = "hide"; + if (wsEnabled) row.supports_websockets = true; + else { + delete row.supports_websockets; + delete row.prefer_websockets; + } + models.push(row); + } + return models; +} diff --git a/src/codex/catalog/effort.ts b/src/codex/catalog/effort.ts index 1ccf39d2b7e..b983968a8c4 100644 --- a/src/codex/catalog/effort.ts +++ b/src/codex/catalog/effort.ts @@ -1,3 +1,4 @@ +import { isCodexControlPlaneModel } from "../control-plane-models"; import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { copyFileSync, existsSync, mkdirSync, readFileSync, realpathSync } from "node:fs"; @@ -385,7 +386,7 @@ export function clampEntryToCodexSupportedEfforts( entry: RawEntry, supported: ReadonlySet | null, ): void { - if (!supported) return; + if (!supported || isCodexControlPlaneModel(entry.slug)) return; const levels = Array.isArray(entry.supported_reasoning_levels) ? entry.supported_reasoning_levels as Array<{ effort?: string }> : null; diff --git a/src/codex/control-plane-models.ts b/src/codex/control-plane-models.ts new file mode 100644 index 00000000000..e08e53e0d58 --- /dev/null +++ b/src/codex/control-plane-models.ts @@ -0,0 +1,6 @@ +/** Exact Codex control-plane ids routed to canonical OpenAI, outside the public native roster. */ +export const CODEX_INTERNAL_OPENAI_MODELS: ReadonlySet = new Set(["codex-auto-review"]); + +export function isCodexControlPlaneModel(slug: unknown): boolean { + return typeof slug === "string" && CODEX_INTERNAL_OPENAI_MODELS.has(slug); +} diff --git a/src/router.ts b/src/router.ts index a38eff3888e..0188dc4f5e9 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,3 +1,4 @@ +import { CODEX_INTERNAL_OPENAI_MODELS } from "./codex/control-plane-models"; import type { CodexAccountMode, OcxConfig, OcxProviderConfig } from "./types"; import { createHash } from "node:crypto"; import { peekAuthStore } from "./oauth/store"; @@ -564,9 +565,6 @@ export function comboRouteDecisionTrace( }); } -// Codex uses a small number of control-plane model ids that are not part of the public GPT/o -// naming families. Keep this exact: a broad `codex-*` rule could capture a third-party model. -const CODEX_INTERNAL_OPENAI_MODELS = new Set(["codex-auto-review"]); const MAX_BLOCKED_MODEL_REDIRECT_EDGES = 5; interface BlockedModelRedirectState { diff --git a/structure/catalog.md b/structure/catalog.md index b806e62cd34..7bf6c051365 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -47,7 +47,7 @@ provider-wide fallback. Exact model output limits precede the provider default o - preserves native OpenAI entries from the live catalog or static fallback, and emits gpt-5.6 natives from the pinned upstream models.json snapshot - (`src/codex/data/upstream-models.json` — exact per-slug ladders: luna has no ultra); + (`src/codex/data/upstream-models.json` — exact per-slug ladders: luna has no ultra); When native OpenAI rows are included, `src/codex/catalog/control-plane.ts` separately preserves exactly one hidden `codex-auto-review` row or backfills its pinned metadata, including Low and the exact upstream ladder. It bypasses native synthesis, account cloning, picker/featured ordering, effort clamps and reviewer override stamps; public lists and subagent rosters exclude it. The dependency-free `src/codex/control-plane-models.ts` shares the exact id set with the router without expanding visible native replacement authority; - reads pinned native rows only through `pinnedNativeModelRows()` (`src/codex/catalog/pinned-models.ts`): the codex-rs snapshot first, then rows from `src/codex/data/roster-pinned-models.json` whose slug the snapshot lacks. The roster file holds diff --git a/structure/runtime.md b/structure/runtime.md index ae428036666..c5c95224704 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -1,6 +1,6 @@ # Runtime -The minute sweep checks persisted activation deadlines locally; only missing deadlines trigger metadata discovery. See the [quota activation contract](providers/openai-tiers.md#public-provider-contract). +The minute sweep checks persisted activation deadlines locally; only missing deadlines trigger metadata discovery. See the [quota activation contract](providers/openai-tiers.md#public-provider-contract). Codex control-plane ids are defined in the dependency-free `src/codex/control-plane-models.ts`; `src/router.ts` routes these exact ids unchanged to canonical OpenAI, retaining the existing error when no OpenAI provider is enabled. They do not widen the public native roster. ## Resolved static model policy diff --git a/tests/codex-integration/catalog-auto-review-row.test.ts b/tests/codex-integration/catalog-auto-review-row.test.ts new file mode 100644 index 00000000000..7b4539ff15a --- /dev/null +++ b/tests/codex-integration/catalog-auto-review-row.test.ts @@ -0,0 +1,241 @@ +import { expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { + applyFullModelPickerOrder, buildCatalogEntriesFromObservedState, + CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, mergeCatalogEntriesFromObservedState, + mergeCatalogModelsWithNativeRecovery, type ObservedCatalogMergeInput, +} from "../../src/codex/catalog/build-entries"; +import { applyAutoReviewModelOverride, applyConfiguredAutoReviewModelOverride } from "../../src/codex/catalog/auto-review"; +import { clampCatalogModelsToObservedCodexSupport } from "../../src/codex/catalog/effort"; +import { filterSupportedNativeSlugs, type RawEntry } from "../../src/codex/catalog/parsing"; +import { desktopVisibleNativeSlugs, upstreamNativeEntry, visibleNativeSlugs } from "../../src/codex/catalog/metadata"; +import { SUPPORTED_NATIVE_OPENAI_SLUGS } from "../../src/codex/catalog/native-models"; +import { effectiveSubagentRoster, SPAWN_PRIORITY_FIELD } from "../../src/codex/catalog/subagent-roster"; +import snapshot from "../../src/codex/data/upstream-models.json"; +import { repoPath, repoRoot } from "../helpers/repo-root"; +import { CODEX_INTERNAL_OPENAI_MODELS } from "../../src/codex/control-plane-models"; +import { NoEnabledOpenAiProviderError, routeModel } from "../../src/router"; +import type { OcxConfig } from "../../src/types"; + +const reviewer = "codex-auto-review"; +const upstream = snapshot.models.find(row => row.slug === reviewer)!; +const nativeConfig: OcxConfig = { + port: 10100, defaultProvider: "openai", + providers: { openai: { adapter: "openai-responses", authMode: "forward", baseUrl: "https://chatgpt.com/backend-api/codex", liveModels: false } }, +}; +const otherConfig: OcxConfig = { + port: 10100, defaultProvider: "other", + providers: { other: { adapter: "openai-chat", baseUrl: "https://provider.invalid/v1", models: ["task"], liveModels: false } }, +}; +function merge(catalogModels: RawEntry[] = [], includeNativeOpenAi = true, extra: Partial = {}): RawEntry[] { + return mergeCatalogEntriesFromObservedState({ + catalogModels, baselineCatalogModels: [], routedEntries: [], baseline: new Map(), + featured: [reviewer], modelPickerOrder: [reviewer, "gpt-5.5"], wsEnabled: false, + template: null, disabledModels: new Set(), selectedModelsByProvider: new Map(), + gatheredProviderNames: new Set(), degradedProviderNames: new Set(), legacyCustomModelSlugs: new Set(), + multiAgentMode: "v2", multiAgentV2Enabled: true, exactComboSlugs: new Set(), + hasPhysicalComboProvider: false, includeNativeOpenAi, accountBoundEntries: [], + policy: { ...CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, warningPolicy: "suppress" }, ...extra, + }); +} +function reviewRow(rows: RawEntry[]): RawEntry { + const matches = rows.filter(row => row.slug === reviewer); + expect(matches).toHaveLength(1); + expect(matches[0]!.visibility).toBe("hide"); + return matches[0]!; +} + +test("build backfills the hidden reviewer with its exact upstream ladder and no picker/spawn rank", () => { + const rows = buildCatalogEntriesFromObservedState({ + template: null, gptSlugs: ["gpt-5.5", reviewer], goModels: [], featured: [reviewer], + wsEnabled: false, multiAgentMode: "v2", multiAgentV2Enabled: true, + exactComboSlugs: new Set(), accountSelectors: ["account"], + suppressedBareNativeSlugs: new Set(), disabledNativeAccountSlugs: new Set(), + }); + const row = reviewRow(rows); + expect(row.supported_reasoning_levels).toEqual(upstream.supported_reasoning_levels); + expect(row.supported_reasoning_levels).toEqual(expect.arrayContaining([expect.objectContaining({ effort: "low" })])); + expect(row.default_reasoning_level).toBe(upstream.default_reasoning_level); + expect(row.multi_agent_version).toBe(upstream.multi_agent_version); + expect(row.priority).toBe(upstream.priority); + expect(rows.some(entry => entry.slug === `account/${reviewer}`)).toBe(false); + applyFullModelPickerOrder(rows, [reviewer, "gpt-5.5"]); + expect(row.priority).toBe(upstream.priority); + expect(row[SPAWN_PRIORITY_FIELD]).toBeUndefined(); +}); + +test("routed-only build and merge do not add or retain the native reviewer", () => { + expect(merge([{ ...structuredClone(upstream), visibility: "list" }], false).some(row => row.slug === reviewer)).toBe(false); + const rows = buildCatalogEntriesFromObservedState({ + template: null, gptSlugs: [], goModels: [], wsEnabled: false, + multiAgentMode: "default", multiAgentV2Enabled: false, exactComboSlugs: new Set(), + accountSelectors: [], suppressedBareNativeSlugs: new Set(), disabledNativeAccountSlugs: new Set(), + }); + expect(rows.some(row => row.slug === reviewer)).toBe(false); +}); + +test("merge backfills a single hidden reviewer outside native replacement authority", () => { + const row = reviewRow(merge()); + expect(row.supported_reasoning_levels).toEqual(upstream.supported_reasoning_levels); + expect(row.supported_in_api).toBe(true); + expect(row.priority).toBe(upstream.priority); + expect(upstreamNativeEntry(reviewer)).toBeNull(); + expect(SUPPORTED_NATIVE_OPENAI_SLUGS.has(reviewer)).toBe(false); +}); + +test("merge preserves the first live row over duplicate persisted/generated rows and forces hide", () => { + const live: RawEntry = { ...structuredClone(upstream), visibility: "list", display_name: "Live reviewer", comp_hash: "live", supported_reasoning_levels: [{ effort: "low", description: "Live Low" }] }; + const before = structuredClone(live); + const rows = merge([live, { ...structuredClone(upstream), display_name: "Duplicate" }], true, { + baselineCatalogModels: [{ ...structuredClone(upstream), display_name: "Baseline" }], + routedEntries: [structuredClone(upstream)], + }); + const expected = { ...before, visibility: "hide" }; + delete expected.prefer_websockets; + expect(reviewRow(rows)).toEqual(expected); + expect(live).toEqual(before); + expect(merge(rows)).toEqual(rows); +}); + +test("native recovery preserves a persisted reviewer without admitting it to visible native authority", () => { + const persisted: RawEntry = { ...structuredClone(upstream), comp_hash: "cache-reviewer" }; + const recovered = mergeCatalogModelsWithNativeRecovery([], [[persisted, structuredClone(upstream)]]); + expect(reviewRow(merge(recovered)).comp_hash).toBe("cache-reviewer"); + expect(filterSupportedNativeSlugs(recovered)).toEqual([]); +}); + +test("reviewer stays out of native picker, Desktop, public native lists and both subagent surfaces", () => { + const rows = merge(); + const row = reviewRow(rows); + expect(filterSupportedNativeSlugs(rows)).not.toContain(reviewer); + expect(visibleNativeSlugs(nativeConfig)).not.toContain(reviewer); + expect(desktopVisibleNativeSlugs(nativeConfig)).not.toContain(reviewer); + for (const surface of ["v1", "v2"] as const) { + const roster = effectiveSubagentRoster([reviewer], surface, rows); + expect(roster.candidates.some(model => model.model === reviewer)).toBe(false); + expect(roster.advertised).toEqual([]); + } + expect(row.supported_reasoning_levels).toEqual(upstream.supported_reasoning_levels); + clampCatalogModelsToObservedCodexSupport(rows, new Set(["high"])); + expect(row.supported_reasoning_levels).toEqual(upstream.supported_reasoning_levels); +}); + +test("root/provider/per-model overrides keep precedence without stamping the reviewer", () => { + const rows = merge([], true, { routedEntries: [ + { slug: "other/task", description: "Routed via opencodex → other", visibility: "list" }, + { slug: "other/provider-review", visibility: "list" }, + { slug: "other/model-review", visibility: "list" }, + { slug: "other/root-review", visibility: "list" }, + ] }); + const row = reviewRow(rows); + const before = structuredClone(row); + expect(applyAutoReviewModelOverride(rows, "other/root-review")).toBe("applied"); + expect(row).toEqual(before); + const config: OcxConfig = structuredClone(otherConfig); + config.providers.other!.autoReviewModel = "provider-review"; + config.providers.other!.autoReviewModelOverrides = { task: "model-review" }; + expect(applyConfiguredAutoReviewModelOverride(rows, "other/root-review", config)).toBe("applied"); + expect(rows.find(entry => entry.slug === "other/task")?.auto_review_model_override).toBe("other/model-review"); + expect(rows.find(entry => entry.slug === "other/provider-review")?.auto_review_model_override).toBe("other/provider-review"); + expect(rows.find(entry => entry.slug === "gpt-5.5")?.auto_review_model_override).toBe("other/root-review"); + expect(row).toEqual(before); + applyConfiguredAutoReviewModelOverride(rows, undefined, config); + expect(rows.find(entry => entry.slug === "other/task")?.auto_review_model_override).toBe("other/model-review"); + expect(row).toEqual(before); +}); + +test("an explicit override may target the hidden reviewer without stamping that row", () => { + const rows = merge(); + const row = reviewRow(rows); + const before = structuredClone(row); + expect(applyAutoReviewModelOverride(rows, reviewer)).toBe("applied"); + expect(rows.find(entry => entry.slug === "gpt-5.5")?.auto_review_model_override).toBe(reviewer); + expect(row).toEqual(before); +}); + +test("router keeps the exact reviewer wire id and fails without canonical OpenAI", () => { + expect([...CODEX_INTERNAL_OPENAI_MODELS]).toEqual([reviewer]); + expect(readFileSync(repoPath("src/router.ts"), "utf8")).toMatch(/import\s*\{[^}]*\bCODEX_INTERNAL_OPENAI_MODELS\b[^}]*\}\s*from\s*["']\.\/codex\/control-plane-models["']/); + expect(routeModel(nativeConfig, reviewer)).toMatchObject({ providerName: "openai", modelId: reviewer }); + expect(() => routeModel(otherConfig, reviewer)).toThrow(NoEnabledOpenAiProviderError); +}); + +for (const writer of ["retained-sync", "convergence"] as const) { + for (const includeNative of [true, false]) { + test(`${writer} ${includeNative ? "keeps" : "omits"} the reviewer across two catalog/cache writes`, () => { + const base = repoPath(".tmp", "auto-review-verification"); + mkdirSync(base, { recursive: true }); + const root = mkdtempSync(join(base, "writer-")); + const codexHome = join(root, "codex"); + const ocxHome = join(root, "ocx"); + mkdirSync(codexHome); mkdirSync(ocxHome); + const config = includeNative ? nativeConfig : otherConfig; + writeFileSync(join(ocxHome, "config.json"), JSON.stringify(config)); + writeFileSync(join(codexHome, "config.toml"), 'model_catalog_json = "opencodex-catalog.json"\n'); + writeFileSync(join(codexHome, "opencodex-catalog.json"), JSON.stringify({ models: [ + { slug: "gpt-5.5", visibility: "list", display_name: "GPT-5.5", priority: 1, base_instructions: "You are Codex." }, + { ...structuredClone(upstream), visibility: "list", comp_hash: "persisted" }, + ] })); + const script = ` + const { readFileSync } = require("node:fs"); + const { join } = require("node:path"); + const config = JSON.parse(readFileSync(join(process.env.OPENCODEX_HOME, "config.json"), "utf8")); + const { syncCatalogModels } = require("./src/codex/catalog"); + require("./src/config").saveConfig(config); + const { captureCatalogAdmissionSnapshot } = require("./src/codex/catalog-admission"); + const { gatherCodexCatalogCandidate, commitCodexCatalogCandidate } = require("./src/codex/convergence"); + (async () => { + const passes = []; + for (let i = 0; i < 2; i++) { + if (${writer === "retained-sync"}) { + const synced = await syncCatalogModels(config, { allowWhenDesiredDisabled: true }); + if (synced.refreshOutcome !== "committed") throw new Error(JSON.stringify(synced)); + require("./src/codex/catalog").invalidateCodexModelsCache({ allowWhenDesiredDisabled: true }); + } + else { + const gathered = await gatherCodexCatalogCandidate(captureCatalogAdmissionSnapshot(config)); + if (gathered.kind !== "candidate") throw new Error(JSON.stringify(gathered)); + const committed = await commitCodexCatalogCandidate(gathered.candidate, 1000); + if (committed.kind !== "committed") throw new Error(JSON.stringify(committed)); + } + passes.push(["opencodex-catalog.json", "models_cache.json"].map(file => + JSON.parse(readFileSync(join(process.env.CODEX_HOME, file), "utf8")).models + .filter(row => row.slug === "codex-auto-review"))); + } + if (${writer === "convergence" && includeNative}) { + const { startServer } = require("./src/server"); + const server = startServer(0); + try { + const raw = await fetch(new URL("/v1/models", server.url)); + if (raw.status !== 200) throw new Error("public model list status: " + raw.status); + const data = await raw.json(); + if (data.data.some(row => row.id === "codex-auto-review")) throw new Error("reviewer leaked into public models"); + const rich = await fetch(new URL("/v1/models?client_version=0.160.0", server.url)); + if (rich.status !== 200) throw new Error("Codex model list status: " + rich.status); + const catalog = await rich.json(); + const internal = catalog.models.filter(row => row.slug === "codex-auto-review"); + if (internal.length !== 1 || internal[0].visibility !== "hide") throw new Error("missing hidden reviewer in Codex HTTP catalog"); + } finally { await server.stop(true); } + } + console.log("AUTO_REVIEW_RESULT=" + JSON.stringify(passes)); + })(); + `; + try { + const result = spawnSync(process.execPath, ["--eval", script], { + cwd: repoRoot(), env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: ocxHome }, + encoding: "utf8", maxBuffer: 8 * 1024 * 1024, + }); + expect({ status: result.status, stderr: result.stderr }).toMatchObject({ status: 0 }); + const passes = JSON.parse(result.stdout.split("\n").find(line => line.startsWith("AUTO_REVIEW_RESULT="))!.slice("AUTO_REVIEW_RESULT=".length)) as RawEntry[][][]; + for (const pass of passes) for (const rows of pass) { + if (includeNative) { + expect(reviewRow(rows).comp_hash).toBe("persisted"); + expect(rows[0]!.supported_reasoning_levels).toEqual(upstream.supported_reasoning_levels); + } else expect(rows).toEqual([]); + } + } finally { rmSync(root, { recursive: true, force: true }); } + }); + } +} diff --git a/tests/codex-integration/catalog-full-picker-order.test.ts b/tests/codex-integration/catalog-full-picker-order.test.ts index 1360d726483..4dd35f915b2 100644 --- a/tests/codex-integration/catalog-full-picker-order.test.ts +++ b/tests/codex-integration/catalog-full-picker-order.test.ts @@ -96,7 +96,7 @@ test("fresh routed rows do not inherit a previously ordered native template's gu applyFullModelPickerOrder(rows, order); expect(effectiveSubagentRoster(featured, "v2", rows)).toEqual(before); - expect(rows.toSorted((a, b) => Number(a.priority) - Number(b.priority)).map(row => row.slug)).toEqual(order); + expect(rows.filter(row => row.visibility === "list").toSorted((a, b) => Number(a.priority) - Number(b.priority)).map(row => row.slug)).toEqual(order); expect(template).toEqual(previousTemplate); }); @@ -457,7 +457,7 @@ test("public catalog wrapper applies saved full order while preserving guidance const natural = build([]); const order = ["gpt-5.5", "p/f", "p/e", "p/d", "p/c", "p/b", "p/a"]; const ordered = build(order); - expect(ordered.toSorted((a, b) => Number(a.priority) - Number(b.priority)).map(row => row.slug)).toEqual(order); + expect(ordered.filter(row => row.visibility === "list").toSorted((a, b) => Number(a.priority) - Number(b.priority)).map(row => row.slug)).toEqual(order); expect(effectiveSubagentRoster(featured, "v1", ordered)).toEqual(effectiveSubagentRoster(featured, "v1", natural)); // Independently mirror the upstream description's visible-priority window, not the OCX helper. const nativeDescription = ordered.toSorted((a, b) => Number(a.priority) - Number(b.priority)) diff --git a/tests/codex-integration/codex-catalog-golden.test.ts b/tests/codex-integration/codex-catalog-golden.test.ts index 4dfd7545666..82f2f8db8a6 100644 --- a/tests/codex-integration/codex-catalog-golden.test.ts +++ b/tests/codex-integration/codex-catalog-golden.test.ts @@ -73,6 +73,7 @@ describe("codex-catalog golden (pure buildCatalogEntries oracle)", () => { // Full structural snapshot (the oracle): exact slug set + priority + ws projection. expect(projection.map(p => `${p.slug}@${p.priority}`).sort()).toEqual([ + "codex-auto-review@43", "gpt-5.5@0", "gpt-5.6-sol@4", "kiro/claude-opus-4.6@1", diff --git a/tests/codex-integration/codex-catalog-sync-hardening.test.ts b/tests/codex-integration/codex-catalog-sync-hardening.test.ts index 08274315198..22eced12bc7 100644 --- a/tests/codex-integration/codex-catalog-sync-hardening.test.ts +++ b/tests/codex-integration/codex-catalog-sync-hardening.test.ts @@ -156,7 +156,7 @@ describe("Codex catalog sync hardening", () => { nativeEntry("gpt-5.6-luna", 6), nativeEntry("gpt-5.3-codex", 104), // legacy -> drop nativeEntry("gpt-5.2", 104), // legacy -> drop - nativeEntry("codex-auto-review", 104),// legacy -> drop + nativeEntry("codex-auto-review", 104),// control plane -> keep hidden nativeEntry("user-native", 10), // user-added -> keep ], }, null, 2) + "\n"); @@ -187,7 +187,8 @@ describe("Codex catalog sync hardening", () => { expect(slugs).toContain("user-native"); // genuine user native preserved expect(slugs).not.toContain("gpt-5.3-codex"); // legacy dropped expect(slugs).not.toContain("gpt-5.2"); // legacy dropped - expect(slugs).not.toContain("codex-auto-review"); // legacy dropped + expect(slugs).toContain("codex-auto-review"); // control plane retained + expect(JSON.parse(readFileSync(catalogPath, "utf8")).models.find((row: { slug: string }) => row.slug === "codex-auto-review").visibility).toBe("hide"); }); test.each(["bare-cache", "account-cache", "account-catalog"])( diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index b41a1beb973..b553673f240 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,5 +1,5 @@ { - "messages-native-scope.test.ts": "claude-integration", + "messages-native-scope.test.ts": "claude-integration", "catalog-auto-review-row.test.ts": "codex-integration", "cli-log-view-filter.test.ts": "cli", "cli-usage-model-search.test.ts": "cli", "cli-companion-usage.test.ts": "cli", From d0dc6e222d94786f41f1cf9991356253dc29a50f Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 8 Oct 2026 10:00:43 +0900 Subject: [PATCH 2/4] fix(catalog): skip the reviewer row when the pin lacks it --- src/codex/catalog/control-plane.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/codex/catalog/control-plane.ts b/src/codex/catalog/control-plane.ts index 90dde448c47..df3bbbb0f27 100644 --- a/src/codex/catalog/control-plane.ts +++ b/src/codex/catalog/control-plane.ts @@ -14,7 +14,9 @@ export function withCodexControlPlaneRows( for (const slug of CODEX_INTERNAL_OPENAI_MODELS) { const source = sourceRows.find(entry => entry.slug === slug) ?? pinnedNativeModelRows().find(entry => entry.slug === slug); - if (!source) throw new Error(`Pinned Codex control-plane model is missing: ${slug}`); + // A pin without the row degrades to Codex's own task-model fallback rather than failing the + // whole catalog write. + if (!source) continue; const row: RawEntry = structuredClone(source); row.visibility = "hide"; if (wsEnabled) row.supports_websockets = true; From 29626d42ee59990f27572ed1c42eedb81f7d65f3 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 8 Oct 2026 10:49:37 +0900 Subject: [PATCH 3/4] fix(catalog): gate and version the hidden auto-review row Project the internal reviewer before multi-agent mode normalization so explicit v2 and native v1 exceptions apply in both catalog builders. Keep it outside account clones, featured/picker ordering and effort synthesis. Require an ordinary bare native OpenAI row in the final catalog before preserving or backfilling a reviewer. Hidden natives count; Reserve, account-only and native-less catalogs do not. Prune persisted orphan reviewers after effort clamps and preserve the missing-pin skip fallback. Add regression coverage for mode projection, suppressed/Reserve-only native sets, account-only catalogs and orphan cleanup. Keep the three CI-failing tests and their expectations unchanged. --- .../docs/reference/configuration/providers.md | 7 +- src/codex/catalog/build-entries.ts | 11 +-- src/codex/catalog/control-plane.ts | 17 ++++- src/codex/catalog/effort.ts | 9 +++ src/codex/catalog/parsing.ts | 3 +- structure/catalog.md | 2 +- .../catalog-auto-review-row.test.ts | 67 +++++++++++++++++-- 7 files changed, 103 insertions(+), 13 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 42013f164d5..b51c9897509 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -482,11 +482,14 @@ choose the model that reviews approval requests. The root `auto_review_model` se below override it per provider. The [provider guide](/guides/providers/#approval-reviewer-per-provider) has the operator workflow and a worked example. -When native OpenAI rows are included, OpenCodex keeps the hidden `codex-auto-review` row so +When native OpenAI rows are included and the final catalog has an ordinary bare native row +other than Reserve (hidden rows count), OpenCodex keeps the hidden `codex-auto-review` row so Codex can select its preferred approval reviewer when no override is configured. It stays out of model pickers, subagent choices, Desktop lists and public `/v1/models` lists. Provider and root reviewer overrides retain their precedence and do not stamp this internal row. Catalogs -without a native OpenAI path omit it, preserving Codex's fallback to the task model. +without an ordinary bare native row, including Reserve-only catalogs, omit it and preserve +Codex's fallback to the task model. The reviewer receives the same multi-agent mode projection +as ordinary native rows. `autoReviewModel` is the provider-wide reviewer target. A value can be a bare model id of that same provider (the catalog row is normalized to the `provider/model` slug) or a full public catalog diff --git a/src/codex/catalog/build-entries.ts b/src/codex/catalog/build-entries.ts index 26ef3931eb8..15f917347a4 100644 --- a/src/codex/catalog/build-entries.ts +++ b/src/codex/catalog/build-entries.ts @@ -308,10 +308,10 @@ export function buildCatalogEntriesFromObservedState({ delete entry.prefer_websockets; } } - return withCodexControlPlaneRows(applyMultiAgentMode(out, multiAgentMode, multiAgentV2Enabled, { + return applyMultiAgentMode(withCodexControlPlaneRows(out, template ? [template] : [], true, wsEnabled), multiAgentMode, multiAgentV2Enabled, { keepNativeChatGptOnV1, preserveDefaultMultiAgentVersion: isReserveCatalogProjection, - }), template ? [template] : [], gptSlugs.length > 0, wsEnabled); + }); } export function resetCatalogRuntimeStateForTests(): void { @@ -926,7 +926,10 @@ export function mergeCatalogEntriesFromObservedState({ // clobber a hide flag back to list. Bare ids disable every account clone; qualified ids disable // only their generated account row. const versionedEntries = applyMultiAgentMode( - applyNativeVisibility(mergedEntries, disabledModels, alignedAccountBoundEntries.length > 0, observedNativeSlugs), + withCodexControlPlaneRows( + applyNativeVisibility(mergedEntries, disabledModels, alignedAccountBoundEntries.length > 0, observedNativeSlugs), + [...catalogModels, ...baselineCatalogModels, ...routedEntries], includeNativeOpenAi, wsEnabled, + ), multiAgentMode, multiAgentV2Enabled, { keepNativeChatGptOnV1, preserveDefaultMultiAgentVersion: isReserveCatalogProjection, nativeDefaults: nativeMultiAgentDefaults }, @@ -953,7 +956,7 @@ export function mergeCatalogEntriesFromObservedState({ delete entry.opencodex_catalog_kind; entry.opencodex_catalog_kind = kind; } - return withCodexControlPlaneRows(versionedEntries, [...catalogModels, ...baselineCatalogModels, ...routedEntries], includeNativeOpenAi, wsEnabled); + return versionedEntries; } /** Merge retained-sync rows using the process-observed Codex feature state. */ diff --git a/src/codex/catalog/control-plane.ts b/src/codex/catalog/control-plane.ts index df3bbbb0f27..fa11779f6af 100644 --- a/src/codex/catalog/control-plane.ts +++ b/src/codex/catalog/control-plane.ts @@ -1,7 +1,22 @@ import { CODEX_INTERNAL_OPENAI_MODELS, isCodexControlPlaneModel } from "../control-plane-models"; +import { COMBO_NAMESPACE } from "../../combos"; +import { CODEX_NATIVE_ALIAS_CATALOG_KIND } from "./kinds"; +import { NATIVE_RESERVE_MODEL, SUPPORTED_NATIVE_OPENAI_SLUGS } from "./native-models"; +import { isReserveCatalogProjection } from "./reserve"; import { pinnedNativeModelRows } from "./pinned-models"; import type { RawEntry } from "./parsing"; +/** Hidden ordinary natives qualify; Reserve, account choices and routed aliases do not. */ +export function hasOrdinaryNativeOpenAiRow(entries: readonly RawEntry[]): boolean { + return entries.some(entry => typeof entry.slug === "string" + && !entry.slug.includes("/") + && SUPPORTED_NATIVE_OPENAI_SLUGS.has(entry.slug) + && entry.slug !== NATIVE_RESERVE_MODEL + && !isReserveCatalogProjection(entry) + && entry.opencodex_catalog_kind !== CODEX_NATIVE_ALIAS_CATALOG_KIND + && entry.owned_by !== COMBO_NAMESPACE); +} + /** Keep internal reviewer metadata separate from native synthesis, picker ordering and account clones. */ export function withCodexControlPlaneRows( entries: RawEntry[], @@ -10,7 +25,7 @@ export function withCodexControlPlaneRows( wsEnabled: boolean, ): RawEntry[] { const models = entries.filter(entry => !isCodexControlPlaneModel(entry.slug)); - if (!includeNativeOpenAi) return models; + if (!includeNativeOpenAi || !hasOrdinaryNativeOpenAiRow(models)) return models; for (const slug of CODEX_INTERNAL_OPENAI_MODELS) { const source = sourceRows.find(entry => entry.slug === slug) ?? pinnedNativeModelRows().find(entry => entry.slug === slug); diff --git a/src/codex/catalog/effort.ts b/src/codex/catalog/effort.ts index b983968a8c4..5d4152210ec 100644 --- a/src/codex/catalog/effort.ts +++ b/src/codex/catalog/effort.ts @@ -1,3 +1,4 @@ +import { hasOrdinaryNativeOpenAiRow } from "./control-plane"; import { isCodexControlPlaneModel } from "../control-plane-models"; import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; @@ -533,6 +534,14 @@ export function clampCatalogModelsToObservedCodexSupport( else index += 1; } + // Only Reserve rows are currently omitted above; ordinary natives keep a fallback ladder. + // Also repair a persisted orphan so a removed Reserve can never leave a reviewer behind. + if (!hasOrdinaryNativeOpenAiRow(models)) { + for (let index = models.length - 1; index >= 0; index -= 1) { + if (isCodexControlPlaneModel(models[index]!.slug)) models.splice(index, 1); + } + } + return { removedEfforts: [...removed].sort(), affectedModels: affected, diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index 16b75401adb..0fcb2de5112 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -1,3 +1,4 @@ +import { isCodexControlPlaneModel } from "../control-plane-models"; import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { copyFileSync, existsSync, mkdirSync, readFileSync, realpathSync } from "node:fs"; @@ -700,7 +701,7 @@ export function catalogEntryIsNativeChatGpt(entry: RawEntry): boolean { && entry.use_responses_lite === true && hasNativeOpenAiCapabilityMetadata(routedNativeSlug) ) return true; - if (UPSTREAM_NATIVE_ENTRIES.has(slug) || SUPPORTED_NATIVE_OPENAI_SLUGS.has(slug)) return true; + if (isCodexControlPlaneModel(slug) || UPSTREAM_NATIVE_ENTRIES.has(slug) || SUPPORTED_NATIVE_OPENAI_SLUGS.has(slug)) return true; return false; } diff --git a/structure/catalog.md b/structure/catalog.md index 7bf6c051365..f5b58173e0f 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -47,7 +47,7 @@ provider-wide fallback. Exact model output limits precede the provider default o - preserves native OpenAI entries from the live catalog or static fallback, and emits gpt-5.6 natives from the pinned upstream models.json snapshot - (`src/codex/data/upstream-models.json` — exact per-slug ladders: luna has no ultra); When native OpenAI rows are included, `src/codex/catalog/control-plane.ts` separately preserves exactly one hidden `codex-auto-review` row or backfills its pinned metadata, including Low and the exact upstream ladder. It bypasses native synthesis, account cloning, picker/featured ordering, effort clamps and reviewer override stamps; public lists and subagent rosters exclude it. The dependency-free `src/codex/control-plane-models.ts` shares the exact id set with the router without expanding visible native replacement authority; + (`src/codex/data/upstream-models.json` — exact per-slug ladders: luna has no ultra); When native OpenAI rows are included and the final catalog has an ordinary bare native row other than Reserve (hidden rows count), `src/codex/catalog/control-plane.ts` separately preserves exactly one hidden `codex-auto-review` row or backfills its pinned metadata, including Low and the exact upstream ladder. It receives the ordinary native multi-agent projection but bypasses native synthesis, account cloning, picker/featured ordering, reasoning-ladder clamps and reviewer override stamps; Reserve-only/native-less catalogs omit it, and the final effort clamp removes persisted orphan reviewers; public lists and subagent rosters exclude it. The dependency-free `src/codex/control-plane-models.ts` shares the exact id set with the router without expanding visible native replacement authority; - reads pinned native rows only through `pinnedNativeModelRows()` (`src/codex/catalog/pinned-models.ts`): the codex-rs snapshot first, then rows from `src/codex/data/roster-pinned-models.json` whose slug the snapshot lacks. The roster file holds diff --git a/tests/codex-integration/catalog-auto-review-row.test.ts b/tests/codex-integration/catalog-auto-review-row.test.ts index 7b4539ff15a..c6a74144b86 100644 --- a/tests/codex-integration/catalog-auto-review-row.test.ts +++ b/tests/codex-integration/catalog-auto-review-row.test.ts @@ -5,14 +5,17 @@ import { join } from "node:path"; import { applyFullModelPickerOrder, buildCatalogEntriesFromObservedState, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, mergeCatalogEntriesFromObservedState, - mergeCatalogModelsWithNativeRecovery, type ObservedCatalogMergeInput, + mergeCatalogModelsWithNativeRecovery, type ObservedCatalogMergeInput, type ObservedCatalogEntryBuildInput, } from "../../src/codex/catalog/build-entries"; import { applyAutoReviewModelOverride, applyConfiguredAutoReviewModelOverride } from "../../src/codex/catalog/auto-review"; import { clampCatalogModelsToObservedCodexSupport } from "../../src/codex/catalog/effort"; import { filterSupportedNativeSlugs, type RawEntry } from "../../src/codex/catalog/parsing"; import { desktopVisibleNativeSlugs, upstreamNativeEntry, visibleNativeSlugs } from "../../src/codex/catalog/metadata"; -import { SUPPORTED_NATIVE_OPENAI_SLUGS } from "../../src/codex/catalog/native-models"; +import { NATIVE_RESERVE_MODEL, SUPPORTED_NATIVE_OPENAI_SLUGS } from "../../src/codex/catalog/native-models"; import { effectiveSubagentRoster, SPAWN_PRIORITY_FIELD } from "../../src/codex/catalog/subagent-roster"; +import { CODEX_ACCOUNT_BOUND_CATALOG_KIND } from "../../src/codex/catalog/account-models"; +import { RESERVE_METADATA_SOURCE_FIELD } from "../../src/codex/catalog/reserve"; +import { CODEX_NATIVE_ALIAS_CATALOG_KIND } from "../../src/codex/catalog/kinds"; import snapshot from "../../src/codex/data/upstream-models.json"; import { repoPath, repoRoot } from "../helpers/repo-root"; import { CODEX_INTERNAL_OPENAI_MODELS } from "../../src/codex/control-plane-models"; @@ -40,6 +43,62 @@ function merge(catalogModels: RawEntry[] = [], includeNativeOpenAi = true, extra policy: { ...CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, warningPolicy: "suppress" }, ...extra, }); } +function build(extra: Partial = {}): RawEntry[] { + return buildCatalogEntriesFromObservedState({ + template: null, gptSlugs: ["gpt-5.5"], goModels: [], wsEnabled: false, + multiAgentMode: "v2", multiAgentV2Enabled: true, exactComboSlugs: new Set(), + accountSelectors: [], suppressedBareNativeSlugs: new Set(), disabledNativeAccountSlugs: new Set(), + ...extra, + }); +} +const reserve: RawEntry = { + slug: `personal/${NATIVE_RESERVE_MODEL}`, visibility: "list", + opencodex_catalog_kind: CODEX_ACCOUNT_BOUND_CATALOG_KIND, + [RESERVE_METADATA_SOURCE_FIELD]: NATIVE_RESERVE_MODEL, + supported_reasoning_levels: [{ effort: "xhigh", description: "Only xhigh" }], + default_reasoning_level: "xhigh", +}; + +test.each(["v1", "v2"] as const)("reviewer receives explicit %s mode in build and merge", mode => { + expect(reviewRow(build({ multiAgentMode: mode })).multi_agent_version).toBe(mode); + expect(reviewRow(merge([], true, { multiAgentMode: mode })).multi_agent_version).toBe(mode); +}); + +test("reviewer follows the native v1 exception to explicit v2 mode", () => { + expect(reviewRow(build({ keepNativeChatGptOnV1: true })).multi_agent_version).toBe("v1"); + expect(reviewRow(merge([], true, { keepNativeChatGptOnV1: true })).multi_agent_version).toBe("v1"); +}); + +test("Reserve-only or native-less final catalogs omit persisted reviewers in both writers", () => { + for (const gptSlugs of [[], [NATIVE_RESERVE_MODEL], [reviewer], ["gpt-5.5"]]) { + const rows = build({ gptSlugs, suppressedBareNativeSlugs: new Set(["gpt-5.5"]) }); + expect(rows.some(row => row.slug === reviewer)).toBe(false); + } + for (const catalogModels of [[], [{ slug: NATIVE_RESERVE_MODEL }], [{ slug: "user-native" }], + [{ slug: "gpt-5.5", opencodex_catalog_kind: CODEX_NATIVE_ALIAS_CATALOG_KIND }]]) { + const rows = merge([...catalogModels, structuredClone(upstream)], true, { + accountBoundEntries: [reserve], + policy: { nativeBackfillSlugs: [], unsupportedNativeEntries: "drop", warningPolicy: "suppress" }, + }); + expect(rows.some(row => row.slug === reviewer)).toBe(false); + } +}); + +test("hidden ordinary bare native rows qualify while account-only rows do not", () => { + const policy = { nativeBackfillSlugs: [], unsupportedNativeEntries: "drop" as const, warningPolicy: "suppress" as const }; + const rows = merge([{ slug: "gpt-5.5", visibility: "hide" }], true, { disabledModels: new Set(["gpt-5.5"]), policy }); + expect(reviewRow(rows).visibility).toBe("hide"); + const accountOnly = merge([], true, { policy, accountBoundEntries: [{ slug: "personal/gpt-5.5", visibility: "list" }] }); + expect(accountOnly.some(row => row.slug === reviewer)).toBe(false); +}); + +test("final effort clamp removes an orphan reviewer when its only native-looking row is Reserve", () => { + const rows = [{ slug: "external/model", visibility: "list" }, structuredClone(reserve), structuredClone(upstream)]; + const diagnostic = clampCatalogModelsToObservedCodexSupport(rows, new Set(["medium"])); + expect(rows.map(row => row.slug)).toEqual(["external/model"]); + expect(diagnostic.affectedModels).toContain(reserve.slug); +}); + function reviewRow(rows: RawEntry[]): RawEntry { const matches = rows.filter(row => row.slug === reviewer); expect(matches).toHaveLength(1); @@ -58,7 +117,7 @@ test("build backfills the hidden reviewer with its exact upstream ladder and no expect(row.supported_reasoning_levels).toEqual(upstream.supported_reasoning_levels); expect(row.supported_reasoning_levels).toEqual(expect.arrayContaining([expect.objectContaining({ effort: "low" })])); expect(row.default_reasoning_level).toBe(upstream.default_reasoning_level); - expect(row.multi_agent_version).toBe(upstream.multi_agent_version); + expect(row.multi_agent_version).toBe("v2"); expect(row.priority).toBe(upstream.priority); expect(rows.some(entry => entry.slug === `account/${reviewer}`)).toBe(false); applyFullModelPickerOrder(rows, [reviewer, "gpt-5.5"]); @@ -92,7 +151,7 @@ test("merge preserves the first live row over duplicate persisted/generated rows baselineCatalogModels: [{ ...structuredClone(upstream), display_name: "Baseline" }], routedEntries: [structuredClone(upstream)], }); - const expected = { ...before, visibility: "hide" }; + const expected = { ...before, visibility: "hide", multi_agent_version: "v2", opencodex_multi_agent_version_origin: "v1" }; delete expected.prefer_websockets; expect(reviewRow(rows)).toEqual(expected); expect(live).toEqual(before); From 0fe19a46d57f59b7cd428f3edb78277de2b2fb66 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 8 Oct 2026 11:20:42 +0900 Subject: [PATCH 4/4] test(layout): compact two registrations to restore layout.json headroom --- scripts/test-layout/layout.json | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index d01131fec9a..76f17e5504a 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -23,10 +23,8 @@ "cli-update-restart-child.test.ts": "cli", "cli-update-restart-transport.test.ts": "cli", "cli-update-restart-home.test.ts": "cli", - "codex-shim-overlay.test.ts": "codex-integration", - "codex-shim-migration.test.ts": "codex-integration", - "codex-shim-overlay-transaction.test.ts": "codex-integration", - "codex-shim-overlay-readiness.test.ts": "codex-integration", + "codex-shim-overlay.test.ts": "codex-integration", "codex-shim-migration.test.ts": "codex-integration", + "codex-shim-overlay-transaction.test.ts": "codex-integration", "codex-shim-overlay-readiness.test.ts": "codex-integration", "codex-shim-overlay-candidates.test.ts": "codex-integration", "gui-pair-delivery.test.ts": "gui", "usage-antigravity-55.test.ts": "usage",