modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix: dispatch the connect event after the listen stream is up

Browse source 
Copilot review round 2: opening the stream after `dispatchTypedEvent("connect")`
left a real gap. The managed list states start their initial `refresh()` from
that event, so `tools/list` could go out ahead of `subscriptions/listen` — and a
list the server changed in that window would notify nobody, leaving the UI
stale with no way to notice.

The handler registration, the initial logging level, and the stream open now all
run before the `connect` dispatch, so no consumer can act on the connection
until the notification channel is established (or its retry armed). Costs one
listen round-trip on a modern connect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
This commit is contained in:
cliffhall 2026-08-10 23:49:20 -04:00
commit d08a53027a
2 changed files with 33 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

24
clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts
View file

@ -373,6 +373,30 @@ describe("resource subscriptions era fork (#1630)", () => {
);
});
it("has the stream up before the connect event fires", async () => {
// The managed list states start their initial `refresh()` from the
// `connect` event, so the stream has to be established (or its retry
// armed) first: dispatching earlier would let `tools/list` go out ahead of
// `subscriptions/listen`, and a list the server changed in that window
// would notify nobody.
const started = await startToolsOnlyServer({ tools: true });
const connected = new InspectorClient(
{ type: "streamable-http", url: started.url },
{
environment: { transport: createTransportNode },
versionNegotiation: eraToVersionNegotiation("modern"),
},
);
let streamAtConnectEvent: boolean | undefined;
connected.addEventListener("connect", () => {
streamAtConnectEvent = internals(connected).modernSubscription !== null;
});
await connected.connect();
client = connected;
expect(streamAtConnectEvent).toBe(true);
});
it("connects anyway when the connect-time listen fails, and retries", async () => {
// The handshake succeeded; every request-scoped feature works without the
// stream, so the failure is handed to the reconnect machinery instead of

11
core/mcp/inspectorClient.ts
View file

@ -1936,8 +1936,6 @@ export class InspectorClient extends InspectorClientEventTarget {
// 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(
@ -2067,6 +2065,15 @@ export class InspectorClient extends InspectorClientEventTarget {
// server with no resources would have no way in, since a subscribe click
// was the only thing that ever opened the stream (#1920).
await this.openModernListenStreamOnConnect();
// Last, so the notification channel is established (or its retry armed)
// before any consumer acts on the connection. The managed list states
// start their initial `refresh()` from this event, so dispatching earlier
// would let `tools/list` go out ahead of `subscriptions/listen` — and a
// list the server changes in that window would notify nobody, leaving the
// UI stale with no way to notice (#1920). The cost is one listen
// round-trip added to connect on the modern era.
this.dispatchTypedEvent("connect");
} catch (error) {
if (!isConnectAuthRecoveryError(error)) {
this.status = "error";