Skip to content

perf: build era wire schemas lazily through memoized factories#2476

Open
felixweinberger wants to merge 2 commits into
mainfrom
fweinberger/lazy-era-schemas
Open

perf: build era wire schemas lazily through memoized factories#2476
felixweinberger wants to merge 2 commits into
mainfrom
fweinberger/lazy-era-schemas

Conversation

@felixweinberger

Copy link
Copy Markdown
Contributor

Importing the client or server package currently constructs both frozen protocol-revision wire-schema graphs (2025-11-25 and 2026-07-28) at module evaluation, before any message is validated. This PR moves each revision's schema definitions into a buildSchemas() factory memoized at module level, so the construction cost is paid on first validation instead of at import.

Motivation and Context

A consumer that imports @modelcontextprotocol/client or @modelcontextprotocol/server pays for building ~150 zod schemas per protocol revision, twice over (both revisions), per bundled copy — all before it has connected to anything. For short-lived CLI-style consumers this is pure startup overhead on every invocation.

The fix:

  • Each revision's schema definitions move verbatim into a buildSchemas() factory memoized at module level (the CallToolResultWireSchema wire seam moves verbatim from the 2025 registry into the same factory).
  • The revision registries keep method membership and the exported method lists as static null-valued key objects mapped over the same wire unions — the both-direction drift guards remain compile errors, and importing a registry constructs nothing.
  • Codecs and the input-required maps pull schemas through the memo at first use.
  • Each revision's schemas.ts becomes an eager named-export shim over the memo for tests and tooling; the shim is not on any runtime import path and tree-shakes out of the published bundles.

