feat(compat): export InMemoryTransport from public API surface#1834
feat(compat): export InMemoryTransport from public API surface#1834felixweinberger merged 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: fbf1fa3 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: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
felixweinberger
force-pushed
the
fweinberger/migration-doc-fixes
branch
from
32d24b0 to
5ecd8b9
Compare
felixweinberger
changed the title
felixweinberger
force-pushed
the
fweinberger/migration-doc-fixes
branch
from
5ecd8b9 to
9a350f9
Compare
There was a problem hiding this comment.
Two of my prior inline comments were incorrect and should be disregarded: the InMemoryTransport comment at line 502 (the PR correctly adds it to the public barrel, making the docs claim valid) and the transport-scope comment at line 135 (the v1 Before example correctly uses let at module scope). The real issues to address are in the After (v2) SSE example — see the new inline comments for details.
Extended reasoning...
Overview
This PR touches three files: docs/migration-SKILL.md, docs/migration.md, and packages/core/src/exports/public/index.ts. The core code change is adding InMemoryTransport to the public barrel export. The doc changes fill gaps in the migration guide: fixing the core package description, adding an SSE to StreamableHTTP example, adding missing schema-to-method-string table rows, and noting callToolStream() drops the schema parameter.
Prior review corrections
Both of my previous inline comments were based on incorrect analysis:
- Line 502 (InMemoryTransport): I claimed the export did not exist, but the PR adds it to
packages/core/src/exports/public/index.ts, which is re-exported by both@modelcontextprotocol/serverand@modelcontextprotocol/client. The docs are correct after this PR. - Line 135 (transport scope): I claimed
transportwas declared asconstinside the GET callback, but the v1 Before example declareslet transport: SSEServerTransport;at module scope. The example is correct.
Real issues (new inline comments)
The After (v2) SSE example at lines 131-139 has two real problems: randomUUID is called without an import from node:crypto (runtime ReferenceError on first request), and creating a new NodeStreamableHTTPServerTransport per request with sessionIdGenerator: () => randomUUID() breaks session continuity because each fresh transport instance starts with sessionId === undefined, causing all follow-up requests to fail with HTTP 400/404. The new inline comments on this PR capture these issues correctly.
Security risks
None -- this is a documentation and public export change with no security-sensitive logic.
Level of scrutiny
The InMemoryTransport export addition is a small, appropriate API surface change. Most doc fixes are correct. The SSE example needs the missing import and a corrected transport pattern before merging.
felixweinberger
force-pushed
the
fweinberger/migration-doc-fixes
branch
2 times, most recently
from
26b1ddc to
c9c03a9
Compare
|
@claude review |
There was a problem hiding this comment.
LGTM — previous feedback all addressed; remaining inline notes are minor nits (changeset, export ordering).
Extended reasoning...
Overview
This PR does two things: (1) adds InMemoryTransport to the curated public barrel at packages/core/src/exports/public/index.ts so it surfaces via @modelcontextprotocol/server and @modelcontextprotocol/client, and (2) tightens the v1→v2 migration docs (docs/migration.md, docs/migration-SKILL.md) with a working SSE→StreamableHTTP example, additional method-string table rows, a callToolStream schema-removal example, and a corrected InMemoryTransport section. A 3-line doc comment is added to inMemory.ts.
Prior feedback resolved
I left several comments on earlier revisions. At HEAD all of them are addressed: the v1 SSE example now declares let transport in outer scope; the v2 example uses sessionIdGenerator: undefined (stateless) with prose explaining when to use a session Map; the InMemoryTransport section heading was renamed and the export was actually added to the public barrel; callToolStream is in both docs' section-11 prose and tables; and the InitializedNotificationSchema row was added to migration.md.
Security risks
None. The only runtime change is re-exporting an existing testing transport class — no auth, crypto, network, or input-handling logic is touched.
Level of scrutiny
Low-to-moderate. ~90% of the diff is documentation prose. The single code change (one new export { InMemoryTransport } line) follows the exact pattern already used for InMemoryTaskStore/InMemoryTaskMessageQueue in the same file, so there's no novel design decision here. The class itself is unchanged apart from a JSDoc note.
Other factors
The two remaining inline notes are cosmetic: a missing changeset (already flagged by changeset-bot; easy to add via the bot's link) and the new export line splitting the transport.js comment group. Neither affects correctness. CI is green per the PR description, and the doc snippets were verified against the referenced source files.
|
A needed one - |
Reframe both migration.md and migration-SKILL.md to lead with the backwards-compatible upgrade path: bump @modelcontextprotocol/sdk to ^2 and let deprecation warnings guide further migration. New content: - Prerequisites section: zod ^4.2.0 runtime-crash callout, moduleResolution bundler/nodenext requirement, bun file: cache note - "core is bundled" wording (never appears in node_modules) - Zod schemas no longer exported → parseJSONRPCMessage / specTypeSchema / isSpecType mapping; /zod-schemas escape hatch - Custom-method handler registration + spec-method extension warning - Transitive-v1-dependency type-mix guidance - OAuthError.errorCode → .code rename Mapping-table rows added: ResourceTemplateType, ZodRawShapeCompat, RequestInfo, FetchLike, OAuthProtectedResourceMetadata, server/auth/errors.js, server/express.js, inMemory.js, roots/list, completion/complete, resources/subscribe|unsubscribe|templates/list, tasks/get|result, notifications/roots/list_changed, notifications/elicitation/complete, callToolStream. Absorbs the doc edits from #1834 (SSE before/after example, InMemoryTransport export location, validator import simplification, method-string and callToolStream additions).
Slimmed reproduction of #1834 with only the code change (doc edits move to F1). InMemoryTransport was previously unreachable by end users — the migration guide pointed at it for in-process testing, but core is a private package with no published exports. Adding it to core/public makes it available via the @modelcontextprotocol/server and @modelcontextprotocol/client re-export barrels. Also adds the JSDoc note clarifying it is intended for testing, and a regression test asserting the public-barrel export resolves to the same class.
felixweinberger
force-pushed
the
fweinberger/migration-doc-fixes
branch
from
aaa3352 to
a0723c7
Compare
felixweinberger
changed the title
Slimmed reproduction of #1834 with only the code change (doc edits move to F1). InMemoryTransport was previously unreachable by end users — the migration guide pointed at it for in-process testing, but core is a private package with no published exports. Adding it to core/public makes it available via the @modelcontextprotocol/server and @modelcontextprotocol/client re-export barrels. Also adds the JSDoc note clarifying it is intended for testing, and a regression test asserting the public-barrel export resolves to the same class.
felixweinberger
force-pushed
the
fweinberger/migration-doc-fixes
branch
from
a0723c7 to
5dc3338
Compare
Reframe both migration.md and migration-SKILL.md to lead with the backwards-compatible upgrade path: bump @modelcontextprotocol/sdk to ^2 and let deprecation warnings guide further migration. New content: - Prerequisites section: zod ^4.2.0 runtime-crash callout, moduleResolution bundler/nodenext requirement, bun file: cache note - "core is bundled" wording (never appears in node_modules) - Zod schemas no longer exported → parseJSONRPCMessage / specTypeSchema / isSpecType mapping; /zod-schemas escape hatch - Custom-method handler registration + spec-method extension warning - Transitive-v1-dependency type-mix guidance - OAuthError.errorCode → .code rename Mapping-table rows added: ResourceTemplateType, ZodRawShapeCompat, RequestInfo, FetchLike, OAuthProtectedResourceMetadata, server/auth/errors.js, server/express.js, inMemory.js, roots/list, completion/complete, resources/subscribe|unsubscribe|templates/list, tasks/get|result, notifications/roots/list_changed, notifications/elicitation/complete, callToolStream. Absorbs the doc edits from #1834 (SSE before/after example, InMemoryTransport export location, validator import simplification, method-string and callToolStream additions).
Reframe both migration.md and migration-SKILL.md to lead with the backwards-compatible upgrade path: bump @modelcontextprotocol/sdk to ^2 and let deprecation warnings guide further migration. New content: - Prerequisites section: zod ^4.2.0 runtime-crash callout, moduleResolution bundler/nodenext requirement, bun file: cache note - "core is bundled" wording (never appears in node_modules) - Zod schemas no longer exported → parseJSONRPCMessage / specTypeSchema / isSpecType mapping; /zod-schemas escape hatch - Custom-method handler registration + spec-method extension warning - Transitive-v1-dependency type-mix guidance - OAuthError.errorCode → .code rename Mapping-table rows added: ResourceTemplateType, ZodRawShapeCompat, RequestInfo, FetchLike, OAuthProtectedResourceMetadata, server/auth/errors.js, server/express.js, inMemory.js, roots/list, completion/complete, resources/subscribe|unsubscribe|templates/list, tasks/get|result, notifications/roots/list_changed, notifications/elicitation/complete, callToolStream. Absorbs the doc edits from #1834 (SSE before/after example, InMemoryTransport export location, validator import simplification, method-string and callToolStream additions).
There was a problem hiding this comment.
LGTM — fbf1fa3 addresses my last comment by including the migration.md fix here so the export and its migration-guide entry land atomically; the migration-SKILL.md row noted inline is a non-blocking follow-up.
Extended reasoning...
Overview
This PR exports InMemoryTransport from the curated public barrel (packages/core/src/exports/public/index.ts), making it importable from @modelcontextprotocol/server and @modelcontextprotocol/client. It also adds a usage-guidance JSDoc paragraph to inMemory.ts, a changeset bumping server/client (patch), and rewrites the InMemoryTransport section of docs/migration.md to reflect the new public import path.
Progress since last review
My open inline comment on public/index.ts:73 flagged that, after the doc edits were split out to F1, this PR would land an export while migration.md still said the symbol was removed. Commit fbf1fa3 brought the minimal migration.md fix back into this PR — heading now reads "InMemoryTransport moved" and the v2 snippet imports from @modelcontextprotocol/server / @modelcontextprotocol/client. I verified both packages export * from '@modelcontextprotocol/core/public' (server/src/index.ts:50, client/src/index.ts:81), so the doc claim is correct. The earlier grouping nit (89418d8) and missing-changeset nit are also resolved.
Security risks
None. This is a one-line re-export of an existing, already-tested in-process transport class plus prose/JSDoc. No auth, crypto, network, or input-handling changes.
Level of scrutiny
Low-to-moderate. It does add a public export, which REVIEW.md treats as a deliberate decision — but this is a v1-parity restoration (v1 shipped it at sdk/inMemory.js), it's part of the labeled v2-bc series, and a maintainer explicitly endorsed it in-thread ("A needed one — useful for users' integration test suites"). The implementation is mechanical: one export line placed under the existing Transport block, no behavioral change to InMemoryTransport itself.
Other factors
The remaining inline nit (migration-SKILL.md lacks a sdk/inMemory.js → client/server row in its import-mapping table) is real per CLAUDE.md's dual-doc convention but non-blocking — TypeScript would immediately surface the unresolved import, and migration.md now covers it. Fine as a one-line follow-up here or in F1.
| ### `InMemoryTransport` moved | ||
|
|
||
| `InMemoryTransport` has been removed from the public API surface. It was previously used for in-process client-server connections and testing. | ||
|
|
||
| For **testing**, import it directly from the internal core package: | ||
| `InMemoryTransport` is now exported from `@modelcontextprotocol/client` and `@modelcontextprotocol/server` (both re-export it). It is still intended for in-process client-server connections and testing. | ||
|
|
||
| ```typescript | ||
| // v1 | ||
| import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; | ||
|
|
||
| // v2 (testing only — @modelcontextprotocol/core is internal, not for production use) | ||
| import { InMemoryTransport } from '@modelcontextprotocol/core'; | ||
| // v2 | ||
| import { InMemoryTransport } from '@modelcontextprotocol/server'; | ||
| // or | ||
| import { InMemoryTransport } from '@modelcontextprotocol/client'; | ||
| ``` |
There was a problem hiding this comment.
🟡 nit: docs/migration.md now has the InMemoryTransport v1→v2 mapping, but docs/migration-SKILL.md still has no entry for it — its section 3 "Types / shared imports" table (lines 62-69) lists sdk/types.js, sdk/shared/*.js etc. but not @modelcontextprotocol/sdk/inMemory.js. CLAUDE.md:27-30 requires migration mappings in both files; consider adding a row | @modelcontextprotocol/sdk/inMemory.js | @modelcontextprotocol/client or @modelcontextprotocol/server | to the SKILL doc's shared-imports table.
Extended reasoning...
What the issue is
Commit fbf1fa3 brought docs/migration.md back into this PR with the corrected InMemoryTransport section (heading now "InMemoryTransport moved", v2 import shown as @modelcontextprotocol/server / @modelcontextprotocol/client). That resolves the primary concern from the still-open comment on public/index.ts:73. However, the companion docs/migration-SKILL.md was not updated and contains zero references to InMemoryTransport or sdk/inMemory.js (verified via grep).
The specific gap
migration-SKILL.md section 3 "Import Mapping" → "Types / shared imports" (lines 62-69) is the table an LLM-driven mechanical migration scans to rewrite v1 import paths. It currently has rows for:
@modelcontextprotocol/sdk/types.js@modelcontextprotocol/sdk/shared/protocol.js@modelcontextprotocol/sdk/shared/transport.js@modelcontextprotocol/sdk/shared/uriTemplate.js@modelcontextprotocol/sdk/shared/auth.js@modelcontextprotocol/sdk/shared/stdio.js
…but no row for @modelcontextprotocol/sdk/inMemory.js, which is exactly the v1 path that migration.md now documents ("// v1 → import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'").
Why the repo's process expects this
CLAUDE.md:27-30 states: "When making breaking changes, document them in both: docs/migration.md … docs/migration-SKILL.md". REVIEW.md's Tests & docs checklist repeats the dual-doc requirement. This PR now updates one but not the other.
Relationship to the open comment on public/index.ts:73
That comment's primary claim — that migration.md still says "removed from public API" and points at the internal @modelcontextprotocol/core package — is now stale after fbf1fa3. The SKILL.md gap was only mentioned as a side note buried in step 5 of its proof. Since the headline issue is resolved but the SKILL.md half of the dual-doc requirement remains unaddressed, surfacing it as a focused follow-up here.
Step-by-step proof
- v1 user code contains
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';. - An LLM agent loads
docs/migration-SKILL.md(the filemigration.mditself points to as "LLM-optimized … for tools like Claude Code to mechanically apply all the changes"). - The agent scans section 3's import-mapping tables for
@modelcontextprotocol/sdk/inMemory.js. It is absent from both the client/server tables and the "Types / shared imports" table at lines 62-69. - Grep confirms
InMemoryTransportappears nowhere in the file, so no other section provides a fallback rewrite rule. - The agent leaves the v1 import unchanged → TypeScript emits
Cannot find module '@modelcontextprotocol/sdk/inMemory.js'after the package swap.
Impact
Low — TypeScript would immediately flag the unresolved import and the fix is obvious from migration.md. This is a doc-completeness nit, not a code defect, but it does violate the explicit dual-doc convention in CLAUDE.md.
How to fix
Add one row to migration-SKILL.md's "Types / shared imports" table (after line 69):
| `@modelcontextprotocol/sdk/inMemory.js` | `@modelcontextprotocol/client` or `@modelcontextprotocol/server` |
2 participants
Part of the v2 backwards-compatibility series — see reviewer guide.
Exports
InMemoryTransportfrom the public API surface (was internal-only). Doc edits previously in this PR moved to the migration-guide overhaul PR.Motivation and Context
InMemoryTransport.createLinkedPair()is the standard way to test client/server in-process. v1 exported it fromsdk/inMemory.js; v2 had it only in the internal core barrel.How Has This Been Tested?
pnpm typecheck:all && pnpm test:allgreenBreaking Changes
None — additive.
Types of changes
Additional context
Stacks on: none. Slimmed from original — doc edits moved to F1.