feat(core): 3-arg setRequestHandler/setNotificationHandler with validator-agnostic paramsSchema

[felixweinberger]

Part of the v2 BC series — see reviewer guide. Stacks on #1891.

Adds the 3-arg form: setRequestHandler('vendor/x', paramsSchema, handler) where paramsSchema is any Standard Schema (Zod/Valibot/ArkType). Ergonomic alternative to writing a full Zod request schema with method literal.

Motivation and Context

#1891 restores the v1 ZodSchema form so existing code keeps working. This PR adds a validator-agnostic 3-arg ergonomic on top: pass the method string and a paramsSchema, handler receives validated params (after _meta stripped, absent params normalized to {}).

How Has This Been Tested?

packages/core/test/shared/threeArgHandlers.test.ts — round-trip, invalid-params rejection, {} normalization, hand-rolled non-Zod StandardSchema.

Breaking Changes

None — additive.

Types of changes

• New feature (non-breaking change which adds functionality)

Additional context

Stacks on #1891 (fweinberger/protocol-concrete).

[changeset-bot]

🦋 Changeset detected

Latest commit: 1c4736b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/client	Minor
@modelcontextprotocol/server	Minor
@modelcontextprotocol/express	Major
@modelcontextprotocol/fastify	Major
@modelcontextprotocol/hono	Major
@modelcontextprotocol/node	Major

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@1916

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@1916

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@1916

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@1916

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@1916

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@1916

commit: 1c4736b

[felixweinberger]

@claude review

[claude]

Implementation looks sound and the two inline notes are nits, but since this adds a new public overload (and exports StandardSchemaV1) it's an API-surface decision that's worth a maintainer signing off on rather than me auto-approving.

Extended reasoning...

Overview

Adds a third call form to setRequestHandler/setNotificationHandler on Protocol (and re-declared on Client/Server): (method: string, paramsSchema: StandardSchemaV1, handler). Handler receives validated params with _meta stripped and absent params normalized to {}. Also relaxes validateStandardSchema's generic bound from StandardSchemaWithJSON to StandardSchemaV1 (a widening), adds parseStandardSchema, exports the StandardSchemaV1 type publicly, updates both migration docs and both custom-method examples, and adds a new test file. 11 files touched.

Security risks

None identified — no auth, transport, or I/O changes; validation failures correctly map to ProtocolErrorCode.InvalidParams.

Level of scrutiny

This is a new public API overload plus a new public type export, which per this repo's review conventions ("burden of proof is on addition", "one way to do things") is exactly the category that should get human design sign-off. The implementation itself is straightforward and the dispatch logic (Zod-schema vs 2-arg vs 3-arg via maybeHandler === undefined) reads correctly, but whether to ship a third registration form alongside the existing string-method and Zod-schema forms is a design call I shouldn't make unilaterally.

Other factors

• Part of the v2-bc series stacking on #1891; author explicitly requested review.
• Tests cover round-trip, invalid-params rejection, and a hand-rolled non-Zod Standard Schema; the two inline comments are nits (a thin wrapper and a test-name/coverage gap), not blockers.
• Client/Server only override setRequestHandler, so the 3-arg setNotificationHandler correctly inherits from Protocol — verified that's intentional and works.
• Changeset and docs accurately describe the behavior.

[felixweinberger]

@claude review

[claude]

No new findings beyond the two earlier nits. This adds a third public overload to setRequestHandler/setNotificationHandler and exports StandardSchemaV1 — given the repo's bar for API-surface additions, a maintainer should sign off on the design.

Extended reasoning...

Overview

Adds a 3-arg (method, paramsSchema, handler) overload to Protocol.setRequestHandler/setNotificationHandler (and the Client/Server overrides), where paramsSchema is any Standard Schema. Widens validateStandardSchema's constraint from StandardSchemaWithJSON to StandardSchemaV1, adds an internal parseStandardSchema wrapper, exports the StandardSchemaV1 type from the public barrel, updates both migration docs and both custom-method examples, and adds a dedicated test file. Part of the v2-BC series stacked on #1891.

Security risks

None identified. The new path validates user-supplied params with the caller's own schema and throws ProtocolError(InvalidParams) on failure — same surface as the existing forms. No auth, transport, or serialization changes.

Level of scrutiny

Medium-high. The implementation is small and the dispatch logic (isZodLikeSchema → 3-arg → 2-arg) is straightforward and covered by tests. But this is a public API addition that introduces a parallel way to register custom-method handlers alongside the existing Zod-schema form, and adds a new public type export. Per this repo's review conventions ("burden of proof is on addition", "one way to do things"), API-shape decisions like this should get explicit maintainer approval rather than bot auto-approval — even though it's part of a planned series.

