From 90ee47dd566bab4b51bbaab9cce2dd6deb68835d Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Sun, 16 Aug 2026 12:22:47 +0300 Subject: [PATCH 1/3] fix(core): never send notifications/cancelled for the initialize handshake The spec is explicit that a client MUST NOT attempt to cancel its initialize request, but the outbound cancel closure fired for any in-flight request: aborting connect()'s AbortSignal, or letting the handshake time out, put a forbidden cancellation on the wire naming the initialize request id. Guard the send on the request method. The caller's promise still rejects with the same abort/timeout error and connect() still tears the connection down; only the wire notification is suppressed. The guard sits inside the non-stream-close branch, so the modern per-request-stream path is unchanged - initialize is legacy-era only, absent from the modern registry. The e2e conformance suite already tracked this as a known deviation, so drop the knownFailures entry on protocol:cancel:initialize-not-cancellable; its 8 transport x era cells now pass as real assertions. Ports #1932 onto the core-internal layout. Fixes #998. --- .../no-cancel-notification-for-initialize.md | 9 +++ packages/core-internal/src/shared/protocol.ts | 36 ++++++---- .../test/shared/protocol.test.ts | 68 +++++++++++++++++++ test/e2e/requirements.ts | 7 +- 4 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 .changeset/no-cancel-notification-for-initialize.md diff --git a/.changeset/no-cancel-notification-for-initialize.md b/.changeset/no-cancel-notification-for-initialize.md new file mode 100644 index 0000000000..b7acd086d3 --- /dev/null +++ b/.changeset/no-cancel-notification-for-initialize.md @@ -0,0 +1,9 @@ +--- +'@modelcontextprotocol/core-internal': patch +'@modelcontextprotocol/client': patch +'@modelcontextprotocol/server': patch +--- + +Stop sending `notifications/cancelled` for the `initialize` handshake. The spec is explicit that a client MUST NOT attempt to cancel its `initialize` request, but the outbound cancel path fired for any in-flight request: aborting the `AbortSignal` passed to `connect()`, or letting the handshake hit its timeout, put a forbidden cancellation on the wire naming the initialize request id. + +The local behaviour is unchanged — the caller's promise still rejects with the same abort/timeout error, and `connect()` still tears the connection down. Only the wire notification is suppressed. Every other method keeps the existing cancellation path. diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index 0a19770082..13d2c9a309 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -1451,19 +1451,29 @@ export abstract class Protocol { this._progressHandlers.delete(messageId); if (requestAbort === undefined) { - this._transport - ?.send( - this._envelopeOutbound({ - jsonrpc: '2.0', - method: 'notifications/cancelled', - params: { - requestId: messageId, - reason: String(reason) - } - }), - { relatedRequestId, resumptionToken, onresumptiontoken } - ) - .catch(error => this._onerror(new Error(`Failed to send cancellation: ${error}`))); + // "A client MUST NOT attempt to cancel its `initialize` + // request" (spec basic/lifecycle, mirrored on + // `CancelledNotification`). The handshake is the one request + // whose cancellation is forbidden outright, so an abort or + // timeout on it settles purely locally: the promise still + // rejects below, but nothing goes on the wire. Only the + // legacy era can reach this — `initialize` is absent from the + // modern registry, which negotiates via `server/discover`. + if (request.method !== 'initialize') { + this._transport + ?.send( + this._envelopeOutbound({ + jsonrpc: '2.0', + method: 'notifications/cancelled', + params: { + requestId: messageId, + reason: String(reason) + } + }), + { relatedRequestId, resumptionToken, onresumptiontoken } + ) + .catch(error => this._onerror(new Error(`Failed to send cancellation: ${error}`))); + } } else { // Modern-era per-request-stream transport: aborting the // request's underlying stream IS the spec cancel signal. diff --git a/packages/core-internal/test/shared/protocol.test.ts b/packages/core-internal/test/shared/protocol.test.ts index 2ecdc40adc..3790f73b75 100644 --- a/packages/core-internal/test/shared/protocol.test.ts +++ b/packages/core-internal/test/shared/protocol.test.ts @@ -911,6 +911,74 @@ describe('protocol tests', () => { expect(tx.lastRequestSignal?.aborted).toBe(true); expect(cancelledSent(tx.sent)).toHaveLength(0); }); + + // "A client MUST NOT attempt to cancel its `initialize` request." The + // handshake is exempt from the POST path above on every transport: an + // abort or timeout rejects the caller locally and sends nothing. Both + // triggers are covered because they reach cancel() by different routes + // (the caller's signal vs the timeout handler). + describe('the initialize handshake is never cancelled on the wire', () => { + test('aborting an in-flight initialize sends NO notifications/cancelled', async () => { + // ARRANGE + const sent: JSONRPCMessage[] = []; + const tx = new MockTransport(); + tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { + sent.push(m); + }; + const proto = createTestProtocol(); + await proto.connect(tx); + setNegotiatedProtocolVersion(proto, '2025-11-25'); + + // ACT + const ac = new AbortController(); + const pending = testRequest(proto, { method: 'initialize', params: {} }, z.object({}), { signal: ac.signal }); + ac.abort('user cancel'); + + // ASSERT — rejects locally, wire stays clean + await expect(pending).rejects.toThrow(); + expect(cancelledSent(sent)).toHaveLength(0); + }); + + test('timing out an in-flight initialize sends NO notifications/cancelled', async () => { + // ARRANGE + const sent: JSONRPCMessage[] = []; + const tx = new MockTransport(); + tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { + sent.push(m); + }; + const proto = createTestProtocol(); + await proto.connect(tx); + setNegotiatedProtocolVersion(proto, '2025-11-25'); + + // ACT + const pending = testRequest(proto, { method: 'initialize', params: {} }, z.object({}), { timeout: 0 }); + + // ASSERT + await expect(pending).rejects.toThrow(); + expect(cancelledSent(sent)).toHaveLength(0); + }); + + test('every other method still POSTs notifications/cancelled (regression guard)', async () => { + // ARRANGE + const sent: JSONRPCMessage[] = []; + const tx = new MockTransport(); + tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { + sent.push(m); + }; + const proto = createTestProtocol(); + await proto.connect(tx); + setNegotiatedProtocolVersion(proto, '2025-11-25'); + + // ACT + const ac = new AbortController(); + const pending = testRequest(proto, { method: 'example', params: {} }, z.object({}), { signal: ac.signal }); + ac.abort('user cancel'); + + // ASSERT + await expect(pending).rejects.toThrow(); + expect(cancelledSent(sent)).toHaveLength(1); + }); + }); }); }); diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index a3439e1ceb..66205ed09f 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -143,12 +143,7 @@ export const REQUIREMENTS: Record = { transports: STATEFUL_TRANSPORTS, source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/cancellation#behavior-requirements', behavior: 'The client never sends notifications/cancelled for the initialize request.', - note: 'Stateless hosting creates a fresh server per request and has no standalone GET stream, so there is no server→client channel to deliver/observe these.', - knownFailures: [ - { - note: 'SDK sends notifications/cancelled for initialize when connect() is aborted; spec says initialize MUST NOT be cancelled.' - } - ] + note: 'Stateless hosting creates a fresh server per request and has no standalone GET stream, so there is no server→client channel to deliver/observe these.' }, 'protocol:cancel:late-response-ignored': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/cancellation#timing-considerations', From 190c3b0e89e968177a9492d284f459e73b850390 Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Mon, 17 Aug 2026 10:19:35 +0000 Subject: [PATCH 2/3] Address review nits: document the initialize cancel exemption, dedupe test arrange - docs/migration/upgrade-to-v2.md, support-2026-07-28.md: the "cancelled-on-timeout signal is unchanged on legacy-era connections and stdio" claim is now false for the initialize handshake, so say so next to it. - protocol.test.ts: hoist the recording single-channel arrange block into a connectSingleChannel(version) helper (it was copy-pasted four times in the block). No-Verification-Needed: docs and test-only change --- docs/migration/support-2026-07-28.md | 8 +-- docs/migration/upgrade-to-v2.md | 6 ++- .../test/shared/protocol.test.ts | 54 ++++++++----------- 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/docs/migration/support-2026-07-28.md b/docs/migration/support-2026-07-28.md index 13e869c0f2..0ca0c46aef 100644 --- a/docs/migration/support-2026-07-28.md +++ b/docs/migration/support-2026-07-28.md @@ -236,9 +236,11 @@ coverage, spawn `serveStdio` as a child process. On a 2026-07-28 Streamable HTTP connection, aborting an in-flight client request (`signal` / timeout) closes that request's SSE response stream — the spec cancellation signal — instead of POSTing `notifications/cancelled`. Nothing to change in calling -code. 2025-era connections and stdio at any era still send `notifications/cancelled`. -Custom `Transport` implementations that open one underlying request per outbound message -and honor `TransportSendOptions.requestSignal` may opt in by declaring +code. 2025-era connections and stdio at any era still send `notifications/cancelled` +(except for the `initialize` handshake, which the spec forbids cancelling — an aborted +or timed-out `connect()` rejects locally and sends nothing). Custom `Transport` +implementations that open one underlying request per outbound message and honor +`TransportSendOptions.requestSignal` may opt in by declaring `readonly hasPerRequestStream = true`. ### `ctx.mcpReq.log()` and the per-request `logLevel` diff --git a/docs/migration/upgrade-to-v2.md b/docs/migration/upgrade-to-v2.md index 19f4127733..8f4cd5990d 100644 --- a/docs/migration/upgrade-to-v2.md +++ b/docs/migration/upgrade-to-v2.md @@ -1506,7 +1506,11 @@ rewrite required unless noted. on those survive verbatim. The cancelled-on-timeout signal is unchanged on legacy-era connections and on stdio/in-memory at any era; on 2026-era Streamable HTTP the cancel signal is the per-request stream close instead of a `notifications/cancelled` POST - (see [support-2026-07-28.md](./support-2026-07-28.md)). + (see [support-2026-07-28.md](./support-2026-07-28.md)). The one exemption is the + `initialize` handshake: an aborted or timed-out `connect()` still rejects locally, but + no `notifications/cancelled` goes on the wire — the spec forbids cancelling + `initialize`, and v1 sent one anyway. v1 tests asserting that notification need + re-baselining. - **Also unchanged: SSE reconnection exhaustion.** `StreamableHTTPClientTransport`'s standalone GET-stream reconnection behavior and its exhaustion signal carry over from v1: when retries run out, the transport emits `onerror` with a plain `Error` whose diff --git a/packages/core-internal/test/shared/protocol.test.ts b/packages/core-internal/test/shared/protocol.test.ts index dc6982f6d5..2fb0f64813 100644 --- a/packages/core-internal/test/shared/protocol.test.ts +++ b/packages/core-internal/test/shared/protocol.test.ts @@ -866,6 +866,23 @@ describe('protocol tests', () => { const cancelledSent = (sent: JSONRPCMessage[]): JSONRPCMessage[] => sent.filter(m => 'method' in m && m.method === 'notifications/cancelled'); + /** + * Connects a fresh protocol over a single-channel transport (stdio / + * in-memory shape: no `hasPerRequestStream`) at `version`, recording + * every outbound message. + */ + const connectSingleChannel = async (version: string) => { + const sent: JSONRPCMessage[] = []; + const tx = new MockTransport(); + tx.send = async (m: JSONRPCMessage) => { + sent.push(m); + }; + const proto = createTestProtocol(); + await proto.connect(tx); + setNegotiatedProtocolVersion(proto, version); + return { proto, sent }; + }; + test('modern era + per-request-stream transport: abort closes the stream, NO notifications/cancelled', async () => { const tx = new PerRequestStreamTransport(); const proto = createTestProtocol(); @@ -888,15 +905,7 @@ describe('protocol tests', () => { }); test('modern era + single-channel transport (no hasPerRequestStream): POSTs notifications/cancelled', async () => { - // stdio / in-memory shape: hasPerRequestStream is undefined. - const sent: JSONRPCMessage[] = []; - const tx = new MockTransport(); - tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { - sent.push(m); - }; - const proto = createTestProtocol(); - await proto.connect(tx); - setNegotiatedProtocolVersion(proto, '2026-07-28'); + const { proto, sent } = await connectSingleChannel('2026-07-28'); const ac = new AbortController(); const pending = testRequest(proto, { method: 'example', params: {} }, z.object({}), { signal: ac.signal }); @@ -946,14 +955,7 @@ describe('protocol tests', () => { describe('the initialize handshake is never cancelled on the wire', () => { test('aborting an in-flight initialize sends NO notifications/cancelled', async () => { // ARRANGE - const sent: JSONRPCMessage[] = []; - const tx = new MockTransport(); - tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { - sent.push(m); - }; - const proto = createTestProtocol(); - await proto.connect(tx); - setNegotiatedProtocolVersion(proto, '2025-11-25'); + const { proto, sent } = await connectSingleChannel('2025-11-25'); // ACT const ac = new AbortController(); @@ -967,14 +969,7 @@ describe('protocol tests', () => { test('timing out an in-flight initialize sends NO notifications/cancelled', async () => { // ARRANGE - const sent: JSONRPCMessage[] = []; - const tx = new MockTransport(); - tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { - sent.push(m); - }; - const proto = createTestProtocol(); - await proto.connect(tx); - setNegotiatedProtocolVersion(proto, '2025-11-25'); + const { proto, sent } = await connectSingleChannel('2025-11-25'); // ACT const pending = testRequest(proto, { method: 'initialize', params: {} }, z.object({}), { timeout: 0 }); @@ -986,14 +981,7 @@ describe('protocol tests', () => { test('every other method still POSTs notifications/cancelled (regression guard)', async () => { // ARRANGE - const sent: JSONRPCMessage[] = []; - const tx = new MockTransport(); - tx.send = async (m: JSONRPCMessage, _opts?: TransportSendOptions) => { - sent.push(m); - }; - const proto = createTestProtocol(); - await proto.connect(tx); - setNegotiatedProtocolVersion(proto, '2025-11-25'); + const { proto, sent } = await connectSingleChannel('2025-11-25'); // ACT const ac = new AbortController(); From 10ea87bca32c4041b1b644161f117980887ae0b5 Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Mon, 17 Aug 2026 10:36:26 +0000 Subject: [PATCH 3/3] e2e: restrict initialize-not-cancellable to inMemory, matching its test body The scenario ignores the transport arg and always taps an InMemoryTransport pair (it has to see outbound messages before connect() resolves), so with the knownFailures entry gone the STATEFUL_TRANSPORTS declaration just ran the same body four times per axis. Declare transports: ['inMemory'] with a note that states the real constraint instead of the copy-pasted stateless-hosting one. No-Verification-Needed: e2e manifest / test-only change --- test/e2e/requirements.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 66205ed09f..75311be0e3 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -140,10 +140,10 @@ export const REQUIREMENTS: Record = { note: 'Stateless hosting creates a fresh server per request and has no standalone GET stream, so there is no server→client channel to deliver/observe these.' }, 'protocol:cancel:initialize-not-cancellable': { - transports: STATEFUL_TRANSPORTS, + transports: ['inMemory'], source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/cancellation#behavior-requirements', behavior: 'The client never sends notifications/cancelled for the initialize request.', - note: 'Stateless hosting creates a fresh server per request and has no standalone GET stream, so there is no server→client channel to deliver/observe these.' + note: "The behavior itself is transport-agnostic (shared/protocol.ts), but the test must tap the client's outbound messages before connect() resolves, which only the in-memory wiring supports." }, 'protocol:cancel:late-response-ignored': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/cancellation#timing-considerations',