Make provider snapshot refresh explicit (#988)

docs/providers.md

@@ -18,6 +18,16 @@ Existing direct providers: `claude` (in `providers/claude/agent.ts`), `codex` (`
 
 ---
 
+## Provider Snapshot Refresh Contract
+
+The daemon keeps one global provider snapshot, keyed to the home directory, for settings, selectors, and old model/mode list requests. Snapshot reads may probe providers only while the snapshot is cold. Once an entry is warm, its `ready`, `error`, or `unavailable` state stays cached until the user forces a refresh from settings/provider management.
+
+Do not add TTL revalidation, focus-triggered refreshes, selector-open refreshes, or config-reload refreshes. Registry/config replacement may update visible metadata such as label, description, default mode, enabled state, and provider membership, but it must not spawn provider processes. If a provider needs to be re-probed after a config change, route that through the explicit settings refresh path.
+
+Boundary tests should assert observable behavior: cold reads may call provider availability/model/mode discovery; warm reads and registry replacement must not; explicit full or targeted refreshes must.
+
+---
+
 ## ACP Provider Checklist
 
 ### 1. Create the provider class

packages/app/src/screens/settings/providers-section.test.tsx

@@ -347,6 +347,26 @@ describe("ProvidersSection", () => {
     expect(patchConfigMock).toHaveBeenCalledWith({
       providers: { claude: { enabled: false } },
     });
+    expect(refreshMock).not.toHaveBeenCalled();
     expect(container?.querySelector('[data-testid="provider-diagnostic-sheet"]')).toBeNull();
   });
+
+  it("forces a provider snapshot refresh from the settings refresh action", async () => {
+    snapshotState.entries = [claudeEntry];
+    configState.config = makeConfig();
+
+    render();
+
+    const refreshButton = container?.querySelector<HTMLElement>(
+      '[role="button"][aria-label="Refresh providers"]',
+    );
+    expect(refreshButton).not.toBeNull();
+
+    await act(async () => {
+      refreshButton?.dispatchEvent(new window.MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(refreshMock).toHaveBeenCalledTimes(1);
+    expect(patchConfigMock).not.toHaveBeenCalled();
+  });
 });

packages/server/src/server/agent/provider-snapshot-manager.test.ts

@@ -51,7 +51,6 @@ const TEST_CAPABILITIES = {
 } as const;
 
 describe("ProviderSnapshotManager", () => {
-  const ttlMs = 5 * 60 * 1_000;
   const projectCwd = resolve("/tmp/project");
   const projectACwd = resolve("/tmp/project-a");
   const projectBCwd = resolve("/tmp/project-b");
@@ -663,12 +662,10 @@ describe("ProviderSnapshotManager", () => {
     fetchModels.resolve([createModel("codex", "gpt-5.2")]);
   });
 
-  test("getSnapshot returns stale ready entries and starts background warm-up when snapshot is older than TTL", async () => {
-    let now = 1_000;
+  test("warm getSnapshot keeps ready entries cached without probing again", async () => {
     const fetchModels = vi
       .fn<(cwd?: string) => Promise<AgentModelDefinition[]>>()
-      .mockResolvedValueOnce([createModel("codex", "gpt-5.1")])
-      .mockResolvedValueOnce([createModel("codex", "gpt-5.2")]);
+      .mockResolvedValue([createModel("codex", "gpt-5.1")]);
     const { registry, handles } = createRegistry([
       createMockProvider({
         provider: "codex",
@@ -676,10 +673,7 @@ describe("ProviderSnapshotManager", () => {
         fetchModes: async () => [createMode("auto")],
       }),
     ]);
-    const manager = new ProviderSnapshotManager(registry, createTestLogger(), {
-      ttlMs,
-      now: () => now,
-    });
+    const manager = new ProviderSnapshotManager(registry, createTestLogger());
 
     manager.getSnapshot(projectCwd);
 
@@ -689,114 +683,26 @@ describe("ProviderSnapshotManager", () => {
       );
     });
 
-    now += ttlMs + 1;
+    const firstWarmRead = manager.getSnapshot(projectCwd);
+    const secondWarmRead = manager.getSnapshot(projectCwd);
+    const thirdWarmRead = manager.getSnapshot(projectCwd);
 
-    const staleSnapshot = manager.getSnapshot(projectCwd);
-
-    expect(getProviderEntry(staleSnapshot, "codex")).toMatchObject({
+    expect(getProviderEntry(firstWarmRead, "codex")).toMatchObject({
       provider: "codex",
       status: "ready",
       models: [createModel("codex", "gpt-5.1")],
       modes: [createMode("auto")],
     });
-
-    await vi.waitFor(() => {
-      expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(2);
-    });
-
-    await vi.waitFor(() => {
-      expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.models?.[0]?.id).toBe(
-        "gpt-5.2",
-      );
-    });
-
-    manager.destroy();
-  });
-
-  test("getSnapshot does not trigger a second warm-up while a stale re-warm is already in flight", async () => {
-    let now = 2_000;
-    const staleRefreshModels = deferred<AgentModelDefinition[]>();
-    const fetchModels = vi
-      .fn<(cwd?: string) => Promise<AgentModelDefinition[]>>()
-      .mockResolvedValueOnce([createModel("codex", "gpt-5.1")])
-      .mockImplementationOnce(async () => staleRefreshModels.promise);
-    const { registry, handles } = createRegistry([
-      createMockProvider({
-        provider: "codex",
-        fetchModels: async (cwd) => fetchModels(cwd),
-        fetchModes: async () => [createMode("auto")],
-      }),
-    ]);
-    const manager = new ProviderSnapshotManager(registry, createTestLogger(), {
-      ttlMs,
-      now: () => now,
-    });
-
-    manager.getSnapshot(projectCwd);
-
-    await vi.waitFor(() => {
-      expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.models?.[0]?.id).toBe(
-        "gpt-5.1",
-      );
-    });
-
-    now += ttlMs + 1;
-
-    const firstStaleSnapshot = manager.getSnapshot(projectCwd);
-    const secondStaleSnapshot = manager.getSnapshot(projectCwd);
-
-    expect(getProviderEntry(firstStaleSnapshot, "codex")?.models?.[0]?.id).toBe("gpt-5.1");
-    expect(getProviderEntry(secondStaleSnapshot, "codex")?.models?.[0]?.id).toBe("gpt-5.1");
-
-    await vi.waitFor(() => {
-      expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(2);
-    });
-
-    staleRefreshModels.resolve([createModel("codex", "gpt-5.2")]);
-
-    await vi.waitFor(() => {
-      expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.models?.[0]?.id).toBe(
-        "gpt-5.2",
-      );
-    });
-
-    expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(2);
-
-    manager.destroy();
-  });
-
-  test("getSnapshot does not re-warm when the cached snapshot is still fresh", async () => {
-    let now = 3_000;
-    const { registry, handles } = createRegistry([
-      createMockProvider({
-        provider: "codex",
-        fetchModels: async () => [createModel("codex", "gpt-5.1")],
-        fetchModes: async () => [createMode("auto")],
-      }),
-    ]);
-    const manager = new ProviderSnapshotManager(registry, createTestLogger(), {
-      ttlMs,
-      now: () => now,
-    });
-
-    manager.getSnapshot(projectCwd);
-
-    await vi.waitFor(() => {
-      expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.status).toBe("ready");
-    });
-
-    now += ttlMs - 1;
-
-    const freshSnapshot = manager.getSnapshot(projectCwd);
-
-    expect(getProviderEntry(freshSnapshot, "codex")?.models?.[0]?.id).toBe("gpt-5.1");
+    expect(getProviderEntry(secondWarmRead, "codex")?.models?.[0]?.id).toBe("gpt-5.1");
+    expect(getProviderEntry(thirdWarmRead, "codex")?.models?.[0]?.id).toBe("gpt-5.1");
+    expect(handles.codex?.isAvailable).toHaveBeenCalledTimes(1);
     expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(1);
+    expect(handles.codex?.fetchModes).toHaveBeenCalledTimes(1);
 
     manager.destroy();
   });
 
-  test("getSnapshot re-warms snapshots in error and unavailable states after TTL", async () => {
-    let now = 4_000;
+  test("warm error and unavailable entries stay cached until explicit refresh", async () => {
     const unavailableFetchModels = vi
       .fn<(cwd?: string) => Promise<AgentModelDefinition[]>>()
       .mockResolvedValue([createModel("codex", "gpt-5.2")]);
@@ -821,10 +727,7 @@ describe("ProviderSnapshotManager", () => {
         fetchModes: async () => [createMode("default")],
       }),
     ]);
-    const manager = new ProviderSnapshotManager(registry, createTestLogger(), {
-      ttlMs,
-      now: () => now,
-    });
+    const manager = new ProviderSnapshotManager(registry, createTestLogger());
 
     manager.getSnapshot(projectCwd);
 
@@ -835,12 +738,17 @@ describe("ProviderSnapshotManager", () => {
       expect(getProviderEntry(manager.getSnapshot(projectCwd), "claude")?.status).toBe("error");
     });
 
-    now += ttlMs + 1;
+    const firstWarmRead = manager.getSnapshot(projectCwd);
+    const secondWarmRead = manager.getSnapshot(projectCwd);
 
-    const staleSnapshot = manager.getSnapshot(projectCwd);
+    expect(getProviderEntry(firstWarmRead, "codex")?.status).toBe("unavailable");
+    expect(getProviderEntry(firstWarmRead, "claude")?.status).toBe("error");
+    expect(getProviderEntry(secondWarmRead, "codex")?.status).toBe("unavailable");
+    expect(getProviderEntry(secondWarmRead, "claude")?.status).toBe("error");
+    expect(unavailableIsAvailable).toHaveBeenCalledTimes(1);
+    expect(errorFetchModels).toHaveBeenCalledTimes(1);
 
-    expect(getProviderEntry(staleSnapshot, "codex")?.status).toBe("unavailable");
-    expect(getProviderEntry(staleSnapshot, "claude")?.status).toBe("error");
+    await manager.refreshSettingsSnapshot({ providers: ["codex", "claude"] });
 
     await vi.waitFor(() => {
       expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.status).toBe("ready");
@@ -853,28 +761,34 @@ describe("ProviderSnapshotManager", () => {
     expect(getProviderEntry(manager.getSnapshot(projectCwd), "claude")?.models?.[0]?.id).toBe(
       "sonnet",
     );
+    expect(unavailableIsAvailable).toHaveBeenCalledTimes(2);
+    expect(errorFetchModels).toHaveBeenCalledTimes(2);
 
     manager.destroy();
   });
 
-  test("getSnapshot respects an injected TTL", async () => {
-    let now = 5_000;
-    const customTtlMs = 100;
-    const fetchModels = vi
-      .fn<(cwd?: string) => Promise<AgentModelDefinition[]>>()
-      .mockResolvedValueOnce([createModel("codex", "gpt-5.1")])
-      .mockResolvedValueOnce([createModel("codex", "gpt-5.2")]);
-    const { registry, handles } = createRegistry([
+  test("providers added after warm-up stay unprobed until explicit refresh", async () => {
+    const initial = createMockProvider({
+      provider: "codex",
+      fetchModels: async () => [createModel("codex", "gpt-5.1")],
+      fetchModes: async () => [createMode("auto")],
+    });
+    const added = createMockProvider({
+      provider: "zai",
+      label: "Z.AI",
+      fetchModels: async () => [createModel("zai", "glm-4.6")],
+      fetchModes: async () => [createMode("plan")],
+    });
+    const { registry } = createRegistry([initial]);
+    const { registry: nextRegistry, handles: nextHandles } = createRegistry([
       createMockProvider({
         provider: "codex",
-        fetchModels: async (cwd) => fetchModels(cwd),
+        fetchModels: async () => [createModel("codex", "gpt-5.1")],
         fetchModes: async () => [createMode("auto")],
       }),
+      added,
     ]);
-    const manager = new ProviderSnapshotManager(registry, createTestLogger(), {
-      ttlMs: customTtlMs,
-      now: () => now,
-    });
+    const manager = new ProviderSnapshotManager(registry, createTestLogger());
 
     manager.getSnapshot(projectCwd);
 
@@ -884,22 +798,28 @@ describe("ProviderSnapshotManager", () => {
       );
     });
 
-    now += customTtlMs - 1;
-    manager.getSnapshot(projectCwd);
-    expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(1);
+    manager.replaceRegistry(nextRegistry);
 
-    now += 2;
-    manager.getSnapshot(projectCwd);
-
-    await vi.waitFor(() => {
-      expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(2);
+    const snapshot = manager.getSnapshot(projectCwd);
+    expect(getProviderEntry(snapshot, "zai")).toMatchObject({
+      provider: "zai",
+      status: "unavailable",
+      enabled: true,
+      label: "Z.AI",
     });
+    expect(nextHandles.zai?.createClient).not.toHaveBeenCalled();
+    expect(nextHandles.zai?.fetchModels).not.toHaveBeenCalled();
 
-    await vi.waitFor(() => {
-      expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.models?.[0]?.id).toBe(
-        "gpt-5.2",
-      );
+    await manager.refreshSettingsSnapshot({ providers: ["zai"] });
+
+    expect(getProviderEntry(manager.getSnapshot(projectCwd), "zai")).toMatchObject({
+      provider: "zai",
+      status: "ready",
+      models: [createModel("zai", "glm-4.6")],
+      modes: [createMode("plan")],
     });
+    expect(nextHandles.zai?.createClient).toHaveBeenCalledTimes(1);
+    expect(nextHandles.zai?.fetchModels).toHaveBeenCalledTimes(1);
 
     manager.destroy();
   });
@@ -1099,6 +1019,51 @@ describe("ProviderSnapshotManager", () => {
     manager.destroy();
   });
 
+  test("replaceRegistry updates warmed provider metadata without probing", async () => {
+    const original = createMockProvider({
+      provider: "codex",
+      fetchModels: async () => [createModel("codex", "gpt-5.1")],
+      fetchModes: async () => [createMode("auto")],
+    });
+    const updated = createMockProvider({
+      provider: "codex",
+      label: "Codex CLI",
+      description: "Updated provider description",
+      defaultModeId: "agent",
+      fetchModels: async () => [createModel("codex", "gpt-5.2")],
+      fetchModes: async () => [createMode("agent")],
+    });
+    const { registry, handles } = createRegistry([original]);
+    const { registry: nextRegistry, handles: nextHandles } = createRegistry([updated]);
+    const manager = new ProviderSnapshotManager(registry, createTestLogger());
+
+    manager.getSnapshot(projectCwd);
+
+    await vi.waitFor(() => {
+      expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")?.status).toBe("ready");
+    });
+
+    manager.replaceRegistry(nextRegistry);
+    await Promise.resolve();
+
+    expect(getProviderEntry(manager.getSnapshot(projectCwd), "codex")).toMatchObject({
+      provider: "codex",
+      status: "ready",
+      models: [createModel("codex", "gpt-5.1")],
+      modes: [createMode("auto")],
+      label: "Codex CLI",
+      description: "Updated provider description",
+      defaultModeId: "agent",
+    });
+    expect(handles.codex?.fetchModels).toHaveBeenCalledTimes(1);
+    expect(handles.codex?.fetchModes).toHaveBeenCalledTimes(1);
+    expect(nextHandles.codex?.createClient).not.toHaveBeenCalled();
+    expect(nextHandles.codex?.fetchModels).not.toHaveBeenCalled();
+    expect(nextHandles.codex?.fetchModes).not.toHaveBeenCalled();
+
+    manager.destroy();
+  });
+
   test("snapshot includes user-defined providers from the registry", async () => {
     const { registry } = createRegistry([
       createMockProvider({ provider: "claude" }),

packages/server/src/server/agent/provider-snapshot-manager.ts

@@ -8,14 +8,11 @@ import { withTimeout } from "../../utils/promise-timeout.js";
 import type { AgentProvider, ProviderSnapshotEntry } from "./agent-sdk-types.js";
 import type { ProviderDefinition } from "./provider-registry.js";
 
-const DEFAULT_SNAPSHOT_TTL_MS = 300_000;
 const DEFAULT_REFRESH_TIMEOUT_MS = 30_000;
 
 type ProviderSnapshotChangeListener = (entries: ProviderSnapshotEntry[], cwd: string) => void;
 interface ProviderSnapshotManagerOptions {
-  ttlMs?: number;
   refreshTimeoutMs?: number;
-  now?: () => number;
 }
 interface ProviderSnapshotRefreshOptions {
   cwd: string;
@@ -32,13 +29,10 @@ interface ProviderLoad {
 
 export class ProviderSnapshotManager {
   private readonly snapshots = new Map<string, Map<AgentProvider, ProviderSnapshotEntry>>();
-  private readonly lastCheckedAts = new Map<string, number>();
   private readonly providerLoads = new Map<string, Map<AgentProvider, ProviderLoad>>();
   private readonly events = new EventEmitter();
   private destroyed = false;
-  private readonly ttlMs: number;
   private readonly refreshTimeoutMs: number;
-  private readonly now: () => number;
   private providerRegistry: Record<AgentProvider, ProviderDefinition>;
 
   constructor(
@@ -47,9 +41,7 @@ export class ProviderSnapshotManager {
     options: ProviderSnapshotManagerOptions = {},
   ) {
     this.providerRegistry = providerRegistry;
-    this.ttlMs = options.ttlMs ?? DEFAULT_SNAPSHOT_TTL_MS;
     this.refreshTimeoutMs = options.refreshTimeoutMs ?? DEFAULT_REFRESH_TIMEOUT_MS;
-    this.now = options.now ?? Date.now;
   }
 
   getSnapshot(_cwd?: string): ProviderSnapshotEntry[] {
@@ -62,8 +54,9 @@ export class ProviderSnapshotManager {
     }
     const missingProviders = this.getProviderIds().filter((provider) => !entries.has(provider));
     if (missingProviders.length > 0) {
-      this.resetSnapshotToLoading(resolvedCwd, missingProviders);
-      void this.warmUp(resolvedCwd, missingProviders);
+      for (const provider of missingProviders) {
+        entries.set(provider, this.createUnprobedEntry(provider));
+      }
     }
     const providerLoads = this.providerLoads.get(resolvedCwd);
     const loadingProviders = Array.from(entries.values())
@@ -72,9 +65,6 @@ export class ProviderSnapshotManager {
     if (loadingProviders.length > 0) {
       void this.warmUp(resolvedCwd, loadingProviders);
     }
-    if (this.shouldRevalidate(resolvedCwd)) {
-      void this.warmUp(resolvedCwd);
-    }
     return entriesToArray(entries);
   }
 
@@ -84,9 +74,6 @@ export class ProviderSnapshotManager {
     this.resetSnapshotToLoading(snapshotCwd, providers);
     this.emitChange(snapshotCwd);
     await this.refreshProviders(snapshotCwd, providers ?? this.getProviderIds());
-    if (!providers) {
-      this.lastCheckedAts.set(snapshotCwd, this.now());
-    }
   }
 
   async refreshSettingsSnapshot(
@@ -99,9 +86,6 @@ export class ProviderSnapshotManager {
     this.resetSnapshotToLoading(homeCwd, providers);
     this.emitChange(homeCwd);
     await this.refreshProviders(homeCwd, providersToRefresh);
-    if (!providers) {
-      this.lastCheckedAts.set(homeCwd, this.now());
-    }
   }
 
   async warmUpSnapshotForCwd(options: ProviderSnapshotRefreshOptions): Promise<void> {
@@ -142,7 +126,6 @@ export class ProviderSnapshotManager {
     this.destroyed = true;
     this.events.removeAllListeners();
     this.snapshots.clear();
-    this.lastCheckedAts.clear();
     this.providerLoads.clear();
   }
 
@@ -151,10 +134,8 @@ export class ProviderSnapshotManager {
 
     for (const cwd of this.snapshots.keys()) {
       this.providerLoads.delete(cwd);
-      this.lastCheckedAts.delete(cwd);
-      this.snapshots.set(cwd, this.createLoadingEntries());
+      this.snapshots.set(cwd, this.reconcileSnapshotForRegistry(cwd));
       this.emitChange(cwd);
-      void this.warmUp(cwd);
     }
   }
 
@@ -174,6 +155,51 @@ export class ProviderSnapshotManager {
     return entries;
   }
 
+  private createUnprobedEntry(provider: AgentProvider): ProviderSnapshotEntry {
+    const definition = this.providerRegistry[provider];
+    return {
+      provider,
+      status: "unavailable",
+      enabled: definition?.enabled ?? true,
+      label: definition?.label,
+      description: definition?.description,
+      defaultModeId: definition?.defaultModeId ?? null,
+    };
+  }
+
+  private reconcileSnapshotForRegistry(cwd: string): Map<AgentProvider, ProviderSnapshotEntry> {
+    const existing = this.snapshots.get(cwd);
+    const entries = new Map<AgentProvider, ProviderSnapshotEntry>();
+
+    for (const provider of this.getProviderIds()) {
+      const definition = this.providerRegistry[provider];
+      const current = existing?.get(provider);
+      const metadata = {
+        provider,
+        enabled: definition?.enabled ?? true,
+        label: definition?.label,
+        description: definition?.description,
+        defaultModeId: definition?.defaultModeId ?? null,
+      };
+
+      if (!definition?.enabled || !current || current.status === "loading") {
+        entries.set(provider, {
+          ...metadata,
+          status: "unavailable",
+          enabled: definition?.enabled ?? true,
+        });
+        continue;
+      }
+
+      entries.set(provider, {
+        ...current,
+        ...metadata,
+      });
+    }
+
+    return entries;
+  }
+
   private async warmUp(cwd: string, providers?: AgentProvider[]): Promise<void> {
     const providersToRefresh = providers ?? this.getProviderIds();
 
@@ -182,9 +208,6 @@ export class ProviderSnapshotManager {
       providers: providersToRefresh,
       force: false,
     });
-    if (!providers) {
-      this.lastCheckedAts.set(cwd, this.now());
-    }
   }
 
   private async refreshProviders(cwd: string, providers: AgentProvider[]): Promise<void> {
@@ -337,17 +360,6 @@ export class ProviderSnapshotManager {
     this.events.emit("change", entriesToArray(snapshot), cwdKey);
   }
 
-  private shouldRevalidate(cwdKey: string): boolean {
-    if (this.providerLoads.has(cwdKey)) {
-      return false;
-    }
-    const lastCheckedAt = this.lastCheckedAts.get(cwdKey);
-    if (lastCheckedAt === undefined) {
-      return false;
-    }
-    return this.now() - lastCheckedAt > this.ttlMs;
-  }
-
   private getOrCreateSnapshot(cwdKey: string): Map<AgentProvider, ProviderSnapshotEntry> {
     const existing = this.snapshots.get(cwdKey);
     if (existing) {