diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index bfc084cf0ae..b51c9897509 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -478,10 +478,19 @@ 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 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 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 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 adb067020a5..117a43f5aa1 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,7 +2,7 @@ "version": 1, "root": "tests", "explicit": { - "claude-picker-upload.test.ts": "claude-integration", "messages-native-scope.test.ts": "claude-integration", + "claude-picker-upload.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", @@ -19,10 +19,8 @@ "cli-capability-provider-workflows.test.ts": "cli", "cli-capability-access-workflows.test.ts": "cli", "cli-capability-lab-workflows.test.ts": "cli", "skill-ocx-workflows.test.ts": "ci-workflows", "cli-capability-data.test.ts": "cli", "skill-ocx-generated.test.ts": "ci-workflows", "cli-update-restart.test.ts": "cli", "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", 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..15f917347a4 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,7 +308,7 @@ export function buildCatalogEntriesFromObservedState({ delete entry.prefer_websockets; } } - return applyMultiAgentMode(out, multiAgentMode, multiAgentV2Enabled, { + return applyMultiAgentMode(withCodexControlPlaneRows(out, template ? [template] : [], true, wsEnabled), multiAgentMode, multiAgentV2Enabled, { keepNativeChatGptOnV1, preserveDefaultMultiAgentVersion: isReserveCatalogProjection, }); @@ -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 => @@ -921,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 }, diff --git a/src/codex/catalog/control-plane.ts b/src/codex/catalog/control-plane.ts new file mode 100644 index 00000000000..fa11779f6af --- /dev/null +++ b/src/codex/catalog/control-plane.ts @@ -0,0 +1,45 @@ +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[], + sourceRows: readonly RawEntry[], + includeNativeOpenAi: boolean, + wsEnabled: boolean, +): RawEntry[] { + const models = entries.filter(entry => !isCodexControlPlaneModel(entry.slug)); + 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); + // 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; + 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..5d4152210ec 100644 --- a/src/codex/catalog/effort.ts +++ b/src/codex/catalog/effort.ts @@ -1,3 +1,5 @@ +import { hasOrdinaryNativeOpenAiRow } from "./control-plane"; +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 +387,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; @@ -532,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/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..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); + (`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/structure/runtime.md b/structure/runtime.md index c66ddfd0172..6a501b4c35a 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..c6a74144b86 --- /dev/null +++ b/tests/codex-integration/catalog-auto-review-row.test.ts @@ -0,0 +1,300 @@ +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, 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 { 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"; +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 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); + 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("v2"); + 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", multi_agent_version: "v2", opencodex_multi_agent_version_origin: "v1" }; + 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 b3b636b8207..8f364ef005d 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,6 +1,6 @@ { "claude-picker-upload.test.ts": "claude-integration", - "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",