From ff003c89d3ce6c4f060fc93f258851222753f678 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 13:16:53 -0400 Subject: [PATCH 1/9] feat: apply auto-refresh setting live + diff-aware list-changed indicator (#1444) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to #1402. 1. Live setting application — InspectorClient gains setServerSettings() (on the protocol + fake); App pushes edited settings onto the live client when the Server Settings modal closes for the connected server, so toggling auto-refresh-on-list-changed takes effect without a reconnect (the managed state reads getServerSettings() at notification time). 2. Diff-aware indicator — in flag-only mode, on */list_changed the managed Tools/Prompts/Resources state now fetches the new list and compares it to the current one, lighting the indicator only when it actually differs. The displayed list stays put until the user pulls via Refresh (pull-on-demand preserved). Servers like everything re-send identical lists on list_changed; this suppresses the false-positive indicator. Refactors each manager's refresh into fetch + apply helpers shared with the peek path. Trade-off: item 2 reintroduces a fetch on */list_changed (to compare), but unlike the pre-#1402 behavior it never replaces the displayed list under the user — only the indicator reflects a real change. Co-Authored-By: Claude Opus 4.8 (1M context) --- clients/web/src/App.test.tsx | 1 + clients/web/src/App.tsx | 5 ++ .../mcp/state/managedPromptsState.test.ts | 51 ++++++++++---- .../mcp/state/managedResourcesState.test.ts | 51 ++++++++++---- .../core/mcp/state/managedToolsState.test.ts | 67 ++++++++++++++---- .../integration/mcp/inspectorClient.test.ts | 26 +++++++ core/mcp/__tests__/fakeInspectorClient.ts | 4 ++ core/mcp/inspectorClient.ts | 12 ++++ core/mcp/inspectorClientProtocol.ts | 1 + core/mcp/state/managedPromptsState.ts | 66 ++++++++++++------ core/mcp/state/managedResourcesState.ts | 68 +++++++++++++------ core/mcp/state/managedToolsState.ts | 66 ++++++++++++------ 12 files changed, 314 insertions(+), 104 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index d9871056d..cf3f0f4c3 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -69,6 +69,7 @@ vi.mock("@inspector/core/mcp/index.js", () => { getPendingElicitations = vi.fn().mockReturnValue([]); getRoots = vi.fn().mockReturnValue([]); setRoots = vi.fn().mockResolvedValue(undefined); + setServerSettings = vi.fn(); } const instances: FakeInspectorClient[] = []; return { diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index c8b589988..40bc883c0 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1989,6 +1989,11 @@ function App() { settingsModalTargetId === activeServerId && settingsDraft ) { + // Push the edited settings onto the live client so settings the managed + // state reads at notification time (auto-refresh-on-list-changed) take + // effect without a reconnect (#1444). Connection-time inputs (transport, + // OAuth, timeouts) still only apply on the next connect. + inspectorClient.setServerSettings(settingsDraft); const nextRoots = cleanRoots(settingsDraft.roots); const currentRoots = cleanRoots(inspectorClient.getRoots()); if (JSON.stringify(nextRoots) !== JSON.stringify(currentRoots)) { diff --git a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts index e98fbc436..600b3b22d 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -26,6 +26,14 @@ function waitForPromptsChange(state: ManagedPromptsState): Promise { }); } +function waitForListChanged(state: ManagedPromptsState): Promise { + return new Promise((resolve) => { + state.addEventListener("listChangedChange", (e) => resolve(e.detail), { + once: true, + }); + }); +} + describe("ManagedPromptsState", () => { let client: FakeInspectorClient; let state: ManagedPromptsState; @@ -135,15 +143,34 @@ describe("ManagedPromptsState", () => { expect(next.map((p) => p.name)).toEqual(["a"]); }); - it("promptsListChanged does NOT auto-refresh by default (the user pulls via Refresh)", async () => { + it("promptsListChanged peeks but does NOT replace the displayed list by default", async () => { + // Diff-aware (#1444): the notification fetches to compare, but the + // displayed list stays put until the user pulls via Refresh. client.setStatus("connected"); client.queuePromptPages({ prompts: [prompt("a"), prompt("b")] }); + const changed = waitForListChanged(state); client.dispatchTypedEvent("promptsListChanged"); - // Yield so a stray refresh would have landed. - await Promise.resolve(); - await Promise.resolve(); - expect(client.listPrompts).not.toHaveBeenCalled(); - expect(state.getPrompts()).toEqual([]); + expect(await changed).toBe(true); // the peeked list differs from [] + expect(client.listPrompts).toHaveBeenCalled(); // it fetched to compare + expect(state.getPrompts()).toEqual([]); // ...but did not replace the display + }); + + it("promptsListChanged does NOT light the indicator when the list is unchanged", async () => { + client.setStatus("connected"); + client.queuePromptPages({ prompts: [prompt("a")] }); + await state.refresh(); + expect(state.getPrompts().map((p) => p.name)).toEqual(["a"]); + + let fired = false; + state.addEventListener("listChangedChange", () => { + fired = true; + }); + client.queuePromptPages({ prompts: [prompt("a")] }); + client.dispatchTypedEvent("promptsListChanged"); + await new Promise((r) => setTimeout(r, 0)); + expect(client.listPrompts).toHaveBeenCalledTimes(2); // refresh + peek + expect(fired).toBe(false); + expect(state.getListChanged()).toBe(false); }); it("promptsListChanged auto-refreshes when the server opts in", async () => { @@ -206,14 +233,6 @@ describe("ManagedPromptsState", () => { }); describe("listChanged (#1402)", () => { - function waitForListChanged(s: ManagedPromptsState): Promise { - return new Promise((resolve) => { - s.addEventListener("listChangedChange", (e) => resolve(e.detail), { - once: true, - }); - }); - } - it("starts cleared", () => { expect(state.getListChanged()).toBe(false); }); @@ -230,7 +249,9 @@ describe("ManagedPromptsState", () => { it("clearListChanged resets the flag and dispatches false", async () => { client.setStatus("connected"); client.queuePromptPages({ prompts: [prompt("a")] }); + const set = waitForListChanged(state); client.dispatchTypedEvent("promptsListChanged"); + await set; // wait for the async peek to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -251,7 +272,9 @@ describe("ManagedPromptsState", () => { it("disconnect clears the flag", async () => { client.setStatus("connected"); client.queuePromptPages({ prompts: [prompt("a")] }); + const set = waitForListChanged(state); client.dispatchTypedEvent("promptsListChanged"); + await set; // wait for the async peek to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); diff --git a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts index c4c8db3e1..c1f417ee5 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -28,6 +28,14 @@ function waitForResourcesChange( }); } +function waitForListChanged(state: ManagedResourcesState): Promise { + return new Promise((resolve) => { + state.addEventListener("listChangedChange", (e) => resolve(e.detail), { + once: true, + }); + }); +} + describe("ManagedResourcesState", () => { let client: FakeInspectorClient; let state: ManagedResourcesState; @@ -142,17 +150,36 @@ describe("ManagedResourcesState", () => { expect(next.map((r) => r.uri)).toEqual(["a://1"]); }); - it("resourcesListChanged does NOT auto-refresh by default (the user pulls via Refresh)", async () => { + it("resourcesListChanged peeks but does NOT replace the displayed list by default", async () => { + // Diff-aware (#1444): the notification fetches to compare, but the + // displayed list stays put until the user pulls via Refresh. client.setStatus("connected"); client.queueResourcePages({ resources: [resource("a://1"), resource("a://2")], }); + const changed = waitForListChanged(state); client.dispatchTypedEvent("resourcesListChanged"); - // Yield so a stray refresh would have landed. - await Promise.resolve(); - await Promise.resolve(); - expect(client.listResources).not.toHaveBeenCalled(); - expect(state.getResources()).toEqual([]); + expect(await changed).toBe(true); // the peeked list differs from [] + expect(client.listResources).toHaveBeenCalled(); // it fetched to compare + expect(state.getResources()).toEqual([]); // ...but did not replace display + }); + + it("resourcesListChanged does NOT light the indicator when the list is unchanged", async () => { + client.setStatus("connected"); + client.queueResourcePages({ resources: [resource("a://1")] }); + await state.refresh(); + expect(state.getResources().map((r) => r.uri)).toEqual(["a://1"]); + + let fired = false; + state.addEventListener("listChangedChange", () => { + fired = true; + }); + client.queueResourcePages({ resources: [resource("a://1")] }); + client.dispatchTypedEvent("resourcesListChanged"); + await new Promise((r) => setTimeout(r, 0)); + expect(client.listResources).toHaveBeenCalledTimes(2); // refresh + peek + expect(fired).toBe(false); + expect(state.getListChanged()).toBe(false); }); it("resourcesListChanged auto-refreshes when the server opts in", async () => { @@ -215,14 +242,6 @@ describe("ManagedResourcesState", () => { }); describe("listChanged (#1402)", () => { - function waitForListChanged(s: ManagedResourcesState): Promise { - return new Promise((resolve) => { - s.addEventListener("listChangedChange", (e) => resolve(e.detail), { - once: true, - }); - }); - } - it("starts cleared", () => { expect(state.getListChanged()).toBe(false); }); @@ -239,7 +258,9 @@ describe("ManagedResourcesState", () => { it("clearListChanged resets the flag and dispatches false", async () => { client.setStatus("connected"); client.queueResourcePages({ resources: [resource("a://1")] }); + const set = waitForListChanged(state); client.dispatchTypedEvent("resourcesListChanged"); + await set; // wait for the async peek to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -260,7 +281,9 @@ describe("ManagedResourcesState", () => { it("disconnect clears the flag", async () => { client.setStatus("connected"); client.queueResourcePages({ resources: [resource("a://1")] }); + const set = waitForListChanged(state); client.dispatchTypedEvent("resourcesListChanged"); + await set; // wait for the async peek to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index bf839861d..9b4d99a2e 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -26,6 +26,14 @@ function waitForToolsChange(state: ManagedToolsState): Promise { }); } +function waitForListChanged(state: ManagedToolsState): Promise { + return new Promise((resolve) => { + state.addEventListener("listChangedChange", (e) => resolve(e.detail), { + once: true, + }); + }); +} + describe("ManagedToolsState", () => { let client: FakeInspectorClient; let state: ManagedToolsState; @@ -133,15 +141,38 @@ describe("ManagedToolsState", () => { expect(next.map((t) => t.name)).toEqual(["a"]); }); - it("toolsListChanged does NOT auto-refresh by default (the user pulls via Refresh)", async () => { + it("toolsListChanged peeks but does NOT replace the displayed list by default", async () => { + // Diff-aware (#1444): the notification fetches to compare, but the + // displayed list stays put until the user pulls via Refresh. client.setStatus("connected"); client.queueToolPages({ tools: [tool("a"), tool("b")] }); + const changed = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); - // Yield so a stray refresh would have landed. - await Promise.resolve(); - await Promise.resolve(); - expect(client.listTools).not.toHaveBeenCalled(); - expect(state.getTools()).toEqual([]); + expect(await changed).toBe(true); // the peeked list differs from [] + expect(client.listTools).toHaveBeenCalled(); // it fetched to compare + expect(state.getTools()).toEqual([]); // ...but did not replace the display + }); + + it("toolsListChanged does NOT light the indicator when the list is unchanged", async () => { + // The everything-server case: a list_changed that re-sends an identical + // list must not light the indicator. + client.setStatus("connected"); + client.queueToolPages({ tools: [tool("a")] }); + await state.refresh(); + expect(state.getTools().map((t) => t.name)).toEqual(["a"]); + + let fired = false; + state.addEventListener("listChangedChange", () => { + fired = true; + }); + // Re-send the same single-tool page on the next peek. + client.queueToolPages({ tools: [tool("a")] }); + client.dispatchTypedEvent("toolsListChanged"); + // Let the async peek (fetch + compare) settle. + await new Promise((r) => setTimeout(r, 0)); + expect(client.listTools).toHaveBeenCalledTimes(2); // refresh + peek + expect(fired).toBe(false); + expect(state.getListChanged()).toBe(false); }); it("toolsListChanged auto-refreshes when the server opts in", async () => { @@ -158,6 +189,18 @@ describe("ManagedToolsState", () => { expect(autoClient.listTools).toHaveBeenCalled(); }); + it("honors a live setServerSettings toggle without a reconnect (#1444)", async () => { + // Starts in flag-only mode (no settings); flip auto-refresh on live. + client.setStatus("connected"); + client.setServerSettings(AUTO_REFRESH_SETTINGS); + client.queueToolPages({ tools: [tool("a")] }); + const changed = waitForToolsChange(state); + client.dispatchTypedEvent("toolsListChanged"); + // The list is applied (auto-refresh), proving the manager read the new + // setting at notification time rather than a connect-time snapshot. + expect((await changed).map((t) => t.name)).toEqual(["a"]); + }); + it("statusChange to disconnected clears tools and dispatches toolsChange", async () => { client.setStatus("connected"); client.queueToolPages({ tools: [tool("a")] }); @@ -207,14 +250,6 @@ describe("ManagedToolsState", () => { }); describe("listChanged (#1402)", () => { - function waitForListChanged(s: ManagedToolsState): Promise { - return new Promise((resolve) => { - s.addEventListener("listChangedChange", (e) => resolve(e.detail), { - once: true, - }); - }); - } - it("starts cleared", () => { expect(state.getListChanged()).toBe(false); }); @@ -231,7 +266,9 @@ describe("ManagedToolsState", () => { it("clearListChanged resets the flag and dispatches false", async () => { client.setStatus("connected"); client.queueToolPages({ tools: [tool("a")] }); + const set = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); + await set; // wait for the async peek to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -252,7 +289,9 @@ describe("ManagedToolsState", () => { it("disconnect clears the flag", async () => { client.setStatus("connected"); client.queueToolPages({ tools: [tool("a")] }); + const set = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); + await set; // wait for the async peek to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index c4610610d..05e635d14 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -654,6 +654,32 @@ describe("InspectorClient", () => { }); describe("Server Data Management", () => { + it("getServerSettings returns the constructor settings; setServerSettings replaces them live (#1444)", () => { + const initial = { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 0, + autoRefreshOnListChanged: false, + roots: [], + }; + client = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { + environment: { transport: createTransportNode }, + serverSettings: initial, + }, + ); + expect(client.getServerSettings()?.autoRefreshOnListChanged).toBe(false); + client.setServerSettings({ ...initial, autoRefreshOnListChanged: true }); + expect(client.getServerSettings()?.autoRefreshOnListChanged).toBe(true); + }); + it("should auto-fetch server contents when enabled", async () => { client = new InspectorClient( { diff --git a/core/mcp/__tests__/fakeInspectorClient.ts b/core/mcp/__tests__/fakeInspectorClient.ts index 916d12e13..07ef71826 100644 --- a/core/mcp/__tests__/fakeInspectorClient.ts +++ b/core/mcp/__tests__/fakeInspectorClient.ts @@ -184,6 +184,10 @@ export class FakeInspectorClient return this.serverSettings; } + setServerSettings(settings: InspectorServerSettings): void { + this.serverSettings = settings; + } + getAppRendererClient(): AppRendererClient | null { return this.appRendererClient; } diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index c6d2c5c17..0243761ec 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -1316,6 +1316,18 @@ export class InspectorClient extends InspectorClientEventTarget { return this.serverSettings; } + /** + * Replace the in-memory per-server settings on a live client. Lets a settings + * edit (e.g. toggling auto-refresh-on-list-changed) take effect on the + * current connection without a reconnect — the managed list state reads + * `getServerSettings()` at notification time, so the next `list_changed` + * notification honors the new value (#1444). Connection-time inputs + * (transport, OAuth, timeouts) still only apply on the next connect. + */ + setServerSettings(settings: InspectorServerSettings): void { + this.serverSettings = settings; + } + /** * Set the logging level for the MCP server * @param level Logging level to set diff --git a/core/mcp/inspectorClientProtocol.ts b/core/mcp/inspectorClientProtocol.ts index 5a6eb2506..007b811b1 100644 --- a/core/mcp/inspectorClientProtocol.ts +++ b/core/mcp/inspectorClientProtocol.ts @@ -57,6 +57,7 @@ export interface InspectorClientProtocol extends InspectorClientEventTarget { getInstructions(): string | undefined; getProtocolVersion(): string | undefined; getServerSettings(): InspectorServerSettings | undefined; + setServerSettings(settings: InspectorServerSettings): void; getAppRendererClient(): AppRendererClient | null; // Connection control diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index 21dfd9179..ddf84066c 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -41,13 +41,13 @@ export class ManagedPromptsState extends TypedEventTarget { // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise just flag the change for the indicator - // and let the user pull via Refresh, which is what makes the indicator - // meaningful (#1402). + // new list immediately. Otherwise peek: fetch and compare, lighting the + // indicator only when the list actually changed — many servers re-send an + // identical list on `list_changed` (#1402, #1444). if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { void this.refresh(); } else { - this.setListChanged(true); + void this.peekForChange(); } }; const onStatusChange = (): void => { @@ -102,27 +102,33 @@ export class ManagedPromptsState extends TypedEventTarget): Promise { + const next = await this.fetchPrompts(metadata); + // `null` means not connected — leave the current list untouched. + if (next === null) return this.getPrompts(); + this.applyPrompts(next); + return this.getPrompts(); + } + + /** + * Fetch all pages without mutating state or dispatching — used by both + * refresh (apply) and peek (compare). Returns `null` when not connected, or + * `[]` when the server doesn't advertise the `prompts` capability (calling + * prompts/list there returns -32601 "Method not found", which would spam the + * console; empty list is the right semantics). + */ + private async fetchPrompts( + metadata?: Record, + ): Promise { const client = this.client; - if (!client || client.getStatus() !== "connected") { - return this.getPrompts(); - } - // Gate on the server's `prompts` capability — calling prompts/list against - // a server that doesn't advertise it returns -32601 "Method not found", - // which then surfaces in the console for every connect against a - // prompts-less server. Empty list is the right semantics for "this server - // doesn't support prompts." - if (!client.getCapabilities()?.prompts) { - this.prompts = []; - this.dispatchTypedEvent("promptsChange", this.prompts); - return this.getPrompts(); - } + if (!client || client.getStatus() !== "connected") return null; + if (!client.getCapabilities()?.prompts) return []; const effectiveMetadata = metadata ?? this._metadata; - this.prompts = []; + let prompts: Prompt[] = []; let cursor: string | undefined; let pageCount = 0; do { const result = await client.listPrompts(cursor, effectiveMetadata); - this.prompts = cursor ? [...this.prompts, ...result.prompts] : result.prompts; + prompts = cursor ? [...prompts, ...result.prompts] : result.prompts; cursor = result.nextCursor; pageCount++; if (pageCount >= MAX_PAGES) { @@ -131,8 +137,28 @@ export class ManagedPromptsState extends TypedEventTarget { + const next = await this.fetchPrompts(); + if (next === null) return; + if (JSON.stringify(next) !== JSON.stringify(this.prompts)) { + this.setListChanged(true); + } } destroy(): void { diff --git a/core/mcp/state/managedResourcesState.ts b/core/mcp/state/managedResourcesState.ts index e8118022f..a5530ea6f 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -42,13 +42,13 @@ export class ManagedResourcesState extends TypedEventTarget { // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise just flag the change for the indicator - // and let the user pull via Refresh, which is what makes the indicator - // meaningful (#1402). + // new list immediately. Otherwise peek: fetch and compare, lighting the + // indicator only when the list actually changed — many servers re-send an + // identical list on `list_changed` (#1402, #1444). if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { void this.refresh(); } else { - this.setListChanged(true); + void this.peekForChange(); } }; const onStatusChange = (): void => { @@ -106,29 +106,33 @@ export class ManagedResourcesState extends TypedEventTarget): Promise { + const next = await this.fetchResources(metadata); + // `null` means not connected — leave the current list untouched. + if (next === null) return this.getResources(); + this.applyResources(next); + return this.getResources(); + } + + /** + * Fetch all pages without mutating state or dispatching — used by both + * refresh (apply) and peek (compare). Returns `null` when not connected, or + * `[]` when the server doesn't advertise the `resources` capability (calling + * resources/list there returns -32601 "Method not found", which would spam + * the console; empty list is the right semantics). + */ + private async fetchResources( + metadata?: Record, + ): Promise { const client = this.client; - if (!client || client.getStatus() !== "connected") { - return this.getResources(); - } - // Gate on the server's `resources` capability — calling resources/list - // against a server that doesn't advertise it returns -32601 "Method not - // found", which then surfaces in the console for every connect against a - // resources-less server. Empty list is the right semantics for "this - // server doesn't support resources." - if (!client.getCapabilities()?.resources) { - this.resources = []; - this.dispatchTypedEvent("resourcesChange", this.resources); - return this.getResources(); - } + if (!client || client.getStatus() !== "connected") return null; + if (!client.getCapabilities()?.resources) return []; const effectiveMetadata = metadata ?? this._metadata; - this.resources = []; + let resources: Resource[] = []; let cursor: string | undefined; let pageCount = 0; do { const result = await client.listResources(cursor, effectiveMetadata); - this.resources = cursor - ? [...this.resources, ...result.resources] - : result.resources; + resources = cursor ? [...resources, ...result.resources] : result.resources; cursor = result.nextCursor; pageCount++; if (pageCount >= MAX_PAGES) { @@ -137,8 +141,28 @@ export class ManagedResourcesState extends TypedEventTarget { + const next = await this.fetchResources(); + if (next === null) return; + if (JSON.stringify(next) !== JSON.stringify(this.resources)) { + this.setListChanged(true); + } } destroy(): void { diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index 5490004f7..aaa25b3de 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -41,13 +41,13 @@ export class ManagedToolsState extends TypedEventTarget { // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise just flag the change for the indicator - // and let the user pull via Refresh, which is what makes the indicator - // meaningful (#1402). + // new list immediately. Otherwise peek: fetch and compare, lighting the + // indicator only when the list actually changed — many servers re-send an + // identical list on `list_changed` (#1402, #1444). if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { void this.refresh(); } else { - this.setListChanged(true); + void this.peekForChange(); } }; const onStatusChange = (): void => { @@ -99,27 +99,33 @@ export class ManagedToolsState extends TypedEventTarget): Promise { + const next = await this.fetchTools(metadata); + // `null` means not connected — leave the current list untouched. + if (next === null) return this.getTools(); + this.applyTools(next); + return this.getTools(); + } + + /** + * Fetch all pages without mutating state or dispatching — used by both + * refresh (apply) and peek (compare). Returns `null` when not connected, or + * `[]` when the server doesn't advertise the `tools` capability (calling + * tools/list there returns -32601 "Method not found", which would spam the + * console; empty list is the right semantics). + */ + private async fetchTools( + metadata?: Record, + ): Promise { const client = this.client; - if (!client || client.getStatus() !== "connected") { - return this.getTools(); - } - // Gate on the server's `tools` capability — calling tools/list against a - // server that doesn't advertise it returns -32601 "Method not found", - // which then surfaces in the console for every connect against a - // tools-less server. Empty list is the right semantics for "this server - // doesn't support tools." - if (!client.getCapabilities()?.tools) { - this.tools = []; - this.dispatchTypedEvent("toolsChange", this.tools); - return this.getTools(); - } + if (!client || client.getStatus() !== "connected") return null; + if (!client.getCapabilities()?.tools) return []; const effectiveMetadata = metadata ?? this._metadata; - this.tools = []; + let tools: Tool[] = []; let cursor: string | undefined; let pageCount = 0; do { const result = await client.listTools(cursor, effectiveMetadata); - this.tools = cursor ? [...this.tools, ...result.tools] : result.tools; + tools = cursor ? [...tools, ...result.tools] : result.tools; cursor = result.nextCursor; pageCount++; if (pageCount >= MAX_PAGES) { @@ -128,8 +134,28 @@ export class ManagedToolsState extends TypedEventTarget { + const next = await this.fetchTools(); + if (next === null) return; + if (JSON.stringify(next) !== JSON.stringify(this.tools)) { + this.setListChanged(true); + } } destroy(): void { From 6b4fb7289aeb7d613c96484e855d0bc20d5c6ab6 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 13:23:27 -0400 Subject: [PATCH 2/9] refactor(core): make the diff-aware peek clear the indicator on revert (#1444 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit peekForChange now sets the list-changed flag to whether the server list differs from the displayed one (setListChanged(differs)) instead of only ever setting it true. So a later notification that reverts the server back to the displayed list clears the indicator — nothing left to pull. Adds a clear-on-revert regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/mcp/state/managedToolsState.test.ts | 19 +++++++++++++++++++ core/mcp/state/managedPromptsState.ts | 17 +++++++++-------- core/mcp/state/managedResourcesState.ts | 19 +++++++++++-------- core/mcp/state/managedToolsState.ts | 17 +++++++++-------- 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index 9b4d99a2e..a225d4753 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -175,6 +175,25 @@ describe("ManagedToolsState", () => { expect(state.getListChanged()).toBe(false); }); + it("clears the indicator when a later notification reverts the list to the displayed one (#1444)", async () => { + client.setStatus("connected"); + client.queueToolPages({ tools: [tool("a")] }); + await state.refresh(); // displayed = [a] + + // Server adds a tool → indicator lights. + client.queueToolPages({ tools: [tool("a"), tool("b")] }); + const lit = waitForListChanged(state); + client.dispatchTypedEvent("toolsListChanged"); + expect(await lit).toBe(true); + + // Server reverts to [a] (matches the displayed list) → indicator clears. + client.queueToolPages({ tools: [tool("a")] }); + const cleared = waitForListChanged(state); + client.dispatchTypedEvent("toolsListChanged"); + expect(await cleared).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + it("toolsListChanged auto-refreshes when the server opts in", async () => { const autoClient = new FakeInspectorClient({ capabilities: { tools: {} }, diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index ddf84066c..889e6ad0e 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -147,18 +147,19 @@ export class ManagedPromptsState extends TypedEventTarget { const next = await this.fetchPrompts(); if (next === null) return; - if (JSON.stringify(next) !== JSON.stringify(this.prompts)) { - this.setListChanged(true); - } + this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.prompts)); } destroy(): void { diff --git a/core/mcp/state/managedResourcesState.ts b/core/mcp/state/managedResourcesState.ts index a5530ea6f..90e45c5a8 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -151,18 +151,21 @@ export class ManagedResourcesState extends TypedEventTarget { const next = await this.fetchResources(); if (next === null) return; - if (JSON.stringify(next) !== JSON.stringify(this.resources)) { - this.setListChanged(true); - } + this.setListChanged( + JSON.stringify(next) !== JSON.stringify(this.resources), + ); } destroy(): void { diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index aaa25b3de..64ff15c35 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -144,18 +144,19 @@ export class ManagedToolsState extends TypedEventTarget { const next = await this.fetchTools(); if (next === null) return; - if (JSON.stringify(next) !== JSON.stringify(this.tools)) { - this.setListChanged(true); - } + this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.tools)); } destroy(): void { From b183d5e2a3583a84af291c4421c962f4beda9f37 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 13:40:24 -0400 Subject: [PATCH 3/9] refactor(core): consolidate managed-list state managers onto a shared base (#1444) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four per-primitive list managers (tools, prompts, resources, resource templates) were near-identical copies of the same fetch/apply/peek/connect/ disconnect/list_changed machinery, a standing drift risk. Extract a generic ManagedListState base parameterized by a config (change event, list_changed notification, capability key, page fetcher, and whether the list drives an indicator). Each manager is now a ~40-line subclass: its config plus a typed getter alias. Public contracts are unchanged — same per-manager *Change / listChangedChange events and get*/refresh/getListChanged/clearListChanged methods — so the hooks and all existing tests pass untouched. Templates opt out of the indicator (supportsIndicator: false): a list_changed auto-refreshes only when the server opts in, else does nothing. Net -245 lines. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/mcp/state/managedListState.ts | 224 ++++++++++++++++++ core/mcp/state/managedPromptsState.ts | 172 ++------------ .../state/managedResourceTemplatesState.ts | 150 +++--------- core/mcp/state/managedResourcesState.ts | 178 ++------------ core/mcp/state/managedToolsState.ts | 169 ++----------- 5 files changed, 324 insertions(+), 569 deletions(-) create mode 100644 core/mcp/state/managedListState.ts diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts new file mode 100644 index 000000000..188dcfb5b --- /dev/null +++ b/core/mcp/state/managedListState.ts @@ -0,0 +1,224 @@ +/** + * ManagedListState: shared base for the per-primitive list state managers + * (tools, prompts, resources, resource templates). Each keeps a full list in + * sync with the server — loaded on connect, cleared on disconnect, paginated on + * refresh — and (for the three with a sidebar indicator) tracks a "list changed + * since last refresh" flag. + * + * Subclasses are thin: they supply a config (which list method to page, which + * capability gates it, which `*Change` / `*ListChanged` events to use) and a + * typed getter alias (`getTools()` etc.). All behavior lives here so the four + * managers can't drift (#1444). + */ + +import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; +import type { InspectorClientEventMap } from "../inspectorClientEventTarget.js"; +import type { ServerCapabilities } from "@modelcontextprotocol/sdk/types.js"; +import { TypedEventTarget } from "../typedEventTarget.js"; + +const MAX_PAGES = 100; + +/** Every managed-list event map carries the list-changed indicator event. */ +export interface ManagedListEventMap { + listChangedChange: boolean; +} + +/** A single page of a list result, normalized across the SDK list methods. */ +export interface ListPage { + items: T[]; + nextCursor?: string; +} + +export interface ManagedListConfig { + /** The `*Change` event this manager dispatches (e.g. "toolsChange"). */ + changeEvent: keyof M; + /** The client notification that signals the list changed. */ + listChangedEvent: keyof InspectorClientEventMap; + /** Server capability gating the list call (empty list when absent). */ + capabilityKey: keyof ServerCapabilities; + /** Human label used in the pagination-cap error message. */ + itemLabel: string; + /** Fetch a single page from the client. */ + fetchPage: ( + client: InspectorClientProtocol, + cursor: string | undefined, + metadata: Record | undefined, + ) => Promise>; + /** + * Whether this list drives a list-changed indicator. When true, a + * `list_changed` in non-auto-refresh mode peeks-and-diffs to light the + * indicator; when false (resource templates, which have no indicator of + * their own), a `list_changed` in non-auto mode does nothing — the list is + * pulled via the screen's Refresh instead. + */ + supportsIndicator: boolean; +} + +export abstract class ManagedListState< + T, + M extends ManagedListEventMap, +> extends TypedEventTarget { + protected items: T[] = []; + protected client: InspectorClientProtocol | null = null; + private unsubscribe: (() => void) | null = null; + private _metadata: Record | undefined = undefined; + private listChanged = false; + private readonly config: ManagedListConfig; + + constructor( + client: InspectorClientProtocol, + config: ManagedListConfig, + ) { + super(); + this.client = client; + this.config = config; + const onConnect = (): void => { + void this.refresh(); + }; + const onListChanged = (): void => { + // When the server opts into auto-refresh (per-server setting), pull the + // new list immediately. Otherwise, for lists with an indicator, peek and + // diff so the indicator lights only on a real change; lists without an + // indicator simply wait for the user's Refresh (#1402, #1444). + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + void this.refresh(); + } else if (config.supportsIndicator) { + void this.peekForChange(); + } + }; + const onStatusChange = (): void => { + if (this.client?.getStatus() === "disconnected") { + this.items = []; + this.dispatchChange(); + this.setListChanged(false); + } + }; + this.client.addEventListener("connect", onConnect); + this.client.addEventListener(config.listChangedEvent, onListChanged); + this.client.addEventListener("statusChange", onStatusChange); + this.unsubscribe = () => { + if (this.client) { + this.client.removeEventListener("connect", onConnect); + this.client.removeEventListener(config.listChangedEvent, onListChanged); + this.client.removeEventListener("statusChange", onStatusChange); + } + this.client = null; + }; + } + + /** Defensive copy of the current list. */ + protected getItems(): T[] { + return [...this.items]; + } + + /** Whether a `list_changed` since the last refresh actually changed the list. */ + getListChanged(): boolean { + return this.listChanged; + } + + /** + * Clear the list-changed flag — called when the user refreshes the list. The + * peek/auto-refresh on the notification leaves the indicator set until the + * user acknowledges by pulling. + */ + clearListChanged(): void { + this.setListChanged(false); + } + + /** + * Dispatch a configured event by name. `dispatchTypedEvent`'s + * `EventMap[K] extends void ? [] : [detail]` overload can't resolve when the + * key is a generic `keyof M`, so we narrow the method signature here. The + * concrete subclass event maps keep the call sites type-safe. + */ + private emit(type: keyof M, detail: unknown): void { + ( + this.dispatchTypedEvent as unknown as (t: keyof M, d: unknown) => void + )(type, detail); + } + + private setListChanged(value: boolean): void { + if (this.listChanged === value) return; + this.listChanged = value; + this.emit("listChangedChange", value); + } + + setMetadata(metadata?: Record): void { + this._metadata = metadata; + } + + async refresh(metadata?: Record): Promise { + const next = await this.fetchAll(metadata); + // `null` means not connected — leave the current list untouched. + if (next === null) return this.getItems(); + this.applyItems(next); + return this.getItems(); + } + + /** + * Fetch all pages without mutating state or dispatching — used by both + * refresh (apply) and peek (compare). Returns `null` when not connected, or + * `[]` when the server doesn't advertise the gating capability (calling the + * list method there returns -32601 "Method not found", which would spam the + * console; empty list is the right semantics). + */ + private async fetchAll( + metadata?: Record, + ): Promise { + const client = this.client; + if (!client || client.getStatus() !== "connected") return null; + if (!client.getCapabilities()?.[this.config.capabilityKey]) return []; + const effectiveMetadata = metadata ?? this._metadata; + let items: T[] = []; + let cursor: string | undefined; + let pageCount = 0; + do { + const page = await this.config.fetchPage( + client, + cursor, + effectiveMetadata, + ); + items = cursor ? [...items, ...page.items] : page.items; + cursor = page.nextCursor; + pageCount++; + if (pageCount >= MAX_PAGES) { + throw new Error( + `Maximum pagination limit (${MAX_PAGES} pages) reached while listing ${this.config.itemLabel}`, + ); + } + } while (cursor); + return items; + } + + /** Commit a fetched list as the current one and notify subscribers. */ + private applyItems(items: T[]): void { + this.items = items; + this.dispatchChange(); + } + + private dispatchChange(): void { + this.emit(this.config.changeEvent, this.getItems()); + } + + /** + * Fetch on `list_changed` and track whether the server's list differs from + * what's displayed. The displayed list is left untouched — the user still + * pulls the new one via Refresh (pull-on-demand). Many servers re-send an + * identical list on `list_changed`; this keeps the indicator dark in that + * case, and also clears it if a later notification reverts the server back + * to the displayed list (nothing left to pull). The flag is order-sensitive: + * a reorder is a visible change the user would see on Refresh, so it counts + * (#1444). + */ + private async peekForChange(): Promise { + const next = await this.fetchAll(); + if (next === null) return; + this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.items)); + } + + destroy(): void { + this.unsubscribe?.(); + this.unsubscribe = null; + this.items = []; + } +} diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index 889e6ad0e..9d31128a2 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -1,170 +1,42 @@ /** - * ManagedPromptsState: holds full prompt list, syncs on promptsListChanged. - * - * Ported from v1.5/main. v2 substitutes `InspectorClientProtocol` for the - * concrete `InspectorClient` since the runtime class is not yet ported. + * ManagedPromptsState: holds the full prompt list, in sync with the server. + * A thin subclass of ManagedListState — behavior lives in the base (#1444). */ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; -import { TypedEventTarget } from "../typedEventTarget.js"; - -const MAX_PAGES = 100; +import { ManagedListState } from "./managedListState.js"; export interface ManagedPromptsStateEventMap { promptsChange: Prompt[]; /** * Fires when the "list changed since last refresh" flag flips. True when a - * `prompts/list_changed` notification arrives, false once the user refreshes - * or the connection drops. Drives the sidebar list-changed indicator (#1402). + * `prompts/list_changed` brings a list that differs from what's displayed, + * false once the user refreshes, the change reverts, or the connection + * drops. Drives the sidebar list-changed indicator (#1402). */ listChangedChange: boolean; } -/** - * State manager that keeps a full prompt list in sync with the server. - * Subscribes to connect, promptsListChanged, and statusChange; fetches all - * pages on refresh. - */ -export class ManagedPromptsState extends TypedEventTarget { - private prompts: Prompt[] = []; - private client: InspectorClientProtocol | null = null; - private unsubscribe: (() => void) | null = null; - private _metadata: Record | undefined = undefined; - private listChanged = false; - +export class ManagedPromptsState extends ManagedListState< + Prompt, + ManagedPromptsStateEventMap +> { constructor(client: InspectorClientProtocol) { - super(); - this.client = client; - const onConnect = (): void => { - void this.refresh(); - }; - const onPromptsListChanged = (): void => { - // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise peek: fetch and compare, lighting the - // indicator only when the list actually changed — many servers re-send an - // identical list on `list_changed` (#1402, #1444). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else { - void this.peekForChange(); - } - }; - const onStatusChange = (): void => { - if (this.client?.getStatus() === "disconnected") { - this.prompts = []; - this.dispatchTypedEvent("promptsChange", []); - this.setListChanged(false); - } - }; - this.client.addEventListener("connect", onConnect); - this.client.addEventListener("promptsListChanged", onPromptsListChanged); - this.client.addEventListener("statusChange", onStatusChange); - this.unsubscribe = () => { - if (this.client) { - this.client.removeEventListener("connect", onConnect); - this.client.removeEventListener( - "promptsListChanged", - onPromptsListChanged, - ); - this.client.removeEventListener("statusChange", onStatusChange); - } - this.client = null; - }; + super(client, { + changeEvent: "promptsChange", + listChangedEvent: "promptsListChanged", + capabilityKey: "prompts", + itemLabel: "prompts", + supportsIndicator: true, + fetchPage: async (c, cursor, metadata) => { + const result = await c.listPrompts(cursor, metadata); + return { items: result.prompts, nextCursor: result.nextCursor }; + }, + }); } getPrompts(): Prompt[] { - return [...this.prompts]; - } - - /** Whether a `prompts/list_changed` arrived since the last user refresh. */ - getListChanged(): boolean { - return this.listChanged; - } - - /** - * Clear the list-changed flag — called when the user refreshes the list - * (the auto-refresh on the notification leaves it set so the indicator - * stays visible until acknowledged). - */ - clearListChanged(): void { - this.setListChanged(false); - } - - private setListChanged(value: boolean): void { - if (this.listChanged === value) return; - this.listChanged = value; - this.dispatchTypedEvent("listChangedChange", value); - } - - setMetadata(metadata?: Record): void { - this._metadata = metadata; - } - - async refresh(metadata?: Record): Promise { - const next = await this.fetchPrompts(metadata); - // `null` means not connected — leave the current list untouched. - if (next === null) return this.getPrompts(); - this.applyPrompts(next); - return this.getPrompts(); - } - - /** - * Fetch all pages without mutating state or dispatching — used by both - * refresh (apply) and peek (compare). Returns `null` when not connected, or - * `[]` when the server doesn't advertise the `prompts` capability (calling - * prompts/list there returns -32601 "Method not found", which would spam the - * console; empty list is the right semantics). - */ - private async fetchPrompts( - metadata?: Record, - ): Promise { - const client = this.client; - if (!client || client.getStatus() !== "connected") return null; - if (!client.getCapabilities()?.prompts) return []; - const effectiveMetadata = metadata ?? this._metadata; - let prompts: Prompt[] = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listPrompts(cursor, effectiveMetadata); - prompts = cursor ? [...prompts, ...result.prompts] : result.prompts; - cursor = result.nextCursor; - pageCount++; - if (pageCount >= MAX_PAGES) { - throw new Error( - `Maximum pagination limit (${MAX_PAGES} pages) reached while listing prompts`, - ); - } - } while (cursor); - return prompts; - } - - /** Commit a fetched list as the current one and notify subscribers. */ - private applyPrompts(prompts: Prompt[]): void { - this.prompts = prompts; - this.dispatchTypedEvent("promptsChange", this.prompts); - } - - /** - * Fetch on `list_changed` and track whether the server's list differs from - * what's displayed. The displayed list is left untouched — the user still - * pulls the new one via Refresh (pull-on-demand). Many servers re-send an - * identical list on `list_changed`; this keeps the indicator dark in that - * case, and also clears it if a later notification reverts the server back - * to the displayed list (nothing left to pull). The flag is order-sensitive: - * a reorder is a visible change the user would see on Refresh, so it counts - * (#1444). - */ - private async peekForChange(): Promise { - const next = await this.fetchPrompts(); - if (next === null) return; - this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.prompts)); - } - - destroy(): void { - this.unsubscribe?.(); - this.unsubscribe = null; - this.prompts = []; + return this.getItems(); } } diff --git a/core/mcp/state/managedResourceTemplatesState.ts b/core/mcp/state/managedResourceTemplatesState.ts index 839863f1e..745df426d 100644 --- a/core/mcp/state/managedResourceTemplatesState.ts +++ b/core/mcp/state/managedResourceTemplatesState.ts @@ -1,133 +1,51 @@ /** - * ManagedResourceTemplatesState: holds the full resource template list, loaded - * on connect and on demand via refresh(). + * ManagedResourceTemplatesState: holds the full resource template list, in sync + * with the server. A thin subclass of ManagedListState (#1444). * - * Ported from v1.5/main. v2 substitutes `InspectorClientProtocol` for the - * concrete `InspectorClient` since the runtime class is not yet ported. + * Templates have no list-changed indicator of their own — the Resources + * screen's indicator (driven by `resourcesListChanged`) covers them, and its + * Refresh re-fetches templates too. So `supportsIndicator` is false: a + * `resourceTemplatesListChanged` auto-refreshes only when the server opts in + * via `autoRefreshOnListChanged`; otherwise it does nothing and the user pulls + * via the Resources Refresh (#1402). */ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { ResourceTemplate } from "@modelcontextprotocol/sdk/types.js"; -import { TypedEventTarget } from "../typedEventTarget.js"; - -const MAX_PAGES = 100; +import { ManagedListState } from "./managedListState.js"; export interface ManagedResourceTemplatesStateEventMap { resourceTemplatesChange: ResourceTemplate[]; + /** + * Carried only to satisfy the ManagedListState base; templates have no + * indicator, so this never fires. + */ + listChangedChange: boolean; } -/** - * State manager that keeps a full resource template list. Subscribes to connect - * (initial load) and statusChange (clear on disconnect); fetches all pages on - * refresh. Templates have no list-changed indicator of their own — the - * Resources screen's indicator (driven by `resourcesListChanged`) covers them, - * and its Refresh re-fetches templates too. So on `resourceTemplatesListChanged` - * it auto-refreshes ONLY when the server opts in via `autoRefreshOnListChanged`; - * otherwise it does nothing and the user pulls via the Resources Refresh (#1402). - */ -export class ManagedResourceTemplatesState extends TypedEventTarget { - private resourceTemplates: ResourceTemplate[] = []; - private client: InspectorClientProtocol | null = null; - private unsubscribe: (() => void) | null = null; - private _metadata: Record | undefined = undefined; - +export class ManagedResourceTemplatesState extends ManagedListState< + ResourceTemplate, + ManagedResourceTemplatesStateEventMap +> { constructor(client: InspectorClientProtocol) { - super(); - this.client = client; - const onConnect = (): void => { - void this.refresh(); - }; - const onResourceTemplatesListChanged = (): void => { - // Auto-refresh only when the server opts in. With auto-refresh off, - // templates stay as-is until the user pulls via the Resources Refresh — - // templates have no indicator of their own (the Resources indicator, - // driven by `resourcesListChanged`, covers this same notification). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } - }; - const onStatusChange = (): void => { - if (this.client?.getStatus() === "disconnected") { - this.resourceTemplates = []; - this.dispatchTypedEvent("resourceTemplatesChange", []); - } - }; - this.client.addEventListener("connect", onConnect); - this.client.addEventListener( - "resourceTemplatesListChanged", - onResourceTemplatesListChanged, - ); - this.client.addEventListener("statusChange", onStatusChange); - this.unsubscribe = () => { - if (this.client) { - this.client.removeEventListener("connect", onConnect); - this.client.removeEventListener( - "resourceTemplatesListChanged", - onResourceTemplatesListChanged, - ); - this.client.removeEventListener("statusChange", onStatusChange); - } - this.client = null; - }; + super(client, { + changeEvent: "resourceTemplatesChange", + listChangedEvent: "resourceTemplatesListChanged", + // Templates are gated on the broader `resources` capability. + capabilityKey: "resources", + itemLabel: "resource templates", + supportsIndicator: false, + fetchPage: async (c, cursor, metadata) => { + const result = await c.listResourceTemplates(cursor, metadata); + return { + items: result.resourceTemplates, + nextCursor: result.nextCursor, + }; + }, + }); } getResourceTemplates(): ResourceTemplate[] { - return [...this.resourceTemplates]; - } - - setMetadata(metadata?: Record): void { - this._metadata = metadata; - } - - async refresh( - metadata?: Record, - ): Promise { - const client = this.client; - if (!client || client.getStatus() !== "connected") { - return this.getResourceTemplates(); - } - // Gate on the server's `resources` capability — the MCP spec doesn't define - // a separate `resourceTemplates` capability; resources/templates/list is - // part of the resources surface. Calling it against a server that doesn't - // advertise resources returns -32601 "Method not found", which then - // surfaces in the console for every connect against a resources-less - // server. Empty list is the right semantics for "this server doesn't - // support resources." - if (!client.getCapabilities()?.resources) { - this.resourceTemplates = []; - this.dispatchTypedEvent( - "resourceTemplatesChange", - this.resourceTemplates, - ); - return this.getResourceTemplates(); - } - const effectiveMetadata = metadata ?? this._metadata; - this.resourceTemplates = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listResourceTemplates( - cursor, - effectiveMetadata, - ); - this.resourceTemplates = cursor - ? [...this.resourceTemplates, ...result.resourceTemplates] - : result.resourceTemplates; - cursor = result.nextCursor; - pageCount++; - if (pageCount >= MAX_PAGES) { - throw new Error( - `Maximum pagination limit (${MAX_PAGES} pages) reached while listing resource templates`, - ); - } - } while (cursor); - this.dispatchTypedEvent("resourceTemplatesChange", this.resourceTemplates); - return this.getResourceTemplates(); - } - - destroy(): void { - this.unsubscribe?.(); - this.unsubscribe = null; - this.resourceTemplates = []; + return this.getItems(); } } diff --git a/core/mcp/state/managedResourcesState.ts b/core/mcp/state/managedResourcesState.ts index 90e45c5a8..369965084 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -1,176 +1,42 @@ /** - * ManagedResourcesState: holds full resource list, syncs on resourcesListChanged. - * - * Ported from v1.5/main. v2 substitutes `InspectorClientProtocol` for the - * concrete `InspectorClient` since the runtime class is not yet ported. + * ManagedResourcesState: holds the full resource list, in sync with the server. + * A thin subclass of ManagedListState — behavior lives in the base (#1444). */ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { Resource } from "@modelcontextprotocol/sdk/types.js"; -import { TypedEventTarget } from "../typedEventTarget.js"; - -const MAX_PAGES = 100; +import { ManagedListState } from "./managedListState.js"; export interface ManagedResourcesStateEventMap { resourcesChange: Resource[]; /** * Fires when the "list changed since last refresh" flag flips. True when a - * `resources/list_changed` notification arrives, false once the user - * refreshes or the connection drops. Drives the list-changed indicator - * (#1402). + * `resources/list_changed` brings a list that differs from what's displayed, + * false once the user refreshes, the change reverts, or the connection + * drops. Drives the list-changed indicator (#1402). */ listChangedChange: boolean; } -/** - * State manager that keeps a full resource list in sync with the server. - * Subscribes to connect, resourcesListChanged, and statusChange; fetches all - * pages on refresh. - */ -export class ManagedResourcesState extends TypedEventTarget { - private resources: Resource[] = []; - private client: InspectorClientProtocol | null = null; - private unsubscribe: (() => void) | null = null; - private _metadata: Record | undefined = undefined; - private listChanged = false; - +export class ManagedResourcesState extends ManagedListState< + Resource, + ManagedResourcesStateEventMap +> { constructor(client: InspectorClientProtocol) { - super(); - this.client = client; - const onConnect = (): void => { - void this.refresh(); - }; - const onResourcesListChanged = (): void => { - // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise peek: fetch and compare, lighting the - // indicator only when the list actually changed — many servers re-send an - // identical list on `list_changed` (#1402, #1444). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else { - void this.peekForChange(); - } - }; - const onStatusChange = (): void => { - if (this.client?.getStatus() === "disconnected") { - this.resources = []; - this.dispatchTypedEvent("resourcesChange", []); - this.setListChanged(false); - } - }; - this.client.addEventListener("connect", onConnect); - this.client.addEventListener( - "resourcesListChanged", - onResourcesListChanged, - ); - this.client.addEventListener("statusChange", onStatusChange); - this.unsubscribe = () => { - if (this.client) { - this.client.removeEventListener("connect", onConnect); - this.client.removeEventListener( - "resourcesListChanged", - onResourcesListChanged, - ); - this.client.removeEventListener("statusChange", onStatusChange); - } - this.client = null; - }; + super(client, { + changeEvent: "resourcesChange", + listChangedEvent: "resourcesListChanged", + capabilityKey: "resources", + itemLabel: "resources", + supportsIndicator: true, + fetchPage: async (c, cursor, metadata) => { + const result = await c.listResources(cursor, metadata); + return { items: result.resources, nextCursor: result.nextCursor }; + }, + }); } getResources(): Resource[] { - return [...this.resources]; - } - - /** Whether a `resources/list_changed` arrived since the last user refresh. */ - getListChanged(): boolean { - return this.listChanged; - } - - /** - * Clear the list-changed flag — called when the user refreshes the list - * (the auto-refresh on the notification leaves it set so the indicator - * stays visible until acknowledged). - */ - clearListChanged(): void { - this.setListChanged(false); - } - - private setListChanged(value: boolean): void { - if (this.listChanged === value) return; - this.listChanged = value; - this.dispatchTypedEvent("listChangedChange", value); - } - - setMetadata(metadata?: Record): void { - this._metadata = metadata; - } - - async refresh(metadata?: Record): Promise { - const next = await this.fetchResources(metadata); - // `null` means not connected — leave the current list untouched. - if (next === null) return this.getResources(); - this.applyResources(next); - return this.getResources(); - } - - /** - * Fetch all pages without mutating state or dispatching — used by both - * refresh (apply) and peek (compare). Returns `null` when not connected, or - * `[]` when the server doesn't advertise the `resources` capability (calling - * resources/list there returns -32601 "Method not found", which would spam - * the console; empty list is the right semantics). - */ - private async fetchResources( - metadata?: Record, - ): Promise { - const client = this.client; - if (!client || client.getStatus() !== "connected") return null; - if (!client.getCapabilities()?.resources) return []; - const effectiveMetadata = metadata ?? this._metadata; - let resources: Resource[] = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listResources(cursor, effectiveMetadata); - resources = cursor ? [...resources, ...result.resources] : result.resources; - cursor = result.nextCursor; - pageCount++; - if (pageCount >= MAX_PAGES) { - throw new Error( - `Maximum pagination limit (${MAX_PAGES} pages) reached while listing resources`, - ); - } - } while (cursor); - return resources; - } - - /** Commit a fetched list as the current one and notify subscribers. */ - private applyResources(resources: Resource[]): void { - this.resources = resources; - this.dispatchTypedEvent("resourcesChange", this.resources); - } - - /** - * Fetch on `list_changed` and track whether the server's list differs from - * what's displayed. The displayed list is left untouched — the user still - * pulls the new one via Refresh (pull-on-demand). Many servers re-send an - * identical list on `list_changed`; this keeps the indicator dark in that - * case, and also clears it if a later notification reverts the server back - * to the displayed list (nothing left to pull). The flag is order-sensitive: - * a reorder is a visible change the user would see on Refresh, so it counts - * (#1444). - */ - private async peekForChange(): Promise { - const next = await this.fetchResources(); - if (next === null) return; - this.setListChanged( - JSON.stringify(next) !== JSON.stringify(this.resources), - ); - } - - destroy(): void { - this.unsubscribe?.(); - this.unsubscribe = null; - this.resources = []; + return this.getItems(); } } diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index 64ff15c35..6e3aef3f6 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -1,167 +1,42 @@ /** - * ManagedToolsState: holds full tool list, syncs on toolsListChanged. - * - * Ported from v1.5/main. v2 substitutes `InspectorClientProtocol` for the - * concrete `InspectorClient` since the runtime class is not yet ported. + * ManagedToolsState: holds the full tool list, in sync with the server. + * A thin subclass of ManagedListState — behavior lives in the base (#1444). */ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -import { TypedEventTarget } from "../typedEventTarget.js"; - -const MAX_PAGES = 100; +import { ManagedListState } from "./managedListState.js"; export interface ManagedToolsStateEventMap { toolsChange: Tool[]; /** * Fires when the "list changed since last refresh" flag flips. True when a - * `tools/list_changed` notification arrives, false once the user refreshes - * or the connection drops. Drives the sidebar list-changed indicator (#1402). + * `tools/list_changed` brings a list that differs from what's displayed, + * false once the user refreshes, the change reverts, or the connection + * drops. Drives the sidebar list-changed indicator (#1402). */ listChangedChange: boolean; } -/** - * State manager that keeps a full tool list in sync with the server. - * Subscribes to client's connect (initial load), toolsListChanged, and - * statusChange; fetches all pages on refresh. - */ -export class ManagedToolsState extends TypedEventTarget { - private tools: Tool[] = []; - private client: InspectorClientProtocol | null = null; - private unsubscribe: (() => void) | null = null; - private _metadata: Record | undefined = undefined; - private listChanged = false; - +export class ManagedToolsState extends ManagedListState< + Tool, + ManagedToolsStateEventMap +> { constructor(client: InspectorClientProtocol) { - super(); - this.client = client; - const onConnect = (): void => { - void this.refresh(); - }; - const onToolsListChanged = (): void => { - // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise peek: fetch and compare, lighting the - // indicator only when the list actually changed — many servers re-send an - // identical list on `list_changed` (#1402, #1444). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else { - void this.peekForChange(); - } - }; - const onStatusChange = (): void => { - if (this.client?.getStatus() === "disconnected") { - this.tools = []; - this.dispatchTypedEvent("toolsChange", []); - this.setListChanged(false); - } - }; - this.client.addEventListener("connect", onConnect); - this.client.addEventListener("toolsListChanged", onToolsListChanged); - this.client.addEventListener("statusChange", onStatusChange); - this.unsubscribe = () => { - if (this.client) { - this.client.removeEventListener("connect", onConnect); - this.client.removeEventListener("toolsListChanged", onToolsListChanged); - this.client.removeEventListener("statusChange", onStatusChange); - } - this.client = null; - }; + super(client, { + changeEvent: "toolsChange", + listChangedEvent: "toolsListChanged", + capabilityKey: "tools", + itemLabel: "tools", + supportsIndicator: true, + fetchPage: async (c, cursor, metadata) => { + const result = await c.listTools(cursor, metadata); + return { items: result.tools, nextCursor: result.nextCursor }; + }, + }); } getTools(): Tool[] { - return [...this.tools]; - } - - /** Whether a `tools/list_changed` arrived since the last user refresh. */ - getListChanged(): boolean { - return this.listChanged; - } - - /** - * Clear the list-changed flag — called when the user refreshes the list - * (the auto-refresh on the notification leaves it set so the indicator - * stays visible until acknowledged). - */ - clearListChanged(): void { - this.setListChanged(false); - } - - private setListChanged(value: boolean): void { - if (this.listChanged === value) return; - this.listChanged = value; - this.dispatchTypedEvent("listChangedChange", value); - } - - setMetadata(metadata?: Record): void { - this._metadata = metadata; - } - - async refresh(metadata?: Record): Promise { - const next = await this.fetchTools(metadata); - // `null` means not connected — leave the current list untouched. - if (next === null) return this.getTools(); - this.applyTools(next); - return this.getTools(); - } - - /** - * Fetch all pages without mutating state or dispatching — used by both - * refresh (apply) and peek (compare). Returns `null` when not connected, or - * `[]` when the server doesn't advertise the `tools` capability (calling - * tools/list there returns -32601 "Method not found", which would spam the - * console; empty list is the right semantics). - */ - private async fetchTools( - metadata?: Record, - ): Promise { - const client = this.client; - if (!client || client.getStatus() !== "connected") return null; - if (!client.getCapabilities()?.tools) return []; - const effectiveMetadata = metadata ?? this._metadata; - let tools: Tool[] = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listTools(cursor, effectiveMetadata); - tools = cursor ? [...tools, ...result.tools] : result.tools; - cursor = result.nextCursor; - pageCount++; - if (pageCount >= MAX_PAGES) { - throw new Error( - `Maximum pagination limit (${MAX_PAGES} pages) reached while listing tools`, - ); - } - } while (cursor); - return tools; - } - - /** Commit a fetched list as the current one and notify subscribers. */ - private applyTools(tools: Tool[]): void { - this.tools = tools; - this.dispatchTypedEvent("toolsChange", this.tools); - } - - /** - * Fetch on `list_changed` and track whether the server's list differs from - * what's displayed. The displayed list is left untouched — the user still - * pulls the new one via Refresh (pull-on-demand). Many servers re-send an - * identical list on `list_changed`; this keeps the indicator dark in that - * case, and also clears it if a later notification reverts the server back - * to the displayed list (nothing left to pull). The flag is order-sensitive: - * a reorder is a visible change the user would see on Refresh, so it counts - * (#1444). - */ - private async peekForChange(): Promise { - const next = await this.fetchTools(); - if (next === null) return; - this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.tools)); - } - - destroy(): void { - this.unsubscribe?.(); - this.unsubscribe = null; - this.tools = []; + return this.getItems(); } } From 93e821c119b3fa05599f618b27b0d0d4f1c10305 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 14:32:21 -0400 Subject: [PATCH 4/9] perf(core): coalesce bursts of list_changed peeks (#1444 review) A rapid burst of */list_changed notifications fired one concurrent paginated fetch per notification. Guard peekForChange with an in-flight flag: while a peek is fetching, a new notification queues a single re-run instead of launching a concurrent fetch, and the running peek loops once more on completion to capture the latest server state. Collapses a burst into sequential peeks. Adds a coalescing test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/mcp/state/managedToolsState.test.ts | 26 +++++++++++++++++ core/mcp/state/managedListState.ts | 28 +++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index a225d4753..3024ece24 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -175,6 +175,32 @@ describe("ManagedToolsState", () => { expect(state.getListChanged()).toBe(false); }); + it("coalesces a burst of list_changed into sequential peeks, not N concurrent fetches (#1444)", async () => { + client.setStatus("connected"); + // Hang the first peek's fetch so the rest of the burst lands while it's in + // flight; queue a page for the single coalesced re-run. + let release: (value: { tools: Tool[] }) => void = () => {}; + client.listTools.mockImplementationOnce( + () => + new Promise<{ tools: Tool[] }>((resolve) => { + release = resolve; + }), + ); + client.queueToolPages({ tools: [tool("a")] }); + + client.dispatchTypedEvent("toolsListChanged"); + client.dispatchTypedEvent("toolsListChanged"); + client.dispatchTypedEvent("toolsListChanged"); + // While the first peek's fetch is hung, only one fetch has started. + await Promise.resolve(); + expect(client.listTools).toHaveBeenCalledTimes(1); + + // Releasing it runs exactly one coalesced re-run, not two more. + release({ tools: [tool("a")] }); + await new Promise((r) => setTimeout(r, 0)); + expect(client.listTools).toHaveBeenCalledTimes(2); + }); + it("clears the indicator when a later notification reverts the list to the displayed one (#1444)", async () => { client.setStatus("connected"); client.queueToolPages({ tools: [tool("a")] }); diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index 188dcfb5b..025a44e57 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -64,6 +64,11 @@ export abstract class ManagedListState< private _metadata: Record | undefined = undefined; private listChanged = false; private readonly config: ManagedListConfig; + // Coalesce a burst of `list_changed` notifications: while a peek is fetching, + // a new notification just queues a single re-run instead of firing another + // concurrent paginated fetch. + private peeking = false; + private peekQueued = false; constructor( client: InspectorClientProtocol, @@ -211,9 +216,26 @@ export abstract class ManagedListState< * (#1444). */ private async peekForChange(): Promise { - const next = await this.fetchAll(); - if (next === null) return; - this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.items)); + // A peek is already fetching — queue a single re-run rather than launching + // a concurrent paginated fetch. The in-flight peek loops once more on + // completion to capture the latest server state. + if (this.peeking) { + this.peekQueued = true; + return; + } + this.peeking = true; + try { + do { + this.peekQueued = false; + const next = await this.fetchAll(); + if (next === null) return; + this.setListChanged( + JSON.stringify(next) !== JSON.stringify(this.items), + ); + } while (this.peekQueued); + } finally { + this.peeking = false; + } } destroy(): void { From 66a4715c5f2c84face4f01bb4b6516927186d764 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 15:43:37 -0400 Subject: [PATCH 5/9] feat(core): debounce list_changed bursts into a single fetch (#1444) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-flight guard only coalesced fetches that actually overlapped — when a server (e.g. everything) emits a burst of */list_changed spaced just far enough apart that each list call returns before the next notification, we still fired one list call per notification. Add a real debounce in the ManagedListState base: a notification (re)starts a timer and the refresh/peek runs once the burst settles, collapsing N notifications into one fetch. The in-flight coalescing guard stays as a second line of defense for a post-debounce notification landing during a slow paginated fetch. The delay is injectable (DEFAULT_LIST_CHANGED_DEBOUNCE_MS = 250; subclasses take an optional constructor arg) so tests run with 0 for fast, determinist ic behavior. Timer is cleared on disconnect, unsubscribe, and destroy. Adds debounce + in-flight regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcp/state/managedPromptsState.test.ts | 8 +-- .../managedResourceTemplatesState.test.ts | 14 +++-- .../mcp/state/managedResourcesState.test.ts | 8 +-- .../core/mcp/state/managedToolsState.test.ts | 38 ++++++++---- .../state/resourceSubscriptionsState.test.ts | 8 +-- .../core/react/useManagedPrompts.test.tsx | 2 +- .../useManagedResourceTemplates.test.tsx | 2 +- .../core/react/useManagedResources.test.tsx | 2 +- .../test/core/react/useManagedTools.test.tsx | 2 +- core/mcp/state/managedListState.ts | 58 +++++++++++++++---- core/mcp/state/managedPromptsState.ts | 11 +++- .../state/managedResourceTemplatesState.ts | 11 +++- core/mcp/state/managedResourcesState.ts | 11 +++- core/mcp/state/managedToolsState.ts | 11 +++- 14 files changed, 135 insertions(+), 51 deletions(-) diff --git a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts index 600b3b22d..ba6edb095 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -43,7 +43,7 @@ describe("ManagedPromptsState", () => { // exercise the live `listPrompts` path; capability-absent tests below // override this. client = new FakeInspectorClient({ capabilities: { prompts: {} } }); - state = new ManagedPromptsState(client); + state = new ManagedPromptsState(client, 0); }); it("starts with empty prompts", () => { @@ -69,7 +69,7 @@ describe("ManagedPromptsState", () => { capabilities: { tools: {}, resources: {} }, }); promptless.setStatus("connected"); - const promptlessState = new ManagedPromptsState(promptless); + const promptlessState = new ManagedPromptsState(promptless, 0); const result = await promptlessState.refresh(); expect(result).toEqual([]); @@ -81,7 +81,7 @@ describe("ManagedPromptsState", () => { // there, not only the publicly-callable refresh(). const promptless = new FakeInspectorClient({ capabilities: { tools: {} } }); promptless.setStatus("connected"); - const promptlessState = new ManagedPromptsState(promptless); + const promptlessState = new ManagedPromptsState(promptless, 0); promptless.dispatchTypedEvent("connect"); // Yield so the async refresh chained off connect runs. @@ -179,7 +179,7 @@ describe("ManagedPromptsState", () => { serverSettings: AUTO_REFRESH_SETTINGS, }); autoClient.setStatus("connected"); - const autoState = new ManagedPromptsState(autoClient); + const autoState = new ManagedPromptsState(autoClient, 0); autoClient.queuePromptPages({ prompts: [prompt("a")] }); const changed = waitForPromptsChange(autoState); autoClient.dispatchTypedEvent("promptsListChanged"); diff --git a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts index ea26751dd..41538af9f 100644 --- a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts @@ -40,7 +40,7 @@ describe("ManagedResourceTemplatesState", () => { // tests below override this. (Templates are gated on the `resources` // capability — the spec defines no separate `resourceTemplates` one.) client = new FakeInspectorClient({ capabilities: { resources: {} } }); - state = new ManagedResourceTemplatesState(client); + state = new ManagedResourceTemplatesState(client, 0); }); it("starts with empty resource templates", () => { @@ -67,7 +67,10 @@ describe("ManagedResourceTemplatesState", () => { capabilities: { tools: {}, prompts: {} }, }); resourceless.setStatus("connected"); - const resourcelessState = new ManagedResourceTemplatesState(resourceless); + const resourcelessState = new ManagedResourceTemplatesState( + resourceless, + 0, + ); const result = await resourcelessState.refresh(); expect(result).toEqual([]); @@ -81,7 +84,10 @@ describe("ManagedResourceTemplatesState", () => { capabilities: { tools: {} }, }); resourceless.setStatus("connected"); - const resourcelessState = new ManagedResourceTemplatesState(resourceless); + const resourcelessState = new ManagedResourceTemplatesState( + resourceless, + 0, + ); resourceless.dispatchTypedEvent("connect"); // Yield so the async refresh chained off connect runs. @@ -166,7 +172,7 @@ describe("ManagedResourceTemplatesState", () => { serverSettings: AUTO_REFRESH_SETTINGS, }); autoClient.setStatus("connected"); - const autoState = new ManagedResourceTemplatesState(autoClient); + const autoState = new ManagedResourceTemplatesState(autoClient, 0); autoClient.queueResourceTemplatePages({ resourceTemplates: [template("a")], }); diff --git a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts index c1f417ee5..4b21dcb3f 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -45,7 +45,7 @@ describe("ManagedResourcesState", () => { // tests exercise the live `listResources` path; capability-absent tests // below override this. client = new FakeInspectorClient({ capabilities: { resources: {} } }); - state = new ManagedResourcesState(client); + state = new ManagedResourcesState(client, 0); }); it("starts with empty resources", () => { @@ -72,7 +72,7 @@ describe("ManagedResourcesState", () => { capabilities: { tools: {}, prompts: {} }, }); resourceless.setStatus("connected"); - const resourcelessState = new ManagedResourcesState(resourceless); + const resourcelessState = new ManagedResourcesState(resourceless, 0); const result = await resourcelessState.refresh(); expect(result).toEqual([]); @@ -86,7 +86,7 @@ describe("ManagedResourcesState", () => { capabilities: { tools: {} }, }); resourceless.setStatus("connected"); - const resourcelessState = new ManagedResourcesState(resourceless); + const resourcelessState = new ManagedResourcesState(resourceless, 0); resourceless.dispatchTypedEvent("connect"); // Yield so the async refresh chained off connect runs. @@ -188,7 +188,7 @@ describe("ManagedResourcesState", () => { serverSettings: AUTO_REFRESH_SETTINGS, }); autoClient.setStatus("connected"); - const autoState = new ManagedResourcesState(autoClient); + const autoState = new ManagedResourcesState(autoClient, 0); autoClient.queueResourcePages({ resources: [resource("a://1")] }); const changed = waitForResourcesChange(autoState); autoClient.dispatchTypedEvent("resourcesListChanged"); diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index 3024ece24..7ffb7a67f 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -43,7 +43,7 @@ describe("ManagedToolsState", () => { // exercise the live `listTools` path; capability-absent tests below // override this. client = new FakeInspectorClient({ capabilities: { tools: {} } }); - state = new ManagedToolsState(client); + state = new ManagedToolsState(client, 0); }); it("starts with empty tools", () => { @@ -69,7 +69,7 @@ describe("ManagedToolsState", () => { capabilities: { prompts: {}, resources: {} }, }); toolless.setStatus("connected"); - const toollessState = new ManagedToolsState(toolless); + const toollessState = new ManagedToolsState(toolless, 0); const result = await toollessState.refresh(); expect(result).toEqual([]); @@ -81,7 +81,7 @@ describe("ManagedToolsState", () => { // there, not only the publicly-callable refresh(). const toolless = new FakeInspectorClient({ capabilities: { prompts: {} } }); toolless.setStatus("connected"); - const toollessState = new ManagedToolsState(toolless); + const toollessState = new ManagedToolsState(toolless, 0); toolless.dispatchTypedEvent("connect"); // Yield so the async refresh chained off connect runs. @@ -175,10 +175,23 @@ describe("ManagedToolsState", () => { expect(state.getListChanged()).toBe(false); }); - it("coalesces a burst of list_changed into sequential peeks, not N concurrent fetches (#1444)", async () => { + it("debounces a burst of list_changed into a single fetch (#1444)", async () => { + // The everything-server case: a rapid burst of notifications must collapse + // into one list call once it settles, not one per notification. + client.setStatus("connected"); + client.queueToolPages({ tools: [tool("a")] }); + const changed = waitForListChanged(state); + client.dispatchTypedEvent("toolsListChanged"); + client.dispatchTypedEvent("toolsListChanged"); + client.dispatchTypedEvent("toolsListChanged"); + expect(await changed).toBe(true); + expect(client.listTools).toHaveBeenCalledTimes(1); // one debounced fetch + }); + + it("coalesces a peek that fires while an earlier one is still fetching (#1444)", async () => { + // Defense beyond the debounce: a post-debounce notification landing during + // an in-flight peek queues a single re-run instead of a concurrent fetch. client.setStatus("connected"); - // Hang the first peek's fetch so the rest of the burst lands while it's in - // flight; queue a page for the single coalesced re-run. let release: (value: { tools: Tool[] }) => void = () => {}; client.listTools.mockImplementationOnce( () => @@ -188,14 +201,17 @@ describe("ManagedToolsState", () => { ); client.queueToolPages({ tools: [tool("a")] }); + // First notification → debounced peek starts and hangs. client.dispatchTypedEvent("toolsListChanged"); + await new Promise((r) => setTimeout(r, 0)); + expect(client.listTools).toHaveBeenCalledTimes(1); + + // Second notification while the peek is hung → coalesced, no concurrent fetch. client.dispatchTypedEvent("toolsListChanged"); - client.dispatchTypedEvent("toolsListChanged"); - // While the first peek's fetch is hung, only one fetch has started. - await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); expect(client.listTools).toHaveBeenCalledTimes(1); - // Releasing it runs exactly one coalesced re-run, not two more. + // Releasing the first peek runs exactly one coalesced re-run. release({ tools: [tool("a")] }); await new Promise((r) => setTimeout(r, 0)); expect(client.listTools).toHaveBeenCalledTimes(2); @@ -226,7 +242,7 @@ describe("ManagedToolsState", () => { serverSettings: AUTO_REFRESH_SETTINGS, }); autoClient.setStatus("connected"); - const autoState = new ManagedToolsState(autoClient); + const autoState = new ManagedToolsState(autoClient, 0); autoClient.queueToolPages({ tools: [tool("a")] }); const changed = waitForToolsChange(autoState); autoClient.dispatchTypedEvent("toolsListChanged"); diff --git a/clients/web/src/test/core/mcp/state/resourceSubscriptionsState.test.ts b/clients/web/src/test/core/mcp/state/resourceSubscriptionsState.test.ts index cc9c6ef63..44d2ddee1 100644 --- a/clients/web/src/test/core/mcp/state/resourceSubscriptionsState.test.ts +++ b/clients/web/src/test/core/mcp/state/resourceSubscriptionsState.test.ts @@ -60,7 +60,7 @@ describe("ResourceSubscriptionsState", () => { }); it("resolves Resource references via ManagedResourcesState when provided", async () => { - const resourcesState = new ManagedResourcesState(client); + const resourcesState = new ManagedResourcesState(client, 0); client.queueResourcePages({ resources: [resource("file:///a", { name: "Alpha", title: "Title A" })], }); @@ -154,7 +154,7 @@ describe("ResourceSubscriptionsState", () => { }); it("re-resolves Resource references when ManagedResourcesState refreshes", async () => { - const resourcesState = new ManagedResourcesState(client); + const resourcesState = new ManagedResourcesState(client, 0); const state = new ResourceSubscriptionsState(client, resourcesState); client.dispatchTypedEvent("resourceSubscriptionsChange", ["file:///a"]); expect(state.getSubscriptions()[0].resource.name).toBe("file:///a"); @@ -169,7 +169,7 @@ describe("ResourceSubscriptionsState", () => { }); it("does not re-emit on resourcesChange when no URIs are subscribed", async () => { - const resourcesState = new ManagedResourcesState(client); + const resourcesState = new ManagedResourcesState(client, 0); const state = new ResourceSubscriptionsState(client, resourcesState); const handler = vi.fn(); state.addEventListener("subscriptionsChange", handler); @@ -199,7 +199,7 @@ describe("ResourceSubscriptionsState", () => { }); it("destroy unsubscribes from client and resources state events", () => { - const resourcesState = new ManagedResourcesState(client); + const resourcesState = new ManagedResourcesState(client, 0); const state = new ResourceSubscriptionsState(client, resourcesState); client.dispatchTypedEvent("resourceSubscriptionsChange", ["file:///a"]); expect(state.getSubscriptions()).toHaveLength(1); diff --git a/clients/web/src/test/core/react/useManagedPrompts.test.tsx b/clients/web/src/test/core/react/useManagedPrompts.test.tsx index 489c15091..815beb571 100644 --- a/clients/web/src/test/core/react/useManagedPrompts.test.tsx +++ b/clients/web/src/test/core/react/useManagedPrompts.test.tsx @@ -18,7 +18,7 @@ describe("useManagedPrompts", () => { status: "connected", capabilities: { prompts: {} }, }); - state = new ManagedPromptsState(client); + state = new ManagedPromptsState(client, 0); }); it("returns the initial prompts snapshot from the state", async () => { diff --git a/clients/web/src/test/core/react/useManagedResourceTemplates.test.tsx b/clients/web/src/test/core/react/useManagedResourceTemplates.test.tsx index ac097da80..87505871a 100644 --- a/clients/web/src/test/core/react/useManagedResourceTemplates.test.tsx +++ b/clients/web/src/test/core/react/useManagedResourceTemplates.test.tsx @@ -18,7 +18,7 @@ describe("useManagedResourceTemplates", () => { status: "connected", capabilities: { resources: {} }, }); - state = new ManagedResourceTemplatesState(client); + state = new ManagedResourceTemplatesState(client, 0); }); it("returns the initial templates snapshot from the state", async () => { diff --git a/clients/web/src/test/core/react/useManagedResources.test.tsx b/clients/web/src/test/core/react/useManagedResources.test.tsx index 72eeab2ae..154eb15c1 100644 --- a/clients/web/src/test/core/react/useManagedResources.test.tsx +++ b/clients/web/src/test/core/react/useManagedResources.test.tsx @@ -18,7 +18,7 @@ describe("useManagedResources", () => { status: "connected", capabilities: { resources: {} }, }); - state = new ManagedResourcesState(client); + state = new ManagedResourcesState(client, 0); }); it("returns the initial resources snapshot from the state", async () => { diff --git a/clients/web/src/test/core/react/useManagedTools.test.tsx b/clients/web/src/test/core/react/useManagedTools.test.tsx index aadeb0a7e..742437ebc 100644 --- a/clients/web/src/test/core/react/useManagedTools.test.tsx +++ b/clients/web/src/test/core/react/useManagedTools.test.tsx @@ -18,7 +18,7 @@ describe("useManagedTools", () => { status: "connected", capabilities: { tools: {} }, }); - state = new ManagedToolsState(client); + state = new ManagedToolsState(client, 0); }); it("returns the initial tools snapshot from the state", async () => { diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index 025a44e57..5d1dcadbd 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -18,6 +18,14 @@ import { TypedEventTarget } from "../typedEventTarget.js"; const MAX_PAGES = 100; +/** + * Default delay (ms) for debouncing `list_changed` notifications. Servers + * (e.g. the everything server) can emit a rapid burst; debouncing collapses the + * burst into a single refresh/peek once it settles instead of one list call per + * notification (#1444). + */ +export const DEFAULT_LIST_CHANGED_DEBOUNCE_MS = 250; + /** Every managed-list event map carries the list-changed indicator event. */ export interface ManagedListEventMap { listChangedChange: boolean; @@ -52,6 +60,8 @@ export interface ManagedListConfig { * pulled via the screen's Refresh instead. */ supportsIndicator: boolean; + /** Debounce delay (ms) for `list_changed` bursts. */ + debounceMs: number; } export abstract class ManagedListState< @@ -64,9 +74,12 @@ export abstract class ManagedListState< private _metadata: Record | undefined = undefined; private listChanged = false; private readonly config: ManagedListConfig; - // Coalesce a burst of `list_changed` notifications: while a peek is fetching, - // a new notification just queues a single re-run instead of firing another - // concurrent paginated fetch. + // Debounce a burst of `list_changed` notifications into a single + // refresh/peek once it settles. + private listChangedTimer: ReturnType | null = null; + // Second line of defense beyond the debounce: while a peek is fetching, a new + // (post-debounce) notification queues a single re-run instead of firing + // another concurrent paginated fetch. private peeking = false; private peekQueued = false; @@ -81,18 +94,20 @@ export abstract class ManagedListState< void this.refresh(); }; const onListChanged = (): void => { - // When the server opts into auto-refresh (per-server setting), pull the - // new list immediately. Otherwise, for lists with an indicator, peek and - // diff so the indicator lights only on a real change; lists without an - // indicator simply wait for the user's Refresh (#1402, #1444). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else if (config.supportsIndicator) { - void this.peekForChange(); - } + // Debounce: collapse a burst of notifications into one refresh/peek once + // it settles, instead of one list call per notification (#1444). + if (this.listChangedTimer !== null) clearTimeout(this.listChangedTimer); + this.listChangedTimer = setTimeout(() => { + this.listChangedTimer = null; + this.runListChanged(); + }, config.debounceMs); }; const onStatusChange = (): void => { if (this.client?.getStatus() === "disconnected") { + if (this.listChangedTimer !== null) { + clearTimeout(this.listChangedTimer); + this.listChangedTimer = null; + } this.items = []; this.dispatchChange(); this.setListChanged(false); @@ -102,6 +117,10 @@ export abstract class ManagedListState< this.client.addEventListener(config.listChangedEvent, onListChanged); this.client.addEventListener("statusChange", onStatusChange); this.unsubscribe = () => { + if (this.listChangedTimer !== null) { + clearTimeout(this.listChangedTimer); + this.listChangedTimer = null; + } if (this.client) { this.client.removeEventListener("connect", onConnect); this.client.removeEventListener(config.listChangedEvent, onListChanged); @@ -111,6 +130,21 @@ export abstract class ManagedListState< }; } + /** + * The debounced list-changed action. When the server opts into auto-refresh + * (per-server setting), pull the new list immediately. Otherwise, for lists + * with an indicator, peek and diff so the indicator lights only on a real + * change; lists without an indicator simply wait for the user's Refresh + * (#1402, #1444). + */ + private runListChanged(): void { + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + void this.refresh(); + } else if (this.config.supportsIndicator) { + void this.peekForChange(); + } + } + /** Defensive copy of the current list. */ protected getItems(): T[] { return [...this.items]; diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index 9d31128a2..79e5d7f43 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -5,7 +5,10 @@ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; -import { ManagedListState } from "./managedListState.js"; +import { + ManagedListState, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} from "./managedListState.js"; export interface ManagedPromptsStateEventMap { promptsChange: Prompt[]; @@ -22,13 +25,17 @@ export class ManagedPromptsState extends ManagedListState< Prompt, ManagedPromptsStateEventMap > { - constructor(client: InspectorClientProtocol) { + constructor( + client: InspectorClientProtocol, + debounceMs = DEFAULT_LIST_CHANGED_DEBOUNCE_MS, + ) { super(client, { changeEvent: "promptsChange", listChangedEvent: "promptsListChanged", capabilityKey: "prompts", itemLabel: "prompts", supportsIndicator: true, + debounceMs, fetchPage: async (c, cursor, metadata) => { const result = await c.listPrompts(cursor, metadata); return { items: result.prompts, nextCursor: result.nextCursor }; diff --git a/core/mcp/state/managedResourceTemplatesState.ts b/core/mcp/state/managedResourceTemplatesState.ts index 745df426d..fd4d22e8f 100644 --- a/core/mcp/state/managedResourceTemplatesState.ts +++ b/core/mcp/state/managedResourceTemplatesState.ts @@ -12,7 +12,10 @@ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { ResourceTemplate } from "@modelcontextprotocol/sdk/types.js"; -import { ManagedListState } from "./managedListState.js"; +import { + ManagedListState, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} from "./managedListState.js"; export interface ManagedResourceTemplatesStateEventMap { resourceTemplatesChange: ResourceTemplate[]; @@ -27,7 +30,10 @@ export class ManagedResourceTemplatesState extends ManagedListState< ResourceTemplate, ManagedResourceTemplatesStateEventMap > { - constructor(client: InspectorClientProtocol) { + constructor( + client: InspectorClientProtocol, + debounceMs = DEFAULT_LIST_CHANGED_DEBOUNCE_MS, + ) { super(client, { changeEvent: "resourceTemplatesChange", listChangedEvent: "resourceTemplatesListChanged", @@ -35,6 +41,7 @@ export class ManagedResourceTemplatesState extends ManagedListState< capabilityKey: "resources", itemLabel: "resource templates", supportsIndicator: false, + debounceMs, fetchPage: async (c, cursor, metadata) => { const result = await c.listResourceTemplates(cursor, metadata); return { diff --git a/core/mcp/state/managedResourcesState.ts b/core/mcp/state/managedResourcesState.ts index 369965084..c07e7a781 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -5,7 +5,10 @@ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { Resource } from "@modelcontextprotocol/sdk/types.js"; -import { ManagedListState } from "./managedListState.js"; +import { + ManagedListState, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} from "./managedListState.js"; export interface ManagedResourcesStateEventMap { resourcesChange: Resource[]; @@ -22,13 +25,17 @@ export class ManagedResourcesState extends ManagedListState< Resource, ManagedResourcesStateEventMap > { - constructor(client: InspectorClientProtocol) { + constructor( + client: InspectorClientProtocol, + debounceMs = DEFAULT_LIST_CHANGED_DEBOUNCE_MS, + ) { super(client, { changeEvent: "resourcesChange", listChangedEvent: "resourcesListChanged", capabilityKey: "resources", itemLabel: "resources", supportsIndicator: true, + debounceMs, fetchPage: async (c, cursor, metadata) => { const result = await c.listResources(cursor, metadata); return { items: result.resources, nextCursor: result.nextCursor }; diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index 6e3aef3f6..aeb8e2914 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -5,7 +5,10 @@ import type { InspectorClientProtocol } from "../inspectorClientProtocol.js"; import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -import { ManagedListState } from "./managedListState.js"; +import { + ManagedListState, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} from "./managedListState.js"; export interface ManagedToolsStateEventMap { toolsChange: Tool[]; @@ -22,13 +25,17 @@ export class ManagedToolsState extends ManagedListState< Tool, ManagedToolsStateEventMap > { - constructor(client: InspectorClientProtocol) { + constructor( + client: InspectorClientProtocol, + debounceMs = DEFAULT_LIST_CHANGED_DEBOUNCE_MS, + ) { super(client, { changeEvent: "toolsChange", listChangedEvent: "toolsListChanged", capabilityKey: "tools", itemLabel: "tools", supportsIndicator: true, + debounceMs, fetchPage: async (c, cursor, metadata) => { const result = await c.listTools(cursor, metadata); return { items: result.tools, nextCursor: result.nextCursor }; From 47239167d1905d9d96cd8a97ddf5c6ab7392109d Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 15:50:42 -0400 Subject: [PATCH 6/9] refactor(core): extend the in-flight guard to the auto-refresh path (#1444 review) The overlap guard was only on the peek path; a burst-during-a-slow auto- refresh could let a slower older fetch clobber a newer list (applyItems is last-write-wins). Lift the guard from peekForChange up to runListChanged so it wraps both the auto-refresh and peek actions: while one is fetching, a new post-debounce notification queues a single re-run. Adds an auto-refresh coalescing test alongside the peek one. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/mcp/state/managedToolsState.test.ts | 33 +++++++++ core/mcp/state/managedListState.ts | 71 ++++++++++--------- 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index 7ffb7a67f..f52b658b0 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -217,6 +217,39 @@ describe("ManagedToolsState", () => { expect(client.listTools).toHaveBeenCalledTimes(2); }); + it("coalesces an auto-refresh that fires while an earlier one is still fetching (#1444)", async () => { + // The guard covers the auto-refresh path too, so a slow refresh can't be + // clobbered by an overlapping one from a later notification. + const autoClient = new FakeInspectorClient({ + capabilities: { tools: {} }, + serverSettings: AUTO_REFRESH_SETTINGS, + }); + autoClient.setStatus("connected"); + const autoState = new ManagedToolsState(autoClient, 0); + let release: (value: { tools: Tool[] }) => void = () => {}; + autoClient.listTools.mockImplementationOnce( + () => + new Promise<{ tools: Tool[] }>((resolve) => { + release = resolve; + }), + ); + autoClient.queueToolPages({ tools: [tool("a")] }); + + autoClient.dispatchTypedEvent("toolsListChanged"); + await new Promise((r) => setTimeout(r, 0)); + expect(autoClient.listTools).toHaveBeenCalledTimes(1); + + autoClient.dispatchTypedEvent("toolsListChanged"); + await new Promise((r) => setTimeout(r, 0)); + expect(autoClient.listTools).toHaveBeenCalledTimes(1); // coalesced + + release({ tools: [tool("a")] }); + await new Promise((r) => setTimeout(r, 0)); + expect(autoClient.listTools).toHaveBeenCalledTimes(2); + // The (coalesced) auto-refresh applied the new list. + expect(autoState.getTools().map((t) => t.name)).toEqual(["a"]); + }); + it("clears the indicator when a later notification reverts the list to the displayed one (#1444)", async () => { client.setStatus("connected"); client.queueToolPages({ tools: [tool("a")] }); diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index 5d1dcadbd..32dd95831 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -77,11 +77,11 @@ export abstract class ManagedListState< // Debounce a burst of `list_changed` notifications into a single // refresh/peek once it settles. private listChangedTimer: ReturnType | null = null; - // Second line of defense beyond the debounce: while a peek is fetching, a new - // (post-debounce) notification queues a single re-run instead of firing - // another concurrent paginated fetch. - private peeking = false; - private peekQueued = false; + // Second line of defense beyond the debounce: while a list-changed action is + // fetching, a new (post-debounce) notification queues a single re-run instead + // of firing another concurrent paginated fetch. + private running = false; + private runQueued = false; constructor( client: InspectorClientProtocol, @@ -131,17 +131,35 @@ export abstract class ManagedListState< } /** - * The debounced list-changed action. When the server opts into auto-refresh - * (per-server setting), pull the new list immediately. Otherwise, for lists - * with an indicator, peek and diff so the indicator lights only on a real - * change; lists without an indicator simply wait for the user's Refresh - * (#1402, #1444). + * The debounced list-changed action, guarded against overlap: if one is + * already fetching, a new (post-debounce) notification queues a single re-run + * rather than launching a concurrent paginated fetch. This covers both the + * auto-refresh path (whose last-write-wins `applyItems` could otherwise let a + * slow older fetch clobber a newer list) and the peek path (#1444). */ private runListChanged(): void { - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else if (this.config.supportsIndicator) { - void this.peekForChange(); + if (this.running) { + this.runQueued = true; + return; + } + void this.runListChangedOnce(); + } + + private async runListChangedOnce(): Promise { + this.running = true; + try { + do { + this.runQueued = false; + // Read the setting at fire time so a `setServerSettings` toggle that + // lands mid-burst is honored on the settled action. + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + await this.refresh(); + } else if (this.config.supportsIndicator) { + await this.peekForChange(); + } + } while (this.runQueued); + } finally { + this.running = false; } } @@ -250,26 +268,11 @@ export abstract class ManagedListState< * (#1444). */ private async peekForChange(): Promise { - // A peek is already fetching — queue a single re-run rather than launching - // a concurrent paginated fetch. The in-flight peek loops once more on - // completion to capture the latest server state. - if (this.peeking) { - this.peekQueued = true; - return; - } - this.peeking = true; - try { - do { - this.peekQueued = false; - const next = await this.fetchAll(); - if (next === null) return; - this.setListChanged( - JSON.stringify(next) !== JSON.stringify(this.items), - ); - } while (this.peekQueued); - } finally { - this.peeking = false; - } + // Overlap is handled by the runListChanged guard, so this just fetches and + // diffs against the displayed list. + const next = await this.fetchAll(); + if (next === null) return; + this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.items)); } destroy(): void { From 7927de4161ec16fd2adbb721503012babf5ffe14 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 17:09:00 -0400 Subject: [PATCH 7/9] feat(core): light the indicator without fetching when auto-refresh is off (#1444) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per maintainer feedback: with auto-refresh off, a list_changed should not issue any list call. The diff-aware peek (fetch-to-compare) meant one tools/list went out per settled burst even with auto-refresh off, which the maintainer doesn't want. Revert flag-only mode to lighting the indicator blindly on a (debounced) list_changed — no network — and the user pulls the new list via Refresh. Trade-off (accepted): the indicator lights even when the server re-sent an identical list (the everything-server case), since we no longer fetch to compare. Keeps the debounce (one indicator light per burst), the auto-refresh path + its in-flight guard, the live setServerSettings, and the ManagedListState consolidation. Removes peekForChange and its tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcp/state/managedPromptsState.test.ts | 31 ++---- .../mcp/state/managedResourcesState.test.ts | 33 ++----- .../core/mcp/state/managedToolsState.test.ts | 95 +++---------------- core/mcp/state/managedListState.ts | 30 +++--- core/mcp/state/managedPromptsState.ts | 6 +- core/mcp/state/managedResourcesState.ts | 6 +- core/mcp/state/managedToolsState.ts | 6 +- 7 files changed, 47 insertions(+), 160 deletions(-) diff --git a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts index ba6edb095..aa5c397e8 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -143,34 +143,15 @@ describe("ManagedPromptsState", () => { expect(next.map((p) => p.name)).toEqual(["a"]); }); - it("promptsListChanged peeks but does NOT replace the displayed list by default", async () => { - // Diff-aware (#1444): the notification fetches to compare, but the - // displayed list stays put until the user pulls via Refresh. + it("promptsListChanged lights the indicator without fetching by default (#1444)", async () => { + // Auto-refresh off: a list_changed lights the indicator with NO list call; + // the user pulls the new list via Refresh. client.setStatus("connected"); - client.queuePromptPages({ prompts: [prompt("a"), prompt("b")] }); const changed = waitForListChanged(state); client.dispatchTypedEvent("promptsListChanged"); - expect(await changed).toBe(true); // the peeked list differs from [] - expect(client.listPrompts).toHaveBeenCalled(); // it fetched to compare - expect(state.getPrompts()).toEqual([]); // ...but did not replace the display - }); - - it("promptsListChanged does NOT light the indicator when the list is unchanged", async () => { - client.setStatus("connected"); - client.queuePromptPages({ prompts: [prompt("a")] }); - await state.refresh(); - expect(state.getPrompts().map((p) => p.name)).toEqual(["a"]); - - let fired = false; - state.addEventListener("listChangedChange", () => { - fired = true; - }); - client.queuePromptPages({ prompts: [prompt("a")] }); - client.dispatchTypedEvent("promptsListChanged"); - await new Promise((r) => setTimeout(r, 0)); - expect(client.listPrompts).toHaveBeenCalledTimes(2); // refresh + peek - expect(fired).toBe(false); - expect(state.getListChanged()).toBe(false); + expect(await changed).toBe(true); + expect(client.listPrompts).not.toHaveBeenCalled(); // no automatic fetch + expect(state.getPrompts()).toEqual([]); // displayed list untouched }); it("promptsListChanged auto-refreshes when the server opts in", async () => { diff --git a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts index 4b21dcb3f..5a855fa1f 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -150,36 +150,15 @@ describe("ManagedResourcesState", () => { expect(next.map((r) => r.uri)).toEqual(["a://1"]); }); - it("resourcesListChanged peeks but does NOT replace the displayed list by default", async () => { - // Diff-aware (#1444): the notification fetches to compare, but the - // displayed list stays put until the user pulls via Refresh. + it("resourcesListChanged lights the indicator without fetching by default (#1444)", async () => { + // Auto-refresh off: a list_changed lights the indicator with NO list call; + // the user pulls the new list via Refresh. client.setStatus("connected"); - client.queueResourcePages({ - resources: [resource("a://1"), resource("a://2")], - }); const changed = waitForListChanged(state); client.dispatchTypedEvent("resourcesListChanged"); - expect(await changed).toBe(true); // the peeked list differs from [] - expect(client.listResources).toHaveBeenCalled(); // it fetched to compare - expect(state.getResources()).toEqual([]); // ...but did not replace display - }); - - it("resourcesListChanged does NOT light the indicator when the list is unchanged", async () => { - client.setStatus("connected"); - client.queueResourcePages({ resources: [resource("a://1")] }); - await state.refresh(); - expect(state.getResources().map((r) => r.uri)).toEqual(["a://1"]); - - let fired = false; - state.addEventListener("listChangedChange", () => { - fired = true; - }); - client.queueResourcePages({ resources: [resource("a://1")] }); - client.dispatchTypedEvent("resourcesListChanged"); - await new Promise((r) => setTimeout(r, 0)); - expect(client.listResources).toHaveBeenCalledTimes(2); // refresh + peek - expect(fired).toBe(false); - expect(state.getListChanged()).toBe(false); + expect(await changed).toBe(true); + expect(client.listResources).not.toHaveBeenCalled(); // no automatic fetch + expect(state.getResources()).toEqual([]); // displayed list untouched }); it("resourcesListChanged auto-refreshes when the server opts in", async () => { diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index f52b658b0..afc946cab 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -141,80 +141,32 @@ describe("ManagedToolsState", () => { expect(next.map((t) => t.name)).toEqual(["a"]); }); - it("toolsListChanged peeks but does NOT replace the displayed list by default", async () => { - // Diff-aware (#1444): the notification fetches to compare, but the - // displayed list stays put until the user pulls via Refresh. + it("toolsListChanged lights the indicator without fetching by default (#1444)", async () => { + // Auto-refresh off: a list_changed lights the indicator with NO list call; + // the user pulls the new list via Refresh. client.setStatus("connected"); - client.queueToolPages({ tools: [tool("a"), tool("b")] }); const changed = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); - expect(await changed).toBe(true); // the peeked list differs from [] - expect(client.listTools).toHaveBeenCalled(); // it fetched to compare - expect(state.getTools()).toEqual([]); // ...but did not replace the display + expect(await changed).toBe(true); + expect(client.listTools).not.toHaveBeenCalled(); // no automatic fetch + expect(state.getTools()).toEqual([]); // displayed list untouched }); - it("toolsListChanged does NOT light the indicator when the list is unchanged", async () => { - // The everything-server case: a list_changed that re-sends an identical - // list must not light the indicator. + it("debounces a burst of list_changed into a single indicator light, no fetch (#1444)", async () => { + // The everything-server case: a rapid burst collapses to one indicator + // light once it settles, and never fetches in flag-only mode. client.setStatus("connected"); - client.queueToolPages({ tools: [tool("a")] }); - await state.refresh(); - expect(state.getTools().map((t) => t.name)).toEqual(["a"]); - - let fired = false; + let fired = 0; state.addEventListener("listChangedChange", () => { - fired = true; + fired++; }); - // Re-send the same single-tool page on the next peek. - client.queueToolPages({ tools: [tool("a")] }); - client.dispatchTypedEvent("toolsListChanged"); - // Let the async peek (fetch + compare) settle. - await new Promise((r) => setTimeout(r, 0)); - expect(client.listTools).toHaveBeenCalledTimes(2); // refresh + peek - expect(fired).toBe(false); - expect(state.getListChanged()).toBe(false); - }); - - it("debounces a burst of list_changed into a single fetch (#1444)", async () => { - // The everything-server case: a rapid burst of notifications must collapse - // into one list call once it settles, not one per notification. - client.setStatus("connected"); - client.queueToolPages({ tools: [tool("a")] }); - const changed = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); client.dispatchTypedEvent("toolsListChanged"); client.dispatchTypedEvent("toolsListChanged"); - expect(await changed).toBe(true); - expect(client.listTools).toHaveBeenCalledTimes(1); // one debounced fetch - }); - - it("coalesces a peek that fires while an earlier one is still fetching (#1444)", async () => { - // Defense beyond the debounce: a post-debounce notification landing during - // an in-flight peek queues a single re-run instead of a concurrent fetch. - client.setStatus("connected"); - let release: (value: { tools: Tool[] }) => void = () => {}; - client.listTools.mockImplementationOnce( - () => - new Promise<{ tools: Tool[] }>((resolve) => { - release = resolve; - }), - ); - client.queueToolPages({ tools: [tool("a")] }); - - // First notification → debounced peek starts and hangs. - client.dispatchTypedEvent("toolsListChanged"); - await new Promise((r) => setTimeout(r, 0)); - expect(client.listTools).toHaveBeenCalledTimes(1); - - // Second notification while the peek is hung → coalesced, no concurrent fetch. - client.dispatchTypedEvent("toolsListChanged"); - await new Promise((r) => setTimeout(r, 0)); - expect(client.listTools).toHaveBeenCalledTimes(1); - - // Releasing the first peek runs exactly one coalesced re-run. - release({ tools: [tool("a")] }); await new Promise((r) => setTimeout(r, 0)); - expect(client.listTools).toHaveBeenCalledTimes(2); + expect(fired).toBe(1); // one debounced flip + expect(state.getListChanged()).toBe(true); + expect(client.listTools).not.toHaveBeenCalled(); }); it("coalesces an auto-refresh that fires while an earlier one is still fetching (#1444)", async () => { @@ -250,25 +202,6 @@ describe("ManagedToolsState", () => { expect(autoState.getTools().map((t) => t.name)).toEqual(["a"]); }); - it("clears the indicator when a later notification reverts the list to the displayed one (#1444)", async () => { - client.setStatus("connected"); - client.queueToolPages({ tools: [tool("a")] }); - await state.refresh(); // displayed = [a] - - // Server adds a tool → indicator lights. - client.queueToolPages({ tools: [tool("a"), tool("b")] }); - const lit = waitForListChanged(state); - client.dispatchTypedEvent("toolsListChanged"); - expect(await lit).toBe(true); - - // Server reverts to [a] (matches the displayed list) → indicator clears. - client.queueToolPages({ tools: [tool("a")] }); - const cleared = waitForListChanged(state); - client.dispatchTypedEvent("toolsListChanged"); - expect(await cleared).toBe(false); - expect(state.getListChanged()).toBe(false); - }); - it("toolsListChanged auto-refreshes when the server opts in", async () => { const autoClient = new FakeInspectorClient({ capabilities: { tools: {} }, diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index 32dd95831..6d89f0fdf 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -75,10 +75,11 @@ export abstract class ManagedListState< private listChanged = false; private readonly config: ManagedListConfig; // Debounce a burst of `list_changed` notifications into a single - // refresh/peek once it settles. + // refresh (or one indicator light) once it settles. private listChangedTimer: ReturnType | null = null; - // Second line of defense beyond the debounce: while a list-changed action is - // fetching, a new (post-debounce) notification queues a single re-run instead + // Second line of defense beyond the debounce, for the auto-refresh path: + // while a refresh is fetching, a new (post-debounce) notification queues a + // single re-run instead // of firing another concurrent paginated fetch. private running = false; private runQueued = false; @@ -131,11 +132,12 @@ export abstract class ManagedListState< } /** - * The debounced list-changed action, guarded against overlap: if one is - * already fetching, a new (post-debounce) notification queues a single re-run - * rather than launching a concurrent paginated fetch. This covers both the - * auto-refresh path (whose last-write-wins `applyItems` could otherwise let a - * slow older fetch clobber a newer list) and the peek path (#1444). + * The debounced list-changed action. With auto-refresh on, pull the new list + * (guarded against overlap so a slow older fetch can't clobber a newer list + * via last-write-wins `applyItems`). With auto-refresh off, lights the + * indicator without any network — the user pulls the new list via Refresh + * (#1402, #1444). Lists without an indicator (resource templates) do nothing + * in the off case. */ private runListChanged(): void { if (this.running) { @@ -155,7 +157,7 @@ export abstract class ManagedListState< if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { await this.refresh(); } else if (this.config.supportsIndicator) { - await this.peekForChange(); + this.setListChanged(true); } } while (this.runQueued); } finally { @@ -168,7 +170,7 @@ export abstract class ManagedListState< return [...this.items]; } - /** Whether a `list_changed` since the last refresh actually changed the list. */ + /** Whether a `list_changed` arrived since the last refresh (indicator on). */ getListChanged(): boolean { return this.listChanged; } @@ -267,14 +269,6 @@ export abstract class ManagedListState< * a reorder is a visible change the user would see on Refresh, so it counts * (#1444). */ - private async peekForChange(): Promise { - // Overlap is handled by the runListChanged guard, so this just fetches and - // diffs against the displayed list. - const next = await this.fetchAll(); - if (next === null) return; - this.setListChanged(JSON.stringify(next) !== JSON.stringify(this.items)); - } - destroy(): void { this.unsubscribe?.(); this.unsubscribe = null; diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index 79e5d7f43..75435a885 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -14,9 +14,9 @@ export interface ManagedPromptsStateEventMap { promptsChange: Prompt[]; /** * Fires when the "list changed since last refresh" flag flips. True when a - * `prompts/list_changed` brings a list that differs from what's displayed, - * false once the user refreshes, the change reverts, or the connection - * drops. Drives the sidebar list-changed indicator (#1402). + * `prompts/list_changed` arrives (auto-refresh off), false once the user + * refreshes or the connection drops. Drives the sidebar list-changed + * indicator (#1402). */ listChangedChange: boolean; } diff --git a/core/mcp/state/managedResourcesState.ts b/core/mcp/state/managedResourcesState.ts index c07e7a781..5387a8c6e 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -14,9 +14,9 @@ export interface ManagedResourcesStateEventMap { resourcesChange: Resource[]; /** * Fires when the "list changed since last refresh" flag flips. True when a - * `resources/list_changed` brings a list that differs from what's displayed, - * false once the user refreshes, the change reverts, or the connection - * drops. Drives the list-changed indicator (#1402). + * `resources/list_changed` arrives (auto-refresh off), false once the user + * refreshes or the connection drops. Drives the list-changed indicator + * (#1402). */ listChangedChange: boolean; } diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index aeb8e2914..bafb32369 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -14,9 +14,9 @@ export interface ManagedToolsStateEventMap { toolsChange: Tool[]; /** * Fires when the "list changed since last refresh" flag flips. True when a - * `tools/list_changed` brings a list that differs from what's displayed, - * false once the user refreshes, the change reverts, or the connection - * drops. Drives the sidebar list-changed indicator (#1402). + * `tools/list_changed` arrives (auto-refresh off), false once the user + * refreshes or the connection drops. Drives the sidebar list-changed + * indicator (#1402). */ listChangedChange: boolean; } From 14e14aa922d4002a16a615cbdb9f396a567b7bac Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 17:14:26 -0400 Subject: [PATCH 8/9] docs(core): scrub stale peek docstrings after the blind-indicator revert (#1445 review) The diff-aware revert removed peekForChange's body but left its docstring, which orphaned onto destroy() and described fetch-to-compare/clear-on-revert behavior that no longer exists. Replace it with a one-line destroy() doc, and update clearListChanged's doc (peek -> blind light). Co-Authored-By: Claude Opus 4.8 (1M context) --- core/mcp/state/managedListState.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index 6d89f0fdf..514681ab8 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -177,8 +177,8 @@ export abstract class ManagedListState< /** * Clear the list-changed flag — called when the user refreshes the list. The - * peek/auto-refresh on the notification leaves the indicator set until the - * user acknowledges by pulling. + * blind light on the notification leaves the indicator set until the user + * acknowledges by pulling. */ clearListChanged(): void { this.setListChanged(false); @@ -259,16 +259,7 @@ export abstract class ManagedListState< this.emit(this.config.changeEvent, this.getItems()); } - /** - * Fetch on `list_changed` and track whether the server's list differs from - * what's displayed. The displayed list is left untouched — the user still - * pulls the new one via Refresh (pull-on-demand). Many servers re-send an - * identical list on `list_changed`; this keeps the indicator dark in that - * case, and also clears it if a later notification reverts the server back - * to the displayed list (nothing left to pull). The flag is order-sensitive: - * a reorder is a visible change the user would see on Refresh, so it counts - * (#1444). - */ + /** Unsubscribe from the client and drop the list; idempotent. */ destroy(): void { this.unsubscribe?.(); this.unsubscribe = null; From 79026a5331b348c5d8983b3f1f7d1adeb3f3c312 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 8 Jun 2026 17:20:44 -0400 Subject: [PATCH 9/9] docs(core): finish scrubbing stale peek references after the revert (#1445 review) Round-7's scrub missed four more peek/fetch-to-compare mentions in managedListState.ts (supportsIndicator, fetchAll, the debounce-default and onListChanged comments) plus six 'async peek' comments in the state tests. Update them all to the blind-indicator semantics. Doc/comment-only; no behavior change. grep -i peek over core/mcp/state now returns nothing. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/mcp/state/managedPromptsState.test.ts | 4 ++-- .../mcp/state/managedResourcesState.test.ts | 4 ++-- .../core/mcp/state/managedToolsState.test.ts | 4 ++-- core/mcp/state/managedListState.ts | 18 ++++++++++-------- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts index aa5c397e8..d4f597d98 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -232,7 +232,7 @@ describe("ManagedPromptsState", () => { client.queuePromptPages({ prompts: [prompt("a")] }); const set = waitForListChanged(state); client.dispatchTypedEvent("promptsListChanged"); - await set; // wait for the async peek to set the flag + await set; // wait for the debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -255,7 +255,7 @@ describe("ManagedPromptsState", () => { client.queuePromptPages({ prompts: [prompt("a")] }); const set = waitForListChanged(state); client.dispatchTypedEvent("promptsListChanged"); - await set; // wait for the async peek to set the flag + await set; // wait for the debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); diff --git a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts index 5a855fa1f..3a6119a87 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -239,7 +239,7 @@ describe("ManagedResourcesState", () => { client.queueResourcePages({ resources: [resource("a://1")] }); const set = waitForListChanged(state); client.dispatchTypedEvent("resourcesListChanged"); - await set; // wait for the async peek to set the flag + await set; // wait for the debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -262,7 +262,7 @@ describe("ManagedResourcesState", () => { client.queueResourcePages({ resources: [resource("a://1")] }); const set = waitForListChanged(state); client.dispatchTypedEvent("resourcesListChanged"); - await set; // wait for the async peek to set the flag + await set; // wait for the debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index afc946cab..73b5bed27 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -295,7 +295,7 @@ describe("ManagedToolsState", () => { client.queueToolPages({ tools: [tool("a")] }); const set = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); - await set; // wait for the async peek to set the flag + await set; // wait for the debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -318,7 +318,7 @@ describe("ManagedToolsState", () => { client.queueToolPages({ tools: [tool("a")] }); const set = waitForListChanged(state); client.dispatchTypedEvent("toolsListChanged"); - await set; // wait for the async peek to set the flag + await set; // wait for the debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index 514681ab8..4faa4ed96 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -21,8 +21,9 @@ const MAX_PAGES = 100; /** * Default delay (ms) for debouncing `list_changed` notifications. Servers * (e.g. the everything server) can emit a rapid burst; debouncing collapses the - * burst into a single refresh/peek once it settles instead of one list call per - * notification (#1444). + * burst into a single action once it settles (one indicator light when + * auto-refresh is off, or one fetch when on) instead of one per notification + * (#1444). */ export const DEFAULT_LIST_CHANGED_DEBOUNCE_MS = 250; @@ -54,8 +55,8 @@ export interface ManagedListConfig { ) => Promise>; /** * Whether this list drives a list-changed indicator. When true, a - * `list_changed` in non-auto-refresh mode peeks-and-diffs to light the - * indicator; when false (resource templates, which have no indicator of + * `list_changed` in non-auto-refresh mode lights the indicator blindly (no + * list call); when false (resource templates, which have no indicator of * their own), a `list_changed` in non-auto mode does nothing — the list is * pulled via the screen's Refresh instead. */ @@ -95,8 +96,9 @@ export abstract class ManagedListState< void this.refresh(); }; const onListChanged = (): void => { - // Debounce: collapse a burst of notifications into one refresh/peek once - // it settles, instead of one list call per notification (#1444). + // Debounce: collapse a burst of notifications into one settled action + // (indicator light when off, fetch when on) instead of one per + // notification (#1444). if (this.listChangedTimer !== null) clearTimeout(this.listChangedTimer); this.listChangedTimer = setTimeout(() => { this.listChangedTimer = null; @@ -215,8 +217,8 @@ export abstract class ManagedListState< } /** - * Fetch all pages without mutating state or dispatching — used by both - * refresh (apply) and peek (compare). Returns `null` when not connected, or + * Fetch all pages, then `applyItems` commits them (see `refresh`). Returns + * `null` when not connected, or * `[]` when the server doesn't advertise the gating capability (calling the * list method there returns -32601 "Method not found", which would spam the * console; empty list is the right semantics).