From 104f29de65708f49507a117ae4264ba301cf8ca8 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 1 Oct 2026 00:04:24 +0900 Subject: [PATCH] fix(models): apply new-model policy before discovery, sync and export publication Carries #6260 with a maintainer fix: a file-loaded config keeps its file provenance after discovery inventory drift, so stale discovery is refused under the mutation coordinator instead of disabling a model in memory that a later unrelated save would persist over the operator's choice. Co-authored-by: colthreepv <2657230+colthreepv@users.noreply.github.com> --- .../src/content/docs/guides/model-routing.md | 3 + .../content/docs/ja/guides/model-routing.md | 9 + .../content/docs/ko/guides/model-routing.md | 9 + .../content/docs/ru/guides/model-routing.md | 9 + .../docs/zh-cn/guides/model-routing.md | 7 + scripts/test-layout/layout.json | 4 + src/codex/catalog/retained-sync.ts | 9 + src/config.ts | 7 +- src/config/live-reconcile.ts | 18 ++ src/config/rebase-provenance.ts | 12 + src/providers/new-model-policy-runtime.ts | 72 ++++++ src/providers/new-model-policy.ts | 25 ++ src/server/management/model-rows.ts | 36 ++- src/server/management/shared.ts | 11 +- structure/catalog.md | 2 +- structure/clients/integrations.md | 8 +- structure/config.md | 4 +- structure/gui-and-management-api.md | 2 +- structure/providers-and-adapters.md | 1 + structure/runtime.md | 2 +- .../codex-sync-new-model-policy.test.ts | 236 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 4 + .../new-model-policy-runtime.test.ts | 203 +++++++++++++++ tests/providers/new-model-policy.test.ts | 154 +++++++++++- .../model-export-new-model-policy.test.ts | 236 ++++++++++++++++++ .../server-new-model-policy-arrival.test.ts | 154 ++++++++++++ 26 files changed, 1220 insertions(+), 17 deletions(-) create mode 100644 src/providers/new-model-policy-runtime.ts create mode 100644 tests/codex-integration/codex-sync-new-model-policy.test.ts create mode 100644 tests/providers/new-model-policy-runtime.test.ts create mode 100644 tests/server/model-export-new-model-policy.test.ts create mode 100644 tests/server/server-new-model-policy-arrival.test.ts diff --git a/docs-site/src/content/docs/guides/model-routing.md b/docs-site/src/content/docs/guides/model-routing.md index 74f08b67ec0..f8e66dee0f3 100644 --- a/docs-site/src/content/docs/guides/model-routing.md +++ b/docs-site/src/content/docs/guides/model-routing.md @@ -85,6 +85,9 @@ Routing and catalog visibility are separate controls: - Fresh installs set `modelDiscovery.newModelPolicy` to `"off"`. After the first successful live fetch establishes a baseline, later arrivals are appended to `disabledModels` and carry a **NEW** dashboard badge until enabled or acknowledged. Existing installs remain `"on"` until opted in. + The policy applies before publishing newly discovered models through `/v1/models`, the dashboard, + client configuration exports, or a Codex catalog sync (including service startup). + Enabling an arrival manually keeps it enabled on later refreshes and exports. Use `ocx models new-policy off` globally, add `--provider ` for an override, and inspect `ocx models new-arrivals [--json]`. Failed/degraded fetches never change the baseline. Providers with a non-empty `selectedModels` (including preset mode) are already curated, so this policy is diff --git a/docs-site/src/content/docs/ja/guides/model-routing.md b/docs-site/src/content/docs/ja/guides/model-routing.md index 7dda1864c0a..900185715a3 100644 --- a/docs-site/src/content/docs/ja/guides/model-routing.md +++ b/docs-site/src/content/docs/ja/guides/model-routing.md @@ -83,6 +83,15 @@ model ID は変更しません。`openai-apikey/` は API key transport ブロックしません。 - プロバイダーの `selectedModels` が空でなければカタログ許可リストとして動作します。ライブモデル探索と 直接ルーティングはそのままに、カタログと `/v1/models` に公開するモデルだけ絞ります。 +- `modelDiscovery.newModelPolicy` が `"off"` の場合、最初の成功したライブ探索で基準を確立し、 + その後に追加されたモデルを `disabledModels` に入れます。これは `/v1/models`、ダッシュボード、 + クライアント設定のエクスポート、サービス起動時を含む Codex カタログ同期で公開する前に + 適用されます。手動で有効化したモデルは、その後の更新やエクスポートでも有効なままです。 + 新規インストールの既定値は `"off"`、既存のインストールは変更するまで `"on"` です。 + `ocx models new-policy off` で全体に適用し、`--provider ` で個別に上書きします。 + `ocx models new-arrivals [--json]` で追加モデルを確認できます。失敗または縮退した探索では + 基準は変わりません。`selectedModels` が空でないプロバイダーはすでに選別済みのため、 + このポリシーによる自動無効化は行いません。 - `provider.disabled: true` のプロバイダーはカタログ探索から除外されます。明示的 `provider/model` リクエストは 失敗し、`defaultModel` / `models[]` 検査でもスキップします。 - `providerContextCaps` はプロバイダーごとに Codex に表示するコンテキスト上限を指定します。 diff --git a/docs-site/src/content/docs/ko/guides/model-routing.md b/docs-site/src/content/docs/ko/guides/model-routing.md index ac6425f3423..d20f8320c7d 100644 --- a/docs-site/src/content/docs/ko/guides/model-routing.md +++ b/docs-site/src/content/docs/ko/guides/model-routing.md @@ -82,6 +82,15 @@ fallback하지 않습니다. 막지는 않습니다. - 프로바이더의 `selectedModels`가 비어 있지 않으면 카탈로그 허용 목록으로 동작합니다. 실시간 모델 탐색과 직접 라우팅은 그대로 두고, 카탈로그와 `/v1/models`에 내보낼 모델만 줄입니다. +- `modelDiscovery.newModelPolicy`가 `"off"`이면 첫 실시간 탐색 성공 시 기준을 만들고, + 이후 추가된 모델을 `disabledModels`에 넣습니다. 이 정책은 `/v1/models`, 대시보드, + 클라이언트 설정 내보내기, 서비스 시작을 포함한 Codex 카탈로그 동기화에서 새 모델을 공개하기 + 전에 적용됩니다. 수동으로 활성화한 모델은 이후 새로고침과 내보내기에서도 활성 상태를 유지합니다. + 새 설치의 기본값은 `"off"`이며, 기존 설치는 직접 바꾸기 전까지 `"on"`을 유지합니다. + `ocx models new-policy off`로 전역 정책을 설정하고, `--provider `으로 프로바이더별 + 정책을 지정합니다. `ocx models new-arrivals [--json]`으로 새 모델을 확인할 수 있습니다. + 실패하거나 제한된 탐색은 기준을 바꾸지 않습니다. `selectedModels`가 비어 있지 않은 + 프로바이더는 이미 선택 목록을 사용하므로 이 정책으로 모델을 자동 비활성화하지 않습니다. - `provider.disabled: true`인 프로바이더는 카탈로그 탐색에서 제외됩니다. 명시적 `provider/model` 요청은 실패하고, `defaultModel` / `models[]` 검사에서도 건너뜁니다. - `providerContextCaps`는 공급자별로 Codex에 표시할 컨텍스트 상한을 지정합니다. diff --git a/docs-site/src/content/docs/ru/guides/model-routing.md b/docs-site/src/content/docs/ru/guides/model-routing.md index ce96fc95b8d..117802629dc 100644 --- a/docs-site/src/content/docs/ru/guides/model-routing.md +++ b/docs-site/src/content/docs/ru/guides/model-routing.md @@ -88,6 +88,15 @@ description: Как opencodex решает, какой провайдер буд - Непустой `selectedModels` провайдера — ещё один список разрешённых для каталога. Живое обнаружение и прямая маршрутизация продолжают работать; сужается только вывод в каталог и `/v1/models`. +- При `modelDiscovery.newModelPolicy: "off"` первое успешное живое обнаружение задаёт базовый + список; последующие новые модели добавляются в `disabledModels` до публикации в `/v1/models`, + дашборде, экспорте конфигурации клиентов и синхронизации каталога Codex, включая запуск сервиса. + Модель, включённая вручную, остаётся включённой при последующих обновлениях и экспортах. + Новые установки используют `"off"`; существующие сохраняют `"on"`, пока пользователь не изменит + политику. `ocx models new-policy off` задаёт общую политику; `--provider ` переопределяет + её для провайдера. `ocx models new-arrivals [--json]` показывает новые модели. Неудачное или + деградировавшее обнаружение не меняет базовый список. Для провайдеров с непустым `selectedModels` + автоматическое отключение по этой политике не применяется: их список уже отобран. - `provider.disabled: true` убирает провайдера из обнаружения каталога. Явные запросы `provider/model` завершаются ошибкой, а проверки `defaultModel` / `models[]` его пропускают. - `providerContextCaps` задаёт видимые для Codex лимиты контекста по провайдерам. diff --git a/docs-site/src/content/docs/zh-cn/guides/model-routing.md b/docs-site/src/content/docs/zh-cn/guides/model-routing.md index 5bfe9d28bcb..1b51859ec35 100644 --- a/docs-site/src/content/docs/zh-cn/guides/model-routing.md +++ b/docs-site/src/content/docs/zh-cn/guides/model-routing.md @@ -72,6 +72,13 @@ transport；这些凭证路径互不 fallback。 仍保留在目录中，但会改为 `visibility: "hide"`。它**不会**拒绝对该模型的直接请求。 - 提供商的非空 `selectedModels` 是另一层目录 allowlist。实时发现和直接路由仍然有效；它只会缩小 目录和 `/v1/models` 输出的模型范围。 +- 当 `modelDiscovery.newModelPolicy` 为 `"off"` 时，首次成功的实时发现建立基准，后续新增 + 模型会加入 `disabledModels`。此策略会在通过 `/v1/models`、仪表盘、客户端配置导出或 + Codex 目录同步（包括服务启动）发布新模型之前生效。手动启用的模型在后续刷新和导出中仍保持 + 启用。新安装默认使用 `"off"`，已有安装在主动更改前保持 `"on"`。使用 + `ocx models new-policy off` 设置全局策略，添加 `--provider ` 覆盖单个提供商的设置； + 使用 `ocx models new-arrivals [--json]` 查看新增模型。失败或降级的发现不会改变基准。 + 非空 `selectedModels` 已经限定了提供商的模型列表，因此此策略不会再自动禁用模型。 - `provider.disabled: true` 会把该提供商排除在目录发现之外。显式 `provider/model` 请求会失败， `defaultModel` / `models[]` 扫描也会跳过它。 - `providerContextCaps` 为各提供商设置 Codex 可见的上下文上限。`contextCapValue` 是仪表盘的默认值， diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 9454aec7f0f..2c11df1cbbd 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -598,6 +598,7 @@ "codex-spark-visibility.test.ts": "codex-integration", "codex-sqlite-home.test.ts": "codex-integration", "codex-sync-api.test.ts": "codex-integration", + "codex-sync-new-model-policy.test.ts": "codex-integration", "codex-sync-response.test.ts": "codex-integration", "codex-tool-mode.test.ts": "codex-integration", "codex-transition-state-adoption.test.ts": "codex-integration", @@ -1080,6 +1081,7 @@ "management-metrics-export.test.ts": "server", "management-model-roster-gather-race.test.ts": "server", "management-model-roster.test.ts": "server", + "model-export-new-model-policy.test.ts": "server", "management-origin-tls.test.ts": "server", "management-provider-compat-carry.test.ts": "server", "management-provider-hide-raw-reasoning.test.ts": "server", @@ -1112,6 +1114,7 @@ "model-cache.test.ts": "codex-integration", "model-costs-management-api.test.ts": "server", "model-discovery-management-api.test.ts": "server", + "server-new-model-policy-arrival.test.ts": "server", "model-display-names-management-api.test.ts": "codex-integration", "model-metadata-sync.test.ts": "codex-integration", "model-pinned-effort-config.test.ts": "config", @@ -1159,6 +1162,7 @@ "native-profile-startup.test.ts": "codex-integration", "native-profile-store.test.ts": "codex-integration", "new-model-policy.test.ts": "providers", + "new-model-policy-runtime.test.ts": "providers", "nous-oauth-live.test.ts": "providers", "nous-oauth.test.ts": "providers", "novita-provider.test.ts": "providers", diff --git a/src/codex/catalog/retained-sync.ts b/src/codex/catalog/retained-sync.ts index 9e2ab48c989..3078e4d8a60 100644 --- a/src/codex/catalog/retained-sync.ts +++ b/src/codex/catalog/retained-sync.ts @@ -7,6 +7,7 @@ import { legacyCustomModelCatalogSlugs } from "../custom-model-catalog-migration import { getCodexHome } from "../paths"; import type { OcxConfig } from "../../types"; import { pendingModelSelectionProviders } from "../../providers/initial-model-selection"; +import { captureModelDiscoveryBaseline, finalizeModelDiscovery } from "../../providers/new-model-policy-runtime"; import { OPENAI_CODEX_PROVIDER_ID } from "../../providers/openai-tiers"; import { providerCodexAccountMode } from "../../providers/registry"; import { COMBO_NAMESPACE } from "../../combos"; @@ -602,10 +603,13 @@ export async function syncCatalogModels( evidence: retainedCatalogSyncEvidence(config, preflightRead.catalogPath, preflightRead.catalog), processEvidence: retainedCatalogProcessEvidence(), }; + const discoveryBaseline = captureModelDiscoveryBaseline(config); + const providerContentRevisions = new Map(); const [goModels, modelEntitlements] = await Promise.all([ gatherRoutedModels(config, { comboOmissions, providerModelOutcomes, + providerContentRevisions, }), resolveAdmittedCodexModelEntitlements(config), ]); @@ -629,6 +633,11 @@ export async function syncCatalogModels( const current = revalidateRetainedCatalogSync(config, prepared); if (current === null) return null; if (!isCodexModelEntitlementSnapshotCurrent(modelEntitlements)) return null; + // Revalidate before adopting discovery: its scoped config mutation would otherwise + // invalidate our own evidence. K -> C is the catalog/config lock order. + if (!finalizeModelDiscovery(config, discoveryBaseline, goModels, + providerModelOutcomes.filter(outcome => outcome.state === "authoritative").map(outcome => outcome.provider), + providerContentRevisions)) return null; return writeRetainedCatalogSync({ config, goModels, diff --git a/src/config.ts b/src/config.ts index 37191f58c72..566a85be8b6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -11,6 +11,7 @@ import { import { refreshConfigDerivedRegistries } from "./config/derived-registries"; import { clearPendingConfigDeletions, + recordFileBackedConfig, projectConfigRebaseProvenance, } from "./config/rebase-provenance"; import { getConfigDir, getConfigPath, hardenConfigDir } from "./config/paths"; @@ -249,7 +250,7 @@ export function loadConfig(): OcxConfig { warnDegradedCatalogAutoRefresh(parsed); warnDegradedCodexPool(parsed); warnDegradedCredentialGroups(parsed); - return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)); + return recordFileBackedConfig(withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)), configPath); } // Only object-shaped configs are repairable. Spreading another JSON value // into defaults can manufacture a valid config and bypass the invalid-file @@ -285,7 +286,7 @@ export function loadConfig(): OcxConfig { warnDegradedCatalogAutoRefresh(parsed); warnDegradedCodexPool(parsed); warnDegradedCredentialGroups(parsed); - return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)); + return recordFileBackedConfig(withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)), configPath); } // Still failing, but if every complaint is about one or more named entries // in an independent section, drop exactly those and keep the rest. Falling @@ -315,7 +316,7 @@ export function loadConfig(): OcxConfig { warnDegradedCatalogAutoRefresh(parsed); warnDegradedCodexPool(parsed); warnDegradedCredentialGroups(parsed); - return withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)); + return recordFileBackedConfig(withRefreshedCostOverlays(normalizeClaudeSubagentEffort(normalizeNativeSubagentSync(config, parsed), parsed)), configPath); } } // Merge couldn't fix it — truly broken config diff --git a/src/config/live-reconcile.ts b/src/config/live-reconcile.ts index 085b2529a8f..d33aa9746b0 100644 --- a/src/config/live-reconcile.ts +++ b/src/config/live-reconcile.ts @@ -41,6 +41,24 @@ const claudeCodeBaseline = new WeakMap(); * reconciliation paths below. */ const liveConfigBaseline = new WeakMap(); + +/** + * Adopt a committed discovery decision and its merge baseline as one synchronous step. + * Visibility writers assign and save synchronously; any future writer yielding between + * those steps must preserve pending edits through a three-way merge here. + */ +export function adoptPersistedModelDiscovery( + config: OcxConfig, + persisted: Pick, +): void { + config.modelDiscovery = structuredClone(persisted.modelDiscovery); + config.disabledModels = structuredClone(persisted.disabledModels); + const baseline = liveConfigBaseline.get(config); + if (baseline) { + baseline.modelDiscovery = structuredClone(persisted.modelDiscovery); + baseline.disabledModels = structuredClone(persisted.disabledModels); + } +} /** * The live config retains the address of the socket Bun actually opened, while * this map retains the operator's desired address for the next process start. diff --git a/src/config/rebase-provenance.ts b/src/config/rebase-provenance.ts index b9a28eca0ce..96a35fd181b 100644 --- a/src/config/rebase-provenance.ts +++ b/src/config/rebase-provenance.ts @@ -1,5 +1,17 @@ import type { OcxConfig } from "../types"; +// Object provenance survives disk drift without adding serializable config fields. +const fileBackedConfigs = new WeakMap(); + +export function recordFileBackedConfig(config: OcxConfig, path: string): OcxConfig { + fileBackedConfigs.set(config, path); + return config; +} + +export function fileBackedConfigPath(config: OcxConfig): string | undefined { + return fileBackedConfigs.get(config); +} + const pendingTopLevelDeletions = new WeakMap>(); const pendingObjectChildDeletions = new WeakMap>>(); export const CONFIG_REBASE_PROVENANCE_KEY = "configRebaseProvenance"; diff --git a/src/providers/new-model-policy-runtime.ts b/src/providers/new-model-policy-runtime.ts new file mode 100644 index 00000000000..57e1c86d45a --- /dev/null +++ b/src/providers/new-model-policy-runtime.ts @@ -0,0 +1,72 @@ +import { isDeepStrictEqual } from "node:util"; +import { mutatePersistedConfig, readConfigDiagnostics, validateConfigCandidate } from "../config"; +import { fileBackedConfigPath } from "../config/rebase-provenance"; +import { getConfigPath } from "../config/paths"; +import { observeModelCacheRevision } from "../codex/model-cache"; +import { adoptPersistedModelDiscovery } from "../config/live-reconcile"; +import type { OcxConfig } from "../types"; +import { reconcileSuccessfulModelDiscoveries } from "./new-model-policy"; + +function inventory(config: OcxConfig): unknown { + const validated = validateConfigCandidate(config); + if (!validated.ok) return null; + const providers = Object.fromEntries(Object.entries(validated.config.providers).map(([name, provider]) => [name, { + ...provider, + // Initial selection may finish during this discovery, but a re-registration is distinct. + initialModelSelection: provider.initialModelSelection?.registrationId, + }])); + // Compare normalized persisted data, not injected fetch functions. Never log this identity. + return JSON.parse(JSON.stringify({ providers, combos: validated.config.combos, + policy: validated.config.modelDiscovery?.newModelPolicy })); +} + +export function captureModelDiscoveryBaseline(config: OcxConfig) { + const identity = inventory(config); + const disk = readConfigDiagnostics(); + return { identity, persisted: identity !== null && disk.source === "file" + && isDeepStrictEqual(identity, inventory(disk.config)) }; +} + +/** Publish new-arrival choices before returning discovery rows to any visibility consumer. */ +export function finalizeModelDiscovery( + config: OcxConfig, + baseline: ReturnType, + models: Iterable<{ provider: string; id: string; custom?: boolean }>, + authoritativeProviders: string[], + revisions: Map, +): boolean { + const filePath = fileBackedConfigPath(config); + const rows = [...models]; + const now = new Date().toISOString(); + const current = () => (filePath === undefined || filePath === getConfigPath()) + && isDeepStrictEqual(baseline.identity, inventory(config)) + && [...revisions].every(([provider, revision]) => observeModelCacheRevision(provider) === revision); + if (!current()) return false; + const reconcile = (target: OcxConfig) => reconcileSuccessfulModelDiscoveries({ + config: target, models: rows, authoritativeProviders, now, mode: "discovery", + }); + if (!baseline.persisted && filePath === undefined) { + // Standalone/synthetic callers may project their own config, never overwrite another home. + reconcile(config); + return true; + } + // A file-loaded instance remains file-backed after inventory drift. The locked + // freshness gate below refuses stale rows without creating unsaved live disables. + try { + const result = mutatePersistedConfig(fresh => { + if (!current() || !isDeepStrictEqual(baseline.identity, inventory(fresh))) { + return { changed: false, value: null }; + } + // Re-evaluate against the latest baseline and manual choices under the mutation lock. + // A second caller then adopts a first caller's result rather than disabling an ID twice. + const changed = reconcile(fresh); + return { changed, value: { modelDiscovery: fresh.modelDiscovery, disabledModels: fresh.disabledModels } }; + }); + if (result.status === "unavailable" || !result.value) return false; + adoptPersistedModelDiscovery(config, result.value); + return true; + } catch { + // Do not publish an unrecorded arrival when persistence failed. The caller retries discovery. + return false; + } +} diff --git a/src/providers/new-model-policy.ts b/src/providers/new-model-policy.ts index b9ac16af377..f1a07f5311e 100644 --- a/src/providers/new-model-policy.ts +++ b/src/providers/new-model-policy.ts @@ -8,6 +8,17 @@ export const MAX_RECENT_ARRIVALS_PER_PROVIDER = 50; export type KnownModelBaseline = NonNullable["knownModels"]>[string]; export type NewModelPolicy = "on" | "off"; +/** + * Which side of the reconciliation is running. + * + * `converge` (the default) is the catalog convergence cycle: it advances the removal grace and + * retires ids missing for {@link MODEL_REMOVAL_GRACE_FETCHES} cycles. `discovery` is the HTTP read + * side (GET /v1/models), which is polled far more often than convergence runs; it absorbs genuine + * arrivals and reappearances but never advances removal accounting, so a transiently absent id + * cannot be retired by repeated polls. + */ +export type NewModelPolicyMode = "converge" | "discovery"; + export interface NewModelPolicyResult { newIds: string[]; nextBaseline: KnownModelBaseline; @@ -44,6 +55,8 @@ export function applyNewModelPolicy(options: { policy: NewModelPolicy; hasSelectedModels?: boolean; now: string; + /** See {@link NewModelPolicyMode}; defaults to `converge`. */ + mode?: NewModelPolicyMode; }): NewModelPolicyResult { const discovered = [...new Set(options.discoveredIds)].sort(); const prior = options.baseline; @@ -60,8 +73,17 @@ export function applyNewModelPolicy(options: { const seen = new Set(discovered); const newIds = discovered.filter(id => !active.has(id) && !removed.has(id)); const missing: Record = {}; + // Read-side discovery must not advance the removal grace: a per-poll increment would retire a + // transiently absent id after three polls. Absence keeps its recorded count (and a reappearance + // clears it) instead of moving toward `removed`. + const discovery = options.mode === "discovery"; for (const id of active) { if (seen.has(id)) continue; + if (discovery) { + const priorCount = prior.missing?.[id]; + if (priorCount !== undefined) missing[id] = priorCount; + continue; + } const count = (prior.missing?.[id] ?? 0) + 1; if (count >= MODEL_REMOVAL_GRACE_FETCHES) { active.delete(id); @@ -99,6 +121,8 @@ export function reconcileSuccessfulModelDiscoveries(options: { models: Iterable<{ provider: string; id: string; custom?: boolean }>; authoritativeProviders: Iterable; now: string; + /** See {@link NewModelPolicyMode}; defaults to `converge`. */ + mode?: NewModelPolicyMode; }): boolean { const byProvider = new Map(); for (const model of options.models) { @@ -118,6 +142,7 @@ export function reconcileSuccessfulModelDiscoveries(options: { policy: effectiveNewModelPolicy(options.config, provider), hasSelectedModels: (configured.selectedModels?.length ?? 0) > 0, now: options.now, + mode: options.mode, }); if (result.overflow) continue; const priorBaseline = known[provider]; diff --git a/src/server/management/model-rows.ts b/src/server/management/model-rows.ts index 28a24d4690a..971b91fc72a 100644 --- a/src/server/management/model-rows.ts +++ b/src/server/management/model-rows.ts @@ -12,6 +12,7 @@ import type { CatalogModel } from "../../codex/catalog"; import { observeModelCacheRevision } from "../../codex/model-cache"; import { captureExportConfigAdmission, + detachedConfigSnapshot, isExportConfigAdmissionCurrent, type ExportConfigAdmission, } from "../../config/admitted-identity"; @@ -37,6 +38,9 @@ import type { OcxConfig } from "../../types"; import { enrichProviderFromRegistry } from "../../providers/derive"; import { ensureCodexEntitlementFreshness } from "../../codex/model-entitlements"; import { fetchAllModels } from "./shared"; +import { CatalogGatherBusyError } from "../../codex/catalog/routed-gather"; +import { captureModelDiscoveryBaseline, finalizeModelDiscovery } from "../../providers/new-model-policy-runtime"; +import { reconcileSuccessfulModelDiscoveries } from "../../providers/new-model-policy"; import { initialModelSelectionPending, pendingModelSelectionProviders } from "../../providers/initial-model-selection"; import { catalogFastRowEligible, fastRowId } from "../fast-row"; import { knownEffortRowIds } from "../effort-row"; @@ -471,8 +475,10 @@ export async function loadExportModels( // The configuration this pass will use from beginning to end, proved to be the one on disk. // Without it there is nothing that may be retained, and the caller still gets its rows: only the // preview authority is withheld. - const admission = captureExportConfigAdmission(config); - const admitted = admission?.config ?? config; + let admission = captureExportConfigAdmission(config); + let admitted = admission?.config ?? config; + const discoveryBaseline = models === undefined ? captureModelDiscoveryBaseline(admitted) : null; + const outcomes: Array<{ provider: string; state: "authoritative" | "degraded" }> = []; // The gather stamps each provider as it chooses its rows, so the roster and the revisions that // vouch for it come from the same moment. Sampling afterwards would let a concurrent flight's // publication be recorded against rows it never produced. @@ -483,10 +489,34 @@ export async function loadExportModels( // had, and runs alongside as it did inside the projection. const roster = models === undefined ? (await Promise.all([ - (await import("../../codex/catalog")).gatherRoutedModels(admitted, { providerContentRevisions: gathered }), + (await import("../../codex/catalog")).gatherRoutedModels(admitted, { + providerContentRevisions: gathered, providerModelOutcomes: outcomes, + }), ensureCodexEntitlementFreshness(admitted, { waitMs: 3_000 }), ]))[0] : models; + if (discoveryBaseline !== null) { + const authoritative = outcomes.filter(outcome => outcome.state === "authoritative").map(outcome => outcome.provider); + if (admission !== null && isExportConfigAdmissionCurrent(admission, config) + && [...gathered].every(([provider, revision]) => observeModelCacheRevision(provider) === revision)) { + if (!finalizeModelDiscovery(config, discoveryBaseline, roster, authoritative, gathered)) { + lastExportSnapshot = null; + throw new CatalogGatherBusyError(); + } + // Only our scoped discovery decision moved: bind the projection to the committed state. + admission = captureExportConfigAdmission(config); + admitted = admission?.config ?? config; + } else { + // Preserve the old-response contract after a race, without persisting stale evidence or + // retaining it as a current preview. Apply visibility policy only to a detached projection. + const projection = detachedConfigSnapshot(admitted); + if (projection === null) throw new CatalogGatherBusyError(); + reconcileSuccessfulModelDiscoveries({ config: projection, models: roster, + authoritativeProviders: authoritative, now: new Date().toISOString(), mode: "discovery" }); + admitted = projection; + admission = null; + } + } const rows = await listManagementModelRows(admitted, { models: roster }); // Management deliberately lists the full roster so hidden models can be enabled. // A client picker must also honor the provider selection, not just its blocklist. diff --git a/src/server/management/shared.ts b/src/server/management/shared.ts index d69adc83d47..2244d510beb 100644 --- a/src/server/management/shared.ts +++ b/src/server/management/shared.ts @@ -1,5 +1,7 @@ import { randomUUID } from "node:crypto"; import { captureInitialSelectionBaseline, finalizeInitialModelSelection } from "../../providers/initial-model-selection-runtime"; +import { captureModelDiscoveryBaseline, finalizeModelDiscovery } from "../../providers/new-model-policy-runtime"; +import { CatalogGatherBusyError } from "../../codex/catalog/routed-gather"; import { initialModelSelectionPending } from "../../providers/initial-model-selection"; import { readFileSync } from "node:fs"; import type { CatalogModel } from "../../codex/catalog"; @@ -269,14 +271,19 @@ export async function fetchAllModels( ): Promise { const { gatherRoutedModels } = await import("../../codex/catalog"); const baseline = captureInitialSelectionBaseline(config); - if (!baseline) return gatherRoutedModels(config, providerContentRevisions ? { providerContentRevisions } : undefined); + const discoveryBaseline = captureModelDiscoveryBaseline(config); + const revisions = providerContentRevisions ?? new Map(); const outcomes: Array<{ provider: string; state: "authoritative" | "degraded" }> = []; const models = await gatherRoutedModels(config, { providerModelOutcomes: outcomes, - ...(providerContentRevisions ? { providerContentRevisions } : {}), + providerContentRevisions: revisions, }); finalizeInitialModelSelection(config, baseline, uniqueCatalogModelsForPublicList(models), outcomes.filter(outcome => outcome.state === "authoritative").map(outcome => outcome.provider)); + if (!finalizeModelDiscovery(config, discoveryBaseline, models, + outcomes.filter(outcome => outcome.state === "authoritative").map(outcome => outcome.provider), revisions)) { + throw new CatalogGatherBusyError(); + } return models; } diff --git a/structure/catalog.md b/structure/catalog.md index 2ce2a2f73fa..f26cad389c5 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -91,7 +91,7 @@ this rule, and uninstall keeps the [manifest validation and residual reporting c Cache invalidation reports an unchanged derived cache separately from a failed rewrite. `ocx sync-cache` treats identical bytes as a successful no-op, preserving the cache mtime and avoiding a needless app-server restart; malformed catalogs and write failures remain errors. -`src/codex/catalog/model-visibility.ts` also excludes models owned by disabled providers, including custom rows. `src/codex/catalog/routed-gather.ts` does not inherit provider configuration into custom rows while that provider is disabled. +`src/codex/catalog/model-visibility.ts` excludes disabled providers, including custom rows; `src/codex/catalog/routed-gather.ts` does not inherit their configuration into custom rows. HTTP discovery, startup/explicit sync (`src/codex/catalog/retained-sync.ts`), and client exports (`src/server/management/model-rows.ts`) apply new-model policy before publication. `src/providers/new-model-policy-runtime.ts` commits authoritative arrivals and automatic disables together under the config mutation lock, without rewriting an unchanged roster. It rechecks provider inventory and cache revisions and refuses publication on failed persistence. Retained sync reconciles after catalog evidence revalidation, preserving K-to-C lock order. Exports recapture their snapshot after committing discovery; superseded gathers only project policy onto a detached copy and retain no preview. Supplied rosters and read-only previews do not reconcile. Synthetic configurations only project their own state. File-backed configs retain their load provenance when their inventory diverges from disk; the locked fresh-config comparison refuses stale publication without mutating live policy or its merge baseline. A reload or explicit adoption is required before that inventory can publish again. Discovery absorbs arrivals/reappearances without advancing removal grace; convergence retains removal accounting. Bootstrap, custom rows, explicit selections, and degraded discovery retain the pure policy rules. Coverage: `tests/server/server-new-model-policy-arrival.test.ts` (HTTP), `tests/codex-integration/codex-sync-new-model-policy.test.ts` (startup/sync), `tests/server/model-export-new-model-policy.test.ts` (exports/previews), and `tests/providers/new-model-policy-runtime.test.ts` (persistence/concurrency). On the default `opencodex-catalog.json` path, sync deliberately uses two catalog sources: Codex's bundled catalog supplies a current native entry template, while the actual on-disk catalog supplies diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 790b0b5796b..a1c31f165cb 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -132,9 +132,11 @@ writes the provider cache. With no usable snapshot the request is refused, which process and equally a snapshot retired because the configuration or the provider cache moved. The roster is gathered and projected from a detached copy of the configuration (`src/config/admitted-identity.ts`) taken before the gather, so an edit that lands mid-load changes -neither half of the result; the same admission is revalidated against the resident object and the -configuration file before anything is retained, so such an edit leaves no snapshot rather than one -recorded under a state its rows never had. The identity a caller carries between a preview and the +neither half of the result. `src/server/management/model-rows.ts` revalidates the admission and gathered +cache revisions before committing the [new-arrival policy](../catalog.md#shared-catalog), then captures +the resulting configuration for projection and retention. A superseded gather applies policy only +to its detached projection and retains no snapshot; a failed discovery commit refuses the export. +The identity a caller carries between a preview and the mutation that confirms it is process-local and opaque, and describes nothing about the configuration. The Integrations collection read populates one when discovery succeeds and the configuration can be diff --git a/structure/config.md b/structure/config.md index 21af76a0260..41b5a4e4d74 100644 --- a/structure/config.md +++ b/structure/config.md @@ -57,7 +57,7 @@ and fresh-location examples. Diagnostics do not introduce a fallback or alter fi `src/config/persisted-mutation.ts` owns schema-valid on-disk mutations under the shared lock. It rechecks the file before committing, retries a changed snapshot up to three times, and returns unavailable for missing, invalid, or persistently conflicting config. Its one-shot -test seam and the mutation types remain re-exported through `src/config.ts`. +test seam and the mutation types remain re-exported through `src/config.ts`. Successful, repaired, and salvaged file loads record their source path in the private WeakMap owned by `src/config/rebase-provenance.ts`; defaults and synthetic objects do not acquire it. This metadata is never serialized. Inventory drift cannot turn a file-backed instance into a synthetic discovery caller, and changing the resolved config home refuses publication. `src/config/paths.ts` is the single owner of `OPENCODEX_HOME` expansion and resolution. It exposes the config directory and `config.json` path and retains the existing cache rule: a relative home is @@ -75,7 +75,7 @@ process-wide temp sequence, symlink target resolution, no-follow directory-entry externally writable integration directories, real-home test guard, owner manifest, Windows ACL hardening, scrub-before-unlink failure path, and explicit residual-temp errors. A caller must not replace it with a local temp-and-rename shortcut. Publication failures in -`src/config/persist-unlocked.ts` and `src/config/live-reconcile.ts` follow the [publication-aware rollback contract](gui-and-management-api.md#durable-provider-patch). +`src/config/persist-unlocked.ts` and `src/config/live-reconcile.ts` follow the [publication-aware rollback contract](gui-and-management-api.md#durable-provider-patch). `src/config/live-reconcile.ts` adopts committed model discovery and disabled selectors together with their scoped live merge baselines. A later manual enable therefore removes the automatic disable instead of having a stale three-way merge restore it. Unrelated live config baselines are not advanced by that adoption. Windows hardening there is applied once per write, not once per harden call. Both calls stay `required: true` and still fail the write closed, but the pre-rename call resolves through the diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 13da2971e76..019e62da729 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -254,7 +254,7 @@ per-request first-party callback reads that live object; a failed write leaves i | Windows tray | `GET/POST /api/windows-tray` controls an owned, per-user HKCU login tray. The tray delegates fixed actions to the CLI and is never a proxy supervisor or restart-protection signal. | | Updates | `GET /api/update/check`, `POST /api/update/run`, and `GET /api/update/status` own dashboard self-update state. A launched worker PID is persisted in `update-job.json`; dead PIDs recover immediately, while legacy active records without a PID recover only after ten minutes. Live PIDs remain exclusive regardless of record age. `GET /api/update/check` and `POST /api/update/run` await one per-channel asynchronous registry lookup and write successful results through to the package cache; run passes that result to the job starter. `GET /api/update/badge` only reads the cache and reports unknown after 40 hours, on missing cache, or on channel mismatch. The badge links to the update surface rather than gating other actions. `GET /api/update/badge?surface=desktop&session=` projects only that process-local Tauri session; missing or expired state is unknown and never falls back to the package cache. `POST /api/update/desktop-snapshot` is a 1 KiB bounded display-state mutation with no install permission. It accepts the existing admin-token principal or a dedicated single-use, ten-second snapshot capability bound to nonce, method, path, PID, port and the SHA-256 digest of the exact body bytes; that capability cannot authorize another route. | | Providers | Create/update/delete ordinary provider configs and enrich registry metadata. A `POST /api/providers` overwrite of an existing name keeps the eight operator compatibility settings (`PROVIDER_COMPAT_CARRY_FIELDS` in `src/server/management/provider-overwrite-carry.ts`) and the stored key pool only while the destination (adapter, normalized base URL, auth mode when named) is unchanged. It keeps omitted `hideRawReasoning` on every overwrite of the same name, including destination moves, because it is a display policy; a submitted boolean wins. It never merges the rest of the old row. `PATCH` is a field mask and keeps every field it does not name; `hideRawReasoning` accepts a boolean or `null` to clear it. The reserved `openai` card exposes Pool(default)/Direct account mode; `openai-apikey` remains the separate API route. | -| Models | Fetch routed model lists, disabled model visibility, and catalog-facing ids. New non-OAuth registration holds exposure until authoritative discovery; 20 or more distinct switch rows start OFF without disabling the provider. Pending rows cannot accept visibility changes. | +| Models | Fetch routed model lists, disabled model visibility, and catalog-facing ids. New non-OAuth registration holds exposure until authoritative discovery; 20 or more distinct switch rows start OFF without disabling the provider. Pending rows cannot accept visibility changes. Ordinary model reads also apply the [new-arrival catalog contract](catalog.md#shared-catalog) before returning models. With a matching persisted configuration, discovery records the baseline, NEW badges, and automatic disables together; HTTP listing does not wait for a separate Codex sync. | | OAuth | Login/status/logout for OAuth-backed providers, plus multiauth account management: `GET /api/oauth/accounts`, `PUT /api/oauth/accounts/active`, `PUT /api/oauth/accounts/alias`, `PUT /api/oauth/accounts/pause`, and `DELETE /api/oauth/accounts` list masked accounts per provider, switch the active one, edit its display-only alias, pause/resume a generic OAuth account, and remove one. Paused generic OAuth accounts are excluded from request selection, 429 failover, and proactive token refresh; pausing an active account selects the next usable account when available, and resuming an account restores an active selection if the current one remains paused. With no unpaused account, requests return 403 rather than a login error. Kiro account-list rows include the current automatic-selection projection and closed exclusion reason; an active singleton may still send. The login flow itself is `GET /api/oauth/providers`, `POST /api/oauth/login`, `POST /api/oauth/login/code`, `POST /api/oauth/login/cancel`, `POST /api/oauth/logout`, and `GET /api/oauth/status`; pool controls are `GET/PUT/PATCH /api/oauth/accounts/pool` and `POST /api/oauth/accounts/clear-cooldown`. Login accepts `addAccount: true` to force a fresh browser identity. Meta Muse login start and manual-code continuation require the server-resolved `gui-session` principal before credential acquisition or code submission (including reauth); see the [provider contract](providers-and-adapters.md). Device flows return a structured `deviceCode`; the GUI highlights and copies it before the user opens the verification page. | | Key providers | `GET /api/key-providers` exposes API-key provider presets for setup and dashboard flows, and `GET/POST/DELETE /api/keys` owns the proxy's own admission keys. Machine links live in `src/server/management/link-routes.ts`: dashboard sessions reach `GET /api/link/candidates`, `POST /api/link/probe`, `POST /api/link/confirm-host`, `POST /api/link/apply` and `POST /api/link/join`, meaning a paired session or, on a standalone runtime, the current loopback-issued session on trusted loopback ingress; join also refuses a runtime that is not standalone (`409 standalone_required`) or not listening on its configured port (`409 join_port_mismatch`) before any SSH, then restarts this runtime as a client; `GET /api/link/status` reports `joinAvailable` to GUI-session callers so the dashboard enables the Child role only when a join can succeed; `GET /api/link/status` and `DELETE /api/link/{id}` also accept the admin token on a trusted loopback ingress; `POST /api/link/issue` accepts only that admin token. Tailscale-identity sessions are refused on every link route. See [Remote Link](remote-link.md). Multi-key pool per key-auth provider: `GET /api/providers/keys`, `POST /api/providers/keys`, `PUT /api/providers/keys/active`, `PUT /api/providers/keys/alias`, `DELETE /api/providers/keys` masked list, add (upsert + activate), switch, rename, and remove keys. `provider.apiKey` always mirrors the active pool entry so routing stays single-key. | | OpenAI account mode | Report one OpenAI Codex card with Pool/Direct controls and one API-key card. Mode PATCH persists live without restart or catalog identity changes; Pool owns account/quota controls and Direct uses caller/main login only. Main-account DTOs report real credential presence and terminal `needsReauth` state instead of treating missing/invalid native auth as an unknown quota. Selection order has its own route: `PUT /api/codex-auth/accounts/priority` takes `{ id, priority }`, where `priority` is an integer -100..100 or `null` to restore the default, accepts `__main__`, 404s an unknown id, and echoes the stored value. Re-ordering never clears thread affinity, so the response carries no `appliesImmediately`, but it does release any pin — see [`openai-tiers.md`](providers/openai-tiers.md) for why. `PUT /api/codex-auth/active` with a null id releases one too, but that drops the operator's account selection along with it, so this route is the only operator-facing way to clear a pin while leaving the selected account in place. `GET /api/codex-auth/active` reports `pinned`, true only while the manually selected account is still the effective active one, plus `pinnedAccountId`, which names the pinned account whether or not it is the active one. Surfaces should render `pinnedAccountId`: under round-robin and fill-first the pin caps the tier ceiling at its own tier while the strategy cursor moves freely inside that tier, so `pinned` goes false on a sibling's turn even though the pin is still suppressing every higher tier — which is why the dashboard badges `pinnedAccountId` and the GUI controller tracks only the id. `pinned` answers the narrower question of whether routing is *currently* on the operator's choice; no surface in this repo asks it, and a new one almost certainly wants the id instead. | diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 9e9b284e68b..7099a226f97 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -215,6 +215,7 @@ The image/video loop bounds each hidden iteration before replay or fulfillment; [media iteration retention](transports/inventory.md#media-iteration-retention). Live model discovery is bounded and registry-driven through `src/providers/model-discovery.ts`. +Ordinary discovery records the new-arrival policy before exposing its rows through the shared management fetcher; see the [catalog contract](catalog.md#shared-catalog). Only authoritative provider results advance the baseline, and manual choices survive subsequent refreshes. Custom providers keep the conventional `${baseUrl}/models` request, normalized by `providerModelsUrl` the same way `openaiChatCompletionsUrl` normalizes the send path: outer whitespace and trailing slashes are trimmed and an already-pasted `/models` is not doubled, so a diff --git a/structure/runtime.md b/structure/runtime.md index aba5f201412..b9681900ad1 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -28,7 +28,7 @@ Native steering follows [the shared WebSocket contract](transports/streaming-hea Responses admission and finalization are composed through the [core module ownership](transports/responses.md#core-module-ownership). Kiro's optional account-load admission is process-local and request-owned; its slot ends with the response body or cancellation. Other providers retain their admission path. -Catalog HTTP acquisition follows the [proxy-routing contract](catalog.md#remote-catalog-http-proxy-routing). CLI model lists expose shared catalog estimates as `price`; `models price` retains the saved override in `cost` and adds `effectiveCost`. Explicit zero overrides win; unknown prices remain null, and automatic defaults are never persisted to `modelCosts`. Covered by `tests/cli/cli-models.test.ts` and `tests/cli/cli-models-price.test.ts`. +Catalog HTTP acquisition follows the [proxy-routing contract](catalog.md#remote-catalog-http-proxy-routing). CLI model lists expose shared catalog estimates as `price`; `models price` retains the saved override in `cost` and adds `effectiveCost`. Explicit zero overrides win; unknown prices remain null, and automatic defaults are never persisted to `modelCosts`. Covered by `tests/cli/cli-models.test.ts` and `tests/cli/cli-models-price.test.ts`. Ordinary model discovery, startup sync, and explicit sync apply the [new-arrival catalog contract](catalog.md#shared-catalog) before publishing rows. The scoped persisted decision also updates the live config merge baseline, so subsequent visibility writes preserve manual re-enablement. A file-loaded inventory that differs from disk refuses publication without adding unsaved live disables; its load provenance follows the [config contract](config.md). OAuth refresh coordination follows the [refresh-lock identity contract](catalog.md#accounts-namespaces-and-pool-rotation): a fresh unreadable lock remains held, and release requires matching descriptor identity. A failed path-identity probe preserves the refresh callback outcome. Cooperating lock metadata changes serialize through the existing SQLite mutation transaction; release keeps the descriptor open through identity comparison and any unlink, then closes it. Failed metadata writes remove only a matching owned path after successful coordination; unknown identity, failed probes or unavailable coordination retain the path for stale recovery. Async refresh work holds no metadata transaction. diff --git a/tests/codex-integration/codex-sync-new-model-policy.test.ts b/tests/codex-integration/codex-sync-new-model-policy.test.ts new file mode 100644 index 00000000000..5dcc9b481ad --- /dev/null +++ b/tests/codex-integration/codex-sync-new-model-policy.test.ts @@ -0,0 +1,236 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { loadConfig, saveConfig } from "../../src/config"; +import { setPersistedConfigMutationBeforeCommitForTests } from "../../src/config/persisted-mutation"; +import { clearModelCache } from "../../src/codex/model-cache"; +import { readCodexCatalogPath } from "../../src/codex/catalog/parsing"; +import { refreshCodexModelCatalog } from "../../src/codex/refresh"; +import { syncModelsToCodex } from "../../src/codex/sync"; +import { syncCodexOnStartIfEnabled } from "../../src/codex/desired-state"; +import type { OcxConfig } from "../../src/types"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; +import { createTestCaseLifecycle } from "../helpers/test-sandbox-cleanup"; +import { SERVER_BUDGET_MS } from "../helpers/test-budget"; + +/** + * Startup and explicit `syncModelsToCodex` reach the catalog writer through + * `refreshCodexModelCatalog -> syncCatalogModels` rather than the `fetchAllModels` refresh. + * These cases pin the contract for that path: a sync-discovered arrival obeys the persisted + * `newModelPolicy`, is absorbed into the persisted `knownModels` / `recentArrivals` / + * `disabledModels`, the provider roster is refetched on every sync, a manual re-enable + * survives because the baseline absorbed the arrival, and a failed arrival-persistence + * commit publishes no catalog change. + * + * The real startup wrapper, the real `syncModelsToCodex`, and the real gathering/catalog + * writer run here; only the unrelated injection and admission seams are stubbed, as the + * sibling `codex-sync-api` suite already does. + */ +const provider = "sync-fixture"; +const existing = ["model-a", "model-b"]; +const arrival = "model-c"; +const now = "2026-01-01T00:00:00Z"; + +let home: TempHome; +let lifecycle: ReturnType; +let ids: string[] = []; +let discoveries: string[][] = []; +let upstream: ReturnType; + +beforeEach(() => { + home = createTempHome("ocx-sync-policy-"); + mkdirSync(home.codexHome, { recursive: true }); + lifecycle = createTestCaseLifecycle(); + clearModelCache(provider); +}); + +afterEach(async () => { + try { + await lifecycle.close(); + } finally { + setPersistedConfigMutationBeforeCommitForTests(null); + clearModelCache(provider); + home.remove(); + } +}); + +function startUpstream(): void { + ids = [...existing]; + discoveries = []; + upstream = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: request => { + if (request.method !== "GET" || new URL(request.url).pathname !== "/v1/models") { + return new Response("Unexpected upstream request", { status: 404 }); + } + discoveries.push([...ids]); + return Response.json({ data: ids.map(id => ({ id })) }); + } }); + lifecycle.ownStop(() => upstream.stop(true)); +} + +/** + * Give the real writer a Codex catalog to merge into. + * + * Without a readable source `syncCatalogModels` refuses before it gathers, and the + * case would fail for a fixture reason rather than the behavior under test. A `config.toml` + * pointing at a non-default catalog mirrors the hardened sync suite and keeps the + * seeded file authoritative. + */ +function seedCodexCatalog(): void { + writeFileSync(join(home.codexHome, "config.toml"), 'model_catalog_json = "sync-catalog.json"\n', "utf8"); + writeFileSync(join(home.codexHome, "sync-catalog.json"), `${JSON.stringify({ models: [{ + slug: "gpt-5.5", display_name: "GPT-5.5", description: "native", priority: 0, + visibility: "list", shell_type: "shell_command", comp_hash: "native-comp-hash", + model_messages: { instructions_template: "You are Codex." }, + base_instructions: "You are Codex, a coding agent based on GPT-5.", + supported_reasoning_levels: [{ effort: "medium", description: "m" }], + }] }, null, 2)}\n`); +} + +function persistedConfig(policy: { global: "on" | "off"; local?: "on" | "off" }): OcxConfig { + return { + port: 0, defaultProvider: provider, + providers: { [provider]: { + adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, + apiKey: "fixture-key", allowPrivateNetwork: true, liveModels: true, models: [...existing], + ...(policy.local === undefined ? {} : { newModelPolicy: policy.local }), + } }, + modelDiscovery: { newModelPolicy: policy.global, knownModels: { + [provider]: { ids: [...existing], removed: [], updatedAt: now }, + } }, + }; +} + +const deps = { + // `deps` REPLACES the module defaults (no partial merge), so the real catalog + // refresh must be listed explicitly or the sync silently swallows an undefined + // call and writes no catalog. + refreshCodexModelCatalog, + admitCodexWrite: () => ({ kind: "admitted" as const }), + injectCodexConfig: async () => ({ success: true, message: "injected" }), + currentExternalCodexModelProvider: () => null, +}; + +/** Real startup gate -> real `syncModelsToCodex` -> real catalog writer. */ +async function syncOnStartup(config: OcxConfig): Promise { + const outcome = await syncCodexOnStartIfEnabled( + 10_100, + config, + port => syncModelsToCodex(port, config, null, deps), + ); + expect(outcome.ran).toBe(true); +} + +function catalogSlugs(): string[] { + const catalog = JSON.parse(readFileSync(readCodexCatalogPath(), "utf8")) as { + models?: Array<{ slug?: string }>; + }; + return (catalog.models ?? []).flatMap(entry => typeof entry.slug === "string" ? [entry.slug] : []); +} + +test("policy off: a sync-discovered arrival is hidden, disabled, and recorded", () => lifecycle.run(async () => { + startUpstream(); + seedCodexCatalog(); + const config = persistedConfig({ global: "off" }); + saveConfig(config); + await syncOnStartup(config); + // Fixture sanity: the real writer published the two known rows before the arrival. + expect(catalogSlugs()).toContain(`${provider}/model-a`); + expect(catalogSlugs()).toContain(`${provider}/model-b`); + expect(discoveries.at(-1)).toEqual(existing); + + ids = [...existing, arrival]; + clearModelCache(provider); + const beforeRefetch = discoveries.length; + await syncOnStartup(config); + // The sync refetch proves the arrival came from a fresh roster read, not a stale cache. + expect(discoveries.length).toBeGreaterThan(beforeRefetch); + expect(discoveries.at(-1)).toEqual([...existing, arrival]); + + const after = catalogSlugs(); + expect(after).toContain(`${provider}/model-a`); + expect(after).not.toContain(`${provider}/model-c`); + + const persisted = loadConfig(); + expect(persisted.disabledModels ?? []).toContain(`${provider}/model-c`); + expect(persisted.modelDiscovery?.knownModels?.[provider]?.ids).toEqual([...existing, arrival]); + expect(persisted.modelDiscovery?.recentArrivals?.[provider] ?? []) + .toContainEqual(expect.objectContaining({ id: arrival })); +}), SERVER_BUDGET_MS); + +test("policy on: the same sync-discovered arrival stays visible", () => lifecycle.run(async () => { + startUpstream(); + seedCodexCatalog(); + const config = persistedConfig({ global: "on" }); + saveConfig(config); + + ids = [...existing, arrival]; + clearModelCache(provider); + await syncOnStartup(config); + + expect(discoveries.at(-1)).toEqual([...existing, arrival]); + expect(catalogSlugs()).toContain(`${provider}/model-c`); + const persisted = loadConfig(); + expect(persisted.disabledModels ?? []).not.toContain(`${provider}/model-c`); + expect(persisted.modelDiscovery?.recentArrivals?.[provider] ?? []) + .toContainEqual(expect.objectContaining({ id: arrival })); +}), SERVER_BUDGET_MS); + +test("a manual re-enable holds because the sync absorbed the arrival into the baseline", () => lifecycle.run(async () => { + startUpstream(); + seedCodexCatalog(); + const config = persistedConfig({ global: "off" }); + saveConfig(config); + await syncOnStartup(config); + + ids = [...existing, arrival]; + clearModelCache(provider); + const beforeRefetch = discoveries.length; + await syncOnStartup(config); + expect(discoveries.length).toBeGreaterThan(beforeRefetch); + expect(discoveries.at(-1)).toEqual([...existing, arrival]); + expect(catalogSlugs()).not.toContain(`${provider}/model-c`); + + // Manual re-enable: drop the auto-added disable and sync again. The baseline already + // absorbed model-c, so it is no longer a new arrival and must stay visible. + const reenabled = loadConfig(); + reenabled.disabledModels = (reenabled.disabledModels ?? []).filter(slug => slug !== `${provider}/model-c`); + saveConfig(reenabled); + clearModelCache(provider); + const beforeReenableRefetch = discoveries.length; + await syncOnStartup(reenabled); + + expect(discoveries.length).toBeGreaterThan(beforeReenableRefetch); + expect(discoveries.at(-1)).toEqual([...existing, arrival]); + expect(catalogSlugs()).toContain(`${provider}/model-c`); + expect(loadConfig().disabledModels ?? []).not.toContain(`${provider}/model-c`); +}), SERVER_BUDGET_MS); + +test("an arrival whose persistence fails publishes no catalog change", () => lifecycle.run(async () => { + startUpstream(); + seedCodexCatalog(); + const config = persistedConfig({ global: "off" }); + saveConfig(config); + await syncOnStartup(config); + const catalogBefore = readFileSync(readCodexCatalogPath(), "utf8"); + expect(catalogSlugs()).toContain(`${provider}/model-b`); + + ids = [...existing, arrival]; + clearModelCache(provider); + // The arrival decision commits through the persisted-mutation coordinator. This one-shot + // seam corrupts config.json after that decision but before the freshness revalidation, so + // the commit is unavailable and the catalog must not be republished with the arrival. + let hookRan = false; + const corruptConfig = "{ not-json"; + setPersistedConfigMutationBeforeCommitForTests(() => { + hookRan = true; + writeFileSync(home.path("config.json"), corruptConfig, "utf8"); + }); + await syncOnStartup(config); + + // The seam must have fired, and the refused commit must have left the corrupt bytes in + // place rather than rewriting config.json; a vacuous pass would miss both. + expect(hookRan).toBe(true); + expect(readFileSync(home.path("config.json"), "utf8")).toBe(corruptConfig); + expect(readFileSync(readCodexCatalogPath(), "utf8")).toBe(catalogBefore); + expect(catalogSlugs()).not.toContain(`${provider}/model-c`); +}), SERVER_BUDGET_MS); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index ac3ad1f2837..3bf189236ef 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -607,6 +607,7 @@ "codex-spark-visibility.test.ts": "codex-integration", "codex-sqlite-home.test.ts": "codex-integration", "codex-sync-api.test.ts": "codex-integration", + "codex-sync-new-model-policy.test.ts": "codex-integration", "codex-sync-response.test.ts": "codex-integration", "codex-tool-mode.test.ts": "codex-integration", "codex-transition-state-adoption.test.ts": "codex-integration", @@ -1091,6 +1092,7 @@ "management-metrics-export.test.ts": "server", "management-model-roster-gather-race.test.ts": "server", "management-model-roster.test.ts": "server", + "model-export-new-model-policy.test.ts": "server", "management-origin-tls.test.ts": "server", "management-provider-compat-carry.test.ts": "server", "management-provider-hide-raw-reasoning.test.ts": "server", @@ -1123,6 +1125,7 @@ "model-cache.test.ts": "codex-integration", "model-costs-management-api.test.ts": "server", "model-discovery-management-api.test.ts": "server", + "server-new-model-policy-arrival.test.ts": "server", "model-display-names-management-api.test.ts": "codex-integration", "model-metadata-sync.test.ts": "codex-integration", "model-pinned-effort-config.test.ts": "config", @@ -1170,6 +1173,7 @@ "native-profile-startup.test.ts": "codex-integration", "native-profile-store.test.ts": "codex-integration", "new-model-policy.test.ts": "providers", + "new-model-policy-runtime.test.ts": "providers", "nous-oauth-live.test.ts": "providers", "nous-oauth.test.ts": "providers", "novita-provider.test.ts": "providers", diff --git a/tests/providers/new-model-policy-runtime.test.ts b/tests/providers/new-model-policy-runtime.test.ts new file mode 100644 index 00000000000..65ceb74f9b4 --- /dev/null +++ b/tests/providers/new-model-policy-runtime.test.ts @@ -0,0 +1,203 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { readFileSync, writeFileSync } from "node:fs"; +import { loadConfig, saveConfig } from "../../src/config"; +import { armClaudeCodeBaseline, saveConfigPreservingClaudeCode } from "../../src/config/live-reconcile"; +import { setPersistedConfigMutationBeforeCommitForTests } from "../../src/config/persisted-mutation"; +import { clearModelCache, observeModelCacheRevision, setCached } from "../../src/codex/model-cache"; +import { captureModelDiscoveryBaseline, finalizeModelDiscovery } from "../../src/providers/new-model-policy-runtime"; +import type { OcxConfig } from "../../src/types"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; + +let home: TempHome; +const rows = ["a", "b", "c"].map(id => ({ provider: "vendor", id })); +const stamp = "2026-01-01T00:00:00Z"; +function fixture(): OcxConfig { + return { port: 10100, defaultProvider: "vendor", providers: { vendor: { + adapter: "openai-chat", baseUrl: "https://fixture.invalid/v1", models: ["a", "b"], newModelPolicy: "off", + } }, disabledModels: ["vendor/b"], modelDiscovery: { + knownModels: { vendor: { ids: ["a", "b"], removed: [], updatedAt: stamp } }, + } }; +} +const bytes = () => readFileSync(home.path("config.json"), "utf8"); +function complete(config: OcxConfig, providers = ["vendor"]) { + return finalizeModelDiscovery(config, captureModelDiscoveryBaseline(config), rows, providers, new Map()); +} +beforeEach(() => { home = createTempHome("ocx-arrival-persistence-"); }); +afterEach(() => { + setPersistedConfigMutationBeforeCommitForTests(null); + clearModelCache("vendor"); + home.remove(); +}); + +test("discovery persists arrival and disable together, then preserves a manual enable without rewriting", () => { + saveConfig(fixture()); + const config = loadConfig(); + expect(complete(config)).toBe(true); + expect(loadConfig().disabledModels).toEqual(["vendor/b", "vendor/c"]); + expect(loadConfig().modelDiscovery?.knownModels?.vendor.ids).toEqual(["a", "b", "c"]); + const enabled = loadConfig(); + enabled.disabledModels = ["vendor/b"]; + saveConfig(enabled); + const before = bytes(); + expect(complete(config)).toBe(true); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(bytes()).toBe(before); +}); + +test("a concurrent writer's manual enable and unrelated edit survive mutation rebasing", () => { + saveConfig(fixture()); + const config = loadConfig(); + setPersistedConfigMutationBeforeCommitForTests(() => { + const concurrent = loadConfig(); + concurrent.modelDiscovery!.knownModels!.vendor.ids.push("c"); + concurrent.disabledModels = ["vendor/b"]; + concurrent.defaultModel = "a"; + writeFileSync(home.path("config.json"), JSON.stringify(concurrent)); + }); + expect(complete(config)).toBe(true); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(loadConfig().defaultModel).toBe("a"); +}); + +test("stale discovery cannot update a replaced provider or expose an uncommitted result", () => { + saveConfig(fixture()); + const config = loadConfig(); + const baseline = captureModelDiscoveryBaseline(config); + const replacement = loadConfig(); + replacement.providers.vendor.baseUrl = "https://replacement.invalid/v1"; + saveConfig(replacement); + const before = bytes(); + expect(finalizeModelDiscovery(config, baseline, rows, ["vendor"], new Map())).toBe(false); + expect(bytes()).toBe(before); + expect(config.modelDiscovery?.knownModels?.vendor.ids).toEqual(["a", "b"]); +}); + +test("a superseded cache snapshot is refused before advancing the baseline", () => { + saveConfig(fixture()); + const config = loadConfig(); + const baseline = captureModelDiscoveryBaseline(config); + const revisions = new Map([["vendor", observeModelCacheRevision("vendor")]]); + setCached("vendor", rows); + const before = bytes(); + expect(finalizeModelDiscovery(config, baseline, rows, ["vendor"], revisions)).toBe(false); + expect(bytes()).toBe(before); +}); + +test("degraded discovery never advances the baseline or records arrivals", () => { + saveConfig(fixture()); + const config = loadConfig(); + const before = bytes(); + expect(complete(config, [])).toBe(true); + expect(bytes()).toBe(before); + expect(config.modelDiscovery?.recentArrivals).toBeUndefined(); +}); + +test("a first authoritative discovery bootstraps without hiding the existing catalog", () => { + const initial = fixture(); + delete initial.modelDiscovery; + saveConfig(initial); + const config = loadConfig(); + expect(complete(config)).toBe(true); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(loadConfig().modelDiscovery?.knownModels?.vendor.ids).toEqual(["a", "b", "c"]); +}); + +test("standalone and unrelated synthetic configurations never replace persisted provider state", () => { + const standalone = fixture(); + expect(complete(standalone)).toBe(true); + expect(standalone.disabledModels).toContain("vendor/c"); + const persisted = fixture(); + persisted.providers.vendor.baseUrl = "https://other.invalid/v1"; + saveConfig(persisted); + const before = bytes(); + expect(complete(fixture())).toBe(true); + expect(bytes()).toBe(before); +}); + +test("failed persistence refuses publication and does not advance in-memory discovery", () => { + saveConfig(fixture()); + const config = loadConfig(); + setPersistedConfigMutationBeforeCommitForTests(() => writeFileSync(home.path("config.json"), "invalid")); + expect(complete(config)).toBe(false); + expect(config.modelDiscovery?.knownModels?.vendor.ids).toEqual(["a", "b"]); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(bytes()).toBe("invalid"); +}); + +test("repeated discovery of a partial roster does not write or advance removal grace", () => { + saveConfig(fixture()); + const config = loadConfig(); + const before = bytes(); + for (let poll = 0; poll < 4; poll++) { + expect(finalizeModelDiscovery(config, captureModelDiscoveryBaseline(config), + [{ provider: "vendor", id: "a" }], ["vendor"], new Map())).toBe(true); + } + expect(bytes()).toBe(before); + expect(config.modelDiscovery?.knownModels?.vendor.ids).toEqual(["a", "b"]); + expect(config.modelDiscovery?.knownModels?.vendor.missing).toBeUndefined(); +}); + +test("a stale file-backed discovery preserves another writer's enabled model through an unrelated save", () => { + saveConfig(fixture()); + const config = loadConfig(); + armClaudeCodeBaseline(config); + const concurrent = loadConfig(); + concurrent.providers.vendor.models!.push("c"); + concurrent.modelDiscovery!.knownModels!.vendor.ids.push("c"); + saveConfig(concurrent); + const before = bytes(); + + const published = complete(config); + expect(bytes()).toBe(before); + config.contextCapValue = 320000; + saveConfigPreservingClaudeCode(config); + expect(loadConfig().disabledModels).toEqual(["vendor/b"]); + expect(loadConfig().providers.vendor.models).toEqual(["a", "b", "c"]); + expect(loadConfig().contextCapValue).toBe(320000); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(published).toBe(false); +}); + + +test.each([false, true])("an unarmed file-backed caller rejects stale discovery (detached baseline: %s)", detached => { + saveConfig(fixture()); + const config = loadConfig(); + const concurrent = loadConfig(); + concurrent.providers.vendor.models!.push("c"); + concurrent.modelDiscovery!.knownModels!.vendor.ids.push("c"); + saveConfig(concurrent); + const before = bytes(); + const input = detached ? structuredClone(config) : config; + const baseline = captureModelDiscoveryBaseline(input); + expect(finalizeModelDiscovery(config, baseline, rows, ["vendor"], new Map())).toBe(false); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(config.modelDiscovery?.knownModels?.vendor.ids).toEqual(["a", "b"]); + expect(bytes()).toBe(before); +}); + +test("a file-backed provider replaced before discovery cannot be treated as synthetic", () => { + saveConfig(fixture()); + const config = loadConfig(); + const replacement = loadConfig(); + replacement.providers.vendor.baseUrl = "https://replacement.invalid/v1"; + saveConfig(replacement); + const before = bytes(); + expect(complete(config)).toBe(false); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(bytes()).toBe(before); +}); + +test("file-backed discovery cannot publish to a different config home", () => { + saveConfig(fixture()); + const config = loadConfig(); + const other = createTempHome("ocx-arrival-other-home-"); + try { + saveConfig(fixture()); + const before = readFileSync(other.path("config.json"), "utf8"); + expect(complete(config)).toBe(false); + expect(config.disabledModels).toEqual(["vendor/b"]); + expect(readFileSync(other.path("config.json"), "utf8")).toBe(before); + } finally { + other.remove(); + } +}); diff --git a/tests/providers/new-model-policy.test.ts b/tests/providers/new-model-policy.test.ts index dbf591bec38..06b8fabd927 100644 --- a/tests/providers/new-model-policy.test.ts +++ b/tests/providers/new-model-policy.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { applyNewModelPolicy, reconcileSuccessfulModelDiscoveries } from "../../src/providers/new-model-policy"; +import { + applyNewModelPolicy, + effectiveNewModelPolicy, + MODEL_REMOVAL_GRACE_FETCHES, + reconcileSuccessfulModelDiscoveries, +} from "../../src/providers/new-model-policy"; const now = "2026-08-24T02:11:00Z"; @@ -79,4 +84,151 @@ describe("new-model policy", () => { expect(config.disabledModels).toEqual(["vendor/b"]); expect(config.modelDiscovery!.knownModels!.vendor!.updatedAt).toBe(now); }); + + /** + * GET /v1/models is polled far more often than convergence runs, so the read side reconciles + * from the same roster repeatedly. Advancing the removal grace once per poll would retire a + * transiently absent id after three polls and write config.json each time. Only convergence may + * move an id toward `removed`. + */ + describe("removal grace is advanced by convergence only", () => { + const seedBaseline = { ids: ["a", "b", "c"], removed: [] as string[], updatedAt: "2026-01-01T00:00:00Z" }; + const roster = [{ provider: "vendor", id: "a" }, { provider: "vendor", id: "b" }]; + + test("read-side reconciliation never retires an id that disappears from the roster", () => { + let baseline = seedBaseline; + for (let poll = 0; poll < MODEL_REMOVAL_GRACE_FETCHES * 4; poll++) { + const result = applyNewModelPolicy({ + provider: "vendor", discoveredIds: ["a", "b"], baseline, policy: "off", now, mode: "discovery", + }); + expect(result.newIds).toEqual([]); + expect(result.nextBaseline.ids).toEqual(["a", "b", "c"]); + expect(result.nextBaseline.removed).toEqual([]); + expect(result.nextBaseline.missing).toBeUndefined(); + baseline = result.nextBaseline; + } + }); + + test("convergence still retires the same id after the grace fetches (control)", () => { + let baseline = seedBaseline; + for (let cycle = 0; cycle < MODEL_REMOVAL_GRACE_FETCHES; cycle++) { + baseline = applyNewModelPolicy({ + provider: "vendor", discoveredIds: ["a", "b"], baseline, policy: "off", now, mode: "converge", + }).nextBaseline; + } + expect(baseline.ids).toEqual(["a", "b"]); + expect(baseline.removed).toEqual(["c"]); + }); + + test("read-side reconciliation still hides a genuine arrival but a reappearance clears its pending removal", () => { + const arrival = applyNewModelPolicy({ + provider: "vendor", discoveredIds: ["a", "b"], baseline: { ids: ["a"], removed: [], updatedAt: now }, + policy: "off", now, mode: "discovery", + }); + expect(arrival.newIds).toEqual(["b"]); + expect(arrival.slugsToDisable).toEqual(["vendor/b"]); + + const resumed = applyNewModelPolicy({ + provider: "vendor", discoveredIds: ["a", "c"], + baseline: { ids: ["a", "c"], removed: [], missing: { c: MODEL_REMOVAL_GRACE_FETCHES - 1 }, updatedAt: now }, + policy: "off", now, mode: "discovery", + }); + expect(resumed.nextBaseline.missing).toBeUndefined(); + expect(resumed.nextBaseline.removed).toEqual([]); + }); + + test("repeated read-side reconciliation of the same roster never shrinks the baseline or reports a change", () => { + const config = { + port: 10100, defaultProvider: "vendor", providers: { vendor: {} }, + modelDiscovery: { newModelPolicy: "off" as const, knownModels: { vendor: structuredClone(seedBaseline) } }, + } as Parameters[0]["config"]; + for (let poll = 0; poll < MODEL_REMOVAL_GRACE_FETCHES * 3; poll++) { + const changed = reconcileSuccessfulModelDiscoveries({ + config, models: roster, authoritativeProviders: ["vendor"], now, mode: "discovery", + }); + expect(changed).toBe(false); + } + const known = config.modelDiscovery!.knownModels!.vendor!; + expect(known.ids).toEqual(["a", "b", "c"]); + expect(known.removed).toEqual([]); + expect(known.missing).toBeUndefined(); + }); + + test("repeated convergence of the same shrinking roster retires the id (control)", () => { + const config = { + port: 10100, defaultProvider: "vendor", providers: { vendor: {} }, + modelDiscovery: { newModelPolicy: "off" as const, knownModels: { vendor: structuredClone(seedBaseline) } }, + } as Parameters[0]["config"]; + for (let cycle = 0; cycle < MODEL_REMOVAL_GRACE_FETCHES; cycle++) { + reconcileSuccessfulModelDiscoveries({ + config, models: roster, authoritativeProviders: ["vendor"], now, + }); + } + expect(config.modelDiscovery!.knownModels!.vendor!.removed).toEqual(["c"]); + }); + }); + + describe("effective per-provider policy override", () => { + /** Minimal convergence config: one known id, an optional provider-level override. */ + const convergenceConfig = (global: "on" | "off", local?: "on" | "off") => ({ + port: 10100, defaultProvider: "vendor", + providers: { vendor: local ? { newModelPolicy: local } : {} }, + modelDiscovery: { + newModelPolicy: global, + knownModels: { vendor: { ids: ["a"], removed: [], updatedAt: "2026-01-01T00:00:00Z" } }, + }, + } as Parameters[0]["config"]); + + test("the provider value wins, and an inherited/absent value falls back to the global flag", () => { + const config = { + port: 10100, defaultProvider: "alpha", + providers: { + alpha: { newModelPolicy: "off" as const }, beta: { newModelPolicy: "on" as const }, + gamma: { newModelPolicy: "inherit" as const }, delta: {}, + }, + modelDiscovery: { newModelPolicy: "on" as const }, + } as Parameters[0]; + expect(effectiveNewModelPolicy(config, "alpha")).toBe("off"); + expect(effectiveNewModelPolicy(config, "beta")).toBe("on"); + expect(effectiveNewModelPolicy(config, "gamma")).toBe("on"); + expect(effectiveNewModelPolicy(config, "delta")).toBe("on"); + const globalOff = { ...config, modelDiscovery: { newModelPolicy: "off" as const } } as typeof config; + expect(effectiveNewModelPolicy(globalOff, "delta")).toBe("off"); + expect(effectiveNewModelPolicy(globalOff, "beta")).toBe("on"); + }); + + test("a provider off override hides a genuine arrival under a global on", () => { + const config = convergenceConfig("on", "off"); + const changed = reconcileSuccessfulModelDiscoveries({ + config, + models: [{ provider: "vendor", id: "a" }, { provider: "vendor", id: "b" }], + authoritativeProviders: ["vendor"], now, + }); + expect(changed).toBe(true); + expect(config.disabledModels).toEqual(["vendor/b"]); + }); + + test("a provider on override lets a genuine arrival through under a global off", () => { + const config = convergenceConfig("off", "on"); + const changed = reconcileSuccessfulModelDiscoveries({ + config, + models: [{ provider: "vendor", id: "a" }, { provider: "vendor", id: "b" }], + authoritativeProviders: ["vendor"], now, + }); + expect(changed).toBe(true); + expect(config.disabledModels).toBeUndefined(); + expect(config.modelDiscovery!.recentArrivals!.vendor).toEqual([{ id: "b", at: now }]); + }); + + test("an explicit disabled choice survives a provider on override", () => { + const config = convergenceConfig("off", "on"); + config.disabledModels = ["vendor/b"]; + reconcileSuccessfulModelDiscoveries({ + config, + models: [{ provider: "vendor", id: "a" }, { provider: "vendor", id: "b" }], + authoritativeProviders: ["vendor"], now, + }); + expect(config.disabledModels).toEqual(["vendor/b"]); + }); + }); }); diff --git a/tests/server/model-export-new-model-policy.test.ts b/tests/server/model-export-new-model-policy.test.ts new file mode 100644 index 00000000000..55960454b04 --- /dev/null +++ b/tests/server/model-export-new-model-policy.test.ts @@ -0,0 +1,236 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { loadConfig, saveConfig } from "../../src/config"; +import { setPersistedConfigMutationBeforeCommitForTests } from "../../src/config/persisted-mutation"; +import { clearModelCache } from "../../src/codex/model-cache"; +import { CatalogGatherBusyError } from "../../src/codex/catalog/routed-gather"; +import { + loadExportModels, + previewExportModels, + resetExportSnapshotForTests, +} from "../../src/server/management/model-rows"; +import type { OcxConfig } from "../../src/types"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; +import { createTestCaseLifecycle } from "../helpers/test-sandbox-cleanup"; +import { SERVER_BUDGET_MS } from "../helpers/test-budget"; + +/** + * The export roster (`loadExportModels`) is the ONE loader behind `/api/client-config`, the + * Integrations picker and the CLI model list. It reaches providers through `gatherRoutedModels` + * rather than `fetchAllModels`, so when it is the FIRST consumer of a discovery the effective + * `newModelPolicy` must still apply before the arrival can be published to a client (#6260). + * + * These cases drive the real loader against a fake local upstream and check the observable + * contract: a policy-disabled arrival is absent from the export and persisted, the baseline is + * absorbed, a manual re-enable survives the next export, a preview never gathers or writes, and a + * failed persistence publishes nothing. + */ +const provider = "export-fixture"; +const existing = ["model-a", "model-b"]; +const arrival = "model-c"; +const now = "2026-01-01T00:00:00Z"; + +let home: TempHome; +let lifecycle: ReturnType; +let ids: string[] = []; +let discoveries: string[][] = []; +let upstream: ReturnType; + +beforeEach(() => { + home = createTempHome("ocx-model-export-"); + mkdirSync(home.codexHome, { recursive: true }); + lifecycle = createTestCaseLifecycle(); + // `lastExportSnapshot` is module state shared by every case in this process. + resetExportSnapshotForTests(); + clearModelCache(provider); +}); + +afterEach(async () => { + setPersistedConfigMutationBeforeCommitForTests(null); + try { + await lifecycle.close(); + } finally { + resetExportSnapshotForTests(); + clearModelCache(provider); + home.remove(); + } +}); + +function startUpstream(beforeRespond?: () => void): void { + ids = [...existing]; + discoveries = []; + upstream = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: request => { + if (request.method !== "GET" || new URL(request.url).pathname !== "/v1/models") { + return new Response("Unexpected upstream request", { status: 404 }); + } + discoveries.push([...ids]); + beforeRespond?.(); + return Response.json({ data: ids.map(id => ({ id })) }); + } }); + lifecycle.ownStop(() => upstream.stop(true)); +} + +/** Persist a baseline whose known roster is already A/B, so C is a genuine new arrival. */ +function seedConfig(policy: { global: "on" | "off"; local?: "on" | "off" }): void { + const config: OcxConfig = { + port: 0, hostname: "127.0.0.1", defaultProvider: provider, + providers: { [provider]: { + adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, + apiKey: "fixture-key", allowPrivateNetwork: true, liveModels: true, models: [...existing], + ...(policy.local === undefined ? {} : { newModelPolicy: policy.local }), + } }, + modelDiscovery: { newModelPolicy: policy.global, knownModels: { + [provider]: { ids: [...existing], removed: [], updatedAt: now }, + } }, + }; + saveConfig(config); +} + +/** The export roster as a client sees it, restricted to this fixture's provider. */ +async function exportIds(): Promise { + const models = await loadExportModels(loadConfig()); + return models.filter(model => model.provider === provider).map(model => model.namespaced).sort(); +} + +const cases: Array<{ name: string; global: "on" | "off"; local?: "on" | "off"; visible: boolean }> = [ + { name: "provider override off beats global on", global: "on", local: "off", visible: false }, + { name: "inherited global off", global: "off", visible: false }, + { name: "control: inherited global on keeps the arrival", global: "on", visible: true }, +]; + +test.each(cases)("export as first consumer: $name", policy => lifecycle.run(async () => { + startUpstream(); + seedConfig(policy); + // The seeded baseline already resolves to A/B on both sides, so nothing arrives yet. + expect(await exportIds()).toEqual([`${provider}/model-a`, `${provider}/model-b`]); + + ids = [...existing, arrival]; + clearModelCache(provider); + const before = discoveries.length; + const after = await exportIds(); + // Fresh roster read, not a stale cache: the arrival came from a new upstream fetch. + expect(discoveries.length).toBeGreaterThan(before); + expect(discoveries.at(-1)).toEqual([...existing, arrival]); + expect(after).toContain(`${provider}/model-a`); + expect(after).toContain(`${provider}/model-b`); + // Direct reason, before any persistence claim: the published roster must already agree with + // the effective policy on the very first export that discovers the arrival. + if (policy.visible) expect(after).toContain(`${provider}/model-c`); + else expect(after).not.toContain(`${provider}/model-c`); + + const persisted = loadConfig(); + expect(persisted.modelDiscovery?.knownModels?.[provider]?.ids).toEqual([...existing, arrival]); + expect(persisted.modelDiscovery?.recentArrivals?.[provider] ?? []) + .toContainEqual(expect.objectContaining({ id: arrival })); + if (policy.visible) { + expect(persisted.disabledModels ?? []).not.toContain(`${provider}/model-c`); + } else { + expect(persisted.disabledModels ?? []).toContain(`${provider}/model-c`); + } +}), SERVER_BUDGET_MS); + +test("a manual re-enable survives the next export because the first export absorbed the arrival", + () => lifecycle.run(async () => { + startUpstream(); + seedConfig({ global: "off" }); + + ids = [...existing, arrival]; + clearModelCache(provider); + expect(await exportIds()).not.toContain(`${provider}/model-c`); + expect(loadConfig().disabledModels ?? []).toContain(`${provider}/model-c`); + + // Drop the auto-added disable — the operator re-enabling the arrival in the Models tab. + const reenabled = loadConfig(); + reenabled.disabledModels = (reenabled.disabledModels ?? []).filter(slug => slug !== `${provider}/model-c`); + saveConfig(reenabled); + clearModelCache(provider); + const before = discoveries.length; + const after = await exportIds(); + // The baseline absorbed C on the first export, so it is no longer new and must stay visible. + expect(discoveries.length).toBeGreaterThan(before); + expect(after).toContain(`${provider}/model-c`); + expect(loadConfig().disabledModels ?? []).not.toContain(`${provider}/model-c`); + }), SERVER_BUDGET_MS); + +test("a mid-gather configuration change falls back to the pre-gather projection", + () => lifecycle.run(async () => { + // Land the operator's manual disable while the discovery request is open: after the export + // captured its admission, before the gather resolves. The decision is then stale. + let mutated = false; + startUpstream(() => { + if (mutated) return; + mutated = true; + const live = loadConfig(); + live.disabledModels = [...(live.disabledModels ?? []), `${provider}/model-b`]; + saveConfig(live); + }); + seedConfig({ global: "off" }); + ids = [...existing, arrival]; + clearModelCache(provider); + + const after = await exportIds(); + expect(mutated).toBe(true); + // Pre-gather projection: B was not disabled when the roster was admitted, so A and B survive, + // and the stale Off decision still hides C because the policy applies to the projection in + // memory without committing anything. + expect(after).toContain(`${provider}/model-a`); + expect(after).toContain(`${provider}/model-b`); + expect(after).not.toContain(`${provider}/model-c`); + + // The stale decision never commits: the live/disk manual disable stands, the arrival is not + // absorbed into the persisted baseline, and no snapshot is retained for a preview. + const persisted = loadConfig(); + expect(persisted.disabledModels ?? []).toContain(`${provider}/model-b`); + expect(persisted.disabledModels ?? []).not.toContain(`${provider}/model-c`); + expect(persisted.modelDiscovery?.knownModels?.[provider]?.ids).toEqual([...existing]); + expect(previewExportModels(loadConfig())).toBeNull(); + }), SERVER_BUDGET_MS); + +test("previewExportModels serves the retained roster without gathering or writing", + () => lifecycle.run(async () => { + startUpstream(); + seedConfig({ global: "off" }); + ids = [...existing, arrival]; + clearModelCache(provider); + + // The FIRST export is already the authority: it absorbs and persists the arrival, and the + // retained snapshot must be usable from that load without a second steady-state pass. + const first = await exportIds(); + expect(first).not.toContain(`${provider}/model-c`); + + const configBefore = readFileSync(home.path("config.json"), "utf8"); + const before = discoveries.length; + const preview = previewExportModels(loadConfig()); + expect(preview).not.toBeNull(); + // Read-only in both directions: no upstream fetch and no rewrite of the configuration file. + expect(discoveries.length).toBe(before); + expect(readFileSync(home.path("config.json"), "utf8")).toBe(configBefore); + expect(preview!.filter(model => model.provider === provider).map(model => model.namespaced).sort()) + .toEqual(first); + // A repeated preview is stable: still no gather, still no write. + expect(previewExportModels(loadConfig())).toEqual(preview); + expect(discoveries.length).toBe(before); + expect(readFileSync(home.path("config.json"), "utf8")).toBe(configBefore); + }), SERVER_BUDGET_MS); + +test("a persistence failure raises CatalogGatherBusyError and publishes nothing", + () => lifecycle.run(async () => { + startUpstream(); + seedConfig({ global: "off" }); + ids = [...existing, arrival]; + clearModelCache(provider); + + // Corrupt the configuration at the commit seam so the arrival decision cannot persist. + const corrupt = "{ not-json"; + let hookRan = false; + setPersistedConfigMutationBeforeCommitForTests(() => { + hookRan = true; + writeFileSync(home.path("config.json"), corrupt, "utf8"); + }); + + await expect(loadExportModels(loadConfig())).rejects.toThrow(CatalogGatherBusyError); + + // The seam fired, and the refused commit left the corrupt bytes rather than publishing. + expect(hookRan).toBe(true); + expect(readFileSync(home.path("config.json"), "utf8")).toBe(corrupt); + }), SERVER_BUDGET_MS); diff --git a/tests/server/server-new-model-policy-arrival.test.ts b/tests/server/server-new-model-policy-arrival.test.ts new file mode 100644 index 00000000000..62240d76474 --- /dev/null +++ b/tests/server/server-new-model-policy-arrival.test.ts @@ -0,0 +1,154 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { loadConfig, saveConfig } from "../../src/config"; +import { setPersistedConfigMutationBeforeCommitForTests } from "../../src/config/persisted-mutation"; +import { clearModelCache } from "../../src/codex/model-cache"; +import type { OcxConfig } from "../../src/types"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; +import { SERVER_BUDGET_MS } from "../helpers/test-budget"; +import { createTestCaseLifecycle } from "../helpers/test-sandbox-cleanup"; + +const provider = "fixture-catalog"; +const existing = ["model-a", "model-b"]; +const arrival = "model-c"; +const now = "2026-01-01T00:00:00Z"; +let home: TempHome; +let lifecycle: ReturnType; + +beforeEach(() => { + home = createTempHome("ocx-model-arrival-"); + mkdirSync(home.codexHome, { recursive: true }); + lifecycle = createTestCaseLifecycle(); + clearModelCache(provider); +}); + +afterEach(async () => { + setPersistedConfigMutationBeforeCommitForTests(null); + try { + await lifecycle.close(); + } finally { + clearModelCache(provider); + home.remove(); + } +}); + +const cases: Array<{ + name: string; + global: "on" | "off"; + local?: "on" | "off"; + visible: boolean; +}> = [ + { name: "provider off overrides global on", global: "on", local: "off", visible: false }, + { name: "inherited global off", global: "off", visible: false }, + { name: "provider on overrides global off", global: "off", local: "on", visible: true }, + { name: "inherited global on", global: "on", visible: true }, +]; + +// Discovery is driven through the real HTTP route, never by calling policy reconciliation, so the +// assertions below check the persisted contract under test (#6260): an absorbed baseline, a +// recorded arrival, a surviving manual block, and a manual re-enable that holds across refresh. +test.each(cases)("new arrival before catalog convergence: $name", policy => lifecycle.run(async () => { + let ids = [...existing]; + const discoveries: string[][] = []; + const upstream = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: request => { + if (request.method !== "GET" || new URL(request.url).pathname !== "/v1/models") { + return new Response("Unexpected upstream request", { status: 404 }); + } + discoveries.push([...ids]); + return Response.json({ data: ids.map(id => ({ id })) }); + } }); + lifecycle.ownStop(() => upstream.stop(true)); + const config: OcxConfig = { + port: 0, hostname: "127.0.0.1", defaultProvider: provider, + providers: { [provider]: { + adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, + apiKey: "fixture-key", allowPrivateNetwork: true, liveModels: true, models: [...existing], + ...(policy.local === undefined ? {} : { newModelPolicy: policy.local }), + } }, + disabledModels: [`${provider}/model-b`], + modelDiscovery: { newModelPolicy: policy.global, knownModels: { + [provider]: { ids: [...existing], removed: [], updatedAt: now }, + } }, + }; + saveConfig(config); + expect(loadConfig().modelDiscovery?.knownModels?.[provider]?.ids).toEqual(existing); + const { startServer } = await import("../../src/server"); + lifecycle.abort.signal.throwIfAborted(); + const server = startServer(0); + lifecycle.ownStop(() => server.stop(true)); + const read = async () => { + const response = await fetch(new URL("/v1/models", server.url), { signal: lifecycle.abort.signal }); + expect(response.status).toBe(200); + return (await response.json() as { data: Array<{ id: string }> }).data + .map(model => model.id).filter(id => id.startsWith(`${provider}/`)).sort(); + }; + expect(await read()).toEqual([`${provider}/model-a`]); + expect(discoveries.at(-1)).toEqual(existing); + const previousDiscoveries = discoveries.length; + ids = [...existing, arrival]; + // Exercise a fresh discovery without waiting for the production cache TTL to expire. + clearModelCache(provider); + const after = await read(); + expect(discoveries.length).toBeGreaterThan(previousDiscoveries); + expect(discoveries.at(-1)).toEqual([...existing, arrival]); + expect(after).toContain(`${provider}/model-a`); + expect(after).not.toContain(`${provider}/model-b`); + expect(after).toEqual(policy.visible + ? [`${provider}/model-a`, `${provider}/model-c`] + : [`${provider}/model-a`]); + + // A successful discovery must persist, not only shape this response. The baseline absorbs the + // arrival (so the next discovery does not re-flag it), the arrival is recorded for the operator, + // and the manual block on model-b survives the same convergence that disabled model-c. + const persisted = loadConfig(); + expect(persisted.modelDiscovery?.knownModels?.[provider]?.ids).toEqual([...existing, arrival]); + expect(persisted.modelDiscovery?.recentArrivals?.[provider] ?? []) + .toContainEqual(expect.objectContaining({ id: arrival })); + expect(persisted.disabledModels ?? []).toContain(`${provider}/model-b`); + if (policy.visible) expect(persisted.disabledModels ?? []).not.toContain(`${provider}/model-c`); + else expect(persisted.disabledModels ?? []).toContain(`${provider}/model-c`); + + // Manual re-enable through the management route the CLI uses. It only sticks because the baseline + // above absorbed model-c; a later discovery that still saw it as new would re-flag and re-hide it. + const adminToken = readFileSync(home.path("admin-api-token"), "utf8").trim(); + const enable = await fetch(new URL("/api/model-visibility", server.url), { + method: "PUT", + headers: { "content-type": "application/json", "x-opencodex-api-key": adminToken }, + body: JSON.stringify({ scope: "models", provider, enabled: true, targets: [{ id: arrival }] }), + signal: lifecycle.abort.signal, + }); + expect(enable.status).toBe(200); + expect(loadConfig().disabledModels ?? []).not.toContain(`${provider}/model-c`); + + clearModelCache(provider); + expect(await read()).toEqual([`${provider}/model-a`, `${provider}/model-c`]); +}), SERVER_BUDGET_MS); + +test("failed arrival persistence returns retryable HTTP 503 without publishing models", () => lifecycle.run(async () => { + const upstream = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: () => + Response.json({ data: [...existing, arrival].map(id => ({ id })) }), + }); + lifecycle.ownStop(() => upstream.stop(true)); + saveConfig({ + port: 0, hostname: "127.0.0.1", defaultProvider: provider, + providers: { [provider]: { + adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, + apiKey: "fixture-key", allowPrivateNetwork: true, liveModels: true, models: [...existing], + newModelPolicy: "off", + } }, + modelDiscovery: { knownModels: { + [provider]: { ids: [...existing], removed: [], updatedAt: now }, + } }, + }); + const { startServer } = await import("../../src/server"); + lifecycle.abort.signal.throwIfAborted(); + const server = startServer(0); + lifecycle.ownStop(() => server.stop(true)); + setPersistedConfigMutationBeforeCommitForTests(() => writeFileSync(home.path("config.json"), "invalid")); + const response = await fetch(new URL("/v1/models", server.url), { signal: lifecycle.abort.signal }); + expect(response.status).toBe(503); + expect(response.headers.get("retry-after")).toBe("1"); + const body = await response.json() as { error: { code: string }; data?: unknown }; + expect(body.error.code).toBe("catalog_busy"); + expect(body.data).toBeUndefined(); +}), SERVER_BUDGET_MS);