Skip to content

Commit 9bb567b

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 the child once via the transport's public close()/start() and run the plain initialize handshake there. A transport the caller closed mid-probe is never respawned, and an HTTP-class mid-probe close keeps rejecting with the typed connect error. StdioClientTransport.start() clears the read buffer so a dead predecessor's late flush cannot corrupt the next child's first frame.
1 parent f60dff0 commit 9bb567b

10 files changed

Lines changed: 513 additions & 7 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 the transport's own `close()`/`start()` and completes the plain `initialize` handshake there — stdio servers built on SDKs that terminate on any pre-`initialize` request (the official Rust SDK, rmcp) previously hard-failed under `'auto'`. A transport the caller closed mid-probe is never respawned, and on HTTP-class transports a mid-probe close keeps rejecting with the typed connect error. `StdioClientTransport.start()` now also clears the read buffer, so a dead predecessor's late flush cannot corrupt the next child's first frame.

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: 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. A transport the caller closed during the probe is never respawned, and on HTTP a mid-probe close is ambiguous — never era evidence — so `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 {

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: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ export class StdioClientTransport implements Transport {
103103
private _readBuffer: ReadBuffer;
104104
private _serverParams: StdioServerParameters;
105105
private _stderrStream: PassThrough | null = null;
106+
/** True after a local `close()` until the next `start()` — the negotiation layer's legacy-fallback respawn checks it so a caller's kill decision is never overridden. */
107+
private _closedByCaller = false;
106108

107109
onclose?: () => void;
108110
onerror?: (error: Error) => void;
@@ -126,6 +128,14 @@ export class StdioClientTransport implements Transport {
126128
);
127129
}
128130

131+
this._closedByCaller = false;
132+
// A dead predecessor's late flush must not prepend to the new child's first frame.
133+
this._readBuffer.clear();
134+
if (this._stderrStream?.writableEnded) {
135+
// stderr: 'pipe' — the previous child's exit ended the stream; each life pipes into a fresh one.
136+
this._stderrStream = new PassThrough();
137+
}
138+
129139
return new Promise((resolve, reject) => {
130140
this._process = spawn(this._serverParams.command, this._serverParams.args ?? [], {
131141
// merge default env with server env because mcp server needs some env vars
@@ -183,6 +193,9 @@ export class StdioClientTransport implements Transport {
183193
* If `stderr` piping was requested, a `PassThrough` stream is returned _immediately_, allowing callers to
184194
* attach listeners before the `start` method is invoked. This prevents loss of any early
185195
* error output emitted by the child process.
196+
*
197+
* Under `versionNegotiation: { mode: 'auto' }`, a legacy fallback may respawn the child with a
198+
* fresh stream — re-read this getter after `connect()` resolves.
186199
*/
187200
get stderr(): Stream | null {
188201
if (this._stderrStream) {
@@ -217,6 +230,7 @@ export class StdioClientTransport implements Transport {
217230
}
218231

219232
async close(): Promise<void> {
233+
this._closedByCaller = true;
220234
if (this._process) {
221235
const processToClose = this._process;
222236
this._process = undefined;

packages/client/src/client/versionNegotiation.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727

2828
import { UnauthorizedError } from './auth';
2929
import type { ProbeEnvironment, ProbeOutcome, ProbeTransportKind, ProbeVerdict } from './probeClassifier';
30-
import { classifyProbeOutcome } from './probeClassifier';
30+
import { classifyProbeOutcome, describeError } from './probeClassifier';
3131

3232
/**
3333
* Probe policy for `'auto'` and pinned negotiation modes.
@@ -71,7 +71,11 @@ export interface VersionNegotiationProbeOptions {
7171
* outcome is definitive modern evidence. Network outage rejects with a typed
7272
* connect error; a probe timeout falls back to `initialize` on stdio (a silent
7373
* server on a local pipe is a legacy server) and rejects with a typed timeout
74-
* error on HTTP (silence there is an outage).
74+
* error on HTTP (silence there is an outage). A connection closed mid-probe
75+
* follows the same split: on stdio a child that exits on the unrecognized
76+
* probe is a legacy server — the client respawns it once (`close()` +
77+
* `start()`) and runs `initialize` on the fresh child; on HTTP it rejects
78+
* with the same typed connect error as any probe transport failure.
7579
* - `{ pin: '<version>' }` — modern era at exactly the pinned revision: the
7680
* connect-time `server/discover` must offer it. No fallback — anything else
7781
* fails loudly with a typed error.
@@ -264,6 +268,16 @@ class ProbeWindow {
264268
});
265269
}
266270

271+
/**
272+
* The legacy-fallback respawn opened a fresh connection: the forwarded
273+
* mid-window close belonged to the previous life, so the successor's
274+
* eventual close is a NEW event — detach()'s spent-close guard must not
275+
* swallow it for the caller's pre-set observer.
276+
*/
277+
noteRespawn(): void {
278+
this._closeDelivered = false;
279+
}
280+
267281
/** Detach the window's handlers, restoring any the caller pre-set, leaving the transport's own `start` untouched. */
268282
detach(): void {
269283
this._pending = undefined;
@@ -339,7 +353,9 @@ function normalizeReply(reply: RawProbeReply, timeoutMs: number): ProbeOutcome {
339353
return { kind: 'network-error', error };
340354
}
341355
case 'closed': {
342-
return { kind: 'network-error', error: new Error('Connection closed during the version negotiation probe') };
356+
// Not folded into network-error: the classifier's closed row is
357+
// transport-aware (stdio legacy signal vs typed error).
358+
return { kind: 'closed' };
343359
}
344360
case 'timeout': {
345361
return { kind: 'timeout', timeoutMs };
@@ -437,6 +453,32 @@ export async function negotiateEra(
437453
'pre-2026-07-28 protocol version to fall back to'
438454
);
439455
}
456+
if (outcome.kind === 'closed') {
457+
// The probe consumed the connection (the child exited on
458+
// the unrecognized request): respawn via the public
459+
// lifecycle, then run the fallback handshake there. The
460+
// flag read happens BEFORE our own close() below — the
461+
// only close this path issues — and start() resets it,
462+
// so it can only be true when the CALLER closed
463+
// mid-probe: never override that kill decision.
464+
if ((deps.transport as { _closedByCaller?: boolean })._closedByCaller) {
465+
throw new SdkError(
466+
SdkErrorCode.EraNegotiationFailed,
467+
'Version negotiation failed: the transport was closed during the server/discover probe'
468+
);
469+
}
470+
try {
471+
await deps.transport.close();
472+
await deps.transport.start();
473+
} catch (error) {
474+
throw new SdkError(
475+
SdkErrorCode.EraNegotiationFailed,
476+
`Version negotiation failed: the transport could not restart for the legacy fallback: ${describeError(error)}`,
477+
{ cause: error }
478+
);
479+
}
480+
window.noteRespawn();
481+
}
440482
return { era: 'legacy' };
441483
}
442484
case 'error': {

packages/client/test/client/probeClassifier.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,22 @@ describe('row: timeout — transport-aware verdict', () => {
293293
});
294294
});
295295

296+
describe('row: closed — transport-aware verdict, symmetric with the timeout row', () => {
297+
test('stdio: a child that exits on the unrecognized probe is a legacy signal → legacy fallback', () => {
298+
expect(classify({ kind: 'closed' }, { transportKind: 'stdio' })).toEqual({ kind: 'legacy' });
299+
});
300+
301+
test('HTTP: a mid-probe close is ambiguous — the same typed connect error as any probe network failure', () => {
302+
const verdict = classify({ kind: 'closed' }, { transportKind: 'http' });
303+
expect(verdict.kind).toBe('error');
304+
if (verdict.kind === 'error') {
305+
expect(verdict.error).toBeInstanceOf(SdkError);
306+
expect((verdict.error as SdkError).code).toBe(SdkErrorCode.EraNegotiationFailed);
307+
expect(verdict.error.message).toContain('Connection closed during the version negotiation probe');
308+
}
309+
});
310+
});
311+
296312
describe('row: browser opaque CORS/preflight TypeError, PROBE PHASE ONLY → legacy fallback (F-7)', () => {
297313
test('browser environment + bare TypeError → legacy', () => {
298314
expect(classify({ kind: 'network-error', error: new TypeError('Failed to fetch') }, { environment: 'browser' })).toEqual({

packages/client/test/client/stdio.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,61 @@ test('should respect custom maxBufferSize option', async () => {
9999
await closed;
100100
});
101101

102+
test("a dead child's late stdout flush landing between close() and start() cannot poison the next life's first frame", async () => {
103+
// The grandchild inherits (and so holds open) the child's stdout pipe and
104+
// writes a stale partial frame into it AFTER close() returned: close()
105+
// races its 2s teardown wait, sees the child already exited, and returns
106+
// with the pipe still held, so the flush lands in the shared read buffer
107+
// between close() and the next start(). start() clears the buffer, so the
108+
// next life's first JSON-RPC line cannot parse as <stale><json>.
109+
const FLUSHING_SERVER_SCRIPT = String.raw`
110+
const { spawn } = require('child_process');
111+
spawn(process.execPath, ['-e', "setTimeout(() => process.stdout.write('stale:'), 2400); setTimeout(() => {}, 4000);"], {
112+
stdio: 'inherit'
113+
});
114+
let buffer = '';
115+
process.stdin.on('data', chunk => {
116+
buffer += chunk.toString();
117+
let index;
118+
while ((index = buffer.indexOf('\n')) !== -1) {
119+
const line = buffer.slice(0, index);
120+
buffer = buffer.slice(index + 1);
121+
if (line.trim() === '') continue;
122+
let message;
123+
try {
124+
message = JSON.parse(line);
125+
} catch {
126+
continue;
127+
}
128+
if (message.method === 'whoami' && message.id !== undefined) {
129+
process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: message.id, result: { pid: process.pid } }) + '\n');
130+
}
131+
}
132+
});
133+
process.stdin.on('end', () => process.exit(0));
134+
`;
135+
const transport = new StdioClientTransport({ command: process.execPath, args: ['-e', FLUSHING_SERVER_SCRIPT] });
136+
137+
await transport.start();
138+
await transport.close();
139+
140+
// Let the predecessor's stale flush land in the close()->start() window.
141+
await new Promise(resolve => setTimeout(resolve, 700));
142+
143+
await transport.start();
144+
const reply = await new Promise<JSONRPCMessage>((resolve, reject) => {
145+
const timer = setTimeout(() => reject(new Error('whoami timed out — stale residue poisoned the frame')), 5000);
146+
transport.onmessage = message => {
147+
clearTimeout(timer);
148+
resolve(message);
149+
};
150+
transport.send({ jsonrpc: '2.0', id: 1, method: 'whoami' }).catch(reject);
151+
});
152+
expect(reply).toMatchObject({ id: 1, result: { pid: transport.pid } });
153+
154+
await transport.close();
155+
}, 15_000);
156+
102157
test('should fire onerror and close when ReadBuffer overflows', async () => {
103158
const client = new StdioClientTransport({
104159
command: 'node',

0 commit comments

Comments
 (0)