Skip to content

Commit 513abdb

Browse files
pcarletonYuan325
pcarleton
and
Yuan325
authored
fix(sep-2575): resolve the remaining untested traceability rows (#300)
* chore: additional server conformance tests for SEP-2575 * fix(sep-2575): use the spec's notifications filter object in subscriptions/listen The scenario sent params.subscriptions: [{type: 'tools/list-changed'}], a shape that does not exist in the SEP. SubscriptionsListenRequestParams requires params.notifications: {toolsListChanged?, promptsListChanged?, ...} (see schema.ts SubscriptionFilter and the canonical SubscriptionsListenRequest example). A compliant server would either reject the old request as invalid params or treat it as subscribing to nothing, failing the ack/list-changed checks. Also makes the everything-server's acknowledgment echo the agreed notifications filter, as the SubscriptionsAcknowledgedNotification schema requires. * fix(sep-2575): match the spec's notifications/*/list_changed method names The list-changed checks and the everything-server used notifications/tools/list-changed (hyphen); the spec's method is notifications/tools/list_changed (underscore) per ToolListChangedNotification / PromptListChangedNotification in schema.ts. With the hyphen form the notification-filter check cannot catch a real leak, and the list-changed checks fail a server that emits the correct name. Also drops the f.params.toolsListChanged === true fallback match: that field is the subscriptions/listen request filter, not a notification parameter, so it never appears on a compliant server's notification. * fix(sep-2575): keep collected frames when the listen stream read times out listenToStream aborts the fetch after timeoutMs. If the abort fired while blocked in reader.read(), the rejection propagated to the outer catch and the helper returned [] — discarding every frame already received. A compliant server holds a subscriptions/listen stream open indefinitely, so the timeout is the *normal* way these reads end; previously every subscription check would report a false 'no frames received' FAILURE against such a server. The current everything-server only avoided this by res.end()ing immediately after the ack. * fix(sep-2575): open the listen stream before triggering list mutations The notification-filter and list-changed checks fired the mutation trigger *before* opening the subscriptions/listen stream. A compliant server only delivers list-changed notifications to streams that are open at the time of the change, so the SHOULD checks would fail any server that does not replay past changes to new subscribers, and the filter check could never observe a real leak. listenToStream now takes an onFirstFrame hook; the three trigger-based checks open the stream, wait for the acknowledgment, then fire the mutation on a separate connection and keep reading. The everything-server now keeps subscriptions/listen streams open, registers them with their filters, and fans the list_changed notification out to open streams when a trigger tool runs - instead of unconditionally emitting a list-changed frame on every stream open (which itself violated the notification-filter requirement in spirit: it announced a change when nothing had changed). * fix(sep-2575): check the subscription id on every listen-stream notification server-tags-subscription-id only inspected the acknowledgment frame, so a server that tags the ack but omits io.modelcontextprotocol/subscriptionId from subsequent notifications passed. The check now triggers a tool-list change once the stream is acknowledged and asserts every notification frame on the stream carries the id in params._meta. Also drops the f.body.method / f.params.method fallbacks when resolving a frame's method - those shapes don't exist in JSON-RPC and only masked malformed frames. * fix(sep-2575): report WARNING, not FAILURE, for the SHOULD-level list-changed checks The spec text behind server-sends-{tools,prompts}-list-changed-on-subscription is a SHOULD ('servers that declared the listChanged capability SHOULD send a notification...'). Severity follows the spec keyword: MUST -> FAILURE, SHOULD -> WARNING. runCheck now accepts a warning flag and the two list-changed checks use it on their failure paths. * fix(sep-2575): skip stream checks when the diagnostic tool is not exposed http-server-no-independent-requests-on-stream and server-no-log-without-loglevel reported SUCCESS when the server rejected the tools/call outright (e.g. test_streaming_elicitation / test_logging_tool do not exist): a single error frame contains no independent request and no log notification, so the check passed without exercising anything. The sibling list-changed checks already SKIP when their trigger tool is missing; these now do the same. * chore(sep-2575): exclude the four unobservable server requirements from traceability Marks the section-C rows of #296 as excluded rather than untested: no-prior-context, no-connection-reuse-required, disconnect-is-cancel and stops-on-cancel describe internal server state that a black-box harness cannot observe on the wire. With these excluded and the part-B checks landed, every testable server-side SEP-2575 requirement is covered; the remaining untested rows are all client-side and blocked on a SEP-2575-aware reference client. * fix(sep-2575): emit the request-metadata checks even when the client never connects The reference SDK conformance client has no handler registered for the request-metadata scenario, so it exits without sending a request, the scenario emits zero checks, the suite reports "0 passed, 0 failed", and the traceability manifest records all seven client-side requirements as untested. Declare the scenario check IDs up front and backfill any that were never emitted as FAILURE from getChecks(): the emitted ID set is now the same for every client, and a client that never connects fails loudly. A check that is legitimately not applicable opts out by being emitted as SKIPPED explicitly (version-header-matches-meta now does this when either version field is absent). The positive-path test pins the declared list against the reference client in both directions. Also refresh the traceability manifest from a run against typescript-sdk@22595b96: the seven client rows flip to tested and the manifest catches up with the server-side scenarios from #297. --------- Co-authored-by: Yuan Teoh <yuanteoh@google.com>
1 parent 7f1b871 commit 513abdb

3 files changed

Lines changed: 160 additions & 59 deletions

File tree

‎src/scenarios/client/request-metadata.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
} from './auth/test_helpers/testClient';
66
import { getHandler } from '../../../examples/clients/typescript/everything-client';
77
import { getScenario } from '../index';
8+
import { DECLARED_CHECK_IDS } from './request-metadata';
89

