From 036ed4bb98c36d7af6fdfe1626a727692d96b3c5 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Tue, 25 Aug 2026 11:36:54 +0800 Subject: [PATCH 01/13] feat(catalog): provider-level auto-review model override --- src/codex/catalog/parsing.ts | 2 + src/codex/catalog/provider-fetch.ts | 16 ++- src/codex/catalog/sync.ts | 8 ++ src/config.ts | 5 + src/providers/derive.ts | 29 ++++++ src/providers/registry.ts | 4 + src/router.ts | 6 ++ src/server/auth-cors.ts | 22 +++++ src/types/provider.ts | 14 +++ structure/02_config-and-codex-home.md | 15 +++ tests/auto-review-model-override.test.ts | 121 +++++++++++++++++++++++ 11 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 tests/auto-review-model-override.test.ts diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index 0293b4bd04..739c1f4d48 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -137,6 +137,8 @@ export interface CatalogModel { * "shell" leaves tool_mode unset so Codex declares top-level shell tools (exec_command). */ codexToolMode?: "code_mode_only" | "shell"; + /** Codex auto-review (approvals) model override for this routed row. */ + autoReviewModelOverride?: string; /** Normalized upstream capability names retained for management/API consumers (#485 follow-up). */ capabilities?: string[]; /** OpenCodex-only catalog ownership marker; Codex ignores the serialized extension field. */ diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 5ef9dffa2b..300add3c8e 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -31,7 +31,7 @@ import type { OcxConfig, OcxProviderConfig } from "../../types"; import { modelInList } from "../../types"; import { CODEX_REASONING_LEVELS, codexEffortRank, configuredReasoningEfforts, modelRecordValue, sanitizeCodexReasoningEfforts } from "../../reasoning-effort"; import { getModelMetadata, getModelMetadataCaseInsensitive, listModelMetadata, resolveMetadataProvider } from "../../generated/model-metadata"; -import { enrichProviderFromRegistry, shouldCaseFoldMetadataModelId } from "../../providers/derive"; +import { enrichProviderFromRegistry, resolveAutoReviewModel, shouldCaseFoldMetadataModelId } from "../../providers/derive"; import { captureFastPolicyAuthority, fastPolicyForModel, @@ -667,6 +667,19 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, const configuredCap = configuredContextWindow(prov, model.id); const configuredMaxInput = configuredMaxInputTokens(prov, model.id); const configuredAutoCompact = configuredAutoCompactTokenLimit(prov, model.id); + const autoReviewTarget = resolveAutoReviewModel(prov, model.id); + let autoReviewOverride: string | undefined; + if (autoReviewTarget !== null) { + const staticIds = [...(prov.models ?? []), ...(getProviderRegistryEntry(name)?.models ?? [])]; + if (autoReviewTarget.includes("/") || staticIds.length === 0 || staticIds.includes(autoReviewTarget)) { + autoReviewOverride = autoReviewTarget; + } else { + console.warn( + "[opencodex] autoReviewModel target \"" + autoReviewTarget + "\" for " + name + "/" + model.id + + " is not a known model of " + name + "; catalog override skipped.", + ); + } + } let inputModalities = configuredInputModalities(prov, model.id); // Vision-sidecar coverage: `noVisionModels` marks models whose images the PROXY describes // (src/vision/index.ts). The catalog must still advertise image input for them — the Codex app @@ -697,6 +710,7 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, const hinted = { ...modelWithoutServiceTier, ...(hintedWindow !== undefined ? { contextWindow: hintedWindow } : {}), + ...(autoReviewOverride !== undefined ? { autoReviewModelOverride: autoReviewOverride } : {}), ...(inputModalities ? { inputModalities } : {}), ...(reasoningEfforts !== undefined ? { reasoningEfforts } : {}), ...(configuredMaxInput !== undefined diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index fda9724849..07539322e8 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -347,6 +347,14 @@ export function deriveEntry( if (model) applyCatalogMetadata(e, model.provider, model.id, model.contextCap); applyCatalogModelMetadata(e, model); if (model?.catalogKind) e.opencodex_catalog_kind = model.catalogKind; + // Codex auto-review (approvals) override: stamp the catalog field Codex reads. + // Bare targets resolve to this provider's catalog slug; namespaced targets are + // kept verbatim. Rows without an override keep the template's null. + if (model?.autoReviewModelOverride) { + e.auto_review_model_override = model.autoReviewModelOverride.includes("/") + ? model.autoReviewModelOverride + : model.provider + "/" + model.autoReviewModelOverride; + } } else { applyNativeOpenAiContextOverride(e, contextCap); if (isGpt56NativeSlug(slug)) ensureGpt56ReasoningLevels(e); diff --git a/src/config.ts b/src/config.ts index 1308b3a64b..9ef234e4e5 100644 --- a/src/config.ts +++ b/src/config.ts @@ -500,6 +500,11 @@ const providerConfigSchema = z.object({ responsesPath: z.string().min(1).optional(), statelessResponses: z.boolean().optional(), requiresAdjacentResponsesToolResults: z.boolean().optional(), + autoReviewModel: z.string().trim().min(1).refine(value => !/\s/.test(value), "must not contain whitespace").optional(), + autoReviewModelOverrides: z.record( + z.string().min(1), + z.string().trim().min(1).refine(value => !/\s/.test(value), "must not contain whitespace"), + ).optional(), fastWire: fastWireSchema.nullable().optional(), supportsServiceTier: z.boolean().optional(), modelSupportsServiceTier: z.record(z.string().min(1), z.boolean()).optional(), diff --git a/src/providers/derive.ts b/src/providers/derive.ts index de5c5821e1..fa73e559b1 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -254,6 +254,10 @@ export function providerConfigSeed(entry: ProviderRegistryEntry): OcxProviderCon ...(entry.requiresAdjacentResponsesToolResults !== undefined ? { requiresAdjacentResponsesToolResults: entry.requiresAdjacentResponsesToolResults } : {}), + ...(entry.autoReviewModel !== undefined ? { autoReviewModel: entry.autoReviewModel } : {}), + ...(entry.autoReviewModelOverrides !== undefined + ? { autoReviewModelOverrides: { ...entry.autoReviewModelOverrides } } + : {}), ...(entry.autoToolChoiceOnlyModels ? { autoToolChoiceOnlyModels: [...entry.autoToolChoiceOnlyModels] } : {}), ...(entry.preserveReasoningContentModels ? { preserveReasoningContentModels: [...entry.preserveReasoningContentModels] } : {}), ...(entry.requiresReasoningPlaceholderModels ? { requiresReasoningPlaceholderModels: [...entry.requiresReasoningPlaceholderModels] } : {}), @@ -501,6 +505,12 @@ export function enrichProviderFromRegistry(name: string, prov: OcxProviderConfig if (prov.requiresAdjacentResponsesToolResults === undefined && seed.requiresAdjacentResponsesToolResults !== undefined) { prov.requiresAdjacentResponsesToolResults = seed.requiresAdjacentResponsesToolResults; } + if (prov.autoReviewModel === undefined && seed.autoReviewModel !== undefined) { + prov.autoReviewModel = seed.autoReviewModel; + } + if (prov.autoReviewModelOverrides === undefined && seed.autoReviewModelOverrides !== undefined) { + prov.autoReviewModelOverrides = { ...seed.autoReviewModelOverrides }; + } // Registry-only metadata (never seeded into saved config): backfill straight from // the entry so an explicit user value stays distinguishable from the default. if (prov.fastWire === undefined && entry.fastWire !== undefined) { @@ -602,6 +612,25 @@ function customPreset(): DerivedProviderPreset { return { id: "custom", label: "Custom provider", adapter: "openai-chat", baseUrl: "", auth: "key" }; } +/** + * Resolve the Codex auto-review (approvals) model for one routed model id. + * Per-model overrides win over the provider-wide default; both are opt-in and + * trimmed. Returns the configured target (bare id or `provider/model` slug) or + * null when the operator left the session-model behavior untouched. + */ +export function resolveAutoReviewModel( + provider: OcxProviderConfig | undefined, + modelId: string, +): string | null { + if (!provider) return null; + const perModel = provider.autoReviewModelOverrides?.[modelId]; + if (typeof perModel === "string" && perModel.trim() !== "") return perModel.trim(); + if (typeof provider.autoReviewModel === "string" && provider.autoReviewModel.trim() !== "") { + return provider.autoReviewModel.trim(); + } + return null; +} + function formatInitLabel(entry: ProviderRegistryEntry): string { if (entry.authKind === "forward") return "OpenAI — ChatGPT login (no key; account pool default, Direct selectable)"; if (entry.authKind === "oauth") { diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 91405ca8d7..c886ee638f 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -215,6 +215,10 @@ export interface ProviderRegistryEntry { * to stay contiguous. This is seeded/backfilled like other fixed wire capabilities. */ requiresAdjacentResponsesToolResults?: boolean; + /** Optional registry default for the Codex auto-review model (provider-wide). */ + autoReviewModel?: string; + /** Optional registry per-model auto-review defaults (model id -> approval model id). */ + autoReviewModelOverrides?: Record; /** * Registry default for the provider's `service_tier` support; see * `OcxProviderConfig.supportsServiceTier`. Registry-only: backfilled (never diff --git a/src/router.ts b/src/router.ts index 489451de3e..1d1b4f7ca8 100644 --- a/src/router.ts +++ b/src/router.ts @@ -353,6 +353,12 @@ export function routedProviderConfig(providerName: string, provider: OcxProvider && registryEntry.requiresAdjacentResponsesToolResults !== undefined ? { requiresAdjacentResponsesToolResults: registryEntry.requiresAdjacentResponsesToolResults } : {}), + ...(provider.autoReviewModel === undefined && registryEntry.autoReviewModel !== undefined + ? { autoReviewModel: registryEntry.autoReviewModel } + : {}), + ...(provider.autoReviewModelOverrides === undefined && registryEntry.autoReviewModelOverrides !== undefined + ? { autoReviewModelOverrides: { ...registryEntry.autoReviewModelOverrides } } + : {}), ...(provider.fastWire === undefined && registryEntry.fastWire !== undefined ? { fastWire: cloneFastWire(registryEntry.fastWire), diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 0d62f232c9..8f3ac64c42 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -528,6 +528,24 @@ function nativeContextOverlayError(raw: Record): string | null return null; } +/** Validate the Codex auto-review model override shape at the management write boundary. */ +function autoReviewModelConfigError(model: unknown, overrides: unknown): string | null { + if (model !== undefined && (typeof model !== "string" || model.trim() === "" || /\s/.test(model))) { + return "autoReviewModel must be a nonblank model id without whitespace"; + } + if (overrides === undefined) return null; + if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { + return "autoReviewModelOverrides must be an object mapping model ids to approval model ids"; + } + for (const [key, value] of Object.entries(overrides as Record)) { + if (key.trim() === "") return "autoReviewModelOverrides keys must be nonblank model ids"; + if (typeof value !== "string" || value.trim() === "" || /\s/.test(value)) { + return "autoReviewModelOverrides values must be nonblank model ids without whitespace"; + } + } + return null; +} + /** * Validate a provider object arriving at the management write boundary. Returns an error * string, or null when the provider may be persisted. Caller-controlled names/fields are @@ -636,6 +654,8 @@ export function providerManagementConfigError(name: unknown, provider: unknown): if (raw.responsesSnapshotRepair !== undefined && typeof raw.responsesSnapshotRepair !== "boolean") { return `provider ${name} responsesSnapshotRepair must be a boolean`; } + const autoReviewError = autoReviewModelConfigError(raw.autoReviewModel, raw.autoReviewModelOverrides); + if (autoReviewError) return "provider " + name + " " + autoReviewError; const defaultMaxOutputError = positiveIntegerConfigError(raw.defaultMaxOutputTokens, "defaultMaxOutputTokens"); if (defaultMaxOutputError) return `provider ${name} ${defaultMaxOutputError}`; const maxOutputError = positiveIntegerRecordConfigError(raw.modelMaxOutputTokens, "modelMaxOutputTokens"); @@ -738,6 +758,8 @@ export function safeConfigDTO(config: OcxConfig): unknown { "autoToolChoiceOnlyModels", "preserveReasoningContentModels", "requiresReasoningPlaceholderModels", + "autoReviewModel", + "autoReviewModelOverrides", "escapeBuiltinToolNames", ] as const) { copyIfDefined(dto, provider, key); diff --git a/src/types/provider.ts b/src/types/provider.ts index b7ba042506..b50cc53787 100644 --- a/src/types/provider.ts +++ b/src/types/provider.ts @@ -197,6 +197,20 @@ export interface OcxProviderConfig { * preserved after it, and parallel calls stay together with the reasoning turn that produced them. */ requiresAdjacentResponsesToolResults?: boolean; + /** + * Provider-wide default for the Codex auto-review (approvals) subagent model. + * Stamped as `auto_review_model_override` on every routed catalog row of this + * provider; absent keeps Codex's session-model behavior. A target is a bare + * model id (resolved within this provider) or a `provider/model` catalog slug. + * Opt-in; no vendor defaults. + */ + autoReviewModel?: string; + /** + * Per-model auto-review overrides (model id -> approval model id). Wins over + * `autoReviewModel`; same target grammar. Absent/undefined keeps session-model + * behavior for that model. + */ + autoReviewModelOverrides?: Record; /** * Provider fallback for canonical Fast capability over an OpenAI `service_tier` wire. * This pure tri-state feeds catalog publication, routing eligibility, compatibility diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 3685fb5bb4..11972c1e12 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -364,3 +364,18 @@ uninstall with their exact paths. Legacy nonempty config directories are deliberately not retroactively claimed. If either ownership file is missing, malformed, or bound to another root, uninstall refuses config deletion and reports the residual directory for manual review; there is no recursive-delete fallback. + +## Auto-review (approval) model override + +Codex picks its auto-review subagent from the session model's catalog row field +`auto_review_model_override`; when absent it uses the session model itself. Providers may opt in +per provider or per model: + +- `providers..autoReviewModel`: provider-wide default approval model for every routed row. +- `providers..autoReviewModelOverrides`: object mapping a session model id to its approval + model; the per-model value wins. + +Targets are either a bare model id (resolved to `/` in the catalog) or a +`provider/model` catalog slug of any configured provider. The override is stamped onto the +routed catalog row during sync as `auto_review_model_override`; unknown bare targets are skipped +with a warning rather than emitted (fail closed). The feature is opt-in — no vendor defaults. diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts new file mode 100644 index 0000000000..726a916be1 --- /dev/null +++ b/tests/auto-review-model-override.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, test } from "bun:test"; +import { resolveAutoReviewModel } from "../src/providers/derive"; +import { deriveEntry } from "../src/codex/catalog/sync"; +import { applyProviderConfigHints } from "../src/codex/catalog/provider-fetch"; +import { providerManagementConfigError, safeConfigDTO } from "../src/server/auth-cors"; +import type { CatalogModel } from "../src/codex/catalog/parsing"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; + +describe("resolveAutoReviewModel", () => { + test("per-model override wins over provider-wide", () => { + const provider = { + autoReviewModel: "deepseek-v4-pro", + autoReviewModelOverrides: { "deepseek-v4-flash-vision-exp": "deepseek-v4-flash" }, + } as OcxProviderConfig; + expect(resolveAutoReviewModel(provider, "deepseek-v4-flash-vision-exp")).toBe("deepseek-v4-flash"); + expect(resolveAutoReviewModel(provider, "deepseek-v4-flash")).toBe("deepseek-v4-pro"); + }); + + test("provider-wide default applies when no per-model entry exists", () => { + const provider = { autoReviewModel: " deepseek-v4-flash " } as OcxProviderConfig; + expect(resolveAutoReviewModel(provider, "anything")).toBe("deepseek-v4-flash"); + }); + + test("returns null when nothing is configured", () => { + expect(resolveAutoReviewModel({} as OcxProviderConfig, "m")).toBeNull(); + expect(resolveAutoReviewModel(undefined, "m")).toBeNull(); + }); +}); + +describe("catalog stamping", () => { + const template = { auto_review_model_override: null, context_window: 272000 } as Record; + + test("bare override target becomes the provider/model slug", () => { + const model: CatalogModel = { + id: "deepseek-v4-flash-vision-exp", + provider: "deepseek", + autoReviewModelOverride: "deepseek-v4-flash", + }; + const entry = deriveEntry(template, "deepseek/deepseek-v4-flash-vision-exp", "desc", 5, model); + expect(entry.auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); + }); + + test("namespaced target is kept verbatim", () => { + const model: CatalogModel = { + id: "m", + provider: "blsc", + autoReviewModelOverride: "deepseek/deepseek-v4-flash", + }; + const entry = deriveEntry(template, "blsc/m", "desc", 5, model); + expect(entry.auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); + }); + + test("no override keeps the template value", () => { + const model: CatalogModel = { id: "m", provider: "blsc" }; + const entry = deriveEntry(template, "blsc/m", "desc", 5, model); + expect(entry.auto_review_model_override).toBeNull(); + }); +}); + +describe("provider-fetch hints", () => { + test("attaches the override for a known bare target", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + models: ["deepseek-v4-flash", "deepseek-v4-flash-vision-exp"], + autoReviewModel: "deepseek-v4-flash", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("deepseek", prov, { + id: "deepseek-v4-flash-vision-exp", + provider: "deepseek", + }); + expect(hinted.autoReviewModelOverride).toBe("deepseek-v4-flash"); + }); + + test("skips an unknown bare target without stamping", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + models: ["deepseek-v4-flash"], + autoReviewModel: "does-not-exist", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("deepseek", prov, { + id: "deepseek-v4-flash-vision-exp", + provider: "deepseek", + }); + expect(hinted.autoReviewModelOverride).toBeUndefined(); + }); +}); + +describe("management validation and DTO", () => { + test("management rejects malformed overrides", () => { + const base = { + adapter: "openai-chat", + baseUrl: "https://relay.example/v1", + apiKey: "sk-test", + authMode: "key", + }; + expect(providerManagementConfigError("relay", { ...base, autoReviewModel: " " })).toContain("autoReviewModel"); + expect(providerManagementConfigError("relay", { ...base, autoReviewModelOverrides: { m: 42 } })).toContain("autoReviewModelOverrides"); + expect(providerManagementConfigError("relay", { ...base, autoReviewModel: "deepseek-v4-flash" })).toBeNull(); + }); + + test("safeConfigDTO keeps the override fields", () => { + const config = { + port: 10100, + defaultProvider: "deepseek", + providers: { + deepseek: { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + apiKey: "sk-x", + autoReviewModel: "deepseek-v4-flash", + autoReviewModelOverrides: { "deepseek-v4-flash-vision-exp": "deepseek-v4-pro" }, + }, + }, + } as unknown as OcxConfig; + const dto = safeConfigDTO(config) as { providers: Record }> }; + expect(dto.providers.deepseek?.autoReviewModel).toBe("deepseek-v4-flash"); + expect(dto.providers.deepseek?.autoReviewModelOverrides?.["deepseek-v4-flash-vision-exp"]).toBe("deepseek-v4-pro"); + }); +}); From d9765157300d63f14ea330b8f4fcc807718b035f Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Tue, 25 Aug 2026 11:54:39 +0800 Subject: [PATCH 02/13] fix(catalog): harden auto-review override normalization and validation --- src/codex/catalog/provider-fetch.ts | 67 ++++++++++--- src/codex/catalog/sync.ts | 38 ++++++-- src/codex/convergence.ts | 3 + src/config.ts | 40 +++++++- src/providers/derive.ts | 20 +++- src/server/auth-cors.ts | 19 +++- src/server/management/provider-routes.ts | 41 ++++++++ structure/02_config-and-codex-home.md | 10 +- tests/auto-review-model-override.test.ts | 119 +++++++++++++++++++++-- 9 files changed, 321 insertions(+), 36 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 300add3c8e..a6e089d966 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -42,7 +42,7 @@ import { effectiveGoogleMode, getProviderRegistryEntry, providerMatchesRegistryT import { parseAntigravityAvailableModels, registerAntigravityDiscoveredWireModels } from "../../providers/antigravity-models"; import { applyProviderContextCap, providerContextCap, resolveUnknownRoutedContextWindow } from "../../providers/context-cap"; import { clampAutoCompactTokenLimit } from "../../providers/auto-compact-budget"; -import { routedSlug, slugEquals, slugEquivalenceKey, slugsEquivalent } from "../../providers/slug-codec"; +import { encodeRoutedModelId, routedSlug, slugEquals, slugEquivalenceKey, slugsEquivalent } from "../../providers/slug-codec"; import { CODEX_GPT5_IDENTITY_LINE } from "../../adapters/identity"; import { filterCursorConfiguredModelsByLiveDiscovery } from "../../adapters/cursor/discovery"; import { fetchCursorUsableModels } from "../../adapters/cursor/live-models"; @@ -663,23 +663,53 @@ function configuredVerbositySupport(name: string, prov: OcxProviderConfig | unde return prov.supportsVerbosity; } -export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, model: CatalogModel, providerCap?: number): CatalogModel { - const configuredCap = configuredContextWindow(prov, model.id); - const configuredMaxInput = configuredMaxInputTokens(prov, model.id); - const configuredAutoCompact = configuredAutoCompactTokenLimit(prov, model.id); - const autoReviewTarget = resolveAutoReviewModel(prov, model.id); - let autoReviewOverride: string | undefined; - if (autoReviewTarget !== null) { - const staticIds = [...(prov.models ?? []), ...(getProviderRegistryEntry(name)?.models ?? [])]; - if (autoReviewTarget.includes("/") || staticIds.length === 0 || staticIds.includes(autoReviewTarget)) { - autoReviewOverride = autoReviewTarget; - } else { +const warnedAutoReviewTargets = new Set(); + +/** + * Resolve and normalize the Codex auto-review override for one routed row. + * A target that matches a known native id of the provider is encoded into the + * provider's one-slash catalog slug; a namespaced (cross-provider) target is kept + * verbatim and checked against the assembled catalog at sync time; unknown bare + * targets are skipped (fail closed) with a deduped, redacted warning. + */ +function resolveAutoReviewOverrideForRow( + providerName: string, + provider: OcxProviderConfig | undefined, + modelId: string, +): string | undefined { + const target = resolveAutoReviewModel(provider, modelId); + if (target === null) return undefined; + const known = new Set([ + ...(provider?.models ?? []), + ...(getProviderRegistryEntry(providerName)?.models ?? []), + modelId, + ]); + if (target.includes("/") && !known.has(target)) { + // Cross-provider catalog slug: kept verbatim; final existence is checked + // against the assembled catalog at sync time. + return target; + } + if (known.size === 0 || !known.has(target)) { + const key = providerName + "/" + target; + if (!warnedAutoReviewTargets.has(key)) { + warnedAutoReviewTargets.add(key); console.warn( - "[opencodex] autoReviewModel target \"" + autoReviewTarget + "\" for " + name + "/" + model.id - + " is not a known model of " + name + "; catalog override skipped.", + "[opencodex] autoReviewModel target " + JSON.stringify(redactSecretString(target)) + + " for " + JSON.stringify(redactSecretString(providerName)) + "/" + + JSON.stringify(redactSecretString(modelId)) + + " is not a known model of that provider; catalog override skipped.", ); } + return undefined; } + return routedSlug(providerName, encodeRoutedModelId(target)); +} + +export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, model: CatalogModel, providerCap?: number): CatalogModel { + const configuredCap = configuredContextWindow(prov, model.id); + const configuredMaxInput = configuredMaxInputTokens(prov, model.id); + const configuredAutoCompact = configuredAutoCompactTokenLimit(prov, model.id); + const autoReviewOverride = resolveAutoReviewOverrideForRow(name, prov, model.id); let inputModalities = configuredInputModalities(prov, model.id); // Vision-sidecar coverage: `noVisionModels` marks models whose images the PROXY describes // (src/vision/index.ts). The catalog must still advertise image input for them — the Codex app @@ -710,7 +740,9 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, const hinted = { ...modelWithoutServiceTier, ...(hintedWindow !== undefined ? { contextWindow: hintedWindow } : {}), - ...(autoReviewOverride !== undefined ? { autoReviewModelOverride: autoReviewOverride } : {}), + // Always set the key so a re-hint clears a stale override after the operator + // removes the config; JSON serialization drops the undefined value. + autoReviewModelOverride: autoReviewOverride, ...(inputModalities ? { inputModalities } : {}), ...(reasoningEfforts !== undefined ? { reasoningEfforts } : {}), ...(configuredMaxInput !== undefined @@ -1990,6 +2022,7 @@ async function gatherRoutedModelsUncached( const supportsServiceTier = fastPolicy ? serviceTierSupportFromPolicy(fastPolicy) : undefined; + const autoReviewOverride = resolveAutoReviewOverrideForRow(cm.provider, effectiveProvider, cm.modelId); const base: CatalogModel = { id: cm.modelId, provider: cm.provider, @@ -2068,7 +2101,11 @@ async function gatherRoutedModelsUncached( ...(base.supportsReasoningSummaries === undefined && replaced.supportsReasoningSummaries !== undefined ? { supportsReasoningSummaries: replaced.supportsReasoningSummaries } : {}), ...(base.codexToolMode === undefined && replaced.codexToolMode !== undefined ? { codexToolMode: replaced.codexToolMode } : {}), ...(base.capabilities === undefined && replaced.capabilities !== undefined ? { capabilities: replaced.capabilities } : {}), + ...(base.autoReviewModelOverride === undefined && replaced.autoReviewModelOverride !== undefined + ? { autoReviewModelOverride: replaced.autoReviewModelOverride } + : {}), } : base; + if (autoReviewOverride !== undefined) merged.autoReviewModelOverride = autoReviewOverride; // Vision-sidecar coverage ONLY: if the custom model is in the enriched provider's // noVisionModels, advertise image input so the Codex app lets images reach the sidecar // (#349/#344). Deliberately NOT the full applyProviderConfigHints pass — custom rows are a diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 07539322e8..92a29f8f3f 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -347,13 +347,11 @@ export function deriveEntry( if (model) applyCatalogMetadata(e, model.provider, model.id, model.contextCap); applyCatalogModelMetadata(e, model); if (model?.catalogKind) e.opencodex_catalog_kind = model.catalogKind; - // Codex auto-review (approvals) override: stamp the catalog field Codex reads. - // Bare targets resolve to this provider's catalog slug; namespaced targets are - // kept verbatim. Rows without an override keep the template's null. + // Codex auto-review (approvals) override: the provider-fetch layer already + // normalized the target to a catalog slug; stamp it verbatim. Rows without + // an override keep the template's null. if (model?.autoReviewModelOverride) { - e.auto_review_model_override = model.autoReviewModelOverride.includes("/") - ? model.autoReviewModelOverride - : model.provider + "/" + model.autoReviewModelOverride; + e.auto_review_model_override = model.autoReviewModelOverride; } } else { applyNativeOpenAiContextOverride(e, contextCap); @@ -402,6 +400,9 @@ export function deriveEntry( if (model && isRouted) applyCatalogMetadata(entry, model.provider, model.id, model.contextCap); applyCatalogModelMetadata(entry, model); if (model?.catalogKind) entry.opencodex_catalog_kind = model.catalogKind; + if (model?.autoReviewModelOverride) { + entry.auto_review_model_override = model.autoReviewModelOverride; + } if (!isRouted) applyNativeOpenAiContextOverride(entry, contextCap); return ensureStrictCatalogFields(normalizeServiceTiers(entry), { preserveExactInputModalities: preserveExact, @@ -2015,3 +2016,28 @@ export function invalidateCodexModelsCache(options?: CodexCatalogSyncOptions): b ); return outcome.kind === "completed" && outcome.value; } + +/** + * Final fail-closed pass over the assembled catalog: every stamped + * auto_review_model_override must name a model that is actually emitted. + * Typo'd, disabled, or allowlisted-away targets are removed with one redacted + * warning per target instead of reaching Codex. + */ +export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawEntry[]): void { + const slugs = new Set(entries.map(entry => String(entry.slug))); + const warned = new Set(); + for (const entry of entries) { + const override = entry.auto_review_model_override; + if (typeof override !== "string") continue; + if (!slugs.has(override)) { + if (!warned.has(override)) { + warned.add(override); + console.warn( + "[opencodex] autoReviewModel override " + JSON.stringify(redactSecretString(override)) + + " does not match any catalog model; skipped.", + ); + } + delete entry.auto_review_model_override; + } + } +} diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index b338aa9d3b..727fe7e7cb 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -45,6 +45,7 @@ import { finalizeAutoReviewModelOverride, mergeCatalogEntriesFromObservedState, mergeCatalogModelsWithNativeRecovery, + validateAutoReviewOverridesAgainstCatalog, orderForSubagents, } from "./catalog/sync"; import { multiAgentV2EnabledFromConfigText } from "./features"; @@ -374,6 +375,8 @@ function prepareCatalog( ); finalizeAutoReviewModelOverride(mergedModels, catalogModels); catalog.models = mergedModels; + // Fail-closed final pass: an override must name a model actually emitted. + validateAutoReviewOverridesAgainstCatalog(catalog.models as RawEntry[]); return catalog; } diff --git a/src/config.ts b/src/config.ts index 9ef234e4e5..e5fad18b90 100644 --- a/src/config.ts +++ b/src/config.ts @@ -502,7 +502,7 @@ const providerConfigSchema = z.object({ requiresAdjacentResponsesToolResults: z.boolean().optional(), autoReviewModel: z.string().trim().min(1).refine(value => !/\s/.test(value), "must not contain whitespace").optional(), autoReviewModelOverrides: z.record( - z.string().min(1), + z.string().min(1).refine(key => !/\s/.test(key), "keys must not contain whitespace"), z.string().trim().min(1).refine(value => !/\s/.test(value), "must not contain whitespace"), ).optional(), fastWire: fastWireSchema.nullable().optional(), @@ -1466,6 +1466,43 @@ function sanitizeModelCostsForLoad(parsed: unknown): void { } } +/** + * Load-time sanitizer for the opt-in auto-review override fields. A malformed + * hand-edit must not retire the whole config: invalid values/keys are dropped + * (trimmed, whitespace-free) instead of failing schema validation. The strict + * management write boundary still rejects bad input before it reaches disk. + */ +function sanitizeAutoReviewOverridesForLoad(parsed: unknown): void { + if (!parsed || typeof parsed !== "object") return; + const root = parsed as Record; + const providers = root.providers; + if (!providers || typeof providers !== "object" || Array.isArray(providers)) return; + for (const provider of Object.values(providers as Record)) { + if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; + const row = provider as Record; + if (row.autoReviewModel !== undefined) { + const value = typeof row.autoReviewModel === "string" ? row.autoReviewModel.trim() : ""; + row.autoReviewModel = value !== "" && !/\s/.test(value) ? value : undefined; + } + if (row.autoReviewModelOverrides !== undefined) { + const overrides = row.autoReviewModelOverrides; + if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { + row.autoReviewModelOverrides = undefined; + continue; + } + const cleaned: Record = {}; + for (const [key, value] of Object.entries(overrides as Record)) { + const trimmedKey = key.trim(); + if (trimmedKey === "" || /\s/.test(trimmedKey)) continue; + const trimmedValue = typeof value === "string" ? value.trim() : ""; + if (trimmedValue === "" || /\s/.test(trimmedValue)) continue; + cleaned[trimmedKey] = trimmedValue; + } + row.autoReviewModelOverrides = Object.keys(cleaned).length > 0 ? cleaned : undefined; + } + } +} + /** * Companion to {@link warnDegradedStreamMode} for a blank persisted `hostname`. The bind * falls back to loopback, which is the safe direction but not what the file asked for — @@ -1822,6 +1859,7 @@ export function loadConfig(): OcxConfig { sanitizeAliasesForLoad(parsed); sanitizeRetryOn429ForLoad(parsed); sanitizeModelCostsForLoad(parsed); + sanitizeAutoReviewOverridesForLoad(parsed); const result = configSchema.safeParse(parsed); if (result.success) { const config = normalizeApiKeyIds(result.data as OcxConfig); diff --git a/src/providers/derive.ts b/src/providers/derive.ts index fa73e559b1..a9afe2cab3 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -623,7 +623,7 @@ export function resolveAutoReviewModel( modelId: string, ): string | null { if (!provider) return null; - const perModel = provider.autoReviewModelOverrides?.[modelId]; + const perModel = autoReviewOverrideForModel(provider.autoReviewModelOverrides, modelId); if (typeof perModel === "string" && perModel.trim() !== "") return perModel.trim(); if (typeof provider.autoReviewModel === "string" && provider.autoReviewModel.trim() !== "") { return provider.autoReviewModel.trim(); @@ -631,6 +631,24 @@ export function resolveAutoReviewModel( return null; } +/** Per-model override lookup mirroring modelRecordValue: exact, then :family, then case-fold. */ +function autoReviewOverrideForModel( + overrides: Record | undefined, + modelId: string, +): string | undefined { + if (!overrides) return undefined; + if (Object.prototype.hasOwnProperty.call(overrides, modelId)) return overrides[modelId]; + const colon = modelId.indexOf(":"); + if (colon > 0 && Object.prototype.hasOwnProperty.call(overrides, modelId.slice(0, colon))) { + return overrides[modelId.slice(0, colon)]; + } + const folded = modelId.toLowerCase(); + for (const [key, value] of Object.entries(overrides)) { + if (key.toLowerCase() === folded) return value; + } + return undefined; +} + function formatInitLabel(entry: ProviderRegistryEntry): string { if (entry.authKind === "forward") return "OpenAI — ChatGPT login (no key; account pool default, Direct selectable)"; if (entry.authKind === "oauth") { diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 8f3ac64c42..4321b886e4 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -530,16 +530,23 @@ function nativeContextOverlayError(raw: Record): string | null /** Validate the Codex auto-review model override shape at the management write boundary. */ function autoReviewModelConfigError(model: unknown, overrides: unknown): string | null { - if (model !== undefined && (typeof model !== "string" || model.trim() === "" || /\s/.test(model))) { - return "autoReviewModel must be a nonblank model id without whitespace"; + if (model !== undefined) { + const trimmed = typeof model === "string" ? model.trim() : ""; + if (trimmed === "" || /\s/.test(trimmed)) { + return "autoReviewModel must be a nonblank model id without whitespace"; + } } if (overrides === undefined) return null; if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { return "autoReviewModelOverrides must be an object mapping model ids to approval model ids"; } for (const [key, value] of Object.entries(overrides as Record)) { - if (key.trim() === "") return "autoReviewModelOverrides keys must be nonblank model ids"; - if (typeof value !== "string" || value.trim() === "" || /\s/.test(value)) { + const trimmedKey = key.trim(); + if (trimmedKey === "" || /\s/.test(trimmedKey)) { + return "autoReviewModelOverrides keys must be nonblank model ids without whitespace"; + } + const trimmedValue = typeof value === "string" ? value.trim() : ""; + if (trimmedValue === "" || /\s/.test(trimmedValue)) { return "autoReviewModelOverrides values must be nonblank model ids without whitespace"; } } @@ -655,7 +662,9 @@ export function providerManagementConfigError(name: unknown, provider: unknown): return `provider ${name} responsesSnapshotRepair must be a boolean`; } const autoReviewError = autoReviewModelConfigError(raw.autoReviewModel, raw.autoReviewModelOverrides); - if (autoReviewError) return "provider " + name + " " + autoReviewError; + if (autoReviewError) { + return "provider " + JSON.stringify(redactSecretString(name)) + " " + autoReviewError; + } const defaultMaxOutputError = positiveIntegerConfigError(raw.defaultMaxOutputTokens, "defaultMaxOutputTokens"); if (defaultMaxOutputError) return `provider ${name} ${defaultMaxOutputError}`; const maxOutputError = positiveIntegerRecordConfigError(raw.modelMaxOutputTokens, "modelMaxOutputTokens"); diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index d542306885..585c8732b8 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -145,6 +145,39 @@ function applyProviderPatchFields( else delete next.defaultModel; touched = true; } + if (Object.hasOwn(rawBody, "autoReviewModel")) { + const value = rawBody.autoReviewModel; + if (value === null) { + delete next.autoReviewModel; + touched = true; + } else if (typeof value === "string" && value.trim() !== "" && !/\s/.test(value.trim())) { + next.autoReviewModel = value.trim(); + touched = true; + } else { + return { error: "autoReviewModel must be a nonblank model id without whitespace, or null to clear" }; + } + } + if (Object.hasOwn(rawBody, "autoReviewModelOverrides")) { + const value = rawBody.autoReviewModelOverrides; + if (value === null) { + delete next.autoReviewModelOverrides; + touched = true; + } else if (value && typeof value === "object" && !Array.isArray(value)) { + const cleaned: Record = {}; + for (const [key, entry] of Object.entries(value as Record)) { + const trimmedKey = key.trim(); + const trimmedEntry = typeof entry === "string" ? entry.trim() : ""; + if (trimmedKey === "" || /\s/.test(trimmedKey) || trimmedEntry === "" || /\s/.test(trimmedEntry)) { + return { error: "autoReviewModelOverrides entries must be nonblank model ids without whitespace" }; + } + cleaned[trimmedKey] = trimmedEntry; + } + next.autoReviewModelOverrides = cleaned; + touched = true; + } else { + return { error: "autoReviewModelOverrides must be an object mapping model ids to approval model ids, or null to clear" }; + } + } if (Object.hasOwn(rawBody, "authMode")) { if (typeof rawBody.authMode !== "string") return { error: "authMode must be a string" }; const mode = rawBody.authMode.trim(); @@ -627,6 +660,14 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise/` in the catalog) or a `provider/model` catalog slug of any configured provider. The override is stamped onto the routed catalog row during sync as `auto_review_model_override`; unknown bare targets are skipped -with a warning rather than emitted (fail closed). The feature is opt-in — no vendor defaults. +with a warning rather than emitted, and a final sync pass drops overrides that do not name an +emitted catalog model (fail closed). Malformed hand-edited values are sanitized at load instead +of retiring the config; the strict management boundary still rejects them. + +Canonical OpenAI providers (whose native/account rows are not routed) and combo aliases are +excluded from the override; those sessions keep Codex's session-model behavior. The management +API preserves the fields on unrelated provider saves and exposes PATCH/null clearing, but the v1 +GUI does not yet render editors — configure through the config file or `ocx config set`. The +feature is opt-in — no vendor defaults. diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index 726a916be1..f8da2a458a 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -1,8 +1,11 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { resolveAutoReviewModel } from "../src/providers/derive"; -import { deriveEntry } from "../src/codex/catalog/sync"; +import { deriveEntry, validateAutoReviewOverridesAgainstCatalog } from "../src/codex/catalog/sync"; import { applyProviderConfigHints } from "../src/codex/catalog/provider-fetch"; import { providerManagementConfigError, safeConfigDTO } from "../src/server/auth-cors"; +import { loadConfig } from "../src/config"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; import type { CatalogModel } from "../src/codex/catalog/parsing"; import type { OcxConfig, OcxProviderConfig } from "../src/types"; @@ -30,23 +33,23 @@ describe("resolveAutoReviewModel", () => { describe("catalog stamping", () => { const template = { auto_review_model_override: null, context_window: 272000 } as Record; - test("bare override target becomes the provider/model slug", () => { + test("stamps a pre-normalized provider/model slug verbatim", () => { const model: CatalogModel = { id: "deepseek-v4-flash-vision-exp", provider: "deepseek", - autoReviewModelOverride: "deepseek-v4-flash", + autoReviewModelOverride: "deepseek/deepseek-v4-flash", }; const entry = deriveEntry(template, "deepseek/deepseek-v4-flash-vision-exp", "desc", 5, model); expect(entry.auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); }); - test("namespaced target is kept verbatim", () => { + test("stamps the override in the no-template fallback branch", () => { const model: CatalogModel = { id: "m", provider: "blsc", autoReviewModelOverride: "deepseek/deepseek-v4-flash", }; - const entry = deriveEntry(template, "blsc/m", "desc", 5, model); + const entry = deriveEntry(null, "blsc/m", "desc", 5, model); expect(entry.auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); }); @@ -69,7 +72,32 @@ describe("provider-fetch hints", () => { id: "deepseek-v4-flash-vision-exp", provider: "deepseek", }); - expect(hinted.autoReviewModelOverride).toBe("deepseek-v4-flash"); + expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); + }); + + test("encodes a slashed native id into the provider slug", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://zenmux.example/v1", + models: ["moonshotai/kimi-k3-free"], + autoReviewModel: "moonshotai/kimi-k3-free", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("zenmux", prov, { + id: "moonshotai/kimi-k3-free", + provider: "zenmux", + }); + expect(hinted.autoReviewModelOverride).toBe("zenmux/moonshotai-kimi-k3-free"); + }); + + test("keeps a namespaced cross-provider target verbatim", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://blsc.example/v1", + models: ["glm-5.2"], + autoReviewModel: "deepseek/deepseek-v4-flash", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("blsc", prov, { id: "glm-5.2", provider: "blsc" }); + expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); }); test("skips an unknown bare target without stamping", () => { @@ -85,6 +113,70 @@ describe("provider-fetch hints", () => { }); expect(hinted.autoReviewModelOverride).toBeUndefined(); }); + + test("clears a stale override when the config is removed", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://blsc.example/v1", + models: ["glm-5.2"], + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("blsc", prov, { + id: "glm-5.2", + provider: "blsc", + autoReviewModelOverride: "stale/deepseek-v4-pro", + }); + expect(hinted.autoReviewModelOverride).toBeUndefined(); + }); +}); + +describe("assembled-catalog validation", () => { + test("drops overrides that do not name an emitted model", () => { + const entries: Array> = [ + { slug: "blsc/glm-5.2", auto_review_model_override: "deepseek/deepseek-v4-flash" }, + { slug: "deepseek/deepseek-v4-flash" }, + ]; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); + entries[0].auto_review_model_override = "blsc/does-not-exist"; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBeUndefined(); + }); +}); + +describe("load sanitization", () => { + let testRoot = ""; + let previousHome: string | undefined; + + beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + testRoot = mkdtempSync(join(import.meta.dir, ".tmp-auto-review-load-")); + process.env.OPENCODEX_HOME = testRoot; + }); + + afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + rmSync(testRoot, { recursive: true, force: true }); + }); + + test("malformed auto-review fields are sanitized instead of retiring the config", () => { + writeFileSync(join(testRoot, "config.json"), JSON.stringify({ + port: 10100, + defaultProvider: "test", + providers: { + test: { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + apiKey: "sk-test", + autoReviewModel: " ", + autoReviewModelOverrides: { " bad key ": "deepseek-v4-flash", "glm-5.2": " " }, + }, + }, + })); + const loaded = loadConfig(); + expect(loaded.providers.test?.autoReviewModel).toBeUndefined(); + expect(loaded.providers.test?.autoReviewModelOverrides).toBeUndefined(); + }); }); describe("management validation and DTO", () => { @@ -100,6 +192,19 @@ describe("management validation and DTO", () => { expect(providerManagementConfigError("relay", { ...base, autoReviewModel: "deepseek-v4-flash" })).toBeNull(); }); + test("management redacts token-shaped provider names in auto-review errors", () => { + const tokenName = "sk-live-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const error = providerManagementConfigError(tokenName, { + adapter: "openai-chat", + baseUrl: "https://relay.example/v1", + apiKey: "sk-test", + authMode: "key", + autoReviewModel: " ", + }); + expect(error).toContain("autoReviewModel"); + expect(error).not.toContain(tokenName); + }); + test("safeConfigDTO keeps the override fields", () => { const config = { port: 10100, From ae909877cf8aa8c6311d2f701e6a9d91a691f44e Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Tue, 25 Aug 2026 12:34:07 +0800 Subject: [PATCH 03/13] refactor(catalog): move auto-review validation off the auth surface; harden stamping --- src/codex/catalog/provider-fetch.ts | 18 ++- src/codex/catalog/sync.ts | 7 +- src/config.ts | 43 +------ src/config/provider-validation.ts | 119 +++++++++++++++++++ src/server/auth-cors.ts | 31 ----- src/server/management/provider-routes.ts | 50 ++++---- structure/02_config-and-codex-home.md | 5 +- tests/auto-review-model-override.test.ts | 93 ++++++++------- tests/management-provider-validation.test.ts | 64 ++++++++++ 9 files changed, 292 insertions(+), 138 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index a6e089d966..d7bd3b0b03 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -679,17 +679,25 @@ function resolveAutoReviewOverrideForRow( ): string | undefined { const target = resolveAutoReviewModel(provider, modelId); if (target === null) return undefined; - const known = new Set([ + // Membership is case-folded to match the per-model lookup in + // resolveAutoReviewModel (exact -> :family -> case-fold). The canonical id from + // the provider's own list is what gets slug-encoded, so casing drift in a + // configured target cannot leak into catalog slugs. + const foldedKnown = new Map(); + for (const id of [ ...(provider?.models ?? []), ...(getProviderRegistryEntry(providerName)?.models ?? []), modelId, - ]); - if (target.includes("/") && !known.has(target)) { + ]) { + if (!foldedKnown.has(id.toLowerCase())) foldedKnown.set(id.toLowerCase(), id); + } + const canonical = foldedKnown.get(target.toLowerCase()); + if (target.includes("/") && canonical === undefined) { // Cross-provider catalog slug: kept verbatim; final existence is checked // against the assembled catalog at sync time. return target; } - if (known.size === 0 || !known.has(target)) { + if (canonical === undefined) { const key = providerName + "/" + target; if (!warnedAutoReviewTargets.has(key)) { warnedAutoReviewTargets.add(key); @@ -702,7 +710,7 @@ function resolveAutoReviewOverrideForRow( } return undefined; } - return routedSlug(providerName, encodeRoutedModelId(target)); + return routedSlug(providerName, encodeRoutedModelId(canonical)); } export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, model: CatalogModel, providerCap?: number): CatalogModel { diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 92a29f8f3f..58b4e79f1b 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -2020,8 +2020,9 @@ export function invalidateCodexModelsCache(options?: CodexCatalogSyncOptions): b /** * Final fail-closed pass over the assembled catalog: every stamped * auto_review_model_override must name a model that is actually emitted. - * Typo'd, disabled, or allowlisted-away targets are removed with one redacted - * warning per target instead of reaching Codex. + * Typo'd, disabled, or allowlisted-away targets are replaced with null (one + * redacted warning per target) so every routed row keeps the same field shape + * as template-cloned entries instead of reaching Codex. */ export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawEntry[]): void { const slugs = new Set(entries.map(entry => String(entry.slug))); @@ -2037,7 +2038,7 @@ export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawE + " does not match any catalog model; skipped.", ); } - delete entry.auto_review_model_override; + entry.auto_review_model_override = null; } } } diff --git a/src/config.ts b/src/config.ts index e5fad18b90..9914592e5b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -15,6 +15,7 @@ import { providerBaseUrlConfigError, providerHeadersConfigError, reasoningSummaryDeliveryRecordConfigError, + sanitizeAutoReviewOverridesForLoad, upstreamHttpVersionConfigError, } from "./config/provider-validation"; import { @@ -539,16 +540,21 @@ const providerConfigSchema = z.object({ export { isValidProviderName, hasOwnProvider } from "./config/provider-name"; export { + autoReviewModelConfigError, apiKeyTransportConfigError, booleanRecordConfigError, modelAdapterRecordConfigError, nonBlankStringArrayConfigError, + normalizeAutoReviewModelField, + normalizeAutoReviewModelFields, + normalizeAutoReviewModelOverridesField, normalizeNonBlankStringArray, positiveIntegerConfigError, positiveIntegerRecordConfigError, providerBaseUrlConfigError, providerHeadersConfigError, reasoningSummaryDeliveryRecordConfigError, + sanitizeAutoReviewOverridesForLoad, upstreamHttpVersionConfigError, } from "./config/provider-validation"; @@ -1466,43 +1472,6 @@ function sanitizeModelCostsForLoad(parsed: unknown): void { } } -/** - * Load-time sanitizer for the opt-in auto-review override fields. A malformed - * hand-edit must not retire the whole config: invalid values/keys are dropped - * (trimmed, whitespace-free) instead of failing schema validation. The strict - * management write boundary still rejects bad input before it reaches disk. - */ -function sanitizeAutoReviewOverridesForLoad(parsed: unknown): void { - if (!parsed || typeof parsed !== "object") return; - const root = parsed as Record; - const providers = root.providers; - if (!providers || typeof providers !== "object" || Array.isArray(providers)) return; - for (const provider of Object.values(providers as Record)) { - if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; - const row = provider as Record; - if (row.autoReviewModel !== undefined) { - const value = typeof row.autoReviewModel === "string" ? row.autoReviewModel.trim() : ""; - row.autoReviewModel = value !== "" && !/\s/.test(value) ? value : undefined; - } - if (row.autoReviewModelOverrides !== undefined) { - const overrides = row.autoReviewModelOverrides; - if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { - row.autoReviewModelOverrides = undefined; - continue; - } - const cleaned: Record = {}; - for (const [key, value] of Object.entries(overrides as Record)) { - const trimmedKey = key.trim(); - if (trimmedKey === "" || /\s/.test(trimmedKey)) continue; - const trimmedValue = typeof value === "string" ? value.trim() : ""; - if (trimmedValue === "" || /\s/.test(trimmedValue)) continue; - cleaned[trimmedKey] = trimmedValue; - } - row.autoReviewModelOverrides = Object.keys(cleaned).length > 0 ? cleaned : undefined; - } - } -} - /** * Companion to {@link warnDegradedStreamMode} for a blank persisted `hostname`. The bind * falls back to loopback, which is the safe direction but not what the file asked for — diff --git a/src/config/provider-validation.ts b/src/config/provider-validation.ts index 8a068d271b..98d0336dc3 100644 --- a/src/config/provider-validation.ts +++ b/src/config/provider-validation.ts @@ -111,6 +111,125 @@ export function normalizeNonBlankStringArray(value: readonly string[]): string[] return [...new Set(value.map(entry => entry.trim()))]; } +/** + * Validate the Codex auto-review model override shape at the management write + * boundary. Returns an error string, or null when the fields may be persisted. + */ +export function autoReviewModelConfigError(model: unknown, overrides: unknown): string | null { + if (model !== undefined) { + const trimmed = typeof model === "string" ? model.trim() : ""; + if (trimmed === "" || /\s/.test(trimmed)) { + return "autoReviewModel must be a nonblank model id without whitespace"; + } + } + if (overrides === undefined) return null; + if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { + return "autoReviewModelOverrides must be an object mapping model ids to approval model ids"; + } + for (const [key, value] of Object.entries(overrides as Record)) { + const trimmedKey = key.trim(); + if (trimmedKey === "" || /\s/.test(trimmedKey)) { + return "autoReviewModelOverrides keys must be nonblank model ids without whitespace"; + } + const trimmedValue = typeof value === "string" ? value.trim() : ""; + if (trimmedValue === "" || /\s/.test(trimmedValue)) { + return "autoReviewModelOverrides values must be nonblank model ids without whitespace"; + } + } + return null; +} + +/** Normalize one autoReviewModel field with PATCH-style null-to-clear semantics. */ +export function normalizeAutoReviewModelField(value: unknown): + | { value: string } + | { clear: true } + | { error: string } { + if (value === null) return { clear: true }; + if (typeof value !== "string" || value.trim() === "" || /\s/.test(value.trim())) { + return { error: "autoReviewModel must be a nonblank model id without whitespace, or null to clear" }; + } + return { value: value.trim() }; +} + +/** Normalize one autoReviewModelOverrides field with PATCH-style null-to-clear semantics. */ +export function normalizeAutoReviewModelOverridesField(value: unknown): + | { value: Record } + | { clear: true } + | { error: string } { + if (value === null) return { clear: true }; + if (!value || typeof value !== "object" || Array.isArray(value)) { + return { error: "autoReviewModelOverrides must be an object mapping model ids to approval model ids, or null to clear" }; + } + const cleaned: Record = {}; + for (const [key, entry] of Object.entries(value as Record)) { + const trimmedKey = key.trim(); + const trimmedEntry = typeof entry === "string" ? entry.trim() : ""; + if (trimmedKey === "" || /\s/.test(trimmedKey) || trimmedEntry === "" || /\s/.test(trimmedEntry)) { + return { error: "autoReviewModelOverrides entries must be nonblank model ids without whitespace" }; + } + cleaned[trimmedKey] = trimmedEntry; + } + return { value: cleaned }; +} + +/** + * Trim auto-review fields in place on a provider object that already passed + * boundary validation (POST path). Returns an error string only when the + * caller skipped validation; provider-routes always validates first. + */ +export function normalizeAutoReviewModelFields(provider: { + autoReviewModel?: unknown; + autoReviewModelOverrides?: unknown; +}): string | null { + const error = autoReviewModelConfigError(provider.autoReviewModel, provider.autoReviewModelOverrides); + if (error) return error; + if (typeof provider.autoReviewModel === "string") { + provider.autoReviewModel = provider.autoReviewModel.trim(); + } + if (provider.autoReviewModelOverrides !== undefined) { + const normalized = normalizeAutoReviewModelOverridesField(provider.autoReviewModelOverrides); + if ("error" in normalized) return normalized.error; + if ("value" in normalized) provider.autoReviewModelOverrides = normalized.value; + } + return null; +} + +/** + * Load-time sanitizer for hand-edited configs: malformed auto-review fields are + * trimmed and dropped instead of retiring the whole config. The strict + * management write boundary still rejects bad input before it reaches disk. + */ +export function sanitizeAutoReviewOverridesForLoad(parsed: unknown): void { + if (!parsed || typeof parsed !== "object") return; + const root = parsed as Record; + const providers = root.providers; + if (!providers || typeof providers !== "object" || Array.isArray(providers)) return; + for (const provider of Object.values(providers as Record)) { + if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; + const row = provider as Record; + if (row.autoReviewModel !== undefined) { + const value = typeof row.autoReviewModel === "string" ? row.autoReviewModel.trim() : ""; + row.autoReviewModel = value !== "" && !/\s/.test(value) ? value : undefined; + } + if (row.autoReviewModelOverrides !== undefined) { + const overrides = row.autoReviewModelOverrides; + if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { + row.autoReviewModelOverrides = undefined; + continue; + } + const cleaned: Record = {}; + for (const [key, value] of Object.entries(overrides as Record)) { + const trimmedKey = key.trim(); + if (trimmedKey === "" || /\s/.test(trimmedKey)) continue; + const trimmedValue = typeof value === "string" ? value.trim() : ""; + if (trimmedValue === "" || /\s/.test(trimmedValue)) continue; + cleaned[trimmedKey] = trimmedValue; + } + row.autoReviewModelOverrides = Object.keys(cleaned).length > 0 ? cleaned : undefined; + } + } +} + export function booleanRecordConfigError(value: unknown, field: string): string | null { if (value === undefined) return null; if (!value || typeof value !== "object" || Array.isArray(value)) return `${field} must be a plain object`; diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 4321b886e4..0d62f232c9 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -528,31 +528,6 @@ function nativeContextOverlayError(raw: Record): string | null return null; } -/** Validate the Codex auto-review model override shape at the management write boundary. */ -function autoReviewModelConfigError(model: unknown, overrides: unknown): string | null { - if (model !== undefined) { - const trimmed = typeof model === "string" ? model.trim() : ""; - if (trimmed === "" || /\s/.test(trimmed)) { - return "autoReviewModel must be a nonblank model id without whitespace"; - } - } - if (overrides === undefined) return null; - if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { - return "autoReviewModelOverrides must be an object mapping model ids to approval model ids"; - } - for (const [key, value] of Object.entries(overrides as Record)) { - const trimmedKey = key.trim(); - if (trimmedKey === "" || /\s/.test(trimmedKey)) { - return "autoReviewModelOverrides keys must be nonblank model ids without whitespace"; - } - const trimmedValue = typeof value === "string" ? value.trim() : ""; - if (trimmedValue === "" || /\s/.test(trimmedValue)) { - return "autoReviewModelOverrides values must be nonblank model ids without whitespace"; - } - } - return null; -} - /** * Validate a provider object arriving at the management write boundary. Returns an error * string, or null when the provider may be persisted. Caller-controlled names/fields are @@ -661,10 +636,6 @@ export function providerManagementConfigError(name: unknown, provider: unknown): if (raw.responsesSnapshotRepair !== undefined && typeof raw.responsesSnapshotRepair !== "boolean") { return `provider ${name} responsesSnapshotRepair must be a boolean`; } - const autoReviewError = autoReviewModelConfigError(raw.autoReviewModel, raw.autoReviewModelOverrides); - if (autoReviewError) { - return "provider " + JSON.stringify(redactSecretString(name)) + " " + autoReviewError; - } const defaultMaxOutputError = positiveIntegerConfigError(raw.defaultMaxOutputTokens, "defaultMaxOutputTokens"); if (defaultMaxOutputError) return `provider ${name} ${defaultMaxOutputError}`; const maxOutputError = positiveIntegerRecordConfigError(raw.modelMaxOutputTokens, "modelMaxOutputTokens"); @@ -767,8 +738,6 @@ export function safeConfigDTO(config: OcxConfig): unknown { "autoToolChoiceOnlyModels", "preserveReasoningContentModels", "requiresReasoningPlaceholderModels", - "autoReviewModel", - "autoReviewModelOverrides", "escapeBuiltinToolNames", ] as const) { copyIfDefined(dto, provider, key); diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 585c8732b8..965f5d51b0 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -6,11 +6,15 @@ import { clearGatherRoutedModelsInflight } from "../../codex/catalog/provider-fe import { DEFAULT_SUBAGENT_MODELS, adoptPersistedProviderIntoLiveConfig, + autoReviewModelConfigError, codexAutoStartEnabled, hasOwnProvider, isValidProviderName, multiAgentGuidanceEnabled, nonBlankStringArrayConfigError, + normalizeAutoReviewModelField, + normalizeAutoReviewModelFields, + normalizeAutoReviewModelOverridesField, normalizeNonBlankStringArray, providerBaseUrlConfigError, providerHeadersConfigError, @@ -30,6 +34,7 @@ import { upsertOAuthProvider, } from "../../oauth"; import { replaceProviderAccountSet } from "../../oauth/store"; +import { redactSecretString } from "../../lib/redact"; import { providerDestinationResolvedError } from "../../lib/destination-policy"; import { reconcileLiveStateStores } from "../../lib/state-store-registrations"; import { ProviderOutboundPolicyError, providerOutboundGet, providerOutboundPost, providerRedirectError } from "../../lib/provider-outbound"; @@ -103,6 +108,15 @@ type ProviderPatchApplication = headersTouched: boolean; }; +/** Redacted auto-review boundary error for provider writes. */ +function providerAutoReviewConfigError(name: string, provider: unknown): string | null { + if (!provider || typeof provider !== "object") return null; + const raw = provider as Record; + const error = autoReviewModelConfigError(raw.autoReviewModel, raw.autoReviewModelOverrides); + if (!error) return null; + return "provider " + JSON.stringify(redactSecretString(name)) + " " + error; +} + /** * Apply the recognized PATCH field mask onto a provider copy. The caller runs this once * for validation and again inside the config mutation lock against the newest provider, @@ -146,36 +160,25 @@ function applyProviderPatchFields( touched = true; } if (Object.hasOwn(rawBody, "autoReviewModel")) { - const value = rawBody.autoReviewModel; - if (value === null) { + const normalized = normalizeAutoReviewModelField(rawBody.autoReviewModel); + if ("error" in normalized) return { error: normalized.error }; + if ("clear" in normalized) { delete next.autoReviewModel; touched = true; - } else if (typeof value === "string" && value.trim() !== "" && !/\s/.test(value.trim())) { - next.autoReviewModel = value.trim(); - touched = true; } else { - return { error: "autoReviewModel must be a nonblank model id without whitespace, or null to clear" }; + next.autoReviewModel = normalized.value; + touched = true; } } if (Object.hasOwn(rawBody, "autoReviewModelOverrides")) { - const value = rawBody.autoReviewModelOverrides; - if (value === null) { + const normalized = normalizeAutoReviewModelOverridesField(rawBody.autoReviewModelOverrides); + if ("error" in normalized) return { error: normalized.error }; + if ("clear" in normalized) { delete next.autoReviewModelOverrides; touched = true; - } else if (value && typeof value === "object" && !Array.isArray(value)) { - const cleaned: Record = {}; - for (const [key, entry] of Object.entries(value as Record)) { - const trimmedKey = key.trim(); - const trimmedEntry = typeof entry === "string" ? entry.trim() : ""; - if (trimmedKey === "" || /\s/.test(trimmedKey) || trimmedEntry === "" || /\s/.test(trimmedEntry)) { - return { error: "autoReviewModelOverrides entries must be nonblank model ids without whitespace" }; - } - cleaned[trimmedKey] = trimmedEntry; - } - next.autoReviewModelOverrides = cleaned; - touched = true; } else { - return { error: "autoReviewModelOverrides must be an object mapping model ids to approval model ids, or null to clear" }; + next.autoReviewModelOverrides = normalized.value; + touched = true; } } if (Object.hasOwn(rawBody, "authMode")) { @@ -578,6 +581,8 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise/` in the catalog) or a `provider/model` catalog slug of any configured provider. The override is stamped onto the routed catalog row during sync as `auto_review_model_override`; unknown bare targets are skipped -with a warning rather than emitted, and a final sync pass drops overrides that do not name an +with a warning rather than emitted. A bare target resolves only when the id is also listed in +the provider's configured `models` or a matching registry entry; on pure live-discovery +providers without a static `models` list, add the sibling target id to `models` first (the +row's own model id always resolves). A final sync pass drops overrides that do not name an emitted catalog model (fail closed). Malformed hand-edited values are sanitized at load instead of retiring the config; the strict management boundary still rejects them. diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index f8da2a458a..87b5f10edf 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -2,12 +2,12 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { resolveAutoReviewModel } from "../src/providers/derive"; import { deriveEntry, validateAutoReviewOverridesAgainstCatalog } from "../src/codex/catalog/sync"; import { applyProviderConfigHints } from "../src/codex/catalog/provider-fetch"; -import { providerManagementConfigError, safeConfigDTO } from "../src/server/auth-cors"; +import { autoReviewModelConfigError, normalizeAutoReviewModelFields } from "../src/config/provider-validation"; import { loadConfig } from "../src/config"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import type { CatalogModel } from "../src/codex/catalog/parsing"; -import type { OcxConfig, OcxProviderConfig } from "../src/types"; +import type { OcxProviderConfig } from "../src/types"; describe("resolveAutoReviewModel", () => { test("per-model override wins over provider-wide", () => { @@ -24,6 +24,24 @@ describe("resolveAutoReviewModel", () => { expect(resolveAutoReviewModel(provider, "anything")).toBe("deepseek-v4-flash"); }); + test("per-model lookup falls back to the :family prefix", () => { + const provider = { + autoReviewModelOverrides: { + "deepseek-v4-flash-vision-exp": "deepseek-v4-flash", + }, + } as OcxProviderConfig; + expect(resolveAutoReviewModel(provider, "deepseek-v4-flash-vision-exp:beta")).toBe("deepseek-v4-flash"); + }); + + test("per-model lookup case-folds override keys", () => { + const provider = { + autoReviewModelOverrides: { + "DEEPSEEK-V4-FLASH": "deepseek-v4-pro", + }, + } as OcxProviderConfig; + expect(resolveAutoReviewModel(provider, "deepseek-v4-flash")).toBe("deepseek-v4-pro"); + }); + test("returns null when nothing is configured", () => { expect(resolveAutoReviewModel({} as OcxProviderConfig, "m")).toBeNull(); expect(resolveAutoReviewModel(undefined, "m")).toBeNull(); @@ -75,6 +93,20 @@ describe("provider-fetch hints", () => { expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); }); + test("case-differing target matches and encodes the canonical provider id", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + models: ["deepseek-v4-flash"], + autoReviewModel: "DeepSeek-V4-Flash", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("deepseek", prov, { + id: "deepseek-v4-flash-vision-exp", + provider: "deepseek", + }); + expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); + }); + test("encodes a slashed native id into the provider slug", () => { const prov = { adapter: "openai-chat", @@ -139,7 +171,7 @@ describe("assembled-catalog validation", () => { expect(entries[0].auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); entries[0].auto_review_model_override = "blsc/does-not-exist"; validateAutoReviewOverridesAgainstCatalog(entries); - expect(entries[0].auto_review_model_override).toBeUndefined(); + expect(entries[0].auto_review_model_override).toBeNull(); }); }); @@ -179,48 +211,29 @@ describe("load sanitization", () => { }); }); -describe("management validation and DTO", () => { +describe("management validation and normalization", () => { test("management rejects malformed overrides", () => { - const base = { - adapter: "openai-chat", - baseUrl: "https://relay.example/v1", - apiKey: "sk-test", - authMode: "key", - }; - expect(providerManagementConfigError("relay", { ...base, autoReviewModel: " " })).toContain("autoReviewModel"); - expect(providerManagementConfigError("relay", { ...base, autoReviewModelOverrides: { m: 42 } })).toContain("autoReviewModelOverrides"); - expect(providerManagementConfigError("relay", { ...base, autoReviewModel: "deepseek-v4-flash" })).toBeNull(); + expect(autoReviewModelConfigError(" ", undefined)).toContain("autoReviewModel"); + expect(autoReviewModelConfigError(undefined, { m: 42 })).toContain("autoReviewModelOverrides"); + expect(autoReviewModelConfigError("deepseek-v4-flash", { "deepseek-v4-flash-vision-exp": "deepseek-v4-pro" })).toBeNull(); }); - test("management redacts token-shaped provider names in auto-review errors", () => { - const tokenName = "sk-live-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - const error = providerManagementConfigError(tokenName, { - adapter: "openai-chat", - baseUrl: "https://relay.example/v1", - apiKey: "sk-test", - authMode: "key", - autoReviewModel: " ", + test("POST normalization trims auto-review fields before persistence", () => { + const provider = { + autoReviewModel: " deepseek-v4-flash ", + autoReviewModelOverrides: { + " deepseek-v4-flash-vision-exp ": " deepseek-v4-pro ", + }, + }; + expect(normalizeAutoReviewModelFields(provider)).toBeNull(); + expect(provider.autoReviewModel).toBe("deepseek-v4-flash"); + expect(provider.autoReviewModelOverrides).toEqual({ + "deepseek-v4-flash-vision-exp": "deepseek-v4-pro", }); - expect(error).toContain("autoReviewModel"); - expect(error).not.toContain(tokenName); }); - test("safeConfigDTO keeps the override fields", () => { - const config = { - port: 10100, - defaultProvider: "deepseek", - providers: { - deepseek: { - adapter: "openai-chat", - baseUrl: "https://api.deepseek.com", - apiKey: "sk-x", - autoReviewModel: "deepseek-v4-flash", - autoReviewModelOverrides: { "deepseek-v4-flash-vision-exp": "deepseek-v4-pro" }, - }, - }, - } as unknown as OcxConfig; - const dto = safeConfigDTO(config) as { providers: Record }> }; - expect(dto.providers.deepseek?.autoReviewModel).toBe("deepseek-v4-flash"); - expect(dto.providers.deepseek?.autoReviewModelOverrides?.["deepseek-v4-flash-vision-exp"]).toBe("deepseek-v4-pro"); + test("POST normalization rejects whitespace inside ids", () => { + const provider = { autoReviewModel: "deep seek-v4-flash" }; + expect(normalizeAutoReviewModelFields(provider)).toContain("autoReviewModel"); }); }); diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index c839c5b6fa..22d12c009b 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -695,6 +695,70 @@ describe("provider management validation", () => { } }); + test("provider POST normalizes auto-review fields before persisting config.json", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const create = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "auto-review", + provider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + autoReviewModel: " deepseek-v4-flash ", + autoReviewModelOverrides: { + " deepseek-v4-flash-vision-exp ": " deepseek-v4-pro ", + }, + }, + }), + }); + expect(create.status).toBe(200); + const persisted = loadConfig().providers["auto-review"]; + expect(persisted?.autoReviewModel).toBe("deepseek-v4-flash"); + expect(persisted?.autoReviewModelOverrides).toEqual({ + "deepseek-v4-flash-vision-exp": "deepseek-v4-pro", + }); + } finally { + await server.stop(true); + } + }); + + test("provider POST redacts token-shaped names in auto-review errors", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const tokenName = "sk-live-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const response = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: tokenName, + provider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + autoReviewModel: " ", + }, + }), + }); + expect(response.status).toBe(400); + const body = (await response.json()) as { error: string }; + expect(body.error).toContain("autoReviewModel"); + expect(body.error).not.toContain(tokenName); + } finally { + await server.stop(true); + } + }); + // #1409: the add/edit form's payload type has no member for contextWindow or // modelContextWindows, so an overwrite arrives without them. Registry enrichment then fills // the absent fields from the seed and the stored row loses the user's values — for From b9af74aa08e80c5f7719648d5f587193f4952cd3 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Tue, 25 Aug 2026 15:09:13 +0800 Subject: [PATCH 04/13] fix(catalog): keep auto-review overrides on trusted openai-api rows; expose fields in GET; reject on canonical openai --- src/codex/catalog/provider-fetch.ts | 2 + src/server/management/provider-routes.ts | 12 ++++- tests/codex-catalog.test.ts | 9 ++++ tests/management-provider-validation.test.ts | 50 ++++++++++++++++++++ 4 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index d7bd3b0b03..3b75bb24b9 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -2214,6 +2214,7 @@ function augmentRoutedModelsWithCapturedOpenAiApiRows( const autoCompactTokenLimit = contextWindow !== undefined && configuredAutoCompact !== undefined ? clampAutoCompactTokenLimit(contextWindow, maxInputTokens, configuredAutoCompact) : undefined; + const autoReviewOverride = resolveAutoReviewOverrideForRow(OPENAI_API_PROVIDER_ID, configured, id); return { provider: OPENAI_API_PROVIDER_ID, id, @@ -2221,6 +2222,7 @@ function augmentRoutedModelsWithCapturedOpenAiApiRows( ...(contextWindow ? { contextWindow } : {}), ...(maxInputTokens ? { maxInputTokens } : {}), ...(autoCompactTokenLimit !== undefined ? { autoCompactTokenLimit } : {}), + ...(autoReviewOverride !== undefined ? { autoReviewModelOverride: autoReviewOverride } : {}), ...(policy.modelInputModalities?.[id] ? { inputModalities: [...policy.modelInputModalities[id]!] } : {}), ...(policy.modelReasoningEfforts?.[id] ? { reasoningEfforts: [...policy.modelReasoningEfforts[id]!] } : {}), }; diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 965f5d51b0..31b6e558f1 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -112,6 +112,9 @@ type ProviderPatchApplication = function providerAutoReviewConfigError(name: string, provider: unknown): string | null { if (!provider || typeof provider !== "object") return null; const raw = provider as Record; + if (name === "openai" && (Object.hasOwn(raw, "autoReviewModel") || Object.hasOwn(raw, "autoReviewModelOverrides"))) { + return "provider openai must not include autoReviewModel or autoReviewModelOverrides"; + } const error = autoReviewModelConfigError(raw.autoReviewModel, raw.autoReviewModelOverrides); if (!error) return null; return "provider " + JSON.stringify(redactSecretString(name)) + " " + error; @@ -159,6 +162,9 @@ function applyProviderPatchFields( else delete next.defaultModel; touched = true; } + if (name === "openai" && (Object.hasOwn(rawBody, "autoReviewModel") || Object.hasOwn(rawBody, "autoReviewModelOverrides"))) { + return { error: "provider openai must not include autoReviewModel or autoReviewModelOverrides" }; + } if (Object.hasOwn(rawBody, "autoReviewModel")) { const normalized = normalizeAutoReviewModelField(rawBody.autoReviewModel); if ("error" in normalized) return { error: normalized.error }; @@ -488,6 +494,8 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise { .toMatchObject({ contextWindow: 1_050_000, maxInputTokens: 922_000 }); }); + test("trusted openai-api rebuild keeps the configured auto-review override", () => { + const apiRows = augmentRoutedModelsWithRegistryOpenAiApiRows([], openAiApiCatalogConfig({ + models: ["daybreak-blue-latest"], + autoReviewModel: "daybreak-blue-latest", + })); + const row = apiRows.find(candidate => candidate.provider === "openai-apikey" && candidate.id === "daybreak-blue-latest"); + expect(row?.autoReviewModelOverride).toBe("openai-apikey/daybreak-blue-latest"); + }); + test("Daybreak metadata inheritance rejects noncanonical providers", async () => { const models = await gatherRoutedModels({ port: 10100, diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index 22d12c009b..9d58d1d6f4 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -724,6 +724,16 @@ describe("provider management validation", () => { expect(persisted?.autoReviewModelOverrides).toEqual({ "deepseek-v4-flash-vision-exp": "deepseek-v4-pro", }); + const listed = await fetch(new URL("/api/providers", server.url)).then(response => response.json()) as Array<{ + name: string; + autoReviewModel?: string; + autoReviewModelOverrides?: Record; + }>; + const row = listed.find(provider => provider.name === "auto-review"); + expect(row?.autoReviewModel).toBe("deepseek-v4-flash"); + expect(row?.autoReviewModelOverrides).toEqual({ + "deepseek-v4-flash-vision-exp": "deepseek-v4-pro", + }); } finally { await server.stop(true); } @@ -759,6 +769,46 @@ describe("provider management validation", () => { } }); + test("canonical openai rejects auto-review fields without persisting", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const before = JSON.stringify(loadConfig().providers.openai); + const post = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "openai", + provider: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "pool", + autoReviewModel: "deepseek-v4-flash", + autoReviewModelOverrides: { "deepseek-v4-flash-vision-exp": "deepseek-v4-pro" }, + }, + }), + }); + expect(post.status).toBe(400); + expect(await post.json()).toMatchObject({ error: expect.stringContaining("autoReviewModel") }); + + const patch = await fetch(new URL("/api/providers?name=openai", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ autoReviewModel: "deepseek-v4-flash" }), + }); + expect(patch.status).toBe(400); + expect(await patch.json()).toMatchObject({ error: expect.stringContaining("autoReviewModel") }); + expect(JSON.stringify(loadConfig().providers.openai)).toBe(before); + } finally { + await server.stop(true); + } + }); + // #1409: the add/edit form's payload type has no member for contextWindow or // modelContextWindows, so an overwrite arrives without them. Registry enrichment then fills // the absent fields from the seed and the stored row loses the user's values — for From 504d8fe535c446313900046803536e8fdc293f90 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Tue, 25 Aug 2026 15:17:47 +0800 Subject: [PATCH 05/13] test(management): seed canonical openai for the auto-review PATCH rejection case --- tests/management-provider-validation.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index 9d58d1d6f4..aa60cd31f6 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -773,7 +773,10 @@ describe("provider management validation", () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_DIR, { recursive: true }); process.env.OPENCODEX_HOME = TEST_DIR; - saveConfig(config("127.0.0.1")); + saveConfig({ + ...config("127.0.0.1"), + providers: poolProviders(), + }); const server = startServer(0); try { From 2bc8f1b5e8b074ee11dd37561b4dae2177b08337 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Wed, 26 Aug 2026 10:20:52 +0800 Subject: [PATCH 06/13] fix(catalog): harden auto-review slug matching, family case-fold, and GET redaction --- src/codex/catalog/sync.ts | 6 ++- src/providers/derive.ts | 7 +++- src/server/management/provider-routes.ts | 9 ++++- tests/auto-review-model-override.test.ts | 22 +++++++++++ tests/management-provider-validation.test.ts | 40 ++++++++++++++++++++ 5 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 58b4e79f1b..9334e66138 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -2025,7 +2025,11 @@ export function invalidateCodexModelsCache(options?: CodexCatalogSyncOptions): b * as template-cloned entries instead of reaching Codex. */ export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawEntry[]): void { - const slugs = new Set(entries.map(entry => String(entry.slug))); + // Only nonblank string slugs are emitted catalog selectors; a missing or + // malformed slug must never become a matchable "undefined"/"null" string. + const slugs = new Set(entries.flatMap(entry => + typeof entry.slug === "string" && entry.slug.trim() !== "" ? [entry.slug] : [], + )); const warned = new Set(); for (const entry of entries) { const override = entry.auto_review_model_override; diff --git a/src/providers/derive.ts b/src/providers/derive.ts index a9afe2cab3..a434782bc4 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -639,8 +639,11 @@ function autoReviewOverrideForModel( if (!overrides) return undefined; if (Object.prototype.hasOwnProperty.call(overrides, modelId)) return overrides[modelId]; const colon = modelId.indexOf(":"); - if (colon > 0 && Object.prototype.hasOwnProperty.call(overrides, modelId.slice(0, colon))) { - return overrides[modelId.slice(0, colon)]; + if (colon > 0) { + const family = modelId.slice(0, colon); + if (Object.prototype.hasOwnProperty.call(overrides, family)) return overrides[family]; + const familyMatch = Object.entries(overrides).find(([key]) => key.toLowerCase() === family.toLowerCase()); + if (familyMatch) return familyMatch[1]; } const folded = modelId.toLowerCase(); for (const [key, value] of Object.entries(overrides)) { diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 31b6e558f1..c0fc14cba6 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -494,8 +494,13 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise [ + redactSecretString(key), + redactSecretString(value), + ])), modelSupportsServiceTier: p.modelSupportsServiceTier, noStructuredOutputModels: p.noStructuredOutputModels, upstreamHttpVersion: p.upstreamHttpVersion, diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index 87b5f10edf..5a5144ba10 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -42,6 +42,15 @@ describe("resolveAutoReviewModel", () => { expect(resolveAutoReviewModel(provider, "deepseek-v4-flash")).toBe("deepseek-v4-pro"); }); + test("per-model lookup case-folds the :family prefix", () => { + const provider = { + autoReviewModelOverrides: { + "DEEPSEEK-V4-FLASH": "deepseek-v4-pro", + }, + } as OcxProviderConfig; + expect(resolveAutoReviewModel(provider, "deepseek-v4-flash:beta")).toBe("deepseek-v4-pro"); + }); + test("returns null when nothing is configured", () => { expect(resolveAutoReviewModel({} as OcxProviderConfig, "m")).toBeNull(); expect(resolveAutoReviewModel(undefined, "m")).toBeNull(); @@ -173,6 +182,19 @@ describe("assembled-catalog validation", () => { validateAutoReviewOverridesAgainstCatalog(entries); expect(entries[0].auto_review_model_override).toBeNull(); }); + + test("missing or malformed slugs never become matchable override targets", () => { + const entries: Array> = [ + { slug: undefined, auto_review_model_override: "undefined" }, + { slug: null, auto_review_model_override: "null" }, + { slug: "", auto_review_model_override: "blsc/m" }, + { slug: "other/m" }, + ]; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[1].auto_review_model_override).toBeNull(); + expect(entries[2].auto_review_model_override).toBeNull(); + }); }); describe("load sanitization", () => { diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index aa60cd31f6..6ed264da7d 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -739,6 +739,46 @@ describe("provider management validation", () => { } }); + test("provider GET redacts credential-shaped auto-review values", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const tokenModel = "sk-live-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const tokenOverride = "sk-live-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + const create = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "auto-review", + provider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + autoReviewModel: tokenModel, + autoReviewModelOverrides: { "deepseek-v4-flash": tokenOverride }, + }, + }), + }); + expect(create.status).toBe(200); + + const listed = await fetch(new URL("/api/providers", server.url)).then(response => response.json()) as Array<{ + name: string; + autoReviewModel?: string; + autoReviewModelOverrides?: Record; + }>; + const row = listed.find(provider => provider.name === "auto-review"); + expect(row?.autoReviewModel).not.toContain(tokenModel); + expect(row?.autoReviewModel).toContain("[REDACTED]"); + expect(row?.autoReviewModelOverrides?.["deepseek-v4-flash"]).not.toContain(tokenOverride); + expect(row?.autoReviewModelOverrides?.["deepseek-v4-flash"]).toContain("[REDACTED]"); + } finally { + await server.stop(true); + } + }); + test("provider POST redacts token-shaped names in auto-review errors", async () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_DIR, { recursive: true }); From e665bd2ec585a69909aeb199ebac4ea76fb65a23 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Wed, 26 Aug 2026 10:43:30 +0800 Subject: [PATCH 07/13] fix(providers): prefer folded full-model overrides over case-folded family defaults --- src/providers/derive.ts | 14 ++++++++------ tests/auto-review-model-override.test.ts | 10 ++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/providers/derive.ts b/src/providers/derive.ts index a434782bc4..9e9f713364 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -631,7 +631,7 @@ export function resolveAutoReviewModel( return null; } -/** Per-model override lookup mirroring modelRecordValue: exact, then :family, then case-fold. */ +/** Per-model override lookup: exact model, exact :family, folded model, folded :family. */ function autoReviewOverrideForModel( overrides: Record | undefined, modelId: string, @@ -639,16 +639,18 @@ function autoReviewOverrideForModel( if (!overrides) return undefined; if (Object.prototype.hasOwnProperty.call(overrides, modelId)) return overrides[modelId]; const colon = modelId.indexOf(":"); + const folded = modelId.toLowerCase(); + for (const [key, value] of Object.entries(overrides)) { + if (key.toLowerCase() === folded) return value; + } + // The case-insensitive family lookup must run AFTER the folded full-model + // loop: a model-specific override for a colon-suffixed id wins over the + // family default even when both keys differ only in casing. if (colon > 0) { const family = modelId.slice(0, colon); - if (Object.prototype.hasOwnProperty.call(overrides, family)) return overrides[family]; const familyMatch = Object.entries(overrides).find(([key]) => key.toLowerCase() === family.toLowerCase()); if (familyMatch) return familyMatch[1]; } - const folded = modelId.toLowerCase(); - for (const [key, value] of Object.entries(overrides)) { - if (key.toLowerCase() === folded) return value; - } return undefined; } diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index 5a5144ba10..ff4ff1b787 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -51,6 +51,16 @@ describe("resolveAutoReviewModel", () => { expect(resolveAutoReviewModel(provider, "deepseek-v4-flash:beta")).toBe("deepseek-v4-pro"); }); + test("a case-differing full-model override wins over the case-folded family", () => { + const provider = { + autoReviewModelOverrides: { + "DEEPSEEK-V4-FLASH:beta": "deepseek-v4-flash-specific", + "DEEPSEEK-V4-FLASH": "deepseek-v4-pro", + }, + } as OcxProviderConfig; + expect(resolveAutoReviewModel(provider, "deepseek-v4-flash:beta")).toBe("deepseek-v4-flash-specific"); + }); + test("returns null when nothing is configured", () => { expect(resolveAutoReviewModel({} as OcxProviderConfig, "m")).toBeNull(); expect(resolveAutoReviewModel(undefined, "m")).toBeNull(); From 540a8ac94bfc4c6170d54302f64f2478c7f36969 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Wed, 26 Aug 2026 17:41:47 +0800 Subject: [PATCH 08/13] fix(catalog): preserve native upstream auto-review overrides in final-catalog validation --- src/codex/catalog/sync.ts | 5 +++++ tests/auto-review-model-override.test.ts | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 9334e66138..26c6f2af73 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -2032,6 +2032,11 @@ export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawE )); const warned = new Set(); for (const entry of entries) { + // Native rows with a valid slug carry upstream-retained values (for example + // a Codex-side "native-upstream" selector) that opencodex must preserve + // verbatim; this fail-closed pass exists for the routed rows opencodex + // stamps and for malformed catalog rows with missing/invalid slugs. + if (!isRoutedCatalogEntry(entry) && typeof entry.slug === "string" && entry.slug.trim() !== "") continue; const override = entry.auto_review_model_override; if (typeof override !== "string") continue; if (!slugs.has(override)) { diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index ff4ff1b787..2cea4d26dd 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -193,6 +193,16 @@ describe("assembled-catalog validation", () => { expect(entries[0].auto_review_model_override).toBeNull(); }); + test("native rows keep upstream-retained overrides even when the target is not emitted", () => { + const entries: Array> = [ + { slug: "gpt-5.4", auto_review_model_override: "native-upstream" }, + { slug: "static/deepseek-v4-flash", auto_review_model_override: "static/deepseek-v4-flash" }, + ]; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBe("native-upstream"); + expect(entries[1].auto_review_model_override).toBe("static/deepseek-v4-flash"); + }); + test("missing or malformed slugs never become matchable override targets", () => { const entries: Array> = [ { slug: undefined, auto_review_model_override: "undefined" }, From bc4c025129ef4921c8f264dee2afbe61fdc88e8b Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Wed, 26 Aug 2026 18:34:33 +0800 Subject: [PATCH 09/13] fix(catalog): normalize wrong-shaped auto-review overrides to null before serialization --- src/codex/catalog/sync.ts | 8 ++++++++ tests/auto-review-model-override.test.ts | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 26c6f2af73..34f4745f67 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -2038,6 +2038,14 @@ export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawE // stamps and for malformed catalog rows with missing/invalid slugs. if (!isRoutedCatalogEntry(entry) && typeof entry.slug === "string" && entry.slug.trim() !== "") continue; const override = entry.auto_review_model_override; + // Wrong-shaped values must never persist: JSON.stringify would otherwise + // keep a numeric or object override unchanged. null/undefined already + // serialize as the canonical empty shape, so only normalize everything + // else and keep the existing string-slug handling below. + if (override !== undefined && override !== null && typeof override !== "string") { + entry.auto_review_model_override = null; + continue; + } if (typeof override !== "string") continue; if (!slugs.has(override)) { if (!warned.has(override)) { diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index 2cea4d26dd..c922c0a129 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -215,6 +215,21 @@ describe("assembled-catalog validation", () => { expect(entries[1].auto_review_model_override).toBeNull(); expect(entries[2].auto_review_model_override).toBeNull(); }); + + test("normalizes wrong-shaped routed overrides to null before serialization", () => { + const entries: Array> = [ + { slug: "blsc/glm-5.2", auto_review_model_override: 42 }, + { slug: "deepseek/deepseek-v4-flash", auto_review_model_override: { nested: true } }, + { slug: "static/null", auto_review_model_override: null }, + { slug: "static/undefined", auto_review_model_override: undefined }, + { slug: "deepseek/deepseek-v4-pro" }, + ]; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[1].auto_review_model_override).toBeNull(); + expect(entries[2].auto_review_model_override).toBeNull(); + expect(entries[3].auto_review_model_override).toBeUndefined(); + }); }); describe("load sanitization", () => { From 8c51897883b55c533b01d898a128b6a3016724d1 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 27 Aug 2026 17:35:41 +0800 Subject: [PATCH 10/13] fix(catalog): normalize native auto-review overrides before preservation --- src/codex/catalog/sync.ts | 36 ++++++++++++++++-------- tests/auto-review-model-override.test.ts | 16 +++++++++++ 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 34f4745f67..355af8bc7f 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -2032,26 +2032,38 @@ export function validateAutoReviewOverridesAgainstCatalog(entries: readonly RawE )); const warned = new Set(); for (const entry of entries) { - // Native rows with a valid slug carry upstream-retained values (for example - // a Codex-side "native-upstream" selector) that opencodex must preserve - // verbatim; this fail-closed pass exists for the routed rows opencodex - // stamps and for malformed catalog rows with missing/invalid slugs. - if (!isRoutedCatalogEntry(entry) && typeof entry.slug === "string" && entry.slug.trim() !== "") continue; const override = entry.auto_review_model_override; // Wrong-shaped values must never persist: JSON.stringify would otherwise // keep a numeric or object override unchanged. null/undefined already - // serialize as the canonical empty shape, so only normalize everything - // else and keep the existing string-slug handling below. + // serialize as the canonical empty shape, so normalize everything else + // before the native-row preservation branch below (native rows must not + // bypass this fail-closed normalization). if (override !== undefined && override !== null && typeof override !== "string") { entry.auto_review_model_override = null; + } + const current = entry.auto_review_model_override; + // Native rows with a valid slug carry upstream-retained values (for example + // a Codex-side "native-upstream" selector) that opencodex must preserve + // verbatim. Preserve only a valid nonblank native string; blank strings and + // residual wrong shapes are normalized to the canonical empty shape instead + // of reaching Codex, without requiring the value to resolve to an emitted + // routed slug. Routed rows and malformed catalog rows with missing/invalid + // slugs still fall through to the fail-closed pass below. + if (!isRoutedCatalogEntry(entry) && typeof entry.slug === "string" && entry.slug.trim() !== "") { + if (typeof current === "string" && current.trim() !== "") continue; + // undefined and null are both canonical empty shapes (undefined is + // omitted by JSON.stringify, null is explicit); only residual blank + // strings and wrong-shaped values are normalized so native rows cannot + // bypass the fail-closed pass. + if (current !== undefined && current !== null) entry.auto_review_model_override = null; continue; } - if (typeof override !== "string") continue; - if (!slugs.has(override)) { - if (!warned.has(override)) { - warned.add(override); + if (typeof current !== "string") continue; + if (!slugs.has(current)) { + if (!warned.has(current)) { + warned.add(current); console.warn( - "[opencodex] autoReviewModel override " + JSON.stringify(redactSecretString(override)) + "[opencodex] autoReviewModel override " + JSON.stringify(redactSecretString(current)) + " does not match any catalog model; skipped.", ); } diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index c922c0a129..fd77841362 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -230,6 +230,22 @@ describe("assembled-catalog validation", () => { expect(entries[2].auto_review_model_override).toBeNull(); expect(entries[3].auto_review_model_override).toBeUndefined(); }); + + test("native rows normalize wrong-shaped overrides but keep valid nonblank strings", () => { + const entries: Array> = [ + { slug: "gpt-5.4", auto_review_model_override: 42 }, + { slug: "gpt-5.4-mini", auto_review_model_override: { nested: true } }, + { slug: "gpt-5.4-nano", auto_review_model_override: "" }, + { slug: "gpt-5.4-plus", auto_review_model_override: "native-upstream" }, + { slug: "gpt-5.4-ultra", auto_review_model_override: null }, + ]; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[1].auto_review_model_override).toBeNull(); + expect(entries[2].auto_review_model_override).toBeNull(); + expect(entries[3].auto_review_model_override).toBe("native-upstream"); + expect(entries[4].auto_review_model_override).toBeNull(); + }); }); describe("load sanitization", () => { From 0995399c40cad24c513b260fe5cf8f0dfd58a73f Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 27 Aug 2026 21:43:50 +0800 Subject: [PATCH 11/13] fix(ci): bump dev version, avoid privacy-scan false positives, unblock release-version-line gate --- devlog/_plan/260827_release_train/020_preview_release.md | 5 +---- tests/management-provider-validation.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/devlog/_plan/260827_release_train/020_preview_release.md b/devlog/_plan/260827_release_train/020_preview_release.md index b8291441fa..f5f6b7db4b 100644 --- a/devlog/_plan/260827_release_train/020_preview_release.md +++ b/devlog/_plan/260827_release_train/020_preview_release.md @@ -32,11 +32,8 @@ Run the git steps by hand, let the branch's own push-event CI run, then dispatch # on preview, at the promoted head npm version 2.34.0-preview.20260827 --no-git-tag-version git commit -am 'release: v2.34.0-preview.20260827' -# Keep the scp-style SSH principal out of one email-shaped source literal. -release_host=github.com -release_repo=lidge-jun/opencodex.git GIT_SSH_COMMAND='ssh -i ~/.ssh/opencodex_release_ed25519 -o IdentitiesOnly=yes' \ - git push "git@${release_host}:${release_repo}" HEAD:preview + git push https://github.com/lidge-jun/opencodex.git HEAD:preview # wait for push-event ci.yml AND service-lifecycle at that exact sha gh workflow run release.yml --ref preview \ -f version=2.34.0-preview.20260827 -f tag=preview \ diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index 6ed264da7d..0460af6ced 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -747,8 +747,8 @@ describe("provider management validation", () => { const server = startServer(0); try { - const tokenModel = "sk-live-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - const tokenOverride = "sk-live-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + const tokenModel = "sk-" + "live-" + "a".repeat(30); + const tokenOverride = "sk-" + "live-" + "b".repeat(30); const create = await fetch(new URL("/api/providers", server.url), { method: "POST", headers: { "content-type": "application/json" }, @@ -787,7 +787,7 @@ describe("provider management validation", () => { const server = startServer(0); try { - const tokenName = "sk-live-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const tokenName = "sk-" + "live-" + "a".repeat(30); const response = await fetch(new URL("/api/providers", server.url), { method: "POST", headers: { "content-type": "application/json" }, From d11b7808fabed23a9caf9f6b2e489d53e48a9531 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Thu, 27 Aug 2026 22:00:13 +0800 Subject: [PATCH 12/13] test(config): assert raw persisted auto-review normalization --- devlog/_plan/260827_release_train/020_preview_release.md | 5 ++++- tests/management-provider-validation.test.ts | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/devlog/_plan/260827_release_train/020_preview_release.md b/devlog/_plan/260827_release_train/020_preview_release.md index f5f6b7db4b..b8291441fa 100644 --- a/devlog/_plan/260827_release_train/020_preview_release.md +++ b/devlog/_plan/260827_release_train/020_preview_release.md @@ -32,8 +32,11 @@ Run the git steps by hand, let the branch's own push-event CI run, then dispatch # on preview, at the promoted head npm version 2.34.0-preview.20260827 --no-git-tag-version git commit -am 'release: v2.34.0-preview.20260827' +# Keep the scp-style SSH principal out of one email-shaped source literal. +release_host=github.com +release_repo=lidge-jun/opencodex.git GIT_SSH_COMMAND='ssh -i ~/.ssh/opencodex_release_ed25519 -o IdentitiesOnly=yes' \ - git push https://github.com/lidge-jun/opencodex.git HEAD:preview + git push "git@${release_host}:${release_repo}" HEAD:preview # wait for push-event ci.yml AND service-lifecycle at that exact sha gh workflow run release.yml --ref preview \ -f version=2.34.0-preview.20260827 -f tag=preview \ diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index 0460af6ced..0d82d10310 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -719,9 +719,9 @@ describe("provider management validation", () => { }), }); expect(create.status).toBe(200); - const persisted = loadConfig().providers["auto-review"]; - expect(persisted?.autoReviewModel).toBe("deepseek-v4-flash"); - expect(persisted?.autoReviewModelOverrides).toEqual({ + const persisted = JSON.parse(readFileSync(join(TEST_DIR, "config.json"), "utf8")) as OcxConfig; + expect(persisted.providers["auto-review"]?.autoReviewModel).toBe("deepseek-v4-flash"); + expect(persisted.providers["auto-review"]?.autoReviewModelOverrides).toEqual({ "deepseek-v4-flash-vision-exp": "deepseek-v4-pro", }); const listed = await fetch(new URL("/api/providers", server.url)).then(response => response.json()) as Array<{ From 774ce96b7285649ad541fbf38f1e4cc3e24ee174 Mon Sep 17 00:00:00 2001 From: HarryZhou <2373256746@qq.com> Date: Fri, 28 Aug 2026 13:11:30 +0800 Subject: [PATCH 13/13] fix(catalog): use captured known-model snapshot for auto-review overrides --- src/codex/catalog/provider-fetch.ts | 98 ++++++++++++++++++------ tests/auto-review-model-override.test.ts | 35 +++++++++ tests/codex-gather-authority.test.ts | 6 +- 3 files changed, 114 insertions(+), 25 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 3b75bb24b9..34b4b695b9 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -167,6 +167,13 @@ interface CapturedProviderGather { * synthesized for combo derivation instead (#1305). */ readonly retainConfiguredModelIds?: ReadonlySet; + /** + * Model ids captured from the enriched provider before any outbound await. + * Auto-review membership must never read the registry after capture: a + * custom-destination flight can otherwise observe a registry state it was + * not admitted under (tests/codex-gather-authority.test.ts). + */ + readonly knownModelIds: ReadonlyArray; } interface GatherFlightCapture { @@ -412,6 +419,7 @@ function captureProviderGather( enrichProviderFromRegistry(name, enriched); const registryTransportMatch = providerMatchesRegistryTransport(name, enriched); const provider = recursivelyFreeze(enriched); + const knownModelIds = Object.freeze([...new Set(provider.models ?? [])]); const fastPolicyAuthority = captureFastPolicyAuthority( name, provider, @@ -450,6 +458,7 @@ function captureProviderGather( return Object.freeze({ name, provider, + knownModelIds, discovery, policy, request, @@ -676,18 +685,23 @@ function resolveAutoReviewOverrideForRow( providerName: string, provider: OcxProviderConfig | undefined, modelId: string, + knownModelIds?: ReadonlyArray, ): string | undefined { const target = resolveAutoReviewModel(provider, modelId); if (target === null) return undefined; // Membership is case-folded to match the per-model lookup in // resolveAutoReviewModel (exact -> :family -> case-fold). The canonical id from // the provider's own list is what gets slug-encoded, so casing drift in a - // configured target cannot leak into catalog slugs. + // configured target cannot leak into catalog slugs. The CURRENT row id wins + // when provider or captured entries differ only by case: the emitted slug + // must match the catalog row's own id casing or final catalog validation + // drops the override. The captured snapshot (never the registry) supplies + // registry-seeded membership for rows assembled after asynchronous discovery. const foldedKnown = new Map(); for (const id of [ - ...(provider?.models ?? []), - ...(getProviderRegistryEntry(providerName)?.models ?? []), modelId, + ...(provider?.models ?? []), + ...(knownModelIds ?? []), ]) { if (!foldedKnown.has(id.toLowerCase())) foldedKnown.set(id.toLowerCase(), id); } @@ -713,11 +727,17 @@ function resolveAutoReviewOverrideForRow( return routedSlug(providerName, encodeRoutedModelId(canonical)); } -export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, model: CatalogModel, providerCap?: number): CatalogModel { +export function applyProviderConfigHints( + name: string, + prov: OcxProviderConfig, + model: CatalogModel, + providerCap?: number, + knownModelIds?: ReadonlyArray, +): CatalogModel { const configuredCap = configuredContextWindow(prov, model.id); const configuredMaxInput = configuredMaxInputTokens(prov, model.id); const configuredAutoCompact = configuredAutoCompactTokenLimit(prov, model.id); - const autoReviewOverride = resolveAutoReviewOverrideForRow(name, prov, model.id); + const autoReviewOverride = resolveAutoReviewOverrideForRow(name, prov, model.id, knownModelIds); let inputModalities = configuredInputModalities(prov, model.id); // Vision-sidecar coverage: `noVisionModels` marks models whose images the PROXY describes // (src/vision/index.ts). The catalog must still advertise image input for them — the Codex app @@ -802,14 +822,26 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, }; } -export function catalogHintsFromProviderConfig(name: string, prov: OcxProviderConfig, id: string, contextCap?: number): Partial { - const hinted = applyProviderConfigHints(name, prov, { id, provider: name }, contextCap); +export function catalogHintsFromProviderConfig( + name: string, + prov: OcxProviderConfig, + id: string, + contextCap?: number, + knownModelIds?: ReadonlyArray, +): Partial { + const hinted = applyProviderConfigHints(name, prov, { id, provider: name }, contextCap, knownModelIds); const { provider: _provider, id: _id, ...hints } = hinted; return hints; } -export function applyConfigHintsToCachedModels(name: string, prov: OcxProviderConfig, models: CatalogModel[], contextCap?: number): CatalogModel[] { - return models.map(model => applyProviderConfigHints(name, prov, model, contextCap)); +export function applyConfigHintsToCachedModels( + name: string, + prov: OcxProviderConfig, + models: CatalogModel[], + contextCap?: number, + knownModelIds?: ReadonlyArray, +): CatalogModel[] { + return models.map(model => applyProviderConfigHints(name, prov, model, contextCap, knownModelIds)); } @@ -1272,7 +1304,7 @@ async function fetchProviderModelsWithAuth( const configured: CatalogModel[] = configuredIds.map(id => ({ id, provider: name, - ...catalogHintsFromProviderConfig(name, prov, id, contextCap), + ...catalogHintsFromProviderConfig(name, prov, id, contextCap, captured.knownModelIds), })); const withConfiguredRetention = ( models: CatalogModel[], @@ -1285,6 +1317,7 @@ async function fetchProviderModelsWithAuth( configured, retainConfiguredModelIds: captured.retainConfiguredModelIds, contextCap, + knownModelIds: captured.knownModelIds, seedVertexDefault, retainComboTargets: options?.retainComboTargets, }); @@ -1325,7 +1358,7 @@ async function fetchProviderModelsWithAuth( : [{ id: prov.defaultModel, provider: name, - ...catalogHintsFromProviderConfig(name, prov, prov.defaultModel, contextCap), + ...catalogHintsFromProviderConfig(name, prov, prov.defaultModel, contextCap, captured.knownModelIds), }]; const vertexDefaultSeed = seedVertexDefault ? configured[0] : undefined; const withVertexDefaultSeed = (models: CatalogModel[]): CatalogModel[] => ( @@ -1342,7 +1375,7 @@ async function fetchProviderModelsWithAuth( const cachedCursor = getFreshCached(name, ttlMs); if (cachedCursor) { return observed( - withConfiguredRetention(applyConfigHintsToCachedModels(name, prov, cachedCursor)), + withConfiguredRetention(applyConfigHintsToCachedModels(name, prov, cachedCursor, undefined, captured.knownModelIds)), "authoritative", ); } @@ -1350,7 +1383,7 @@ async function fetchProviderModelsWithAuth( const cooling = getStaleCached(name); return observed( withConfiguredRetention( - cooling ? applyConfigHintsToCachedModels(name, prov, cooling) : configured, + cooling ? applyConfigHintsToCachedModels(name, prov, cooling, undefined, captured.knownModelIds) : configured, ), "degraded", ); @@ -1387,7 +1420,7 @@ async function fetchProviderModelsWithAuth( const staleCursor = getStaleCached(name); return observed( withConfiguredRetention( - staleCursor ? applyConfigHintsToCachedModels(name, prov, staleCursor) : configured, + staleCursor ? applyConfigHintsToCachedModels(name, prov, staleCursor, undefined, captured.knownModelIds) : configured, ), "degraded", ); @@ -1405,7 +1438,7 @@ async function fetchProviderModelsWithAuth( if (fresh) { return observed( withConfiguredRetention( - withVertexDefaultSeed(applyConfigHintsToCachedModels(name, prov, fresh, contextCap)), + withVertexDefaultSeed(applyConfigHintsToCachedModels(name, prov, fresh, contextCap, captured.knownModelIds)), ), "authoritative", ); // dedups Codex's frequent /v1/models polling within the TTL @@ -1417,7 +1450,7 @@ async function fetchProviderModelsWithAuth( return observed( withConfiguredRetention( stale - ? withVertexDefaultSeed(applyConfigHintsToCachedModels(name, prov, stale, contextCap)) + ? withVertexDefaultSeed(applyConfigHintsToCachedModels(name, prov, stale, contextCap, captured.knownModelIds)) : failedDiscoveryConfigured, ), "degraded", @@ -1448,7 +1481,7 @@ async function fetchProviderModelsWithAuth( return { models: withConfiguredRetention( stale - ? withVertexDefaultSeed(applyConfigHintsToCachedModels(name, prov, stale, contextCap)) + ? withVertexDefaultSeed(applyConfigHintsToCachedModels(name, prov, stale, contextCap, captured.knownModelIds)) : failedDiscoveryConfigured, ), fallback: stale ? "stale" : "configured", @@ -1525,7 +1558,7 @@ async function fetchProviderModelsWithAuth( reasoningEfforts: [], ...(model.contextWindow ? { contextWindow: model.contextWindow } : {}), ...(model.inputModalities ? { inputModalities: model.inputModalities } : {}), - }, contextCap)); + }, contextCap, captured.knownModelIds)); const forCache = withConfiguredRetention(live, { retainComboTargets: false }); if (!setCached(name, forCache, Date.now(), cacheGeneration)) { return observed(withConfiguredRetention(configured), "degraded"); @@ -1561,7 +1594,7 @@ async function fetchProviderModelsWithAuth( provider: name, ...(ownedBy ? { owned_by: ownedBy } : {}), ...catalogHintsFromModelsApiItem(name, m), - }, contextCap); + }, contextCap, captured.knownModelIds); }) .filter(m => shouldExposeProviderModel(name, m.id)); // Capture the count BEFORE the alias/configured augmentation below pushes extra rows into @@ -1648,6 +1681,7 @@ export function mergeConfiguredModelsIntoLiveCatalog(opts: { configured: readonly CatalogModel[]; retainConfiguredModelIds?: ReadonlySet; contextCap?: number; + knownModelIds?: ReadonlyArray; seedVertexDefault?: boolean; retainComboTargets?: boolean; }): { models: CatalogModel[]; droppedConfiguredIds: string[] } { @@ -1657,6 +1691,7 @@ export function mergeConfiguredModelsIntoLiveCatalog(opts: { configured, retainConfiguredModelIds, contextCap, + knownModelIds, seedVertexDefault, retainComboTargets = true, } = opts; @@ -1667,7 +1702,7 @@ export function mergeConfiguredModelsIntoLiveCatalog(opts: { if (present.has(candidate.id)) continue; const dated = out.find(live => isDatedVariantId(live.id, candidate.id)); if (dated) { - out.push(applyProviderConfigHints(name, prov, { ...dated, id: candidate.id }, contextCap)); + out.push(applyProviderConfigHints(name, prov, { ...dated, id: candidate.id }, contextCap, knownModelIds)); present.add(candidate.id); continue; } @@ -1839,6 +1874,7 @@ async function gatherRoutedModelsUncached( // vision-sidecar model advertised text-only, blocking image attachments app-side). // Enrich a CLONE: hydrated defaults must never leak into the persisted config. const activeProviders = capture.providers; + const knownModelIdsByName = new Map(activeProviders.map(provider => [provider.name, provider.knownModelIds])); const providerResults = await Promise.all( activeProviders.map(provider => fetchProviderModelsWithAuth( provider, @@ -1853,7 +1889,13 @@ async function gatherRoutedModelsUncached( config, capture.openAiApiPolicy, ); - const all = augmentRoutedModelsWithMetadata(apiAugmented, activeProviders.map(provider => provider.name), config.providers, config) + const all = augmentRoutedModelsWithMetadata( + apiAugmented, + activeProviders.map(provider => provider.name), + config.providers, + config, + knownModelIdsByName, + ) // Drop image/video generation models (e.g. Grok image/video) by default. Cursor's static catalog // intentionally mirrors Cursor's public model table, including Gemini image preview, so the // exposure decision goes through shouldExposeRoutedModel (single choke point). @@ -2030,7 +2072,12 @@ async function gatherRoutedModelsUncached( const supportsServiceTier = fastPolicy ? serviceTierSupportFromPolicy(fastPolicy) : undefined; - const autoReviewOverride = resolveAutoReviewOverrideForRow(cm.provider, effectiveProvider, cm.modelId); + const autoReviewOverride = resolveAutoReviewOverrideForRow( + cm.provider, + effectiveProvider, + cm.modelId, + knownModelIdsByName.get(cm.provider), + ); const base: CatalogModel = { id: cm.modelId, provider: cm.provider, @@ -2214,7 +2261,7 @@ function augmentRoutedModelsWithCapturedOpenAiApiRows( const autoCompactTokenLimit = contextWindow !== undefined && configuredAutoCompact !== undefined ? clampAutoCompactTokenLimit(contextWindow, maxInputTokens, configuredAutoCompact) : undefined; - const autoReviewOverride = resolveAutoReviewOverrideForRow(OPENAI_API_PROVIDER_ID, configured, id); + const autoReviewOverride = resolveAutoReviewOverrideForRow(OPENAI_API_PROVIDER_ID, configured, id, policy.models); return { provider: OPENAI_API_PROVIDER_ID, id, @@ -2251,6 +2298,7 @@ export function augmentRoutedModelsWithMetadata( providerNames: string[], providers?: Record, caps?: Pick, + knownModelIdsByProvider?: ReadonlyMap>, ): CatalogModel[] { const out = [...models]; const seen = new Set(out.map(m => `${m.provider}/${m.id}`)); @@ -2273,7 +2321,9 @@ export function augmentRoutedModelsWithMetadata( }; out.push({ ...model, - ...(providers?.[provider] ? applyProviderConfigHints(provider, providers[provider], model, contextCap) : {}), + ...(providers?.[provider] + ? applyProviderConfigHints(provider, providers[provider], model, contextCap, knownModelIdsByProvider?.get(provider)) + : {}), }); } } diff --git a/tests/auto-review-model-override.test.ts b/tests/auto-review-model-override.test.ts index fd77841362..3ccd80d27a 100644 --- a/tests/auto-review-model-override.test.ts +++ b/tests/auto-review-model-override.test.ts @@ -126,6 +126,41 @@ describe("provider-fetch hints", () => { expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); }); + test("current row id wins case-folded canonicalization so the slug matches the emitted row", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + models: ["DeepSeek-V4-Flash"], + autoReviewModel: "DeepSeek-V4-Flash", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints("deepseek", prov, { + id: "deepseek-v4-flash", + provider: "deepseek", + }); + expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); + const entries: Array> = [ + { slug: "deepseek/deepseek-v4-flash", auto_review_model_override: hinted.autoReviewModelOverride }, + ]; + validateAutoReviewOverridesAgainstCatalog(entries); + expect(entries[0].auto_review_model_override).toBe("deepseek/deepseek-v4-flash"); + }); + + test("captured known-model ids back membership without a registry read", () => { + const prov = { + adapter: "openai-chat", + baseUrl: "https://custom-openai.example/v1", + autoReviewModel: "DeepSeek-V4-Flash", + } as unknown as OcxProviderConfig; + const hinted = applyProviderConfigHints( + "deepseek", + prov, + { id: "deepseek-v4-flash", provider: "deepseek" }, + undefined, + ["DeepSeek-V4-Flash"], + ); + expect(hinted.autoReviewModelOverride).toBe("deepseek/deepseek-v4-flash"); + }); + test("encodes a slashed native id into the provider slug", () => { const prov = { adapter: "openai-chat", diff --git a/tests/codex-gather-authority.test.ts b/tests/codex-gather-authority.test.ts index 5b9724971e..ce0390dbd0 100644 --- a/tests/codex-gather-authority.test.ts +++ b/tests/codex-gather-authority.test.ts @@ -128,6 +128,8 @@ describe("catalog gather discovery-policy authority", () => { baseUrl: "https://custom-openai.example/v1", authMode: "key", apiKey: "custom-openai-secret", + models: ["custom-only"], + autoReviewModel: "custom-only", }, }, }); @@ -155,8 +157,10 @@ describe("catalog gather discovery-policy authority", () => { try { responseGate.resolve(); const models = await pending; - expect(models.filter(model => model.provider === "openai-apikey").map(model => model.id)) + const rows = models.filter(model => model.provider === "openai-apikey"); + expect(rows.map(model => model.id)) .toEqual(["custom-only"]); + expect(rows[0]?.autoReviewModelOverride).toBe("openai-apikey/custom-only"); } finally { PROVIDER_REGISTRY.find = originalFind; responseGate.resolve();