Skip to content

Commit 50232f7

Browse files
fix(client): handle connection close during the version negotiation probe
A stdio child that exits on the unrecognized server/discover probe is a legacy signal, symmetric with the stdio probe-timeout rule: respawn it once with a fresh transport start() (the probe's close event already tore the connection down) and run the plain initialize handshake there. The respawn is gated on a close-provenance stamp (well-known symbol in a leaf module, implemented by StdioClientTransport): a transport the caller closed mid-probe, or one that does not report provenance, rejects with the typed connect error naming the close, and HTTP-class mid-probe closes keep rejecting typed. Transport hygiene: start() clears the read buffer and scopes child event handlers per life so a dead predecessor's deferred events cannot touch a successor; close() unpipes the child's stderr after teardown and always ends the aggregate stream (also for a never-started transport); a respawned life whose predecessor's stream was never read drops stderr instead of buffering toward backpressure.
1 parent f60dff0 commit 50232f7

14 files changed

Lines changed: 860 additions & 25 deletions

File tree

.changeset/probe-close-handling.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
---
4+
5+
Handle a connection that closes during the `server/discover` version-negotiation probe. On stdio, a child that exits on the unrecognized probe is a legacy signal (symmetric with the stdio probe-timeout rule): under `versionNegotiation: { mode: 'auto' }` the client now respawns it once via a fresh transport `start()` — the probe's close event already tore the connection down — and completes the plain `initialize` handshake there (the exit must close the child's stdio pipes to register as a close; an exit hidden behind a helper process holding them open falls to the probe-timeout path instead) — stdio servers built on SDKs that terminate on any pre-`initialize` request (the official Rust SDK, rmcp) previously hard-failed under `'auto'`. The respawn happens only for transports that positively report close provenance (an internal well-known-symbol stamp implemented by `StdioClientTransport`): a transport the caller closed mid-probe, or a custom transport that does not track provenance, rejects with the typed connect error instead — and on HTTP-class transports a mid-probe close keeps rejecting with the typed connect error. `StdioClientTransport` hygiene: `start()` clears the read buffer and scopes child event handlers per life, so a dead predecessor's deferred `close`/stdout events can no longer touch a successor after a `close()` → `start()` restart; `close()` detaches the child's stderr pipe and ends the aggregate stream once teardown completes — also for a never-started transport — so a dead predecessor cannot corrupt the next child's first frame, `stderr` readers of a closed transport always see `end`, and a restarted life begins on a fresh stream (re-read the `stderr` getter; a fresh stream nobody ever read drops output instead of buffering toward backpressure).

docs/migration/support-2026-07-28.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,17 @@ a dead HTTP server is never misreported as legacy. One browser-specific exceptio
8282
opaque CORS/preflight `TypeError` during the probe falls back to the legacy era, because
8383
deployed 2025 servers commonly have CORS allow-lists that predate the 2026 headers.
8484