910
// A bad client that does not send _meta
1011
async function badClient(serverUrl: string) {
@@ -182,6 +183,65 @@ describe('request-metadata client scenario — positive test', () => {
182183
checks.find((c) => c.id === 'sep-2575-client-retry-supported-version')
183184
?.status
184185
).toBe('SUCCESS');
186+
187+
// Declared ↔ emitted completeness, both directions: every declared check
188+
// is genuinely observed, and every emitted check is declared.
189+
for (const check of checks) {
190+
expect(DECLARED_CHECK_IDS).toContain(check.id);
191+
expect(check.errorMessage ?? '').not.toContain('not observed');
192+
}
193+
expect(new Set(checks.map((c) => c.id))).toEqual(
194+
new Set(DECLARED_CHECK_IDS)
195+
);
196+
});
197+
});
198+
199+
describe('request-metadata client scenario — client never connects', () => {
200+
// A client with no handler for this scenario exits without sending a
201+
// request. Every declared check must still be emitted (as FAILURE) instead
202+
// of reporting "0 passed, 0 failed".
203+
test('emits every declared check as FAILURE when no request is received', async () => {
204+
const scenario = getScenario('request-metadata');
205+
if (!scenario) {
206+
throw new Error('Scenario not found');
207+
}
208+
209+
await scenario.start();
210+
try {
211+
const checks = scenario.getChecks();
212+
const byId = new Map(checks.map((c) => [c.id, c]));
213+
214+
for (const id of DECLARED_CHECK_IDS) {
215+
const check = byId.get(id);
216+
expect(check, `expected check ${id} to be emitted`).toBeDefined();
217+
expect(check?.status, `expected ${id} to be FAILURE`).toBe('FAILURE');
218+
expect(check?.errorMessage).toContain('never sent a request');
219+
}
220+
expect(checks).toHaveLength(DECLARED_CHECK_IDS.length);
221+
} finally {
222+
await scenario.stop();
223+
}
224+
});
225+
226+
test('does not overwrite checks recorded from a real request', async () => {
227+
const runner = new InlineClientRunner(badClient);
228+
await runClientAgainstScenario(runner, 'request-metadata', {
229+
expectedFailureSlugs: [
230+
'sep-2575-client-populates-meta',
231+
'sep-2575-http-client-sends-version-header'
232+
]
233+
});
234+
235+
const scenario = getScenario('request-metadata');
236+
const checks = scenario!.getChecks();
237+
// badClient connects, so its failures must carry the observed request's
238+
// details, not the "never sent a request" backfill.
239+
const populatesMeta = checks.find(
240+
(c) => c.id === 'sep-2575-client-populates-meta'
241+
);
242+
expect(populatesMeta?.errorMessage ?? '').not.toContain(
243+
'never sent a request'
244+
);
185245
});
186246
});
187247

‎src/scenarios/client/request-metadata.ts‎

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,21 @@ import {
66
DRAFT_PROTOCOL_VERSION
77
} from '../../types';
88