Other factors

• Nothing has changed since my prior pass at 9da988e; the two 🟡 nits (the parseStandardSchema round-trip wrapper and the vacuous _meta-stripping test name) remain open but are non-blocking.
• The changeset only bumps client/server; @modelcontextprotocol/core also gains a public export (StandardSchemaV1) — worth a quick check that omitting a core entry is intentional.
• Overload ordering looks correct: a Zod schema passed as arg 1 is routed to the compat path before the 3-arg StandardSchema branch, so no ambiguity between the deprecated and new forms.

[felixweinberger]

@claude review

[claude]

No correctness issues found (the two prior nits are optional), but this adds a new public overload to setRequestHandler/setNotificationHandler plus a new StandardSchemaV1 type export — that's an API-surface decision a maintainer should sign off on rather than me.

Extended reasoning...

Overview

Adds a third call form to Protocol.setRequestHandler / setNotificationHandler: (methodString, paramsSchema, handler) where paramsSchema is any Standard Schema. Touches packages/core/src/shared/protocol.ts (new overload + _wrapParamsSchemaHandler helper), mirrors the overload in Client and Server overrides, widens validateStandardSchema to accept StandardSchemaV1 (not just StandardSchemaWithJSON), adds an internal parseStandardSchema helper, exports the StandardSchemaV1 type publicly, updates migration docs and examples, and adds a new test file.

Security risks

None identified. No auth, transport, or HTTP-status logic touched. Validation failures correctly map to ProtocolErrorCode.InvalidParams. _meta is stripped before user-schema validation so progress tokens etc. don't leak into or fail user schemas.

Level of scrutiny

Medium-high — this is a public API addition, not a bugfix or mechanical change. Per the repo's review conventions, the burden of proof is on addition and "one way to do things" applies: the PR introduces a third registration form alongside the existing two (spec-method-string and Zod-schema-with-literal), with the Zod-schema form now marked @deprecated in favor of this one. That's a reasonable direction but it's an API-shape decision (what the canonical custom-method registration looks like in v2) that should be confirmed by a maintainer, not auto-approved.

Other factors

• Part of the tracked v2-bc series stacked on #1891, so the direction is presumably already agreed; still worth an explicit human LGTM on the final shape.
• New public type export (StandardSchemaV1) — intentional per the diff but expands the surface.
• My two prior inline comments are nits (test-name accuracy and an internal helper that could be inlined); both explicitly optional and don't block.
• Nothing has changed since my prior review at commit 05a01a1; this run found no new issues.

[felixweinberger]

@claude review

[felixweinberger]

@claude review

[felixweinberger]

@claude review

[claude]

All three rounds of nits are now addressed (split tests in 99794a8, JSDoc wording in cdc022b) and I have nothing further on correctness — deferring only because this adds new public API surface (the 3-arg overloads on Protocol/Client/Server plus the StandardSchemaV1 type export), which per the repo's review conventions warrants a maintainer sign-off on the design.

Extended reasoning...

Overview

This PR adds a third overload to setRequestHandler / setNotificationHandler on Protocol, Client, and Server: (method: string, paramsSchema: StandardSchemaV1, handler). The handler receives validated params (with _meta stripped and absent params normalized to {}). It also exports the StandardSchemaV1 type from the public barrel, widens validateStandardSchema's generic bound from StandardSchemaWithJSON to StandardSchemaV1, adds a small parseStandardSchema helper, updates both migration docs, rewrites the two custom-method examples to use the new form, and adds a dedicated test file with five cases including a hand-rolled non-Zod StandardSchema. A changeset bumps client/server as minor.

Prior feedback resolved

