fix(providers): drop a removed provider's custom models (#1273)

src/cli/provider.ts

@@ -12,6 +12,7 @@ import { apiKeyTransportConfigError, hasOwnProvider, isValidProviderName, loadCo
 import { hasHelpFlag } from "./help";
 import { getProviderRegistryEntry, PROVIDER_REGISTRY } from "../providers/registry";
 import { providerConfigSeed } from "../providers/derive";
+import { dropProviderCustomModels } from "../providers/provider-id-rewrite";
 import type { OcxProviderConfig } from "../types";
 import { findLiveProxy } from "../server/proxy-liveness";
 import { syncModelsToCodex } from "../codex/sync";
@@ -302,6 +303,7 @@ function handleRemove(args: string[]): void {
   }
 
   delete config.providers[name];
+  const droppedCustomModels = dropProviderCustomModels(config, name);
   validateAndSave(config);
 
 
@@ -312,11 +314,16 @@ function handleRemove(args: string[]): void {
       remainingProviders: Object.keys(config.providers),
       defaultProvider: config.defaultProvider,
       needsSync: true,
+      ...(droppedCustomModels > 0 ? { droppedCustomModels } : {}),
     }, null, 2));
     return;
   }
 
   console.log(`✅ Provider "${name}" removed.`);
+  if (droppedCustomModels > 0) {
+    const plural = droppedCustomModels === 1 ? "model" : "models";
+    console.log(`   Also removed ${droppedCustomModels} custom ${plural} that belonged to it.`);
+  }
 }
 
 // ---------------------------------------------------------------------------

src/providers/provider-id-rewrite.ts

@@ -148,3 +148,32 @@ export function rewriteProviderReferences(config: OcxConfig, from: string, to: s
 
   return { changed, collisions };
 }
+
+/**
+ * Drop the custom-model rows that belonged to a provider being removed.
+ *
+ * The sibling of the rename pass above. `rewriteProviderReferences` already
+ * carries `customModels[].provider` across a rename, so the array tracks the
+ * provider lifecycle — but removal used to delete only `config.providers[name]`
+ * and leave the rows behind. Those orphans still reach `/api/models` and the
+ * generated Codex catalog, which key on the row rather than on provider
+ * existence, so they surface as models that resolve to nothing (#1273).
+ *
+ * Only the rows are touched: the `customModelCatalogMigration` marker records
+ * one-time ownership of pre-marker rows and must survive removal unchanged, or
+ * an older binary's view of that ownership silently changes.
+ *
+ * Returns the number of rows dropped so callers can report it.
+ */
+export function dropProviderCustomModels(config: OcxConfig, provider: string): number {
+  const existing = config.customModels;
+  if (!Array.isArray(existing) || existing.length === 0) return 0;
+  const kept = existing.filter(model => model.provider !== provider);
+  if (kept.length === existing.length) return 0;
+  // Match the add/remove routes: an emptied list is dropped rather than left as
+  // `[]`, so the `customModels` field is absent either way. Only that field —
+  // the `customModelCatalogMigration` marker is deliberately left in place.
+  if (kept.length > 0) config.customModels = kept;
+  else delete config.customModels;
+  return existing.length - kept.length;
+}

src/server/management/provider-routes.ts

@@ -615,13 +615,20 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise<Resp
     const { saveConfigPreservingClaudeCode: save } = await import("../../config");
     if (fallbackDefault) config.defaultProvider = fallbackDefault;
     delete config.providers[name];
+    const { dropProviderCustomModels } = await import("../../providers/provider-id-rewrite");
+    const droppedCustomModels = dropProviderCustomModels(config, name);
     setProviderContextCap(config, name, false);
     save(config);
     reconcileLiveStateStores();
     const { clearModelCache: clearCache } = await import("../../codex/model-cache");
     clearCache(name);
     const catalogRefresh = await convergeCodexCatalog();
-    return jsonResponse({ success: true, ...(fallbackDefault ? { defaultProvider: fallbackDefault } : {}), catalogRefresh });
+    return jsonResponse({
+      success: true,
+      ...(fallbackDefault ? { defaultProvider: fallbackDefault } : {}),
+      ...(droppedCustomModels > 0 ? { droppedCustomModels } : {}),
+      catalogRefresh,
+    });
   }
 
   if (url.pathname === "/api/provider-context-caps" && req.method === "GET") {

tests/cli-provider.test.ts

@@ -223,6 +223,45 @@ describe("ocx provider", () => {
     }
   });
 
