Improve v1→v2 migration ergonomics: Zod-compatible specTypeSchemas + codemod fixes#2277
Open
felixweinberger wants to merge 7 commits into
Open
Improve v1→v2 migration ergonomics: Zod-compatible specTypeSchemas + codemod fixes#2277felixweinberger wants to merge 7 commits into
felixweinberger wants to merge 7 commits into
Conversation
The spec schemas in specTypeSchemas are typed as Standard Schema, which only exposes the ['~standard'].validate() interface. The underlying runtime values are still Zod schemas, so .parse()/.safeParse() exist but are hidden by the type. Code written against the previous API validated with SomeSchema.parse(value) and SomeSchema.safeParse(value). With the schemas now reached through the specTypeSchemas map and typed as Standard Schema, those calls no longer compile, forcing every validation site to be rewritten to ['~standard'].validate() with manual remapping of .success/.data/.error — and .parse() (which throws) has no one-line Standard Schema equivalent at all, so those sites must hand-roll a validate-then-throw block, changing the thrown error type from a structured ZodError to a generic Error. Surface parse() and safeParse() on each specTypeSchemas entry (type-only; the runtime values already carry these methods) so existing validation code migrates by a reference rename — SomeSchema.parse(x) becomes specTypeSchemas.Some.parse(x) — with identical runtime behavior, including the ZodError thrown on invalid input. Only these two methods are exposed; the rest of the Zod schema surface stays internal. New code should still prefer the library-agnostic Standard Schema interface or isSpecType. Adds tests covering parse() success/throw and safeParse() result shape.
…riting The spec-schema-access transform special-cased .parse() and .safeParse(): .parse() was left in place with an action-required diagnostic (the call did not compile against v2 and the import was dead), and .safeParse() was either collapsed to isSpecType.X() or rewritten to ['~standard'].validate() with the result's .success/.data/.error accesses remapped across the enclosing block. The remapping is fragile and changes behavior — .error became .issues, losing the structured Zod error, and .parse() sites had no automatic migration at all. Now that specTypeSchemas entries expose Zod-compatible .parse()/.safeParse(), collapse all of this into the same rename the transform already used for other methods: XSchema.<m>(...) becomes specTypeSchemas.X.<m>(...), leaving the call and any result-property access untouched. For .parse()/.safeParse() this is a behavior-preserving rename (emit an info diagnostic); for Zod methods that are not exposed on the entry (.extend, .parseAsync, …) the renamed call will not typecheck, so flag it inline with an action-required comment. Removes the captured-safeParse remapping machinery and its helpers. Updates the transform tests to the rename contract and retargets the comment-insertion mechanism tests onto .parseAsync() (still action-required).
…jects The shared protocol types (types.js, shared/*) live in both the client and server packages, so the codemod has to decide which package to import them from. It read that decision from package.json — but a project being migrated from v1 still has the single `@modelcontextprotocol/sdk` dependency, not the split `client`/`server` packages, so the project type came back "unknown" for the whole run. Every file that imported only shared types then defaulted to the server package with an action-required warning telling the user to pick by hand. Infer the project type from the source instead: when the split deps are absent, scan for `@modelcontextprotocol/sdk/client/` and `.../server/` imports. Using both → "both", one → that side, neither → "unknown". A project that uses both client and server APIs is now correctly "both". For a "both" project, importing shared types from either package compiles (both re-export them from core), so resolve to server and emit an info note rather than an action-required warning. "unknown" still warns. For a client-only or server-only project the inferred type now routes shared types to the installed package instead of defaulting to a server package that was never added. The scan is bounded (skips node_modules/dist/etc, file budget, early-exit once both signals are seen).
The handler-registration transform rewrites setRequestHandler(XSchema, …) and setNotificationHandler(XSchema, …) to the v2 spec form by looking the schema up in a schema→method table. The task schemas added to the spec were missing from that table, so a handler like setNotificationHandler(TaskStatusNotificationSchema, …) fell through to the generic "use the 3-arg form" diagnostic and was left for manual migration. Add the task entries: tasks/get, tasks/result, tasks/list, tasks/cancel, and the notifications/tasks/status notification. These are spec methods, so the rewritten two-argument call (method string + handler) resolves to the spec overload of setRequestHandler/setNotificationHandler and typechecks.
…hemas specTypeSchemas entries now expose Zod-compatible parse()/safeParse(), so code that validated with a *Schema constant migrates by renaming the reference rather than rewriting to the Standard Schema validate() call and remapping its result. Show that path alongside the existing isSpecType and ['~standard'].validate() options, and note that new code should still prefer the library-agnostic forms.
The import-path and mock-path transforms looked up specifiers with an exact match against IMPORT_MAP, which is keyed on the canonical .js-suffixed file form (e.g. .../types.js, .../server/index.js). Consumers using bundler/node16 module resolution import the same modules without the extension (.../types) or in directory form (.../server, which resolves to server/index.js). Those fell through to 'Unknown SDK import path: ... Manual migration required' even though the .js twin was mapped — and a single file could migrate .../server/index.js fine while failing on .../inMemory. Add resolveImportMapping(): try the literal key, then the .js-file form, then the /index.js directory form. Wire it into both transforms. On a codebase that mixes extension styles this clears the great majority of the otherwise-manual import diagnostics (measured: 18 -> 0 on one such consumer). Adds tests for extensionless (/types, /shared/transport, /inMemory) and directory-style (/server, /client) specifiers, plus a guard that genuinely unknown subpaths still report.
🦋 Changeset detectedLatest commit: e9965a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 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/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
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
1 participant
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.
Improves the ergonomics and fidelity of the v1→v2 migration, validated by running the codemod end-to-end against two large production MCP codebases (one client+server host, one client). One small, type-only SDK compatibility shim plus several codemod fixes; the rest of the v2 API is left as designed.
Motivation and Context
Migrating two large real consumers surfaced a handful of rough edges where the migration was either non-mechanical, silently wrong, or noisier than necessary:
specTypeSchemasvalidation migration was a rewrite, not a rename. v1 code validated withSomeSchema.parse(value)/.safeParse(value). Because the schemas are now reached viaspecTypeSchemas.Xand typed as Standard Schema,.parse()had no one-line equivalent (it throws;validate()doesn't), so those sites had no automatic migration, and.safeParse()sites were rewritten to['~standard'].validate()with.success/.data/.errorremapped — which also changes the thrown error type. The runtime values are still Zod schemas, so this PR surfacesparse()/safeParse()on each entry (type-only) and changes the codemod to migrate by a reference rename with identical behavior. Standard Schema (['~standard'].validate()) remains the recommended API.client/serverdeps, which a v1 project doesn't have yet, so every file importing only shared protocol types defaulted to the server package with an action-required warning. It now infers the type from the source'ssdk/client/vssdk/server/imports; a project using both resolves shared types to the server package with an informational note.setRequestHandler/setNotificationHandlerfortasks/*fell through to manual-migration diagnostics..jsform, so extensionless (.../types) and directory-style (.../server) specifiers reported "Unknown SDK import path" even though the.jstwin was mapped. One consumer used mixed extension styles and hit this on the majority of its SDK imports.How Has This Been Tested?
@modelcontextprotocol/core: typecheck clean, 477 tests pass (incl. newparse/safeParsecoverage).client+servertypecheck clean.@modelcontextprotocol/codemod: typecheck clean, 357 tests pass (transforms + integration).as any/@ts-ignore/.skip); the import-normalization fix alone cleared the majority of that codebase's otherwise-manual import diagnostics.Breaking Changes
None. The SDK change is additive (two methods surfaced on values that already had them at runtime); the rest are codemod and docs improvements.
Types of changes
Checklist
Additional context
Relationship to recently-merged codemod work (#2274 / #2137): this PR changes the spec-schema validation migration approach introduced there. #2274 rewrites
.parse()/.safeParse()tospecTypeSchemas.X['~standard'].validate()and remaps the result; this PR instead exposes Zod-compatibleparse()/safeParse()on the entries and does a behavior-preserving rename, which avoids the unmigrated-.parse()gap and the.success/.data/.errorremap (and the implicitZodError→generic-error change). TheStreamableHTTPError→SdkHttpErrorcodemod fix from #2274/#2137 is intentionally not duplicated here. Happy to split the SDK shim out from the codemod fixes if the maintainers prefer to review them separately — the codemod schema-rename change depends on the shim, but the project-type, task-method, and import-normalization fixes are independent.