Skip to content

Commit fefb6e0

Browse files
feat(compat): mark setRequestHandler/setNotificationHandler Zod-schema overloads @deprecated; address review
- @deprecated on the Zod-schema overloads of setRequestHandler / setNotificationHandler (Protocol + Client + Server). The 3-arg (method, paramsSchema, handler) form is the non-deprecated path for custom methods. request()/mcpReq.send()/callTool schema overloads are NOT deprecated (only typed form for custom-method results). - Unify positional-schema detection via isResultSchemaLike() (~standard or .parse) used by request(), mcpReq.send() and callTool(). - request()/mcpReq.send() now reject with a clear TypeError when a non-spec method is called without a result schema (was: undefined schema crashed in parseSchema downstream). - Avoid redundant spec-schema parses: _setRequestHandlerByMethod() takes skipSpecParse; Server/Client overrides pass true for their per-method wrappers and for the Zod-schema form. Aligns Protocol with subclasses (both skip when user supplied a schema). - compatSchema.ts: drop @internal from file header (ZodLikeRequestSchema is publicly exported); add isResultSchemaLike helper. - Tests: client setRequestHandlerSchemaParity (elicitation+sampling), compatSchema helpers, runtime-guard cases for request()/mcpReq.send().
1 parent d2e046d commit fefb6e0

8 files changed

Lines changed: 204 additions & 33 deletions

File tree

