feat(catalog): per-provider retainModels opt-in (#1690, carries #2860)

devlog/_plan/260902_nonbug_adoption_backlog/040_wp4_retain_models.md

@@ -0,0 +1,111 @@
+# wp4 — #1690 retainModels allowlist (two rival PRs)
+
+Issue #1690 (score 58, labels enhancement/catalog). Two open drafts implement it:
+
+| | PR #2860 (rrmlima) | PR #2122 (chilung-cgu) |
+| --- | --- | --- |
+| size | +132/-2, 3 files | +726/-25, 15 files |
+| CI at head | fully green (test 1-4, macos, ci) | only hygiene/label/target ran |
+| base | e546c160b, 310 behind dev; cherry-picks cleanly onto `fcf0da257` | same base |
+| retention | `shouldRetainConfiguredProviderModel(name, id, prov)` + `modelInList` | new `providerRetainModels` set inside the merge loop |
+| ids must also be in `models`? | yes (purely retentive) | no (`retainModels` folded into `configuredIds`) |
+| config validation | none (relies on `.passthrough()`) | zod `retainModels` + `nonBlankStringArrayConfigError` |
+| management API / DTO | none | none (neither exposes it via PATCH) |
+| rename migration | none | adds `retainModels` to `MODEL_ID_LISTS` |
+| 404 diagnostic | none | new module state + `warnRetainedModel404Once` wired into 4 request handlers, `CatalogModel.retainedWithoutDiscovery` |
+| docs | none | 5 locales, one table row each |
+
+## Decision
+
+Adopt **#2860 as the base commit** (cherry-picked as `2be9d505d` on
+`codex/retain-models-1690`), then add the pieces that make the opt-in
+discoverable and safe to hand-edit. #2122 is closed as superseded with credit for
+the config/migration design.
+
+Why #2860 over #2122: the retention decision belongs in the one predicate the
+merge loop already consults; #2122 adds a second set beside it. #2122's 404
+diagnostic requires module-level maps keyed by provider, a new `CatalogModel`
+field, and edits to `core.ts`, `chat-completions.ts`, `chat-native.ts`,
+`claude-messages.ts` — four hot request paths — to print one warning that the
+upstream error body already carries (`model_not_found`). That is the wrong
+trade for this cycle; the existing `warnDroppedConfiguredIdsOnce` stays as the
+diagnostic for ids that are *not* retained.
+
+## What this cycle adds on top of #2860
+
+1. **`retainModels` alone is enough.** `configuredIds` in
+   `fetchProviderModelsWithAuth` becomes the ordered union of the Vertex seed,
+   `prov.models`, and `prov.retainModels`. An operator who writes
+   `"retainModels": ["gemini-3.7-flash"]` should not have to repeat the id in
+   `models`; requiring both is the footgun #2122 correctly avoided. #2860's
+   "does not invent ids" test flips to assert the union.
+2. **Schema + load normalization** (`src/config.ts`): `retainModels:
+   z.array(z.string().min(1)).transform(normalizeNonBlankStringArray).optional()`
+   next to `noStructuredOutputModels`, plus the same `superRefine` entry so a
+   hand-edited `"retainModels": "x"` fails with a path instead of being silently
+   passed through.
+3. **Management PATCH + DTO** (`src/server/management/provider-routes.ts`):
+   `retainModels` accepted like `noStructuredOutputModels` (`null` clears,
+   empty array clears, validated with `nonBlankStringArrayConfigError`), and
+   returned in the safe provider DTO so the dashboard/API round-trips it.
+4. **CLI opt-in** (`src/cli/provider-runtime.ts`):
+   `ocx provider edit <name> --retain-models <id,id|->`. This is the easy
+   switch: one flag, no JSON editing, `-` clears. Usage string and
+   `skills/ocx` surface are regenerated if the capability registry changes
+   (it does not — `provider edit` already exists; only the flag list grows).
+5. **Rename migration** (`src/providers/model-rename-migration.ts`):
+   `"retainModels"` added to `MODEL_ID_LISTS` so a retired id is renamed
+   rather than resurrected as a ghost row.
+6. **Docs** (`docs-site/.../reference/configuration/providers.md`): one table
+   row after `selectedModels`, and a short paragraph in "Static model
+   allowlists" contrasting `selectedModels` (narrows) with `retainModels`
+   (preserves). English only this cycle; locales already lag on
+   `noStructuredOutputModels` and a missing row does not contradict.
+
+## Explicitly not in this cycle
+
+- Seeding `CALLABLE_CONFIGURED_COMPATIBILITY_MODELS` with antigravity
+  `gemini-3.7-flash` (issue step 4). That is a product default, and #1683 is a
+  separate issue; with the config key available it no longer needs a release.
+- GUI field. The dashboard provider editor is untouched; the PATCH contract is
+  ready for it, and a later PR can add the input with its screenshot.
+- 404-time warning. See "Decision".
+
+## Acceptance criteria
+
+- `retainModels` absent/empty → catalog identical to today (existing
+  `tests/codex-catalog.test.ts` retention tests untouched and green).
+- `retainModels: ["x"]`, live omits `x`, `x` not in `models` → `x` present
+  with provider hints applied; `droppedConfiguredIds` excludes it.
+- `retainModels: ["x"]`, live returns `x` → single row, no duplicate.
+- `liveModels: false` → `retainModels` ids are part of the static list.
+- Config load rejects `retainModels: "x"` / `[""]` with a
+  `providers.<name>.retainModels` path; trims and dedupes valid input.
+- Management PATCH sets/clears; DTO echoes; CLI flag round-trips through PATCH.
+- A test through `fetchProviderModels` (not only `mergeConfiguredModelsIntoLiveCatalog`) proves a retain-only id survives both live discovery and `liveModels: false` (audit r1 blocker 2).
+- CLI treats `-` before `csv` so `--retain-models -` clears (audit r1 blocker 3).
+- `providerCatalogFingerprint` includes `retainModels`.
+- Migration renames a retired id inside `retainModels`.
+- `bun x tsc --noEmit` clean, `bun run privacy:scan` clean, focused files:
+  `tests/catalog-retain-models.test.ts`, `tests/codex-catalog.test.ts`,
+  `tests/management-provider-validation.test.ts`,
+  `tests/model-rename-migration.test.ts` (if present), provider-runtime CLI test.
+
+## Files
+
+- `src/codex/catalog/provider-fetch.ts` — configuredIds union (on top of #2860).
+- `src/config.ts` — schema + superRefine.
+- `src/server/management/provider-routes.ts` — PATCH + DTO.
+- `src/server/auth-cors.ts` — `providerManagementConfigError` validation + safe-config DTO key list (audit r1 blocker 1).
+- `src/cli/provider-runtime.ts` — `--retain-models`.
+- `src/providers/model-rename-migration.ts` — list entry.
+- `src/types/provider.ts` — already added by #2860 (doc comment adjusted for union).
+- `docs-site/src/content/docs/reference/configuration/providers.md`.
+- `tests/catalog-retain-models.test.ts` (extend), `tests/management-provider-validation.test.ts` (extend).
+
+## Closure
+
+PR targets `dev`, `Closes #1690`, description names #2860 as the carried
+source commit (`12e69c200`) and #2122 as design input. After landing: close
+#1690 with the landing SHA, close #2860 as landed-via-carry (author credited in
+the squash trailer), close #2122 as superseded with the reasoning above.

