Skip to content

Commit 95dd2e2

Browse files
committed
fix(runner): close connections that finish after scenario timeout
A session can already exist while SDK connect() is awaiting the notifications/initialized acknowledgment. If the runner times out then, the original cleanup sweep has no returned connection to track. Mark the scenario finished before its sweep, reject new connection attempts from that context, and close a pending connection if it resolves after completion before handing it to the abandoned scenario. A real HTTP/SDK regression holds the initialization acknowledgment until the runner returns, then verifies one late-session DELETE, no abandoned probe, and a successful successor under a one-session cap. It fails on the original PR head with a leaked session and HTTP503. The original tracking fix and its three tests are preserved. Zero × Youngseok Oh
1 parent a95deff commit 95dd2e2

2 files changed

Lines changed: 181 additions & 5 deletions

File tree

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import http from 'http';
2+
import type { AddressInfo } from 'net';
3+
import { expect, onTestFinished, test } from 'vitest';
4+
import { runServerConformanceTest } from './server';
5+
6+
function deferred<T>() {
7+
let resolve!: (value: T) => void;
8+
const promise = new Promise<T>((done) => {
9+
resolve = done;
10+
});
11+
return { promise, resolve };
12+
}
13+
14+
test('closes a connection completed after timeout before the abandoned scenario can use it', async () => {
15+
const initialized = deferred<http.ServerResponse>();
16+
const lateActivity = deferred<void>();
17+
const liveSessions = new Set<string>();
18+
const events: Array<{ method: string; sessionId?: string }> = [];
19+
let issuedSessions = 0;
20+
21+
const respond = (res: http.ServerResponse, body: unknown) => {
22+
res.writeHead(200, { 'Content-Type': 'application/json' });
23+
res.end(JSON.stringify(body));
24+
};
25+
26+
// Real SDK Client + StreamableHTTPClientTransport, driven by the actual
27+
// runner. This wire fixture has room for one live session at a time.
28+
const server = http.createServer((req, res) => {
29+
const sessionId = req.headers['mcp-session-id'] as string | undefined;
30+
if (req.method === 'DELETE') {
31+
events.push({ method: 'DELETE', sessionId });
32+
if (sessionId) liveSessions.delete(sessionId);
33+
res.writeHead(204).end();
34+
if (sessionId === 's1') lateActivity.resolve();
35+
return;
36+
}
37+
if (req.method === 'GET') {
38+
events.push({ method: 'GET', sessionId });
39+
res.writeHead(200, { 'Content-Type': 'text/event-stream' });
40+
res.write(': open\n\n');
41+
return;
42+
}
43+
44+
let raw = '';
45+
req.on('data', (chunk) => (raw += chunk));
46+
req.on('end', () => {
47+
const msg = JSON.parse(raw);
48+
if (msg.method === 'initialize') {
49+
if (liveSessions.size > 0) {
50+
events.push({ method: 'REJECT initialize' });
51+
res.writeHead(503).end('Session capacity exhausted');
52+
return;
53+
}
54+
const sessionId = `s${++issuedSessions}`;
55+
liveSessions.add(sessionId);
56+
events.push({ method: 'POST initialize', sessionId });
57+
res.setHeader('mcp-session-id', sessionId);
58+
respond(res, {
59+
jsonrpc: '2.0',
60+
id: msg.id,
61+
result: {
62+
protocolVersion: msg.params.protocolVersion,
63+
capabilities: { prompts: {} },
64+
serverInfo: { name: 'late-connect-probe', version: '0.0.1' }
65+
}
66+
});
67+
return;
68+
}
69+
70+
events.push({ method: `POST ${msg.method}`, sessionId });
71+
if (msg.method === 'notifications/initialized') {
72+
if (sessionId === 's1') {
73+
// The SDK awaits this HTTP acknowledgment before connect() resolves.
74+
// A real session already exists, but ctx.connect() is still pending.
75+
initialized.resolve(res);
76+
return;
77+
}
78+
res.writeHead(202).end();
79+
return;
80+
}
81+
if (msg.method === 'prompts/list' && sessionId === 's2') {
82+
respond(res, {
83+
jsonrpc: '2.0',
84+
id: msg.id,
85+
result: { prompts: [{ name: 'p', description: 'healthy prompt' }] }
86+
});
87+
return;
88+
}
89+
90+
respond(res, {
91+
jsonrpc: '2.0',
92+
id: msg.id,
93+
error: { code: -32601, message: `Method not found: ${msg.method}` }
94+
});
95+
// The unfixed runner hands s1 to the abandoned scenario. Its failed
96+
// prompts/list skips the scenario's close(), leaving no later sweeper.
97+
if (sessionId === 's1') lateActivity.resolve();
98+
});
99+
});
100+
onTestFinished(async () => {
101+
server.closeAllConnections();
102+
await new Promise<void>((resolve) => server.close(() => resolve()));
103+
});
104+
105+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
106+
const url = `http://127.0.0.1:${(server.address() as AddressInfo).port}/mcp`;
107+
108+
const pendingRun = runServerConformanceTest(
109+
url,
110+
'prompts-list',
111+
undefined,
112+
'2025-06-18',
113+
false,
114+
1000
115+
);
116+
const acknowledgment = await initialized.promise;
117+
const timedOut = await pendingRun;
118+
expect(timedOut.checks).toContainEqual(
119+
expect.objectContaining({ id: 'scenario-timeout', status: 'FAILURE' })
120+
);
121+
expect([...liveSessions]).toEqual(['s1']);
122+
expect(events.some((event) => event.method === 'DELETE')).toBe(false);
123+
124+
events.push({ method: 'runner returned scenario-timeout' });
125+
acknowledgment.writeHead(202).end();
126+
// Positive network barrier: either cleanup terminates s1 or the abandoned
127+
// scenario incorrectly probes it. No quiet-time sleeps decide the result.
128+
await lateActivity.promise;
129+
130+
const healthy = await runServerConformanceTest(
131+
url,
132+
'prompts-list',
133+
undefined,
134+
'2025-06-18'
135+
);
136+
const observation = {
137+
lateSessionDeletes: events.filter(
138+
(event) => event.sessionId === 's1' && event.method === 'DELETE'
139+
).length,
140+
lateSessionProbes: events.filter(
141+
(event) =>
142+
event.sessionId === 's1' && event.method === 'POST prompts/list'
143+
).length,
144+
healthyPromptStatus: healthy.checks.find(
145+
(check) => check.id === 'prompts-list'
146+
)?.status,
147+
healthyFailures: healthy.checks.filter(
148+
(check) => check.status === 'FAILURE'
149+
).length,
150+
healthySessionDeletes: events.filter(
151+
(event) => event.sessionId === 's2' && event.method === 'DELETE'
152+
).length,
153+
liveSessions: [...liveSessions]
154+
};
155+
// Capture state before any fixture teardown can hide a leaked session.
156+
console.log('late connection wire events:', JSON.stringify(events));
157+
console.log('late connection observation:', JSON.stringify(observation));
158+
expect(observation).toEqual({
159+
lateSessionDeletes: 1,
160+
lateSessionProbes: 0,
161+
healthyPromptStatus: 'SUCCESS',
162+
healthyFailures: 0,
163+
healthySessionDeletes: 1,
164+
liveSessions: []
165+
});
166+
});