packages/client/src/client/client.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import {
5555
extractTaskManagerOptions,
5656
GetPromptResultSchema,
5757
InitializeResultSchema,
58+
isResultSchemaLike,
5859
isZodLikeSchema,
5960
LATEST_PROTOCOL_VERSION,
6061
ListChangedOptionsBaseSchema,
@@ -343,15 +344,16 @@ export class Client extends Protocol<ClientContext> {
343344
method: M,
344345
handler: (request: RequestTypeMap[M], ctx: ClientContext) => ResultTypeMap[M] | Promise<ResultTypeMap[M]>
345346
): void;
346-
/** For spec methods the method-string form is more concise; this overload is the supported call form for non-spec methods or when you want full-envelope validation. */
347+
/** @deprecated Use the 3-arg `(method, paramsSchema, handler)` form for custom methods, or the method-string form for spec methods. */
347348
public override setRequestHandler<T extends ZodLikeRequestSchema>(
348349
requestSchema: T,
349350
handler: (request: ReturnType<T['parse']>, ctx: ClientContext) => Result | Promise<Result>
350351
): void;
351352
public override setRequestHandler(methodOrSchema: string | ZodLikeRequestSchema, schemaHandler: unknown): void {
352353
let method: string;
353354
let handler: (request: Request, ctx: ClientContext) => ClientResult | Promise<ClientResult>;
354-
if (isZodLikeSchema(methodOrSchema)) {
355+
const fromSchema = isZodLikeSchema(methodOrSchema);
356+
if (fromSchema) {
355357
const schema = methodOrSchema;
356358
const userHandler = schemaHandler as (request: unknown, ctx: ClientContext) => Result | Promise<Result>;
357359
method = extractMethodLiteral(schema);
@@ -426,8 +428,8 @@ export class Client extends Protocol<ClientContext> {
426428
return validatedResult;
427429
};
428430

429-
// Install the wrapped handler
430-
return this._setRequestHandlerByMethod(method, wrappedHandler);
431+
// wrappedHandler validates with the spec schema itself; skip the extra parse in the base helper.
432+
return this._setRequestHandlerByMethod(method, wrappedHandler, true);
431433
}
432434

433435
if (method === 'sampling/createMessage') {
@@ -469,12 +471,12 @@ export class Client extends Protocol<ClientContext> {
469471
return validationResult.data;
470472
};
471473

472-
// Install the wrapped handler
473-
return this._setRequestHandlerByMethod(method, wrappedHandler);
474+
// wrappedHandler validates with the spec schema itself; skip the extra parse in the base helper.
475+
return this._setRequestHandlerByMethod(method, wrappedHandler, true);
474476
}
475477

476-
// Other handlers use default behavior
477-
return this._setRequestHandlerByMethod(method, handler);
478+
// Other methods: skip the spec parse only when the user supplied their own schema (it is the source of truth).
479+
return this._setRequestHandlerByMethod(method, handler, fromSchema);
478480
}
479481

480482
protected assertCapability(capability: keyof ServerCapabilities, method: string): void {
@@ -898,7 +900,7 @@ export class Client extends Protocol<ClientContext> {
898900
optionsOrSchema?: RequestOptions | unknown,
899901
maybeOptions?: RequestOptions
900902
): Promise<CallToolResult> {
901-
const arg2IsSchema = optionsOrSchema != null && typeof optionsOrSchema === 'object' && 'parse' in optionsOrSchema;
903+
const arg2IsSchema = isResultSchemaLike(optionsOrSchema);
902904
// v1 allowed `callTool(params, undefined, opts)` (resultSchema was optional-with-default);
903905
// when arg2 is not a schema, prefer arg3 if present so opts aren't dropped.
904906
const options: RequestOptions | undefined = arg2IsSchema
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { CreateMessageRequestSchema, ElicitRequestSchema, InMemoryTransport } from '@modelcontextprotocol/core';
4+
5+
import { Client } from '../../src/client/client.js';
6+
7+
/**
8+
* Mirrors the server-side parity test: registering with the Zod-schema form must
9+
* route through the same per-method wrapper (result-shape validation) as the
10+
* method-string form.
11+
*/
12+
describe('Client.setRequestHandler — Zod-schema form parity', () => {
13+
async function setup(register: (c: Client) => void) {
14+
const client = new Client({ name: 't', version: '1.0' }, { capabilities: { sampling: {}, elicitation: {} } });
15+
register(client);
16+
const [ct, st] = InMemoryTransport.createLinkedPair();
17+
await ct.start();
18+
// Minimal server-side stub on ct so Client.connect's initialize handshake completes.
19+
ct.onmessage = m => {
20+
const msg = m as { id?: number; method?: string };
21+
if (msg.method === 'initialize') {
22+
void ct.send({
23+
jsonrpc: '2.0',
24+
id: msg.id!,
25+
result: { protocolVersion: '2025-06-18', serverInfo: { name: 's', version: '1.0' }, capabilities: {} }
26+
});
27+
}
28+
};
29+
await client.connect(st);
30+
return { ct };
31+
}
32+
33+
async function send(
34+
ct: InMemoryTransport,
35+
method: string,
36+
params: Record<string, unknown>
37+
): Promise<{ result?: unknown; error?: unknown }> {
38+
return await new Promise(resolve => {
39+
ct.onmessage = m => {
40+
const msg = m as { id?: number; result?: unknown; error?: unknown };
41+
if (msg.id === 99 && ('result' in msg || 'error' in msg)) resolve(msg);
42+
};
43+
void ct.send({ jsonrpc: '2.0', id: 99, method, params });
44+
});
45+
}
46+
47+
it('elicitation/create — schema form gets the same result-validation as string form', async () => {
48+
const invalidElicitResult = { action: 'nope' };
49+
const params = { mode: 'form', message: 'q', requestedSchema: { type: 'object', properties: {} } };
50+
51+
const viaString = await setup(c => c.setRequestHandler('elicitation/create', () => invalidElicitResult as never));
52+
const viaSchema = await setup(c => c.setRequestHandler(ElicitRequestSchema, () => invalidElicitResult as never));
53+
54+
const stringRes = await send(viaString.ct, 'elicitation/create', params);
55+
const schemaRes = await send(viaSchema.ct, 'elicitation/create', params);
56+
57+
expect((stringRes.error as { message: string }).message).toContain('Invalid elicitation result');
58+
expect(schemaRes.error).toEqual(stringRes.error);
59+
});
60+
61+
it('sampling/createMessage — schema form gets the same result-validation as string form', async () => {
62+
const invalidSamplingResult = { role: 'assistant' };
63+
const params = { messages: [], maxTokens: 1 };
64+
65+
const viaString = await setup(c => c.setRequestHandler('sampling/createMessage', () => invalidSamplingResult as never));
66+
const viaSchema = await setup(c => c.setRequestHandler(CreateMessageRequestSchema, () => invalidSamplingResult as never));
67+
68+
const stringRes = await send(viaString.ct, 'sampling/createMessage', params);
69+
const schemaRes = await send(viaSchema.ct, 'sampling/createMessage', params);
70+
71+
expect((stringRes.error as { message: string }).message).toContain('Invalid');
72+
expect(schemaRes.error).toEqual(stringRes.error);
73+
});
74+
});

packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,5 @@ export * from './validators/fromJsonSchema.js';
4949

5050
// Core types only - implementations are exported via separate entry points
5151
export type { ZodLikeRequestSchema } from './util/compatSchema.js';
52-
export { extractMethodLiteral, isZodLikeSchema } from './util/compatSchema.js';
52+
export { extractMethodLiteral, isResultSchemaLike, isZodLikeSchema } from './util/compatSchema.js';
5353
export type { JsonSchemaType, JsonSchemaValidator, jsonSchemaValidator, JsonSchemaValidatorResult } from './validators/types.js';

packages/core/src/shared/protocol.ts

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import {
4545
SUPPORTED_PROTOCOL_VERSIONS
4646
} from '../types/index.js';
4747
import type { ZodLikeRequestSchema } from '../util/compatSchema.js';
48-
import { extractMethodLiteral, isZodLikeSchema } from '../util/compatSchema.js';
48+
import { extractMethodLiteral, isResultSchemaLike, isZodLikeSchema } from '../util/compatSchema.js';
4949
import type { AnySchema, SchemaOutput } from '../util/schema.js';
5050
import { parseSchema } from '../util/schema.js';
5151
import type { TaskContext, TaskManagerHost, TaskManagerOptions, TaskRequestOptions } from './taskManager.js';
@@ -611,10 +611,15 @@ export abstract class Protocol<ContextT extends BaseContext> {
611611
_meta: request.params?._meta,
612612
signal: abortController.signal,
613613
send: ((r: Request, optionsOrSchema?: TaskRequestOptions | AnySchema, maybeOptions?: TaskRequestOptions) => {
614-
if (optionsOrSchema && '~standard' in optionsOrSchema) {
615-
return sendRequest(r, optionsOrSchema, maybeOptions);
614+
if (isResultSchemaLike(optionsOrSchema)) {
615+
return sendRequest(r, optionsOrSchema as AnySchema, maybeOptions);
616616
}
617617
const resultSchema = getResultSchema(r.method as RequestMethod);
618+
if (!resultSchema) {
619+
return Promise.reject(
620+
new TypeError(`mcpReq.send(): '${r.method}' is not a spec method; pass a result schema as the second argument.`)
621+
);
622+
}
618623
return sendRequest(r, resultSchema, optionsOrSchema);
619624
}) as BaseContext['mcpReq']['send'],
620625
notify: sendNotification
@@ -807,10 +812,15 @@ export abstract class Protocol<ContextT extends BaseContext> {
807812
/** For spec methods the one-argument form is more concise; this overload is the supported call form for non-spec methods or custom result shapes. */
808813
request<T extends AnySchema>(request: Request, resultSchema: T, options?: RequestOptions): Promise<SchemaOutput<T>>;
809814
request(request: Request, optionsOrSchema?: RequestOptions | AnySchema, maybeOptions?: RequestOptions): Promise<unknown> {
810-
if (optionsOrSchema && '~standard' in optionsOrSchema) {
811-
return this._requestWithSchema(request, optionsOrSchema, maybeOptions);
815+
if (isResultSchemaLike(optionsOrSchema)) {
816+
return this._requestWithSchema(request, optionsOrSchema as AnySchema, maybeOptions);
812817
}
813818
const schema = getResultSchema(request.method as RequestMethod);
819+
if (!schema) {
820+
return Promise.reject(
821+
new TypeError(`request(): '${request.method}' is not a spec method; pass a result schema as the second argument.`)
822+
);
823+
}
814824
return this._requestWithSchema(request, schema, optionsOrSchema);
815825
}
816826

@@ -1043,7 +1053,7 @@ export abstract class Protocol<ContextT extends BaseContext> {
10431053
method: M,
10441054
handler: (request: RequestTypeMap[M], ctx: ContextT) => Result | Promise<Result>
10451055
): void;
1046-
/** For spec methods the method-string form is more concise; this overload is the supported call form for non-spec methods or when you want full-envelope validation. */
1056+
/** @deprecated Use the 3-arg `(method, paramsSchema, handler)` form for custom methods, or the method-string form for spec methods. */
10471057
setRequestHandler<T extends ZodLikeRequestSchema>(
10481058
requestSchema: T,
10491059
handler: (request: ReturnType<T['parse']>, ctx: ContextT) => Result | Promise<Result>
@@ -1052,9 +1062,14 @@ export abstract class Protocol<ContextT extends BaseContext> {
10521062
if (isZodLikeSchema(method)) {
10531063
const requestSchema = method;
10541064
const methodStr = extractMethodLiteral(requestSchema);
1055-
this.assertRequestHandlerCapability(methodStr);
1056-
this._requestHandlers.set(methodStr, (request, ctx) =>
1057-
Promise.resolve((handler as (req: unknown, ctx: ContextT) => Result | Promise<Result>)(requestSchema.parse(request), ctx))
1065+
// The user's schema is the source of truth; spec-schema validation is skipped to avoid double-parsing.
1066+
this._setRequestHandlerByMethod(
1067+
methodStr,
1068+
(request, ctx) =>
1069+
Promise.resolve(
1070+
(handler as (req: unknown, ctx: ContextT) => Result | Promise<Result>)(requestSchema.parse(request), ctx)
1071+
),
1072+
true
10581073
);
10591074
return;
10601075
}
@@ -1064,9 +1079,22 @@ export abstract class Protocol<ContextT extends BaseContext> {
10641079
/**
10651080
* Registers a request handler by method string, bypassing the public overload set.
10661081
* Used by `Client`/`Server` overrides to forward without `as RequestMethod` casts.
1082+
*
1083+
* @param skipSpecParse - When `true`, the handler is stored directly without
1084+
* wrapping it in spec-schema validation. Set this when the caller has already
1085+
* applied its own validation (Zod-schema form, or a method-specific wrapper)
1086+
* to avoid parsing the same request multiple times.
10671087
*/
1068-
protected _setRequestHandlerByMethod(method: string, handler: (request: Request, ctx: ContextT) => Result | Promise<Result>): void {
1088+
protected _setRequestHandlerByMethod(
1089+
method: string,
1090+
handler: (request: Request, ctx: ContextT) => Result | Promise<Result>,
1091+
skipSpecParse = false
1092+
): void {
10691093
this.assertRequestHandlerCapability(method);
1094+
if (skipSpecParse) {
1095+
this._requestHandlers.set(method, (request, ctx) => Promise.resolve(handler(request, ctx)));
1096+
return;
1097+
}
10701098
const schema = getRequestSchema(method as RequestMethod);
10711099
this._requestHandlers.set(method, (request, ctx) => {
10721100
const parsed = schema ? (schema.parse(request) as Request) : request;
@@ -1101,7 +1129,7 @@ export abstract class Protocol<ContextT extends BaseContext> {
11011129
method: M,
11021130
handler: (notification: NotificationTypeMap[M]) => void | Promise<void>
11031131
): void;
1104-
/** For spec methods the method-string form is more concise; this overload is the supported call form for non-spec methods or when you want full-envelope validation. */
1132+
/** @deprecated Use the 3-arg `(method, paramsSchema, handler)` form for custom methods, or the method-string form for spec methods. */
11051133
setNotificationHandler<T extends ZodLikeRequestSchema>(
11061134
notificationSchema: T,
11071135
handler: (notification: ReturnType<T['parse']>) => void | Promise<void>

packages/core/src/util/compatSchema.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@
44
* v1 accepted a Zod object whose `.shape.method` is `z.literal('<method>')`.
55
* v2 also accepts the method string directly. These helpers detect the schema
66
* form and extract the literal so the dispatcher can route to the correct path.
7-
*
8-
* @internal
97
*/
108

119
/**
12-
* Minimal structural type for a Zod object schema. The `method` literal is
13-
* checked at runtime by `extractMethodLiteral`; the type-level constraint
14-
* is intentionally loose because zod v4's `ZodLiteral` doesn't surface `.value`
15-
* in its declared type (only at runtime).
10+
* Minimal structural type for a v1-style Zod request/notification schema: an
11+
* object schema whose `.shape.method` is a string literal. The `method` literal
12+
* is checked at runtime; the type-level constraint is intentionally loose
13+
* because zod v4's `ZodLiteral` doesn't surface `.value` in its declared type
14+
* (only at runtime).
1615
*/
1716
export interface ZodLikeRequestSchema {
1817
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -39,3 +38,13 @@ export function extractMethodLiteral(schema: ZodLikeRequestSchema): string {
3938
}
4039
return value;
4140
}
41+
42+
/**
43+
* True if `arg` looks like a result schema passed positionally to
44+
* `request()` / `callTool()` / `mcpReq.send()`. Detects either the
45+
* Standard Schema marker (`~standard`) or a Zod-style `parse` function so the
46+
* v1 schema-argument form is recognised regardless of zod major version.
47+
*/
48+
export function isResultSchemaLike(arg: unknown): arg is object {
49+
return arg != null && typeof arg === 'object' && ('~standard' in arg || typeof (arg as { parse?: unknown }).parse === 'function');
50+
}

packages/core/test/shared/customMethods.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ describe('request() — explicit result schema overload', () => {
106106
const r = await a.request({ method: 'ping' });
107107
expect(r).toEqual({});
108108
});
109+
110+
it('throws a clear error for a non-spec method without a result schema', async () => {
111+
const { a, b } = await makePair();
112+
b.setRequestHandler(EchoRequest, req => ({ reply: req.params.msg }));
113+
// @ts-expect-error — bypassing the overload set to exercise the runtime guard
114+
await expect(a.request({ method: 'acme/echo', params: { msg: 'x' } })).rejects.toThrow(/'acme\/echo' is not a spec method/);
115+
});
109116
});
110117

111118
describe('ctx.mcpReq.send() — explicit result schema overload', () => {
@@ -132,6 +139,16 @@ describe('ctx.mcpReq.send() — explicit result schema overload', () => {
132139
await a.request({ method: 'acme/outer' }, z.object({}));
133140
expect(pingResult).toEqual({});
134141
});
142+
143+
it('throws a clear error for a non-spec method without a result schema', async () => {
144+
const { a, b } = await makePair();
145+
b.setRequestHandler(z.object({ method: z.literal('acme/outer') }), async (_req, ctx) => {
146+
// @ts-expect-error — bypassing the overload set to exercise the runtime guard
147+
await ctx.mcpReq.send({ method: 'acme/nope' });
148+
return {};
149+
});
150+
await expect(a.request({ method: 'acme/outer' }, z.object({}))).rejects.toThrow(/'acme\/nope' is not a spec method/);
151+
});
135152
});
136153

137154
describe('notification() mock-assignability', () => {
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { z } from 'zod';
3+
4+
import { extractMethodLiteral, isResultSchemaLike, isZodLikeSchema } from '../../src/util/compatSchema.js';
5+
6+
describe('compatSchema helpers', () => {
7+
describe('isZodLikeSchema', () => {
8+
it('detects a Zod object schema', () => {
9+
expect(isZodLikeSchema(z.object({ method: z.literal('x') }))).toBe(true);
10+
});
11+
it('rejects strings, plain objects, and null', () => {
12+
expect(isZodLikeSchema('tools/call')).toBe(false);
13+
expect(isZodLikeSchema({ shape: {} })).toBe(false);
14+
expect(isZodLikeSchema(null)).toBe(false);
15+
});
16+
});
17+
18+
describe('extractMethodLiteral', () => {
19+
it('reads the method literal from a Zod object schema', () => {
20+
expect(extractMethodLiteral(z.object({ method: z.literal('acme/echo') }))).toBe('acme/echo');
21+
});
22+
it('throws when no string method literal is present', () => {
23+
expect(() => extractMethodLiteral(z.object({ method: z.string() }))).toThrow(TypeError);
24+
expect(() => extractMethodLiteral(z.object({}))).toThrow(TypeError);
25+
});
26+
});
27+
28+
describe('isResultSchemaLike', () => {
29+
it('detects Standard Schema (~standard) and Zod-style parse()', () => {
30+
expect(isResultSchemaLike(z.string())).toBe(true);
31+
expect(isResultSchemaLike({ '~standard': { version: 1, vendor: 't', validate: () => ({ value: 1 }) } })).toBe(true);
32+
expect(isResultSchemaLike({ parse: (v: unknown) => v })).toBe(true);
33+
});
34+
it('rejects RequestOptions-shaped objects, undefined, and primitives', () => {
35+
expect(isResultSchemaLike({ timeout: 100 })).toBe(false);
36+
expect(isResultSchemaLike(undefined)).toBe(false);
37+
expect(isResultSchemaLike('x')).toBe(false);
38+
});
39+
});
40+
});

0 commit comments

Comments
 (0)