9+
/**
10+
* Every check ID this scenario can emit. Declared-but-unemitted checks are
11+
* backfilled as FAILURE by getChecks(), so the emitted ID set is the same for
12+
* every client. The positive-path test asserts this list is exact.
13+
*/
14+
export const DECLARED_CHECK_IDS = [
15+
'sep-2575-http-client-sends-version-header',
16+
'sep-2575-client-populates-meta',
17+
'sep-2575-http-version-header-matches-meta',
18+
'sep-2575-client-declares-roots-capability',
19+
'sep-2575-client-declares-sampling-capability',
20+
'sep-2575-client-declares-elicitation-capability',
21+
'sep-2575-client-retry-supported-version'
22+
] as const;
23+
924
export class RequestMetadataScenario implements Scenario {
1025
name = 'request-metadata';
1126
readonly source = { introducedIn: DRAFT_PROTOCOL_VERSION } as const;
@@ -15,10 +30,12 @@ export class RequestMetadataScenario implements Scenario {
1530
private server: http.Server | null = null;
1631
private checks: ConformanceCheck[] = [];
1732
private hasSimulatedRejection = false;
33+
private requestsObserved = 0;
1834

1935
async start(): Promise<ScenarioUrls> {
2036
this.hasSimulatedRejection = false;
2137
this.checks = [];
38+
this.requestsObserved = 0;
2239
return new Promise((resolve, reject) => {
2340
this.server = http.createServer((req, res) => {
2441
this.handleRequest(req, res);
@@ -46,6 +63,30 @@ export class RequestMetadataScenario implements Scenario {
4663
}
4764

4865
getChecks(): ConformanceCheck[] {
66+
// Declared but never emitted -> FAILURE. A check that is legitimately not
67+
// applicable must be emitted as SKIPPED explicitly to avoid this.
68+
for (const id of DECLARED_CHECK_IDS) {
69+
if (!this.checks.some((c) => c.id === id)) {
70+
this.checks.push({
71+
id,
72+
name: 'NotObserved',
73+
description: `Declared check ${id} was never emitted`,
74+
status: 'FAILURE',
75+
errorMessage:
76+
this.requestsObserved === 0
77+
? 'Check was not observed: the client never sent a request to the scenario server (no handler registered for this scenario?)'
78+
: 'Check was not observed: no request exercised it',
79+
timestamp: new Date().toISOString(),
80+
specReferences: [
81+
{
82+
id: 'SEP-2575',
83+
url: 'https://modelcontextprotocol.io/specification/draft/basic/index#meta'
84+
}
85+
],
86+
details: { observed: false, requestsObserved: this.requestsObserved }
87+
});
88+
}
89+
}
4990
return this.checks;
5091
}
5192

@@ -68,6 +109,7 @@ export class RequestMetadataScenario implements Scenario {
68109
});
69110
req.on('end', () => {
70111
const request = JSON.parse(body);
112+
this.requestsObserved++;
71113

72114
// Extract version and headers
73115
const meta = request.params?._meta;
@@ -117,25 +159,30 @@ export class RequestMetadataScenario implements Scenario {
117159
});
118160

119161
// 3. "The header value MUST match the io.modelcontextprotocol/protocolVersion
120-
// field carried in the request body's _meta." Only meaningful when both
121-
// are present; absence is already covered by the two checks above.
122-
if (headerVersion !== undefined && metaVersion !== undefined) {
123-
this.addOrUpdateCheck({
124-
id: 'sep-2575-http-version-header-matches-meta',
125-
name: 'ClientVersionHeaderMatchesMeta',
126-
description:
127-
'MCP-Protocol-Version header matches _meta.protocolVersion',
128-
status: headerVersion === metaVersion ? 'SUCCESS' : 'FAILURE',
129-
timestamp: new Date().toISOString(),
130-
specReferences: [
131-
{
132-
id: 'SEP-2575',
133-
url: 'https://modelcontextprotocol.io/specification/draft/basic/transports#protocol-version-header'
134-
}
135-
],
136-
details: { headerVersion, metaVersion }
137-
});
138-
}
162+
// field carried in the request body's _meta." Only comparable when both
163+
// are present; absence is already covered by the two checks above, so
164+
// emit SKIPPED rather than falling through to the declared-check failure.
165+
const bothVersionsPresent =
166+
headerVersion !== undefined && metaVersion !== undefined;
167+
this.addOrUpdateCheck({
168+
id: 'sep-2575-http-version-header-matches-meta',
169+
name: 'ClientVersionHeaderMatchesMeta',
170+
description:
171+
'MCP-Protocol-Version header matches _meta.protocolVersion',
172+
status: !bothVersionsPresent
173+
? 'SKIPPED'
174+
: headerVersion === metaVersion
175+
? 'SUCCESS'
176+
: 'FAILURE',
177+
timestamp: new Date().toISOString(),
178+
specReferences: [
179+
{
180+
id: 'SEP-2575',
181+
url: 'https://modelcontextprotocol.io/specification/draft/basic/transports#protocol-version-header'
182+
}
183+
],
184+
details: { headerVersion, metaVersion }
185+
});
139186

140187
// 4. Optional client capabilities conditional verification
141188
const capabilities = meta?.['io.modelcontextprotocol/clientCapabilities'];

‎src/seps/traceability.json‎

Lines changed: 34 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"schemaVersion": 1,
33
"docs": "https://github.com/modelcontextprotocol/conformance/blob/main/AGENTS.md#traceability-manifest",
4-
"source": "typescript-sdk@4e153aef0538",
4+
"source": "typescript-sdk@22595b96",
55
"seps": {
66
"837": {
77
"yaml": "src/seps/sep-837.yaml",
@@ -381,7 +381,7 @@
381381
"requirements": [
382382
{
383383
"check": "sep-2575-client-populates-meta",
384-
"status": "untested",
384+
"status": "tested",
385385
"text": "Every client request MUST include the following io.modelcontextprotocol/* fields in _meta: protocolVersion, clientInfo, clientCapabilities.",
386386
"url": "https://modelcontextprotocol.io/specification/draft/basic/index#meta"
387387
},
@@ -399,20 +399,10 @@
399399
},
400400
{
401401
"check": "sep-2575-server-tags-subscription-id",
402-
"status": "untested",
402+
"status": "tested",
403403
"text": "On notifications delivered via a subscriptions/listen stream, the server MUST include io.modelcontextprotocol/subscriptionId in _meta so the client can correlate the notification with the originating subscription request.",
404404
"url": "https://modelcontextprotocol.io/specification/draft/basic/index#meta"
405405
},
406-
{
407-
"check": "sep-2575-server-stateless-no-prior-context",
408-
"status": "untested",
409-
"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)."
410-
},
411-
{
412-
"check": "sep-2575-server-stateless-no-connection-reuse-required",
413-
"status": "untested",
414-
"text": "Servers MUST NOT require that a client reuse the same connection to perform related operations."
415-
},
416406
{
417407
"check": "sep-2575-server-unsupported-version-error",
418408
"status": "tested",
@@ -421,7 +411,7 @@
421411
},
422412
{
423413
"check": "sep-2575-client-retry-supported-version",
424-
"status": "untested",
414+
"status": "tested",
425415
"text": "The client SHOULD select a mutually supported version from the supported list and retry the request, or surface an error to the user if no compatible version exists.",
426416
"url": "https://modelcontextprotocol.io/specification/draft/basic/lifecycle#protocol-version-negotiation"
427417
},
@@ -433,31 +423,19 @@
433423
},
434424
{
435425
"check": "sep-2575-http-server-no-independent-requests-on-stream",
436-
"status": "untested",
426+
"status": "tested",
437427
"text": "The server MUST NOT send independent JSON-RPC requests on this stream. Server-to-client interactions are embedded as input requests inside an IncompleteResult.",
438428
"url": "https://modelcontextprotocol.io/specification/draft/basic/transports#receiving-messages-1"
439429
},
440-
{
441-
"check": "sep-2575-http-server-disconnect-is-cancel",
442-
"status": "untested",
443-
"text": "Closing the SSE response stream MUST be treated by the server as cancellation of that request.",
444-
"url": "https://modelcontextprotocol.io/specification/draft/basic/transports#cancellation-1"
445-
},
446-
{
447-
"check": "sep-2575-http-server-stops-on-cancel",
448-
"status": "untested",
449-
"text": "The server SHOULD stop work on the cancelled request as soon as practical and MUST NOT send any further messages for it [HTTP].",
450-
"url": "https://modelcontextprotocol.io/specification/draft/basic/transports#cancellation-1"
451-
},
452430
{
453431
"check": "sep-2575-http-client-sends-version-header",
454-
"status": "untested",
432+
"status": "tested",
455433
"text": "Every POST request to the MCP endpoint MUST include an MCP-Protocol-Version header.",
456434
"url": "https://modelcontextprotocol.io/specification/draft/basic/transports#protocol-version-header"
457435
},
458436
{
459437
"check": "sep-2575-http-version-header-matches-meta",
460-
"status": "untested",
438+
"status": "tested",
461439
"text": "The header value MUST match the io.modelcontextprotocol/protocolVersion field carried in the request body _meta.",
462440
"url": "https://modelcontextprotocol.io/specification/draft/basic/transports#protocol-version-header"
463441
},
@@ -481,31 +459,31 @@
481459
},
482460
{
483461
"check": "sep-2575-server-honors-notification-filter",
484-
"status": "untested",
462+
"status": "tested",
485463
"text": "The server MUST NOT send notification types the client has not explicitly requested.",
486464
"url": "https://modelcontextprotocol.io/specification/draft/basic/utilities/subscriptions#opening-a-stream"
487465
},
488466
{
489467
"check": "sep-2575-server-sends-subscription-ack",
490-
"status": "untested",
468+
"status": "tested",
491469
"text": "The server MUST send notifications/subscriptions/acknowledged as the first message on the stream.",
492470
"url": "https://modelcontextprotocol.io/specification/draft/basic/utilities/subscriptions#acknowledgment"
493471
},
494472
{
495473
"check": "sep-2575-client-declares-elicitation-capability",
496-
"status": "untested",
474+
"status": "tested",
497475
"text": "Clients that support elicitation MUST declare the elicitation capability in _meta.io.modelcontextprotocol/clientCapabilities on each request.",
498476
"url": "https://modelcontextprotocol.io/specification/draft/client/elicitation#capabilities"
499477
},
500478
{
501479
"check": "sep-2575-client-declares-roots-capability",
502-
"status": "untested",
480+
"status": "tested",
503481
"text": "Clients that support roots MUST declare the roots capability in _meta.io.modelcontextprotocol/clientCapabilities on each request.",
504482
"url": "https://modelcontextprotocol.io/specification/draft/client/roots#capabilities"
505483
},
506484
{
507485
"check": "sep-2575-client-declares-sampling-capability",
508-
"status": "untested",
486+
"status": "tested",
509487
"text": "Clients that support sampling MUST declare the sampling capability in _meta.io.modelcontextprotocol/clientCapabilities on each request.",
510488
"url": "https://modelcontextprotocol.io/specification/draft/client/sampling#capabilities"
511489
},
@@ -517,24 +495,40 @@
517495
},
518496
{
519497
"check": "sep-2575-server-sends-prompts-list-changed-on-subscription",
520-
"status": "untested",
498+
"status": "tested",
521499
"text": "[A server with the listChanged] capability SHOULD send a notification to clients that have opened a subscriptions/listen stream with promptsListChanged: true.",
522500
"url": "https://modelcontextprotocol.io/specification/draft/server/prompts#list-changed-notification"
523501
},
524502
{
525503
"check": "sep-2575-server-sends-tools-list-changed-on-subscription",
526-
"status": "untested",
504+
"status": "tested",
527505
"text": "[A server with the listChanged] capability SHOULD send a notification to clients that have opened a subscriptions/listen stream with toolsListChanged: true.",
528506
"url": "https://modelcontextprotocol.io/specification/draft/server/tools#list-changed-notification"
529507
},
530508
{
531509
"check": "sep-2575-server-no-log-without-loglevel",
532-
"status": "untested",
510+
"status": "tested",
533511
"text": "The server MUST NOT emit notifications/message for a request that does not include [io.modelcontextprotocol/logLevel in _meta].",
534512
"url": "https://modelcontextprotocol.io/specification/draft/server/utilities/logging#per-request-log-level"
535513
}
536514
],
537515
"excluded": [
516+
{
517+
"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).",
518+
"reason": "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"
519+
},
520+
{
521+
"text": "Servers MUST NOT require that a client reuse the same connection to perform related operations.",
522+
"reason": "not observable from a black-box harness; every harness request already arrives on an independent connection — see https://github.com/modelcontextprotocol/conformance/issues/296"
523+
},
524+
{
525+
"text": "Closing the SSE response stream MUST be treated by the server as cancellation of that request.",
526+
"reason": "\"treated as cancellation\" is internal server state; once the stream is closed there is no channel left on which to observe the effect — see https://github.com/modelcontextprotocol/conformance/issues/296"
527+
},
528+
{
529+
"text": "The server SHOULD stop work on the cancelled request as soon as practical and MUST NOT send any further messages for it [HTTP].",
530+
"reason": "\"stop work as soon as practical\" is unobservable from a black-box harness, and \"no further messages\" cannot be verified once the response stream is closed — see https://github.com/modelcontextprotocol/conformance/issues/296"
531+
},
538532
{
539533
"text": "State that needs to span multiple requests (e.g., long-running tasks, application-level handles) MUST be referenced by an explicit identifier the client passes on each request.",
540534
"reason": "architectural guidance, observable only via subscriptionId/task-id rows already listed"
@@ -587,9 +581,9 @@
587581
"sep-2575-request-meta-invalid-missing-protocol-version"
588582
],
589583
"summary": {
590-
"tested": 8,
591-
"untested": 18,
592-
"excluded": 9,
584+
"tested": 22,
585+
"untested": 0,
586+
"excluded": 13,
593587
"untracked": 11,
594588
"unkeyed": 0
595589
}

0 commit comments

Comments
 (0)