‎src/runner/server.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,21 +203,31 @@ export async function runServerConformanceTest(
203203
);
204204

205205
const openConnections = new Set<Connection>();
206+
let scenarioFinished = false;
206207
const ctx: RunContext = {
207208
serverUrl,
208209
specVersion: resolvedSpecVersion,
209-
connect: async (opts) =>
210-
trackConnection(
211-
openConnections,
212-
await connectFor(resolvedSpecVersion)(serverUrl, opts)
213-
)
210+
connect: async (opts) => {
211+
if (scenarioFinished) {
212+
throw new Error(`Scenario '${scenarioName}' has already finished`);
213+
}
214+
const conn = await connectFor(resolvedSpecVersion)(serverUrl, opts);
215+
// A handshake can finish after the timeout's cleanup sweep. Close it
216+
// here and keep the abandoned scenario from issuing any more probes.
217+
if (scenarioFinished) {
218+
await conn.close();
219+
throw new Error(`Scenario '${scenarioName}' has already finished`);
220+
}
221+
return trackConnection(openConnections, conn);
222+
}
214223
};
215224
resetWireValidation();
216225
const checks = await runScenarioBounded(
217226
scenario.run(ctx),
218227
scenarioName,
219228
timeout
220229
);
230+
scenarioFinished = true;
221231
await closeOpenConnections(openConnections);
222232
checks.push(...wireSchemaChecks(resolvedSpecVersion));
223233

0 commit comments

Comments
 (0)