From 2e77e6fe5f75f77e3f7f8686607d02672f4aafb4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 1 Jun 2026 10:49:35 -0400 Subject: [PATCH 1/2] fix(state): dispatch connect event after capabilities are populated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list-state managers (tools/prompts/resources/templates) refresh on the "connect" event and, since #1395, gate their list RPC on getCapabilities(). But connect() dispatched "connect" before fetchServerInfo() populated this.capabilities, and refresh() runs synchronously up to its first await — so the capability gate executed during the synchronous dispatch, read undefined capabilities (reset to undefined on every disconnect), tripped, and wiped every list to empty on connect. Tools, prompts, resources, and resource templates all vanished against any server. Move the "connect" dispatch to after the awaited fetchServerInfo() so capabilities are in place before any manager refreshes. The unit tests missed this because their FakeInspectorClient has capabilities populated from construction; added an integration regression test wiring a real ManagedToolsState on connect and asserting the tool list is non-empty (fails without the fix). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/mcp/inspectorClient.test.ts | 27 +++++++++++++++++++ core/mcp/inspectorClient.ts | 9 +++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index fa97142ce..e3d78ab94 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -10,6 +10,7 @@ import { PagedPromptsState, ManagedResourcesState, ManagedPromptsState, + ManagedToolsState, } from "@inspector/core/mcp/state/index.js"; import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; import { SamplingCreateMessage } from "@inspector/core/mcp/samplingCreateMessage.js"; @@ -687,6 +688,32 @@ describe("InspectorClient", () => { // Client no longer stores tools; listTools() still returns server tools when called expect((await client.listTools()).tools.length).toBeGreaterThan(0); }); + + it("ManagedToolsState populates on connect (regression: capability gate must see capabilities before the connect event)", async () => { + // Regression for #1395 + connect-ordering: the managed list-state managers + // refresh on the "connect" event and gate their list RPC on + // getCapabilities(). If "connect" is dispatched before fetchServerInfo() + // populates capabilities, the synchronous gate reads undefined and wipes + // the list to empty — tools/prompts/resources all vanish on every connect. + client = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + const toolsState = new ManagedToolsState(client); + + await client.connect(); + // Settle: the connect-triggered refresh() is async past its capability gate. + await new Promise((r) => setTimeout(r, 100)); + + expect(client.getCapabilities()?.tools).toBeDefined(); + expect(toolsState.getTools().length).toBeGreaterThan(0); + + toolsState.destroy(); + }); }); describe("Tool Methods", () => { diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index d68b2a6df..8775644a0 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -656,11 +656,16 @@ export class InspectorClient extends InspectorClientEventTarget { } this.status = "connected"; this.dispatchTypedEvent("statusChange", this.status); - this.dispatchTypedEvent("connect"); - // Always fetch server info (capabilities, serverInfo, instructions) - this is just cached data from initialize + // Always fetch server info (capabilities, serverInfo, instructions) - this is just cached data from initialize. + // Must run BEFORE the "connect" event: the managed list-state managers + // refresh on "connect" and gate their list RPC on getCapabilities() (see + // #1395). If "connect" fired first, that gate would read undefined + // capabilities and wipe tools/prompts/resources to empty on every connect. await this.fetchServerInfo(); + this.dispatchTypedEvent("connect"); + // Set initial logging level if configured and server supports it if (this.initialLoggingLevel && this.capabilities?.logging) { await this.client.setLoggingLevel( From 1ba24b5b045fdb8547471c5536c179b708237115 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 1 Jun 2026 11:00:06 -0400 Subject: [PATCH 2/2] test(integration): harden connect-race regression test per review Address the two non-blocking suggestions from the PR review: - Replace the 100ms settle sleep with waitForEvent() on the managers' change events, so the test is deterministic against slow CI instead of racing a fixed timeout. - Assert a second affected primitive (ManagedResourcesState alongside ManagedToolsState). The fix is a single shared line, but covering two distinct capabilities guards against a future change gating one primitive differently. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/integration/mcp/inspectorClient.test.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index e3d78ab94..63adc5a3e 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -689,12 +689,15 @@ describe("InspectorClient", () => { expect((await client.listTools()).tools.length).toBeGreaterThan(0); }); - it("ManagedToolsState populates on connect (regression: capability gate must see capabilities before the connect event)", async () => { + it("managed list states populate on connect (regression: capability gate must see capabilities before the connect event)", async () => { // Regression for #1395 + connect-ordering: the managed list-state managers // refresh on the "connect" event and gate their list RPC on // getCapabilities(). If "connect" is dispatched before fetchServerInfo() // populates capabilities, the synchronous gate reads undefined and wipes // the list to empty — tools/prompts/resources all vanish on every connect. + // The single fix is shared by all the managed states, so we cover tools + // and resources (two distinct capabilities) to guard against a future + // change gating only one primitive differently. client = new InspectorClient( { type: "stdio", @@ -704,15 +707,22 @@ describe("InspectorClient", () => { { environment: { transport: createTransportNode } }, ); const toolsState = new ManagedToolsState(client); + const resourcesState = new ManagedResourcesState(client); + // Await the change events the connect-triggered refresh() emits rather than + // a fixed sleep — refresh() is async past its synchronous capability gate. + const toolsChanged = waitForEvent(toolsState, "toolsChange"); + const resourcesChanged = waitForEvent(resourcesState, "resourcesChange"); await client.connect(); - // Settle: the connect-triggered refresh() is async past its capability gate. - await new Promise((r) => setTimeout(r, 100)); + await Promise.all([toolsChanged, resourcesChanged]); expect(client.getCapabilities()?.tools).toBeDefined(); expect(toolsState.getTools().length).toBeGreaterThan(0); + expect(client.getCapabilities()?.resources).toBeDefined(); + expect(resourcesState.getResources().length).toBeGreaterThan(0); toolsState.destroy(); + resourcesState.destroy(); }); });