refactor: move the schema source modules into @modelcontextprotocol/core#2477
Merged
Conversation
The MCP spec schemas (types/schemas.ts), the OAuth/OpenID schemas (shared/auth.ts), the protocol constants (types/constants.ts), and the three JSON value types move verbatim from core-internal into packages/core/src — core now owns the schema sources instead of bundling them out of core-internal through build-only subpath aliases. Mechanism: - core's curated root entry re-exports the same public surface as before (unchanged 172 exports), now from its own local modules. - A new ./internal subpath re-exports the moved modules wholesale for the sibling SDK packages (internal helper schemas, auth types, constants, JSON types — names that are deliberately not public on the root entry). - core-internal keeps the old module paths as one-to-one named re-export shims, so no importer or test changes anywhere. - client/server/server-legacy declare @modelcontextprotocol/core as a real dependency; their bundles keep @modelcontextprotocol/core/internal as an external runtime import (explicit tsdown external entry) instead of carrying their own bundled schema copies. - The build-only core-internal/schemas + core-internal/auth tsconfig/tsdown aliases are deleted everywhere; the per-package tsconfigs gain a single source-first alias for @modelcontextprotocol/core/internal so typecheck and vitest stay build-order independent. Layering stays acyclic: core depends only on zod; core-internal depends on core; the wire era modules are untouched (their frozen copies and their runtime constants imports resolve through the shims unchanged).
…home Test-only adaptations to the schema-source move; each guard keeps pinning the same invariant, only pointed at the new canonical location: - packageTopologyPins: pin @modelcontextprotocol/core's export map as ['.', './internal'] — the internal seam is deliberate, not public API. - coreSchemas: read the spec-schema source from core's own src/schemas.ts (the auth group still reads core-internal's authSchemas registry). - wireOnlyHiding: read the @deprecated task-schema and constants sources from packages/core/src (the old paths are now re-export shims with no doc comments to scan). - codemod authSchemaNames: core's barrel now ends its auth block with "} from './auth'" instead of the deleted build-only alias specifier. - cloudflareWorkers: generalize packServerPackage to packWorkspacePackage and install the workspace core tarball alongside the server tarball — the packed server resolves @modelcontextprotocol/core/internal at runtime, which the registry copy of core does not carry yet.
The schema sources moved into @modelcontextprotocol/core with the sibling packages resolving @modelcontextprotocol/core/internal at runtime. That seam can rot in ways no existing test catches; this locks it down: - Exact sibling pins: client/server/server-legacy (and core-internal, for consistency) depend on core via workspace:* so pnpm publishes an exact version pin instead of a caret range. The ./internal surface is only guaranteed for the core version each sibling was built against, so a caret range would let installs mix skewed versions. - Changesets fixed group for core + client + server + server-legacy, keeping the pinned versions releasable in lockstep. - ./internal is labeled as an SDK-internal contract in its header, with @internal JSDoc on the re-exports (source-level only: the dts bundler flattens the re-exports and drops statement-level comments, so the built declarations do not carry the tag). - A client boundary test parses the built dists in both directions: every name the client bundle imports or re-exports from core must resolve against core's built entry export lists (skew), and sentinel schemas that exist only in core's modules must never appear as definitions in the client bundle (re-inlining via lost external config or eager aliases). - A core-internal shim-purity test pins the old schema/auth/constants module paths as pure re-export forwards: no zod import, no local definitions, imports only from @modelcontextprotocol/core/internal — so new schemas can't accrete at the old paths and silently miss core's published entries. The shim headers now state that rule, and the type-only JSON value re-export in types.ts is pinned as erasable. No-Verification-Needed: tests, manifest pins, release config, and comments only — no runtime surface change
No-Verification-Needed: release-metadata-only change
🦋 Changeset detectedLatest commit: f5d53b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 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 |
6 tasks
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
barrelClean and coreBoundary each rebuilt a missing dist from their own beforeAll; vitest runs test files in separate workers, so a cold checkout raced two pnpm builds in the same package and the clean step deleted files under the other worker's reader. Route both through a shared single-flight helper: an atomic mkdir lock with one canonical sentinel set per package, a stale-lock steal for holders that died without cleanup, an async build bounded by a timeout so the worker's event loop and vitest's hook timer stay live, and only EEXIST treated as contention. No-Verification-Needed: test-only change, no runtime surface
Contributor
Author
|
@claude review |
The builds take seconds; the previous bounds were sized by stacking worst cases on worst cases. Keep the ordering invariant (build kill < waiter deadline < stale-lock steal < hook timeout) at realistic magnitudes: 60s/90s/120s with 180s and 240s hooks. No-Verification-Needed: test-only timeout constants
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
Author
|
Reopening as a fresh PR to retrigger the automated review. |
… blind spots - Add typesVersions for the ./internal subpath so moduleResolution:node10 consumers resolve its declarations (exports maps are invisible there) - Update wire-schemas and packages docs: core now arrives transitively as the shared runtime schema graph of client/server/server-legacy - Boundary test: scan dist recursively (validators/ chunks were excluded) and reject bare side-effect imports of core, which have no from-clause
Contributor
Author
|
@claude review |
mattzcarey
previously approved these changes
Jul 13, 2026
KKonstantinov
previously approved these changes
Jul 13, 2026
6 tasks
The MCP spec schemas (types/schemas.ts), the OAuth/OpenID schemas (shared/auth.ts), the protocol constants (types/constants.ts), and the three JSON value types move verbatim from core-internal into packages/core/src — core now owns the schema sources instead of bundling them out of core-internal through build-only subpath aliases. Mechanism: - core's curated root entry re-exports the same public surface as before (unchanged 172 exports), now from its own local modules. - A new ./internal subpath re-exports the moved modules wholesale for the sibling SDK packages (internal helper schemas, auth types, constants, JSON types — names that are deliberately not public on the root entry). - core-internal keeps the old module paths as one-to-one named re-export shims, so no importer or test changes anywhere. - client/server/server-legacy declare @modelcontextprotocol/core as a real dependency; their bundles keep @modelcontextprotocol/core/internal as an external runtime import (explicit tsdown external entry) instead of carrying their own bundled schema copies. - The build-only core-internal/schemas + core-internal/auth tsconfig/tsdown aliases are deleted everywhere; the per-package tsconfigs gain a single source-first alias for @modelcontextprotocol/core/internal so typecheck and vitest stay build-order independent. Layering stays acyclic: core depends only on zod; core-internal depends on core; the wire era modules are untouched (their frozen copies and their runtime constants imports resolve through the shims unchanged).
…home Test-only adaptations to the schema-source move; each guard keeps pinning the same invariant, only pointed at the new canonical location: - packageTopologyPins: pin @modelcontextprotocol/core's export map as ['.', './internal'] — the internal seam is deliberate, not public API. - coreSchemas: read the spec-schema source from core's own src/schemas.ts (the auth group still reads core-internal's authSchemas registry). - wireOnlyHiding: read the @deprecated task-schema and constants sources from packages/core/src (the old paths are now re-export shims with no doc comments to scan). - codemod authSchemaNames: core's barrel now ends its auth block with "} from './auth'" instead of the deleted build-only alias specifier. - cloudflareWorkers: generalize packServerPackage to packWorkspacePackage and install the workspace core tarball alongside the server tarball — the packed server resolves @modelcontextprotocol/core/internal at runtime, which the registry copy of core does not carry yet.
The schema sources moved into @modelcontextprotocol/core with the sibling packages resolving @modelcontextprotocol/core/internal at runtime. That seam can rot in ways no existing test catches; this locks it down: - Exact sibling pins: client/server/server-legacy (and core-internal, for consistency) depend on core via workspace:* so pnpm publishes an exact version pin instead of a caret range. The ./internal surface is only guaranteed for the core version each sibling was built against, so a caret range would let installs mix skewed versions. - Changesets fixed group for core + client + server + server-legacy, keeping the pinned versions releasable in lockstep. - ./internal is labeled as an SDK-internal contract in its header, with @internal JSDoc on the re-exports (source-level only: the dts bundler flattens the re-exports and drops statement-level comments, so the built declarations do not carry the tag). - A client boundary test parses the built dists in both directions: every name the client bundle imports or re-exports from core must resolve against core's built entry export lists (skew), and sentinel schemas that exist only in core's modules must never appear as definitions in the client bundle (re-inlining via lost external config or eager aliases). - A core-internal shim-purity test pins the old schema/auth/constants module paths as pure re-export forwards: no zod import, no local definitions, imports only from @modelcontextprotocol/core/internal — so new schemas can't accrete at the old paths and silently miss core's published entries. The shim headers now state that rule, and the type-only JSON value re-export in types.ts is pinned as erasable. No-Verification-Needed: tests, manifest pins, release config, and comments only — no runtime surface change
No-Verification-Needed: release-metadata-only change
barrelClean and coreBoundary each rebuilt a missing dist from their own beforeAll; vitest runs test files in separate workers, so a cold checkout raced two pnpm builds in the same package and the clean step deleted files under the other worker's reader. Route both through a shared single-flight helper: an atomic mkdir lock with one canonical sentinel set per package, a stale-lock steal for holders that died without cleanup, an async build bounded by a timeout so the worker's event loop and vitest's hook timer stay live, and only EEXIST treated as contention. No-Verification-Needed: test-only change, no runtime surface
The builds take seconds; the previous bounds were sized by stacking worst cases on worst cases. Keep the ordering invariant (build kill < waiter deadline < stale-lock steal < hook timeout) at realistic magnitudes: 60s/90s/120s with 180s and 240s hooks. No-Verification-Needed: test-only timeout constants
… blind spots - Add typesVersions for the ./internal subpath so moduleResolution:node10 consumers resolve its declarations (exports maps are invisible there) - Update wire-schemas and packages docs: core now arrives transitively as the shared runtime schema graph of client/server/server-legacy - Boundary test: scan dist recursively (validators/ chunks were excluded) and reject bare side-effect imports of core, which have no from-clause
…ureBuilt, fix stale doc claim - The no-restricted-imports group and the layering-invariant regex only matched the old types/schemas path; a wire/rev module importing the same mutable schemas via @modelcontextprotocol/core would have passed both. - ensureBuilt's sentinels now include late-written artifacts (dts pass, CJS validator) so an interrupted build cannot satisfy the fast path, and the stale-lock steal is atomic via rename. - upgrade-to-v2.md still claimed core is not required by client/server.
# Conflicts: # packages/client/test/helpers/ensureBuilt.ts
felixweinberger
dismissed stale reviews from KKonstantinov and mattzcarey
via
f5d53b8
Comment on lines
+36
to
+38
| // Read the schema source modules directly so the groups cannot silently drift: the spec | ||
| // group against core's own src/schemas.ts (the canonical home since the move), the auth | ||
| // group against core-internal's `authSchemas` registry (specTypeSchema.ts stays there). |
There was a problem hiding this comment.
🟡 The drift-guard comment at line 29 still says the spec *Schema constants are read "from core-internal/src/types/schemas.ts", but this PR changed the code below (line 46) to read core's own ../src/schemas.ts — and the PR-added paragraph at lines 36-38 says exactly that, so the same comment block now names two contradictory sources of truth. A one-line reword of line 29 (e.g. "spec *Schema constants from src/schemas.ts (this package)") fixes it.
Extended reasoning...
What the issue is. The explanatory comment in packages/core/test/coreSchemas.test.ts describes the two schema groups the drift guard pins. Item 1 (line 29) still reads:
1. spec *Schema constants from core-internal/src/types/schemas.ts (minus internal helpers with no public spec type ...)
But this PR moved the spec schema source into core itself, and the code directly below was updated accordingly: line 46 now calls exportedSchemaConsts(readCore('../src/schemas.ts'), ...) — core's own src/schemas.ts, not core-internal's. The PR even appended a corrective paragraph at lines 36-38 in the same comment block: "Read the schema source modules directly so the groups cannot silently drift: the spec group against core's own src/schemas.ts (the canonical home since the move) ...". So the block now tells a future maintainer two different sources of truth for the spec group.
Why the stale sentence can never be true again. core-internal/src/types/schemas.ts is now a forwarding-only shim, and the PR's own new schemaShims.test.ts explicitly forbids it from ever containing export const definitions (the "declares a local binding" assertion). The regex this test applies, /^export const (\\w+Schema)\\b/gm, would match nothing at the old path — so line 29 describes a source that is structurally impossible going forward.
Step-by-step proof. (1) Read line 29: it names core-internal/src/types/schemas.ts as the source of the spec group. (2) Read line 46: readCore('../src/schemas.ts') resolves to packages/core/src/schemas.ts. (3) Read lines 36-38 (added in this diff): they say the spec group is checked "against core's own src/schemas.ts (the canonical home since the move)" — a direct intra-comment contradiction with (1). (4) Open packages/core-internal/src/types/schemas.ts at HEAD: it is a pure re-export list with a header saying "Add new schemas in packages/core/src/schemas.ts, never here"; running line 46's regex against it yields zero export const matches, so the line-29 description is not merely stale but impossible.
Why nothing catches it. The stale sentence is prose; the test logic itself is correct and passes, and no guard pins comment accuracy. Note the auth half of the comment (item 2, lines 31-35) remains accurate — the code still reads core-internal's authSchemas registry in specTypeSchema.ts — so only the one sentence is wrong.
Impact and fix. Zero runtime impact; the risk is a future maintainer following line 29 to the wrong file when adding a schema. One-line reword of line 29, e.g.: 1. spec *Schema constants from src/schemas.ts (this package — the canonical home since the move) (minus internal helpers ...). This is the same partial-prose-update class already fixed three times in this PR (packages.md, wire-schemas.md, upgrade-to-v2.md); this instance just lives in a test comment instead of docs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Moves the neutral schema modules — the spec schemas, the OAuth/OpenID schemas, and the protocol constants — out of core-internal and into
@modelcontextprotocol/coreas real source.client,server, andserver-legacynow resolve them from core as a regular (exact-pinned) runtime dependency instead of each bundling a private copy.Motivation and Context
#2459 deduplicated the schema graph by rewriting module ids to the core specifier at build time. That produced the right artifacts, but the dependency direction lived only inside three build configs — source still said the schemas belonged to core-internal, and a rename or split could silently re-inline ~2,400 lines of Zod graph into every consumer with all gates green. Moving the source makes the published edge real: core owns the schemas, core-internal forwards from the old paths (one-line-per-name shims; a test keeps them forwarding-only), and the bundlers externalize a declared dependency with no plugin at all. Supersedes #2459.
An application importing more than one of the packages evaluates one schema graph instead of two or three, with shared object identity across packages.
How Has This Been Tested?
Breaking Changes
None. Public APIs, types, and behavior are unchanged. Additive: core gains a
./internalsubpath (documented as an SDK-internal contract that may change in any release); the three packages gain core as an exact-pinned dependency, resolved automatically by npm. core/client/server/server-legacy now version together via a changesets fixed group, so a mismatched pair can never be published or installed.Types of changes
Checklist
Additional context
ERR_PACKAGE_PATH_NOT_EXPORTEDrather than drifting silently../internalonce the barrel export-star cleanup lands.