From 347173fab7363fa25f0a50b1b4b246c72028757f Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Mon, 10 Aug 2026 18:19:33 -0400
Subject: [PATCH] fix: absorb only the teardown's own close, not any
 ConnectionClosed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review feedback (round 3). The predicate absorbed *any* CONNECTION_CLOSED,
while the doc comment claimed it absorbed only the teardown's — so a
transport dropping on its own after the progress notifications but before
`disconnect()` would have been swallowed. These tests assert only on the
notifications, so that regression would have passed green.

Gate the absorption on a teardown flag owned by the helper, which now
returns an `InFlightCall` exposing `disconnectAndSettle()`. Keeping the flag
inside means it can't be raised too early (which would silently reopen the
hole) and the settle can't be forgotten; call sites also collapse from two
statements to one.

Verified: a pre-teardown ConnectionClosed now fails the test and stays
observed (no run-level error), while a teardown-time one is still absorbed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
---
 .../integration/mcp/inspectorClient.test.ts   | 44 +++++++++++++------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts
index 9a3a690c..cc208835 100644
--- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts
+++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts
@@ -106,21 +106,33 @@ async function getTool(client: InspectorClient, name: string): Promise<Tool> {
  * error and fails `npm run ci` even though every test passes (#1947).
  *
  * Attach the handler at call time (not after the assertions) so there is no
- * window in which the rejection can escape, and `await` the returned promise
- * after `disconnect()` so teardown stays ordered.
+ * window in which the rejection can escape, then finish through
+ * `disconnectAndSettle()`, which tears down and awaits the call in one step.
  *
- * Only the teardown's own `CONNECTION_CLOSED` rejection is absorbed. Plain
+ * Only a `CONNECTION_CLOSED` raised *by that teardown* is absorbed. Plain
  * fulfillment is fine too — whether the call beats the teardown is a race, so
- * asserting either outcome would turn this straight back into a flake. Any
- * *other* rejection is re-thrown, so a call that fails for a real reason still
- * fails the test instead of passing on the strength of the progress
- * notifications it managed to emit first.
+ * asserting either outcome would turn this straight back into a flake. Every
+ * other rejection is re-thrown, including a `CONNECTION_CLOSED` that arrives
+ * before teardown begins: a transport that drops on its own after emitting the
+ * progress notifications is a real regression, and absorbing it would let these
+ * tests pass on the strength of the notifications alone.
+ *
+ * The teardown flag is owned by the helper and set inside `disconnectAndSettle`
+ * rather than by the caller, so the flag cannot be raised too early (which would
+ * reopen the hole) and the await cannot be forgotten.
  */
-function settleInFlight(call: Promise<unknown>): Promise<void> {
+interface InFlightCall {
+  /** Disconnect, then await the call — absorbing only this teardown's close. */
+  disconnectAndSettle(client: InspectorClient): Promise<void>;
+}
+
+function settleInFlight(call: Promise<unknown>): InFlightCall {
+  let tearingDown = false;
   const settled = call.then(
     () => undefined,
     (error: unknown) => {
       if (
+        tearingDown &&
         error instanceof SdkError &&
         error.code === SdkErrorCode.ConnectionClosed
       ) {
@@ -135,9 +147,15 @@ function settleInFlight(call: Promise<unknown>): Promise<void> {
   // notifications would sit unobserved for seconds and be reported as an
   // unhandled rejection: precisely the failure this helper exists to prevent.
   // Observe it the moment it exists. This does not swallow anything — `settled`
-  // stays rejected, so the caller's `await` still fails the test.
+  // stays rejected, so the caller's `await` below still fails the test.
   settled.catch(() => undefined);
-  return settled;
+  return {
+    async disconnectAndSettle(client: InspectorClient): Promise<void> {
+      tearingDown = true;
+      await client.disconnect();
+      await settled;
+    },
+  };
 }
 
 /** Get all resources from the client via listResources() (paginates if needed). */
@@ -2308,8 +2326,7 @@ describe("InspectorClient", () => {
         progressToken: progressToken.toString(),
       });
 
-      await client!.disconnect();
-      await inFlight;
+      await inFlight.disconnectAndSettle(client!);
       await server.stop();
     });
 
@@ -2429,8 +2446,7 @@ describe("InspectorClient", () => {
       });
       expect((progressEvents[1] as { total?: number }).total).toBeUndefined();
 
-      await client!.disconnect();
-      await inFlight;
+      await inFlight.disconnectAndSettle(client!);
       await server.stop();
     });