devlog/_plan/260902_nonbug_adoption_backlog/041_wp4_audit_r1_synthesis.md

@@ -0,0 +1,30 @@
+# wp4 audit r1 — synthesis
+
+Reviewer: grok-4.6 subagent (Feynman), read-only, 7 questions on 040. Verdict: **near-pass / GO-WITH-FIXES** (3 blockers, 3 suggestions).
+
+## Answers that confirm the plan
+
+- Union (Q1): safe only if written as an ordered dedupe set `[vertexDefault?, ...models, ...retainModels]`, replacing the current `seed ? [default] : models` ternary. `configured` is the single seed for static `liveModels:false`, the Cursor filter, the degraded fallback, `droppedConfiguredIds`, and hints, so a retain-only id gets the same context/effort maps as a `models[]` entry. The Vertex seed predicate (`models.length === 0`) stays untouched.
+- Family match (Q2): acceptable, same semantics as `noVisionModels`. `retainModels: ["gpt-oss"]` keeps `gpt-oss:120b` and also invents a bare `gpt-oss` row through the union — extra row, never a drop.
+- selectedModels precedence (Q3): `filterCatalogVisibleModels` and `sync.ts` still hide a retained id when `selectedModels` is non-empty and omits it. Keep that; document "retain ≠ visible".
+- PATCH (Q4): copy `provider-routes.ts:386` verbatim; also GET list (:540).
+- CLI (Q5): no `takeListOption`; use `csv(takeOption)` and special-case `"-"` **before** `csv` (`csv("-")` yields `["-"]`).
+- #2122 (Q6): union, schema, `MODEL_ID_LISTS` are the necessary parts; the 404 module maps and `retainedWithoutDiscovery` are not. The `withConfiguredRetention(live→forCache)` change is incidental — the double merge is the OCX-111 combo-cache contract. Do not copy.
+- No-regression (Q7): absent/empty is a no-op; kimi/xai tables unchanged.
+
+## Blockers folded into 040
+
+1. `src/server/auth-cors.ts` was missing from the file list: `providerManagementConfigError` (~693) must validate `retainModels` with `nonBlankStringArrayConfigError`, and the safe-config DTO key list (~794) must include it, otherwise PATCH validation and DTO echo silently miss.
+2. The "liveModels:false / retain-only present" criterion cannot be proven through `mergeConfiguredModelsIntoLiveCatalog` alone. Add a test that goes through `fetchProviderModels`/the gather path so the union at :1307 is actually exercised.
+3. CLI: handle `-` before `csv`.
+
+## Suggestions taken
+
+- Docs state that `selectedModels` still narrows what is visible even for retained ids.
+- `retainModels` added to `providerCatalogFingerprint` (:573) so two providers differing only in that list do not share a discovery flight.
+- Flip #2860's "does not invent ids" test to assert the union.
+
+## Disposition
+
+All three blockers are additive edits inside the already-planned files plus one file (`auth-cors.ts`). No scope change. Proceed to B.
+

