From 2a4e93c621ac58928a85811acff07acfe68f1f40 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Tue, 11 Aug 2026 00:05:56 -0400
Subject: [PATCH] fix: only announce the connection if it is still live after
 the stream open
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Copilot review round 4: every await before the `connect` dispatch is a window
for a `disconnect()` or a transport close to overtake the connect, and the
listen round-trip widened it. Announcing anyway restarts every managed list
refresh against a session being torn down or already dead.

The dispatch is now gated on `status === "connected" && !disconnecting` — the
second because an explicit teardown claims ownership before it awaits
`client.close()`, leaving the status untouched until that block finishes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
---
 .../inspectorClient-subscriptions-era.test.ts | 46 +++++++++++++++++++
 core/mcp/inspectorClient.ts                   | 12 ++++-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts
index 16289fc3..8c2a005a 100644
--- a/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts
+++ b/clients/web/src/test/integration/mcp/inspectorClient-subscriptions-era.test.ts
@@ -397,6 +397,52 @@ describe("resource subscriptions era fork (#1630)", () => {
       expect(streamAtConnectEvent).toBe(true);
     });
 
+    it("does not announce the connection when a disconnect overtakes the stream open", async () => {
+      // The listen round-trip widened the window between the handshake and the
+      // `connect` announcement, so a `disconnect()` can land inside it.
+      // Announcing anyway would restart every managed list refresh against a
+      // session being torn down.
+      const started = await startToolsOnlyServer({ tools: true });
+      const connected = new InspectorClient(
+        { type: "streamable-http", url: started.url },
+        {
+          environment: { transport: createTransportNode },
+          versionNegotiation: eraToVersionNegotiation("modern"),
+        },
+      );
+      client = connected;
+      let connectAnnounced = false;
+      connected.addEventListener("connect", () => {
+        connectAnnounced = true;
+      });
+
+      // Hold the stream open so the disconnect lands while it is in flight.
+      // `openStarted` is what makes the race deterministic: the stub is only
+      // reached after the handshake, so disconnecting before it runs would leave
+      // `releaseOpen` unset and hang the connect.
+      const int = internals(connected);
+      let releaseOpen: () => void = () => {};
+      let markOpenStarted: () => void = () => {};
+      const openStarted = new Promise<void>((resolve) => {
+        markOpenStarted = resolve;
+      });
+      int.refreshModernSubscription = () => {
+        markOpenStarted();
+        return new Promise<void>((resolve) => {
+          releaseOpen = resolve;
+        });
+      };
+
+      const connecting = connected.connect();
+      await openStarted;
+      const disconnecting = connected.disconnect();
+      releaseOpen();
+      await connecting;
+      await disconnecting;
+
+      expect(connectAnnounced).toBe(false);
+    });
+
     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
diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts
index d36f7143..b0b809f3 100644
--- a/core/mcp/inspectorClient.ts
+++ b/core/mcp/inspectorClient.ts
@@ -2073,7 +2073,17 @@ export class InspectorClient extends InspectorClientEventTarget {
       // 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");
+      //
+      // …which is also why the announcement is conditional. Every await above
+      // is a window for a `disconnect()` or a transport `onclose`/`onerror` to
+      // overtake this connect, and the listen round-trip widened it. Announcing
+      // then would restart every managed list refresh against a session being
+      // torn down or already dead. `disconnecting` covers the teardown that has
+      // claimed ownership but is still awaiting `client.close()` — the status is
+      // whatever it was until that block finishes.
+      if (this.status === "connected" && !this.disconnecting) {
+        this.dispatchTypedEvent("connect");
+      }
     } catch (error) {
       if (!isConnectAuthRecoveryError(error)) {
         this.status = "error";