[mattzcarey]

Servers and clients take MCP extensions as { id, setup(ctx) } objects: new McpServer(info, { extensions: [skills, tasks] }). setup runs at construction and adds everything through its context, so the SDK can check names, ids and dependencies for the extension. Stacked on #3047; review the top commit.

const gate: ServerExtension = {
    id: 'com.example/gate',
    setup(ctx) {
        ctx.capability({ exampleData: true });                  // capabilities.extensions[id]; {} by default
        ctx.require('tools');                                   // checked at connect()
        ctx.method('gate/status', { params }, () => ({ armed: true }));
        ctx.use('tools/call', (request, requestCtx, next) => next(request, requestCtx));
        ctx.mcp.registerResource(/* … */);                      // anything McpServer registers
    }
};

Skills (#3047) registers a resource template through McpServer, and Tasks (#2782) needs middleware on tools/call plus its own result kind on the client. One context covers both, and it maps onto SEP-3371's registration, custom methods and middleware:

• ctx.method refuses a core method name, and a method another handler already serves, so two extensions can't silently replace each other.
• ctx.require takes a core capability ('tools', 'resources', …) or another extension id. connect() checks it, so a capability McpServer turns on later, or an extension listed further down, still counts. Behind createMcpHandler, a missing dependency answers -32603 and reports the cause to onerror.
• Extensions are set up in array order. A duplicate id fails construction before any setup runs. Extension middleware runs outside middleware the application adds afterwards with server.use.
• McpServer also accepts { id, attach(mcp) }, so the Skills extension goes in extensions as it ships. extensions is an McpServer option; the low-level Server no longer takes it.
• The client mirrors the shape, with ctx.client, ctx.method for server-to-client requests and ctx.acceptResultType, which lets tools/call carry resultType: "task".
• Protocol.use and Protocol.acceptResultType stay public for applications. McpServer re-throws MissingRequiredClientCapabilityError (-32021) from a tool as a JSON-RPC error.
• Everything above is @experimental. Docs are in docs/advanced/extensions.md.

pnpm -r typecheck, pnpm -r lint, pnpm docs:check and the core-internal, client and server suites ran, including the Skills extension served through extensions unchanged and a require check through createMcpHandler. #2782 adds an end-to-end test with Skills and Tasks on one server.

[changeset-bot]

🦋 Changeset detected

Latest commit: 96868ae

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

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/core-internal	Minor
@modelcontextprotocol/client	Minor
@modelcontextprotocol/server	Minor
@modelcontextprotocol/core	Minor
@modelcontextprotocol/server-legacy	Minor
@modelcontextprotocol/codemod	Minor

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@2820

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2820

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2820

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2820

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: 55c03c8

[claude]

3 optional notes from this repository's REVIEW.md or CLAUDE.md checks were not posted as comments, over this review's limit for such notes; they are on this commit's check card.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🔴 packages/core-internal/src/shared/protocol.ts — Client extensions that wrap elicitation/create (or sampling/createMessage, roots/list) with use silently get no middleware on a 2026-07-28 connection. On the modern era those requests arrive embedded in input_required results and are dispatched through _getRequestHandler at protocol.ts:747, which reads _requestHandlers directly and skips the middleware stack. Fix: resolve every dispatch path through _resolveRequestHandler so use middleware composes around embedded input requests too, which also covers the getRequestHandler host hook in client.ts:752. Only legacy _onrequest traffic composes middleware today, and Client._shouldDropInbound drops modern top-level requests, so on 2026-07-28 the middleware never runs. [also at: packages/core-internal/src/shared/protocol.ts:1896 - Client extension middleware on elicitation/create is skipped for embedded input requests on 2026-07-28 connections, so a gate or rewrite it installs silently does not run there.]

  Why this was flagged

  A user installs a ClientExtension that calls client.use('elicitation/create', mw) as the JSDoc in packages/client/src/client/extension.ts:15 describes, and connects on protocol revision 2026-07-28. The server answers tools/call with an input_required result carrying an embedded elicitation/create request; the Client's _resolveNonCompleteResult (packages/client/src/client/client.ts:749-752) runs runInputRequiredFlow, which calls host.getRequestHandler(method) which is Protocol._getRequestHandler (protocol.ts:746-747) returning this._requestHandlers.get(method). Only _onrequest at protocol.ts:1037 uses the new _resolveRequestHandler, so the middleware array is never consulted here and mw never runs; the raw registered handler answers. On the modern era Client._shouldDropInbound (client.ts:690-696) drops any top-level inbound request, so there is no path on which the middleware applies at all. The base branch has no use, so the regression is that the new API advertises a hook that is a no-op on the default modern negotiation.

  Verification: The pre-existing _getRequestHandler at protocol.ts:746-748 is untouched and still does return this._requestHandlers.get(method);. dispatchInputRequest in packages/core-internal/src/shared/inputRequiredEngine.ts:145-163 calls that handler directly, so middleware registered via use for those methods is never invoked on the embedded path. On 2026-07-28 this is the only path for those methods.

[claude]

🔴 Extension middleware that reads ctx.mcpReq.requestState() gets the raw, unverified wire string, not the value the requestState.verify hook resolved with, as the accessor's JSDoc promises. Middleware composed at protocol.ts:1893 runs outside the wrapped handler, and the verify hook only runs inside it at server.ts:634-645. A middleware that gates or branches on that state therefore acts on attacker-controlled input even when HMAC verification is configured. Fix: run the requestState type check and verify hook before the middleware stack for every method use() can wrap, or make the accessor return undefined until verified and state in the use() JSDoc that middleware sees pre-verification context.

Why this was flagged

A server configures ServerOptions.requestState.verify and installs an extension whose middleware on tools/call reads ctx.mcpReq.requestState() to decide whether to answer itself or call next. A client retries a multi-round-trip request with a tampered requestState string. In _onrequest the context is built with requestStateAccessor(lifted.requestState), the raw string, at protocol.ts:1099, and handler(request, ctx) at protocol.ts:1130 is the composed middleware from protocol.ts:1893. Only when the middleware calls next does Server._wrapHandler reach _invokeInputRequiredCapableHandler, which runs _verifyRequestState at server.ts:634-645 and hands the decoded payload to the handler via withRequestStateValue at server.ts:643. The middleware therefore sees the unverified string while the accessor's JSDoc at protocol.ts:404-420 says it returns 'the value the configured ServerOptions.requestState.verify hook resolved with'. On the base branch there is no middleware seam, so every reader of requestState sat behind the verify hook.

Verification: _onrequest builds the context with the raw lifted string at packages/core-internal/src/shared/protocol.ts:1099. _resolveRequestHandler (:1880-1895) composes middleware around the _wrapHandler-wrapped handler; the verify hook runs only inside that wrapped handler, in Server._invokeInputRequiredCapableHandler at packages/server/src/server/server.ts:633-645. The base branch has no middleware path.

[claude]

🔴 Operators who configure an extension's settings in capabilities.extensions[id] and also list that extension in extensions silently lose those settings. server.ts:364 calls registerCapabilities({ extensions: { [extension.id]: {} } }), and mergeCapabilities (protocol.ts:2041) merges one level deep, so {} replaces whatever the user put under that id. Fix: only advertise {} when this._capabilities.extensions?.[extension.id] is not already set (or deep-merge that key), at both sites: server.ts:364 and client.ts:671. The docs say the maps merge; for this id they are overwritten. [also at: packages/client/src/client/client.ts:671 - Client users who pass extension settings in options.capabilities.extensions[id] and also list that extension in options.extensions lose those settings: the client advertises {} instead. client.ts:671 registers { extensions: { [extension.id]: {} } } after the constructor copied the user's capabilities, and mergeCapabilities merges only one level deep, so the {} replaces the user's entry.]

Why this was flagged

A user constructs new Server(info, { capabilities: { extensions: { 'com.example/gate': { exampleData: true } } }, extensions: [gate] }) (or the same on Client at client.ts:671). The constructor loop at server.ts:363-366 calls registerCapabilities({ extensions: { 'com.example/gate': {} } }). mergeCapabilities at protocol.ts:2048-2051 spreads { ...baseValue, ...addValue } only at the extensions level, so the 'com.example/gate' entry becomes {} and exampleData is gone from the advertised capability. On the base branch extensions is not a ServerOptions key and capabilities.extensions passed by the user is advertised unchanged. docs/advanced/extensions.md:44 says settings 'merge', which holds only when install registers them after the {} write, not for settings given in capabilities.

Verification: When a user passes both capabilities.extensions[id]: {...settings} and extensions: [ext] with the same id at construction, the settings are silently dropped from the advertised capabilities. server.ts:363-366 calls this.registerCapabilities({ extensions: { [extension.id]: {} } }), and mergeCapabilities (packages/core-internal/src/shared/protocol.ts:2041-2054) merges exactly one level deep. No guard or warning exists.

[claude]

🟡 (optional) Extension authors who trust the use JSDoc get a middleware that receives an unparsed, unvalidated request, so reading request.params.name on a malformed tools/call throws and the client gets -32603 instead of the -32602 the base answers. protocol.ts:1811-1812 says the middleware 'receives the parsed request', but protocol.ts:1893 runs it outside the stored handler and the server.ts:549 validateRequest step, so params may be undefined or any shape. Fix: either validate the request against the method's params schema before the middleware stack runs, or make the JSDoc and docs say the request is raw and must be checked by the middleware. [also at: packages/core-internal/src/shared/protocol.ts:1893 - Clients of a server with a tools/call extension can receive results the base branch never puts on the wire: unvalidated bodies, and input_required results that skip the capability check and the 2025 legacy shim. protocol.ts:1884-1893 composes middleware outside the stored handler, so a middleware that answers or rewrites skips the whole server.ts:498 _wrapHandler seam.]

Why this was flagged

Trigger: any server with an extension middleware on tools/call whose body reads request fields, which the JSDoc invites by calling the request 'parsed'; a client sends tools/call with missing or malformed params. The middleware at protocol.ts:1893 gets the raw JSONRPCRequest straight from _onrequest; the only prior checks are the era gate (protocol.ts:1028) and the envelope check (protocol.ts:1053), neither of which validates params. Request-shape validation for tools/call happens inside the wrapped handler at server.ts:549, i.e. only once next is called. On the base branch a malformed tools/call reaches validateRequest first and answers -32602 'Invalid tools/call request'. After the change a middleware that dereferences request.params.name throws a TypeError, which the error half at protocol.ts:1172 maps to -32603 Internal error, and the throw never reaches the base handler. Remedy: run method params validation before composing middleware, or correct the JSDoc and docs to say the request is unvalidated.

Verification: The use JSDoc at protocol.ts:1811-1812 says the middleware "receives the parsed request". But _resolveRequestHandler composes the middleware OUTSIDE the stored handler (protocol.ts:1893), and all schema validation lives inside next, for tools/call at server.ts:549-556, which throws -32602. A TypeError thrown from middleware carries no code, so protocol.ts:1172 maps it to -32603, whereas the base answers -32602.

[claude]

🟡 (optional) Operators hosting with createMcpHandler now answer an unknown method with HTTP 200 instead of 404 once an extension installs middleware on it. protocol.ts:1884-1888 turns a method with middleware but no handler into a handler that throws MethodNotFound asynchronously, so the synchronous -32601 at protocol.ts:1040 never fires. The per-request transport maps -32601 to 404 only inside its dispatch window (perRequestTransport.ts:290), and the envelope check at protocol.ts:1053 now runs first, so a missing envelope answers -32602. Fix: keep -32601 for middleware-only methods on the synchronous pre-dispatch path, e.g. return undefined from _resolveRequestHandler when base is undefined, and say in the use() JSDoc that middleware needs an underlying handler. [also at: packages/core-internal/src/shared/protocol.ts:1037 - 2026-07-28 clients that call a method an extension wraps but no handler serves now get -32602 instead of the base's -32601 when their request lacks the _meta envelope. protocol.ts:1037 returns a composed handler whenever middleware exists, so the handler-existence check at protocol.ts:1039 no longer fires and the envelope check at protocol.ts:1053 answers first.]

Why this was flagged

A server is built with an extension whose install calls server.use('tools/call', mw) but no tool is registered and capabilities.tools is not declared, so no tools/call handler exists; it is hosted via createMcpHandler on 2026-07-28, which dispatches through PerRequestHTTPServerTransport (invoke.ts:61). A client POSTs tools/call. On the base branch _onrequest finds no handler and calls sendErrorResponse(MethodNotFound) synchronously at protocol.ts:1040, inside the transport's dispatch window (perRequestTransport.ts:216-220), so LADDER_ERROR_HTTP_STATUS maps -32601 to HTTP 404. After the change _resolveRequestHandler at protocol.ts:1881-1888 returns a composed function whose base is an async thrower, so handler is defined; the -32601 is thrown from inside the Promise chain at protocol.ts:1162-1182 after the window closed, ladderStatus at perRequestTransport.ts:290 is undefined, and the error is settled with status 200 at perRequestTransport.ts:311. The envelope check at protocol.ts:1053 also now precedes the not-found answer, so the same request without a _meta envelope gets -32602 instead of -32601.

Verification: Triggered when an extension installs middleware via use() on a method that has no registered handler and the server is hosted via createMcpHandler. _resolveRequestHandler (protocol.ts:1880-1894) returns a composed function, so the handler === undefined branch at protocol.ts:1039 is skipped and the -32601 is produced after the window closed, landing on HTTP 200 at perRequestTransport.ts:311.

[claude]

🟡 (optional) Gateway-style servers whose tools call a downstream MCP server now relay the downstream's -32021 to their own client as a JSON-RPC error instead of an isError result. mcp.ts:317-323 re-throws any ProtocolError with code MissingRequiredClientCapability, but a tool that awaits a downstream callTool receives that peer's -32021 rebuilt by ProtocolError.fromError (errors.ts:93-100), so the upstream client is told it lacks a capability the gateway's own client lacked, with HTTP 400 from the per-request transport. Fix: re-throw only a -32021 this server can vouch for, e.g. check error.requiredCapabilities against the request's declared client capabilities (ctx.mcpReq.envelope) and keep the createToolError wrap when they are already declared.

Why this was flagged

A server built on McpServer registers a tool that proxies to another MCP server through a Client and awaits client.callTool(...). The downstream server answers the proxy's call with a JSON-RPC error -32021. Client _onresponse (packages/core-internal/src/shared/protocol.ts:1244) turns that into a MissingRequiredClientCapabilityError via ProtocolError.fromError (packages/core-internal/src/types/errors.ts:93-100) and the tool callback rejects with it. On the base branch the catch at mcp.ts:316 only passes through UrlElicitationRequired, so the upstream client got an isError CallToolResult. After the change mcp.ts:317-323 re-throws it, the upstream client receives error -32021 with the downstream's requiredCapabilities in data, and on PerRequestHTTPServerTransport (perRequestTransport.ts:288-296) that code is answered with HTTP 400 even though the upstream client declared everything this server needs. No safeguard distinguishes a -32021 minted by this server's gates from one relayed from a peer: the check is on the bare code.

Verification: Client _onresponse rebuilds the wire error with ProtocolError.fromError (packages/core-internal/src/shared/protocol.ts:1244), yielding a MissingRequiredClientCapabilityError; the new branch at packages/server/src/server/mcp.ts:317-322 forwards it as a JSON-RPC error. On base the same error became an in-band isError tool result via createToolError.

[claude]

🟡 (optional) If any consumer runs the client or server package on a runtime without ES2023 array methods, every inbound request on a method with middleware throws a TypeError at dispatch instead of being answered. protocol.ts:1891 calls stack.toReversed() synchronously inside _onrequest, before the Promise chain that maps errors to JSON-RPC responses, so the throw escapes into the transport's onmessage. The packages are built with lib: esnext (common/tsconfig/tsconfig.json:4) and ship a browser export (packages/client/package.json:74), and engines.node >= 20 says nothing about browsers. Fix: use [...stack].reverse() or iterate the array backwards so no ES2023 built-in is required on the request path.

Why this was flagged

When the client or server runs on an engine lacking Array.prototype.toReversed and an extension has installed middleware with use, the first request for that method reaches protocol.ts:1037 and _resolveRequestHandler evaluates stack.toReversed() at protocol.ts:1891. That call runs synchronously in _onrequest, outside the Promise.resolve().then(...) chain at protocol.ts:1129 that converts handler errors into error responses, so the TypeError propagates out of _onrequest into the transport's message callback and the request is never answered; the peer waits until its timeout. On the base branch no ES2023 built-in sat on the dispatch path. engines.node >= 20 does not constrain browser or React Native consumers; common/tsconfig/tsconfig.json:3-4 sets target/lib to esnext so nothing downlevels the call. Remedy: reverse with [...stack].reverse() or a descending index loop.

Verification: packages/core-internal/src/shared/protocol.ts:1891 calls stack.toReversed() inside _resolveRequestHandler, which _onrequest calls synchronously at protocol.ts:1037, before the Promise chain that maps handler errors to JSON-RPC error responses. _onrequest runs from the transport's onmessage wrapper at protocol.ts:832 with no try/catch, so the TypeError escapes and the request is never answered.

[claude]

🟡 nit (optional): the JSDoc and docs say only the explicit-schema request() path consults acceptResultType, but the code applies it to typed spec calls too. The check at protocol.ts:1552 lives in _requestWithSchemaViaCodec, shared by request({method}) without a schema and callTool. After an extension declares acceptResultType('tools/call','task'), a typed callTool receiving resultType: 'task' skips the codec and fails with InvalidResult instead of the codec's typed unknown-kind error. Fix: either gate the bypass on the explicit-schema entry (pass a flag from protocol.ts:1305) or correct protocol.ts:1849-1851 and docs/advanced/extensions.md:84 to say typed calls are affected too. [also at: packages/core-internal/src/shared/protocol.ts:1561 - Typed spec callers also hit the acceptResultType bypass, so the JSDoc, docs and changeset saying only the explicit-schema path consults it are false.; packages/core-internal/src/shared/protocol.ts:1849 - nit: CLAUDE.md asks that comments say what the code does: the acceptResultType JSDoc says "Only the explicit-schema request(request, resultSchema) path consults this; typed spec calls keep the closed vocabulary", but both request() overloads reach _requestWithSchemaViaCodec, where _isAcceptedResultType runs before codec.decodeResult regardless of overload.]

Why this was flagged

The JSDoc at packages/core-internal/src/shared/protocol.ts:1849-1851 states 'Only the explicit-schema request(request, resultSchema) path consults this; typed spec calls keep the closed vocabulary', and docs/advanced/extensions.md:84 repeats it. The _isAcceptedResultType check at protocol.ts:1552 is inside the response handler of _requestWithSchemaViaCodec, which protocol.ts:1312 also reaches for typed calls with codecResultValidator. So once acceptResultType('tools/call', 'task') is declared, a typed client.callTool whose raw response carries resultType: 'task' bypasses codec.decodeResult and is validated raw against the spec schema, producing SdkErrorCode.InvalidResult 'Invalid result for tools/call' (the PR's own test at packages/client/test/client/extensions.test.ts:126-127 shows this). On the base branch the 2026 codec answers that response with its typed unknown-kind error. CLAUDE.md principle 6 ('The words are true') asks that comments and docs say what the code does.

Verification: The JSDoc at packages/core-internal/src/shared/protocol.ts:1849-1851 says only the explicit-schema request() path consults this, but the _isAcceptedResultType check at protocol.ts:1552 sits inside _requestWithSchemaViaCodec, which typed request() calls also reach. A typed callTool receiving resultType: 'task' thus skips codec.decodeResult and fails with InvalidResult instead of the base's UnsupportedResultType.

[claude]

Code review found no new issues

No new issues were found in this update; 7 findings from earlier reviews are still open above.

1 optional suggestion (a nit or a note on pre-existing code) was found and not posted.

Still open from earlier reviews (7):

• 🔴 packages/core-internal/src/shared/protocol.ts:1893 — Extension middleware that reads ctx.mcpReq.requestState() gets the raw, unverified wire string, not the value the reque…
• 🔴 packages/server/src/server/server.ts:364 — Operators who configure an extension's settings in capabilities.extensions[id] and also list that extension in extensio…
• Also unresolved: 5 minor or pre-existing.

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[claude]

Code review found no new issues

No new issues were found in this update; 7 findings from earlier reviews are still open above.

1 optional suggestion (a nit or a note on pre-existing code) was found and not posted.

Still open from earlier reviews (7):

• 🔴 packages/core-internal/src/shared/protocol.ts:1901 — Extension middleware that reads ctx.mcpReq.requestState() gets the raw, unverified wire string, not the value the reque…
• 🔴 packages/server/src/server/server.ts:368 — Operators who configure an extension's settings in capabilities.extensions[id] and also list that extension in extensio…
• Also unresolved: 5 minor or pre-existing.

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[claude]

Beyond the inline finding, I also checked the new filesystem surface the Skills extension adds in packages/server/src/ext/skills/fsPath.ts and node.ts for path traversal: splitRelativePath rejects .. and empty segments, fsPathWithin re-checks containment with path.relative after the join, and node.ts opens with O_NOFOLLOW so a final-component symlink cannot escape the skills folder.

Extended reasoning...

Since the last review the PR grew from the generic extension hooks to also include the full Skills extension (SEP-2640), which serves files from a server-side folder over skill:// URIs and resources/directory/read; that path-containment guard was the new security-sensitive surface examined this run and it holds, so this note records only that ruled-out check.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Still open from earlier reviews (7):

• 🔴 packages/core-internal/src/shared/protocol.ts:1901 — Extension middleware that reads ctx.mcpReq.requestState() gets the raw, unverified wire string, not the value the reque…
• Also unresolved: 5 minor or pre-existing, 1 blocking on lines changed since (possibly already fixed).

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[claude]

🟡 nit (optional): maintainers reading the server barrel now see the SEP-2322 HMAC-sealing comment attached to the unrelated ServerExtension export. The new export type { ServerExtension } at packages/server/src/index.ts:59 was inserted between the comment at lines 57-58 and the RequestStateCodec exports it describes. Fix: move the ServerExtension export above the comment (or give it its own one-line comment) so the comment sits on the requestStateCodec exports it is about.

Why this was flagged

The diff adds export type { ServerExtension } from './server/extension'; at packages/server/src/index.ts:59, directly after the two-line comment at index.ts:57-58 that reads 'Opt-in HMAC sealing for the multi-round-trip requestState (SEP-2322): the convenience codec consumers drop into ServerOptions.requestState.verify.' That comment describes RequestStateCodec, RequestStateCodecOptions and createRequestStateCodec at index.ts:60-61, which the insertion pushes away from it. On the base branch the comment is immediately followed by the exports it describes. No runtime behaviour changes; CLAUDE.md principle 6 asks that comments say what the code does, and this one now labels the wrong export.

Verification: nit. The only hunk in packages/server/src/index.ts inserts export type { ServerExtension } from './server/extension'; at line 59, directly beneath the comment at lines 57-58 and above the RequestStateCodec exports (lines 60-61) that comment describes. After the change the comment reads as describing ServerExtension, which has nothing to do with HMAC sealing.

_{Finding ID: f-56f8ec5d}

[claude]

2 other optional suggestions (nits or notes on pre-existing code) were found and not posted.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Still open from earlier reviews (8):

• 🔴 packages/core-internal/src/shared/protocol.ts:1901 — Extension middleware that reads ctx.mcpReq.requestState() gets the raw, unverified wire string, not the value the reque…
• Also unresolved: 6 minor or pre-existing, 1 blocking on lines changed since (possibly already fixed).

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[claude]

🔴 Extension authors can register a handler for a core method such as elicitation/create through ctx.method, replacing the SDK's typed handler, although the JSDoc and docs say it throws. The core-method check at packages/client/src/client/extension.ts:161 only asks the 2026-07-28 codec, whose registry holds no client-served methods and omits legacy-only server methods. Fix: refuse every spec method of any era (an isSpecRequestMethod-style check across both codecs), while still allowing the SEP-2663 reused names tasks/get and tasks/cancel that the Tasks extension registers with explicit schemas, which covers the 2 sites listed. Same pattern at 2 sites (packages/client/src/client/extension.ts:161, packages/server/src/server/extension.ts:189). [also at: packages/server/src/server/extension.ts:188 - Extension authors who call ctx.method with a legacy-only core method name get it registered instead of the refusal the JSDoc and docs promise.]

Why this was flagged

A client built with new Client(info, { capabilities: { elicitation: {} }, extensions: [ext] }) whose setup calls ctx.method('elicitation/create', { params: z.looseObject({}) }, handler). The guard at packages/client/src/client/extension.ts:161 calls codecForVersion(MODERN_WIRE_REVISION).hasRequestMethod(name); the 2026 registry at packages/core-internal/src/wire/rev2026-07-28/registry.ts:45-56 lists only ten server-side methods, so elicitation/create passes. client.assertCanSetRequestHandler at extension.ts:164 only fails when a handler already exists, and Client.assertRequestHandlerCapability at packages/client/src/client/client.ts:1558 passes once the capability is declared, so client.setRequestHandler('elicitation/create', schemas, handler) installs a custom-schema handler for a spec method. The JSDoc at extension.ts:81-84 and docs/advanced/extensions.md:19 promise a throw for a core method. The PR's own test only exercises tools/call on the client (packages/client/test/client/extensions.test.ts:106), which the client never serves.

Verification: packages/client/src/client/extension.ts:160 guards only with codecForVersion(MODERN_WIRE_REVISION).hasRequestMethod(name); that registry (packages/core-internal/src/wire/rev2026-07-28/registry.ts:45-56) holds only ten server-served methods. So with capabilities: { elicitation: {} }, elicitation/create passes the core check and is installed, contrary to the JSDoc (extension.ts:80-83).

_{Finding ID: f-fc33ef97}

[claude]

🟡 nit (optional): readers who copy the documented gate extension get a server that refuses every 2025-era client, even one that declared the extension in initialize. The example at docs/advanced/extensions.md:48 reads only requestCtx.mcpReq.envelope, which a legacy request never carries, so the -32021 throw at line 50 fires on every legacy tools/call. The same guide promises legacy clients advertise the extension in initialize (line 98). Fix: make the example era-aware, falling back to ctx.server.getClientCapabilities() when the envelope is absent, or state that the gate only serves 2026-07-28 connections.

Why this was flagged

A server built from the docs example installs gate and is connected on a 2025-era session. A 2025 client built with this SDK and extensions: [gateClient] sends capabilities.extensions['com.example/gate'] in initialize (packages/client/src/client/client.ts:1095), as docs/advanced/extensions.md:98 says it does. On a legacy request _onrequest sets mcpReq.envelope only when the request carried reserved _meta keys (packages/core-internal/src/shared/protocol.ts:1089), so requestCtx.mcpReq.envelope is undefined and declared is {} at docs/advanced/extensions.md:48. The middleware throws MissingRequiredClientCapabilityError at line 50 and the client gets -32021 for every tools/call. The server's own gates use the era-aware _inputRequestCapabilityView (packages/server/src/server/server.ts:742-746); that view is private, and the example neither uses the public getClientCapabilities() nor says it is 2026-only. On the base branch there is no extension guide, so no reader is steered into this.

Verification: nit — triggered whenever a server built from the documented gate example serves a 2025-era client. docs/advanced/extensions.md:48-50 reads only requestCtx.mcpReq.envelope?.[CLIENT_CAPABILITIES_META_KEY]?.extensions ?? {} and throws MissingRequiredClientCapabilityError when the id is absent. A legacy client never sends an envelope, so on every legacy tools/call declared is {} and the gate throws -32021.

_{Finding ID: f-26792aac}