Skip to content

Commit b139ad9

Browse files
fix(client): handle connection close during the version negotiation probe
A stdio server that exits on the unrecognized server/discover probe (the rmcp shape: terminate on any pre-initialize request) connected fine under mode 'legacy' but hard-failed under mode 'auto'. Classify that close as a legacy signal, symmetric with the stdio probe-timeout rule. StdioClientTransport owns the restart: an internal capability respawns the server after the child died, with generation-scoped child event handlers, the read buffer cleared on child exit, and refusal after a local close(); with stderr 'pipe' the aggregate stream spans restarts and ends exactly once, in close(). The probe layer checks for the capability and asks the transport to restart before running the plain initialize fallback; a transport without it, or whose restart refuses or fails, rejects with the existing typed negotiation error. On HTTP a mid-probe close keeps surfacing as the same typed connect error as any probe transport failure.
1 parent f60dff0 commit b139ad9

11 files changed

Lines changed: 1071 additions & 46 deletions

File tree

.changeset/probe-close-handling.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@modelcontextprotocol/core-internal': patch
3+
'@modelcontextprotocol/client': patch
4+
---
5+
6+
Handle a connection that closes during the `server/discover` version-negotiation probe.
7+
8+
On stdio, a server process that exits on the unrecognized probe is a legacy signal — symmetric with the stdio probe-timeout rule. Servers built on SDKs that terminate on any pre-`initialize` request (the official Rust SDK, rmcp, is the prominent example) previously connected fine under `mode: 'legacy'` but hard-failed under `mode: 'auto'`; `StdioClientTransport` now restarts itself (respawning the server from its retained parameters) and the client completes the plain `initialize` handshake there — the respawned process sees bytes identical to a plain legacy connect. A transport that was closed locally, or that cannot restart, rejects with a typed negotiation error instead: a deliberately terminated server is never resurrected. On HTTP-class transports a mid-probe close keeps surfacing as the same typed connect error as any other probe transport failure — an ambiguous drop is never era evidence.
9+
10+
`StdioClientTransport` restart mechanics: child events are generation-scoped — `start()` opens a generation and `close()` retires it, so a dead child's late events can touch neither a successor nor a closed transport — the read buffer is cleared on child exit, and with `stderr: 'pipe'` the stream spans restarts: it ends exactly once, on `close()`, not on a child's exit, and a `start()` after `close()` begins a fresh stream.

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,16 @@ error. Probe timeouts are **transport-aware**: on **stdio** a server that does n
7878
answer within `timeoutMs` is treated as legacy and the client falls back to `initialize`
7979
on the same stream (some legacy servers never respond to unknown pre-`initialize`
8080
requests at all); on **HTTP** a probe timeout rejects with `SdkError(RequestTimeout)`
81-
a dead HTTP server is never misreported as legacy. One browser-specific exception: an
82-
opaque CORS/preflight `TypeError` during the probe falls back to the legacy era, because
81+
a dead HTTP server is never misreported as legacy. A mid-probe **connection close** is
82+
transport-aware the same way: on **stdio** a server process that exits on the
83+
unrecognized probe is treated as legacy — the transport restarts itself (respawning
84+
the server) and the client falls back to `initialize` there (stdio servers built on
85+
SDKs that terminate on any pre-`initialize` request behave this way); on **HTTP** a
86+
mid-probe close rejects with the same typed connect error as any probe transport
87+
failure (`SdkError(EraNegotiationFailed)`) — an ambiguous drop is never misreported
88+
as legacy. A transport closed locally during the probe is never restarted: `connect()`
89+
rejects with the same typed error. One browser-specific exception: an opaque
90+
CORS/preflight `TypeError` during the probe falls back to the legacy era, because
8391
deployed 2025 servers commonly have CORS allow-lists that predate the 2026 headers.
8492

