perf: resolve schema modules from @modelcontextprotocol/core instead of inlining them#2459
perf: resolve schema modules from @modelcontextprotocol/core instead of inlining them#2459felixweinberger wants to merge 2 commits into
Conversation
…ining them client, server, and server-legacy bundle @modelcontextprotocol/core-internal wholesale, which inlines the spec + OAuth Zod schema modules that are also published verbatim as @modelcontextprotocol/core. An application using client + server therefore evaluates the same large schema graph three times (and nests three copies of the code). Add an externalize-core-schemas rolldown plugin to the three configs that resolves core-internal/src/types/schemas.ts and core-internal/src/shared/auth.ts to @modelcontextprotocol/core at runtime, and declare core as a real dependency of the three packages. One evaluated schema-graph copy is now shared per application. Every name crossing the boundary at runtime is exported by core: - guards.ts / specTypeSchema.ts / client.ts runtime imports are all in core's export list (the specTypeSchema namespace access is keyed by the SPEC_SCHEMA_KEYS allowlist, which excludes internal helpers); - the remaining internal-only exports (BaseRequestParamsSchema, NotificationsParamsSchema, ClientTasksCapabilitySchema, ServerTasksCapabilitySchema, SafeUrlSchema, OptionalSafeUrlSchema) are only imported type-only or within the externalized modules themselves; - ListChangedOptionsBaseSchema was the one runtime-used SDK-internal helper living in schemas.ts, so it moves to types/listChangedOptions.ts and stays bundled (it is an SDK option schema, not a spec schema, and is deliberately not part of core's public surface). Type declarations still inline the schema types via the dts paths mappings — only runtime duplication costs. The wire-revision codec schemas and other core-internal modules remain bundled (they are private and not published anywhere), so client/server keep their direct zod dependency.
|
@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}
| * matching public type (e.g. `BaseRequestParamsSchema`, | ||
| * `NotificationsParamsSchema`, `ClientTasksCapabilitySchema`, `ServerTasksCapabilitySchema`). |
There was a problem hiding this comment.
🟡 The stale 'ListChangedOptionsBaseSchema' entry remains in the INTERNAL_HELPER_SCHEMAS array in packages/core-internal/test/types/specTypeSchema.test.ts (line 169), even though this PR moved that schema out of types/schemas.ts and removed it from the exclusion comment in specTypeSchema.ts and from the parallel list in packages/core/test/coreSchemas.test.ts. The entry is now dead (it can never match Object.keys(schemas)) and the array's "mirrors the exclusion comment" claim is no longer accurate — removing that one line keeps the three lists consistent.
Extended reasoning...
What the issue is: The PR relocates ListChangedOptionsBaseSchema from packages/core-internal/src/types/schemas.ts into the new packages/core-internal/src/types/listChangedOptions.ts (so it stays bundled while schemas.ts gets externalized to @modelcontextprotocol/core). The two lists that track schemas.ts internal helpers were updated accordingly: the exclusion comment in specTypeSchema.ts (this PR drops the name from the "e.g." list) and SPEC_INTERNAL_HELPERS in packages/core/test/coreSchemas.test.ts (entry removed in the diff). But the third parallel list — INTERNAL_HELPER_SCHEMAS in packages/core-internal/test/types/specTypeSchema.test.ts line 169 — still contains 'ListChangedOptionsBaseSchema'.\n\nCode path: That test does import * as schemas from '../../src/types/schemas' and its drift-guard filters Object.keys(schemas) against INTERNAL_HELPER_SCHEMAS to compute the set of exported schemas that must appear in SPEC_SCHEMA_KEYS. After this PR, schemas.ts no longer exports ListChangedOptionsBaseSchema, so the entry never matches anything — it is dead data.\n\nStep-by-step proof:\n1. Post-PR, grep 'export const ListChangedOptionsBaseSchema' packages/core-internal/src/types/schemas.ts returns nothing (the definition now lives in types/listChangedOptions.ts).\n2. The test's namespace import targets ../../src/types/schemas directly (not the types/index.ts barrel), so Object.keys(schemas) no longer contains 'ListChangedOptionsBaseSchema'.\n3. The filter !INTERNAL_HELPER_SCHEMAS.includes(name) therefore never sees that name; the entry has no effect on the assertion.\n4. The array's own comment (line 166) says it "Mirrors the exclusion comment in specTypeSchema.ts" — but this PR edited that exclusion comment to remove ListChangedOptionsBaseSchema, so the mirror claim is now stale.\n\nWhy nothing prevents it: The test still passes — an extra never-matching name in an exclusion list cannot fail the drift guard — so CI does not flag the leftover.\n\nImpact: Purely cosmetic/maintenance drift: a future reader comparing the three lists (specTypeSchema.ts comment, coreSchemas.test.ts, this test) will find them inconsistent, and the dead entry could mask intent if the schema name were ever reintroduced in schemas.ts.\n\nFix: Delete the 'ListChangedOptionsBaseSchema', line from INTERNAL_HELPER_SCHEMAS in packages/core-internal/test/types/specTypeSchema.test.ts (line 169), matching the update already made to the sibling list in packages/core/test/coreSchemas.test.ts.
| - **`@modelcontextprotocol/core-internal/public`** (`packages/core-internal/src/exports/public/index.ts`) — Curated public API. Exports only TypeScript types, error classes, constants, and guards. Re-exported by client and server packages. | ||
| - **`@modelcontextprotocol/client`** and **`@modelcontextprotocol/server`** (`packages/*/src/index.ts`) — Final public surface. Package-specific exports (named explicitly) plus re-exports from `core-internal/public`. | ||
| - **`@modelcontextprotocol/core`** (`packages/core/src/index.ts`) — Public Zod-schema package. Re-exports **only** the `*Schema` Zod constants (MCP spec + OAuth/OpenID), bundled from `core-internal` at build time. The published home for raw runtime validation (`CallToolResultSchema.parse(...)`); runtime-neutral (`zod` is its only dependency). Not consumed by the sibling packages — `client`/`server` keep their own bundled schema copies and stay Zod-free in their public surface. | ||
| - **`@modelcontextprotocol/core`** (`packages/core/src/index.ts`) — Public Zod-schema package. Re-exports **only** the `*Schema` Zod constants (MCP spec + OAuth/OpenID), bundled from `core-internal` at build time. The published home for raw runtime validation (`CallToolResultSchema.parse(...)`); runtime-neutral (`zod` is its only dependency). Consumed by the sibling packages at runtime: `client`/`server`/`server-legacy` bundles resolve the schema modules from this package (see the `externalize-core-schemas` plugin in their tsdown configs) so one evaluated schema-graph copy is shared per application; their public surfaces stay Zod-free. |
There was a problem hiding this comment.
🟡 The user-facing docs still describe the pre-PR relationship between @modelcontextprotocol/core and the client/server packages: docs/advanced/wire-schemas.md:59 says server and client "never depend on it" and docs/get-started/packages.md:69 says "you never install it", but after this PR core is a runtime dependency of client/server/server-legacy and their bundles import the schema modules from it. These two doc sentences should be updated in this PR alongside the CLAUDE.md change that already reflects the new relationship.
Extended reasoning...
What's stale. This PR turns @modelcontextprotocol/core into a real runtime dependency of client, server, and server-legacy: each package.json adds "@modelcontextprotocol/core": "workspace:^" to dependencies, and the new externalize-core-schemas plugin in the three tsdown configs rewrites imports of core-internal/src/types/schemas.ts and core-internal/src/shared/auth.ts to @modelcontextprotocol/core as an external, so the published bundles import the schema modules from that package at runtime. CLAUDE.md line 73 was updated to describe exactly this ("Consumed by the sibling packages at runtime…"), but the user-facing docs were not.\n\nWhere the docs now contradict the implementation.\n\n- docs/advanced/wire-schemas.md:59: "Install it separately (npm install @modelcontextprotocol/core) — @modelcontextprotocol/server and @modelcontextprotocol/client keep a Zod-free public surface and never depend on it." — the "never depend on it" clause is now flatly false, and "install it separately" is misleading since core now arrives transitively with client/server.\n- docs/get-started/packages.md:69: "…so if you only call registerTool and callTool you never install it." — after this PR core is always installed transitively whenever client or server is installed.\n\nConcrete walk-through. A user runs npm install @modelcontextprotocol/server. npm resolves dependencies and pulls @modelcontextprotocol/core into node_modules (the pnpm-lock.yaml diff in this PR shows the same for the workspace). The server bundle's dist/index.mjs now contains import … from '@modelcontextprotocol/core' where the schema graph used to be inlined, so the package is loaded at runtime, not just present on disk. Meanwhile the docs the user reads say the package is something they'd "install separately" and that server/client "never depend on it" — the opposite of what npm ls @modelcontextprotocol/core will show them.\n\nWhy nothing else prevents this. The parts of those sentences about the public surface remain true — client/server still export no Zod schemas and their public API stays Zod-free — so the pages don't fail any snippet-sync or example CI check; only the prose about the dependency graph is wrong, and nothing automated validates prose against package.json.\n\nImpact and fix. No runtime behavior is affected; this is documentation drift only, but it's drift introduced by this PR (which already fixed the equivalent sentence in CLAUDE.md), so it's cheapest to fix here. Suggested edits: in wire-schemas.md, replace "Install it separately … never depend on it" with wording like "It is installed automatically as a dependency of @modelcontextprotocol/server and @modelcontextprotocol/client (their bundles resolve the schema modules from it), but their public surfaces stay Zod-free — import from @modelcontextprotocol/core directly when you need the raw schemas." In packages.md, change "you never install it" to something like "you never need to import it directly (it's installed transitively with client/server)."
| "@modelcontextprotocol/core": "workspace:^", | ||
| "cross-spawn": "catalog:runtimeClientOnly", |
There was a problem hiding this comment.
🟡 This PR adds @modelcontextprotocol/core as a runtime dependency of @modelcontextprotocol/client, @modelcontextprotocol/server, and @modelcontextprotocol/server-legacy and changes their published dist output (schema modules resolved from core instead of inlined), but ships no .changeset/*.md entry — changeset-bot confirms "No Changeset found". Consider adding a patch changeset for the three affected packages so the perf improvement and the new dependency edge get a version bump and a changelog entry.
Extended reasoning...
What's missing
The PR changes the published runtime output of three packages — @modelcontextprotocol/client, @modelcontextprotocol/server, and @modelcontextprotocol/server-legacy — via the new externalize-core-schemas tsdown plugin (their bundles now import the spec + OAuth schema modules from @modelcontextprotocol/core at runtime instead of inlining a copy), and adds "@modelcontextprotocol/core": "workspace:^" as a runtime dependency in each of their package.json files. None of this reaches consumers until new versions are published, yet the PR contains no .changeset/*.md entry — the changeset-bot comment on this PR confirms "No Changeset found".
Why it matters
The repo's release flow is changesets-driven (.changeset/config.json with changelog-github, prerelease mode via pre.json), and every recent behavioral or packaging PR on main ships one — e.g. probe-window-handler-restore.md, standard-header-ows.md, malformed-resource-uri-invalid-params.md, and even pure packaging changes like cjs-dual-builds.md / cjs-ajv-validator-subpath.md. There is also a follow-up commit on main ("chore: add missing changesets for CommonJS builds") showing missing changesets are treated as something to fix. The changeset config only ignores example packages, so nothing auto-generates an entry for these three packages.
Concrete walk-through
- This PR merges as-is with no changeset.
changesetssees no pending entry namingclient/server/server-legacyfrom this PR, so this change alone triggers no version bump — the ~50ms cold-start improvement and the newcoredependency edge stay unpublished.- Later, an unrelated PR adds a changeset that bumps these packages (or the pending beta release train publishes them). The new versions now ship the externalized-schema bundles and the new runtime dependency on
core, but the generated CHANGELOG for those versions contains no line describing thatclient/server/server-legacynow depend on@modelcontextprotocol/coreat runtime — a fact consumers with strict lockfile/audit policies or bundler externals configuration may care about.
Why nothing breaks at merge time
The package.json files already carry the dependency edge, so any future publish is functionally correct; the repo is in prerelease mode and the change will still ride the next "Version Packages" release. The cost is only a delayed and undocumented release — a process/documentation gap rather than a runtime failure, which is why this is a nit rather than a blocking issue.
How to fix
Add a small patch changeset, e.g. .changeset/externalize-core-schemas.md:
---
'@modelcontextprotocol/client': patch
'@modelcontextprotocol/server': patch
'@modelcontextprotocol/server-legacy': patch
---
perf: resolve the spec + OAuth schema modules from @modelcontextprotocol/core at runtime instead of inlining them, so applications using multiple SDK packages evaluate one shared schema-graph copy; core is now a regular runtime dependency of these packages.(The changeset-bot comment even links a pre-filled template for this.)
The repo's CI test job never runs a standalone build — a package's dist exists only if one of its tests builds it (client's barrelClean.test.ts does this for client). Now that client, server, and server-legacy resolve their schema modules from @modelcontextprotocol/core's dist entry at runtime, a missing core build breaks every consumer of a built package, which is exactly how the examples' suites failed in CI. Mirror the barrelClean pattern: build core on demand in beforeAll, then pin the runtime surface the boundary depends on (representative spec + OAuth schema exports and the overall export count).
1 participant
Resolves the spec + OAuth schema modules from
@modelcontextprotocol/coreat runtime instead of inlining them into the client, server, and server-legacy bundles.Motivation and Context
clientandservereach bundle their own copy of core's schema modules, so an application that imports both evaluates the full spec-schema graph (hundreds of zod schema constructions) two to three times at startup. Measured on a large embedding application, deduplicating this to one evaluated copy recovers ~50ms of cold start. It also means one set of schema instances per process rather than several.Type declarations are unchanged — dts still inlines types via the existing paths mapping; only runtime resolution moves to the real
@modelcontextprotocol/coredependency. The one SDK-internal helper used at runtime by client (ListChangedOptionsBaseSchema) moved out of the schema module so it stays bundled, since core deliberately doesn't export it.How Has This Been Tested?
Full workspace suite green (
test:all,typecheck:all,lint:all,build:all); every name crossing the module boundary at runtime verified exported by core (spec schema keys 160/160, guards, 12 auth schemas); tamper test confirms core is a real runtime dependency; verified installable via packed tarballs with a runtime smoke over InMemoryTransport.Breaking Changes
None for consumers — public surfaces unchanged;
corebecomes a regular dependency of client/server/server-legacy (it was already published and versioned in lockstep).Types of changes
Checklist
Additional context
The externalize plugin is duplicated across the three tsdown configs, matching the existing per-package config style.