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

test(fix): probe a single-source route when waiting for the dead transport

Browse source 
Review feedback on #1987: `waitForDeadTransport` polled `/api/mcp/send`,
which answers `transport_error` from two branches — the `isTransportDead()`
short-circuit and the `catch` around a rejected `send()`. Only the first
implies the transport is marked dead, so the stop condition could not tell
them apart from the response alone.

That was safe in practice: `StdioClientTransport.send` rejects only when
`_process` is undefined, and the transport clears `_process` and calls
`onclose` in one synchronous block, so the `catch` branch is not observable
before `markTransportDead()` has run (confirmed empirically — 0/25 probes
returned early). But the safety rested on an SDK-internal ordering this test
neither states nor controls, and if it changed the send test would silently
cover the `catch` branch instead of the short-circuit it names — a regression
with no failing run attached to it.

Poll `/api/mcp/auth-state` instead. For a session connected without an
authState there is no auth provider, so `setAuthState` always throws and
`transport_error` there has exactly one source: the dead-transport
short-circuit. The route never awaits a transport response, so it cannot
hang either. Same cost, exact condition, no dependence on SDK internals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLYmVPVLdeYpE6aMTUsCks
This commit is contained in:
cliffhall 2026-08-12 01:10:03 -04:00
commit b70aed6914
1 changed files with 28 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -79,17 +79,31 @@ async function startUnauthorizedUpstream(): Promise<{
/**
* 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.
* The probe is `/api/mcp/auth-state`, chosen because for a session connected
* *without* an authState it answers `kind: "transport_error"` from exactly one
* place — the `isTransportDead()` short-circuit. Every other outcome is a 400
* from `setAuthState` ("Session has no OAuth auth provider"), so a
* `transport_error` here is proof that `onclose` has already run
* `markTransportDead()`, not merely that some write failed.
*
* 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.
* That single source is the point. `/api/mcp/send` looks like the obvious probe
* but answers `transport_error` from *two* branches: the dead-transport
* short-circuit, and the `catch` around a `send()` that rejected. Only the first
* implies the transport is marked dead, so a stop condition keyed on the
* response cannot tell them apart. It happens to be safe today —
* `StdioClientTransport.send` rejects only when `_process` is undefined, and the
* transport clears `_process` and calls `onclose` in one synchronous block, so
* the `catch` branch cannot be observed before `markTransportDead()` has run —
* but that is an SDK-internal ordering detail, not something this test states or
* controls. If it ever changed, an early return here would hand each call site a
* live session: the send test would quietly exercise the `catch` branch instead
* of the short-circuit it names, and the auth-state test would fall through to
* `setAuthState` and fail on a 400. Probing a single-source route costs nothing
* and does not depend on the ordering holding.
*
* The route also cannot hang: it never awaits a response from the transport, so
* unlike a *request* through `/api/mcp/send` there is nothing here to wait
* forever on a reply the dead child will never send.
*
* Deliberately not `/api/mcp/events`: opening that stream on a dead transport
* calls `sessions.delete(sessionId)`, so the send under test would then answer
@ -103,12 +117,14 @@ async function waitForDeadTransport(
const deadline = Date.now() + timeoutMs;
let last = "(no response)";
while (Date.now() < deadline) {
const res = await fetch(`${h.baseUrl}/api/mcp/send`, {
const res = await fetch(`${h.baseUrl}/api/mcp/auth-state`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
sessionId,
message: { jsonrpc: "2.0", method: "notifications/initialized" },
authState: {
oauthTokens: { access_token: "probe", token_type: "Bearer" },
},
}),
});
const body = (await res.json()) as { kind?: string };