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..34b4b695b9 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, @@ -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"; @@ -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, @@ -663,10 +672,72 @@ function configuredVerbositySupport(name: string, prov: OcxProviderConfig | unde return prov.supportsVerbosity; } -export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, model: CatalogModel, providerCap?: number): CatalogModel { +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, + 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. 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 [ + modelId, + ...(provider?.models ?? []), + ...(knownModelIds ?? []), + ]) { + 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 (canonical === undefined) { + const key = providerName + "/" + target; + if (!warnedAutoReviewTargets.has(key)) { + warnedAutoReviewTargets.add(key); + console.warn( + "[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(canonical)); +} + +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, 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 @@ -697,6 +768,9 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, const hinted = { ...modelWithoutServiceTier, ...(hintedWindow !== undefined ? { contextWindow: hintedWindow } : {}), + // 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 @@ -748,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)); } @@ -1218,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[], @@ -1231,6 +1317,7 @@ async function fetchProviderModelsWithAuth( configured, retainConfiguredModelIds: captured.retainConfiguredModelIds, contextCap, + knownModelIds: captured.knownModelIds, seedVertexDefault, retainComboTargets: options?.retainComboTargets, }); @@ -1271,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[] => ( @@ -1288,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", ); } @@ -1296,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", ); @@ -1333,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", ); @@ -1351,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 @@ -1363,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", @@ -1394,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", @@ -1471,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"); @@ -1507,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 @@ -1594,6 +1681,7 @@ export function mergeConfiguredModelsIntoLiveCatalog(opts: { configured: readonly CatalogModel[]; retainConfiguredModelIds?: ReadonlySet; contextCap?: number; + knownModelIds?: ReadonlyArray; seedVertexDefault?: boolean; retainComboTargets?: boolean; }): { models: CatalogModel[]; droppedConfiguredIds: string[] } { @@ -1603,6 +1691,7 @@ export function mergeConfiguredModelsIntoLiveCatalog(opts: { configured, retainConfiguredModelIds, contextCap, + knownModelIds, seedVertexDefault, retainComboTargets = true, } = opts; @@ -1613,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; } @@ -1785,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, @@ -1799,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). @@ -1976,6 +2072,12 @@ async function gatherRoutedModelsUncached( const supportsServiceTier = fastPolicy ? serviceTierSupportFromPolicy(fastPolicy) : undefined; + const autoReviewOverride = resolveAutoReviewOverrideForRow( + cm.provider, + effectiveProvider, + cm.modelId, + knownModelIdsByName.get(cm.provider), + ); const base: CatalogModel = { id: cm.modelId, provider: cm.provider, @@ -2054,7 +2156,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 @@ -2155,6 +2261,7 @@ function augmentRoutedModelsWithCapturedOpenAiApiRows( const autoCompactTokenLimit = contextWindow !== undefined && configuredAutoCompact !== undefined ? clampAutoCompactTokenLimit(contextWindow, maxInputTokens, configuredAutoCompact) : undefined; + const autoReviewOverride = resolveAutoReviewOverrideForRow(OPENAI_API_PROVIDER_ID, configured, id, policy.models); return { provider: OPENAI_API_PROVIDER_ID, id, @@ -2162,6 +2269,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]!] } : {}), }; @@ -2190,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}`)); @@ -2212,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/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index fda9724849..355af8bc7f 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -347,6 +347,12 @@ 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: 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; + } } else { applyNativeOpenAiContextOverride(e, contextCap); if (isGpt56NativeSlug(slug)) ensureGpt56ReasoningLevels(e); @@ -394,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, @@ -2007,3 +2016,58 @@ 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 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 { + // 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; + // 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 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 current !== "string") continue; + if (!slugs.has(current)) { + if (!warned.has(current)) { + warned.add(current); + console.warn( + "[opencodex] autoReviewModel override " + JSON.stringify(redactSecretString(current)) + + " does not match any catalog model; skipped.", + ); + } + entry.auto_review_model_override = null; + } + } +} 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 1308b3a64b..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 { @@ -500,6 +501,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).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(), supportsServiceTier: z.boolean().optional(), modelSupportsServiceTier: z.record(z.string().min(1), z.boolean()).optional(), @@ -534,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"; @@ -1817,6 +1828,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/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/providers/derive.ts b/src/providers/derive.ts index de5c5821e1..9e9f713364 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,48 @@ 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 = 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(); + } + return null; +} + +/** Per-model override lookup: exact model, exact :family, folded model, folded :family. */ +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(":"); + 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); + const familyMatch = Object.entries(overrides).find(([key]) => key.toLowerCase() === family.toLowerCase()); + if (familyMatch) return familyMatch[1]; + } + 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/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/management/provider-routes.ts b/src/server/management/provider-routes.ts index d542306885..c0fc14cba6 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,18 @@ 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; + 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; +} + /** * 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, @@ -145,6 +162,31 @@ 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 }; + if ("clear" in normalized) { + delete next.autoReviewModel; + touched = true; + } else { + next.autoReviewModel = normalized.value; + touched = true; + } + } + if (Object.hasOwn(rawBody, "autoReviewModelOverrides")) { + const normalized = normalizeAutoReviewModelOverridesField(rawBody.autoReviewModelOverrides); + if ("error" in normalized) return { error: normalized.error }; + if ("clear" in normalized) { + delete next.autoReviewModelOverrides; + touched = true; + } else { + next.autoReviewModelOverrides = normalized.value; + touched = true; + } + } if (Object.hasOwn(rawBody, "authMode")) { if (typeof rawBody.authMode !== "string") return { error: "authMode must be a string" }; const mode = rawBody.authMode.trim(); @@ -452,6 +494,13 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise [ + redactSecretString(key), + redactSecretString(value), + ])), modelSupportsServiceTier: p.modelSupportsServiceTier, noStructuredOutputModels: p.noStructuredOutputModels, upstreamHttpVersion: p.upstreamHttpVersion, @@ -543,6 +592,8 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise 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..6d80312ea7 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -364,3 +364,29 @@ 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. 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. + +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 new file mode 100644 index 0000000000..3ccd80d27a --- /dev/null +++ b/tests/auto-review-model-override.test.ts @@ -0,0 +1,347 @@ +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 { 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 { 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("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("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("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(); + }); +}); + +describe("catalog stamping", () => { + const template = { auto_review_model_override: null, context_window: 272000 } as Record; + + test("stamps a pre-normalized provider/model slug verbatim", () => { + const model: CatalogModel = { + id: "deepseek-v4-flash-vision-exp", + provider: "deepseek", + 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("stamps the override in the no-template fallback branch", () => { + const model: CatalogModel = { + id: "m", + provider: "blsc", + autoReviewModelOverride: "deepseek/deepseek-v4-flash", + }; + const entry = deriveEntry(null, "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/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("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", + 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", () => { + 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(); + }); + + 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).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" }, + { 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(); + }); + + 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(); + }); + + 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", () => { + 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 normalization", () => { + test("management rejects malformed overrides", () => { + 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("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", + }); + }); + + test("POST normalization rejects whitespace inside ids", () => { + const provider = { autoReviewModel: "deep seek-v4-flash" }; + expect(normalizeAutoReviewModelFields(provider)).toContain("autoReviewModel"); + }); +}); diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index a2f5b52fdc..28eced0f82 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -3007,6 +3007,15 @@ describe("Codex catalog routed normalization", () => { .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/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(); diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index c839c5b6fa..0d82d10310 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -695,6 +695,163 @@ 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 = 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<{ + 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); + } + }); + + 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-" + "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" }, + 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 }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const tokenName = "sk-" + "live-" + "a".repeat(30); + 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); + } + }); + + 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"), + providers: poolProviders(), + }); + + 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