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..d4f597d98 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; @@ -35,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", () => { @@ -61,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([]); @@ -73,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. @@ -135,15 +143,15 @@ 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 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"); - // 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); + 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 () => { @@ -152,7 +160,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"); @@ -206,14 +214,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 +230,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 debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -251,7 +253,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 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/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 c4c8db3e1..3a6119a87 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; @@ -37,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", () => { @@ -64,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([]); @@ -78,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. @@ -142,17 +150,15 @@ 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 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"); - // 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); + 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 () => { @@ -161,7 +167,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"); @@ -215,14 +221,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 +237,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 debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -260,7 +260,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 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 bf839861d..73b5bed27 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; @@ -35,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", () => { @@ -61,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([]); @@ -73,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. @@ -133,15 +141,65 @@ 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 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"); - // Yield so a stray refresh would have landed. - await Promise.resolve(); - await Promise.resolve(); + expect(await changed).toBe(true); + expect(client.listTools).not.toHaveBeenCalled(); // no automatic fetch + expect(state.getTools()).toEqual([]); // displayed list untouched + }); + + 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"); + let fired = 0; + state.addEventListener("listChangedChange", () => { + fired++; + }); + client.dispatchTypedEvent("toolsListChanged"); + client.dispatchTypedEvent("toolsListChanged"); + client.dispatchTypedEvent("toolsListChanged"); + await new Promise((r) => setTimeout(r, 0)); + expect(fired).toBe(1); // one debounced flip + expect(state.getListChanged()).toBe(true); expect(client.listTools).not.toHaveBeenCalled(); - expect(state.getTools()).toEqual([]); + }); + + 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("toolsListChanged auto-refreshes when the server opts in", async () => { @@ -150,7 +208,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"); @@ -158,6 +216,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 +277,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 +293,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 debounced notification to set the flag expect(state.getListChanged()).toBe(true); const changed = waitForListChanged(state); @@ -252,7 +316,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 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/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/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/managedListState.ts b/core/mcp/state/managedListState.ts new file mode 100644 index 000000000..4faa4ed96 --- /dev/null +++ b/core/mcp/state/managedListState.ts @@ -0,0 +1,270 @@ +/** + * 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; + +/** + * 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 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; + +/** 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 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. + */ + supportsIndicator: boolean; + /** Debounce delay (ms) for `list_changed` bursts. */ + debounceMs: number; +} + +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; + // Debounce a burst of `list_changed` notifications into a single + // refresh (or one indicator light) once it settles. + private listChangedTimer: ReturnType | null = null; + // 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; + + constructor( + client: InspectorClientProtocol, + config: ManagedListConfig, + ) { + super(); + this.client = client; + this.config = config; + const onConnect = (): void => { + void this.refresh(); + }; + const onListChanged = (): void => { + // 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; + 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); + } + }; + this.client.addEventListener("connect", onConnect); + 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); + this.client.removeEventListener("statusChange", onStatusChange); + } + this.client = null; + }; + } + + /** + * 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) { + 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) { + this.setListChanged(true); + } + } while (this.runQueued); + } finally { + this.running = false; + } + } + + /** Defensive copy of the current list. */ + protected getItems(): T[] { + return [...this.items]; + } + + /** Whether a `list_changed` arrived since the last refresh (indicator on). */ + getListChanged(): boolean { + return this.listChanged; + } + + /** + * Clear the list-changed flag — called when the user refreshes the list. The + * blind light 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, 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). + */ + 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()); + } + + /** Unsubscribe from the client and drop the list; idempotent. */ + destroy(): void { + this.unsubscribe?.(); + this.unsubscribe = null; + this.items = []; + } +} diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index 21dfd9179..75435a885 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -1,143 +1,49 @@ /** - * 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, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} 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` arrives (auto-refresh off), false once the user + * refreshes 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; - - 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 just flag the change for the indicator - // and let the user pull via Refresh, which is what makes the indicator - // meaningful (#1402). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else { - this.setListChanged(true); - } - }; - 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; - }; +export class ManagedPromptsState extends ManagedListState< + Prompt, + ManagedPromptsStateEventMap +> { + 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 }; + }, + }); } 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 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(); - } - const effectiveMetadata = metadata ?? this._metadata; - this.prompts = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listPrompts(cursor, effectiveMetadata); - this.prompts = cursor ? [...this.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); - this.dispatchTypedEvent("promptsChange", this.prompts); - return this.getPrompts(); - } - - 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..fd4d22e8f 100644 --- a/core/mcp/state/managedResourceTemplatesState.ts +++ b/core/mcp/state/managedResourceTemplatesState.ts @@ -1,133 +1,58 @@ /** - * 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, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} 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; - - 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; - }; +export class ManagedResourceTemplatesState extends ManagedListState< + ResourceTemplate, + ManagedResourceTemplatesStateEventMap +> { + constructor( + client: InspectorClientProtocol, + debounceMs = DEFAULT_LIST_CHANGED_DEBOUNCE_MS, + ) { + super(client, { + changeEvent: "resourceTemplatesChange", + listChangedEvent: "resourceTemplatesListChanged", + // Templates are gated on the broader `resources` capability. + capabilityKey: "resources", + itemLabel: "resource templates", + supportsIndicator: false, + debounceMs, + 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 e8118022f..5387a8c6e 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -1,149 +1,49 @@ /** - * 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, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} 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 + * `resources/list_changed` arrives (auto-refresh off), false once the user * refreshes 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; - - 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 just flag the change for the indicator - // and let the user pull via Refresh, which is what makes the indicator - // meaningful (#1402). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else { - this.setListChanged(true); - } - }; - 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; - }; +export class ManagedResourcesState extends ManagedListState< + Resource, + ManagedResourcesStateEventMap +> { + 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 }; + }, + }); } 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 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(); - } - const effectiveMetadata = metadata ?? this._metadata; - this.resources = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listResources(cursor, effectiveMetadata); - this.resources = cursor - ? [...this.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); - this.dispatchTypedEvent("resourcesChange", this.resources); - return this.getResources(); - } - - 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 5490004f7..bafb32369 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -1,140 +1,49 @@ /** - * 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, + DEFAULT_LIST_CHANGED_DEBOUNCE_MS, +} 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` arrives (auto-refresh off), false once the user + * refreshes 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; - - 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 just flag the change for the indicator - // and let the user pull via Refresh, which is what makes the indicator - // meaningful (#1402). - if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { - void this.refresh(); - } else { - this.setListChanged(true); - } - }; - 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; - }; +export class ManagedToolsState extends ManagedListState< + Tool, + ManagedToolsStateEventMap +> { + 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 }; + }, + }); } 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 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(); - } - const effectiveMetadata = metadata ?? this._metadata; - this.tools = []; - let cursor: string | undefined; - let pageCount = 0; - do { - const result = await client.listTools(cursor, effectiveMetadata); - this.tools = cursor ? [...this.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); - this.dispatchTypedEvent("toolsChange", this.tools); - return this.getTools(); - } - - destroy(): void { - this.unsubscribe?.(); - this.unsubscribe = null; - this.tools = []; + return this.getItems(); } }