+  test("provider remove drops that provider's custom models (#1273)", () => {
+    const { dir } = freshConfig({
+      providers: {
+        openai: { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", authMode: "forward" },
+        huggingface: { adapter: "openai-chat", baseUrl: "https://api.hf.test/v1", apiKey: "k" },
+      },
+      customModels: [
+        { id: "keep-1", provider: "openai", modelId: "kept-model" },
+        { id: "drop-1", provider: "huggingface", modelId: "DeepSeek-V4-Flash-0731" },
+      ],
+      // Seeded so the assertion below proves removal does not rewrite one-time
+      // ownership: an older binary must keep seeing the same legacy slugs.
+      customModelCatalogMigration: {
+        version: 1,
+        legacyOwnedSlugs: ["huggingface/DeepSeek-V4-Flash-0731", "openai/kept-model"],
+      },
+    });
+    try {
+      const result = runCli(["provider", "remove", "huggingface", "--json"], { OPENCODEX_HOME: dir });
+      expect(result.status).toBe(0);
+      expect(JSON.parse(result.stdout)).toMatchObject({
+        action: "removed",
+        provider: "huggingface",
+        droppedCustomModels: 1,
+      });
+
+      const config = readConfig(dir);
+      expect(config.customModels).toEqual([
+        { id: "keep-1", provider: "openai", modelId: "kept-model" },
+      ]);
+      expect(config.customModelCatalogMigration).toEqual({
+        version: 1,
+        legacyOwnedSlugs: ["huggingface/DeepSeek-V4-Flash-0731", "openai/kept-model"],
+      });
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
   test("provider remove rejects default provider", () => {
     const { dir } = freshConfig();
     try {

tests/management-provider-validation.test.ts

@@ -1072,6 +1072,69 @@ describe("provider management validation", () => {
     }
   });
 
+  test("provider deletion removes that provider's custom models (#1273)", async () => {
+    if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true });
+    mkdirSync(TEST_DIR, { recursive: true });
+    process.env.OPENCODEX_HOME = TEST_DIR;
+    saveConfig({
+      port: 0,
+      defaultProvider: "test-openai",
+      providers: {
+        "test-openai": {
+          adapter: "openai-chat",
+          baseUrl: "https://api.example.test/v1",
+          apiKey: "sk-secret-value",
+        },
+        removable: {
+          adapter: "openai-chat",
+          baseUrl: "https://api.removable.test/v1",
+          apiKey: "sk-removable",
+        },
+      },
+      customModels: [
+        { id: "keep-1", provider: "test-openai", modelId: "kept-model" },
+        { id: "drop-1", provider: "removable", modelId: "ghost-model" },
+      ],
+      // Seeded so the assertion below covers the real persistence path, not just
+      // the helper: `projectCustomModelCatalogMigration` runs inside the save and
+      // must carry this marker through a provider delete unchanged.
+      customModelCatalogMigration: {
+        version: 1,
+        legacyOwnedSlugs: ["removable/ghost-model", "test-openai/kept-model"],
+      },
+    } as unknown as Parameters<typeof saveConfig>[0]);
+
+    const server = startServer(0);
+    try {
+      const response = await fetch(new URL("/api/providers?name=removable", server.url), {
+        method: "DELETE",
+      });
+      expect(response.status).toBe(200);
+      expect(await response.json()).toMatchObject({ success: true, droppedCustomModels: 1 });
+
+      // The dashboard model page reads this route; a surviving row here is the
+      // ghost model users see pointing at a provider that no longer exists.
+      const customModels = await fetch(new URL("/api/custom-models", server.url));
+      expect(await customModels.json()).toEqual([
+        { id: "keep-1", provider: "test-openai", modelId: "kept-model" },
+      ]);
+
+      const persisted = JSON.parse(readFileSync(join(TEST_DIR, "config.json"), "utf8")) as {
+        customModels?: unknown;
+        customModelCatalogMigration?: unknown;
+      };
+      expect(persisted.customModels).toEqual([
+        { id: "keep-1", provider: "test-openai", modelId: "kept-model" },
+      ]);
+      expect(persisted.customModelCatalogMigration).toEqual({
+        version: 1,
+        legacyOwnedSlugs: ["removable/ghost-model", "test-openai/kept-model"],
+      });
+    } finally {
+      await server.stop(true);
+    }
+  });
+
   test("provider management switches the default and reassigns it when removed", async () => {
     if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true });
     mkdirSync(TEST_DIR, { recursive: true });

tests/provider-id-rewrite.test.ts

@@ -1,7 +1,7 @@
 import { expect, test } from "bun:test";
 import { comboConfigError } from "../src/combos";
 import { providerContextCap } from "../src/providers/context-cap";
-import { rewriteProviderReferences } from "../src/providers/provider-id-rewrite";
+import { dropProviderCustomModels, rewriteProviderReferences } from "../src/providers/provider-id-rewrite";
 import type { OcxConfig, OcxProviderConfig } from "../src/types";
 
 const FROM = "alibaba-token-plan";
@@ -131,3 +131,81 @@ test("does not touch providers[*].selectedModels", () => {
   expect(rewriteProviderReferences(config, FROM, TO).changed).toBe(0);
   expect(config.providers.openrouter!.selectedModels).toEqual([`${FROM}/qwen3.7-max`]);
 });
+
+// ---------------------------------------------------------------------------
+// dropProviderCustomModels — the removal sibling of the rename pass (#1273)
+// ---------------------------------------------------------------------------
+
+function customModelsConfig(models: Array<{ id: string; provider: string; modelId: string }>): OcxConfig {
+  return {
+    providers: {
+      huggingface: { adapter: "openai-chat" },
+      "agnes-ai": { adapter: "openai-chat" },
+    },
+    customModels: models,
+  } as unknown as OcxConfig;
+}
+
+test("removal drops only the departing provider's custom models", () => {
+  const config = customModelsConfig([
+    { id: "a", provider: "agnes-ai", modelId: "agnes-2.5-flash" },
+    { id: "b", provider: "huggingface", modelId: "DeepSeek-V4-Flash-0731" },
+    { id: "c", provider: "huggingface", modelId: "another-model" },
+  ]);
+
+  expect(dropProviderCustomModels(config, "huggingface")).toBe(2);
+  expect(config.customModels).toEqual([
+    { id: "a", provider: "agnes-ai", modelId: "agnes-2.5-flash" },
+  ] as OcxConfig["customModels"]);
+});
+
+test("removing the last custom model deletes the key rather than leaving []", () => {
+  // The add/remove routes drop an emptied list, so the `customModels` field is
+  // absent either way. Only that field: the `customModelCatalogMigration`
+  // marker is deliberately preserved, so the two configs are not identical.
+  const config = customModelsConfig([
+    { id: "b", provider: "huggingface", modelId: "DeepSeek-V4-Flash-0731" },
+  ]);
+
+  expect(dropProviderCustomModels(config, "huggingface")).toBe(1);
+  expect(Object.hasOwn(config, "customModels")).toBe(false);
+});
+
+test("a provider with no custom models is a no-op that leaves the array identical", () => {
+  const rows = [{ id: "a", provider: "agnes-ai", modelId: "agnes-2.5-flash" }];
+  const config = customModelsConfig(rows);
+  const before = config.customModels;
+
+  expect(dropProviderCustomModels(config, "huggingface")).toBe(0);
+  // Same reference, not merely a deep-equal copy: an untouched save must not
+  // look like a mutation to anything comparing identity.
+  expect(config.customModels).toBe(before);
+});
+
+test("an absent customModels key is left absent", () => {
+  const config = { providers: { huggingface: { adapter: "openai-chat" } } } as unknown as OcxConfig;
+  expect(dropProviderCustomModels(config, "huggingface")).toBe(0);
+  expect(Object.hasOwn(config, "customModels")).toBe(false);
+});
+
+test("removal leaves the custom-model ownership marker untouched", () => {
+  // legacyOwnedSlugs records one-time ownership of pre-marker rows. Rewriting it
+  // here would change an older binary's view of what it may delete, which the
+  // migration module explicitly warns against.
+  const config = customModelsConfig([
+    { id: "a", provider: "agnes-ai", modelId: "agnes-2.5-flash" },
+    { id: "b", provider: "huggingface", modelId: "DeepSeek-V4-Flash-0731" },
+  ]);
+  const marker = {
+    version: 1,
+    legacyOwnedSlugs: ["agnes-ai/agnes-2.5-flash", "huggingface/DeepSeek-V4-Flash-0731"],
+  };
+  (config as unknown as Record<string, unknown>).customModelCatalogMigration = marker;
+
+  dropProviderCustomModels(config, "huggingface");
+
+  expect((config as unknown as Record<string, unknown>).customModelCatalogMigration).toEqual({
+    version: 1,
+    legacyOwnedSlugs: ["agnes-ai/agnes-2.5-flash", "huggingface/DeepSeek-V4-Flash-0731"],
+  });
+});