v2: codemod iterations, canonical zod schema exports from sdk-shared#2354
Conversation
… into feature/codemod-iterations-3
IMPORT_MAP was looked up by exact key, so extensionless specifiers like @modelcontextprotocol/sdk/types (vs .../types.js) fell through to an 'Unknown SDK import path' diagnostic and were left unmigrated. Add a shared lookupImportMapping() that tolerates .js/.mjs/.cjs extension variance, and use it for import, re-export, and mock-path resolution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016f6h88mdVxLUdx1cNT96pW
Shared protocol types/constants resolve to either @modelcontextprotocol/client or /server. The codemod read that choice from package.json, but a project mid-migration still declares the single v1 @modelcontextprotocol/sdk dependency, so the project type came back 'unknown' and every file importing only shared symbols defaulted to server with an action-required warning. Infer from source instead: when the v2 split deps are absent, scan for quoted @modelcontextprotocol/sdk/client/ and .../server/ import specifiers (both -> 'both', one -> that side, neither -> 'unknown'). Matching quoted specifiers rather than bare substrings ignores comments/prose and catches extensionless/bare subpaths. For a 'both' project, shared types resolve to server with an info note (both re-export them) instead of an action-required warning; 'unknown' still warns. The scan is bounded (skips heavy dirs, file budget, early-exit). On firebase this cuts the codemod diagnostics from 14 to 2 with 0 introduced typecheck errors.
The handler-registration transform rewrites setRequestHandler(XSchema, …) and setNotificationHandler(XSchema, …) to the v2 spec form via a schema→method table. The task schemas were missing, so a handler like setNotificationHandler(TaskStatusNotificationSchema, …) fell through to the generic "use the 3-argument form" diagnostic and was left for manual migration. Add the task entries: tasks/get, tasks/result, tasks/list, tasks/cancel, and notifications/tasks/status. These are spec methods (the request schemas are members of ServerRequestSchema and the notification is in the notification union), so the rewritten two-argument call resolves to the spec overload of setRequestHandler/setNotificationHandler and typechecks. Co-Authored-By: Felix Weinberger <fweinberger@anthropic.com>
🦋 Changeset detectedLatest commit: 68b2120 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 |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
… arg, preserve import comments - importPaths: aliased named imports now route through the per-symbol splitter (addOrMergeImport carries aliases), so a mixed type+schema import no longer collapses into one v2 package and mis-routes schema constants - schemaParamRemoval: drop a literal `undefined` result-schema argument from request()/callTool() when an options arg follows - importPaths: preserve the first SDK import's leading header/JSDoc comment across the rewrite - batch-test: install pnpm clones standalone (--ignore-workspace --no-frozen-lockfile, CI=true) so repos nested in this workspace actually install
|
Handling some of the edge cases raised by claude[bot], then renaming |
KKonstantinov
dismissed stale reviews from felixweinberger and mattzcarey
via
065e142
… into feature/codemod-iterations-3 # Conflicts: # packages/client/test/client/streamableHttp.test.ts
|
@claude review |
|
@claude review |
1 similar comment
|
@claude review |
…iddleware
eslint
Follow-ups to the core→core-internal / sdk-shared→core rename:
- CLAUDE.md exports section was missing the public @modelcontextprotocol/core package
- middleware express/fastify/hono internal-regex still listed a vestigial 'core' the sweep
could not reach (non-contiguous substring inside the alternation)
|
@claude review |
…red rename
changeset
- core index.ts/tsdown.config.ts/test comments pointed at core/src/... paths that
now live under core-internal/src/...; renamed sdkSharedSchemas→coreSchemas refs
- codemod drift-guard tests carried stale sdkShared* variable names
- @modelcontextprotocol/sdk-shared was never published to npm, so the rename
changeset's 'migrate your imports' entry would mislead the first core changelog;
add-core-public-package.md already provides a single accurate 'new package' entry
…ale validator
comment
- mockPaths: import('…/types.js').then(({ CallToolResultSchema }) => …) was silently
rewritten to the context package (server/client, no Zod schema exports) with no diagnostic.
Generalize destructure handling (getModuleBindingPattern) so a .then(({…}) => …) param is
routed/renamed per-symbol like 'const { … } = await import()' — schema-only → core, mixed
flagged. +3 tests.
- core-internal index.ts: validator-import comment listed @modelcontextprotocol/core/validators/*,
which doesn't exist after the rename (core exports only '.'); point at {client,server}.
There was a problem hiding this comment.
No new issues found on the latest revision — the fixes for the earlier findings (auth-schema routing, JSONRPCResponseSchema rename, lazy context resolution, the fetch-spec-types output path, and the rename leftovers) all look incorporated. That said, this PR introduces a new published package and renames the internal core package, so a maintainer should sign off on the public API surface and the rename before merge.
Extended reasoning...
Overview
This PR (276 files) introduces @modelcontextprotocol/core as the new public home for the spec + OAuth/OpenID Zod schemas, renames the private internal barrel to @modelcontextprotocol/core-internal, and makes a series of codemod accuracy/correctness changes (per-symbol schema routing, project-type inference from source, task-handler method mapping, formatter detection, package.json reconciliation against surviving imports). The latest commits address the findings raised in earlier review rounds — including the previously-missing packages/core-internal directory, the OAuth schema routing gap, the JSONRPCResponseSchema semantic-widening rename, the unguarded undefined-arg removal, the eager project-type diagnostics, the fetch-spec-types.ts output path, and the destructured .then() dynamic-import routing.
Security risks
Low. No auth/crypto logic changes — the OAuth Zod schemas are only re-exported from a new package, not modified. The codemod changes only affect migration tooling that rewrites user source locally; there is no network or credential handling introduced.
Level of scrutiny
High, and human-level. The PR creates a brand-new published npm package with a deliberately curated export surface, takes over the clean core package name, and bumps nine packages via changesets. Per the repo's own conventions (minimalism, burden of proof on addition, every export intentional), the decision to publish the raw Zod schemas as a standalone package — and the bundling-vs-external-dependency trade-off described in the PR — is a design call that maintainers should explicitly endorse, not something to shadow-approve.
Other factors
The bug-hunting system found no new issues on this revision, codemod/core test coverage was substantially expanded (drift guards, batch test against firebase-tools), and the author has been responsive to every prior review round. The remaining open questions are design/ownership questions rather than correctness defects.
3 participants
Introduce
@modelcontextprotocol/coreas the public home for the MCP specification and OAuth/OpenID Zod schemas, and teach the v1→v2 codemod to migrate schema usage to it — plus several codemod accuracy and correctness fixes surfaced by running the migration against real-world v1 repos and by code review. This PR also renames the private internal barrel@modelcontextprotocol/core→core-internal, freeing the cleancorename for the public schema package (published in alpha as@modelcontextprotocol/sdk-shared).Motivation and Context
In v2,
@modelcontextprotocol/serverand@modelcontextprotocol/clientdeliberately expose a Zod-free public surface. That left the raw spec Zod schemas (CallToolResultSchema,ListToolsResultSchema, …) — which v1 users imported from@modelcontextprotocol/sdk/types.js— with no public home. Code that did runtime validation likeCallToolResultSchema.safeParse(value)had nowhere to import the schema from after migrating, and the codemod's previous workaround rewrote those calls into the Standard-Schema form (specTypeSchemas.X['~standard'].validate(...)), which changed user code more than necessary. The same gap applied to the OAuth/OpenID schemas (OAuthTokensSchema,OAuthMetadataSchema, …) that v1 exposed from@modelcontextprotocol/sdk/shared/auth.js.This PR closes that gap and tightens the migration:
@modelcontextprotocol/core— the canonical, public home for the spec Zod schemas and the OAuth/OpenID auth schemas (OAuthTokensSchema,OAuthMetadataSchema,IdJagTokenExchangeResponseSchema, …). It bundles the SDK's internal schema definitions and re-exports only the*SchemaZod constants, so<Name>Schema.parse(value)/.safeParse(value)keep working unchanged — the migration becomes a one-line import-path swap. The auth group mirrors core-internal's own spec-vs-auth split and is kept in sync with core-internal'sauthSchemasregistry by a drift-guard test. Spec types, error classes, enums, and guards continue to live onserver/client. The package is runtime-neutral withzodas its only runtime dependency.core→core-internal,sdk-shared→core). The private, unpublished internal barrel formerly named@modelcontextprotocol/corebecomes@modelcontextprotocol/core-internal, freeing the cleancorename for the public schema package above (published in alpha as@modelcontextprotocol/sdk-shared). A repo-wide, behavior-preserving rename of two packages and their directories —client/server/middleware now consume the internal barrel under its new name. Recorded for the changelog by therename-sdk-shared-to-corechangeset.core.*Schemasymbols imported from@modelcontextprotocol/sdk/types.jsare now routed to@modelcontextprotocol/core(a mixedimport { CallToolResult, CallToolResultSchema }is split — the type resolves by context, the schema to core). The v1 OAuth/OpenID*Schemaconstants from@modelcontextprotocol/sdk/shared/auth.jsare routed the same way (the auth types keep resolving toclient/serverby context). The oldspecSchemaAccesstransform and its generated schema-map are removed in favor of this behavior-preserving import swap.firebase/firebase-tools):@modelcontextprotocol/sdk/typesas well as.../types.js).@modelcontextprotocol/sdkdependency, so detection frompackage.jsoncame back "unknown" and every file importing only shared symbols defaulted to the server package with an action-required warning. The codemod now scans quoted@modelcontextprotocol/sdk/client/…and…/server/…import specifiers to infer the type, routing shared symbols to the installed package and replacing the spurious warnings with — at most — an info note for genuinely ambiguous "both" projects.tasks/get,tasks/result,tasks/list,tasks/cancel,notifications/tasks/status) to their v2 method strings so task handlers migrate to the 2-arg form instead of falling through to manual migration.import * as t … t.CallToolResultSchema.parse()), which can't be split automatically, with an actionable diagnostic pointing at core. The diagnostic now reports the v2 schema name (after renames) so the suggested import is copy-pasteable.undefinedsecond argument is only dropped in files that actually import the MCP SDK (avoids touching unrelated 3-arg calls).\n-rejoin of comment ranges), so CRLF files and blank-line-separated header blocks no longer duplicate the header when the first import is rewritten in place.@modelcontextprotocol/core↔client/serverconsistency. Registering the auth schemas as the canonical core group is keyed off core-internal'sauthSchemas. The one v2-only addition,IdJagTokenExchangeResponse, is now a public spec type: its TypeScript type is exported fromclient/server,'IdJagTokenExchangeResponse'joinsSpecTypeName, andisSpecType.IdJagTokenExchangeResponse/specTypeSchemas.IdJagTokenExchangeResponsevalidate it — matching the already-public OAuth/OpenID spec types. This additive (non-breaking) surface is covered by aclient/serverchangeset.core;.parse()unchanged), with the Zod-freeisSpecType/specTypeSchemasas an alternative; OAuth/OpenID schemas are noted as also moving tocore.How Has This Been Tested?
coretests (schema round-trip + drift guards asserting every spec and auth schema is re-exported). Three drift guards keep the auth set aligned:core's auth exports equal core-internal'sauthSchemas, and the codemod'sAUTH_SCHEMA_NAMES(the v1 set) is a subset ofcore's auth exports.build:all,check:all(typecheck + lint + docs) all green. Thecorebundle was verified runtime-neutral —zodexternal, no Node built-ins, noProtocol/transport/validator code pulled in, and the.d.tsexposes the schemas as real values (not type-only aliases).firebase/firebase-tools— baseline typecheck 0 errors, post-codemod typecheck 0 errors (0 introduced). After the accuracy fixes, the codemod's diagnostics on that repo dropped to just the two genuinely-actionable manual-verification notes (SSE-transport deprecation,ErrorCodesplit). Spot-checked the migrated output: schemas import from@modelcontextprotocol/coreand.parse()calls are preserved verbatim.Breaking Changes
None for existing v2 consumers.
@modelcontextprotocol/coreis a new, additive package.serverandclientgain one small additive (non-breaking) public-API surface — the v2-onlyIdJagTokenExchangeResponsespec type (type export +SpecTypeNamemember +isSpecType/specTypeSchemasentry), covered by aclient/serverchangeset; the privatecore-internalpackage registers it. The middleware packages are unchanged. The package rename is invisible to stable consumers: the internal barrel (core→core-internal) was always private and unpublished, and the only public-name change — the alpha@modelcontextprotocol/sdk-shared→@modelcontextprotocol/core— affects only those who installed the alpha schema package, who update with a one-line import swap (sdk-shared→core) that the codemod applies automatically (rename-sdk-shared-to-corechangeset). The schema-import-location changecoresupports is part of the existing v1→v2 migration (not new breakage) and is applied automatically by the codemod and documented in the migration guide.Types of changes
Checklist
Additional context
corebundlescore-internalrather than owning the schemas. An alternative was prototyped wherecoreowned the schemas andserver/clientreferenced it externally to avoid duplicating the schema bytes. It worked but was invasive (it touchedcore-internal/server/client/middleware build configs and required guarding against a rolldown value-export degradation). This PR takes the lower-footprint approach:corebundles the (private)core-internalschema module via a narrowed alias and re-exports only the*Schemavalues, leavingserver/client's build and schema-bundling untouched (their only public-API delta is the additiveIdJagTokenExchangeResponsespec type noted above). The trade-off is thatserver/clientkeep their own internally-bundled schema copies (not part of their public API) — an accepted cost for a much smaller, lower-risk change.