Validate all wire messages against the per-version spec JSON schema (#399)
* Vendor schema/{version}/schema.json alongside schema.ts
sync-schema now copies the spec repo's JSON Schema for each version into
src/spec-types/{version}.schema.json (same pinned SHA recorded in SOURCE:
modelcontextprotocol@f817239f4d6b1efff2c4dfc2f7af85c985d73076). These are
the inputs for runtime wire-message validation. Vendored verbatim, so they
join schema.ts in .prettierignore.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
* Validate all wire messages against the per-version spec JSON schema
Every JSON-RPC message the harness sends or receives is now validated
(ajv) against the vendored schema.json for the run's spec version, at the
wire choke points: sendStatelessRequest (outbound request, JSON response,
SSE events), connectStateful (request/result/notifications), and both mock
servers (inbound client requests, outbound responses/handler results).
Beyond the JSONRPCMessage envelope, messages are validated against the
most specific definition the schema has: typed requests/notifications by
method const, typed error responses by error.code const (e.g. -32021 ->
MissingRequiredClientCapabilityError, the #376 hallucination), and typed
results via the XxxRequest -> XxxResult pair, with SEP-2322
InputRequiredResult discriminated on resultType.
Violations are recorded per scenario and surfaced by the runners as a
synthetic wire-schema-valid check (FAILURE when the implementation under
test sent an invalid message) plus a wire-schema-harness-error check when
the invalid message was harness-authored. A vitest-wide afterEach guard
fails any self-test that recorded violations, so hallucinated fixtures
can't ship (#376, #271). Opt-outs are explicit and greppable: per-call
skipValidation for deliberately malformed traffic, takeWireViolations()
drains in tests that exercise deliberately broken fixtures.
Offenders the new validation caught, now fixed:
- everything-server stateless path omitted resultType (and the caching
hints on server/discover) required by the draft schema
- sep-2164-empty-contents fixture omitted resultType/ttlMs/cacheScope
(its intended violation, empty contents, is schema-valid prose-only)
- connection/mock-server unit-test fixtures used schema-invalid stubs
(tools/call without name, ListToolsResult without required members,
Tool without inputSchema, ProgressNotification without progressToken,
ElicitRequest without message/requestedSchema)
- http-standard-headers test mocks returned bare { tools: [] } results
- input-required-result-validate-input deliberately sends malformed
inputResponses; now marked skipValidation
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
* Validate the stateful wire at the SDK transport, close review-panel gaps
Address the review-panel findings on the wire-schema validation PR:
- Stateful wire: hook the SDK transport inside connectToServer, wrapping
send/onmessage so every raw wire message in both directions is
validated for the scenario's spec version -- including the initialize
handshake, server->client requests (elicitation/sampling), and the
harness's responses to them. Request ids are correlated per direction
so responses validate against their typed result definition (e.g. an
outbound ElicitResult). The reconstructed stand-ins in
connectStateful (fake id: 0 envelopes, Zod-reparsed results) are
removed; the hook sees the real bytes. The four scenarios that call
connectToServer directly (elicitation-defaults, elicitation-enums,
tools, lifecycle) now pass ctx.specVersion and get validation for
free; the runner surfaces their violations through the existing
recorder. Known remaining gap (documented in the module doc): the
client-auth scenarios' bespoke express mock is not instrumented.
- skipValidation now skips only the request direction; the
implementation's response to a deliberately malformed request is
still validated, with a narrow carve-out tolerating the JSON-RPC 2.0
id: null an error response must carry when the request could not be
processed (the MCP schema's RequestId forbids null). Covered by new
unit tests pinning both the still-validated response and the
narrowness of the carve-out.
- negative-mrtr.test.ts: the file-wide drain now asserts every recorded
violation is implementation-origin (the deliberately broken fixture)
and throws on any harness-origin violation.
- everything-server: hoist the cacheable-methods set to module scope
and replace the per-request res.json monkey-patch with an explicit
sendStatelessJson(res, method, payload) helper applied at each
dispatch site.
- Pin the extracted dispatch maps (methodDefs/errorDefs, e.g.
-32021 -> MissingRequiredClientCapabilityError under the draft) per
spec version in wire-schema.test.ts, so a schema sync that breaks the
const extraction fails loudly instead of degrading to envelope-only
validation. Also note the batch branch's requestMethod forwarding
limitation.
Offender the transport hook flushed out: the everything-server's
test_elicitation_sep1034_defaults tool sent a number-typed elicitation
field with default: 95.5, which the 2025-11-25 schema forbids
(NumberSchema.default is typed integer there; widened to number only in
the draft). The fixture and the scenario's expected default are now the
integer-valued 95, with comments explaining why.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
* Condense comments to three lines max
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
* Re-vendor spec schemas at pinned SHA 71e30695
Regenerate src/spec-types/*.schema.json via npm run sync-schema at the
SHA already recorded in SOURCE. The previously vendored draft schema
predated spec PR #3002, so it still required DiscoverResult.serverInfo
at the top level; the mock server now correctly reports serverInfo via
result _meta, which the stale schema rejected.
* Scope the wire-violation recorder per scenario for parallel suite runs
Client --suite runs execute scenarios concurrently over what was a
module-global violation recorder, so scenarios could steal or wipe each
other's recorded violations. Give each suite scenario its own recorder
via AsyncLocalStorage; code outside a scope (single-scenario runs, the
sequential server suite, vitest hooks) falls back to the global
recorder. Also condense a four-line comment in connection/stateful.ts
to three lines.
* Patch the 2025-11-25 NumberSchema generator bug as a load-time erratum
The released 2025-11-25 schema.json types NumberSchema
minimum/maximum/default as integer, contradicting its own schema.ts
(default?: number) — a typescript-json-schema artifact, fixed for draft
in modelcontextprotocol#2710 and proposed for 2025-11-25 in
modelcontextprotocol#3139. Patch the schema at load time so SDK
fixtures with fractional number defaults (all tier-1 SDKs send 95.5)
are not flagged; restore the sep-1034 scenario and example-server
fixture to 95.5. Delete the erratum when #3139 lands and the schemas
are re-vendored.
Also scope the sequential server-suite loop with withWireRecorder so a
connection leaked by a failed scenario cannot attribute late traffic to
the next scenario.
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Paul Carleton <paulc@anthropic.com>
This commit is contained in:
parent
2a705a811e
commit
89c2c0452e
10 changed files with 14136 additions and 129 deletions
|
|
@ -46,7 +46,7 @@ Implement a tool named \`test_elicitation_sep1034_defaults\` (no arguments) that
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
const connection = await connectToServer(serverUrl);
|
||||
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
|
||||
|
||||
let capturedRequest: any = null;
|
||||
connection.client.setRequestHandler(
|
||||
|
|
|
|||
|
|
@ -47,7 +47,7 @@ Implement a tool named \`test_elicitation_sep1330_enums\` (no arguments) that re
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
const connection = await connectToServer(serverUrl);
|
||||
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
|
||||
|
||||
let capturedRequest: any = null;
|
||||
connection.client.setRequestHandler(
|
||||
|
|
|
|||
|
|
@ -56,6 +56,9 @@ describe('http-custom-header-server-validation — missing fixture policy', () =
|
|||
jsonrpc: '2.0',
|
||||
id: reqBody.id,
|
||||
result: {
|
||||
resultType: 'complete',
|
||||
ttlMs: 0,
|
||||
cacheScope: 'private',
|
||||
tools: [
|
||||
{
|
||||
name: 'plain_tool',
|
||||
|
|
@ -99,6 +102,9 @@ describe('http-custom-header-server-validation — missing fixture policy', () =
|
|||
jsonrpc: '2.0',
|
||||
id: reqBody.id,
|
||||
result: {
|
||||
resultType: 'complete',
|
||||
ttlMs: 0,
|
||||
cacheScope: 'private',
|
||||
tools: [
|
||||
{
|
||||
name: 'numeric_only',
|
||||
|
|
@ -169,7 +175,16 @@ describe('http-header-validation — zero-tools Mcp-Name cases', () => {
|
|||
if (body.method === 'tools/list') {
|
||||
return {
|
||||
status: 200,
|
||||
body: { jsonrpc: '2.0', id: body.id, result: { tools: [] } }
|
||||
body: {
|
||||
jsonrpc: '2.0',
|
||||
id: body.id,
|
||||
result: {
|
||||
resultType: 'complete',
|
||||
ttlMs: 0,
|
||||
cacheScope: 'private',
|
||||
tools: []
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -22,9 +22,12 @@ import { sendStatelessRequest, JsonRpcResponse } from '../../connection';
|
|||
export async function sendRpc(
|
||||
serverUrl: string,
|
||||
method: string,
|
||||
params?: Record<string, unknown>
|
||||
params?: Record<string, unknown>,
|
||||
options?: { skipValidation?: boolean }
|
||||
): Promise<JsonRpcResponse> {
|
||||
const response = await sendStatelessRequest(serverUrl, method, params);
|
||||
const response = await sendStatelessRequest(serverUrl, method, params, {
|
||||
skipValidation: options?.skipValidation
|
||||
});
|
||||
if (!response.body) {
|
||||
throw new Error(
|
||||
`Expected a JSON-RPC response for ${method}, got HTTP ${response.status} (${response.contentType ?? 'no content-type'})`
|
||||
|
|
|
|||
|
|
@ -1567,14 +1567,21 @@ and return a JSON-RPC error or a new InputRequiredResult.`;
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
// Send inputResponses with invalid structure (number instead of object for the response)
|
||||
const resp = await sendRpc(serverUrl, 'tools/call', {
|
||||
name: 'test_input_required_result_elicitation',
|
||||
arguments: {},
|
||||
inputResponses: {
|
||||
user_name: 12345 as unknown as Record<string, unknown>
|
||||
}
|
||||
});
|
||||
// Send inputResponses with invalid structure (number instead of object
|
||||
// for the response). Deliberately schema-invalid, so wire-schema
|
||||
// validation is skipped for this call.
|
||||
const resp = await sendRpc(
|
||||
serverUrl,
|
||||
'tools/call',
|
||||
{
|
||||
name: 'test_input_required_result_elicitation',
|
||||
arguments: {},
|
||||
inputResponses: {
|
||||
user_name: 12345 as unknown as Record<string, unknown>
|
||||
}
|
||||
},
|
||||
{ skipValidation: true }
|
||||
);
|
||||
|
||||
const validateErrors: string[] = [];
|
||||
|
||||
|
|
@ -1599,12 +1606,18 @@ and return a JSON-RPC error or a new InputRequiredResult.`;
|
|||
details: { result: resp.result, error: resp.error }
|
||||
});
|
||||
|
||||
// Also test: send completely malformed inputResponses (null)
|
||||
const resp2 = await sendRpc(serverUrl, 'tools/call', {
|
||||
name: 'test_input_required_result_elicitation',
|
||||
arguments: {},
|
||||
inputResponses: null as unknown as Record<string, unknown>
|
||||
});
|
||||
// Also test: send completely malformed inputResponses (null).
|
||||
// Deliberately schema-invalid; skip wire-schema validation.
|
||||
const resp2 = await sendRpc(
|
||||
serverUrl,
|
||||
'tools/call',
|
||||
{
|
||||
name: 'test_input_required_result_elicitation',
|
||||
arguments: {},
|
||||
inputResponses: null as unknown as Record<string, unknown>
|
||||
},
|
||||
{ skipValidation: true }
|
||||
);
|
||||
|
||||
const protocolErrors: string[] = [];
|
||||
|
||||
|
|
|
|||
|
|
@ -31,7 +31,11 @@ describe('ServerInitializeScenario', () => {
|
|||
testContext(serverUrl)
|
||||
);
|
||||
|
||||
expect(connectToServer).toHaveBeenCalledWith(serverUrl);
|
||||
expect(connectToServer).toHaveBeenCalledWith(
|
||||
serverUrl,
|
||||
{},
|
||||
testContext(serverUrl).specVersion
|
||||
);
|
||||
expect(closeMock).toHaveBeenCalled();
|
||||
expect(fetchMock).toHaveBeenCalledWith(
|
||||
serverUrl,
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ and validates session ID format if one is assigned.`;
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
const connection = await connectToServer(serverUrl);
|
||||
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
|
||||
|
||||
// The connection process already does initialization
|
||||
// Check that we have a connected client
|
||||
|
|
|
|||
|
|
@ -15,6 +15,25 @@ import {
|
|||
InputRequiredResultUnsupportedMethodsScenario,
|
||||
InputRequiredResultTamperedStateScenario
|
||||
} from './input-required-result';
|
||||
import {
|
||||
formatWireViolation,
|
||||
takeWireViolations
|
||||
} from '../../validation/wire-schema';
|
||||
|
||||
// The broken fixture violates the draft schema by design; drain the wire-schema recorder
|
||||
// so the suite-wide guard doesn't re-flag it. Only the *implementation* may be invalid —
|
||||
// a harness-origin violation is a real harness bug and must still fail.
|
||||
afterEach(() => {
|
||||
const { violations } = takeWireViolations();
|
||||
const harnessViolations = violations.filter((v) => v.origin === 'harness');
|
||||
if (harnessViolations.length > 0) {
|
||||
throw new Error(
|
||||
'Harness-origin wire-schema violations in an MRTR negative test ' +
|
||||
'(only the broken fixture may be invalid here):\n ' +
|
||||
harnessViolations.map(formatWireViolation).join('\n ')
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
function getFreePort(): Promise<number> {
|
||||
return new Promise((resolve, reject) => {
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
sep2106KeywordCheckStatus
|
||||
} from './json-schema-2020-12';
|
||||
import { DRAFT_PROTOCOL_VERSION, LATEST_SPEC_VERSION } from '../../types';
|
||||
import { takeWireViolations } from '../../validation/wire-schema';
|
||||
|
||||
function startServer(scriptPath: string, port: number): Promise<ChildProcess> {
|
||||
return new Promise((resolve, reject) => {
|
||||
|
|
@ -158,6 +159,10 @@ describe('Server scenario negative tests', () => {
|
|||
expect(check).toBeDefined();
|
||||
expect(check?.status).toBe('FAILURE');
|
||||
}
|
||||
|
||||
// This fixture's whole point is omitting the caching hints the draft
|
||||
// schema requires, so its responses are wire-schema-invalid by design.
|
||||
expect(takeWireViolations().violations.length).toBeGreaterThan(0);
|
||||
}, 15000);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -485,7 +485,7 @@ Implement tool \`test_tool_with_logging\` with no arguments.
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
const connection = await connectToServer(serverUrl);
|
||||
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
|
||||
const notifications = new NotificationCollector(connection.client);
|
||||
|
||||
// Set logging level to debug
|
||||
|
|
@ -813,7 +813,7 @@ Implement tool \`test_sampling\` with argument:
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
const connection = await connectToServer(serverUrl);
|
||||
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
|
||||
|
||||
let samplingRequested = false;
|
||||
connection.client.setRequestHandler(
|
||||
|
|
@ -950,7 +950,7 @@ Implement tool \`test_elicitation\` with argument:
|
|||
const checks: ConformanceCheck[] = [];
|
||||
|
||||
try {
|
||||
const connection = await connectToServer(serverUrl);
|
||||
const connection = await connectToServer(serverUrl, {}, ctx.specVersion);
|
||||
|
||||
let elicitationRequested = false;
|
||||
connection.client.setRequestHandler(
|
||||
|
|
|
|||
Loading…
Reference in a new issue