docs-site/src/content/docs/reference/configuration/providers.md

@@ -83,6 +83,7 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids.
 | `models?` | `string[]` | Seed/fallback model list. With `liveModels: false`, these are the only discovered models. |
 | `liveModels?` | `boolean` | Fetch the live catalog on start/sync (default `true`). Custom providers use `${baseUrl}/models`; built-ins may use a registry URL and filter. |
 | `selectedModels?` | `string[]` | Catalog allowlist after discovery. Non-empty exposes only those ids; empty or omitted exposes all discovered models. |
+| `retainModels?` | `string[]` | Ids kept in the catalog even when live discovery omits them. They need not be repeated in `models`. Empty or omitted keeps today's behavior. |
 | `contextWindow?` | `number` | Provider-wide context fallback when upstream metadata is absent; otherwise a cap that retains smaller live metadata. The Models dashboard exposes this separately from `providerContextCaps`. |
 | `modelContextWindows?` | `Record<string, number>` | Per-model context fallbacks/caps. These override `contextWindow`: an unknown window uses the configured value, while smaller live metadata remains authoritative. |
 | `modelInputModalities?` | `Record<string, string[]>` | Per-model input hints such as `["text"]` or `["text", "image"]`. |
@@ -575,6 +576,14 @@ silently replaced or truncated.
 Use `selectedModels` when discovery should still run but only selected ids should appear in Codex and
 `/v1/models`. The dashboard retains the full discovered list for later allowlist changes.
 
+Use `retainModels` for the opposite problem: a provider whose `/models` endpoint omits an id that is
+still callable (a private deployment, a preview id, an OpenAI-compatible gateway with a partial
+listing). Listed ids are kept in the routed catalog with the same context and effort hints as
+`models`, and they survive `liveModels: false` too. `selectedModels` still narrows what is visible,
+so an id must be in both lists when an allowlist is active. Retaining an id does not make the
+upstream accept it; a wrong id fails at request time with the upstream error. From the CLI:
+`ocx provider edit <name> --retain-models gemini-3.7-flash,other-id` (`-` clears).
+
 Preview GPT-5.6 fallback entries use the same mechanism. The OpenAI API-key preset seeds base and Pro
 ids with context `922000` and max input `922000`; OpenRouter seeds `openai/gpt-5.6-sol`,
 `openai/gpt-5.6-terra`, and `openai/gpt-5.6-luna` with context `922000`. Pool/Direct advertises

