Port the end-to-end test suite to v2 - #2179
Conversation
Sorry, something went wrong.
Unmodified copy of test/e2e from the v1.x e2e suite (requirements manifest, verifies()-style scenario tests, helpers, fixtures). Does not type-check or run against this branch yet; import paths and v2 API adaptation follow in separate commits.
Map the old deep-path imports onto the v2 workspace packages (client, server, core, node, express), apply the v1-to-v2 codemod renames, and add the test/e2e workspace wiring (package.json, tsconfig, vitest and eslint config).
Fix the remaining type and lint errors against the v2 packages, update the manifest for v2 behavior changes (knownFailures added, removed, or reworded), and defer requirements whose v1 surface no longer exists (legacy McpServer overloads, the bundled OAuth authorization server, callTool result schemas). Rewrite the suite CLAUDE.md for the manifest/verifies structure and pnpm commands.
Cover Standard Schema and fromJsonSchema validation, the Hono, Fastify and Express hosting adapters, custom method handlers, the structured handler context, the new error hierarchy, AuthProvider on the HTTP client transport, reconnection scheduling, protocol version and capability options, and failed task results. New requirements are recorded in the manifest with knownFailures where the SDK does not yet meet the documented behavior.
🦋 Changeset detectedLatest commit: 04cc4ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Sorry, something went wrong.
|
@modelcontextprotocol/client
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2179
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2179
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2179
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2179
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2179
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2179
npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2179 commit: 04cc4ff |
Sorry, something went wrong.
| * `onmessage` (validated as the counterpart) is asserted. Returns the same | ||
| * transport instance (monkey-patched in place). | ||
| */ | ||
| export function sniffTransport<T extends Transport>(transport: T, party: WireParty, opts: SnifferOptions = {}): T { |
There was a problem hiding this comment.
The point of this is to ensure in every e2e test that we're actually sending MCP valid messages over the transport and not randomly introduce new ways for client and server to interact.
It's more of a "defense in depth" idea to lock in that client and server communicate via the transport and we don't inadvertently introduce something non-compliant that's typescript specific for the two to communicate.
Sorry, something went wrong.
| export function verifies(id: string | readonly string[], fn: TestBody, opts?: { title?: string }): void { | ||
| const ids = Array.isArray(id) ? id : [id]; | ||
| for (const rid of ids) registerOne(rid, fn, opts); | ||
| } |
There was a problem hiding this comment.
This is a small wrapper around a standard vitest.test so we can pass args to it and associate every e2e test with a requirement listed in requirements.ts
Sorry, something went wrong.
| export async function wire(transport: Transport, makeServer: ServerFactory, client: Client, sniff: SnifferOptions = {}): Promise<Wired> { | ||
| switch (transport) { | ||
| case 'inMemory': { | ||
| const server = makeServer(); | ||
| const [clientTx, serverTx] = InMemoryTransport.createLinkedPair(); | ||
| await server.connect(serverTx); | ||
| await client.connect(sniffTransport(clientTx, 'client', sniff)); | ||
| return { [Symbol.asyncDispose]: () => Promise.all([client.close(), server.close()]).then(() => {}) }; | ||
| } | ||
| case 'stdio': { | ||
| const server = makeServer(); | ||
| const c2s = new PassThrough(); | ||
| const s2c = new PassThrough(); | ||
| await server.connect(new StdioServerTransport(c2s, s2c)); | ||
| await client.connect(sniffTransport(stdioClientOverPipes(s2c, c2s), 'client', sniff)); | ||
| return { [Symbol.asyncDispose]: () => Promise.all([client.close(), server.close()]).then(() => {}) }; | ||
| } | ||
| case 'streamableHttp': | ||
| case 'streamableHttpStateless': { | ||
| const handle = transport === 'streamableHttpStateless' ? hostStateless(makeServer) : hostPerSession(makeServer); | ||
| const url = new URL('http://in-process/mcp'); | ||
| const fetch = (u: URL | string, init?: RequestInit) => handle.handleRequest(new Request(u, init)); | ||
| await client.connect(sniffTransport(new StreamableHTTPClientTransport(url, { fetch }), 'client', sniff)); | ||
| return { | ||
| fetch, | ||
| url, | ||
| [Symbol.asyncDispose]: () => Promise.all([client.close(), handle.close()]).then(() => {}) | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This is the most important file to ensure that what we set up in an e2e test is representative of how actual SDK consumers would use it.
Rather than setup just a server instance and hit it directly, we go the full loop of defining a server, defining a client, defining a transport (just like a consumer would) and then wire() puts them together for the test suite.
Sorry, something went wrong.
| handleRequest: async req => { | ||
| const sid = req.headers.get('mcp-session-id') ?? undefined; | ||
| const existing = sid ? sessions.get(sid) : undefined; | ||
| if (existing) return existing.handleRequest(req); | ||
| if (sid !== undefined) { | ||
| // Mirror the SDK's documented hosting pattern: an unrecognized session id is | ||
| // rejected at the app level, so the transport's own 404 is never reached. | ||
| return Response.json( | ||
| { | ||
| jsonrpc: '2.0', | ||
| error: { code: -32_000, message: 'Bad Request: No valid session ID provided' }, | ||
| id: null | ||
| }, | ||
| { status: 400, headers: { 'Content-Type': 'application/json' } } | ||
| ); | ||
| } | ||
|
|
||
| const tx = new WebStandardStreamableHTTPServerTransport({ | ||
| sessionIdGenerator: randomUUID, | ||
| onsessioninitialized: id => void sessions.set(id, tx), | ||
| onsessionclosed: id => void sessions.delete(id) | ||
| }); | ||
| await makeServer().connect(tx); | ||
| return tx.handleRequest(req); | ||
| }, | ||
| close: async () => { | ||
| for (const t of sessions.values()) await t.close(); | ||
| sessions.clear(); | ||
| } |
There was a problem hiding this comment.
This is basically the pattern we have in docs and how MCP servers "should" be served currently if you do it with sessions.
Create a session storage somewhere outside the handler, then look up if there's an existing session. If there is one, use the stored transport, otherwise instantiate a new one for the request.
The way to instantiate a server is a serverFactory, not a raw server instance.
Sorry, something went wrong.
| handleRequest: async req => { | ||
| if (req.method !== 'POST') { | ||
| return Response.json( | ||
| { jsonrpc: '2.0', error: { code: -32_000, message: 'Method not allowed.' }, id: null }, | ||
| { | ||
| status: 405, | ||
| headers: { 'Content-Type': 'application/json' } | ||
| } | ||
| ); | ||
| } | ||
| const server = makeServer(); | ||
| const tx = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: undefined }); | ||
| await server.connect(tx); | ||
| cleanups.push(async () => { | ||
| await server.close(); | ||
| await tx.close(); | ||
| }); | ||
| return tx.handleRequest(req); |
There was a problem hiding this comment.
This is the "pseudo-stateless" hosting pattern without storing sessionId anywhere - on every request, instantiate a transport, connect server, handle, throw everything away.
Sorry, something went wrong.
| /** Transports with a persistent server instance / standalone notification stream. */ | ||
| const STATEFUL_TRANSPORTS = ['inMemory', 'stdio', 'streamableHttp'] as const; | ||
|
|
||
| export const REQUIREMENTS: Record<string, Requirement> = { |
There was a problem hiding this comment.
This file is pure data - the idea is for this to be a contract for what behaviors the SDK actually has that are consumer observable and that a consumer might depend on.
A lot of it is sourced from spec, some of it are SDK specific things we have in the SDK like throwing errors, failing certain methods if something is missing etc.
The idea here is that as much as possible this file describes the current behavior of our SDK, as observable by consumers for their use cases.
Then, building a new feature or adding support for a new spec becomes 1 or more requirements that first land here, while ensuring everything else that's accumulated still passes.
Sorry, something went wrong.
| @@ -0,0 +1,97 @@ | |||
| /** | |||
There was a problem hiding this comment.
This coverage test just makes sure that every requirement in requirements.ts is:
- covered by a test OR
- has a knownFailure with a failing test OR
- is explicitly deferred with a note explaining way.
AND
- there are no tests in scenarios/ with slugs that don't match a requirement in reuqirements.ts
Sorry, something went wrong.
| * (stdin EOF → SIGTERM → SIGKILL) is observable. | ||
| */ | ||
|
|
||
| /* eslint-disable unicorn/no-process-exit -- standalone spawned executable; exit codes are the behavior under test */ |
There was a problem hiding this comment.
one-off fixture needed to specifically test stdio spawning behavior, not something we can easily test another way.
Sorry, something went wrong.
There was a problem hiding this comment.
I didn't find any bugs in the automated review pass, but this is a large addition (a new e2e workspace package across 46 files plus a CI job split), and the knownFailure notes recording v1→v2 behavior differences are explicitly framed as discussion points — those judgment calls need a maintainer's review.
Extended reasoning...Overview
This PR ports the end-to-end behavior test suite from the v1.x branch into the v2 monorepo as a new @modelcontextprotocol/test-e2e workspace package (~46 files): a pure-data requirements manifest, a verifies() test wrapper, a wire-format sniffer, hosting helpers, 31 scenario files spanning all transports and the Hono/Fastify/Express adapters, plus a .github/workflows/main.yml change that splits the e2e suite into its own CI job and excludes it from the existing test job's pnpm -r filter.
Security risks
Low. The change is test-only — no production package code is modified. The CodeQL "missing rate limiting" findings target test fixtures with mock authorization handlers, not shipped code. The CI workflow change only adds a job mirroring the existing test job's setup; it does not alter permissions, secrets, or publish steps. Test fixtures that intentionally simulate misbehaving servers (ignoring SIGTERM, writing garbage to stdout) are confined to the e2e fixtures directory.
Level of scrutiny
Moderate despite being test-only. The suite is intended to act as a behavior contract for the SDK going forward, so the manifest's knownFailures entries (e.g. tool validation errors surfacing as isError results rather than JSON-RPC errors, stateless transport reuse no longer rejected, SdkHttpError vs UnauthorizedError after a second 401) encode judgments about which v2 behaviors are intentional — the author explicitly calls these discussion points. A maintainer also needs to weigh the CI-job split, the new workspace package surface, and whether the documented hosting patterns in the helpers match what the SDK recommends.
Other factors
The automated bug-hunting system reported no bugs. The author reports 984 test cells passing locally with 68 documented expected failures, and the package's typecheck/lint are clean. The author left several explanatory inline comments on their own PR but there are no unaddressed reviewer comments. Given the size, the contract-defining nature of the manifest, and the open behavior-change questions, this is not a candidate for shadow approval.
Sorry, something went wrong.
Ports the end-to-end behavior test suite from the v1.x branch (#2167) to the v2 monorepo and extends it with coverage for v2-only features. The suite lives at test/e2e as the @modelcontextprotocol/test-e2e workspace package: requirements.ts is a pure-data manifest of behaviors (each with a spec or SDK source), scenario files cite the requirement ids they prove via verifies(), and every requirement runs across the in-memory, stdio, and Streamable HTTP (stateful + stateless) transports.
The branch is structured as four commits so the port is reviewable step by step:
The manifest also serves as a v1→v2 behavior-change record: five v1.x known failures now pass on v2 (unknown tool / invalid params / unknown resource error codes, elicitation schema validation, sampling tool_result validation), and the port records the places where v2 behaves differently as knownFailures with notes — notably that tool input/output validation errors are returned as isError results rather than JSON-RPC errors, that the stateless transport no longer rejects reuse across requests, and that a second 401 after AuthProvider.onUnauthorized() surfaces as SdkHttpError rather than the documented UnauthorizedError. Those notes are intended as discussion points, not rulings.
Motivation and Context
Locks down the protocol-visible behavior of the SDK across the v1→v2 migration so that intentional behavior changes are explicit (recorded in the manifest) and unintentional ones fail CI. Mirrors the v1.x suite (#2167) so the two branches share requirement ids and can be compared directly.
How Has This Been Tested?
pnpm --filter @modelcontextprotocol/test-e2e test: 984 cells across 31 scenario files — 916 pass, 68 expected failures (documented knownFailures), 0 unexpected, in ~20s. typecheck and lint for the package are clean. Because test/e2e is a workspace package, repo-level pnpm test:all / typecheck:all / lint:all include it automatically; no dedicated CI job has been added yet.
Breaking Changes
None — test-only addition.
Types of changes
Checklist
Additional context