fix(test): wait for the dead transport instead of sleeping 300ms

connectDeadSession spawned a real node subprocess whose only job is to die,
then slept a flat 300ms for the death to propagate. That budget has to cover a
cold spawn(), node's startup, its exit, the parent observing the close, and
onclose marking the session dead — comfortable on an idle box, not under CI
contention or a machine running several agents.

When the budget was missed the session was still live, so isTransportDead() was
false and /api/mcp/send fell through to `await responseWait` for a tools/list
reply the dead child can never send. Nothing bounds that wait, so the test hung
to the project's full 30s ceiling. The timeout IS the symptom of losing the
race, which is why it never presented as a clean assertion failure.

Poll the observable condition instead. Two design points that are load-bearing:

  - The probe is a NOTIFICATION (method, no id). requestIdForSendWait returns
    undefined for it, so the route never awaits a response and cannot hang.
    Probing with a request would reproduce the exact bug being fixed.
  - Not /api/mcp/events, which looks like the natural "transport died" signal:
    opening that stream on a dead transport calls sessions.delete(sessionId),
    so the send under test would then answer 404 instead of transport_error.

Both terminal branches of the route answer transport_error — the
isTransportDead() short-circuit and the catch around a send() that threw on the
dead pipe — and either proves the subsequent real send cannot reach the hanging
path, so both are valid stop conditions.

Verified by reproducing the bug deterministically: with the wait reduced to 0
(the budget always missed) the test times out at 30s; with the poll it passes
10/10. Fixing the helper rather than the test also covers its second call site,
the auth-state test, which sits on the same race and had simply not lost it yet.

connect-crash.test.ts:147 is deliberately left alone. #1985 proposed giving it
the same treatment, but readSseEvents stops at transport_error, which arrives
whenever the child dies — so its 200ms sleep cannot hang, and changing it would
add risk for no gain.

Closes #1985

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>

clients/web/src/test/integration/mcp/remote/remote-auth-branches.test.ts

@@ -76,8 +76,63 @@ async function startUnauthorizedUpstream(): Promise<{
   return { url: `http://127.0.0.1:${port}/mcp`, server };
 }
 
-/** Connect a stdio session whose process crashes almost immediately, then
- * give the onclose handler time to mark the session's transport dead. */
+/**
+ * Poll until the server reports the session's transport as dead (#1985).
+ *
+ * The probe is a **notification** — a `method` with no `id` — which matters:
+ * `requestIdForSendWait` returns `undefined` for it, so `/api/mcp/send` never
+ * awaits a response and cannot hang. Probing with a *request* would reproduce
+ * the very failure this guards against, since a request sent before the
+ * transport is marked dead waits forever for a reply the dead child will never
+ * send.
+ *
+ * Either terminal branch of the route answers `kind: "transport_error"` — the
+ * `isTransportDead()` short-circuit, and the `catch` around a `send()` that
+ * threw on the dead pipe. Both are fine as a stop condition: each one proves the
+ * subsequent real send also cannot reach the hanging path.
+ *
+ * Deliberately not `/api/mcp/events`: opening that stream on a dead transport
+ * calls `sessions.delete(sessionId)`, so the send under test would then answer
+ * 404 instead of the `transport_error` it is asserting.
+ */
+async function waitForDeadTransport(
+  h: Harness,
+  sessionId: string,
+  timeoutMs = 10_000,
+): Promise<void> {
+  const deadline = Date.now() + timeoutMs;
+  let last = "(no response)";
+  while (Date.now() < deadline) {
+    const res = await fetch(`${h.baseUrl}/api/mcp/send`, {
+      method: "POST",
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify({
+        sessionId,
+        message: { jsonrpc: "2.0", method: "notifications/initialized" },
+      }),
+    });
+    const body = (await res.json()) as { kind?: string };
+    if (body.kind === "transport_error") return;
+    last = `HTTP ${res.status} ${JSON.stringify(body)}`;
+    await new Promise((resolve) => setTimeout(resolve, 20));
+  }
+  throw new Error(
+    `transport was never reported dead within ${timeoutMs}ms; last probe: ${last}`,
+  );
+}
+
+/**
+ * Connect a stdio session whose process crashes almost immediately, then wait
+ * until the transport is actually marked dead.
+ *
+ * This used to sleep a flat 300ms, which had to cover a cold `spawn()`, node's
+ * startup, its exit, the parent observing the close, and `onclose` marking the
+ * session dead. Comfortable on an idle machine; not under CI contention. When
+ * the budget was missed the session was still live, so the send under test was
+ * dispatched to a transport whose child was gone and hung for the project's full
+ * 30s timeout (#1985). Waiting on the observable condition costs latency on a
+ * slow machine instead of a false failure.
+ */
 async function connectDeadSession(h: Harness): Promise<string> {
   const config: MCPServerConfig = {
     type: "stdio",
@@ -87,9 +142,7 @@ async function connectDeadSession(h: Harness): Promise<string> {
   const res = await connect(h, config);
   expect(res.status).toBe(200);
   const { sessionId } = (await res.json()) as { sessionId: string };
-  // Give the subprocess time to exit and the transport's onclose handler
-  // to mark the session dead (mirrors connect-crash.test.ts's technique).
-  await new Promise((resolve) => setTimeout(resolve, 300));
+  await waitForDeadTransport(h, sessionId);
   return sessionId;
 }