feat(core): add custom request/notification handler API to Protocol#1846
feat(core): add custom request/notification handler API to Protocol#1846felixweinberger wants to merge 8 commits intofweinberger/migration-doc-fixesfrom
Conversation
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
Adds setCustomRequestHandler, setCustomNotificationHandler, sendCustomRequest, sendCustomNotification (plus remove* variants) to the Protocol class. These allow registering handlers for vendor-specific methods outside the standard RequestMethod/NotificationMethod unions, with user-provided Zod schemas for param/result validation. Custom handlers share the existing _requestHandlers map and dispatch path, so they receive full context (cancellation, task support, send/notify) for free. Capability checks are skipped for custom methods. Also exports InMemoryTransport from core/public so examples and tests can use createLinkedPair() without depending on the internal core barrel, and adds examples/server/src/customMethodExample.ts demonstrating the API.
- Guard setCustom*/removeCustom* against standard MCP method names (throws directing users to setRequestHandler/setNotificationHandler) - Add isRequestMethod/isNotificationMethod runtime predicates - Add comprehensive unit tests (15 cases) for all 6 custom-method APIs - Add ext-apps style example demonstrating mcp-ui/* methods and DOM-style event listeners built on setCustomNotificationHandler - Add @modelcontextprotocol/client path mapping to examples/server tsconfig so the example resolves source instead of dist
…id prototype-chain false positives
…typed-params overloads; migration docs
- sendCustomNotification now delegates to notification() so debouncing and
task-queued delivery apply to custom methods
- sendCustomRequest/sendCustomNotification gain a {params, result}/{params}
schema-bundle overload that validates outbound params before sending
- clarify JSDoc: capability checks are a no-op for custom methods regardless
of enforceStrictCapabilities
- add migration.md / migration-SKILL.md sections for custom protocol methods
felixweinberger
force-pushed
the
fweinberger/custom-method-handlers
branch
from
a2558ec to
e4a5c5c
Compare
felixweinberger
changed the base branch from
main
to
fweinberger/migration-doc-fixes
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| async sendCustomRequest( | ||
| method: string, | ||
| params: Record<string, unknown> | undefined, | ||
| schemaOrBundle: AnySchema | { params: AnySchema; result: AnySchema }, | ||
| options?: RequestOptions | ||
| ): Promise<unknown> { | ||
| let resultSchema: AnySchema; | ||
| if (isSchemaBundle(schemaOrBundle)) { | ||
| const parsed = parseSchema(schemaOrBundle.params, params); | ||
| if (!parsed.success) { | ||
| throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Invalid params for ${method}: ${parsed.error.message}`); | ||
| } | ||
| resultSchema = schemaOrBundle.result; | ||
| } else { | ||
| resultSchema = schemaOrBundle; | ||
| } | ||
| return this._requestWithSchema({ method, params } as Request, resultSchema, options); | ||
| } | ||
|
|
||
| /** | ||
| * Sends a custom (non-standard) notification. | ||
| * | ||
| * Unlike {@linkcode Protocol.notification | notification}, this accepts any method string. It | ||
| * routes through {@linkcode Protocol.notification | notification}, so debouncing and task-queued | ||
| * delivery apply. Capability checks are a no-op for custom methods since | ||
| * `assertNotificationCapability` only covers | ||
| * standard MCP notifications. |
There was a problem hiding this comment.
🔴 In both sendCustomRequest and sendCustomNotification, the schema-bundle overload validates params via parseSchema(...) but then discards parsed.data and sends the original untransformed params over the wire. Any Zod transforms (e.g. .trim(), .toLowerCase()) or defaults (e.g. .default(1)) applied by the schema are silently dropped. Fix: replace params with parsed.data in the _requestWithSchema call and the this.notification(...) call after successful schema-bundle validation.
Extended reasoning...
What the bug is and how it manifests
In sendCustomRequest (around line 1175 of protocol.ts) and sendCustomNotification (around line 1213), when the schema-bundle overload is used, parseSchema(schemaOrBundle.params, params) is called to validate. This invokes z.safeParse internally, which applies all Zod transforms and defaults and returns the transformed value in parsed.data. However, after the !parsed.success guard, parsed.data is immediately discarded. The original params variable containing the untransformed caller-supplied value is what actually gets sent to _requestWithSchema / this.notification.
The specific code path that triggers it
In sendCustomRequest:
if (isSchemaBundle(schemaOrBundle)) {
const parsed = parseSchema(schemaOrBundle.params, params);
if (!parsed.success) { throw ... }
resultSchema = schemaOrBundle.result;
// parsed.data is never used
}
// BUG: sends original params, not parsed.data
return this._requestWithSchema({ method, params } as Request, resultSchema, options);
In sendCustomNotification:
if (schemasOrOptions && 'params' in schemasOrOptions) {
const parsed = parseSchema(schemasOrOptions.params, params);
if (!parsed.success) { throw ... }
// parsed.data is never used
options = maybeOptions;
}
// BUG: sends original params, not parsed.data
return this.notification({ method, params } as Notification, options);
Why existing code does not prevent it
All tests in customMethods.test.ts use schemas like z.object({ query: z.string(), limit: z.number().optional() }) — pure validators with no transforms or defaults. For such schemas parsed.data is structurally identical to params, so the bug is invisible. The schema-bundle test at line ~196 passes { query: 'q' } against a schema with no transforms, so parsed.data === params and the test passes.
What the impact would be
Any user who uses the schema-bundle overload with a schema containing transforms or defaults will silently get incorrect wire data:
- z.string().trim() — caller passes ' hello ', wire sends ' hello ' (untrimmed)
- z.number().default(1) — caller omits page, wire sends undefined (not 1)
- z.string().toLowerCase() — wire sends original casing
- z.coerce.number() — wire sends the uncoerced string
This also creates an asymmetry: inbound handlers (setCustomRequestHandler, lines ~1081-1084) correctly use parsed.data, while the outbound senders do not. Sender and receiver operate on inconsistent data.
How to fix it
Two one-line changes:
// sendCustomRequest: use parsed.data instead of params
return this._requestWithSchema({ method, params: parsed.data } as Request, resultSchema, options);
// sendCustomNotification: use parsed.data instead of params
return this.notification({ method, params: parsed.data } as Notification, options);
The parsed variable must remain in scope at the call site (either hoist it or move the call inside the if block).
Step-by-step proof
- Caller defines const Schema = z.object({ query: z.string().trim(), page: z.number().default(1) }).
- Caller calls client.sendCustomRequest('acme/search', { query: ' hello ' }, { params: Schema, result: ResultSchema }).
- isSchemaBundle returns true; parseSchema(Schema, { query: ' hello ' }) runs z.safeParse.
- parsed.success === true; parsed.data is { query: 'hello', page: 1 } (trimmed + default applied).
- parsed.data is unused; code reaches _requestWithSchema({ method, params: { query: ' hello ' } }, ...).
- Message sent over the wire: { query: ' hello ' } — no page, untrimmed string.
- Server handler (which also parses with Schema) receives { query: 'hello', page: 1 } — inconsistent with what was sent.
…m-method-handlers
2 participants
Adds an explicit API on
Protocolfor registering handlers and sending messages for non-standard / vendor-specific methods, without reintroducing the class-level generic type parameters removed in #1451.Motivation and Context
#1446 changed
setRequestHandlerto take string method names constrained to the closedRequestMethodunion, and #1451 removed the<SendRequestT, SendNotificationT, SendResultT>generics fromProtocol/Server/Client. Together these closed the door on custom protocol extensions: there is no longer a typed way to register a handler for e.g.mcp-ui/initializeoracme/search.This PR adds a small explicit surface instead:
_requestHandlers/_notificationHandlersmaps, so they get the full dispatch path (context, cancellation, tasks, error wrapping) for free'ping','tools/call') and points tosetRequestHandlerinsteadsendCustomNotificationroutes throughnotification()so debouncing and task-queued delivery applysendCustomRequest/sendCustomNotificationaccept an optional schema bundle ({params, result}) for typed outbound params with pre-send validation — closes the typing gap vs v1's class-level genericsenforceStrictCapabilities(theassertCapabilityForMethod/assertNotificationCapabilityswitches have no default case)The primary consumer is
ext-apps, which currently extends v1'sProtocol<SendRequestT, SendNotificationT, SendResultT>to register ~15mcp-ui/*methods. The includedcustomMethodExtAppsExample.tsdemonstrates that pattern is fully expressible on this API.How Has This Been Tested?
packages/core/test/shared/customMethods.test.ts(typed params/results, full ctx, validation errors, collision guard, removal, not-connected, last-wins, prototype-key regression, schema-bundle overloads, debouncing, strict-caps)pnpm build:all,pnpm typecheck:all,pnpm lint:all, core tests 509/509npx tsx examples/server/src/customMethod{,ExtApps}Example.tsBreaking Changes
None. Purely additive to
Protocol. Not exposed onMcpServer(usemcpServer.server.*per existing guidance).Types of changes
Checklist
Additional context
isRequestMethod/isNotificationMethodruntime predicates inschemas.ts(usingObject.hasOwn)@modelcontextprotocol/clientpath mapping toexamples/server/tsconfig.jsondocs/migration.mdanddocs/migration-SKILL.md(methodString, paramsSchema)separately