diff --git a/docs/providers.md b/docs/providers.md index 71c62246e..10dab026e 100644 --- a/docs/providers.md +++ b/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 diff --git a/packages/app/src/screens/settings/providers-section.test.tsx b/packages/app/src/screens/settings/providers-section.test.tsx index d6a696e95..e01d0af1b 100644 --- a/packages/app/src/screens/settings/providers-section.test.tsx +++ b/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( + '[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(); + }); }); diff --git a/packages/server/src/server/agent/provider-snapshot-manager.test.ts b/packages/server/src/server/agent/provider-snapshot-manager.test.ts index 22e1a4f18..6bda87ff2 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.test.ts +++ b/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>() - .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(); - const fetchModels = vi - .fn<(cwd?: string) => Promise>() - .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>() .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>() - .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" }), diff --git a/packages/server/src/server/agent/provider-snapshot-manager.ts b/packages/server/src/server/agent/provider-snapshot-manager.ts index c0fadf059..3ac75868d 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.ts +++ b/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>(); - private readonly lastCheckedAts = new Map(); private readonly providerLoads = new Map>(); private readonly events = new EventEmitter(); private destroyed = false; - private readonly ttlMs: number; private readonly refreshTimeoutMs: number; - private readonly now: () => number; private providerRegistry: Record; 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 { @@ -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 { + const existing = this.snapshots.get(cwd); + const entries = new Map(); + + 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 { 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 { @@ -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 { const existing = this.snapshots.get(cwdKey); if (existing) {