All three of my earlier inline threads are resolved and verified in the current diff: the absent-params and _meta-strip tests are now separate (99794a8), the per-overload JSDoc was reworded from "the supported call form" to "a supported call form (alongside the three-arg form)" across all four sites (cdc022b), and the parseStandardSchema JSDoc was clarified (the author kept the helper for shape parity with parseSchema, which I'd flagged as a reasonable choice).

Security risks

None identified. The new path validates user-supplied params against a caller-provided schema and throws ProtocolError(InvalidParams) on failure — same trust boundary as the existing Zod-schema overload. No auth, transport, or serialization changes.

Level of scrutiny

Moderate-to-high. The implementation itself is small and well-tested, but per REVIEW.md ("burden of proof is on addition", "every new export is intentional") this is a deliberate public-API expansion: three new overload signatures across three classes plus a new type export. It's part of the labeled v2-bc series with a linked reviewer guide, so the design has presumably been discussed, but a maintainer should confirm the shape (e.g., that paramsSchema taking any StandardSchemaV1 rather than StandardSchemaWithJSON is the intended bound, and that exporting StandardSchemaV1 from the public barrel is desired).

Other factors

The overload dispatch (isZodLikeSchema → 2-arg string → 3-arg) is unambiguous and preserves the existing per-method wrapping in Client (elicitation/sampling) and Server (tools/call) since the 3-arg path normalizes to the same (request, ctx) => Result handler shape before those wrappers run. The validateStandardSchema generic widening is source-compatible (StandardSchemaWithJSON extends StandardSchemaV1). Docs and examples are consistent with the implementation.

[claude]

🟡 nit: The new 3-arg else-branch in Server.setRequestHandler (server.ts:260-262) and Client.setRequestHandler (client.ts:372-374) has no vitest coverage — threeArgHandlers.test.ts only exercises the base Protocol path via a bare TestProtocol, and setRequestHandlerSchemaParity.test.ts only covers the string and ZodLike forms. Consider adding a 3-arg case to the schema-parity test (and/or a Client equivalent) so the override dispatch is exercised, per REVIEW.md's 'New behavior has vitest coverage' checklist.

Extended reasoning...

What's missing

packages/core/test/shared/threeArgHandlers.test.ts tests the new 3-arg setRequestHandler/setNotificationHandler form, but only via a bare TestProtocol extends Protocol subclass. That exercises Protocol.setRequestHandler's implementation directly.

However, Server and Client each override setRequestHandler with their own implementation body that re-implements the same 3-way dispatch (isZodLikeSchema / maybeHandler === undefined / else → _wrapParamsSchemaHandler). The new else-branch added in this PR — server.ts:260-262 and client.ts:372-374 — is never reached by any test:

• threeArgHandlers.test.ts calls TestProtocol.setRequestHandler(...), hitting the base Protocol implementation, not the overrides.
• packages/server/test/server/setRequestHandlerSchemaParity.test.ts (lines 44-45, 56) covers only the string form and the ZodLike-schema form on Server — no 3-arg case.
• Grep of packages/server/test and packages/client/test shows no other test invokes the 3-arg form on Server or Client.

Why the override dispatch is worth covering

setRequestHandlerSchemaParity.test.ts exists precisely because the Server/Client overrides duplicate Protocol's dispatch logic in order to layer per-method wrapping (e.g. the tools/call task-result validation wrapper) on top. That test's stated purpose (file header comment, lines 8-12) is to guard that all call forms route through the same per-method wrapping. The 3-arg form is now a third call form that also feeds into that wrapping path, but the parity test doesn't include it.

Step-by-step trace

1. User calls server.setRequestHandler('acme/echo', z.object({ msg: z.string() }), p => ({ reply: p.msg })).
2. Server.setRequestHandler (server.ts:248-264) runs: isZodLikeSchema('acme/echo') → false; maybeHandler === undefined → false; falls to else (server.ts:260-262).
3. handler = this._wrapParamsSchemaHandler('acme/echo', schema, userHandler) — the new code under question.
4. Method ≠ 'tools/call', so it falls through to _setRequestHandlerByMethod(method, handler).

No test currently drives steps 2-4 on a Server or Client instance. The shared _wrapParamsSchemaHandler helper IS tested via TestProtocol, so the validation logic itself is covered — what's uncovered is the ~3-line override dispatch shim and its routing into the per-method wrapping. (Note: the documented use case for the 3-arg form is non-spec methods, so the tools/call-composition interaction is hypothetical rather than a primary concern; the main gap is just that the override branch itself never executes under test.)

Why it's only a nit

The else-branch is a trivial 3-line delegation to a tested helper, and examples/{server,client}/src/customMethodExample.ts compile-check the Server/Client overload signatures. The practical regression risk is low. It's flagged because (a) REVIEW.md asks for vitest coverage on new behavior, and (b) the schema-parity test already sets the precedent of covering each call form on the override — extending it is cheap and consistent.

Suggested fix

Add a 3-arg case alongside the existing two in setRequestHandlerSchemaParity.test.ts, e.g.:

it('three-arg form handles non-spec methods through Server', async () => {
    const { ct } = await setup(s =>
        s.setRequestHandler('acme/echo', z.object({ msg: z.string() }), p => ({ reply: p.msg }))
    );
    const res = await new Promise<{ result?: unknown; error?: unknown }>(resolve => {
        ct.onmessage = m => {
            const msg = m as { result?: unknown; error?: unknown };
            if ('result' in msg || 'error' in msg) resolve(msg);
        };
        ct.send({ jsonrpc: '2.0', id: 1, method: 'acme/echo', params: { msg: 'hi' } });
    });
    expect(res.result).toEqual({ reply: 'hi' });
});

Optionally mirror with a one-liner Client test, or note that Client's override is structurally identical.