85+
A mid-probe **connection close** follows the same transport split: on **stdio** a child
86+
that exits on the unrecognized probe is a legacy signal — the client respawns it once
87+
(transports that positively report close provenance only, as `StdioClientTransport`
88+
does; the exit must close the child's stdio pipes to register, else the probe-timeout
89+
path applies) and falls back to `initialize` on the fresh child; pre-connect `stderr`
90+
subscribers re-read that getter after `connect()`, and a pre-set `onclose` observer
91+
sees the probe child's close. On **HTTP** a mid-probe close rejects with the same typed
92+
`SdkError(EraNegotiationFailed)` as any probe transport failure. (The era-independent
93+
stderr end-of-stream change is described in the
94+
[stdio transport section of the v2 upgrade guide](./upgrade-to-v2.md#stdio-transport).)
95+
8596
```typescript
8697
versionNegotiation: {
8798
mode: 'auto',

docs/migration/upgrade-to-v2.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,12 @@ rewrite required unless noted.
15811581
pending — now reject the pending `send()` and close the transport through
15821582
`onerror`/`onclose` instead of surfacing an unhandled stream error; lifecycle
15831583
tests that pinned a crash-class exit observe a clean shutdown instead.
1584+
- With `stderr: 'pipe'`, `StdioClientTransport.close()` now ends the aggregate stderr
1585+
stream when its teardown completes (previously the stream ended only when the
1586+
child's own stderr closed — never, if that pipe outlived `close()`), and a
1587+
`start()` after `close()` begins a fresh stream — re-read the `stderr` getter, and
1588+
keep reading it: a fresh stream whose predecessor was being read buffers until
1589+
read and can eventually stall a chatty child on stderr backpressure.
15841590
15851591
#### Client list methods
15861592

docs/protocol-versions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ const cli = new Client(
101101

102102
A probe timeout is transport-aware. On stdio a silent server is a legacy server, so `connect()` falls back to `initialize` on the same stream; on HTTP silence is an outage, so `connect()` rejects with `SdkError(RequestTimeout)` instead of misreporting a dead server as legacy. One browser exception: an opaque CORS `TypeError` during the probe falls back to the legacy era, because deployed 2025 servers commonly have allow-lists that predate the 2026 headers.
103103

104+
A connection that closes mid-probe follows the same transport split. On stdio a child that exits on the unrecognized probe is a legacy server — stdio servers built on SDKs that terminate on any pre-`initialize` request (the official Rust SDK, rmcp) behave exactly this way — so under `'auto'` the client respawns the child once and falls back to `initialize` there (the exit must close the child's stdio pipes to register as a close — an exit hidden behind a helper process holding them open falls to the probe-timeout path instead): pre-connect `stderr` subscribers should re-read that getter after `connect()` resolves, and a pre-set `onclose` observer will see the probe child's close. The respawn is limited to transports that positively report close provenance (the SDK's own `StdioClientTransport` does): a transport the caller closed during the probe (closed via `transport.close()` — killing the child process directly is outside the transport's API and indistinguishable from a server-side exit), or a custom stdio-shaped transport that does not track it, is never respawned — and on HTTP a mid-probe close is ambiguous, never era evidence — `connect()` rejects with the same typed `SdkError(EraNegotiationFailed)` as any probe transport failure.
105+
104106
The client's `supportedProtocolVersions` option shapes the probe: its 2026+ entries are the versions the probe offers, and the legacy fallback stays available only while the list keeps a pre-2026 entry. A list with no pre-2026 entry removes the fallback — against a 2025-only server, `connect()` rejects with `SdkError(EraNegotiationFailed)`.
105107

106108
::: warning

examples/cli-client/host/host.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,11 +513,16 @@ export class McpHost {
513513
cwd: entry.cwd,
514514
stderr: 'pipe'
515515
});
516-
transport.stderr?.on('data', (chunk: Buffer) => {
516+
const logStderr = (chunk: Buffer) => {
517517
const line = String(chunk).trim();
518518
if (line) this.ui.serverLog(name, 'stderr', line);
519-
});
519+
};
520+
const preConnectStderr = transport.stderr;
521+
preConnectStderr?.on('data', logStderr);
520522
await client.connect(transport);
523+
// Under 'auto', a legacy fallback may have respawned the child with a
524+
// fresh stderr stream — re-read the getter and subscribe the new life.
525+
if (transport.stderr !== preConnectStderr) transport.stderr?.on('data', logStderr);
521526
}
522527

523528
try {
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* Close-provenance seam between the stdio transport and the negotiation
3+
* engine — a deliberate leaf module: both sides import it without coupling
4+
* the `/stdio` subpath's module graph to the negotiation engine (or vice
5+
* versa). Internal — nothing here is part of the public surface.
6+
*/
7+
import type { Transport } from '@modelcontextprotocol/core-internal';
8+
9+
/**
10+
* Close-provenance contract: a transport opts into the legacy-fallback respawn
11+
* by stamping `this[CLOSED_BY_CALLER] = false` in `start()` and `= true` at the
12+
* top of `close()` — negotiation respawns only on a `false` stamp; a `true`
13+
* stamp (local close) or an absent stamp (no provenance tracked) is never
14+
* respawned. `Symbol.for`: one shared registry key across bundled SDK copies,
15+
* no import needed to interoperate.
16+
*/
17+
export const CLOSED_BY_CALLER: unique symbol = Symbol.for('mcp.closedByCaller');
18+
19+
/** Typed read of the {@linkcode CLOSED_BY_CALLER} stamp — `undefined` when the transport does not implement the contract. */
20+
export function closeProvenance(transport: Transport): boolean | undefined {
21+
const value = (transport as { [CLOSED_BY_CALLER]?: unknown })[CLOSED_BY_CALLER];
22+
return typeof value === 'boolean' ? value : undefined;
23+
}

packages/client/src/client/probeClassifier.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ export type ProbeOutcome =
4848
| { kind: 'network-error'; error: unknown }
4949
/** The transport's auth flow challenged during the probe send (`UnauthorizedError`). */
5050
| { kind: 'auth-required'; error: Error }
51+
/** The transport reported close while the probe awaited its reply. */
52+
| { kind: 'closed' }
5153
/** No response arrived within the probe timeout. */
5254
| { kind: 'timeout'; timeoutMs: number };
5355

@@ -141,6 +143,18 @@ export function classifyProbeOutcome(outcome: ProbeOutcome, context: ProbeClassi
141143
// handshake an auth-gated modern server as legacy.
142144
return { kind: 'error', error: outcome.error };
143145
}
146+
case 'closed': {
147+
if (context.transportKind === 'stdio') {
148+
// A stdio child that exits on the unrecognized probe instead of
149+
// answering is a legacy signal — the same backward-compatibility
150+
// rule as the timeout row below (SDKs like the official Rust one
151+
// terminate the server on ANY pre-initialize request).
152+
return { kind: 'legacy' };
153+
}
154+
// On HTTP a mid-probe close is an ambiguous network condition — the
155+
// same typed connect error as any probe transport failure.
156+
return classifyNetworkError(new Error('Connection closed during the version negotiation probe'), context);
157+
}
144158
case 'timeout': {
145159
if (context.transportKind === 'stdio') {
146160
// Per the stdio transport's backward-compatibility rule, a probe
@@ -253,7 +267,8 @@ function isOpaqueFetchTypeError(error: unknown): boolean {
253267
return error instanceof TypeError || (error instanceof Error && error.name === 'TypeError');
254268
}
255269

256-
function describeError(error: unknown): string {
270+
/** Human-readable rendering of an unknown thrown value for probe diagnostics. */
271+
export function describeError(error: unknown): string {
257272
return error instanceof Error ? error.message : String(error);
258273
}
259274

packages/client/src/client/stdio.ts

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import type { JSONRPCMessage, Transport } from '@modelcontextprotocol/core-inter
77
import { ReadBuffer, SdkError, SdkErrorCode, serializeMessage } from '@modelcontextprotocol/core-internal';
88
import spawn from 'cross-spawn';
99

10+
import { CLOSED_BY_CALLER } from './closeProvenance';
11+
1012
export type StdioServerParameters = {
1113
/**
1214
* The executable to run to start the server.
@@ -126,8 +128,25 @@ export class StdioClientTransport implements Transport {
126128
);
127129
}
128130

131+
// Close-provenance stamp (see CLOSED_BY_CALLER): a fresh life has not been closed locally.
132+
(this as { [CLOSED_BY_CALLER]?: boolean })[CLOSED_BY_CALLER] = false;
133+
// A dead predecessor's late flush must not prepend to the new child's first frame.
134+
this._readBuffer.clear();
135+
if (this._stderrStream?.writableEnded) {
136+
// stderr: 'pipe' — the previous life's stream was ended (by its natural death, or by
137+
// close() completing teardown): this life pipes into a fresh one. Re-read the getter.
138+
const prior = this._stderrStream;
139+
this._stderrStream = new PassThrough();
140+
if (prior.readableFlowing === null && prior.listenerCount('data') === 0 && prior.listenerCount('readable') === 0) {
141+
// Nobody ever read the prior life's stream: keep this one flowing-and-dropping so
142+
// an unobserved chatty child cannot stall on stderr backpressure. A consumer who
143+
// WAS reading re-reads the getter — its fresh stream buffers until then.
144+
this._stderrStream.resume();
145+
}
146+
}
147+
129148
return new Promise((resolve, reject) => {
130-
this._process = spawn(this._serverParams.command, this._serverParams.args ?? [], {
149+
const proc = spawn(this._serverParams.command, this._serverParams.args ?? [], {
131150
// merge default env with server env because mcp server needs some env vars
132151
env: {
133152
...getDefaultEnvironment(),
@@ -138,26 +157,37 @@ export class StdioClientTransport implements Transport {
138157
windowsHide: process.platform === 'win32',
139158
cwd: this._serverParams.cwd
140159
});
141-
142-
this._process.on('error', error => {
160+
this._process = proc;
161+
// Per-life event scope: Node defers a child's 'close' event until its
162+
// stdio pipes close, so a dead predecessor's events can land after a
163+
// close() -> start() restart. Each handler bails when a SUCCESSOR
164+
// occupies the slot; an empty slot (mid-close()) still delivers — that
165+
// is the ordinary teardown notification.
166+
const usurped = () => this._process !== undefined && this._process !== proc;
167+
168+
proc.on('error', error => {
143169
reject(error);
170+
if (usurped()) return;
144171
this.onerror?.(error);
145172
});
146173

147-
this._process.on('spawn', () => {
174+
proc.on('spawn', () => {
148175
resolve();
149176
});
150177

151-
this._process.on('close', _code => {
178+
proc.on('close', _code => {
179+
if (usurped()) return;
152180
this._process = undefined;
153181
this.onclose?.();
154182
});
155183

156-
this._process.stdin?.on('error', error => {
184+
proc.stdin?.on('error', error => {
185+
if (usurped()) return;
157186
this.onerror?.(error);
158187
});
159188

160-
this._process.stdout?.on('data', chunk => {
189+
proc.stdout?.on('data', chunk => {
190+
if (usurped()) return;
161191
try {
162192
this._readBuffer.append(chunk);
163193
this.processReadBuffer();
@@ -167,12 +197,13 @@ export class StdioClientTransport implements Transport {
167197
}
168198
});
169199

170-
this._process.stdout?.on('error', error => {
200+
proc.stdout?.on('error', error => {
201+
if (usurped()) return;
171202
this.onerror?.(error);
172203
});
173204

174-
if (this._stderrStream && this._process.stderr) {
175-
this._process.stderr.pipe(this._stderrStream);
205+
if (this._stderrStream && proc.stderr) {
206+
proc.stderr.pipe(this._stderrStream);
176207
}
177208
});
178209
}
@@ -183,6 +214,11 @@ export class StdioClientTransport implements Transport {
183214
* If `stderr` piping was requested, a `PassThrough` stream is returned _immediately_, allowing callers to
184215
* attach listeners before the `start` method is invoked. This prevents loss of any early
185216
* error output emitted by the child process.
217+
*
218+
* Under `versionNegotiation: { mode: 'auto' }`, a legacy fallback may respawn the child with a
219+
* fresh stream — re-read this getter after `connect()` resolves. A fresh stream whose
220+
* predecessor WAS being read buffers until read: leaving it unread can eventually stall a
221+
* chatty child on stderr backpressure, not merely lose output.
186222
*/
187223
get stderr(): Stream | null {
188224
if (this._stderrStream) {
@@ -217,6 +253,8 @@ export class StdioClientTransport implements Transport {
217253
}
218254

219255
async close(): Promise<void> {
256+
// Close-provenance stamp (see CLOSED_BY_CALLER): a local close is never respawned.
257+
(this as { [CLOSED_BY_CALLER]?: boolean })[CLOSED_BY_CALLER] = true;
220258
if (this._process) {
221259
const processToClose = this._process;
222260
this._process = undefined;
@@ -252,8 +290,21 @@ export class StdioClientTransport implements Transport {
252290
// ignore
253291
}
254292
}
293+
294+
// Teardown is done: a predecessor whose stderr pipe outlives close()
295+
// (a held-open pipe, a SIGKILL still landing) must not end the stream
296+
// a NEXT life pipes into after start()'s recreation check. Detached
297+
// here — not at the top — so shutdown-time stderr (the child's own
298+
// signal/EOF diagnostics) still reaches subscribers during the waits above.
299+
processToClose.stderr?.unpipe();
255300
}
256301

302+
// End the aggregate deterministically (idempotent; also for a transport
303+
// that was never started): after the unpipe above the predecessor's own
304+
// end can no longer arrive, so without this a stream whose child
305+
// outlived close() would NEVER end — hanging finished()/for-await
306+
// readers. start() begins the next life on a fresh stream.
307+
this._stderrStream?.end();
257308
this._readBuffer.clear();
258309
}
259310

0 commit comments

Comments
 (0)