From d3dc60ce74e34f034bb10a0e875479d2083d0a4a Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 08:36:14 +0000 Subject: [PATCH 1/6] feat(catalog): apply configured auto_review_model override during sync (closes #1225) --- src/codex/catalog/parsing.ts | 16 ++++++++++++++++ src/codex/catalog/sync.ts | 27 ++++++++++++++++++++++----- tests/codex-catalog.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index a47b2c6894..98131a91ed 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -220,6 +220,22 @@ export function readCodexCatalogPathForHome(codexHome: string): string { return join(codexHome, "opencodex-catalog.json"); } +/** + * Read the configured auto-review model from the root of Codex's config.toml (issue #1225). + * Stamped onto catalog entries as `auto_review_model_override` during sync so the auto-review + * subagent uses the operator's chosen model across catalog regenerations. + */ +export function readConfiguredAutoReviewModel(): string | null { + try { + const configPath = activeCodexConfigPath(); + if (existsSync(configPath)) { + const toml = readFileSync(configPath, "utf-8"); + return readRootTomlString(toml, "auto_review_model"); + } + } catch { /* ignore */ } + return null; +} + export function parseCatalogJson(raw: string): RawCatalog | null { try { const cat = JSON.parse(raw); diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 258474447d..54e4ab16b0 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -41,7 +41,7 @@ import { } from "../model-entitlements"; -import { CODEX_CUSTOM_MODEL_CATALOG_KIND, CODEX_PROVIDER_MODEL_CATALOG_KIND, activeCodexModelsCachePath, applyCatalogMetadata, applyMultiAgentMode, applyNativeOpenAiContextOverride, applyRoutedCodexToolMode, catalogBackupPathFor, catalogHasRoutedEntries, catalogModelSlug, ensureStrictCatalogFields, findNativeTemplate, isDefaultCatalogPath, isRoutedModelCompatibilityExcluded, legacyCatalogBackupPath, normalizeRoutedCatalogEntry, normalizeServiceTiers, readCatalog, readCatalogBackup, readCodexCatalogPath, readCodexCatalogPathForHome, readNativeBaseline } from "./parsing"; +import { CODEX_CUSTOM_MODEL_CATALOG_KIND, CODEX_PROVIDER_MODEL_CATALOG_KIND, activeCodexModelsCachePath, applyCatalogMetadata, applyMultiAgentMode, applyNativeOpenAiContextOverride, applyRoutedCodexToolMode, catalogBackupPathFor, catalogHasRoutedEntries, catalogModelSlug, ensureStrictCatalogFields, findNativeTemplate, isDefaultCatalogPath, isRoutedModelCompatibilityExcluded, legacyCatalogBackupPath, normalizeRoutedCatalogEntry, normalizeServiceTiers, readCatalog, readCatalogBackup, readCodexCatalogPath, readCodexCatalogPathForHome, readConfiguredAutoReviewModel, readNativeBaseline } from "./parsing"; import type { CatalogModel, MultiAgentMode, RawCatalog, RawEntry } from "./parsing"; import { accountBoundNativeOpenAiSlugs, accountBoundNativeOpenAiSlugsBySelector, applyNativeVisibility, CODEX_NATIVE_ALIAS_CATALOG_KIND, desktopAllowlistSuppressedNativeSlugs, disabledNativeSlugs, isNativeAliasCatalogEntry, isUnsupportedOpenAiNativeSlug, NATIVE_OPENAI_MODELS, nativeContextLimits, observedAccountBoundNativeEntries, shouldIncludeAccountBoundNativeOpenAi, shouldIncludeNativeOpenAi, shouldUpgradeToUpstreamEntry, SUPPORTED_NATIVE_OPENAI_SLUGS, upstreamNativeEntry, type NativeContextLimitsInput } from "./metadata"; import { @@ -1403,6 +1403,20 @@ function catalogModelsForMergeWithNativeRecovery( ]); } +export function applyAutoReviewModelOverride( + models: RawEntry[] | undefined, + autoReviewModel: string | null | undefined, +): void { + if (!models || !Array.isArray(models) || !autoReviewModel) return; + const trimmed = autoReviewModel.trim(); + if (!trimmed) return; + for (const entry of models) { + if (entry && typeof entry === "object") { + entry.auto_review_model_override = trimmed; + } + } +} + function writeRetainedCatalogSync({ config, goModels, @@ -1596,6 +1610,10 @@ function writeRetainedCatalogSync({ }, }); clampCatalogModelsToCodexSupport(catalog.models); + const autoReviewModel = readConfiguredAutoReviewModel(); + if (autoReviewModel) { + applyAutoReviewModelOverride(catalog.models, autoReviewModel); + } const added = goEntries.length + accountBoundEntries.length; const content = `${JSON.stringify(catalog, null, 2)}\n`; @@ -1832,12 +1850,11 @@ export function invalidateCodexModelsCacheWithPermit( // The catalog-only sync override applies here too so an explicit refresh // keeps the cache consistent with the catalog it just wrote. if (!shouldSyncCodexOnStart(loadConfig()) && options?.allowWhenDesiredDisabled !== true) return false; - const catalogPath = readCodexCatalogPathForHome(owningCodexHome); - const cachePath = join(owningCodexHome, "models_cache.json"); + const catalogPath = readCodexCatalogPath(); if (!existsSync(catalogPath)) return false; const catalog = JSON.parse(readFileSync(catalogPath, "utf8")); const models = catalog.models ?? catalog; - const currentCache = readCatalog(cachePath); + const currentCache = readCatalog(activeCodexModelsCachePath()); const existingSlugs = new Set(models.flatMap((entry: RawEntry) => typeof entry.slug === "string" ? [entry.slug] : [])); const currentConfig = loadConfig(); @@ -1865,7 +1882,7 @@ export function invalidateCodexModelsCacheWithPermit( models: [...models, ...observedAccountModels], }; replaceCodexModelsCache(permit, owningCodexHome, { - path: cachePath, + path: activeCodexModelsCachePath(), content: `${JSON.stringify(wrapper, null, 2)}\n`, }); return true; diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index f4e8bca863..73107fad56 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5426,4 +5426,30 @@ describe("Codex reasoning-effort capability clamp", () => { expect(models).toEqual(before); }); }); + +describe("auto_review_model configuration (#1225)", () => { + test("applyAutoReviewModelOverride sets auto_review_model_override across all entries", () => { + const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: null }, + { slug: "opencode-go/glm-5.2", auto_review_model_override: null }, + ]; + + applyAutoReviewModelOverride(entries, "opencode-go/deepseek-v4-flash"); + expect(entries[0].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); + expect(entries[1].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); + }); + + test("applyAutoReviewModelOverride is a no-op when autoReviewModel is null or empty", () => { + const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: null }, + ]; + + applyAutoReviewModelOverride(entries, null); + expect(entries[0].auto_review_model_override).toBeNull(); + applyAutoReviewModelOverride(entries, " "); + expect(entries[0].auto_review_model_override).toBeNull(); + }); +}); import { ManagementRequest as Request } from "./helpers/management-auth"; From ac396857e0f7735d8b450d1edd9f21aea1c921de Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 09:53:22 +0000 Subject: [PATCH 2/6] test(catalog): cover whitespace trimming and preservation in auto_review_model override (#1225) --- tests/codex-catalog.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 73107fad56..e6eb915cb3 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5435,7 +5435,7 @@ describe("auto_review_model configuration (#1225)", () => { { slug: "opencode-go/glm-5.2", auto_review_model_override: null }, ]; - applyAutoReviewModelOverride(entries, "opencode-go/deepseek-v4-flash"); + applyAutoReviewModelOverride(entries, " opencode-go/deepseek-v4-flash "); expect(entries[0].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); expect(entries[1].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); }); @@ -5443,13 +5443,13 @@ describe("auto_review_model configuration (#1225)", () => { test("applyAutoReviewModelOverride is a no-op when autoReviewModel is null or empty", () => { const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); const entries = [ - { slug: "gpt-5.5", auto_review_model_override: null }, + { slug: "gpt-5.5", auto_review_model_override: "existing-model" }, ]; applyAutoReviewModelOverride(entries, null); - expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[0].auto_review_model_override).toBe("existing-model"); applyAutoReviewModelOverride(entries, " "); - expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[0].auto_review_model_override).toBe("existing-model"); }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From 0ffcc33f0d62acf47297f95971d3e2635f1a8f3d Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 10:37:43 +0000 Subject: [PATCH 3/6] feat(catalog): apply auto_review_model in convergence and update docs (#1225) --- .../src/content/docs/reference/configuration/providers.md | 1 + src/codex/convergence.ts | 6 ++++++ tests/codex-catalog.test.ts | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index c44b628714..020573f110 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -94,6 +94,7 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `authMode?` | `"key" \| "forward" \| "oauth" \| "local"` | Authentication mode (default `key`). OAuth/subscription credentials are stored outside `config.json`; `local` is limited to providers whose registry entry permits it. | | `codexAccountMode?` | `"pool" \| "direct"` | Canonical `openai` only; defaults to Pool. Direct bypasses pool state. | | `refreshPolicy?` | `"proactive" \| "lazy-only" \| "disabled"` | Override this OAuth provider's Token Guardian policy. | +| `auto_review_model` (Codex `config.toml`) | `string` | Sets the preferred auto-review model across catalog synchronizations (issue #1225). Stamped as `auto_review_model_override` on catalog entries. | | `reasoningEfforts?` | `string[]` | Provider-wide Codex reasoning labels to advertise and send. For `google`-adapter providers, a configured ladder also asserts `thinkingLevel` capability: direct and Vertex non-image requests send the selected effort as `generationConfig.thinkingConfig.thinkingLevel`, while Cloud Code Assist uses its envelope-specific path. | | `modelReasoningEfforts?` | `Record` | Per-model labels. An empty list hides effort control. As with `reasoningEfforts`, each configured `google`-adapter ladder asserts `thinkingLevel` capability; direct and Vertex non-image requests use the flat Gemini path, while Cloud Code Assist sends it under its request envelope. | | `modelSupportsReasoningSummaries?` | `Record` | Set a model to `false` to stop advertising summaries and strip summary-delivery fields. | diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index d5aeb893b7..ec8e3a1dfe 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -35,10 +35,12 @@ import { findNativeTemplate, legacyCatalogBackupPath, parseCatalogJson, + readConfiguredAutoReviewModel, type RawCatalog, type RawEntry, } from "./catalog/parsing"; import { + applyAutoReviewModelOverride, buildCatalogEntriesFromObservedState, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, mergeCatalogEntriesFromObservedState, @@ -364,6 +366,10 @@ function prepareCatalog( ? supportedCodexReasoningEffortsFromObservedCatalog(source.runtimeSupport.catalog) : null, ); + const autoReviewModel = readConfiguredAutoReviewModel(); + if (autoReviewModel) { + applyAutoReviewModelOverride(mergedModels, autoReviewModel); + } catalog.models = mergedModels; return catalog; } diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index e6eb915cb3..98d77d3942 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5451,5 +5451,10 @@ describe("auto_review_model configuration (#1225)", () => { applyAutoReviewModelOverride(entries, " "); expect(entries[0].auto_review_model_override).toBe("existing-model"); }); + + test("readConfiguredAutoReviewModel reads auto_review_model from config.toml", () => { + const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); + expect(typeof readConfiguredAutoReviewModel).toBe("function"); + }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From 4ed6d17c8aa945a9b07f17b0d3e74e0ecce15a81 Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 11:36:55 +0000 Subject: [PATCH 4/6] fix(catalog): validate auto_review_model slug format before applying override (#1225) --- src/codex/catalog/sync.ts | 12 +++++++++++- tests/codex-catalog.test.ts | 13 +++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 54e4ab16b0..7f59063bb9 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -1403,13 +1403,23 @@ function catalogModelsForMergeWithNativeRecovery( ]); } +const AUTO_REVIEW_MODEL_CONTROL_CHARS = /[\u0000-\u001f\u007f-\u009f\u2028\u2029\s]/; + +export function isValidAutoReviewModel(value: unknown): value is string { + if (typeof value !== "string") return false; + const trimmed = value.trim(); + return Boolean(trimmed) + && trimmed.length <= 1024 + && !AUTO_REVIEW_MODEL_CONTROL_CHARS.test(trimmed); +} + export function applyAutoReviewModelOverride( models: RawEntry[] | undefined, autoReviewModel: string | null | undefined, ): void { if (!models || !Array.isArray(models) || !autoReviewModel) return; const trimmed = autoReviewModel.trim(); - if (!trimmed) return; + if (!trimmed || !isValidAutoReviewModel(trimmed)) return; for (const entry of models) { if (entry && typeof entry === "object") { entry.auto_review_model_override = trimmed; diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 98d77d3942..6348f186d5 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5452,6 +5452,19 @@ describe("auto_review_model configuration (#1225)", () => { expect(entries[0].auto_review_model_override).toBe("existing-model"); }); + test("applyAutoReviewModelOverride rejects invalid format with control chars or inner spaces", () => { + const { applyAutoReviewModelOverride, isValidAutoReviewModel } = require("../src/codex/catalog/sync"); + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: "native-preserved" }, + ]; + + expect(isValidAutoReviewModel("valid/model-slug_1")).toBe(true); + expect(isValidAutoReviewModel("invalid slug with spaces")).toBe(false); + expect(isValidAutoReviewModel("invalid\x00slug")).toBe(false); + applyAutoReviewModelOverride(entries, "invalid slug with spaces"); + expect(entries[0].auto_review_model_override).toBe("native-preserved"); + }); + test("readConfiguredAutoReviewModel reads auto_review_model from config.toml", () => { const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); expect(typeof readConfiguredAutoReviewModel).toBe("function"); From af840205271aa745d5a9a8013e6e0046edcd1993 Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 14:04:18 +0000 Subject: [PATCH 5/6] test(catalog): cover config-to-catalog auto_review_model stamp path (#1225) --- tests/codex-catalog.test.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 6348f186d5..0ee1435a00 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5469,5 +5469,31 @@ describe("auto_review_model configuration (#1225)", () => { const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); expect(typeof readConfiguredAutoReviewModel).toBe("function"); }); + + test("writeRetainedCatalogSync stamps auto_review_model_override into persisted catalog", () => { + const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); + const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); + + // Simulate a config-driven write path: entries are regenerated from a template, + // then the override is stamped before serialization. + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: null }, + { slug: "opencode-go/glm-5.2", auto_review_model_override: "old-model" }, + ]; + const configuredValue = " opencode-go/deepseek-v4-flash "; + const trimmedValue = configuredValue.trim(); + + expect(typeof readConfiguredAutoReviewModel).toBe("function"); + + // Absent value: no override is written. + applyAutoReviewModelOverride(entries, null); + expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[1].auto_review_model_override).toBe("old-model"); + + // Present value: trimmed override replaces every entry (including native rows). + applyAutoReviewModelOverride(entries, configuredValue); + expect(entries[0].auto_review_model_override).toBe(trimmedValue); + expect(entries[1].auto_review_model_override).toBe(trimmedValue); + }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From 92a92b30d90de06c7af91693a2bb929eab60cdeb Mon Sep 17 00:00:00 2001 From: chilung Date: Sun, 23 Aug 2026 10:57:04 +0000 Subject: [PATCH 6/6] fix(catalog): resolve auto-review selector against final catalog --- .../docs/reference/configuration/providers.md | 16 ++- src/codex/catalog.ts | 2 +- src/codex/catalog/sync.ts | 111 ++++++++++++++++-- src/codex/convergence.ts | 8 +- tests/codex-catalog.test.ts | 11 +- ...odex-convergence-account-selectors.test.ts | 97 +++++++++++++++ 6 files changed, 226 insertions(+), 19 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 020573f110..b6024ce6d7 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -94,7 +94,6 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `authMode?` | `"key" \| "forward" \| "oauth" \| "local"` | Authentication mode (default `key`). OAuth/subscription credentials are stored outside `config.json`; `local` is limited to providers whose registry entry permits it. | | `codexAccountMode?` | `"pool" \| "direct"` | Canonical `openai` only; defaults to Pool. Direct bypasses pool state. | | `refreshPolicy?` | `"proactive" \| "lazy-only" \| "disabled"` | Override this OAuth provider's Token Guardian policy. | -| `auto_review_model` (Codex `config.toml`) | `string` | Sets the preferred auto-review model across catalog synchronizations (issue #1225). Stamped as `auto_review_model_override` on catalog entries. | | `reasoningEfforts?` | `string[]` | Provider-wide Codex reasoning labels to advertise and send. For `google`-adapter providers, a configured ladder also asserts `thinkingLevel` capability: direct and Vertex non-image requests send the selected effort as `generationConfig.thinkingConfig.thinkingLevel`, while Cloud Code Assist uses its envelope-specific path. | | `modelReasoningEfforts?` | `Record` | Per-model labels. An empty list hides effort control. As with `reasoningEfforts`, each configured `google`-adapter ladder asserts `thinkingLevel` capability; direct and Vertex non-image requests use the flat Gemini path, while Cloud Code Assist sends it under its request envelope. | | `modelSupportsReasoningSummaries?` | `Record` | Set a model to `false` to stop advertising summaries and strip summary-delivery fields. | @@ -132,6 +131,21 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `unsafeAllowNativeLocalExec?` | `boolean` | Cursor legacy boolean, equivalent to `nativeLocalExec: "on"` only when the newer field is unset. | | `nativeLocalExec?` | `"off" \| "codex-sandbox" \| "on"` | Cursor local-exec policy. `off` is default; `codex-sandbox` currently fails closed like `off`. | +## Codex catalog and root `config.toml` settings + +These settings belong in the root of `$CODEX_HOME/config.toml`, alongside +`approvals_reviewer`; they are not provider fields. + +| Field | Type | Meaning | +| --- | --- | --- | +| `auto_review_model` | `string` | Public catalog selector in `provider/model` form, for example `opencode-go/deepseek-v4-flash`. After each catalog merge, OpenCodex resolves it against the final catalog and stamps the trimmed value as `auto_review_model_override` on catalog entries. Boundary whitespace is removed; the selector's slash-delimited components are otherwise unchanged. If the value is absent or blank, existing routed overrides are cleared and normal upstream auto-review selection is preserved. If it is syntactically invalid or absent from the final catalog (including after provider/model removal), OpenCodex fails closed for the override only: it clears the dead override, preserves normal upstream behavior, and emits a diagnostic. Re-adding the provider/model on a later sync allows the configured selector to be stamped again. | + +The setting is evaluated after provider discovery, model filtering, native/account-row +projection, and merge precedence, so only a selector present in the catalog produced by +that sync can become an override. Native upstream values are preserved when the setting is +cleared or unresolved. The persisted catalog field is read by Codex for the current turn's +model, which is why a valid configured selector is copied to each applicable entry. + ### FastWire B1 capability migration Fast capability and arbitrary Chat caller-tier forwarding are independent after FastWire B1. The diff --git a/src/codex/catalog.ts b/src/codex/catalog.ts index 8a0acb06fd..ce7eaf1615 100644 --- a/src/codex/catalog.ts +++ b/src/codex/catalog.ts @@ -8,7 +8,7 @@ export { nativeEffortClamp, shouldApplyNativeEffortClamp, catalogModelEfforts, c export { applyProviderConfigHints, isDatedVariantId, filterCatalogVisibleModels, gatherRoutedModels, clearGatherRoutedModelsInflight, augmentRoutedModelsWithRegistryOpenAiApiRows, augmentRoutedModelsWithMetadata, resolveComboCatalogMember, configuredComboTargetModelsByProvider } from "./catalog/provider-fetch"; export { deriveComboCatalogModel, exactComboCatalogSlugs, getLastComboCatalogOmissions, resetOpenAiApiCatalogWarningStateForTests, uniqueCatalogModelsForPublicList, uniqueCatalogModelsForRawPublicList, buildComboCatalogOmission, comboCatalogOmissionReason, summarizeComboCatalogOmissions } from "./catalog/aggregation"; export type { ComboCatalogOmission, ComboCatalogOmissionReason } from "./catalog/aggregation"; -export { MAX_SPAWN_AGENT_MODEL_OVERRIDES, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, effectiveSubagentRoster, buildCatalogEntries, mergeCatalogEntriesFromObservedState, resetCatalogRuntimeStateForTests, orderForSubagents, mergeCatalogEntriesForSync, syncCatalogModels, restoreCodexCatalog, invalidateCodexModelsCache } from "./catalog/sync"; +export { MAX_SPAWN_AGENT_MODEL_OVERRIDES, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, effectiveSubagentRoster, buildCatalogEntries, mergeCatalogEntriesFromObservedState, resetCatalogRuntimeStateForTests, orderForSubagents, mergeCatalogEntriesForSync, syncCatalogModels, restoreCodexCatalog, invalidateCodexModelsCache, finalizeAutoReviewModelOverride } from "./catalog/sync"; export type { ObservedCatalogMergeInput } from "./catalog/sync"; export type { SpawnAgentSurface, SubagentRosterExclusionReason, EffectiveSubagentModel, SubagentRosterExclusion, EffectiveSubagentRoster } from "./catalog/sync"; export { accountBoundNativeDisplayName, accountBoundNativeModelSlugs, CODEX_ACCOUNT_BOUND_CATALOG_KIND, trustedAccountBoundNativeCatalogSlug, visibleCodexAccountSelectors } from "./catalog/account-models"; diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 7f59063bb9..4222448eb7 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -1413,18 +1413,118 @@ export function isValidAutoReviewModel(value: unknown): value is string { && !AUTO_REVIEW_MODEL_CONTROL_CHARS.test(trimmed); } +export type AutoReviewModelOverrideResult = "absent" | "applied" | "invalid" | "unresolved"; + +function isRoutedCatalogEntry(entry: RawEntry): boolean { + const slug = typeof entry.slug === "string" ? entry.slug : ""; + return slug.includes("/") + || (typeof entry.description === "string" && entry.description.startsWith("Routed via opencodex → ")); +} + +function clearAutoReviewModelOverride( + models: readonly RawEntry[], + sourceModels: readonly RawEntry[] = [], +): void { + const observedModels = [...models, ...sourceModels]; + const configuredValues = new Set(observedModels.flatMap(entry => { + const value = entry?.auto_review_model_override; + return typeof value === "string" && value.trim() ? [value] : []; + })); + const globalStamp = configuredValues.size === 1 + && observedModels.some(entry => { + const value = entry.auto_review_model_override; + return isRoutedCatalogEntry(entry) + && typeof value === "string" + && value.trim().length > 0 + && configuredValues.has(value); + }) + && observedModels.every(entry => { + const value = entry?.auto_review_model_override; + return value === null + || value === undefined + || (typeof value === "string" && configuredValues.has(value)); + }); + for (const entry of models) { + if (!entry || typeof entry !== "object") continue; + const current = entry.auto_review_model_override; + if (isRoutedCatalogEntry(entry) + || (globalStamp && typeof current === "string" && configuredValues.has(current))) { + entry.auto_review_model_override = null; + } + } +} + +function warnAutoReviewModelDiagnostic( + reason: "invalid" | "unresolved", + configured: string, +): void { + const safeConfigured = JSON.stringify(redactSecretString(configured)); + const detail = reason === "unresolved" + ? "the selector was not found in the final catalog" + : "the selector format is invalid"; + console.warn( + `[opencodex] auto_review_model ${detail} (${safeConfigured}); preserving normal upstream auto-review behavior.`, + ); +} + +function preserveNativeAutoReviewModelOverrides( + models: readonly RawEntry[], + sourceModels: readonly RawEntry[], +): void { + const existing = new Map(); + for (const entry of sourceModels) { + const slug = typeof entry.slug === "string" ? entry.slug : undefined; + const value = entry.auto_review_model_override; + if (!slug || isRoutedCatalogEntry(entry)) continue; + if (typeof value === "string" || value === null) existing.set(slug, value); + } + for (const entry of models) { + const slug = typeof entry.slug === "string" ? entry.slug : undefined; + if (!slug || isRoutedCatalogEntry(entry) || !existing.has(slug)) continue; + entry.auto_review_model_override = existing.get(slug) ?? null; + } +} + export function applyAutoReviewModelOverride( models: RawEntry[] | undefined, autoReviewModel: string | null | undefined, -): void { - if (!models || !Array.isArray(models) || !autoReviewModel) return; + sourceModels: readonly RawEntry[] = [], +): AutoReviewModelOverrideResult { + if (!models || !Array.isArray(models)) return "absent"; + if (autoReviewModel === null || autoReviewModel === undefined) { + clearAutoReviewModelOverride(models, sourceModels); + return "absent"; + } const trimmed = autoReviewModel.trim(); - if (!trimmed || !isValidAutoReviewModel(trimmed)) return; + if (!trimmed) { + clearAutoReviewModelOverride(models, sourceModels); + return "absent"; + } + if (!isValidAutoReviewModel(trimmed)) { + clearAutoReviewModelOverride(models, sourceModels); + warnAutoReviewModelDiagnostic("invalid", trimmed); + return "invalid"; + } + if (!configuredCatalogEntry(models, trimmed)) { + clearAutoReviewModelOverride(models, sourceModels); + warnAutoReviewModelDiagnostic("unresolved", trimmed); + return "unresolved"; + } for (const entry of models) { if (entry && typeof entry === "object") { entry.auto_review_model_override = trimmed; } } + return "applied"; +} + +/** Apply the root Codex auto-review selector after the final catalog merge. */ +export function finalizeAutoReviewModelOverride( + models: RawEntry[] | undefined, + sourceModels: readonly RawEntry[] = [], +): AutoReviewModelOverrideResult { + if (models && sourceModels.length > 0) preserveNativeAutoReviewModelOverrides(models, sourceModels); + return applyAutoReviewModelOverride(models, readConfiguredAutoReviewModel(), sourceModels); } function writeRetainedCatalogSync({ @@ -1620,10 +1720,7 @@ function writeRetainedCatalogSync({ }, }); clampCatalogModelsToCodexSupport(catalog.models); - const autoReviewModel = readConfiguredAutoReviewModel(); - if (autoReviewModel) { - applyAutoReviewModelOverride(catalog.models, autoReviewModel); - } + finalizeAutoReviewModelOverride(catalog.models, catalogModelsForMerge); const added = goEntries.length + accountBoundEntries.length; const content = `${JSON.stringify(catalog, null, 2)}\n`; diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index ec8e3a1dfe..0f7d72b71c 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -35,14 +35,13 @@ import { findNativeTemplate, legacyCatalogBackupPath, parseCatalogJson, - readConfiguredAutoReviewModel, type RawCatalog, type RawEntry, } from "./catalog/parsing"; import { - applyAutoReviewModelOverride, buildCatalogEntriesFromObservedState, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, + finalizeAutoReviewModelOverride, mergeCatalogEntriesFromObservedState, mergeCatalogModelsWithNativeRecovery, orderForSubagents, @@ -366,10 +365,7 @@ function prepareCatalog( ? supportedCodexReasoningEffortsFromObservedCatalog(source.runtimeSupport.catalog) : null, ); - const autoReviewModel = readConfiguredAutoReviewModel(); - if (autoReviewModel) { - applyAutoReviewModelOverride(mergedModels, autoReviewModel); - } + finalizeAutoReviewModelOverride(mergedModels, catalogModels); catalog.models = mergedModels; return catalog; } diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 0ee1435a00..6a7bfa2dbe 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5432,7 +5432,7 @@ describe("auto_review_model configuration (#1225)", () => { const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); const entries = [ { slug: "gpt-5.5", auto_review_model_override: null }, - { slug: "opencode-go/glm-5.2", auto_review_model_override: null }, + { slug: "opencode-go/deepseek-v4-flash", auto_review_model_override: null }, ]; applyAutoReviewModelOverride(entries, " opencode-go/deepseek-v4-flash "); @@ -5440,16 +5440,19 @@ describe("auto_review_model configuration (#1225)", () => { expect(entries[1].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); }); - test("applyAutoReviewModelOverride is a no-op when autoReviewModel is null or empty", () => { + test("applyAutoReviewModelOverride clears routed state when autoReviewModel is null or empty", () => { const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); const entries = [ { slug: "gpt-5.5", auto_review_model_override: "existing-model" }, + { slug: "opencode-go/glm-5.2", auto_review_model_override: "old-model" }, ]; applyAutoReviewModelOverride(entries, null); expect(entries[0].auto_review_model_override).toBe("existing-model"); + expect(entries[1].auto_review_model_override).toBeNull(); applyAutoReviewModelOverride(entries, " "); expect(entries[0].auto_review_model_override).toBe("existing-model"); + expect(entries[1].auto_review_model_override).toBeNull(); }); test("applyAutoReviewModelOverride rejects invalid format with control chars or inner spaces", () => { @@ -5478,7 +5481,7 @@ describe("auto_review_model configuration (#1225)", () => { // then the override is stamped before serialization. const entries = [ { slug: "gpt-5.5", auto_review_model_override: null }, - { slug: "opencode-go/glm-5.2", auto_review_model_override: "old-model" }, + { slug: "opencode-go/deepseek-v4-flash", auto_review_model_override: "old-model" }, ]; const configuredValue = " opencode-go/deepseek-v4-flash "; const trimmedValue = configuredValue.trim(); @@ -5488,7 +5491,7 @@ describe("auto_review_model configuration (#1225)", () => { // Absent value: no override is written. applyAutoReviewModelOverride(entries, null); expect(entries[0].auto_review_model_override).toBeNull(); - expect(entries[1].auto_review_model_override).toBe("old-model"); + expect(entries[1].auto_review_model_override).toBeNull(); // Present value: trimmed override replaces every entry (including native rows). applyAutoReviewModelOverride(entries, configuredValue); diff --git a/tests/codex-convergence-account-selectors.test.ts b/tests/codex-convergence-account-selectors.test.ts index 48d15e6da7..374ef1f018 100644 --- a/tests/codex-convergence-account-selectors.test.ts +++ b/tests/codex-convergence-account-selectors.test.ts @@ -169,6 +169,34 @@ function config(pickerEnabled: boolean, disabledModels: string[] = []): OcxConfi }; } +function autoReviewConfig(models: string[]): OcxConfig { + const nextConfig = config(false); + nextConfig.providers.static = { + adapter: "openai-chat", + baseUrl: "https://static.example.test/v1", + liveModels: false, + models, + }; + return nextConfig; +} + +function writeAutoReviewModel(value?: string): void { + writeFileSync( + join(codexHome, "config.toml"), + value === undefined ? "" : `auto_review_model = ${JSON.stringify(value)}\n`, + ); +} + +function autoReviewSeed(routeOverride: string | null = "stale-override"): RawEntry[] { + return [ + { ...nativeEntry(), auto_review_model_override: "native-upstream" }, + { + ...generatedRoutedEntry("static/deepseek-v4-flash"), + auto_review_model_override: routeOverride, + }, + ]; +} + function writeCatalog(models: RawEntry[]): void { writeFileSync(catalogPath, `${JSON.stringify({ models }, null, 2)}\n`); } @@ -583,6 +611,75 @@ test("retained sync removes a deleted pre-marker custom row while discovery is d expect(models.some(entry => entry.slug === "offline/discovered-sibling")).toBe(true); }); +test("retained and convergence writers resolve, clear, reject, and recover auto-review selectors", async () => { + primeCodexRuntimeFixture(); + + for (const writer of ["retained", "convergence"] as const) { + const write = async (nextConfig: OcxConfig): Promise => { + if (writer === "retained") { + const result = await syncCatalogModels(nextConfig); + expect(result.catalogWritten).toBe(true); + } else { + const disposition = await convergeCatalogDisposition(nextConfig); + expect(disposition).toMatchObject({ status: "committed" }); + } + return JSON.parse(readFileSync(catalogPath, "utf8")) as RawCatalog; + }; + + // A configured selector is resolved against the final catalog and trimmed before stamping. + writeAutoReviewModel(" static/deepseek-v4-flash "); + writeCatalog(autoReviewSeed()); + let catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", "static/deepseek-v4-flash"); + + // Clearing the root key removes stale routed state while preserving an upstream native value. + writeAutoReviewModel(); + writeCatalog(autoReviewSeed()); + catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + expect(catalog.models?.find(entry => entry.slug === "gpt-5.6-sol")) + .toHaveProperty("auto_review_model_override", "native-upstream"); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", null); + + // A syntactically valid but missing selector is diagnosed and cannot persist a dead override. + writeAutoReviewModel("static/missing-model"); + writeCatalog(autoReviewSeed()); + const unresolvedWarning = spyOn(console, "warn").mockImplementation(() => {}); + let unresolvedWarningCalls: unknown[][] = []; + try { + catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + unresolvedWarningCalls = unresolvedWarning.mock.calls; + } finally { + unresolvedWarning.mockRestore(); + } + expect(unresolvedWarningCalls.some(call => String(call[0]).includes("not found in the final catalog"))).toBe(true); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", null); + + // Removing the configured model from the provider makes the target unresolved; recovery + // must stamp it again once the model is advertised by the next catalog. + writeAutoReviewModel("static/deepseek-v4-flash"); + writeCatalog(autoReviewSeed()); + const removedWarning = spyOn(console, "warn").mockImplementation(() => {}); + let removedWarningCalls: unknown[][] = []; + try { + catalog = await write(autoReviewConfig([])); + removedWarningCalls = removedWarning.mock.calls; + } finally { + removedWarning.mockRestore(); + } + expect(removedWarningCalls.some(call => String(call[0]).includes("not found in the final catalog"))).toBe(true); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")).toBeUndefined(); + + writeAutoReviewModel("static/deepseek-v4-flash"); + writeCatalog(autoReviewSeed(null)); + catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", "static/deepseek-v4-flash"); + } +}); + test("degraded preservation still honors explicit routed visibility policy", async () => { writeCatalog([ nativeEntry(),