This eliminates the per-revision schema-construction cost at import (~2 revisions' worth per bundled copy of core-internal). Together with #2458 and #2459, this closes ~55% of a measured consumer import regression and ~96% of its end-to-end startup regression.

How Has This Been Tested?

  • Full package suites pass unchanged: core-internal (1355), server (442), client (721), core; typecheck:all and lint:all clean.
  • The claim that the schemas are unchanged is machine-checked: every zod expression in the two factories was verified token-identical (comments/whitespace-insensitive token-stream comparison) to the corresponding module-level const it replaces — 297 declarations across both revisions, zero differences.
  • Reference identity is covered by the registry pin tests: registry lookups, codecs, and the shim all serve objects from one memo, so the by-reference pins hold under any import order (the pin test now warms the memo via the shim import).
  • Exercised the built packages end-to-end in a scratch consumer: a real client against a real Streamable HTTP server on both protocol revisions — initialize, tools/list, tools/call, malformed-request validation errors, and unknown-method rejection all behave identically; the construction cost observably moves from import to the first validation.

Breaking Changes

None. No public API changes; the schema definitions are token-identical to before; registry lookups keep returning reference-identical schema objects; the exported method lists are unchanged.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

  • The factories are synchronous and memoized (memo ??= build()), so there is no async seam in any validation path and the first-use cost is paid exactly once per process.
  • Membership stays static: the registries' key objects are typed as mapped types over the wire unions, so adding or removing a method without updating the registry is a compile error in both directions.
  • The wire files are behavior-frozen; this change moves their contents verbatim (hence the token-stream check above) rather than editing them.
  • Possible follow-up: extend the existing wire-layer import lint/invariant test to also ban runtime imports of the eager schemas.ts shims from within src/wire, making the laziness invariant mechanical there too.

Changeset: patch for client, server, and server-legacy (their bundles inline core-internal).

Importing the client or server package used to construct both frozen
era wire-schema graphs (2025-11-25 and 2026-07-28) at module
evaluation, before any message was validated. Move each era's schema
definitions into a buildSchemas() factory memoized at module level:

- schema definitions are verbatim from the old era schemas.ts (and the
  CallToolResultWireSchema wire seam verbatim from the 2025 registry,
  which now serves it through the same memo)
- registries keep method membership and the exported method lists as
  static null-valued key objects mapped over the same wire unions, so
  the both-direction drift guards remain compile errors and importing
  a registry constructs nothing
- codecs and inputRequired pull schemas through the memo at first use
- each era schemas.ts becomes an eager named-export shim over the memo
  for tests and tooling; the shim is not on any runtime import path
  and tree-shakes out of the published bundles

Reference identity is preserved: registry lookups, codecs, and the
shim all serve objects from one memo, so the by-reference registry
pins hold (the pin test now warms the memo via the shim import).

Importing client+server gets roughly a third cheaper in a scratch
consumer; all package suites, the examples e2e matrix, typecheck, and
lint pass unchanged.
@felixweinberger felixweinberger requested a review from a team as a code owner July 10, 2026 14:23
@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 74f610c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@modelcontextprotocol/client Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/server-legacy Patch

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

@pkg-pr-new

pkg-pr-new Bot commented Jul 10, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2476

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2476

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2476

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2476

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2476

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2476

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2476

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2476

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2476

commit: 74f610c

Comment thread packages/core-internal/src/wire/rev2025-11-25/schemas.ts
The shim restated the SEP-1686 task deprecations but dropped the
SEP-2577 tags on the logging, sampling, and roots families; restore
each block verbatim from the pre-factoring module so importers keep
IDE strike-through and deprecation lint.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — commit 74f610c fully addresses my earlier comment about the dropped SEP-2577 @deprecated tags, and I found no new issues on this revision. Deferring to a human reviewer only because of the sheer size of the move (both era schema graphs relocated into memoized factories), not because of anything specific I'd flag.

Extended reasoning...

Overview

This PR converts both frozen protocol-revision wire-schema graphs (2025-11-25 and 2026-07-28) from eager module-level construction to memoized buildSchemas() factories in packages/core-internal/src/wire/. The registries keep static null-valued method-key objects (preserving the mapped-type drift guards and the exported method lists), the codecs and input-required maps pull schemas through the memo at first use, and each era's schemas.ts becomes an eager named-export shim over the same memo for tests/tooling. The registry pin test now warms the memo via the shim import and pins CallToolResultWireSchema at its new home.

Prior review status

My earlier review flagged one issue: the rewritten 2025 shim dropped ~25 @deprecated ... (SEP-2577) JSDoc tags (logging, sampling, and roots families) while deliberately restating the SEP-1686 task-family tags. The follow-up commit 74f610c restores those blocks verbatim — I verified the tags are present in the current tree. The multi-agent bug-hunting pass on this revision found no bugs.

Security risks

None identified. The change is a construction-timing refactor: no schema semantics change (the author machine-checked token-identity of all 297 declarations), no new inputs are parsed differently, and the memoized factories are synchronous, so there is no async seam or TOCTOU window in any validation path. Reference identity of the schema objects across registry, codec, and shim is preserved through the single memo and pinned by tests.

Level of scrutiny

This touches the wire-parse contract for every MCP message in both protocol revisions — production-critical code — and the diff is very large (~4,400 lines moved). The mechanical-move claim is well-evidenced (token-stream comparison, full suites passing, end-to-end scratch-consumer verification, by-reference pin tests), and the lazy-construction pattern is sound (memo ??= build() with static membership keys keeping the compile-time drift guards). Still, a verbatim-move refactor of this scale in behavior-frozen files warrants a human maintainer's sign-off — e.g. on the shim-vs-factory layering and the suggested follow-up lint banning runtime imports of the eager shims.

Other factors

Existing coverage is strong: 2,500+ tests across core-internal/server/client pass unchanged, and the registry pin tests specifically guard the reference-identity invariant this refactor depends on. The changeset correctly targets client/server/server-legacy (whose bundles inline core-internal). Duplicate-construction under the two ordering paths (registry-first vs shim-first) resolves to the same memo object, so no double-build hazard exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant