Skip to content

Commit 10620fa

Browse files
committed
fix(lifecycle): keep a teardown failure out of the setup check
connectToServer, the initialize SUCCESS push and close() all sat inside one try, so a close() failure after a completed handshake returned `server-initialize-setup` and discarded the SUCCESS just recorded. That is the same misattribution this PR exists to remove, in the scenario it is named after; prompts.ts, resources.ts and tools.ts already isolate the connect call. Isolate connect so only a handshake failure reports as setup. Teardown is best-effort at this layer to match terminateSessionRaw, which already swallows the session-terminating DELETE, so the only thing that can still reject is the client-side close, and that is not a server conformance result.
1 parent 660b042 commit 10620fa

2 files changed

Lines changed: 47 additions & 24 deletions

File tree

‎src/scenarios/server/lifecycle.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,20 @@ describe('ServerInitializeScenario', () => {
163163
// The session-id check is never reached, so no raw fetch is attempted.
164164
expect(fetchMock).not.toHaveBeenCalled();
165165
});
166+
167+
it('keeps the initialize SUCCESS when the client close fails', async () => {
168+
fetchMock.mockResolvedValue(new Response(null));
169+
closeMock.mockRejectedValueOnce(new Error('transport already closed'));
170+
171+
const checks = await new ServerInitializeScenario().run(
172+
testContext(serverUrl)
173+
);
174+
175+
// Teardown is best-effort, so a close failure must neither retract the
176+
// recorded SUCCESS nor be reported as a setup failure (#248).
177+
expect(checks).toContainEqual(
178+
expect.objectContaining({ id: 'server-initialize', status: 'SUCCESS' })
179+
);
180+
expect(checks.some((c) => c.id === 'server-initialize-setup')).toBe(false);
181+
});
166182
});

‎src/scenarios/server/lifecycle.ts‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import type { RunContext } from '../../connection';
1111
import {
1212
connectToServer,
13+
type MCPClientConnection,
1314
reportSetupFailure,
1415
terminateSessionRaw
1516
} from '../../connection/sdk-client';
@@ -48,38 +49,44 @@ and validates session ID format if one is assigned.`;
4849
const { serverUrl } = ctx;
4950
const checks: ConformanceCheck[] = [];
5051

52+
let connection: MCPClientConnection;
5153
try {
52-
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
53-
54-
// The connection process already does initialization
55-
// Check that we have a connected client
56-
checks.push({
57-
id: 'server-initialize',
58-
name: 'ServerInitialize',
59-
description:
60-
'Server responds to initialize request with valid structure',
61-
status: 'SUCCESS',
62-
timestamp: new Date().toISOString(),
63-
specReferences: [
64-
{
65-
id: 'MCP-Initialize',
66-
url: 'https://modelcontextprotocol.io/specification/2025-06-18/basic/lifecycle#initialization'
67-
}
68-
],
69-
details: {
70-
serverUrl,
71-
connected: true
72-
}
73-
});
74-
75-
await connection.close();
54+
connection = await connectToServer(serverUrl, {}, ctx.specVersion);
7655
} catch (error) {
7756
// The handshake never completed, so neither the initialize check nor the
7857
// session-id check below can be evaluated. Report a single setup failure
7958
// rather than mislabeling it as one specific check failing (#248).
8059
return reportSetupFailure(this.name, error);
8160
}
8261

62+
// The connection process already does initialization
63+
// Check that we have a connected client
64+
checks.push({
65+
id: 'server-initialize',
66+
name: 'ServerInitialize',
67+
description: 'Server responds to initialize request with valid structure',
68+
status: 'SUCCESS',
69+
timestamp: new Date().toISOString(),
70+
specReferences: [
71+
{
72+
id: 'MCP-Initialize',
73+
url: 'https://modelcontextprotocol.io/specification/2025-06-18/basic/lifecycle#initialization'
74+
}
75+
],
76+
details: {
77+
serverUrl,
78+
connected: true
79+
}
80+
});
81+
82+
// Teardown on an already-recorded handshake. The session-terminating
83+
// DELETE inside close() is best-effort by design (see terminateSessionRaw),
84+
// so what can still reject here is the client-side close, which is not a
85+
// server conformance result. Keeping it in the connect try above reported
86+
// it as `server-initialize-setup` and dropped the SUCCESS just recorded,
87+
// which is the misattribution #248 is about.
88+
await connection.close().catch(() => {});
89+
8390
// Check: Session ID visible ASCII validation
8491
// Use a raw fetch to inspect the MCP-Session-Id response header,
8592
// since the SDK client transport does not expose it.

0 commit comments

Comments
 (0)