diff --git a/src/scenarios/server/stateless.test.ts b/src/scenarios/server/stateless.test.ts index a2fb5f32..2821d9bb 100644 --- a/src/scenarios/server/stateless.test.ts +++ b/src/scenarios/server/stateless.test.ts @@ -30,8 +30,7 @@ describe('Stateless Server Scenario Negative Tests', () => { method: 'notifications/subscriptions/acknowledged', params: { _meta: { - 'io.modelcontextprotocol/subscriptionId': - 'global-valid-sub-id' + 'io.modelcontextprotocol/subscriptionId': body.id } } } @@ -66,14 +65,14 @@ describe('Stateless Server Scenario Negative Tests', () => { } const chunk = new TextEncoder().encode(streamData[frameIndex++]); return { value: chunk, done: false }; - } + }, + releaseLock: () => {} }; return { status: responseConfig?.status ?? 200, body: { - getReader: () => mockReader, - releaseLock: () => {} + getReader: () => mockReader } } as unknown as Response; } @@ -316,6 +315,86 @@ describe('Stateless Server Scenario Negative Tests', () => { expect(idCheck?.status).toBe('FAILURE'); }); + test('Fails validation when subscriptionId does not match the listen request id', async () => { + const mockUrl = mockFetchTarget((reqBody) => { + if (reqBody.method === 'subscriptions/listen') { + return { + isStream: true, + status: 200, + frames: [ + { + jsonrpc: '2.0', + method: 'notifications/subscriptions/acknowledged', + params: { + _meta: { + // Spec Violation: subscriptionId must equal the JSON-RPC id + // of the subscriptions/listen request, not an arbitrary token. + 'io.modelcontextprotocol/subscriptionId': + 'server-generated-token' + } + } + } + ] + }; + } + }); + + const scenario = new ServerStatelessScenario(); + const checks = await scenario.run(testContext(mockUrl)); + + const idCheck = findCheck(checks, 'sep-2575-server-tags-subscription-id'); + expect(idCheck?.status).toBe('FAILURE'); + expect(idCheck?.errorMessage).toMatch(/does not match/); + + // Positive: the spec-compliant global fallback for tools/call streams no + // notifications/cancelled, so the new restriction check passes. + const cancelledCheck = findCheck( + checks, + 'sep-2575-server-cancelled-only-for-subscription' + ); + expect(cancelledCheck?.status).toBe('SUCCESS'); + }); + + test('Fails validation when notifications/cancelled appears on a non-subscription response stream', async () => { + const mockUrl = mockFetchTarget((reqBody) => { + if ( + reqBody.method === 'tools/call' && + reqBody.params?.name === 'test_streaming_elicitation' + ) { + return { + isStream: true, + status: 200, + frames: [ + { + jsonrpc: '2.0', + method: 'notifications/cancelled', + params: { requestId: reqBody.id, reason: 'server overload' } + }, + { + jsonrpc: '2.0', + id: reqBody.id, + result: { content: [{ type: 'text', text: 'done' }] } + } + ] + }; + } + }); + + const scenario = new ServerStatelessScenario(); + const checks = await scenario.run(testContext(mockUrl)); + + const cancelledCheck = findCheck( + checks, + 'sep-2575-server-cancelled-only-for-subscription' + ); + expect(cancelledCheck?.status).toBe('FAILURE'); + + // Positive: the spec-compliant global fallback for subscriptions/listen + // echoes the request id, so the strengthened subscription-id check passes. + const idCheck = findCheck(checks, 'sep-2575-server-tags-subscription-id'); + expect(idCheck?.status).toBe('SUCCESS'); + }); + test('Fails validation when tool response stream leaks raw independent JSON-RPC requests', async () => { const mockUrl = mockFetchTarget((reqBody) => { if ( diff --git a/src/scenarios/server/stateless.ts b/src/scenarios/server/stateless.ts index a995b83b..40874567 100644 --- a/src/scenarios/server/stateless.ts +++ b/src/scenarios/server/stateless.ts @@ -168,13 +168,14 @@ export class ServerStatelessScenario implements ClientScenario { params?: any, maxFrames = 3, timeoutMs = 1000, - onFirstFrame?: () => Promise + onFirstFrame?: () => Promise, + id: string | number = Date.now() ): Promise => { const headers = buildStandardHeaders(method, params, { specVersion }); const body = JSON.stringify({ jsonrpc: '2.0', - id: Date.now(), + id, method, ...(params !== undefined ? { params } : {}) }); @@ -772,21 +773,30 @@ export class ServerStatelessScenario implements ClientScenario { } ); + // Open a non-subscription SSE response stream once and reuse the captured + // frames across the independent-request and server-sent-cancelled checks. + let toolStreamFrames: any[] = []; + try { + toolStreamFrames = await listenToStream( + 'tools/call', + { + name: 'test_streaming_elicitation', + arguments: {}, + _meta: validMeta + }, + 3, + 600 + ); + } catch { + // Stream pipeline tracking failed + } + await runCheck( 'sep-2575-http-server-no-independent-requests-on-stream', 'HttpServerNoIndependentRequestsOnStream', 'Request stream contains only IncompleteResult, never independent JSON-RPC requests', - async () => { - const frames = await listenToStream( - 'tools/call', - { - name: 'test_streaming_elicitation', - arguments: {}, - _meta: validMeta - }, - 3, - 600 - ); + () => { + const frames = toolStreamFrames; if (frames.length === 0) { return { @@ -822,6 +832,47 @@ export class ServerStatelessScenario implements ClientScenario { } ); + await runCheck( + 'sep-2575-server-cancelled-only-for-subscription', + 'ServerCancelledOnlyForSubscription', + 'Servers MUST NOT send notifications/cancelled for any purpose other than tearing down a subscriptions/listen stream', + () => { + const frames = toolStreamFrames; + + if (frames.length === 0) { + return { + error: + 'Failed to receive progressive stream chunk execution frames from tools/call handler endpoint' + }; + } + + // If the call was rejected outright (e.g. the diagnostic tool does + // not exist), nothing was streamed and the requirement was not + // exercised - report SKIPPED rather than a vacuous SUCCESS. + if (frames.every((f) => f?.error !== undefined)) { + return { + skipped: true, + details: { + note: 'Server does not expose diagnostic tool test_streaming_elicitation; the response stream could not be exercised.', + response: frames[0] + } + }; + } + + const cancelledFrame = frames.find( + (f) => f?.method === 'notifications/cancelled' + ); + if (cancelledFrame) { + return { + error: + 'Server sent notifications/cancelled on a non-subscription response stream; servers MUST NOT send notifications/cancelled for any purpose other than tearing down a subscriptions/listen stream', + details: { cancelledFrame } + }; + } + return { details: { inspectedFramesCount: frames.length } }; + } + ); + await runCheck( 'sep-2575-server-no-log-without-loglevel', 'ServerNoLogWithoutLogLevel', @@ -880,6 +931,9 @@ export class ServerStatelessScenario implements ClientScenario { // Open a tools-filtered stream and (best-effort) trigger a tool-list // change once it is acknowledged, so the stream carries at least one // post-acknowledgment notification for the subscription-id check. + // The listen request id is fixed up front so the subscription-id check + // can assert that _meta.../subscriptionId echoes it. + const listenRequestId = Date.now(); let streamFrames: any[] = []; try { streamFrames = await listenToStream( @@ -893,7 +947,8 @@ export class ServerStatelessScenario implements ClientScenario { arguments: {}, _meta: validMeta }); - } + }, + listenRequestId ); } catch { // Stream pipeline tracking failed @@ -932,7 +987,7 @@ export class ServerStatelessScenario implements ClientScenario { await runCheck( 'sep-2575-server-tags-subscription-id', 'ServerTagsSubscriptionId', - 'Listen-stream notifications carry _meta.../subscriptionId', + 'Listen-stream notifications carry _meta.../subscriptionId equal to the JSON-RPC id of the subscriptions/listen request', () => { if (streamFrames[0]?.error?.code === -32601) { return { @@ -950,7 +1005,10 @@ export class ServerStatelessScenario implements ClientScenario { } // Every notification delivered on the stream (the acknowledgment and - // anything after it) must carry the subscription id in params._meta. + // anything after it) must carry the subscription id in params._meta, + // and that id must equal the JSON-RPC id of the subscriptions/listen + // request that opened the stream. RequestId is `string | number`, so + // compare via String() to accept either representation. const notificationFrames = streamFrames.filter( (f) => typeof f?.method === 'string' && @@ -965,7 +1023,9 @@ export class ServerStatelessScenario implements ClientScenario { } const untaggedFrames = notificationFrames.filter( - (f) => !f?.params?._meta?.['io.modelcontextprotocol/subscriptionId'] + (f) => + f?.params?._meta?.['io.modelcontextprotocol/subscriptionId'] === + undefined ); if (untaggedFrames.length > 0) { return { @@ -974,6 +1034,22 @@ export class ServerStatelessScenario implements ClientScenario { }; } + const mismatchedFrames = notificationFrames.filter( + (f) => + String( + f?.params?._meta?.['io.modelcontextprotocol/subscriptionId'] + ) !== String(listenRequestId) + ); + if (mismatchedFrames.length > 0) { + return { + error: `${mismatchedFrames.length} of ${notificationFrames.length} listen-stream notification(s) carry an io.modelcontextprotocol/subscriptionId that does not match the subscriptions/listen request id ${JSON.stringify(listenRequestId)}`, + details: { + expectedSubscriptionId: listenRequestId, + mismatchedFrames + } + }; + } + const subId = notificationFrames[0]?.params?._meta?.[ 'io.modelcontextprotocol/subscriptionId' @@ -981,6 +1057,7 @@ export class ServerStatelessScenario implements ClientScenario { return { details: { subscriptionId: subId, + expectedSubscriptionId: listenRequestId, inspectedNotificationCount: notificationFrames.length } }; diff --git a/src/seps/sep-2575.yaml b/src/seps/sep-2575.yaml index 798cae4d..a7719991 100644 --- a/src/seps/sep-2575.yaml +++ b/src/seps/sep-2575.yaml @@ -67,7 +67,13 @@ requirements: - check: sep-2575-server-no-log-without-loglevel text: 'The server MUST NOT emit notifications/message for a request that does not include [io.modelcontextprotocol/logLevel in _meta].' url: https://modelcontextprotocol.io/specification/draft/server/utilities/logging#per-request-log-level + - check: sep-2575-server-cancelled-only-for-subscription + text: 'Servers MUST NOT send notifications/cancelled for any other purpose [than tearing down a subscriptions/listen stream].' + url: https://modelcontextprotocol.io/specification/draft/basic/patterns/cancellation + - text: 'A server MUST send notifications/cancelled referencing a subscriptions/listen request ID when it tears down that subscription stream.' + excluded: 'No portable black-box trigger to force server-initiated subscription teardown.' + url: https://modelcontextprotocol.io/specification/draft/basic/patterns/cancellation - text: 'A server MUST NOT treat connection or process identity as a proxy for conversation or session continuity. / Servers MUST NOT rely on prior requests over the same connection to establish context (e.g., capabilities, protocol version, client identity).' excluded: 'internal server state, not directly wire-observable; the observable consequence (rejecting requests with incomplete _meta rather than falling back to remembered state) is covered by sep-2575-request-meta-invalid-* — see https://github.com/modelcontextprotocol/conformance/issues/296' - text: 'Servers MUST NOT require that a client reuse the same connection to perform related operations.'