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 12 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: cdaca38 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
a2558ec to
e4a5c5c
Compare
|
@claude review |
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
6509f2f to
07c5491
Compare
The {params, result} schema bundle in sendCustomRequest/sendCustomNotification
is a type guard, not a transformer — the caller-provided value is sent as-is,
matching request()/v1 behavior. Transforms/defaults on the params schema are
not applied outbound (parsed.data is intentionally unused on the send path).
Adds JSDoc and a test asserting params are sent verbatim.
1f5fa16 to
98d7742
Compare
…ustom methods
Adds Client.extension(id, settings, {peerSchema?}) and Server.extension(...)
returning an ExtensionHandle that:
- merges settings into capabilities.extensions[id] (advertised in initialize)
- exposes getPeerSettings() with optional schema validation of the peer blob
- wraps setCustom*/sendCustom* with peer-capability gating under
enforceStrictCapabilities
Connects the SEP-2133 capabilities.extensions field to the custom-method API
from #1846. Declare-before-register is structural (you cannot get a handle
without declaring); peer-gating on send mirrors assertCapabilityForMethod.
Stacked on #1846.
…r, drop ext-apps demo
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
packages/core/src/exports/public/index.ts:133-135— The "InMemoryTransport removed from public API" section in docs/migration.md (lines ~514-528) actively contradicts this PR: it tells migrating users that InMemoryTransport was removed and that they must import it from the internal@modelcontextprotocol/corepackage, but this PR re-adds it to the public barrel viapackages/core/src/exports/public/index.ts. After this PR merges, users following the migration guide will unnecessarily import from an internal-only package path (which the guide itself marks as "not for production use") when they could simply use@modelcontextprotocol/serveror@modelcontextprotocol/client.Extended reasoning...
What the bug is and how it manifests
docs/migration.md contains a section titled "InMemoryTransport removed from public API" (roughly lines 514–528). It reads: "InMemoryTransport has been removed from the public API surface" and instructs users to import it from the internal
@modelcontextprotocol/corepackage with the caveat "(testing only — @modelcontextprotocol/core is internal, not for production use)". This section was accurate before this PR, but this PR intentionally re-adds InMemoryTransport to the public exports without updating the migration guide.The specific code path that triggers it
This PR adds
export { InMemoryTransport } from '../../util/inMemory.js';topackages/core/src/exports/public/index.ts. Since@modelcontextprotocol/serverand@modelcontextprotocol/clientboth doexport * from '@modelcontextprotocol/core/public', InMemoryTransport is now importable from either of the public packages. The PR's own example files confirm this: bothcustomMethodExample.tsandcustomMethodExtAppsExample.tsuseimport { InMemoryTransport, Server } from '@modelcontextprotocol/server'. The PR description also explicitly acknowledges the intent: "Also exports InMemoryTransport from @modelcontextprotocol/core/public (needed by the examples; one-line overlap with #1834)".Why existing content does not prevent it
The migration guide was written before InMemoryTransport was re-added. The PR only adds a new "Custom (non-standard) protocol methods" section to migration.md; the pre-existing "InMemoryTransport removed from public API" section was left unchanged. There is no mechanism in the PR review flow to automatically detect that a code change contradicts prose in documentation.
What the impact would be
Any developer migrating from v1 who reads migration.md will follow the incorrect guidance and write
import { InMemoryTransport } from '@modelcontextprotocol/core', importing from a package the guide itself labels as internal and not for production use. The correct, public import —import { InMemoryTransport } from '@modelcontextprotocol/server'(or@modelcontextprotocol/client) — goes undiscovered until they stumble on the PR examples or source code.How to fix it
The "InMemoryTransport removed from public API" section in migration.md should be updated (or removed) to reflect that InMemoryTransport is now part of the public API again. The corrected guidance should direct users to import from
@modelcontextprotocol/serveror@modelcontextprotocol/client, matching the pattern used in the PR's own examples.Step-by-step proof
- User reads migration.md to migrate a v1 codebase that used
InMemoryTransportfor in-process testing. - User finds the "InMemoryTransport removed from public API" section and follows its v2 guidance.
- User writes:
import { InMemoryTransport } from '@modelcontextprotocol/core'; - The guide notes this import is "testing only — @modelcontextprotocol/core is internal, not for production use".
- After this PR merges, the correct import is:
import { InMemoryTransport } from '@modelcontextprotocol/server';(a public, stable package). - The user has been actively misled into using the internal package path that the guide itself discourages.
- User reads migration.md to migrate a v1 codebase that used
…ustom methods
Adds Client.extension(id, settings, {peerSchema?}) and Server.extension(...)
returning an ExtensionHandle that:
- merges settings into capabilities.extensions[id] (advertised in initialize)
- exposes getPeerSettings() with optional schema validation of the peer blob
- wraps setCustom*/sendCustom* with peer-capability gating under
enforceStrictCapabilities
Connects the SEP-2133 capabilities.extensions field to the custom-method API
from #1846. Declare-before-register is structural (you cannot get a handle
without declaring); peer-gating on send mirrors assertCapabilityForMethod.
Stacked on #1846.
… example notification to request stream; add changeset - setCustomRequestHandler/setCustomNotificationHandler now strip _meta from params before validating against the user schema, so .strict() schemas do not reject SDK-injected fields like progressToken. _meta remains available via ctx.mcpReq._meta. Adds regression test. - examples/server/src/customMethodExample.ts: pass relatedRequestId so the acme/statusUpdate notification routes to the request response stream as the comment claims (was going to the standalone SSE stream). - Add .changeset/custom-method-handlers.md (minor bump for client+server).
…ustom methods
Adds Client.extension(id, settings, {peerSchema?}) and Server.extension(...)
returning an ExtensionHandle that:
- merges settings into capabilities.extensions[id] (advertised in initialize)
- exposes getPeerSettings() with optional schema validation of the peer blob
- wraps setCustom*/sendCustom* with peer-capability gating under
enforceStrictCapabilities
Connects the SEP-2133 capabilities.extensions field to the custom-method API
from #1846. Declare-before-register is structural (you cannot get a handle
without declaring); peer-gating on send mirrors assertCapabilityForMethod.
Stacked on #1846.
|
@claude review |
1 similar comment
|
@claude review |
|
Re the review-body finding on |
|
@claude review |
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 510/510npx 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)separatelyInMemoryTransportfrom@modelcontextprotocol/core/public(needed by the examples; one-line overlap with fix(core): export InMemoryTransport, tighten migration docs #1834)