-
Notifications
You must be signed in to change notification settings - Fork 1.8k
refactor(client,server): move stdio transports to ./stdio subpath export #1871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
felixweinberger
wants to merge
5
commits into
main
Choose a base branch
from
fweinberger/browser-stdio-conditional-export
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
46fe6d2
refactor(client,server): move stdio transports to ./stdio subpath export
felixweinberger 35b3755
test: make barrelClean chunk scan transitive (BFS) and CI-safe; map /…
felixweinberger dc84940
fix: add /stdio tsconfig paths in examples/{client,server} and test/i…
felixweinberger c2b85db
test(server): assert StdioServerTransport absent from root barrel; cl…
felixweinberger a4a6100
Merge branch 'main' into fweinberger/browser-stdio-conditional-export
KKonstantinov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@modelcontextprotocol/client': minor | ||
| '@modelcontextprotocol/server': minor | ||
| --- | ||
|
|
||
| Move stdio transports to a `./stdio` subpath export. Import `StdioClientTransport`, `getDefaultEnvironment`, `DEFAULT_INHERITED_ENV_VARS`, and `StdioServerParameters` from `@modelcontextprotocol/client/stdio`, and `StdioServerTransport` from `@modelcontextprotocol/server/stdio`. The package root entries no longer pull in `node:child_process`, `node:stream`, or `cross-spawn`, fixing bundling for browser and Cloudflare Workers targets. Node.js, Bun, and Deno consumers update the import path; runtime behavior is unchanged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // Subpath entry for the stdio client transport. | ||
| // | ||
| // Exported separately from the root entry so that bundling `@modelcontextprotocol/client` for browser or | ||
| // Cloudflare Workers targets does not pull in `node:child_process`, `node:stream`, or `cross-spawn`. Import | ||
| // from `@modelcontextprotocol/client/stdio` only in process-spawning runtimes (Node.js, Bun, Deno). | ||
|
|
||
| export type { StdioServerParameters } from './client/stdio.js'; | ||
| export { DEFAULT_INHERITED_ENV_VARS, getDefaultEnvironment, StdioClientTransport } from './client/stdio.js'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { execFileSync } from 'node:child_process'; | ||
| import { existsSync, readFileSync } from 'node:fs'; | ||
| import { dirname, join } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| import { beforeAll, describe, expect, test } from 'vitest'; | ||
|
|
||
| const pkgDir = join(dirname(fileURLToPath(import.meta.url)), '../..'); | ||
| const distDir = join(pkgDir, 'dist'); | ||
| const NODE_ONLY = /\b(child_process|cross-spawn|node:stream|node:child_process)\b/; | ||
|
|
||
| function chunkImportsOf(entryPath: string): string[] { | ||
| const visited = new Set<string>(); | ||
| const queue = [entryPath]; | ||
| while (queue.length > 0) { | ||
| const file = queue.shift()!; | ||
| if (visited.has(file)) continue; | ||
| visited.add(file); | ||
| const src = readFileSync(file, 'utf8'); | ||
| for (const m of src.matchAll(/from\s+["']\.\/(.+?\.mjs)["']/g)) { | ||
| queue.push(join(dirname(file), m[1]!)); | ||
| } | ||
| } | ||
| visited.delete(entryPath); | ||
| return [...visited]; | ||
| } | ||
|
|
||
| describe('@modelcontextprotocol/client root entry is browser-safe', () => { | ||
| beforeAll(() => { | ||
| if (!existsSync(join(distDir, 'index.mjs')) || !existsSync(join(distDir, 'stdio.mjs'))) { | ||
| execFileSync('pnpm', ['build'], { cwd: pkgDir, stdio: 'inherit' }); | ||
| } | ||
| }, 60_000); | ||
|
|
||
| test('dist/index.mjs contains no process-spawning runtime imports', () => { | ||
| const entry = join(distDir, 'index.mjs'); | ||
| expect(readFileSync(entry, 'utf8')).not.toMatch(NODE_ONLY); | ||
| }); | ||
|
|
||
| test('chunks transitively imported by dist/index.mjs contain no process-spawning runtime imports', () => { | ||
| const entry = join(distDir, 'index.mjs'); | ||
| for (const chunk of chunkImportsOf(entry)) { | ||
| expect({ chunk, content: readFileSync(chunk, 'utf8') }).not.toEqual( | ||
| expect.objectContaining({ content: expect.stringMatching(NODE_ONLY) }) | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| test('dist/stdio.mjs exists and exports StdioClientTransport', () => { | ||
| const stdio = readFileSync(join(distDir, 'stdio.mjs'), 'utf8'); | ||
| expect(stdio).toMatch(/\bStdioClientTransport\b/); | ||
| expect(stdio).toMatch(/\bgetDefaultEnvironment\b/); | ||
| expect(stdio).toMatch(/\bDEFAULT_INHERITED_ENV_VARS\b/); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // Subpath entry for the stdio server transport. | ||
| // | ||
| // Exported separately from the root entry to keep `StdioServerTransport` out of the default bundle | ||
| // surface — server stdio has only type-level Node imports, but matching the client's `./stdio` | ||
| // subpath gives consumers a consistent shape across packages. Import from | ||
| // `@modelcontextprotocol/server/stdio` only in process-stdio runtimes (Node.js, Bun, Deno). | ||
|
|
||
| export { StdioServerTransport } from './server/stdio.js'; | ||
|
felixweinberger marked this conversation as resolved.
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 The
shared/stdio.jsrow should keep pointing at the package root, not the new/stdiosubpath. v1'sshared/stdio.jsexportedReadBuffer,serializeMessage, anddeserializeMessage, which in v2 are still re-exported from the root of@modelcontextprotocol/clientand@modelcontextprotocol/server(viacore/public); the new./stdiosubpaths export only the transport classes. Following this row as written producesimport { ReadBuffer } from '@modelcontextprotocol/client/stdio', which fails to resolve.Extended reasoning...
What the bug is
This PR updates three rows in the
docs/migration-SKILL.mdimport-mapping table to point at the new./stdiosubpath. Two of those updates (sdk/client/stdio.js→@modelcontextprotocol/client/stdioandsdk/server/stdio.js→@modelcontextprotocol/server/stdio) are correct. The third —@modelcontextprotocol/sdk/shared/stdio.js→@modelcontextprotocol/client/stdioor@modelcontextprotocol/server/stdio— is wrong and is a regression introduced by this PR.Why it's wrong
In v1,
@modelcontextprotocol/sdk/shared/stdio.jsexported the message-framing utilities, not transports. The current source atpackages/core/src/shared/stdio.tsconfirms the export set:ReadBuffer(line 7),deserializeMessage(line 44), andserializeMessage(line 48).In v2, those three symbols are re-exported from
packages/core/src/exports/public/index.ts:68:Both
packages/client/src/index.tsandpackages/server/src/index.tsend withexport * from '@modelcontextprotocol/core/public';, so these symbols are available from the root of@modelcontextprotocol/clientand@modelcontextprotocol/server— exactly what the pre-PR table row said.The new
./stdiosubpaths created in this PR do not re-export these utilities.packages/client/src/stdio.tsexports onlyStdioServerParameters,DEFAULT_INHERITED_ENV_VARS,getDefaultEnvironment, andStdioClientTransport.packages/server/src/stdio.tsexports onlyStdioServerTransport. Neither exportsReadBuffer,serializeMessage, ordeserializeMessage.Why nothing catches this
migration-SKILL.mdis an LLM-consumed mapping table, not type-checked code. ThebarrelCleantests added in this PR verify the bundle contents but say nothing about which v2 module each v1 path's symbols actually live in, so the doc regression slips through.Impact
This file is explicitly designed to be loaded as a skill so an LLM can mechanically rewrite imports during a v1→v2 migration. A user with v1 code like
import { ReadBuffer, serializeMessage } from '@modelcontextprotocol/sdk/shared/stdio.js';(custom transport authors are the typical consumers) will have it rewritten toimport { ReadBuffer, serializeMessage } from '@modelcontextprotocol/client/stdio';, which fails with "Module has no exported member 'ReadBuffer'". The pre-PR mapping (root package) worked; this PR breaks it.Step-by-step proof
import { ReadBuffer } from '@modelcontextprotocol/sdk/shared/stdio.js';docs/migration-SKILL.md:69and rewrites toimport { ReadBuffer } from '@modelcontextprotocol/client/stdio';@modelcontextprotocol/client/stdioresolves (perpackages/client/package.jsonexports) todist/stdio.mjs, built frompackages/client/src/stdio.tsStdioClientTransport,getDefaultEnvironment,DEFAULT_INHERITED_ENV_VARS, and theStdioServerParameterstype — noReadBufferModule '"@modelcontextprotocol/client/stdio"' has no exported member 'ReadBuffer'import { ReadBuffer } from '@modelcontextprotocol/client';, which resolves viacore/public/index.ts:68How to fix
Revert just this one row to its pre-PR value:
(Alternatively, also re-export
ReadBuffer/serializeMessage/deserializeMessagefrom both./stdiosubpaths — but since these are pure framing utilities with no Node-only deps, keeping them at the root is consistent with the rest of the "shared" rows and avoids forcing a Node-only subpath import where none is needed.)