8593
```typescript

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 server process that exits on the unrecognized probe is a legacy server — stdio servers built on SDKs that terminate on any pre-`initialize` request behave exactly this way (the official Rust SDK, rmcp, is the prominent example) — so the transport restarts itself, respawning the server, and `connect()` falls back to `initialize` there; the respawned process sees the same bytes a `mode: 'legacy'` connect would have sent. On HTTP a mid-probe close is ambiguous — a proxy drop or a crash, never era evidence — so `connect()` rejects with the same typed `SdkError(EraNegotiationFailed)` as any probe transport failure. A transport closed locally during the probe is never restarted.
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

packages/client/src/client/probeClassifier.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
* Probe outcome classifier (pure module): maps the outcome of the connect-time
33
* `server/discover` probe onto one of four verdicts — modern era, the
44
* spec-mandated `-32022` corrective continuation, legacy fallback (the plain
5-
* 2025 `initialize` handshake on the same connection), or a typed connect error.
5+
* 2025 `initialize` handshake — on the same connection, or on a restarted
6+
* transport when a stdio close consumed it), or a typed connect error.
67
*
78
* The classifier is deliberately conservative: anything it does not positively
89
* recognize as modern resolves to the legacy fallback, and a network outage is a
@@ -48,6 +49,8 @@ export type ProbeOutcome =
4849
| { kind: 'network-error'; error: unknown }
4950
/** The transport's auth flow challenged during the probe send (`UnauthorizedError`). */
5051
| { kind: 'auth-required'; error: Error }
52+
/** The transport reported close while the probe awaited its reply. */
53+
| { kind: 'closed' }
5154
/** No response arrived within the probe timeout. */
5255
| { kind: 'timeout'; timeoutMs: number };
5356

@@ -82,8 +85,13 @@ export type ProbeVerdict =
8285
* arms a loop guard on the second rejection, throwing `error`.
8386
*/
8487
| { kind: 'corrective'; version: string; error: UnsupportedProtocolVersionError }
85-
/** Definitive legacy signal or unrecognized shape: perform the plain legacy `initialize` handshake on the same connection. */
86-
| { kind: 'legacy' }
88+
/**
89+
* Definitive legacy signal or unrecognized shape: perform the plain legacy
90+
* `initialize` handshake on the same connection. `restart` marks the one
91+
* row where the same connection no longer exists (stdio close): the
92+
* handshake needs the transport restarted first.
93+
*/
94+
| { kind: 'legacy'; restart?: true }
8795
/** Typed connect error — never converted to an era verdict. */
8896
| { kind: 'error'; error: Error };
8997

@@ -141,6 +149,21 @@ export function classifyProbeOutcome(outcome: ProbeOutcome, context: ProbeClassi
141149
// handshake an auth-gated modern server as legacy.
142150
return { kind: 'error', error: outcome.error };
143151
}
152+
case 'closed': {
153+
if (context.transportKind === 'stdio') {
154+
// A stdio child that exits on the unrecognized probe instead of
155+
// answering is a legacy signal — the same backward-compatibility
156+
// rule as the timeout row below (SDKs like the official Rust one
157+
// terminate the server on ANY pre-initialize request). The
158+
// stream died with the process, so the fallback needs the
159+
// transport restarted.
160+
return { kind: 'legacy', restart: true };
161+
}
162+
// On HTTP a mid-probe close is an ambiguous network condition
163+
// (proxy drop, crash, redeploy) — the same typed connect error as
164+
// any probe transport failure, never an era verdict.
165+
return classifyNetworkError(new Error('Connection closed during the version negotiation probe'), context);
166+
}
144167
case 'timeout': {
145168
if (context.transportKind === 'stdio') {
146169
// Per the stdio transport's backward-compatibility rule, a probe
@@ -253,7 +276,8 @@ function isOpaqueFetchTypeError(error: unknown): boolean {
253276
return error instanceof TypeError || (error instanceof Error && error.name === 'TypeError');
254277
}
255278

256-
function describeError(error: unknown): string {
279+
/** Human-readable rendering of an unknown thrown value for probe diagnostics. */
280+
export function describeError(error: unknown): string {
257281
return error instanceof Error ? error.message : String(error);
258282
}
259283

packages/client/src/client/stdio.ts

Lines changed: 83 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import process from 'node:process';
33
import type { Stream } from 'node:stream';
44
import { PassThrough } from 'node:stream';
55

6-
import type { JSONRPCMessage, Transport } from '@modelcontextprotocol/core-internal';
7-
import { ReadBuffer, SdkError, SdkErrorCode, serializeMessage } from '@modelcontextprotocol/core-internal';
6+
import type { JSONRPCMessage, RestartableTransport } from '@modelcontextprotocol/core-internal';
7+
import { ReadBuffer, SdkError, SdkErrorCode, serializeMessage, TRANSPORT_RESTART } from '@modelcontextprotocol/core-internal';
88
import spawn from 'cross-spawn';
99

1010
export type StdioServerParameters = {
@@ -98,11 +98,21 @@ export function getDefaultEnvironment(): Record<string, string> {
9898
*
9999
* This transport is only available in Node.js environments.
100100
*/
101-
export class StdioClientTransport implements Transport {
101+
export class StdioClientTransport implements RestartableTransport {
102102
private _process?: ChildProcess;
103103
private _readBuffer: ReadBuffer;
104104
private _serverParams: StdioServerParameters;
105105
private _stderrStream: PassThrough | null = null;
106+
/**
107+
* Monotonic child generation; every child's event handlers check it so a
108+
* dead child's late events cannot touch a successor. `start()` opens a
109+
* generation and `close()` retires it, so a locally-closed child's late
110+
* events (a SIGKILLed child's trailing stdout flush, its eventual 'close')
111+
* are inert — close() already performed the cleanup itself.
112+
*/
113+
private _generation = 0;
114+
/** `true` after a local {@linkcode close} (caller shutdown or the transport's own error recovery); cleared by {@linkcode start}. */
115+
private _closedLocally = false;
106116

107117
onclose?: () => void;
108118
onerror?: (error: Error) => void;
@@ -118,16 +128,26 @@ export class StdioClientTransport implements Transport {
118128

119129
/**
120130
* Starts the server process and prepares to communicate with it.
131+
*
132+
* After the child process exits, `start()` may be called again: a fresh
133+
* process is spawned from the same {@linkcode StdioServerParameters}.
121134
*/
122135
async start(): Promise<void> {
123136
if (this._process) {
124137
throw new Error(
125138
'StdioClientTransport already started! If using Client class, note that connect() calls start() automatically.'
126139
);
127140
}
141+
this._closedLocally = false;
142+
if (this._stderrStream?.writableEnded) {
143+
// close() ended the previous life's aggregate stream; a caller
144+
// start() after close() begins a new life with a fresh stream.
145+
this._stderrStream = new PassThrough();
146+
}
147+
const generation = ++this._generation;
128148

129149
return new Promise((resolve, reject) => {
130-
this._process = spawn(this._serverParams.command, this._serverParams.args ?? [], {
150+
const proc = spawn(this._serverParams.command, this._serverParams.args ?? [], {
131151
// merge default env with server env because mcp server needs some env vars
132152
env: {
133153
...getDefaultEnvironment(),
@@ -138,26 +158,39 @@ export class StdioClientTransport implements Transport {
138158
windowsHide: process.platform === 'win32',
139159
cwd: this._serverParams.cwd
140160
});
161+
this._process = proc;
141162

142-
this._process.on('error', error => {
163+
proc.on('error', error => {
143164
reject(error);
144-
this.onerror?.(error);
165+
if (this._generation === generation) {
166+
this.onerror?.(error);
167+
}
145168
});
146169

147-
this._process.on('spawn', () => {
170+
proc.on('spawn', () => {
148171
resolve();
149172
});
150173

151-
this._process.on('close', _code => {
174+
proc.on('close', _code => {
175+
if (this._generation !== generation) {
176+
return;
177+
}
152178
this._process = undefined;
179+
// Drop any partial trailing line so a restarted transport starts with a clean stream.
180+
this._readBuffer.clear();
153181
this.onclose?.();
154182
});
155183

156-
this._process.stdin?.on('error', error => {
157-
this.onerror?.(error);
184+
proc.stdin?.on('error', error => {
185+
if (this._generation === generation) {
186+
this.onerror?.(error);
187+
}
158188
});
159189

160-
this._process.stdout?.on('data', chunk => {
190+
proc.stdout?.on('data', chunk => {
191+
if (this._generation !== generation) {
192+
return;
193+
}
161194
try {
162195
this._readBuffer.append(chunk);
163196
this.processReadBuffer();
@@ -167,22 +200,41 @@ export class StdioClientTransport implements Transport {
167200
}
168201
});
169202

170-
this._process.stdout?.on('error', error => {
171-
this.onerror?.(error);
203+
proc.stdout?.on('error', error => {
204+
if (this._generation === generation) {
205+
this.onerror?.(error);
206+
}
172207
});
173208

174-
if (this._stderrStream && this._process.stderr) {
175-
this._process.stderr.pipe(this._stderrStream);
209+
if (this._stderrStream && proc.stderr) {
210+
// end: false — the aggregate stream spans children; it ends in close(), never on a child's exit.
211+
proc.stderr.pipe(this._stderrStream, { end: false });
176212
}
177213
});
178214
}
179215

216+
/**
217+
* Version-negotiation close-fallback (internal, keyed by
218+
* `TRANSPORT_RESTART`): respawn the server after the child died out from
219+
* under the connection. Refuses after a local {@linkcode close} — a
220+
* deliberately terminated server is never resurrected.
221+
*/
222+
async [TRANSPORT_RESTART](): Promise<void> {
223+
if (this._closedLocally) {
224+
throw new Error('StdioClientTransport was closed locally; refusing to respawn the server process');
225+
}
226+
await this.start();
227+
}
228+
180229
/**
181230
* The `stderr` stream of the child process, if {@linkcode StdioServerParameters.stderr} was set to `"pipe"` or `"overlapped"`.
182231
*
183232
* If `stderr` piping was requested, a `PassThrough` stream is returned _immediately_, allowing callers to
184233
* attach listeners before the `start` method is invoked. This prevents loss of any early
185-
* error output emitted by the child process.
234+
* error output emitted by the child process. The stream spans restarts — a child's exit
235+
* never ends it; it ends exactly once, when {@linkcode close} is called. A `start()` after
236+
* `close()` begins a new life with a fresh stream: re-read this getter after that `start()`
237+
* (no output is lost — the fresh stream buffers until read).
186238
*/
187239
get stderr(): Stream | null {
188240
if (this._stderrStream) {
@@ -217,8 +269,9 @@ export class StdioClientTransport implements Transport {
217269
}
218270

219271
async close(): Promise<void> {
220-
if (this._process) {
221-
const processToClose = this._process;
272+
this._closedLocally = true;
273+
const processToClose = this._process;
274+
if (processToClose) {
222275
this._process = undefined;
223276

224277
const closePromise = new Promise<void>(resolve => {
@@ -255,6 +308,18 @@ export class StdioClientTransport implements Transport {
255308
}
256309

257310
this._readBuffer.clear();
311+
// Retire the generation AFTER the teardown waits: a child that exited
312+
// during them delivered its 'close' (and onclose) normally; anything
313+
// still in flight past this point belongs to a closed life.
314+
this._generation++;
315+
316+
if (this._stderrStream !== null && !this._stderrStream.writableEnded) {
317+
// The aggregate stderr stream ends exactly once, here. Unpipe first:
318+
// a SIGKILLed child may still flush after we gave up waiting, and a
319+
// write to an ended stream would error.
320+
processToClose?.stderr?.unpipe(this._stderrStream);
321+
this._stderrStream.end();
322+
}
258323
}
259324

260325
send(message: JSONRPCMessage): Promise<void> {

0 commit comments

Comments
 (0)