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/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 794895ba674..392fdfa42bd 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -586,6 +586,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", @@ -1065,6 +1066,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", @@ -1097,6 +1099,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", @@ -1143,6 +1146,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/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/providers/new-model-policy-runtime.ts b/src/providers/new-model-policy-runtime.ts new file mode 100644 index 00000000000..7f14235b4d2 --- /dev/null +++ b/src/providers/new-model-policy-runtime.ts @@ -0,0 +1,66 @@ +import { isDeepStrictEqual } from "node:util"; +import { mutatePersistedConfig, readConfigDiagnostics, validateConfigCandidate } from "../config"; +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 rows = [...models]; + const now = new Date().toISOString(); + const current = () => 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) { + // Standalone/synthetic callers may project their own config, never overwrite another home. + reconcile(config); + return true; + } + 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 930e1410c3d..1597acb2c18 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. 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 117a1fc8b67..410d0b20d57 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -131,9 +131,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 0535f765a97..3906e51bc74 100644 --- a/structure/config.md +++ b/structure/config.md @@ -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 fb6136f84d3..97c974dcc84 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 65443f86644..a7fa27bb899 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -213,6 +213,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 6e8036d85a4..6f69a56f3d2 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. 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 b755124ee7d..90aeaa9abf3 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -597,6 +597,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", @@ -1078,6 +1079,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", @@ -1110,6 +1112,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", @@ -1156,6 +1159,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..d7b624ac007 --- /dev/null +++ b/tests/providers/new-model-policy-runtime.test.ts @@ -0,0 +1,137 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { readFileSync, writeFileSync } from "node:fs"; +import { loadConfig, saveConfig } from "../../src/config"; +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(); +}); 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);