Remove custom providers from settings (#1951)

* feat(providers): remove custom providers from settings Add a destructive removal flow so mistaken custom providers can be deleted from config.json instead of only disabled. * test(app): cover provider removal with e2e Move provider removal coverage out of mocked component tests and into the real Settings flow. * fix(providers): keep removal live after config updates
2026-07-16 16:11:51 +02:00
parent 6aba0370ae
commit e528a0db06
23 changed files with 920 additions and 35 deletions
@@ -2272,6 +2272,47 @@ test("updateProviderRegistry registers a previously unknown provider", async ()
  expect(snapshot.config.provider).toBe("codex");
 });

+test("updateProviderRegistry removes providers omitted from the next registry", async () => {
+  const workdir = mkdtempSync(join(tmpdir(), "agent-manager-test-"));
+  const storagePath = join(workdir, "agents");
+  const storage = new AgentStorage(storagePath, logger);
+  const removedProvider = "zai-claude" as AgentProvider;
+  class RemovedProviderClient extends TestAgentClient {
+    createSessionCalls = 0;
+
+    override async createSession(config: AgentSessionConfig): Promise<AgentSession> {
+      this.createSessionCalls += 1;
+      return await super.createSession(config);
+    }
+  }
+
+  const removedClient = new RemovedProviderClient();
+  const manager = new AgentManager({
+    clients: { codex: new TestAgentClient(), [removedProvider]: removedClient },
+    providerDefinitions: {
+      codex: { enabled: true },
+      [removedProvider]: { enabled: true },
+    },
+    registry: storage,
+    logger,
+  });
+
+  expect(manager.getRegisteredProviderIds()).toContain(removedProvider);
+
+  manager.updateProviderRegistry({
+    providerDefinitions: { codex: { enabled: true } },
+    clients: { codex: new TestAgentClient() },
+  });
+
+  expect(manager.getRegisteredProviderIds()).not.toContain(removedProvider);
+  await expect(
+    manager.createAgent({ provider: removedProvider, cwd: workdir }, undefined, {
+      workspaceId: undefined,
+    }),
+  ).rejects.toThrow("Unknown provider 'zai-claude'");
+  expect(removedClient.createSessionCalls).toBe(0);
+});
+
 test("createAgent passes explicit model strings through to the provider", async () => {
  const workdir = mkdtempSync(join(tmpdir(), "agent-manager-test-"));
  const storagePath = join(workdir, "agents");
@@ -622,11 +622,14 @@ export class AgentManager {
    providerDefinitions: ProviderEnabledMap;
    clients: ProviderClientMap;
  }): void {
+    this.providerEnabled.clear();
    for (const [provider, definition] of Object.entries(input.providerDefinitions)) {
      if (definition) {
        this.providerEnabled.set(provider, definition.enabled);
      }
    }
+
+    this.clients.clear();
    for (const [provider, client] of Object.entries(input.clients)) {
      if (client) {
        this.clients.set(provider, client);
@@ -105,6 +105,7 @@ export interface ProviderSnapshotEntry {
  provider: AgentProvider;
  status: ProviderStatus;
  enabled: boolean;
+  source?: "builtin" | "custom";
  error?: string;
  models?: AgentModelDefinition[];
  modes?: AgentMode[];
@@ -952,6 +952,32 @@ describe("ProviderSnapshotManager applyMutableProviderConfig", () => {
      expect(manager.hasProvider("zai-claude")).toBe(true);
      expect(state.providerDefinitions["zai-claude"]).toMatchObject({ enabled: true });
      expect(manager.listRegisteredProviderIds()).toContain("zai-claude");
+      expect(manager.getSnapshot().find((entry) => entry.provider === "zai-claude")?.source).toBe(
+        "custom",
+      );
+    } finally {
+      manager.destroy();
+    }
+  });
+
+  test("removes startup provider overrides from the live registry", () => {
+    const manager = new ProviderSnapshotManager({
+      logger: createTestLogger(),
+      providerOverrides: {
+        "zai-claude": { extends: "claude", label: "ZAI", enabled: true },
+      },
+    });
+    try {
+      expect(manager.hasProvider("zai-claude")).toBe(true);
+
+      const state = manager.applyMutableProviderConfig({}, { removeProviders: ["zai-claude"] });
+
+      expect(manager.hasProvider("zai-claude")).toBe(false);
+      expect(state.providerDefinitions["zai-claude"]).toBeUndefined();
+      expect(manager.getSnapshot().some((entry) => entry.provider === "zai-claude")).toBe(false);
+
+      manager.applyMutableProviderConfig({ codex: { enabled: false } });
+      expect(manager.hasProvider("zai-claude")).toBe(false);
    } finally {
      manager.destroy();
    }
@@ -27,6 +27,7 @@ import {
  shutdownAgentClients,
  type ProviderDefinition,
 } from "./provider-registry.js";
+import { BUILTIN_PROVIDER_IDS } from "@getpaseo/protocol/provider-manifest";
 import { applyMutableProviderConfigToOverrides } from "../daemon-config-store.js";
 import {
  formatProviderDiagnostic,
@@ -64,6 +65,22 @@ function resolveDiagnosticTimeoutMs(option: number | undefined, refreshTimeoutMs
  return Math.max(refreshTimeoutMs, DEFAULT_DIAGNOSTIC_TIMEOUT_MS);
 }

+function omitProviderOverrides(
+  overrides: Record<string, ProviderOverride> | undefined,
+  providers: readonly string[],
+): Record<string, ProviderOverride> | undefined {
+  if (!overrides || providers.length === 0) {
+    return overrides;
+  }
+
+  const nextOverrides = { ...overrides };
+  for (const provider of providers) {
+    delete nextOverrides[provider];
+  }
+
+  return Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined;
+}
+
 type ProviderSnapshotChangeListener = (entries: ProviderSnapshotEntry[], cwd: string) => void;

 export interface ProviderSnapshotManagerOptions {
@@ -94,6 +111,10 @@ interface ProviderSnapshotReadOptions {
  wait?: boolean;
 }

+interface ApplyMutableProviderConfigOptions {
+  removeProviders?: readonly string[];
+}
+
 interface ProviderSnapshotProviderOptions {
  cwd?: string | null;
  provider: AgentProvider;
@@ -163,7 +184,7 @@ export class ProviderSnapshotManager {
  private readonly extraClients: Partial<Record<AgentProvider, AgentClient>>;
  private runtimeSettings: AgentProviderRuntimeSettingsMap | undefined;
  private providerOverrides: Record<string, ProviderOverride> | undefined;
-  private readonly baseProviderOverrides: Record<string, ProviderOverride> | undefined;
+  private baseProviderOverrides: Record<string, ProviderOverride> | undefined;
  private providerRegistry: Record<AgentProvider, ProviderDefinition>;
  private providerClients: Record<AgentProvider, AgentClient>;

@@ -369,7 +390,12 @@ export class ProviderSnapshotManager {

  applyMutableProviderConfig(
    mutableProviders: MutableDaemonConfig["providers"] | undefined,
+    options: ApplyMutableProviderConfigOptions = {},
  ): AgentManagerProviderState {
+    this.baseProviderOverrides = omitProviderOverrides(
+      this.baseProviderOverrides,
+      options.removeProviders ?? [],
+    );
    this.providerOverrides = applyMutableProviderConfigToOverrides(
      this.baseProviderOverrides,
      mutableProviders,
@@ -503,6 +529,7 @@ export class ProviderSnapshotManager {
        provider,
        status: "error",
        enabled: definition.enabled,
+        source: this.getProviderSource(provider),
        label: definition.label,
        description: definition.description,
        defaultModeId: definition.defaultModeId,
@@ -536,6 +563,11 @@ export class ProviderSnapshotManager {
    }
  }

+  private getProviderSource(provider: AgentProvider): ProviderSnapshotEntry["source"] {
+    const isBuiltin = BUILTIN_PROVIDER_IDS.includes(provider);
+    return !isBuiltin && this.providerOverrides?.[provider]?.extends ? "custom" : "builtin";
+  }
+
  private createLoadingEntries(): Map<AgentProvider, ProviderSnapshotEntry> {
    const entries = new Map<AgentProvider, ProviderSnapshotEntry>();
    for (const provider of this.getProviderIds()) {
@@ -544,6 +576,7 @@ export class ProviderSnapshotManager {
        provider,
        status: "loading",
        enabled: definition?.enabled ?? true,
+        source: this.getProviderSource(provider),
        label: definition?.label,
        description: definition?.description,
        defaultModeId: definition?.defaultModeId ?? null,
@@ -562,6 +595,7 @@ export class ProviderSnapshotManager {
      const metadata = {
        provider,
        enabled: definition?.enabled ?? true,
+        source: this.getProviderSource(provider),
        label: definition?.label,
        description: definition?.description,
        defaultModeId: definition?.defaultModeId ?? null,
@@ -725,6 +759,7 @@ export class ProviderSnapshotManager {
    const snapshot = this.getOrCreateSnapshot(snapshotCwd);
    const base = {
      provider,
+      source: this.getProviderSource(provider),
      label: definition.label,
      description: definition.description,
      defaultModeId: definition.defaultModeId,
@@ -119,6 +119,220 @@ describe("DaemonConfigStore", () => {
    });
  });

+  test("patch removes provider entries from config.json", () => {
+    const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-"));
+    tempDirs.push(paseoHome);
+
+    const configPath = path.join(paseoHome, "config.json");
+    writeFileSync(
+      configPath,
+      `${JSON.stringify(
+        {
+          version: 1,
+          agents: {
+            providers: {
+              gemini: {
+                extends: "acp",
+                label: "Gemini",
+                command: ["gemini", "--acp"],
+              },
+              claude: {
+                enabled: false,
+              },
+            },
+          },
+        },
+        null,
+        2,
+      )}\n`,
+    );
+
+    const store = new DaemonConfigStore(
+      paseoHome,
+      {
+        mcp: { injectIntoAgents: false },
+        browserTools: { enabled: false },
+        providers: {
+          gemini: {},
+          claude: { enabled: false },
+        },
+        metadataGeneration: { providers: [] },
+        autoArchiveAfterMerge: false,
+        enableTerminalAgentHooks: false,
+        appendSystemPrompt: "",
+      },
+      undefined,
+    );
+
+    const next = store.patch({ removeProviders: ["gemini"] });
+
+    expect(next.providers.gemini).toBeUndefined();
+    expect(next.providers.claude).toEqual({ enabled: false });
+    const persisted = loadPersistedConfig(paseoHome);
+    expect(persisted.agents?.providers?.gemini).toBeUndefined();
+    expect(persisted.agents?.providers?.claude).toEqual({ enabled: false });
+  });
+
+  test("patch removes the providers object when the last provider is deleted", () => {
+    const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-"));
+    tempDirs.push(paseoHome);
+
+    const configPath = path.join(paseoHome, "config.json");
+    writeFileSync(
+      configPath,
+      `${JSON.stringify(
+        {
+          version: 1,
+          agents: {
+            providers: {
+              gemini: {
+                extends: "acp",
+                label: "Gemini",
+                command: ["gemini", "--acp"],
+              },
+            },
+          },
+        },
+        null,
+        2,
+      )}\n`,
+    );
+
+    const store = new DaemonConfigStore(
+      paseoHome,
+      {
+        mcp: { injectIntoAgents: false },
+        browserTools: { enabled: false },
+        providers: { gemini: {} },
+        metadataGeneration: { providers: [] },
+        autoArchiveAfterMerge: false,
+        enableTerminalAgentHooks: false,
+        appendSystemPrompt: "",
+      },
+      undefined,
+    );
+
+    store.patch({ removeProviders: ["gemini"] });
+
+    const persisted = loadPersistedConfig(paseoHome);
+    expect(persisted.agents?.providers).toBeUndefined();
+  });
+
+  test("patch removes deleted providers from metadata generation", () => {
+    const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-"));
+    tempDirs.push(paseoHome);
+
+    const configPath = path.join(paseoHome, "config.json");
+    writeFileSync(
+      configPath,
+      `${JSON.stringify(
+        {
+          version: 1,
+          agents: {
+            providers: {
+              gemini: {
+                extends: "acp",
+                label: "Gemini",
+                command: ["gemini", "--acp"],
+              },
+              claude: {
+                enabled: false,
+              },
+            },
+            metadataGeneration: {
+              providers: [
+                { provider: "gemini", model: "flash" },
+                { provider: "claude", model: "haiku" },
+              ],
+            },
+          },
+        },
+        null,
+        2,
+      )}\n`,
+    );
+
+    const store = new DaemonConfigStore(
+      paseoHome,
+      {
+        mcp: { injectIntoAgents: false },
+        browserTools: { enabled: false },
+        providers: {
+          gemini: {},
+          claude: { enabled: false },
+        },
+        metadataGeneration: {
+          providers: [
+            { provider: "gemini", model: "flash" },
+            { provider: "claude", model: "haiku" },
+          ],
+        },
+        autoArchiveAfterMerge: false,
+        enableTerminalAgentHooks: false,
+        appendSystemPrompt: "",
+      },
+      undefined,
+    );
+
+    const next = store.patch({ removeProviders: ["gemini"] });
+
+    expect(next.metadataGeneration.providers).toEqual([{ provider: "claude", model: "haiku" }]);
+    const persisted = loadPersistedConfig(paseoHome);
+    expect(persisted.agents?.metadataGeneration).toEqual({
+      providers: [{ provider: "claude", model: "haiku" }],
+    });
+  });
+
+  test("patch persists provider removal when in-memory config is already clean", () => {
+    const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-"));
+    tempDirs.push(paseoHome);
+
+    const configPath = path.join(paseoHome, "config.json");
+    writeFileSync(
+      configPath,
+      `${JSON.stringify(
+        {
+          version: 1,
+          agents: {
+            providers: {
+              gemini: {
+                extends: "acp",
+                label: "Gemini",
+                command: ["gemini", "--acp"],
+              },
+            },
+            metadataGeneration: {
+              providers: [{ provider: "gemini", model: "flash" }],
+            },
+          },
+        },
+        null,
+        2,
+      )}\n`,
+    );
+
+    const store = new DaemonConfigStore(
+      paseoHome,
+      {
+        mcp: { injectIntoAgents: false },
+        browserTools: { enabled: false },
+        providers: {},
+        metadataGeneration: { providers: [] },
+        autoArchiveAfterMerge: false,
+        enableTerminalAgentHooks: false,
+        appendSystemPrompt: "",
+      },
+      undefined,
+    );
+
+    const next = store.patch({ removeProviders: ["gemini"] });
+
+    expect(next.providers.gemini).toBeUndefined();
+    const persisted = loadPersistedConfig(paseoHome);
+    expect(persisted.agents?.providers).toBeUndefined();
+    expect(persisted.agents?.metadataGeneration).toEqual({ providers: [] });
+  });
+
  test("patch persists append system prompt into config.json", () => {
    const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-"));
    tempDirs.push(paseoHome);
@@ -20,7 +20,11 @@ interface LoggerLike {
  info(...args: unknown[]): void;
 }

-type ConfigListener = (config: MutableDaemonConfig) => void;
+export interface DaemonConfigChangeDetails {
+  removedProviders: readonly string[];
+}
+
+type ConfigListener = (config: MutableDaemonConfig, details: DaemonConfigChangeDetails) => void;
 type FieldChangeHandler = (value: unknown) => void;

 function getLogger(logger: LoggerLike | undefined): LoggerLike | undefined {
@@ -52,6 +56,77 @@ function deepMerge<T extends Record<string, unknown>>(
  return next as T;
 }

+function omitProvidersFromConfig<T extends { providers?: Record<string, unknown> }>(
+  config: T,
+  providers: readonly string[],
+): T {
+  if (providers.length === 0 || !config.providers) {
+    return config;
+  }
+
+  let changed = false;
+  const nextProviders = { ...config.providers };
+  for (const provider of providers) {
+    if (provider in nextProviders) {
+      delete nextProviders[provider];
+      changed = true;
+    }
+  }
+
+  return changed ? ({ ...config, providers: nextProviders } as T) : config;
+}
+
+function omitMetadataGenerationProvidersFromConfig<
+  T extends { metadataGeneration?: { providers?: Array<{ provider?: unknown }> } },
+>(config: T, providers: readonly string[]): T {
+  if (providers.length === 0 || !config.metadataGeneration?.providers) {
+    return config;
+  }
+
+  const removedProviderIds = new Set(providers);
+  const nextProviders = config.metadataGeneration.providers.filter((entry) => {
+    return typeof entry.provider !== "string" || !removedProviderIds.has(entry.provider);
+  });
+  if (nextProviders.length === config.metadataGeneration.providers.length) {
+    return config;
+  }
+
+  return {
+    ...config,
+    metadataGeneration: {
+      ...config.metadataGeneration,
+      providers: nextProviders,
+    },
+  } as T;
+}
+
+function omitProvidersFromOverrides(
+  overrides: Record<string, ProviderOverride> | undefined,
+  providers: readonly string[],
+): Record<string, ProviderOverride> | undefined {
+  if (!overrides) {
+    return undefined;
+  }
+
+  const nextOverrides = { ...overrides };
+  for (const provider of providers) {
+    delete nextOverrides[provider];
+  }
+
+  return Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined;
+}
+
+function omitProvidersFromPersistedAgents(
+  agents: PersistedConfig["agents"],
+): Record<string, unknown> | undefined {
+  if (!agents) {
+    return undefined;
+  }
+
+  const { providers: _providers, ...rest } = agents as Record<string, unknown>;
+  return Object.keys(rest).length > 0 ? rest : undefined;
+}
+
 function getValueAtPath(config: MutableDaemonConfig, path: string): unknown {
  return path
    .split(".")
@@ -100,19 +175,32 @@ export class DaemonConfigStore {

  public patch(partial: MutableDaemonConfigPatch): MutableDaemonConfig {
    const parsedPatch = MutableDaemonConfigPatchSchema.parse(partial);
-    const next = MutableDaemonConfigSchema.parse(deepMerge(this.current, parsedPatch));
+    const { removeProviders = [], ...configPatch } = parsedPatch;
+    const removedProviders = Array.from(new Set(removeProviders));
+    const merged = deepMerge(this.current, configPatch);
+    const next = MutableDaemonConfigSchema.parse(
+      omitMetadataGenerationProvidersFromConfig(
+        omitProvidersFromConfig(merged, removedProviders),
+        removedProviders,
+      ),
+    );

    const changedFieldPaths = Array.from(this.fieldChangeHandlers.keys()).filter((path) => {
      return !isEqualValue(getValueAtPath(this.current, path), getValueAtPath(next, path));
    });
+    const configChanged = !isEqualValue(this.current, next);

-    if (changedFieldPaths.length === 0 && isEqualValue(this.current, next)) {
+    if (!configChanged && removedProviders.length === 0) {
      return this.current;
    }

    // Persist before updating in-memory state so that if persistence fails,
    // runtime and disk stay consistent.
-    this.persistConfig(next);
+    this.persistConfig(next, removedProviders);
+    if (!configChanged) {
+      return this.current;
+    }
+
    this.current = next;

    for (const path of changedFieldPaths) {
@@ -126,8 +214,9 @@ export class DaemonConfigStore {
      }
    }

+    const changeDetails: DaemonConfigChangeDetails = { removedProviders };
    for (const listener of this.changeListeners) {
-      listener(next);
+      listener(next, changeDetails);
    }

    return next;
@@ -157,11 +246,12 @@ export class DaemonConfigStore {
    };
  }

-  private persistConfig(config: MutableDaemonConfig): void {
+  private persistConfig(config: MutableDaemonConfig, removeProviders: readonly string[]): void {
    const persisted = loadPersistedConfig(this.paseoHome, this.logger);
    const nextPersisted = mergeMutableConfigIntoPersistedConfig({
      persisted,
      mutable: config,
+      removeProviders,
    });
    savePersistedConfig(this.paseoHome, nextPersisted, this.logger);
  }
@@ -170,22 +260,27 @@ export class DaemonConfigStore {
 function mergeMutableConfigIntoPersistedConfig(params: {
  persisted: PersistedConfig;
  mutable: MutableDaemonConfig;
+  removeProviders: readonly string[];
 }): PersistedConfig {
-  const { persisted, mutable } = params;
+  const { persisted, mutable, removeProviders } = params;
  const browserToolsEnabled = readBrowserToolsEnabled(mutable);
  const metadataGenerationProviders = readMetadataGenerationProviders(mutable);
-  const providerOverrides = applyMutableProviderConfigToOverrides(
+  const persistedProviderOverrides = omitProvidersFromOverrides(
    persisted.agents?.providers as Record<string, ProviderOverride> | undefined,
+    removeProviders,
+  );
+  const providerOverrides = applyMutableProviderConfigToOverrides(
+    persistedProviderOverrides,
    mutable.providers,
  );
-  const persistedAgents = persisted.agents as Record<string, unknown> | undefined;
+  const persistedAgents = omitProvidersFromPersistedAgents(persisted.agents);
  const persistedMetadataGeneration = {
    providers: metadataGenerationProviders,
  };
  const shouldPersistMetadataGeneration =
    metadataGenerationProviders.length > 0 || persisted.agents?.metadataGeneration !== undefined;

-  let nextAgents = persisted.agents as PersistedConfig["agents"];
+  let nextAgents = persistedAgents as PersistedConfig["agents"];
  if (providerOverrides && Object.keys(providerOverrides).length > 0) {
    nextAgents = {
      ...persistedAgents,
@@ -554,9 +554,10 @@ export class VoiceAssistantWebSocketServer {
      this.speech?.onReadinessChange((snapshot) => {
        this.publishSpeechReadiness(snapshot);
      }) ?? null;
-    this.unsubscribeDaemonConfigChange = this.daemonConfigStore.onChange((config) => {
+    this.unsubscribeDaemonConfigChange = this.daemonConfigStore.onChange((config, details) => {
      const nextAgentManagerState = this.providerSnapshotManager.applyMutableProviderConfig(
        config.providers,
+        { removeProviders: details.removedProviders },
      );
      this.agentManager.updateProviderRegistry(nextAgentManagerState);
      this.broadcastDaemonConfigChanged(config);
@@ -1252,6 +1253,8 @@ export class VoiceAssistantWebSocketServer {
        workspaceGithubRepositorySearch: true,
        // COMPAT(projectCreateDirectory): added in v0.1.108, remove gate after 2027-01-15.
        projectCreateDirectory: true,
+        // COMPAT(providerRemoval): added in v0.1.105, drop the gate when floor >= v0.1.105.
+        providerRemoval: true,
      },
    };
  }