diff --git a/README.md b/README.md index 8fa48afc..d1c9154c 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,24 @@ npx @modelcontextprotocol/conformance server --url [--scenario ] - `checks.json` - Array of conformance check results with pass/fail status +### Wire-schema checks + +Every scenario also validates each JSON-RPC message on the wire against the +spec's JSON schema for the negotiated spec version, and emits up to two +synthetic checks alongside the scenario's own: + +- `wire-schema-valid` - fails when a message _the implementation under test + sent_ violates the spec JSON schema. The failure details include every + violating message and its schema errors. +- `wire-schema-harness-error` - fails when the _harness itself_ sent an + invalid message. This indicates a bug in the conformance suite (or a + deliberately nonconformant fixture), not in the implementation under test; + please report it. + +Scenarios that exchange no instrumented wire traffic (see issue #418) emit +neither check. Like any other check, `wire-schema-valid` can be baselined via +the expected-failures file. + ## Expected Failures SDKs that don't yet pass all conformance tests can specify a baseline of known failures. This allows running conformance tests in CI without failing, while still catching regressions. diff --git a/src/connection/sdk-client.ts b/src/connection/sdk-client.ts index 3928f1a4..d0080504 100644 --- a/src/connection/sdk-client.ts +++ b/src/connection/sdk-client.ts @@ -12,7 +12,7 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; -import { LATEST_SPEC_VERSION, type SpecVersion } from '../types'; +import { type SpecVersion } from '../types'; import { validateWireMessage, type WireOrigin @@ -51,8 +51,8 @@ function instrumentTransport( origin === 'harness' ? harnessRequests : implementationRequests; const peerRequests = origin === 'harness' ? implementationRequests : harnessRequests; - // `send` accepts arrays (2025-03-26 batches); validate each element with - // its own classification, then the batch envelope itself. + // `send` accepts arrays (2025-03-26 batches); walk elements for request/ + // response bookkeeping, validating singles here and batches whole below. for (const m of Array.isArray(message) ? message : [message]) { const msg = (typeof m === 'object' && m !== null ? m : {}) as Record< string, @@ -73,9 +73,14 @@ function instrumentTransport( if (id !== undefined) peerRequests.delete(id); context = `stateful response to '${requestMethod ?? `id ${String(id)}`}'`; } - validateWireMessage(specVersion, m, { origin, context, requestMethod }); + if (!Array.isArray(message)) { + validateWireMessage(specVersion, m, { origin, context, requestMethod }); + } } if (Array.isArray(message)) { + // One whole-array validation covers per-element errors (prefixed [i]) + // and batch legality for the version; validating the elements in the + // loop above too would record every violation twice. validateWireMessage(specVersion, message, { origin, context: 'stateful batch' @@ -119,8 +124,8 @@ function instrumentTransport( */ export async function connectToServer( serverUrl: string, - opts: ConnectOptions = {}, - specVersion: SpecVersion = LATEST_SPEC_VERSION + opts: ConnectOptions, + specVersion: SpecVersion ): Promise { const client = new Client(opts.clientInfo ?? DEFAULT_CLIENT_INFO, { capabilities: opts.capabilities ?? DEFAULT_CAPABILITIES diff --git a/src/connection/select.ts b/src/connection/select.ts index 6e515607..46679005 100644 --- a/src/connection/select.ts +++ b/src/connection/select.ts @@ -46,7 +46,7 @@ export function connectFor( // Pass the version through so requests declare (and are wire-schema // validated against) the spec version the run was invoked with. return isStatefulVersion(specVersion) - ? (serverUrl, opts) => connectStateful(serverUrl, opts, specVersion) + ? (serverUrl, opts) => connectStateful(serverUrl, opts ?? {}, specVersion) : (serverUrl, opts) => connectStateless(serverUrl, specVersion, opts); } diff --git a/src/connection/stateful.ts b/src/connection/stateful.ts index 31286eef..3361f283 100644 --- a/src/connection/stateful.ts +++ b/src/connection/stateful.ts @@ -15,13 +15,13 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import { connectToServer } from './sdk-client'; import type { JSONRPCNotification } from '../spec-types/2025-11-25'; -import { LATEST_SPEC_VERSION, type SpecVersion } from '../types'; +import { type SpecVersion } from '../types'; import { JsonRpcError, type Connection, type ConnectOptions } from './index'; export async function connectStateful( serverUrl: string, - opts: ConnectOptions = {}, - specVersion: SpecVersion = LATEST_SPEC_VERSION + opts: ConnectOptions, + specVersion: SpecVersion ): Promise { // Wire-schema validation happens inside connectToServer, which hooks the // SDK transport's send/onmessage so the real bytes of every message in diff --git a/src/mock-server/mock-server.test.ts b/src/mock-server/mock-server.test.ts index 23cb3ae4..43ecca64 100644 --- a/src/mock-server/mock-server.test.ts +++ b/src/mock-server/mock-server.test.ts @@ -8,7 +8,7 @@ import { CACHEABLE_RESULT_METHODS } from './stateless'; import { STATELESS_SPEC_VERSIONS } from '../connection/select'; -import { DRAFT_PROTOCOL_VERSION } from '../types'; +import { LATEST_SPEC_VERSION, DRAFT_PROTOCOL_VERSION } from '../types'; import { takeWireViolations } from '../validation/wire-schema'; const meta = { @@ -500,9 +500,10 @@ describe('createServerStateful', () => { } it('accepts initialize and routes to handlers, recording non-preamble', async () => { - const srv = await createServerStateful({ - 'tools/list': () => ({ tools: [] }) - }); + const srv = await createServerStateful( + { 'tools/list': () => ({ tools: [] }) }, + LATEST_SPEC_VERSION + ); try { // SDK transport in sessionless mode handles initialize internally; we // can drive it via the SDK Client. @@ -524,9 +525,10 @@ describe('createServerStateful', () => { }); it('derives capabilities from handler keys; non-tools handler does not 500 initialize', async () => { - const srv = await createServerStateful({ - 'prompts/list': () => ({ prompts: [] }) - }); + const srv = await createServerStateful( + { 'prompts/list': () => ({ prompts: [] }) }, + LATEST_SPEC_VERSION + ); try { const { status, contentType } = await postInit(srv.url); expect(status).toBe(200); @@ -537,9 +539,10 @@ describe('createServerStateful', () => { }); it('records requests for unregistered methods (parity with stateless)', async () => { - const srv = await createServerStateful({ - 'tools/list': () => ({ tools: [] }) - }); + const srv = await createServerStateful( + { 'tools/list': () => ({ tools: [] }) }, + LATEST_SPEC_VERSION + ); try { const { Client } = await import('@modelcontextprotocol/sdk/client/index.js'); @@ -565,3 +568,73 @@ describe('createServerStateful', () => { } }); }); + +describe('wire violation attribution', () => { + it('does not blame the harness for the JSON-RPC-mandated id:null on error replies', async () => { + const srv = await createServerStateless({}, DRAFT_PROTOCOL_VERSION); + try { + // An id-less, _meta-less message forces a reject reply carrying + // `id: null` (JSON-RPC 2.0 when the request id cannot be determined). + await post(srv.url, { jsonrpc: '2.0', method: 'tools/call' }, headers); + const { violations } = takeWireViolations(); + expect(violations.filter((v) => v.origin === 'harness')).toEqual([]); + } finally { + await srv.close(); + } + }); + + it('does not record an implementation violation for an unparsed body (wrong Content-Type)', async () => { + const stateless = await createServerStateless({}, DRAFT_PROTOCOL_VERSION); + const stateful = await createServerStateful({}, LATEST_SPEC_VERSION); + try { + for (const url of [stateless.url, stateful.url]) { + await fetch(url, { + method: 'POST', + headers: { 'content-type': 'text/plain' }, + body: '{"jsonrpc":"2.0","id":1,"method":"ping"}' + }); + } + const { violations } = takeWireViolations(); + expect(violations.filter((v) => v.origin === 'implementation')).toEqual( + [] + ); + } finally { + await stateless.close(); + await stateful.close(); + } + }); + + it('validates stateful client traffic against the version the client declares, not the run version', async () => { + const srv = await createServerStateful({}, '2025-11-25'); + try { + const batch = [{ jsonrpc: '2.0', id: 1, method: 'ping' }]; + // Batch arrays are only legal at 2025-03-26; a client that negotiated + // that version declares it on the request. + await post(srv.url, batch, { 'mcp-protocol-version': '2025-03-26' }); + const negotiated = takeWireViolations(); + expect( + negotiated.violations.filter((v) => v.origin === 'implementation') + ).toEqual([]); + + // No header: the header only exists from 2025-06-18, whose spec says + // to assume 2025-03-26 — where the batch is legal. + await post(srv.url, batch); + const headerless = takeWireViolations(); + expect( + headerless.violations.filter((v) => v.origin === 'implementation') + ).toEqual([]); + + await post(srv.url, batch, { 'mcp-protocol-version': '2025-11-25' }); + const runVersion = takeWireViolations(); + expect( + runVersion.violations.some( + (v) => + v.origin === 'implementation' && + v.errors.some((e) => e.includes('batch')) + ) + ).toBe(true); + } finally { + await srv.close(); + } + }); +}); diff --git a/src/mock-server/stateful.ts b/src/mock-server/stateful.ts index 773bca9f..0a80b7f7 100644 --- a/src/mock-server/stateful.ts +++ b/src/mock-server/stateful.ts @@ -13,7 +13,7 @@ import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/ import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import { z } from 'zod'; import type { JSONRPCRequest } from '../spec-types/2025-11-25'; -import { LATEST_SPEC_VERSION, type SpecVersion } from '../types'; +import { isSpecVersion, type SpecVersion } from '../types'; import { validateWireMessage } from '../validation/wire-schema'; import type { MockServer, RequestHandlers } from './index'; @@ -43,7 +43,7 @@ export function capabilitiesFromHandlers( export async function createServerStateful( handlers: RequestHandlers, - specVersion: SpecVersion = LATEST_SPEC_VERSION + specVersion: SpecVersion ): Promise { const recorded: JSONRPCRequest[] = []; const capabilities = capabilitiesFromHandlers(handlers); @@ -100,10 +100,27 @@ export async function createServerStateful( // preamble) at the HTTP layer so unregistered methods are captured too, // matching the stateless impl and the MockServer.recorded contract. const body = req.body; - validateWireMessage(specVersion, body, { - origin: 'implementation', - context: `client request '${body?.method ?? '(unknown)'}' to stateful mock` - }); + // An unparsed body (wrong or missing Content-Type) is a transport fault + // the SDK transport rejects itself, not a JSON-RPC message to validate. + if (body !== undefined) { + // Clients may legitimately negotiate a different protocol version than + // the run's (the README permits ignoring the forwarded --spec-version), + // and they declare it on every post-initialize request; validate their + // traffic against the version they actually speak. + const headerVersion = req.headers['mcp-protocol-version']; + // Absent header: the header only exists from 2025-06-18, whose spec + // says to assume 2025-03-26 — a header-less legacy client is not a + // violation of the run version. + const inboundVersion = isSpecVersion(headerVersion) + ? headerVersion + : headerVersion === undefined + ? '2025-03-26' + : specVersion; + validateWireMessage(inboundVersion, body, { + origin: 'implementation', + context: `client request '${body?.method ?? '(unknown)'}' to stateful mock` + }); + } if ( body?.method && body.method !== 'initialize' && diff --git a/src/mock-server/stateless.ts b/src/mock-server/stateless.ts index 89c4f50a..3d06c86a 100644 --- a/src/mock-server/stateless.ts +++ b/src/mock-server/stateless.ts @@ -202,15 +202,29 @@ export async function createServerStateless( // requests are captured too, matching the stateful impl and the // MockServer.recorded contract. const body = req.body as Record | undefined; - validateWireMessage(wireVersion, req.body, { - origin: 'implementation', - context: `client request '${body?.method ?? '(unknown)'}' to stateless mock` - }); + // An unparsed body (wrong or missing Content-Type) is a transport fault + // surfaced by the HTTP-level checks, not a JSON-RPC message to validate. + if (body !== undefined) { + validateWireMessage(wireVersion, req.body, { + origin: 'implementation', + context: `client request '${body?.method ?? '(unknown)'}' to stateless mock` + }); + } if (body?.method && body.method !== 'server/discover') { recorded.push(req.body as JSONRPCRequest); } const sendJson = (status: number, payload: object, method?: string) => { - validateWireMessage(wireVersion, payload, { + // JSON-RPC 2.0 mandates `id: null` on error replies when the request id + // could not be determined, but the spec's RequestId is string|integer; + // validate with a placeholder id so the mandated null is not recorded + // as a harness violation (mirrors connection/stateless.ts's + // withNullIdCarveOut). + const raw = payload as Record; + const validated = + raw.error !== undefined && raw.id === null + ? { ...raw, id: 0 } + : payload; + validateWireMessage(wireVersion, validated, { origin: 'harness', context: `stateless mock response${method ? ` to '${method}'` : ''}`, requestMethod: method diff --git a/src/types.ts b/src/types.ts index a6e02251..0867e247 100644 --- a/src/types.ts +++ b/src/types.ts @@ -69,6 +69,11 @@ const SPEC_VERSION_TIMELINE: readonly SpecVersion[] = [ DRAFT_PROTOCOL_VERSION ]; +/** True when `v` names a known spec version (dated or draft). */ +export function isSpecVersion(v: unknown): v is SpecVersion { + return SPEC_VERSION_TIMELINE.includes(v as SpecVersion); +} + /** * True when `v` is at or after `threshold` on the spec timeline. Lets a check * gate itself to the version that introduced its requirement (e.g. a draft-only diff --git a/src/validation/wire-schema.test.ts b/src/validation/wire-schema.test.ts index 1c4f763a..98fc4d7b 100644 --- a/src/validation/wire-schema.test.ts +++ b/src/validation/wire-schema.test.ts @@ -1,4 +1,6 @@ import { createServer, type Server } from 'http'; +import rawSchema2025_06_18 from '../spec-types/2025-06-18.schema.json'; +import rawSchema2025_11_25 from '../spec-types/2025-11-25.schema.json'; import { DRAFT_PROTOCOL_VERSION } from '../types'; import { sendStatelessRequest } from '../connection/stateless'; import { @@ -506,4 +508,31 @@ describe('schema errata', () => { ) ).toEqual([]); }); + + it('accepts fractional NumberSchema bounds at 2025-06-18 (same generator bug, no default there)', () => { + expect( + wireSchemaErrors( + '2025-06-18', + elicitWithScore({ type: 'number', minimum: 0.5, maximum: 99.5 }) + ) + ).toEqual([]); + }); + + it('tripwire: delete applySchemaErrata once the vendored schemas are fixed (modelcontextprotocol#3139)', () => { + const numberSchema = (raw: Record, defsKey: string) => + ( + raw[defsKey] as Record< + string, + { properties: Record } + > + ).NumberSchema.properties; + expect( + numberSchema(rawSchema2025_11_25, '$defs').default.type, + 'the re-vendored 2025-11-25 schema no longer needs the erratum — delete its applySchemaErrata branch' + ).toBe('integer'); + expect( + numberSchema(rawSchema2025_06_18, 'definitions').minimum.type, + 'the re-vendored 2025-06-18 schema no longer needs the erratum — delete its applySchemaErrata branch' + ).toBe('integer'); + }); }); diff --git a/src/validation/wire-schema.ts b/src/validation/wire-schema.ts index b7e2b256..8b45849d 100644 --- a/src/validation/wire-schema.ts +++ b/src/validation/wire-schema.ts @@ -75,18 +75,19 @@ interface CompiledSpec { const compiledSpecs = new Map(); -/** Patch known bugs in released (frozen) generated spec schemas at load time - * so validation matches the schema.ts source of truth. Each erratum cites the - * upstream fix; delete it once the dated schema.json is corrected upstream - * and re-vendored. */ +/** Patch known generator bugs in released spec schema.json files at load time + * so validation matches the schema.ts source of truth; delete each branch (a + * test trips) once the dated schema is fixed upstream and re-vendored. */ function applySchemaErrata( specVersion: SpecVersion, schema: Record ): Record { - if (specVersion !== '2025-11-25') return schema; - // 2025-11-25 generates NumberSchema minimum/maximum/default as `integer`, - // contradicting its own schema.ts (`default?: number`). Fixed for draft in - // modelcontextprotocol#2710, which left the released schema.json as-is. + if (specVersion !== '2025-11-25' && specVersion !== '2025-06-18') { + return schema; + } + // NumberSchema minimum/maximum (plus default at 2025-11-25) are generated + // as `integer`, contradicting schema.ts (`number`). Fixed for draft in + // modelcontextprotocol#2710; dated fixes proposed in modelcontextprotocol#3139. const patched = structuredClone(schema); const defs = (patched.$defs ?? patched.definitions) as Record< string,