src/cli/provider-runtime.ts

@@ -24,6 +24,7 @@ const USAGE = `Usage:
       [--auth-mode <key|forward|oauth|local|->] [--note <text|->]
       [--api-key-transport <x-api-key|bearer|->]
       [--headers <json>] [--enabled <on|off>] [--live-models <on|off>]
+      [--retain-models <id,id|->]
       [--allow-private-network <on|off>] [--json]
   ocx provider test <name> [--json]
   ocx provider quota [--refresh] [--json]
@@ -48,6 +49,7 @@ async function edit(argv: string[], deps: RuntimeApiDeps): Promise<void> {
   const note = cleared(takeOption(args, "--note"));
   const apiKeyTransport = cleared(takeOption(args, "--api-key-transport"));
   const headers = takeOption(args, "--headers");
+  const retainModelsRaw = takeOption(args, "--retain-models");
   const enabled = takeBooleanOption(args, "--enabled");
   const liveModels = takeBooleanOption(args, "--live-models");
   const allowPrivateNetwork = takeBooleanOption(args, "--allow-private-network");
@@ -74,6 +76,10 @@ async function edit(argv: string[], deps: RuntimeApiDeps): Promise<void> {
     }
   }
   if (enabled !== undefined) patch.disabled = !enabled;
+  if (retainModelsRaw !== undefined) {
+    // `-` clears, matching the other `edit` scalars; test before csv() or it becomes ["-"].
+    patch.retainModels = retainModelsRaw.trim() === "-" ? null : csv(retainModelsRaw);
+  }
   if (liveModels !== undefined) patch.liveModels = liveModels;
   if (allowPrivateNetwork !== undefined) patch.allowPrivateNetwork = allowPrivateNetwork;
   if (Object.keys(patch).length === 0) throw new CliUsageError("at least one edit option is required", USAGE);

src/codex/catalog/provider-fetch.ts

@@ -571,6 +571,7 @@ function providerCatalogFingerprint(name: string, prov: OcxProviderConfig): Reco
     base: prov.baseUrl ?? "",
     adapter: prov.adapter ?? "",
     models: [...(prov.models ?? [])].sort(),
+    retain: [...(prov.retainModels ?? [])].sort(),
     selected: [...(prov.selectedModels ?? [])].sort(),
     defaultModel: prov.defaultModel ?? null,
     ctx: prov.contextWindow ?? null,
@@ -1304,7 +1305,14 @@ async function fetchProviderModelsWithAuth(
     && prov.googleMode === "vertex"
     && (prov.models?.length ?? 0) === 0
     && Boolean(prov.defaultModel);
-  const configuredIds = seedVertexDefault && prov.defaultModel ? [prov.defaultModel] : (prov.models ?? []);
+  // Ordered dedupe union: Vertex seed, then `models`, then `retainModels`. `configured` is the
+  // single seed for the static path, the degraded fallback, drop diagnostics, and provider hints,
+  // so a retain-only id must enter here or it never exists to be retained (#1690).
+  const configuredIds = [...new Set([
+    ...(seedVertexDefault && prov.defaultModel ? [prov.defaultModel] : []),
+    ...(prov.models ?? []),
+    ...(prov.retainModels ?? []),
+  ])];
   const configured: CatalogModel[] = configuredIds.map(id => ({
     id,
     provider: name,
@@ -1700,9 +1708,14 @@ export function shouldExposeProviderModel(providerName: string, modelId: string)
   return true;
 }
 
-export function shouldRetainConfiguredProviderModel(providerName: string, modelId: string): boolean {
+export function shouldRetainConfiguredProviderModel(
+  providerName: string,
+  modelId: string,
+  prov?: OcxProviderConfig,
+): boolean {
   if (CALLABLE_CONFIGURED_COMPATIBILITY_MODELS[providerName]?.has(modelId)) return true;
   if (providerName === "opencode-free") return modelId === "big-pickle" || modelId.endsWith("-free");
+  if (modelInList(prov?.retainModels, modelId)) return true;
   return false;
 }
 
@@ -1748,7 +1761,7 @@ export function mergeConfiguredModelsIntoLiveCatalog(opts: {
     }
     if (
       seedVertexDefault === true
-      || shouldRetainConfiguredProviderModel(name, candidate.id)
+      || shouldRetainConfiguredProviderModel(name, candidate.id, prov)
       || (retainComboTargets && retainConfiguredModelIds?.has(candidate.id) === true)
     ) {
       out.push(candidate);

src/config.ts

@@ -529,6 +529,9 @@ const providerConfigSchema = z.object({
   noStructuredOutputModels: z.array(z.string().min(1))
     .transform(normalizeNonBlankStringArray)
     .optional(),
+  retainModels: z.array(z.string().min(1))
+    .transform(normalizeNonBlankStringArray)
+    .optional(),
   omitReasoningEffortWithToolsModels: z.array(z.string().min(1))
     .transform(normalizeNonBlankStringArray)
     .optional(),
@@ -1368,6 +1371,17 @@ const configSchema = z.object({
         message: structuredOutputOptOutError,
       });
     }
+    const retainModelsError = nonBlankStringArrayConfigError(
+      (provider as { retainModels?: unknown }).retainModels,
+      "retainModels",
+    );
+    if (retainModelsError) {
+      ctx.addIssue({
+        code: "custom",
+        path: ["providers", redactSecretString(name), "retainModels"],
+        message: retainModelsError,
+      });
+    }
     const toolReasoningOptOutError = nonBlankStringArrayConfigError(
       (provider as { omitReasoningEffortWithToolsModels?: unknown }).omitReasoningEffortWithToolsModels,
       "omitReasoningEffortWithToolsModels",

src/providers/model-rename-migration.ts

@@ -99,6 +99,9 @@ const MODEL_ID_LISTS = [
   // their catalog instead of being renamed. OAuth reconciliation does not cover this
   // field, so the rename has to.
   "selectedModels",
+  // Same reasoning as `selectedModels`: a retired id pinned here would be resurrected as a
+  // ghost row on every discovery instead of following the rename (#1690).
+  "retainModels",
   "noVisionModels",
   "noReasoningModels",
   "noTemperatureModels",

src/server/auth-cors.ts

@@ -694,6 +694,8 @@ export function providerManagementConfigError(name: unknown, provider: unknown):
     "noStructuredOutputModels",
   );
   if (structuredOutputOptOutError) return `provider ${name} ${structuredOutputOptOutError}`;
+  const retainModelsError = nonBlankStringArrayConfigError(raw.retainModels, "retainModels");
+  if (retainModelsError) return `provider ${name} ${retainModelsError}`;
   const toolReasoningOptOutError = nonBlankStringArrayConfigError(
     raw.omitReasoningEffortWithToolsModels,
     "omitReasoningEffortWithToolsModels",
@@ -792,6 +794,7 @@ export function safeConfigDTO(config: OcxConfig): unknown {
       "noTopPModels",
       "noPenaltyModels",
       "noStructuredOutputModels",
+      "retainModels",
       "omitReasoningEffortWithToolsModels",
       "upstreamHttpVersion",
       "autoToolChoiceOnlyModels",

src/server/management/provider-routes.ts

@@ -396,6 +396,19 @@ function applyProviderPatchFields(
     }
     touched = true;
   }
+  if (Object.hasOwn(rawBody, "retainModels")) {
+    const value = rawBody.retainModels;
+    if (value === null) {
+      delete next.retainModels;
+    } else {
+      const error = nonBlankStringArrayConfigError(value, "retainModels");
+      if (error) return { error };
+      const models = normalizeNonBlankStringArray(value as string[]);
+      if (models.length > 0) next.retainModels = models;
+      else delete next.retainModels;
+    }
+    touched = true;
+  }
   if (Object.hasOwn(rawBody, "omitReasoningEffortWithToolsModels")) {
     const value = rawBody.omitReasoningEffortWithToolsModels;
     if (value === null) {
@@ -538,6 +551,7 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise<Resp
       modelAutoCompactTokenLimits: p.modelAutoCompactTokenLimits,
       modelSupportsServiceTier: p.modelSupportsServiceTier,
       noStructuredOutputModels: p.noStructuredOutputModels,
+      retainModels: p.retainModels,
       omitReasoningEffortWithToolsModels: p.omitReasoningEffortWithToolsModels,
       upstreamHttpVersion: p.upstreamHttpVersion,
       authMode: p.authMode,