Export Protocol and mergeCapabilities from client/server package roots#2501
Conversation
🦋 Changeset detectedLatest commit: f914a71 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, one candidate was examined and ruled out: the concern that the newly-public Protocol rewrites handler-thrown -32002 to -32602 on custom methods. The 2025 codec's encodeErrorCode does apply that mapping era-blind, but it matches what the deployed v1.x SDK already emitted, so a v1 Protocol subclass migrating to this export sees identical wire behavior — no regression.
Extended reasoning...
Bugs were found (three nit-level findings, posted inline), so no approval — and independently, making the Protocol engine class public API is a deliberate surface-expansion decision a maintainer should sign off on, even though the ext-apps justification and test coverage (barrel/root pins, no-initialize-on-the-wire behavior pin, codemod suite) are solid. This note records the one refuted candidate so a later review pass does not re-derive it: I confirmed in packages/core-internal/src/wire/rev2025-11-25/codec.ts:137-141 that the -32002→-32602 mapping is documented as matching deployed v1.x behavior, so it is not a behavior change introduced by this PR.
There was a problem hiding this comment.
Beyond the inline nits, this pass also examined the engine's handler-thrown error-code path for the newly public Protocol extension point: the 2025 codec's encodeErrorCode does remap a thrown -32002 to -32602 on custom methods, but this matches what the deployed v1.x SDK already emits (per the comment at packages/core-internal/src/wire/rev2025-11-25/codec.ts:137-141), so it is not a v1→v2 behavior change for Protocol subclassers and was ruled out. Also confirmed 81c5528 addresses all three findings from the previous review round (reserved wire-vocabulary caveat in changeset/docs, the symbolHints codemod diagnostic, and the new custom-methods.md section).
Extended reasoning...
This run's two findings are advisory nits (a docs-fence import completeness issue and incomplete symbolHints coverage in the codemod's dynamic-import/mock/star-re-export paths); they are posted inline. The one refuted candidate — the -32002→-32602 remap on custom methods — was verified against the codec source: the remap is unconditional but identical to v1.x behavior, so the newly exported Protocol class introduces no regression there. The PR itself is a deliberate public-API-surface expansion (minor bumps across client/server/core-internal), so a maintainer should still make the final call; the inline nits don't block that.
There was a problem hiding this comment.
Beyond the inline nits, two concerns were examined and ruled out this run: (1) root-entry runtime-neutrality — shared/protocol was already in the public barrel's runtime graph via the DEFAULT_REQUEST_TIMEOUT_MSEC value export, so promoting the Protocol class adds no new module to the root entry, and both packages' barrelClean.test.ts scan the built dist/index.mjs chunks for Node-only imports; (2) the codemod tests' beforeAll/afterAll mutation of the shared IMPORT_MAP uses a unique synthetic key and deletes it after, so no state leaks across suites.
Extended reasoning...
Bugs were found this run (four nits, posted as inline comments), so no approval decision is being made — the PR also expands the public API surface (Protocol/mergeCapabilities on both package roots), which is a design call for maintainers under this repo's minimalism conventions. This note records only what else was examined and ruled out: CLAUDE.md requires package root entries to stay runtime-neutral for browser/Workers bundlers, and exporting the Protocol class value (previously type-only plus one constant from that module) could plausibly have pulled new runtime modules into the root graph — verified it does not, and the existing barrelClean pins would catch a regression. The IMPORT_MAP test-mutation pattern introduced by the re-pinned removedSymbols tests was also checked for cross-suite leakage and is clean.
ext-apps subclasses Protocol directly and was left without any v2 import path for it. Restore the export on the curated public barrel, which both package roots re-export. The client and server packages each bundle their own compiled copy of the class, so consumers should import it from one package consistently within a process. The codemod now rewrites Protocol/mergeCapabilities imports from shared/protocol.js to the package root like the module's other symbols, instead of dropping them with an action-required marker; its removedSymbols machinery keeps test coverage via a synthetic mapping. Export pins on the barrel and both package roots prevent silent regression.
9dc4581 to
f914a71
Compare
|
Rescoped to the minimal export: dropped the docs/examples section and the codemod import-consistency hints from earlier revisions. We deliberately aren't advertising Protocol as an extension surface — this PR only restores the import so ext-apps isn't blocked, no more. |
1 participant
Makes the
Protocolbase class andmergeCapabilitiesimportable from the@modelcontextprotocol/clientand@modelcontextprotocol/serverpackage roots.Motivation and Context
ext-apps subclasses
Protocoldirectly (see modelcontextprotocol/ext-apps#710) and v2 left it without any import path for the class. This restores the export in the minimal way: one line on the curated public barrel, which both package roots already re-export. Deliberately nothing more — no new documentation or examples advertisingProtocolas an extension surface;Client/Serverremain the supported way to use the SDK.The codemod is updated to match:
Protocol/mergeCapabilitiesimports fromshared/protocol.jsnow rewrite to the package root like the module's other symbols, instead of being deleted with a marker claiming there is no v2 export.How Has This Been Tested?
Protocolsubclass pair exchanges custom methods with no MCP handshake on the wire (the property ext-apps depends on).Breaking Changes
None — additive export. Note: the client and server packages each bundle their own compiled copy of the class, so import it from one package consistently within a process.
Types of changes
Checklist