From 1c33b1061d06affb1c8e972d7014d4726a7a6f45 Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Tue, 30 Jun 2026 13:31:02 +0000 Subject: [PATCH] feat(codemod): manifest handling overhaul and fixes from migrating sample dependents Manifest handling: discover workspace members (npm/yarn/bun workspaces and pnpm-workspace.yaml) under the target root, write only the nearest package.json, and report the exact dependency edit every other affected manifest needs, with hoisted member usage credited to the declaring manifest, v2 additions computed from post-transform import state, and zod added where the wrapped-raw-shape import injection fires in a package that does not declare it (peer and optional declarations respected). File collection no longer follows symlinks and honors --ignore during descent. Zod ranges are classified by the highest version each alternative resolves to, with symptom-accurate warnings. Transform fixes: single-pass nested registration migration; receiver-gated fallback (legacy and register* names) for files without a direct McpServer import; assertion-, guard-, and constructor-aware ErrorCode split with conjunction-pinned guard rewrites, cast repointing, and unused-import cleanup; proven status reads rewritten; context remap for fallback handlers, annotated and forwarded contexts; schema-expression and remove-handler coverage; same-file schema aliases; dynamic-import destructure renames; zod import injection; proof-gated schema drops; context-routed auth types; usage-anchored markers that survive import rewrites; single-file targets with report-only manifests; run-aware advisories; and an honest no-changes summary. --- .changeset/add-sdk-http-error.md | 4 +- .changeset/auth-iss-server-and-overload.md | 4 +- .changeset/auth-iss-validation.md | 4 +- .changeset/codemod-backlog-batch.md | 5 + .changeset/codemod-bigpatient-batch.md | 5 + .changeset/codemod-completable-protocol.md | 5 + .changeset/codemod-context-and-status.md | 5 + .changeset/codemod-longtail-batch.md | 5 + .changeset/codemod-manifest-handling.md | 5 + .changeset/codemod-schema-drop-receivers.md | 5 + .changeset/config.json | 5 +- .changeset/fix-server-protocol-version.md | 2 +- .changeset/fix-unknown-tool-protocol-error.md | 4 +- .changeset/odd-forks-enjoy.md | 2 +- .changeset/pre.json | 208 +++--- .changeset/resource-not-found-32602.md | 6 +- .changeset/sep-2663-tasks-removal.md | 3 +- .changeset/support-standard-json-schema.md | 21 +- .changeset/twelve-dodos-taste.md | 2 +- packages/codemod/README.md | 16 +- packages/codemod/package.json | 1 + packages/codemod/src/cli.ts | 112 ++- .../migrations/v1-to-v2/mappings/importMap.ts | 40 +- .../v1-to-v2/mappings/schemaRouting.ts | 10 + .../v1-to-v2/transforms/completableNesting.ts | 192 +++++ .../v1-to-v2/transforms/contextTypes.ts | 252 ++++++- .../transforms/handlerRegistration.ts | 103 ++- .../v1-to-v2/transforms/importPaths.ts | 123 +++- .../migrations/v1-to-v2/transforms/index.ts | 7 +- .../v1-to-v2/transforms/mcpServerApi.ts | 326 ++++++--- .../v1-to-v2/transforms/mockPaths.ts | 34 +- .../v1-to-v2/transforms/removedApis.ts | 149 +++- .../v1-to-v2/transforms/schemaParamRemoval.ts | 28 +- .../v1-to-v2/transforms/symbolRenames.ts | 488 ++++++++++++- packages/codemod/src/runner.ts | 162 +++- packages/codemod/src/types.ts | 21 +- packages/codemod/src/utils/astUtils.ts | 12 +- packages/codemod/src/utils/importUtils.ts | 7 + .../codemod/src/utils/packageJsonUpdater.ts | 437 +++++++++-- .../codemod/test/commentInsertion.test.ts | 29 + packages/codemod/test/integration.test.ts | 44 +- .../codemod/test/packageJsonUpdater.test.ts | 143 ++-- .../test/v1-to-v2/packageJsonUpdater.test.ts | 690 ++++++++++++++++++ .../transforms/completableNesting.test.ts | 138 ++++ .../v1-to-v2/transforms/contextTypes.test.ts | 257 ++++++- .../transforms/handlerRegistration.test.ts | 132 ++++ .../v1-to-v2/transforms/importPaths.test.ts | 152 ++++ .../v1-to-v2/transforms/mcpServerApi.test.ts | 312 +++++++- .../v1-to-v2/transforms/mockPaths.test.ts | 32 + .../v1-to-v2/transforms/removedApis.test.ts | 122 ++++ .../transforms/schemaParamRemoval.test.ts | 40 + .../v1-to-v2/transforms/symbolRenames.test.ts | 422 +++++++++++ pnpm-lock.yaml | 3 + 53 files changed, 4888 insertions(+), 448 deletions(-) create mode 100644 .changeset/codemod-backlog-batch.md create mode 100644 .changeset/codemod-bigpatient-batch.md create mode 100644 .changeset/codemod-completable-protocol.md create mode 100644 .changeset/codemod-context-and-status.md create mode 100644 .changeset/codemod-longtail-batch.md create mode 100644 .changeset/codemod-manifest-handling.md create mode 100644 .changeset/codemod-schema-drop-receivers.md create mode 100644 packages/codemod/src/migrations/v1-to-v2/transforms/completableNesting.ts create mode 100644 packages/codemod/test/v1-to-v2/packageJsonUpdater.test.ts create mode 100644 packages/codemod/test/v1-to-v2/transforms/completableNesting.test.ts diff --git a/.changeset/add-sdk-http-error.md b/.changeset/add-sdk-http-error.md index 05abc0d8f1..8f591c1b23 100644 --- a/.changeset/add-sdk-http-error.md +++ b/.changeset/add-sdk-http-error.md @@ -1,6 +1,6 @@ --- -"@modelcontextprotocol/core-internal": minor -"@modelcontextprotocol/client": minor +'@modelcontextprotocol/core-internal': minor +'@modelcontextprotocol/client': minor --- Add `SdkHttpError` subclass with typed `.status` / `.statusText` accessors for HTTP transport failures. `StreamableHTTPClientTransport` now throws `SdkHttpError` (which extends `SdkError`) for non-OK HTTP responses; `SSEClientTransport` throws `SdkHttpError` for 401-after-reauth (circuit breaker). diff --git a/.changeset/auth-iss-server-and-overload.md b/.changeset/auth-iss-server-and-overload.md index af376f4c8f..7e07db3f35 100644 --- a/.changeset/auth-iss-server-and-overload.md +++ b/.changeset/auth-iss-server-and-overload.md @@ -1,6 +1,6 @@ --- -"@modelcontextprotocol/client": minor -"@modelcontextprotocol/server-legacy": minor +'@modelcontextprotocol/client': minor +'@modelcontextprotocol/server-legacy': minor --- SEP-2468 follow-up: `transport.finishAuth()` gains a `URLSearchParams` overload (preferred) that extracts `code`/`iss`, validates `iss` first, and on mismatch throws a sanitized `IssuerMismatchError` (no callback `error_description` text); callers remain responsible for `state`. **Behavior change for `@modelcontextprotocol/server-legacy`:** `mcpAuthRouter` now advertises `authorization_response_iss_parameter_supported` (default `true`; `ProxyOAuthServerProvider` reports `false`) and the bundled authorize handler appends `iss` (RFC 9207) to every `res.redirect(...)` your `OAuthServerProvider.authorize()` issues to the client's `redirect_uri`. If your provider redirects another way (`res.writeHead`, a separate consent-page response, or a standalone `authorizationHandler({provider})` without `issuerUrl`), append `params.issuer` as `iss` yourself or set `authorizationResponseIssParameterSupported: false` — otherwise RFC 9207-compliant clients (including this SDK) will reject the callback. diff --git a/.changeset/auth-iss-validation.md b/.changeset/auth-iss-validation.md index 3f0c4283ef..f6008b6a7a 100644 --- a/.changeset/auth-iss-validation.md +++ b/.changeset/auth-iss-validation.md @@ -1,6 +1,6 @@ --- -"@modelcontextprotocol/core-internal": minor -"@modelcontextprotocol/client": minor +'@modelcontextprotocol/core-internal': minor +'@modelcontextprotocol/client': minor --- Implement RFC 9207 / RFC 8414 §3.3 OAuth issuer validation (SEP-2468). `discoverAuthorizationServerMetadata()` now rejects metadata whose `issuer` does not match the discovery URL (opt out via `skipIssuerValidation` / `AuthOptions.skipIssuerMetadataValidation` — security-weakening). `auth()`, `exchangeAuthorization()`, `fetchToken()`, and `transport.finishAuth(code, iss?)` now validate the authorization-callback `iss` against the recorded issuer before redeeming the code; new `IssuerMismatchError` and `validateAuthorizationResponseIssuer()` are exported. diff --git a/.changeset/codemod-backlog-batch.md b/.changeset/codemod-backlog-batch.md new file mode 100644 index 0000000000..fa0929f6f5 --- /dev/null +++ b/.changeset/codemod-backlog-batch.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +Backlog fixes. When the zod import injection fires in a package that declares no zod, the manifest pass adds it (devDependencies when only tests import it) so strict node_modules layouts install cleanly. The ErrorCode-split pairing re-points stale `as ProtocolError`/`as McpError` casts bound to subjects whose assertions it moves to `SdkError`. Handler registration resolves one same-file variable hop (`const S = ListToolsRequestSchema`) before declaring a schema custom. Shorthand and aliased destructures of SDK dynamic imports rename with the static-import pass. Call-shape assertions pinning a registration schema (`expect.objectContaining({ inputSchema: … })`) get an advisory. The guide covers dist-text pins (no CJS-resolvable subpaths, content-hashed chunks, changed quote style, ESM-only output). diff --git a/.changeset/codemod-bigpatient-batch.md b/.changeset/codemod-bigpatient-batch.md new file mode 100644 index 0000000000..0d9173e417 --- /dev/null +++ b/.changeset/codemod-bigpatient-batch.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +Fixes from migrating two large consumers. Registrations nested inside another handler's body no longer crash the transform with a whole-file rollback (calls process inner-first). Legacy `.tool()`/`.prompt()`/`.resource()` calls migrate without a direct `McpServer` import when their shape matches the v1 signature AND the receiver is named like an MCP server (`server`, `harness.mcp`, `this.mockServer`); other receivers are left alone, without hard markers, since their type is unknown to the codemod. `setRequestHandler`/`setNotificationHandler` with a schema _expression_ first argument get a marker pointing at the typed two-argument or custom three-argument form instead of being skipped silently, and `removeRequestHandler`/`removeNotificationHandler` with `Schema.shape.method.value` arguments rewrite to the method string. Destructured trailing callback parameters only count as the context when their keys look like context members, so template-variable destructures stop collecting false markers. The manifest zod note only appears for manifests that actually take part in the migration. diff --git a/.changeset/codemod-completable-protocol.md b/.changeset/codemod-completable-protocol.md new file mode 100644 index 0000000000..6b140df0ad --- /dev/null +++ b/.changeset/codemod-completable-protocol.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': minor +--- + +Three migration-sweep fixes. `completable()` calls whose first argument is an optional-wrapped schema are rewritten to apply `.optional()` to the `completable(...)` result — v2 resolves completion metadata after unwrapping an outer optional wrapper, so the v1 nesting produced empty completion lists without an error; wrapper shapes the codemod cannot invert get an action-required marker instead. Imports of `Protocol` and `mergeCapabilities` from v1's `shared/protocol.js` are no longer rewritten to a member the v2 packages do not export: the symbols are dropped from the rewritten import and an action-required marker explains the replacement (`fallbackRequestHandler` for unrouted inbound requests; a plain object spread for capability merging). The manifest zod-range warning now describes the symptom by vintage — zod 4.0–4.1 ranges fail type-checking (TS2769), while zod-3 ranges fail type-checking or at the first `tools/list` depending on the imported zod entry point. diff --git a/.changeset/codemod-context-and-status.md b/.changeset/codemod-context-and-status.md new file mode 100644 index 0000000000..8c7722f378 --- /dev/null +++ b/.changeset/codemod-context-and-status.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +Two long-standing intervention classes now migrate mechanically. `.code` reads on values an `instanceof SdkHttpError` check proves — in the same condition or the guarded block — rewrite to `.status` (v2 carries the HTTP status there; `.code` is an `SdkErrorCode` string); unprovable reads keep the existing warning. The context-property remap reaches three shapes the call-site scan missed: functions assigned to `fallbackRequestHandler`, parameters annotated with a context type directly or via a same-file alias (accesses remap in place, the parameter keeps its name), and contexts forwarded wholesale to helpers, which get an advisory naming the callee. diff --git a/.changeset/codemod-longtail-batch.md b/.changeset/codemod-longtail-batch.md new file mode 100644 index 0000000000..f36b0fb5f4 --- /dev/null +++ b/.changeset/codemod-longtail-batch.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': minor +--- + +Migration-sweep batch. The ErrorCode split now coordinates with the surrounding check: an all-SDK condition's `instanceof ProtocolError`/`McpError` guard is rewritten to `SdkError`, a guard covering both enums gets a marker asking for a split, and an `ErrorCode` import with no rewritable member access on a v2 specifier is dropped with a marker instead of failing at module link time. Wrapping a raw shape with `z.object()` adds `import { z } from 'zod'` when the file has no `z` value binding (a non-import `z` binding gets a marker instead). The context-parameter rewrite finds the trailing `extra` parameter, covering the three-argument `registerResource` template callback without flagging its `variables` argument. Resource-server auth helpers routed to the frozen server-legacy copy get a marker on value imports and barrel re-exports (an info note for type-only imports), every rewritten `SdkHttpError` constructor site gets a marker, and single-argument `finishAuth(...)` calls in files the run changes get a run-log note (the one-argument `URLSearchParams` form is valid v2, so the note never re-fires on already-migrated trees). The codemod accepts a single source file as target — source rewrites scope to that file and manifest changes are reported, not applied — and the no-changes summary distinguishes "already on the v2 packages", "still on the v1 SDK under a transform subset", and "no MCP SDK imports found". diff --git a/.changeset/codemod-manifest-handling.md b/.changeset/codemod-manifest-handling.md new file mode 100644 index 0000000000..0466916b4d --- /dev/null +++ b/.changeset/codemod-manifest-handling.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': minor +--- + +Overhaul manifest handling. The codemod now discovers workspace-member manifests (npm/yarn/bun `workspaces` and `pnpm-workspace.yaml`), writes only the nearest `package.json`, and reports the exact dependency changes every other affected manifest needs, so you can apply them deliberately. The v2 additions are computed from the post-transform import state of the files each manifest owns, so already-migrated packages still receive the packages their imports need when the v1 dependency is removed; in hoisted monorepos, member usage counts toward the manifest that declares the SDK dependency, with a note naming the contributing members. File collection no longer follows symbolic links (pnpm `node_modules` layouts contain cycles that previously aborted the run) and honors `--ignore` patterns during directory descent. Manifests whose `zod` range cannot satisfy the v2 floor get a warning describing the runtime failure mode. `RunnerResult.packageJsonChanges` is now an array of per-manifest changes with optional `warnings` and `notes`. diff --git a/.changeset/codemod-schema-drop-receivers.md b/.changeset/codemod-schema-drop-receivers.md new file mode 100644 index 0000000000..e1a1ff2aa5 --- /dev/null +++ b/.changeset/codemod-schema-drop-receivers.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +The explicit-`undefined` result-schema removal now requires the same proof as the schema-identifier path: `request()` calls must carry a provably literal spec method, and `callTool()` calls whose first argument is a primitive are left alone. Previously, any file importing an MCP package could have the middle argument deleted from an unrelated `.request()` / `.callTool()` member call on a non-SDK receiver (e.g. a bespoke `end.request('ping', undefined, id)` helper), corrupting the call. diff --git a/.changeset/config.json b/.changeset/config.json index 09102e78e8..0154d8a2b8 100644 --- a/.changeset/config.json +++ b/.changeset/config.json @@ -7,8 +7,5 @@ "access": "public", "baseBranch": "main", "updateInternalDependencies": "patch", - "ignore": [ - "@modelcontextprotocol/examples", - "@mcp-examples/*" - ] + "ignore": ["@modelcontextprotocol/examples", "@mcp-examples/*"] } diff --git a/.changeset/fix-server-protocol-version.md b/.changeset/fix-server-protocol-version.md index 02100923e7..5344b1429e 100644 --- a/.changeset/fix-server-protocol-version.md +++ b/.changeset/fix-server-protocol-version.md @@ -2,4 +2,4 @@ '@modelcontextprotocol/server': patch --- -fix(server): propagate negotiated protocol version to transport in _oninitialize +fix(server): propagate negotiated protocol version to transport in \_oninitialize diff --git a/.changeset/fix-unknown-tool-protocol-error.md b/.changeset/fix-unknown-tool-protocol-error.md index 6d214221ca..e6a0e0d32f 100644 --- a/.changeset/fix-unknown-tool-protocol-error.md +++ b/.changeset/fix-unknown-tool-protocol-error.md @@ -1,6 +1,6 @@ --- -"@modelcontextprotocol/core-internal": minor -"@modelcontextprotocol/server": major +'@modelcontextprotocol/core-internal': minor +'@modelcontextprotocol/server': major --- Fix error handling for unknown tools and resources per MCP spec. diff --git a/.changeset/odd-forks-enjoy.md b/.changeset/odd-forks-enjoy.md index b1f57dc7ee..b9b6261831 100644 --- a/.changeset/odd-forks-enjoy.md +++ b/.changeset/odd-forks-enjoy.md @@ -1,5 +1,5 @@ --- -"@modelcontextprotocol/client": patch +'@modelcontextprotocol/client': patch --- fix(client): append custom Accept headers to spec-required defaults in StreamableHTTPClientTransport diff --git a/.changeset/pre.json b/.changeset/pre.json index 6706064de1..98c404ce1d 100644 --- a/.changeset/pre.json +++ b/.changeset/pre.json @@ -1,106 +1,106 @@ { - "mode": "pre", - "tag": "alpha", - "initialVersions": { - "@modelcontextprotocol/eslint-config": "2.0.0", - "@modelcontextprotocol/tsconfig": "2.0.0", - "@modelcontextprotocol/vitest-config": "2.0.0", - "@modelcontextprotocol/examples": "2.0.0-alpha.0", - "@modelcontextprotocol/client": "2.0.0-alpha.0", - "@modelcontextprotocol/core-internal": "2.0.0-alpha.0", - "@modelcontextprotocol/express": "2.0.0-alpha.0", - "@modelcontextprotocol/fastify": "2.0.0-alpha.0", - "@modelcontextprotocol/hono": "2.0.0-alpha.0", - "@modelcontextprotocol/node": "2.0.0-alpha.0", - "@modelcontextprotocol/server": "2.0.0-alpha.0", - "@modelcontextprotocol/server-legacy": "2.0.0-alpha.0", - "@modelcontextprotocol/core": "2.0.0-alpha.0", - "@modelcontextprotocol/codemod": "2.0.0-alpha.0", - "@modelcontextprotocol/test-conformance": "2.0.0-alpha.0", - "@modelcontextprotocol/test-helpers": "2.0.0-alpha.0", - "@modelcontextprotocol/test-integration": "2.0.0-alpha.0", - "@modelcontextprotocol/test-e2e": "2.0.0-alpha.0" - }, - "changesets": [ - "abort-handlers-on-close", - "add-consumer-sse-e2e", - "add-core-public-package", - "add-e2e-test-suite", - "add-fastify-middleware", - "add-hono-peer-dep", - "add-resource-size-field", - "add-sdk-http-error", - "add-server-legacy-package", - "bound-resumability-version-gates", - "brave-lions-glow", - "busy-rice-smoke", - "busy-weeks-hang", - "cfworker-out-of-barrel", - "codemod-core-routing", - "codemod-infer-project-type", - "codemod-resolve-legacy-imports", - "codemod-streamablehttperror-sdkhttperror", - "codemod-task-handler-methods", - "custom-methods-minimal", - "cyan-cycles-pump", - "draft-spec-non-sep-conformance", - "drop-zod-peer-dep", - "export-inmemory-transport", - "expose-auth-server-discovery", - "expose-icons-on-tools-and-prompts", - "express-resource-server-auth", - "extract-task-manager", - "fast-dragons-lead", - "finish-sdkerror-capability", - "fix-abort-listener-leak", - "fix-conformance-server-leak", - "fix-oauth-5xx-discovery", - "fix-onerror-callbacks", - "fix-server-protocol-version", - "fix-session-status-codes", - "fix-stdio-epipe-crash", - "fix-stdio-windows-hide", - "fix-streamable-close-reentrant", - "fix-streamable-http-error-response", - "fix-task-session-isolation", - "fix-transport-exact-optional-property-types", - "fix-unknown-tool-protocol-error", - "fix-validate-client-metadata-url", - "funky-baths-attack", - "gentle-planets-rest", - "heavy-walls-swim", - "hono-peer-optional", - "idjag-spec-type-export", - "legacy-module-resolution-types", - "oauth-error-http200", - "odd-forks-enjoy", - "quick-islands-occur", - "reconnection-scheduler", - "register-rawshape-compat", - "remove-websocket-transport", - "respect-capability-negotiation", - "restore-task-wire-types", - "rich-hounds-report", - "schema-object-type-for-unions", - "sep-2577-deprecate-runtime-apis", - "sep-2663-tasks-removal", - "sep-414-trace-context-meta-keys", - "shy-times-learn", - "spec-reference-types-2026-07-28", - "spec-type-schema", - "spotty-cats-tickle", - "stdio-max-buffer-size", - "stdio-skip-non-json", - "stdio-subpath-export", - "support-standard-json-schema", - "tame-camels-greet", - "tender-snails-fold", - "token-provider-composable-auth", - "twelve-dodos-taste", - "use-scopes-supported-in-dcr", - "workerd-shim-vendors-cfworker", - "wraphandler-hook", - "zod-json-schema-compat", - "zod-jsonschema-fallback" - ] + "mode": "pre", + "tag": "alpha", + "initialVersions": { + "@modelcontextprotocol/eslint-config": "2.0.0", + "@modelcontextprotocol/tsconfig": "2.0.0", + "@modelcontextprotocol/vitest-config": "2.0.0", + "@modelcontextprotocol/examples": "2.0.0-alpha.0", + "@modelcontextprotocol/client": "2.0.0-alpha.0", + "@modelcontextprotocol/core-internal": "2.0.0-alpha.0", + "@modelcontextprotocol/express": "2.0.0-alpha.0", + "@modelcontextprotocol/fastify": "2.0.0-alpha.0", + "@modelcontextprotocol/hono": "2.0.0-alpha.0", + "@modelcontextprotocol/node": "2.0.0-alpha.0", + "@modelcontextprotocol/server": "2.0.0-alpha.0", + "@modelcontextprotocol/server-legacy": "2.0.0-alpha.0", + "@modelcontextprotocol/core": "2.0.0-alpha.0", + "@modelcontextprotocol/codemod": "2.0.0-alpha.0", + "@modelcontextprotocol/test-conformance": "2.0.0-alpha.0", + "@modelcontextprotocol/test-helpers": "2.0.0-alpha.0", + "@modelcontextprotocol/test-integration": "2.0.0-alpha.0", + "@modelcontextprotocol/test-e2e": "2.0.0-alpha.0" + }, + "changesets": [ + "abort-handlers-on-close", + "add-consumer-sse-e2e", + "add-core-public-package", + "add-e2e-test-suite", + "add-fastify-middleware", + "add-hono-peer-dep", + "add-resource-size-field", + "add-sdk-http-error", + "add-server-legacy-package", + "bound-resumability-version-gates", + "brave-lions-glow", + "busy-rice-smoke", + "busy-weeks-hang", + "cfworker-out-of-barrel", + "codemod-core-routing", + "codemod-infer-project-type", + "codemod-resolve-legacy-imports", + "codemod-streamablehttperror-sdkhttperror", + "codemod-task-handler-methods", + "custom-methods-minimal", + "cyan-cycles-pump", + "draft-spec-non-sep-conformance", + "drop-zod-peer-dep", + "export-inmemory-transport", + "expose-auth-server-discovery", + "expose-icons-on-tools-and-prompts", + "express-resource-server-auth", + "extract-task-manager", + "fast-dragons-lead", + "finish-sdkerror-capability", + "fix-abort-listener-leak", + "fix-conformance-server-leak", + "fix-oauth-5xx-discovery", + "fix-onerror-callbacks", + "fix-server-protocol-version", + "fix-session-status-codes", + "fix-stdio-epipe-crash", + "fix-stdio-windows-hide", + "fix-streamable-close-reentrant", + "fix-streamable-http-error-response", + "fix-task-session-isolation", + "fix-transport-exact-optional-property-types", + "fix-unknown-tool-protocol-error", + "fix-validate-client-metadata-url", + "funky-baths-attack", + "gentle-planets-rest", + "heavy-walls-swim", + "hono-peer-optional", + "idjag-spec-type-export", + "legacy-module-resolution-types", + "oauth-error-http200", + "odd-forks-enjoy", + "quick-islands-occur", + "reconnection-scheduler", + "register-rawshape-compat", + "remove-websocket-transport", + "respect-capability-negotiation", + "restore-task-wire-types", + "rich-hounds-report", + "schema-object-type-for-unions", + "sep-2577-deprecate-runtime-apis", + "sep-2663-tasks-removal", + "sep-414-trace-context-meta-keys", + "shy-times-learn", + "spec-reference-types-2026-07-28", + "spec-type-schema", + "spotty-cats-tickle", + "stdio-max-buffer-size", + "stdio-skip-non-json", + "stdio-subpath-export", + "support-standard-json-schema", + "tame-camels-greet", + "tender-snails-fold", + "token-provider-composable-auth", + "twelve-dodos-taste", + "use-scopes-supported-in-dcr", + "workerd-shim-vendors-cfworker", + "wraphandler-hook", + "zod-json-schema-compat", + "zod-jsonschema-fallback" + ] } diff --git a/.changeset/resource-not-found-32602.md b/.changeset/resource-not-found-32602.md index a578ecda5f..e4f613117f 100644 --- a/.changeset/resource-not-found-32602.md +++ b/.changeset/resource-not-found-32602.md @@ -1,7 +1,7 @@ --- -"@modelcontextprotocol/core-internal": minor -"@modelcontextprotocol/server": major -"@modelcontextprotocol/client": minor +'@modelcontextprotocol/core-internal': minor +'@modelcontextprotocol/server': major +'@modelcontextprotocol/client': minor --- `resources/read` for an unknown URI now answers with JSON-RPC error code `-32602` diff --git a/.changeset/sep-2663-tasks-removal.md b/.changeset/sep-2663-tasks-removal.md index 8316165b50..6da4fba840 100644 --- a/.changeset/sep-2663-tasks-removal.md +++ b/.changeset/sep-2663-tasks-removal.md @@ -3,4 +3,5 @@ '@modelcontextprotocol/server': major '@modelcontextprotocol/client': major --- -SEP-2663: remove 2025-11 experimental tasks (TaskManager, experimental.tasks.* accessors). Tasks are now Extensions Track. + +SEP-2663: remove 2025-11 experimental tasks (TaskManager, experimental.tasks.\* accessors). Tasks are now Extensions Track. diff --git a/.changeset/support-standard-json-schema.md b/.changeset/support-standard-json-schema.md index 38a9998893..a8856479ba 100644 --- a/.changeset/support-standard-json-schema.md +++ b/.changeset/support-standard-json-schema.md @@ -13,9 +13,13 @@ Tool and prompt registration now accepts any schema library that implements the ```typescript import { type } from 'arktype'; -server.registerTool('greet', { - inputSchema: type({ name: 'string' }) -}, async ({ name }) => ({ content: [{ type: 'text', text: `Hello, ${name}!` }] })); +server.registerTool( + 'greet', + { + inputSchema: type({ name: 'string' }) + }, + async ({ name }) => ({ content: [{ type: 'text', text: `Hello, ${name}!` }] }) +); ``` For raw JSON Schema (e.g. TypeBox output), use the new `fromJsonSchema` adapter: @@ -23,12 +27,17 @@ For raw JSON Schema (e.g. TypeBox output), use the new `fromJsonSchema` adapter: ```typescript import { fromJsonSchema } from '@modelcontextprotocol/server'; -server.registerTool('greet', { - inputSchema: fromJsonSchema({ type: 'object', properties: { name: { type: 'string' } } }) -}, handler); +server.registerTool( + 'greet', + { + inputSchema: fromJsonSchema({ type: 'object', properties: { name: { type: 'string' } } }) + }, + handler +); ``` **Breaking changes:** + - `experimental.tasks.getTaskResult()` no longer accepts a `resultSchema` parameter. Returns `GetTaskPayloadResult` (a loose `Result`); cast to the expected type at the call site. - Removed unused exports from `@modelcontextprotocol/core-internal`: `SchemaInput`, `schemaToJson`, `parseSchemaAsync`, `getSchemaShape`, `getSchemaDescription`, `isOptionalSchema`, `unwrapOptionalSchema`. Use the new `standardSchemaToJsonSchema` and `validateStandardSchema` instead. - `completable()` remains Zod-specific (it relies on Zod's `.shape` introspection). diff --git a/.changeset/twelve-dodos-taste.md b/.changeset/twelve-dodos-taste.md index 1b0fdc19d9..3f523fad00 100644 --- a/.changeset/twelve-dodos-taste.md +++ b/.changeset/twelve-dodos-taste.md @@ -1,5 +1,5 @@ --- -"@modelcontextprotocol/express": patch +'@modelcontextprotocol/express': patch --- Add jsonLimit option to createMcpExpressApp diff --git a/packages/codemod/README.md b/packages/codemod/README.md index 52850247bc..f21089fc7c 100644 --- a/packages/codemod/README.md +++ b/packages/codemod/README.md @@ -6,6 +6,9 @@ Codemods for migrating MCP TypeScript SDK code between major versions. ```bash npx @modelcontextprotocol/codemod@alpha v1-to-v2 . + +# or a single source file (manifest changes are reported, not applied) +npx @modelcontextprotocol/codemod@alpha v1-to-v2 src/server.ts ``` The codemod rewrites TypeScript and JavaScript source files @@ -33,8 +36,14 @@ raw shapes with `z.object()`), drop the result-schema argument from `client.requ `@modelcontextprotocol/core`, rename `StreamableHTTPError` → `SdkHttpError` / `IsomorphicHeaders` → `Headers`, rewrite `SchemaInput` → `StandardSchemaWithJSON.InferInput`, route -`ErrorCode.{RequestTimeout,ConnectionClosed}` to `SdkErrorCode`, and rewrite `vi.mock` -/ `jest.mock` / dynamic `import()` paths. +`ErrorCode.{RequestTimeout,ConnectionClosed}` to `SdkErrorCode` (rewriting an +all-SDK condition's `instanceof ProtocolError` guard to `SdkError`, and marking +guards that mix the two enums), add `import { z } from 'zod'` when a wrap needs +it, rewrite `vi.mock` +/ `jest.mock` / dynamic `import()` paths, invert optional completable nesting +(`completable(schema.optional(), cb)` becomes `completable(schema, cb).optional()`), +and drop `Protocol` / `mergeCapabilities` (no v2 export) with an action-required +marker naming the replacement. ## `@mcp-codemod-error` markers @@ -54,7 +63,8 @@ grep -rn '@mcp-codemod-error' . ## What it does NOT cover -CJS→ESM / Node 20 pre-flight, `new Headers()` / `.get()` access rewrites, OAuth +CJS→ESM / Node 20 pre-flight, header **read** rewrites (`ctx.http?.req?.headers` +bracket access → `.get()`; sending plain-record headers keeps working), OAuth error-class consolidation (`instanceof InvalidGrantError` → `OAuthError` + `OAuthErrorCode`), per-scenario `SdkErrorCode` branch selection, `ctx.mcpReq.send()` schema-arg drop, and behavioral adaptation are manual — see the diff --git a/packages/codemod/package.json b/packages/codemod/package.json index fa4a15ec3e..db4f4d7ae1 100644 --- a/packages/codemod/package.json +++ b/packages/codemod/package.json @@ -49,6 +49,7 @@ }, "dependencies": { "commander": "^13.0.0", + "fast-glob": "^3.3.3", "ts-morph": "^28.0.0" }, "devDependencies": { diff --git a/packages/codemod/src/cli.ts b/packages/codemod/src/cli.ts index 8325b352e6..e9738990c7 100644 --- a/packages/codemod/src/cli.ts +++ b/packages/codemod/src/cli.ts @@ -8,6 +8,7 @@ import { Command } from 'commander'; import { listMigrations } from './migrations/index'; import { run } from './runner'; +import type { PackageJsonChange } from './types'; import { DiagnosticLevel } from './types'; import { detectFormatter } from './utils/detectFormatter'; import { CODEMOD_ERROR_PREFIX, formatDiagnostic } from './utils/diagnostics'; @@ -15,10 +16,32 @@ import { CODEMOD_ERROR_PREFIX, formatDiagnostic } from './utils/diagnostics'; const require = createRequire(import.meta.url); const { version } = require('../package.json') as { version: string }; +const SOURCE_FILE_RE = /\.(?:ts|tsx|mts|cts|js|jsx|mjs|cjs)$/; + function quoteArg(arg: string): string { return /\s/.test(arg) ? `"${arg}"` : arg; } +function hasManifestEdits(pc: PackageJsonChange): boolean { + return pc.added.length > 0 || pc.removed.length > 0; +} + +function printManifestChange(pc: PackageJsonChange): void { + console.log(` ${path.relative(process.cwd(), pc.packageJsonPath) || pc.packageJsonPath}`); + if (pc.removed.length > 0) { + console.log(` Removed: ${pc.removed.join(', ')}`); + } + if (pc.added.length > 0) { + console.log(` Added: ${pc.added.join(', ')}`); + } + for (const n of pc.notes ?? []) { + console.log(` Note: ${n}`); + } + for (const w of pc.warnings ?? []) { + console.log(` Warning: ${w}`); + } +} + /** * The codemod transforms the AST but does not reformat — wrapped schemas and * generated string literals can violate a repo's lint/formatting rules. Point @@ -71,8 +94,15 @@ for (const [name, migration] of listMigrations()) { const resolvedDir = path.resolve(targetDir); - if (!existsSync(resolvedDir) || !statSync(resolvedDir).isDirectory()) { - console.error(`\nError: "${resolvedDir}" is not a valid directory.\n`); + const targetExists = existsSync(resolvedDir); + const targetIsFile = targetExists && statSync(resolvedDir).isFile(); + if (!targetExists || (!statSync(resolvedDir).isDirectory() && !(targetIsFile && SOURCE_FILE_RE.test(resolvedDir)))) { + console.error(`\nError: "${resolvedDir}" is not a directory or a TypeScript/JavaScript source file.\n`); + process.exitCode = 1; + return; + } + if (targetIsFile && /\.d\.(?:ts|mts|cts)$/.test(resolvedDir)) { + console.error(`\nError: "${resolvedDir}" is a declaration file — declaration files are not migration targets.\n`); process.exitCode = 1; return; } @@ -96,18 +126,44 @@ for (const [name, migration] of listMigrations()) { }); if (result.filesChanged === 0 && result.diagnostics.length === 0 && !result.packageJsonChanges) { - console.log('No changes needed — code already migrated or no SDK imports found.\n'); + if (result.mcpImportFiles > 0 && result.v1ImportFiles === 0) { + console.log(`No changes needed — ${result.mcpImportFiles} file(s) already import the v2 packages.\n`); + } else if (result.v1ImportFiles > 0) { + console.log( + `No changes applied — ${result.v1ImportFiles} file(s) still import the v1 SDK ` + + `(check which transforms were selected).\n` + ); + } else { + console.log(`No MCP SDK imports found under ${resolvedDir} — nothing to migrate.\n`); + } return; } + if (targetIsFile && result.packageJsonChanges) { + console.log( + 'Single-file run: manifest changes are reported for the owning package but not applied — ' + + 'run the codemod at the package root to apply them.\n' + ); + } + if (result.filesChanged > 0) { console.log(`Changes: ${result.totalChanges} across ${result.filesChanged} file(s)\n`); } if (opts['verbose']) { - console.log('Files modified:'); - for (const fr of result.fileResults) { - console.log(` ${fr.filePath} (${fr.changes} change(s))`); + const modified = result.fileResults.filter(fr => fr.changes > 0); + const diagnosticsOnly = result.fileResults.filter(fr => fr.changes === 0 && fr.diagnostics.length > 0); + if (modified.length > 0) { + console.log('Files modified:'); + for (const fr of modified) { + console.log(` ${fr.filePath} (${fr.changes} change(s))`); + } + } + if (diagnosticsOnly.length > 0) { + console.log('Files with diagnostics only (not modified):'); + for (const fr of diagnosticsOnly) { + console.log(` ${fr.filePath} (${fr.diagnostics.length} diagnostic(s))`); + } } console.log(''); } @@ -149,18 +205,36 @@ for (const [name, migration] of listMigrations()) { console.log(''); } - if (result.packageJsonChanges) { - const pc = result.packageJsonChanges; - if (opts['dryRun']) { - console.log('package.json changes (dry run — not applied):'); - } else { - console.log('package.json updated:'); + const manifestChanges = result.packageJsonChanges ?? []; + // In single-file mode nothing is written, so even the nearest manifest's + // change set is presented as a report. + const appliedManifests = manifestChanges.filter(pc => pc.applied && hasManifestEdits(pc) && !targetIsFile); + const reportedManifests = manifestChanges.filter(pc => (!pc.applied || targetIsFile) && hasManifestEdits(pc)); + const warningOnlyManifests = manifestChanges.filter(pc => !hasManifestEdits(pc)); + if (manifestChanges.length > 0) { + if (appliedManifests.length > 0) { + if (opts['dryRun']) { + console.log('package.json changes (dry run — not applied):'); + } else { + console.log('package.json updated:'); + } + for (const pc of appliedManifests) { + printManifestChange(pc); + } } - if (pc.removed.length > 0) { - console.log(` Removed: ${pc.removed.join(', ')}`); + if (reportedManifests.length > 0) { + console.log( + 'Manifests that declare @modelcontextprotocol/sdk but are not modified by the codemod — apply the listed changes yourself:' + ); + for (const pc of reportedManifests) { + printManifestChange(pc); + } } - if (pc.added.length > 0) { - console.log(` Added: ${pc.added.join(', ')}`); + if (warningOnlyManifests.length > 0) { + console.log('Manifest warnings:'); + for (const pc of warningOnlyManifests) { + printManifestChange(pc); + } } console.log(''); } @@ -177,7 +251,11 @@ for (const [name, migration] of listMigrations()) { } else { const changedFiles = result.fileResults.filter(fr => fr.changes > 0).map(fr => fr.filePath); printFormatGuidance(resolvedDir, changedFiles); - if (result.packageJsonChanges) { + if (reportedManifests.length > 0) { + console.log( + 'Apply the manifest changes listed above, then run your package manager to install the new packages.\n' + ); + } else if (appliedManifests.length > 0) { console.log('Run your package manager to install the new packages.\n'); } console.log('Migration complete. Review the changes and run your build/tests.\n'); diff --git a/packages/codemod/src/migrations/v1-to-v2/mappings/importMap.ts b/packages/codemod/src/migrations/v1-to-v2/mappings/importMap.ts index e861140fd2..6d6871f252 100644 --- a/packages/codemod/src/migrations/v1-to-v2/mappings/importMap.ts +++ b/packages/codemod/src/migrations/v1-to-v2/mappings/importMap.ts @@ -16,7 +16,13 @@ export interface ImportMapping { */ schemaSymbolTarget?: string; removalMessage?: string; - /** No entries currently set this; scaffolding for when a v1 symbol has no v2 equivalent yet. */ + /** + * Symbols from this module that have no v2 export anywhere. They are dropped from + * the rewritten import and the call site gets an action-required marker carrying + * the message, instead of an import of a member the target package does not have. + */ + removedSymbols?: Record; + /** Marks a module-level removal as a known v2 gap (downgrades the removal diagnostic to the v2-gap category). For per-symbol removals use `removedSymbols`. */ isV2Gap?: boolean; /** Emitted as an info diagnostic after a successful move, suggesting eventual migration to v2 equivalents. */ migrationHint?: string; @@ -27,6 +33,18 @@ export interface ImportMapping { subpathSuffix?: string; } +/** + * Resource-server auth helpers whose maintained v2 home is `@modelcontextprotocol/express`; + * the server-legacy/auth copy they route to by default is a frozen v1 snapshot, so import + * and re-export sites get a marker prompting a deliberate re-point. + */ +export const RS_ONLY_AUTH_SYMBOLS: ReadonlySet = new Set([ + 'requireBearerAuth', + 'mcpAuthMetadataRouter', + 'getOAuthProtectedResourceMetadataUrl', + 'OAuthTokenVerifier' +]); + export const IMPORT_MAP: Record = { '@modelcontextprotocol/sdk/client/index.js': { target: '@modelcontextprotocol/client', @@ -114,9 +132,11 @@ export const IMPORT_MAP: Record = { }, '@modelcontextprotocol/sdk/server/auth/types.js': { - target: '@modelcontextprotocol/server-legacy/auth', - status: 'moved', - migrationHint: 'Legacy auth types. AuthInfo is also re-exported by @modelcontextprotocol/server.' + // The module's only export (AuthInfo) is re-exported by both leaf packages, so + // routing by context avoids pulling the deprecated legacy package into projects + // that use no authorization-server helpers. + target: 'RESOLVE_BY_CONTEXT', + status: 'moved' }, '@modelcontextprotocol/sdk/server/auth/provider.js': { target: '@modelcontextprotocol/server-legacy/auth', @@ -149,7 +169,17 @@ export const IMPORT_MAP: Record = { }, '@modelcontextprotocol/sdk/shared/protocol.js': { target: 'RESOLVE_BY_CONTEXT', - status: 'moved' + status: 'moved', + removedSymbols: { + Protocol: + 'The Protocol base class is not exported by the v2 packages. To observe or handle inbound requests ' + + 'that have no registered handler, use client.fallbackRequestHandler / server.fallbackRequestHandler; ' + + 'build custom behavior on Client or Server instead of subclassing Protocol. ' + + 'See the migration guide: Behavioral changes > Client connection & dispatch.', + mergeCapabilities: + 'mergeCapabilities() is not exported by the v2 packages. Pass the complete capabilities object to the ' + + 'Client/Server constructor, or merge capability objects with a plain object spread.' + } }, '@modelcontextprotocol/sdk/shared/transport.js': { target: 'RESOLVE_BY_CONTEXT', diff --git a/packages/codemod/src/migrations/v1-to-v2/mappings/schemaRouting.ts b/packages/codemod/src/migrations/v1-to-v2/mappings/schemaRouting.ts index 7aead7c17a..7a18e5fa0b 100644 --- a/packages/codemod/src/migrations/v1-to-v2/mappings/schemaRouting.ts +++ b/packages/codemod/src/migrations/v1-to-v2/mappings/schemaRouting.ts @@ -37,3 +37,13 @@ export function symbolTargetOverride(name: string, mapping: ImportMapping): stri } return undefined; } + +/** + * Guidance for a symbol from this module that has no v2 export anywhere (it must be + * dropped/flagged rather than routed), or `undefined` when the symbol routes normally. + * Shared by the static-import, re-export, mock, and dynamic-import rewrites so all + * four treat such symbols the same way. + */ +export function removedSymbolGuidance(name: string, mapping: ImportMapping): string | undefined { + return mapping.removedSymbols?.[name]; +} diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/completableNesting.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/completableNesting.ts new file mode 100644 index 0000000000..0ec766cd14 --- /dev/null +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/completableNesting.ts @@ -0,0 +1,192 @@ +import type { CallExpression, Expression, PropertyAccessExpression, SourceFile } from 'ts-morph'; +import { SyntaxKind } from 'ts-morph'; + +import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types'; +import { actionRequired, info, warning } from '../../../utils/diagnostics'; +import { isAnyMcpSpecifier } from '../../../utils/importUtils'; + +/** + * v2 resolves completion metadata on the schema found AFTER unwrapping an outer + * optional wrapper, so the v1 idiom `completable(z.string().optional(), cb)` registers + * the metadata one level too deep: completions come back empty and, when no argument + * carries metadata in the v2 position, the server does not advertise the completions + * capability. The working v2 spelling inverts the nesting: + * `completable(z.string(), cb).optional()`. + * + * Only `optional` wrappers matter — the v2 lookup unwraps exactly one outer optional + * layer and nothing else, so `.default()` / `.nullable()` / `.catch()` arguments keep + * working as-is and are left alone. `.nullish()` is the one non-`optional` tail whose + * outer layer IS an optional wrapper, so it gets a marker with the concrete rewrite. + */ + +type ArgVerdict = + | { kind: 'invert'; innerSchemaText: string } + | { kind: 'nullish' } + | { kind: 'buried-optional' } + | { kind: 'plain-call' } + | { kind: 'opaque' }; + +function unparenthesize(expr: Expression): Expression { + let current = expr; + while (true) { + const paren = current.asKind(SyntaxKind.ParenthesizedExpression); + if (!paren) return current; + current = paren.getExpression(); + } +} + +/** + * Classify the first argument of a completable() call. + * + * - `invert`: outermost wrapper is `optional` and the rewrite is mechanical. + * - `nullish`: outermost wrapper is `.nullish()` (its outer layer is an optional + * wrapper) — marker with the concrete manual rewrite. + * - `buried-optional`: an `optional`/`nullish` link sits deeper in the method chain + * (e.g. `.optional().describe(...)`) — marker; the safe spot for the wrapper + * depends on the chain. + * - `plain-call`: a call chain with no optional wrapper anywhere — leave alone. + * - `opaque`: not a call (identifier, property access, …) — the schema may be + * optional-wrapped where it is defined; one nudge per file. + */ +function classifyArg(arg: Expression): ArgVerdict { + const outer = unparenthesize(arg).asKind(SyntaxKind.CallExpression); + if (!outer) return { kind: 'opaque' }; + + const callee = outer.getExpression().asKind(SyntaxKind.PropertyAccessExpression); + if (callee) { + const name = callee.getName(); + const args = outer.getArguments(); + if (name === 'optional') { + if (args.length === 0) return { kind: 'invert', innerSchemaText: callee.getExpression().getText() }; + if (args.length === 1) return { kind: 'invert', innerSchemaText: args[0]!.getText() }; + return { kind: 'buried-optional' }; + } + if (name === 'nullish') return { kind: 'nullish' }; + } + + // Walk the rest of the chain (and factory-call arguments) looking for a buried + // optional/nullish link, e.g. `z.string().optional().describe('x')` or + // `z.optional(z.string()).meta({...})`. + let current: Expression | undefined = unparenthesize(arg); + while (current) { + const call: CallExpression | undefined = current.asKind(SyntaxKind.CallExpression); + if (!call) break; + const pa: PropertyAccessExpression | undefined = call.getExpression().asKind(SyntaxKind.PropertyAccessExpression); + if (!pa) break; + const name = pa.getName(); + if (name === 'optional' || name === 'nullish') return { kind: 'buried-optional' }; + current = pa.getExpression(); + } + return { kind: 'plain-call' }; +} + +export const completableNestingTransform: Transform = { + name: 'Completable optional-nesting inversion', + id: 'completable-nesting', + apply(sourceFile: SourceFile, _context: TransformContext): TransformResult { + const diagnostics: Diagnostic[] = []; + let changesCount = 0; + const filePath = sourceFile.getFilePath(); + + // Local bindings of `completable` from an MCP package (named import, possibly + // aliased) and MCP namespace bindings (`ns.completable(...)`). importPaths runs + // earlier, so the specifier is normally the v2 package already; the v1 specifier + // is accepted too so the transform also works in isolation. + const localNames = new Set(); + const namespaceNames = new Set(); + for (const imp of sourceFile.getImportDeclarations()) { + if (!isAnyMcpSpecifier(imp.getModuleSpecifierValue())) continue; + for (const ni of imp.getNamedImports()) { + if (ni.getName() === 'completable') localNames.add(ni.getAliasNode()?.getText() ?? 'completable'); + } + const ns = imp.getNamespaceImport(); + if (ns) namespaceNames.add(ns.getText()); + } + if (localNames.size === 0 && namespaceNames.size === 0) { + return { changesCount: 0, diagnostics: [] }; + } + + const completableCalls = sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression).filter((call: CallExpression) => { + const expr = call.getExpression(); + if (expr.getKind() === SyntaxKind.Identifier) return localNames.has(expr.getText()); + const pa = expr.asKind(SyntaxKind.PropertyAccessExpression); + return pa !== undefined && pa.getName() === 'completable' && namespaceNames.has(pa.getExpression().getText()); + }); + + let opaqueArgNoted = false; + // Reverse document order so each replacement leaves the still-unprocessed + // (earlier) nodes' positions intact. + for (const call of completableCalls.toReversed()) { + const args = call.getArguments(); + if (args.length === 0) continue; + const verdict = classifyArg(args[0]! as Expression); + switch (verdict.kind) { + case 'invert': { + const line = call.getStartLineNumber(); + const calleeText = call.getExpression().getText(); + const restArgs = args.slice(1).map(a => a.getText()); + call.replaceWithText(`${calleeText}(${[verdict.innerSchemaText, ...restArgs].join(', ')}).optional()`); + changesCount++; + diagnostics.push( + info( + filePath, + line, + `completable() with an optional schema argument: moved .optional() outside the call — v2 resolves ` + + `completion metadata after unwrapping an outer optional wrapper, so completable(schema, cb).optional() ` + + `keeps both optionality and completions.` + ) + ); + break; + } + case 'nullish': { + diagnostics.push( + actionRequired( + filePath, + call, + `completable() first argument ends in .nullish(), whose outer layer is an optional wrapper — v2 resolves ` + + `completion metadata after unwrapping it, so completions would come back empty. Rewrite as ` + + `completable(schema.nullable(), cb).optional().` + ) + ); + break; + } + case 'buried-optional': { + diagnostics.push( + actionRequired( + filePath, + call, + `completable() first argument contains an optional wrapper inside its method chain — v2 resolves ` + + `completion metadata after unwrapping an outer optional wrapper, so the v1 nesting returns empty ` + + `completion lists. Move the optional wrapping to the completable(...) result ` + + `(completable(schema, cb).optional()) and rebuild the rest of the chain around it.` + ) + ); + break; + } + case 'opaque': { + // The schema may be optional-wrapped where it is defined. One nudge per + // file, no marker (plain schemas via identifiers are common and fine). + if (!opaqueArgNoted) { + opaqueArgNoted = true; + diagnostics.push({ + ...warning( + filePath, + call.getStartLineNumber(), + `completable() receives a schema by reference — verify the referenced schema is not wrapped in ` + + `.optional() inside the call. v2 resolves completion metadata after unwrapping an outer optional ` + + `wrapper, so optionality belongs on the completable(...) result: completable(schema, cb).optional().` + ), + advisoryOnly: true + }); + } + break; + } + case 'plain-call': { + break; + } + } + } + + return { changesCount, diagnostics }; + } +}; diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts index 78ebc2487d..dd8abbd137 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts @@ -7,6 +7,8 @@ import { actionRequired, info } from '../../../utils/diagnostics'; import { hasMcpImports } from '../../../utils/importUtils'; import { CONTEXT_PROPERTY_MAP, CTX_PARAM_NAME, EXTRA_PARAM_NAME } from '../mappings/contextPropertyMap'; +const CONTEXT_LIKE_KEYS = new Set(CONTEXT_PROPERTY_MAP.map(mapping => mapping.from.slice(1))); + const HANDLER_METHODS = new Set(['setRequestHandler', 'setNotificationHandler']); const REGISTER_METHODS = new Set(['registerTool', 'registerPrompt', 'registerResource', 'tool', 'prompt', 'resource']); @@ -29,9 +31,25 @@ function processCallback( const params = callbackNode.getParameters(); if (params.length < 2) return -1; - const extraParam = params[1]!; + // The context argument is always trailing — registerTool/registerPrompt callbacks + // are (args, extra) but a registerResource template read callback is + // (uri, variables, extra) — so find the parameter named `extra` among the + // non-first positions instead of assuming index 1. A destructured trailing + // parameter is selected too, so it reaches the destructuring diagnostic below. + const trailing = params.at(-1)!; + const extraParam = + params.slice(1).find(par => !Node.isObjectBindingPattern(par.getNameNode()) && par.getName() === EXTRA_PARAM_NAME) ?? + (Node.isObjectBindingPattern(trailing.getNameNode()) ? trailing : undefined); + if (extraParam === undefined) return -1; const paramNameNode = extraParam.getNameNode(); if (Node.isObjectBindingPattern(paramNameNode)) { + // A destructured trailing parameter is only the context when its keys look + // like context members — a registerResource template read callback's + // `(uri, { owner, repo })` destructures the URI variables, not the context. + // Match the PROPERTY names (a renamed binding `{ signal: abort }` still + // destructures the context's `signal`). + const propertyNames = paramNameNode.getElements().map(el => el.getPropertyNameNode()?.getText() ?? el.getName()); + if (!propertyNames.some(name => CONTEXT_LIKE_KEYS.has(name))) return -1; diagnostics.push( actionRequired( sourceFile.getFilePath(), @@ -112,7 +130,9 @@ function processCallback( const propName = '.' + parent.getName(); const mapping = sortedMappings.find(m => m.from === propName); if (mapping) { - replacements.push({ node: parent, newText: CTX_PARAM_NAME + mapping.to }); + // Preserve optional chaining: `extra?.signal` stays defensive. + const joiner = parent.hasQuestionDotToken() ? '?' + mapping.to : mapping.to; + replacements.push({ node: parent, newText: CTX_PARAM_NAME + joiner }); continue; } } @@ -155,6 +175,33 @@ function processCallback( ); } + // A context object forwarded wholesale to a helper carries the v1 property shape + // into code this transform never sees — note each callee once. + const forwardedBody = callbackNode.getBody(); + if (forwardedBody) { + const notedCallees = new Set(); + forwardedBody.forEachDescendant(node => { + if (!Node.isCallExpression(node)) return; + const hasBareCtx = node + .getArguments() + .some(arg => Node.isIdentifier(arg) && arg.getText() === CTX_PARAM_NAME && !isKeyPositionIdentifier(arg)); + if (!hasBareCtx) return; + const calleeText = node.getExpression().getText(); + if (notedCallees.has(calleeText)) return; + notedCallees.add(calleeText); + diagnostics.push({ + ...actionRequired( + sourceFile.getFilePath(), + node, + `The context object is forwarded to ${calleeText}(…) — its property shape changed in v2 ` + + `(e.g. extra.signal is now ctx.mcpReq.signal, extra.sendRequest is ctx.mcpReq.send). Update the ` + + `helper's parameter type and property accesses.` + ), + advisoryOnly: true + }); + }); + } + // Warn on destructuring of ctx in body (after text replacement) const freshBody = callbackNode.getBody(); if (freshBody) { @@ -254,6 +301,207 @@ export const contextTypesTransform: Transform = { } } + changesCount += processFallbackHandlerAssignments(sourceFile, diagnostics); + changesCount += remapAnnotatedContextParams(sourceFile, diagnostics); + return { changesCount, diagnostics }; } }; + +/** + * `server.fallbackRequestHandler = async (request, extra) => { … }` — the assigned + * function is a handler callback in everything but registration shape. + */ +function processFallbackHandlerAssignments(sourceFile: SourceFile, diagnostics: Diagnostic[]): number { + let changes = 0; + for (const bin of sourceFile.getDescendantsOfKind(SyntaxKind.BinaryExpression)) { + if (bin.wasForgotten()) continue; + if (bin.getOperatorToken().getKind() !== SyntaxKind.EqualsToken) continue; + const left = bin.getLeft(); + if (!Node.isPropertyAccessExpression(left)) continue; + const name = left.getName(); + if (name !== 'fallbackRequestHandler' && name !== 'fallbackNotificationHandler') continue; + const rhs = bin.getRight(); + if (!Node.isArrowFunction(rhs) && !Node.isFunctionExpression(rhs)) continue; + const result = processCallback(rhs, sourceFile, diagnostics, name, bin.getStartLineNumber()); + if (result > 0) changes += result; + } + return changes; +} + +const CONTEXT_TYPE_NAMES = new Set(['RequestHandlerExtra', 'ServerContext', 'ClientContext']); +/** + * Split a type's text on top-level `|` only (angle brackets tracked). Returns + * undefined for shapes that can never be a bare context reference: intersections, + * object literals, and unbalanced text. + */ +function splitTopLevelUnion(typeText: string): string[] | undefined { + const parts: string[] = []; + let depth = 0; + let current = ''; + for (const ch of typeText) { + if (ch === '<') depth++; + else if (ch === '>') depth--; + if (depth === 0 && (ch === '&' || ch === '{')) return undefined; + if (ch === '|' && depth === 0) { + parts.push(current.trim()); + current = ''; + } else { + current += ch; + } + } + parts.push(current.trim()); + return depth === 0 ? parts.filter(part => part !== '') : undefined; +} + +const CONTEXT_TYPE_RE = /\b(?:RequestHandlerExtra|ServerContext|ClientContext)\b/; + +/** True when `name` is rebound by a function parameter between `node` and `scopeRoot`. */ +function isRebornWithin(node: Node, scopeRoot: Node, name: string): boolean { + let current = node.getParent(); + while (current !== undefined && current !== scopeRoot) { + if ( + (Node.isArrowFunction(current) || + Node.isFunctionExpression(current) || + Node.isFunctionDeclaration(current) || + Node.isMethodDeclaration(current)) && + current.getParameters().some(param => param.getName() === name) + ) { + return true; + } + current = current.getParent(); + } + return false; +} + +/** + * Functions and methods whose parameter is ANNOTATED as a context type (directly, + * `| undefined`-widened, or via same-file aliases resolved to fixpoint) carry the v1 + * property shape in their bodies even when the call-site scan never reaches them — + * private handler methods, alias-typed callbacks. The accesses remap in place; the + * parameter keeps its name. Annotations that mention a context type in a shape the + * remap cannot prove (unions with other types, containers) get an advisory instead + * of silence. + */ +function remapAnnotatedContextParams(sourceFile: SourceFile, diagnostics: Diagnostic[]): number { + // An alias only joins the set when its right-hand side IS a context type (a bare, + // possibly `| undefined`-widened reference to one, or to another accepted alias). + // A wrapper that merely MENTIONS a context type (`{ mcp: ServerContext; signal: … }`) + // has its own property shape — remapping accesses on it would corrupt them — so it + // falls through to the advisory path below. + const aliasNames = new Set(); + const isDirectContextReference = (typeText: string): boolean => { + const parts = splitTopLevelUnion(typeText); + if (parts === undefined) return false; + const names = parts.filter(part => part !== 'undefined' && part !== 'null'); + if (names.length !== 1) return false; + const base = names[0]!.replace(/<[\s\S]*$/, '').trim(); + return /^[A-Za-z_$][\w$]*$/.test(base) && (CONTEXT_TYPE_NAMES.has(base) || aliasNames.has(base)); + }; + let grew = true; + while (grew) { + grew = false; + for (const alias of sourceFile.getTypeAliases()) { + if (aliasNames.has(alias.getName())) continue; + if (isDirectContextReference(alias.getTypeNode()?.getText() ?? '')) { + aliasNames.add(alias.getName()); + grew = true; + } + } + } + + // Aliases that MENTION a context type without being one (wrappers, intersections, + // containers) cannot be remapped, but parameters typed with them still carry v1 + // context members somewhere inside — they get the advisory, never the rewrite. + const aliasMentions = new Set(aliasNames); + grew = true; + while (grew) { + grew = false; + for (const alias of sourceFile.getTypeAliases()) { + if (aliasMentions.has(alias.getName())) continue; + const text = alias.getTypeNode()?.getText() ?? ''; + const mentions = + CONTEXT_TYPE_RE.test(text) || [...aliasMentions].some(known => new RegExp(String.raw`\b${known}\b`).test(text)); + if (mentions) { + aliasMentions.add(alias.getName()); + grew = true; + } + } + } + + const matchesContextType = (annotation: string): boolean => { + // Strip generic arguments and nullable widening; the base must BE the type. + const base = annotation + .replace(/<[\s\S]*$/, '') + .replaceAll(/\s*\|\s*(undefined|null)\s*$/g, '') + .trim(); + return CONTEXT_TYPE_NAMES.has(base) || aliasNames.has(base); + }; + const mentionsContextType = (annotation: string): boolean => + CONTEXT_TYPE_RE.test(annotation) || [...aliasMentions].some(known => new RegExp(String.raw`\b${known}\b`).test(annotation)); + + const sortedMappings = [...CONTEXT_PROPERTY_MAP] + .filter(mapping => mapping.from !== mapping.to) + .toSorted((a, b) => b.from.length - a.from.length); + + let changes = 0; + sourceFile.forEachDescendant(node => { + if ( + !Node.isArrowFunction(node) && + !Node.isFunctionExpression(node) && + !Node.isFunctionDeclaration(node) && + !Node.isMethodDeclaration(node) + ) { + return; + } + for (const param of node.getParameters()) { + const annotation = param.getTypeNode()?.getText(); + if (annotation === undefined) continue; + const nameNode = param.getNameNode(); + if (Node.isObjectBindingPattern(nameNode)) continue; // destructured: handled by the marker path + if (!matchesContextType(annotation)) { + if (mentionsContextType(annotation)) { + diagnostics.push({ + ...actionRequired( + sourceFile.getFilePath(), + param, + `Parameter annotation '${annotation}' mentions a context type in a shape the codemod cannot ` + + `remap — review the body's property accesses (e.g. .signal is now .mcpReq.signal).` + ), + advisoryOnly: true + }); + } + continue; + } + const paramName = param.getName(); + const body = node.getBody(); + if (body === undefined) continue; + + const replacements: { target: import('ts-morph').PropertyAccessExpression; newText: string }[] = []; + body.forEachDescendant(descendant => { + if (!Node.isPropertyAccessExpression(descendant)) return; + const expr = descendant.getExpression(); + if (!Node.isIdentifier(expr) || expr.getText() !== paramName) return; + if (isRebornWithin(descendant, node, paramName)) return; + const mapping = sortedMappings.find(entry => entry.from === '.' + descendant.getName()); + if (mapping === undefined) return; + // Preserve optional chaining: `extra?.signal` stays defensive. + const joiner = descendant.hasQuestionDotToken() ? '?' + mapping.to : mapping.to; + replacements.push({ target: descendant, newText: paramName + joiner }); + }); + if (replacements.length === 0) continue; + for (const { target, newText } of replacements.toSorted((a, b) => b.target.getStart() - a.target.getStart())) { + target.replaceWithText(newText); + changes++; + } + diagnostics.push( + info( + sourceFile.getFilePath(), + param.getStartLineNumber(), + `Remapped v1 context property accesses on '${paramName}' (annotated ${annotation}) to the v2 shape.` + ) + ); + } + }); + return changes; +} diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/handlerRegistration.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/handlerRegistration.ts index 1dd2dcacc6..1ecfa00ce8 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/handlerRegistration.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/handlerRegistration.ts @@ -1,5 +1,5 @@ import type { SourceFile } from 'ts-morph'; -import { Node, SyntaxKind } from 'ts-morph'; +import { Node, SyntaxKind, VariableDeclarationKind } from 'ts-morph'; import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types'; import { actionRequired } from '../../../utils/diagnostics'; @@ -29,6 +29,35 @@ export const handlerRegistrationTransform: Transform = { if (!Node.isPropertyAccessExpression(expr)) continue; const methodName = expr.getName(); + if (methodName === 'removeRequestHandler' || methodName === 'removeNotificationHandler') { + const arg0 = call.getArguments()[0]; + if ( + arg0 === undefined || + Node.isStringLiteral(arg0) || + Node.isNoSubstitutionTemplateLiteral(arg0) || + Node.isTemplateExpression(arg0) + ) + continue; + const shapeMatch = arg0.getText().match(/^([A-Za-z_$][\w$]*)\.shape\.method\.value$/); + const shapeBase = shapeMatch ? (resolveOriginalImportName(sourceFile, shapeMatch[1]!) ?? shapeMatch[1]!) : undefined; + const shapeMethod = shapeBase === undefined ? undefined : ALL_SCHEMA_TO_METHOD[shapeBase]; + if (shapeMatch && shapeMethod !== undefined && isImportedFromMcp(sourceFile, shapeMatch[1]!)) { + arg0.replaceWithText(`'${shapeMethod}'`); + changesCount++; + removeUnusedImport(sourceFile, shapeMatch[1]!, true); + } else { + diagnostics.push({ + ...actionRequired( + sourceFile.getFilePath(), + call, + `${methodName} takes the method string in v2 — replace the schema-derived argument with the ` + + `literal method name (no change needed if this already passes a string).` + ), + advisoryOnly: true + }); + } + continue; + } if (methodName !== 'setRequestHandler' && methodName !== 'setNotificationHandler') { continue; } @@ -37,7 +66,35 @@ export const handlerRegistrationTransform: Transform = { if (args.length < 2) continue; const firstArg = args[0]!; - if (!Node.isIdentifier(firstArg)) continue; + if (!Node.isIdentifier(firstArg)) { + // A string first argument is already the v2 form; any other expression + // (inline schema object, property access, `X.shape.method.value`) is a + // v1 registration the rename path cannot resolve — mark it instead of + // skipping silently. + if (Node.isStringLiteral(firstArg) || Node.isNoSubstitutionTemplateLiteral(firstArg) || Node.isTemplateExpression(firstArg)) + continue; + const shapeMatch = firstArg.getText().match(/^([A-Za-z_$][\w$]*)\.shape\.method\.value$/); + const shapeBase = shapeMatch ? (resolveOriginalImportName(sourceFile, shapeMatch[1]!) ?? shapeMatch[1]!) : undefined; + const shapeMethod = shapeBase === undefined ? undefined : ALL_SCHEMA_TO_METHOD[shapeBase]; + if (shapeMatch && shapeMethod !== undefined && isImportedFromMcp(sourceFile, shapeMatch[1]!)) { + firstArg.replaceWithText(`'${shapeMethod}'`); + changesCount++; + removeUnusedImport(sourceFile, shapeMatch[1]!, true); + } else { + diagnostics.push({ + ...actionRequired( + sourceFile.getFilePath(), + call, + `${methodName} with a non-string first argument: v2 takes a method string — use the typed ` + + `two-argument spec form, or the three-argument custom form ` + + `(${methodName}('method/name', { params, result? }, handler)). No change is needed if the ` + + `argument already holds a method string.` + ), + advisoryOnly: true + }); + } + continue; + } const schemaName = firstArg.getText(); const originalName = resolveOriginalImportName(sourceFile, schemaName) ?? schemaName; @@ -56,7 +113,44 @@ export const handlerRegistrationTransform: Transform = { continue; } - const methodString = ALL_SCHEMA_TO_METHOD[originalName]; + let methodString = ALL_SCHEMA_TO_METHOD[originalName]; + if (methodString === undefined) { + // `const S = ListToolsRequestSchema; setRequestHandler(S, …)` — resolve + // one same-file variable hop before declaring the schema custom. Only an + // unambiguous binding qualifies: exactly one const declaration of the + // name anywhere in the file, so shadowed or reassigned locals keep the + // custom-handler marker instead of being silently rewritten. + const declsOfName = sourceFile + .getDescendantsOfKind(SyntaxKind.VariableDeclaration) + .filter(decl => decl.getName() === schemaName); + const localDecl = + declsOfName.length === 1 && + declsOfName[0]!.getVariableStatement()?.getDeclarationKind() === VariableDeclarationKind.Const + ? declsOfName[0] + : undefined; + const localInit = localDecl?.getInitializer(); + if (localInit !== undefined && Node.isIdentifier(localInit)) { + const initName = localInit.getText(); + const initOriginal = resolveOriginalImportName(sourceFile, initName) ?? initName; + if (REMOVED_TASK_SCHEMAS.has(initOriginal) && isImportedFromMcp(sourceFile, initName)) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + call, + `Task handler registration: ${methodName}(${schemaName}, ...). ` + + `The experimental tasks feature was removed in v2 (SEP-2663); the tasks/* method strings ` + + `are not part of the typed RequestMethod surface. Remove this registration. ` + + `See docs/migration/upgrade-to-v2.md#experimental-tasks-interception-removed.` + ) + ); + continue; + } + const viaLocal = ALL_SCHEMA_TO_METHOD[initOriginal]; + if (viaLocal !== undefined && isImportedFromMcp(sourceFile, initName)) { + methodString = viaLocal; + } + } + } if (!methodString) { diagnostics.push( actionRequired( @@ -70,7 +164,8 @@ export const handlerRegistrationTransform: Transform = { continue; } - if (!isImportedFromMcp(sourceFile, schemaName)) continue; + const viaLocalVariable = ALL_SCHEMA_TO_METHOD[originalName] === undefined; + if (!viaLocalVariable && !isImportedFromMcp(sourceFile, schemaName)) continue; firstArg.replaceWithText(`'${methodString}'`); changesCount++; diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts index 89397901f8..3b70c4ca99 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts @@ -1,14 +1,14 @@ -import type { SourceFile } from 'ts-morph'; +import type { Node, SourceFile } from 'ts-morph'; import { SyntaxKind } from 'ts-morph'; import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types'; -import { renameAllReferences } from '../../../utils/astUtils'; +import { findFirstIdentifierOutsideImports, renameAllReferences } from '../../../utils/astUtils'; import { actionRequired, info, v2Gap, warning } from '../../../utils/diagnostics'; import type { NamedImportSpec } from '../../../utils/importUtils'; import { addOrMergeImport, getSdkExports, getSdkImports, isTypeOnlyImport } from '../../../utils/importUtils'; import { resolveTypesPackage } from '../../../utils/projectAnalyzer'; import { AUTH_SCHEMA_NAMES_NO_V2_PUBLIC_EXPORT } from '../mappings/authSchemaNames'; -import { isAuthImport, lookupImportMapping } from '../mappings/importMap'; +import { isAuthImport, lookupImportMapping, RS_ONLY_AUTH_SYMBOLS } from '../mappings/importMap'; import { isSharedSchemaConst, resolveRenamedName, symbolTargetOverride } from '../mappings/schemaRouting'; import { SIMPLE_RENAMES } from '../mappings/symbolMap'; @@ -102,6 +102,48 @@ export const importPathsTransform: Transform = { continue; } + // Resource-server helpers have a maintained v2 home in @modelcontextprotocol/express; + // the server-legacy/auth copy they are routed to is a frozen v1 snapshot. The re-point + // is a judgment call (the express middleware answers verifier throws of the v1 error + // classes with a generic 500, so verifiers must move to the v2 OAuthError with it) — + // mark the import so the call sites get looked at instead of quietly staying on legacy. + if (mapping.target === '@modelcontextprotocol/server-legacy/auth') { + const matched = namedImports.filter(n => RS_ONLY_AUTH_SYMBOLS.has(n.getName())); + const valueMatchedSpecifiers = matched.filter(n => !typeOnly && !n.isTypeOnly()); + const valueMatched = valueMatchedSpecifiers.map(n => n.getName()); + const typeMatched = matched.filter(n => typeOnly || n.isTypeOnly()).map(n => n.getName()); + if (valueMatched.length > 0) { + // The marker must outlive this pass: the import declaration it + // describes is removed when the path is rerouted, so anchor at the + // first use of one of the helpers (the import line otherwise). + const usageAnchor = + valueMatchedSpecifiers + .map(n => findFirstIdentifierOutsideImports(sourceFile, n.getAliasNode()?.getText() ?? n.getName())) + .find(node => node !== undefined) ?? imp; + diagnostics.push( + actionRequired( + filePath, + usageAnchor, + `${valueMatched.join(', ')}: resource-server auth helpers routed to the frozen ` + + `@modelcontextprotocol/server-legacy/auth copy. The maintained v2 home is ` + + `@modelcontextprotocol/express — when re-pointing, verifiers must throw the v2 OAuthError ` + + `(the express middleware does not recognize the legacy error classes). ` + + `See the migration guide's server auth split section.` + ) + ); + } + if (typeMatched.length > 0) { + diagnostics.push( + info( + filePath, + line, + `${typeMatched.join(', ')}: type-only import routed to @modelcontextprotocol/server-legacy/auth; ` + + `the maintained v2 type lives in @modelcontextprotocol/express — re-point when convenient.` + ) + ); + } + } + if (mapping.status === 'removed') { imp.remove(); changesCount++; @@ -122,7 +164,11 @@ export const importPathsTransform: Transform = { const needsContext = namespaceImport != null || defaultImport != null || - namedImports.some(n => symbolTargetOverride(n.getName(), mapping!) === undefined); + namedImports.some( + n => + mapping!.removedSymbols?.[n.getName()] === undefined && + symbolTargetOverride(n.getName(), mapping!) === undefined + ); if (needsContext) { const base = resolveTypesPackage(context, hasClientImport, hasServerImport, { filePath, line, diagnostics }); targetPackage = mapping.subpathSuffix ? `${base}${mapping.subpathSuffix}` : base; @@ -179,6 +225,36 @@ export const importPathsTransform: Transform = { ); } } + // Qualified accesses to symbols with no v2 export (`ns.Protocol`) can't be fixed by + // moving the namespace binding — flag each accessed one. Expression positions are + // PropertyAccessExpressions; type positions (`let p: ns.Protocol`) are QualifiedNames. + if (mapping.removedSymbols) { + const nsName = namespaceImport.getText(); + const accessedRemoved = new Map(); + for (const pa of sourceFile.getDescendantsOfKind(SyntaxKind.PropertyAccessExpression)) { + const memberName = pa.getName(); + if ( + pa.getExpression().getText() === nsName && + mapping.removedSymbols[memberName] !== undefined && + !accessedRemoved.has(memberName) + ) { + accessedRemoved.set(memberName, pa); + } + } + for (const qn of sourceFile.getDescendantsOfKind(SyntaxKind.QualifiedName)) { + const memberName = qn.getRight().getText(); + if ( + qn.getLeft().getText() === nsName && + mapping.removedSymbols[memberName] !== undefined && + !accessedRemoved.has(memberName) + ) { + accessedRemoved.set(memberName, qn); + } + } + for (const [name, node] of accessedRemoved) { + diagnostics.push(actionRequired(filePath, node, mapping.removedSymbols[name]!)); + } + } usedPackages.add(effectiveTarget); imp.setModuleSpecifier(effectiveTarget); if (mapping.renamedSymbols) { @@ -203,6 +279,18 @@ export const importPathsTransform: Transform = { for (const n of namedImports) { const name = n.getName(); + const removalGuidance = mapping.removedSymbols?.[name]; + if (removalGuidance !== undefined) { + // No v2 package exports this symbol — dropping it (with a marker) beats + // emitting an import of a member the target package does not have. Anchor + // the marker to a usage site: it survives the import rewrites, so the + // runner resolves a live line, and it is where the user must act anyway. + const usageName = n.getAliasNode()?.getText() ?? name; + diagnostics.push( + actionRequired(filePath, findFirstIdentifierOutsideImports(sourceFile, usageName) ?? imp, removalGuidance) + ); + continue; + } const alias = n.getAliasNode()?.getText(); const resolvedName = mapping.renamedSymbols?.[name] ?? name; const specifierTypeOnly = typeOnly || n.isTypeOnly(); @@ -332,6 +420,17 @@ function rewriteExportDeclarations( } } + // A star re-export of a module with removed symbols silently drops them from the + // barrel (the new target never exported them) — flag each, mirroring the + // schema-constant star-export diagnostic below. + if (mapping.removedSymbols && exp.getNamedExports().length === 0) { + for (const [name, guidance] of Object.entries(mapping.removedSymbols)) { + diagnostics.push( + actionRequired(filePath, exp, `Star re-export of ${specifier} will no longer provide ${name}. ${guidance}`) + ); + } + } + if (mapping.symbolTargetOverrides || mapping.schemaSymbolTarget) { const namedExports = exp.getNamedExports(); // A star re-export (`export * from …`, including `export * as ns from …`) has no named @@ -375,7 +474,21 @@ function rewriteExportDeclarations( if (!spec.getAliasNode()) spec.setAlias(name); spec.setName(newName); } - if (REEXPORT_WARNINGS[name]) { + const removalGuidance = mapping.removedSymbols?.[name]; + if (removalGuidance !== undefined) { + diagnostics.push( + actionRequired(filePath, exp, `Re-exported ${name} has no v2 export — remove this re-export. ${removalGuidance}`) + ); + } else if (RS_ONLY_AUTH_SYMBOLS.has(name) && targetPackage === '@modelcontextprotocol/server-legacy/auth') { + diagnostics.push( + actionRequired( + filePath, + exp, + `Re-exported ${name} now points at the frozen @modelcontextprotocol/server-legacy/auth copy; the ` + + `maintained v2 home is @modelcontextprotocol/express. Re-point this barrel entry deliberately.` + ) + ); + } else if (REEXPORT_WARNINGS[name]) { diagnostics.push(actionRequired(filePath, exp, REEXPORT_WARNINGS[name]!)); } } diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/index.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/index.ts index 18121109bf..ca10d9b5f3 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/index.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/index.ts @@ -1,4 +1,5 @@ import type { Transform } from '../../../types'; +import { completableNestingTransform } from './completableNesting'; import { contextTypesTransform } from './contextTypes'; import { handlerRegistrationTransform } from './handlerRegistration'; import { importPathsTransform } from './importPaths'; @@ -29,7 +30,10 @@ import { symbolRenamesTransform } from './symbolRenames'; // 5. handlerRegistration and schemaParamRemoval are independent of each // other but both depend on importPaths having run. // -// 6. mockPaths runs last: handles test mocks and dynamic imports, +// 6. completableNesting runs after importPaths (it matches the rewritten +// completable import) and is independent of the rest. +// +// 7. mockPaths runs last: handles test mocks and dynamic imports, // independent of the other transforms. export const v1ToV2Transforms: Transform[] = [ importPathsTransform, @@ -39,5 +43,6 @@ export const v1ToV2Transforms: Transform[] = [ handlerRegistrationTransform, schemaParamRemovalTransform, contextTypesTransform, + completableNestingTransform, mockPathsTransform ]; diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/mcpServerApi.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/mcpServerApi.ts index 7db21088e1..215ed9ff2c 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/mcpServerApi.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/mcpServerApi.ts @@ -3,7 +3,7 @@ import { Node, SyntaxKind } from 'ts-morph'; import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types'; import { actionRequired, info } from '../../../utils/diagnostics'; -import { isOriginalNameImportedFromMcp, resolveLocalImportName } from '../../../utils/importUtils'; +import { hasMcpImports, isOriginalNameImportedFromMcp, resolveLocalImportName } from '../../../utils/importUtils'; export const mcpServerApiTransform: Transform = { name: 'McpServer API migration', @@ -12,9 +12,15 @@ export const mcpServerApiTransform: Transform = { const diagnostics: Diagnostic[] = []; let changesCount = 0; - if (!isOriginalNameImportedFromMcp(sourceFile, 'McpServer')) { + const hasServerImport = isOriginalNameImportedFromMcp(sourceFile, 'McpServer'); + if (!hasServerImport && !hasMcpImports(sourceFile)) { return { changesCount: 0, diagnostics: [] }; } + // Without a direct McpServer import (harness objects exposing an `mcp` field, + // wrapped servers), legacy-name calls still migrate when their shape provably + // matches the v1 signature; non-matching calls stay silent rather than + // collecting hard markers on receivers whose type the codemod cannot see. + const provableShapesOnly = !hasServerImport; const calls = sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression); @@ -30,101 +36,113 @@ export const mcpServerApiTransform: Transform = { if (!Node.isPropertyAccessExpression(expr)) continue; const methodName = expr.getName(); + const legacyPrequalified = + !provableShapesOnly || + (call.getArguments().length >= 2 && + isStringArg(call.getArguments()[0]!) && + receiverLooksLikeMcpServer(expr.getExpression())); + // The same receiver evidence gates the modern register* names: a file + // with only an MCP type import can carry an unrelated + // `registry.registerTool('x', { inputSchema }, cb)` whose schema must + // not be wrapped. + const registerQualified = !provableShapesOnly || receiverLooksLikeMcpServer(expr.getExpression()); switch (methodName) { case 'tool': { - toolCalls.push(call); + if (legacyPrequalified) toolCalls.push(call); break; } case 'prompt': { - promptCalls.push(call); + if (legacyPrequalified) promptCalls.push(call); break; } case 'resource': { - resourceCalls.push(call); + if (legacyPrequalified) resourceCalls.push(call); break; } case 'registerTool': { - registerToolCalls.push(call); + if (registerQualified) registerToolCalls.push(call); break; } case 'registerPrompt': { - registerPromptCalls.push(call); + if (registerQualified) registerPromptCalls.push(call); break; } case 'registerResource': { - registerResourceCalls.push(call); + if (registerQualified) registerResourceCalls.push(call); break; } } } - for (const call of toolCalls) { - const result = migrateToolCall(call, sourceFile, diagnostics); - if (result) { - changesCount++; - } else { - diagnostics.push( - actionRequired( - sourceFile.getFilePath(), - call, - 'Could not automatically migrate .tool() call. Manual migration required.' - ) - ); - } - } - - for (const call of promptCalls) { - const result = migratePromptCall(call, sourceFile, diagnostics); - if (result) { - changesCount++; - } else { - diagnostics.push( - actionRequired( - sourceFile.getFilePath(), - call, - 'Could not automatically migrate .prompt() call. Manual migration required.' - ) - ); - } - } - - for (const call of resourceCalls) { - const result = migrateResourceCall(call, sourceFile); - if (result) { - changesCount++; - } else { - diagnostics.push( - actionRequired( - sourceFile.getFilePath(), - call, - 'Could not automatically migrate .resource() call. Manual migration required.' - ) - ); - } + // ONE pass in reverse document order across every category: a registration + // nested inside another handler's body must migrate before the enclosing + // call's rewrite replaces the surrounding text and forgets the inner node — + // regardless of which category either call belongs to. + interface PendingCall { + call: CallExpression; + kind: 'tool' | 'prompt' | 'resource' | 'registerTool' | 'registerPrompt' | 'registerResource'; } + const orderedCalls: PendingCall[] = [ + ...toolCalls.map(call => ({ call, kind: 'tool' as const })), + ...promptCalls.map(call => ({ call, kind: 'prompt' as const })), + ...resourceCalls.map(call => ({ call, kind: 'resource' as const })), + ...registerToolCalls.map(call => ({ call, kind: 'registerTool' as const })), + ...registerPromptCalls.map(call => ({ call, kind: 'registerPrompt' as const })), + ...registerResourceCalls.map(call => ({ call, kind: 'registerResource' as const })) + ].toSorted((a, b) => b.call.getPos() - a.call.getPos()); + + const legacyFailure = (call: CallExpression, method: string): void => { + // In fallback mode (no direct McpServer import) the receiver's type is + // unknown — failures stay silent rather than marking non-MCP code. + if (provableShapesOnly) return; + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + call, + `Could not automatically migrate .${method}() call. Manual migration required.` + ) + ); + }; - for (const call of registerToolCalls) { - if (wrapSchemaInConfig(call, 'inputSchema', sourceFile, diagnostics)) { - changesCount++; - } - if (wrapSchemaInConfig(call, 'outputSchema', sourceFile, diagnostics)) { - changesCount++; + for (const { call, kind } of orderedCalls) { + if (call.wasForgotten()) continue; + switch (kind) { + case 'tool': { + if (migrateToolCall(call, sourceFile, diagnostics)) changesCount++; + else legacyFailure(call, 'tool'); + break; + } + case 'prompt': { + if (migratePromptCall(call, sourceFile, diagnostics)) changesCount++; + else legacyFailure(call, 'prompt'); + break; + } + case 'resource': { + if (migrateResourceCall(call, sourceFile)) changesCount++; + else legacyFailure(call, 'resource'); + break; + } + case 'registerTool': { + if (wrapSchemaInConfig(call, 'inputSchema', sourceFile, diagnostics)) changesCount++; + if (!call.wasForgotten() && wrapSchemaInConfig(call, 'outputSchema', sourceFile, diagnostics)) changesCount++; + break; + } + case 'registerPrompt': { + if (wrapSchemaInConfig(call, 'argsSchema', sourceFile, diagnostics)) changesCount++; + break; + } + case 'registerResource': { + if (wrapSchemaInConfig(call, 'uriSchema', sourceFile, diagnostics)) changesCount++; + break; + } } } - for (const call of registerPromptCalls) { - if (wrapSchemaInConfig(call, 'argsSchema', sourceFile, diagnostics)) { - changesCount++; - } - } + flagRemovedTaskOptions(sourceFile, diagnostics); - for (const call of registerResourceCalls) { - if (wrapSchemaInConfig(call, 'uriSchema', sourceFile, diagnostics)) { - changesCount++; - } - } + ensureZodImportForWraps(sourceFile, diagnostics); - flagRemovedTaskOptions(sourceFile, diagnostics); + noteMockShapeAssertions(sourceFile, diagnostics); return { changesCount, diagnostics }; } @@ -141,6 +159,8 @@ function isZodObjectCall(node: Node): boolean { return expr.getName() === 'object' && expr.getExpression().getText() === 'z'; } +const WRAP_NOTE = 'wrapped with z.object().'; + function wrapWithZObject(schemaText: string): string { return `z.object(${schemaText})`; } @@ -155,21 +175,17 @@ function maybeWrapSchema(node: Node): string { function emitWrapDiagnostic(node: Node, sourceFile: SourceFile, call: CallExpression, diagnostics: Diagnostic[]): void { if (Node.isObjectLiteralExpression(node)) { - diagnostics.push( - info( - sourceFile.getFilePath(), - call.getStartLineNumber(), - 'Raw object literal wrapped with z.object(). Verify that zod (z) is imported in this file.' - ) - ); + diagnostics.push(info(sourceFile.getFilePath(), call.getStartLineNumber(), `Raw object literal ${WRAP_NOTE}`)); } else if (!isZodObjectCall(node)) { - diagnostics.push( - actionRequired( + diagnostics.push({ + ...actionRequired( sourceFile.getFilePath(), call, - 'Schema argument is not an object literal — verify it is a z.object() schema. V2 requires a Zod schema, not a raw object.' - ) - ); + 'Could not verify the schema argument is a schema object. Raw shapes are deprecated in v2 — ' + + 'pass a Standard Schema object (e.g. z.object({ … })); no change is needed if it already is one.' + ), + advisoryOnly: true + }); } } @@ -196,13 +212,15 @@ function wrapSchemaInConfig(call: CallExpression, schemaPropertyName: string, so if (!schemaProp) return false; if (Node.isShorthandPropertyAssignment(schemaProp)) { - diagnostics.push( - actionRequired( + diagnostics.push({ + ...actionRequired( sourceFile.getFilePath(), call, - `Shorthand \`{ ${schemaPropertyName} }\` in config: verify the value is a z.object() schema, not a raw object. V2 requires a Zod schema.` - ) - ); + `Shorthand \`{ ${schemaPropertyName} }\` in config: could not verify the value is a schema object. Raw shapes ` + + `are deprecated in v2 — pass a Standard Schema object (e.g. z.object({ … })).` + ), + advisoryOnly: true + }); return false; } @@ -215,23 +233,21 @@ function wrapSchemaInConfig(call: CallExpression, schemaPropertyName: string, so const wrapped = wrapWithZObject(initializer.getText()); initializer.replaceWithText(wrapped); diagnostics.push( - info( - sourceFile.getFilePath(), - call.getStartLineNumber(), - `Raw object literal in ${schemaPropertyName} wrapped with z.object(). Verify that zod (z) is imported in this file.` - ) + info(sourceFile.getFilePath(), call.getStartLineNumber(), `Raw object literal in ${schemaPropertyName} ${WRAP_NOTE}`) ); return true; } if (!isZodObjectCall(initializer)) { - diagnostics.push( - actionRequired( + diagnostics.push({ + ...actionRequired( sourceFile.getFilePath(), call, - `\`${schemaPropertyName}\` value is not an object literal — verify it is a z.object() schema. V2 requires a Zod schema, not a raw object.` - ) - ); + `Could not verify \`${schemaPropertyName}\` is a schema object. Raw shapes are deprecated in v2 — ` + + `pass a Standard Schema object (e.g. z.object({ … })); no change is needed if it already is one.` + ), + advisoryOnly: true + }); } return false; } @@ -452,3 +468,123 @@ function flagRemovedTaskOptions(sourceFile: SourceFile, diagnostics: Diagnostic[ } } } + +/** + * Wrapping a raw shape introduces a `z.object(...)` reference. When the file has no + * `z` binding, add `import { z } from 'zod'` so the rewrite does not leave a dangling + * identifier; the package must also declare zod (the manifest summary warns when its + * declared range cannot satisfy v2's >=4.2 floor). + */ +function ensureZodImportForWraps(sourceFile: SourceFile, diagnostics: Diagnostic[]): void { + const wrapped = diagnostics.some(d => d.message.includes(WRAP_NOTE)); + if (!wrapped) return; + // A value import named `z` (type-only imports are erased and cannot back a + // runtime z.object() call). + const zValueImport = sourceFile.getImportDeclarations().some(decl => { + if (decl.isTypeOnly()) return false; + if (decl.getNamespaceImport()?.getText() === 'z') return true; + if (decl.getDefaultImport()?.getText() === 'z') return true; + return decl.getNamedImports().some(ni => !ni.isTypeOnly() && (ni.getAliasNode()?.getText() ?? ni.getName()) === 'z'); + }); + if (zValueImport) return; + // `z` bound some other way (a variable, or a destructured require) — adding an + // import would redeclare it, and the existing binding may not be zod at all. + const zOtherBinding = + sourceFile.getVariableDeclaration('z') !== undefined || + sourceFile.getDescendantsOfKind(SyntaxKind.BindingElement).some(be => be.getName() === 'z'); + if (zOtherBinding) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + sourceFile.getImportDeclarations()[0] ?? sourceFile, + `Raw shapes were ${WRAP_NOTE.slice(0, -1)}, but \`z\` in this file is not a value import from zod — ` + + 'wire the wrapped schemas to your zod instance manually (zod >=4.2.0 satisfies v2).' + ) + ); + return; + } + sourceFile.addImportDeclaration({ moduleSpecifier: 'zod', namedImports: ['z'] }); + diagnostics.push({ + ...info( + sourceFile.getFilePath(), + 1, + "Added `import { z } from 'zod'` for the wrapped raw shapes. The owning package must declare zod " + + '(>=4.2.0 satisfies v2) — the manifest summary adds or warns accordingly.' + ), + tag: 'zod-injected' + }); +} + +const SCHEMA_CONFIG_KEYS = new Set(['inputSchema', 'outputSchema', 'argsSchema', 'uriSchema']); + +/** + * `expect.objectContaining({ inputSchema: { … } })`-style call-shape assertions pin + * the v1 registration config; after migration the schemas are wrapped (z.object) and + * the literal shape no longer matches. The codemod cannot rewrite the assertion — + * note it. + */ +function noteMockShapeAssertions(sourceFile: SourceFile, diagnostics: Diagnostic[]): void { + for (const call of sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression)) { + if (call.wasForgotten()) continue; + const callee = call.getExpression(); + if (!Node.isPropertyAccessExpression(callee) || callee.getName() !== 'objectContaining') continue; + const arg = call.getArguments()[0]; + if (arg === undefined || !Node.isObjectLiteralExpression(arg)) continue; + const schemaProp = arg.getProperties().find(prop => { + const name = Node.isPropertyAssignment(prop) || Node.isShorthandPropertyAssignment(prop) ? prop.getName() : undefined; + return name !== undefined && SCHEMA_CONFIG_KEYS.has(name); + }); + if (schemaProp === undefined) continue; + diagnostics.push({ + ...actionRequired( + sourceFile.getFilePath(), + call, + `Call-shape assertion pins a registration config schema — v2 configs carry wrapped schema objects ` + + `(z.object({ … })), so a raw-shape literal no longer matches. Assert with the wrapped schema or ` + + `expect.any(Object).` + ), + advisoryOnly: true + }); + } +} + +/** + * Without a direct `McpServer` import the receiver's type is unknown, so the only + * safe rewrites are calls whose receiver itself is named like an MCP server + * (`server.tool(…)`, `harness.mcp.tool(…)`, `this.mockServer.prompt(…)`). The check + * is strictly on the TERMINAL name of the receiver chain: a file that merely imports + * an MCP type can contain shape-identical non-MCP calls — `cli.prompt('q', cb)`, + * `app.resource('users', '/u', handler)`, and members hanging off a server such as + * `this.server.cli.prompt(…)` — which must not be touched. + */ +function receiverLooksLikeMcpServer(receiver: Node): boolean { + const unwrapped = unwrapReceiver(receiver); + if (Node.isIdentifier(unwrapped)) return nameHasMcpServerWord(unwrapped.getText()); + if (Node.isPropertyAccessExpression(unwrapped)) return nameHasMcpServerWord(unwrapped.getName()); + return false; +} + +/** Strip wrappers that do not change which object the method is called on. */ +function unwrapReceiver(node: Node): Node { + let current = node; + for (;;) { + if ( + Node.isParenthesizedExpression(current) || + Node.isNonNullExpression(current) || + Node.isAsExpression(current) || + Node.isSatisfiesExpression(current) + ) { + current = current.getExpression(); + continue; + } + return current; + } +} + +/** True when one of the identifier's camelCase / snake_case words is `mcp` or `server`. */ +function nameHasMcpServerWord(name: string): boolean { + return name + .split(/[^a-zA-Z0-9]+|(?=[A-Z])/) + .filter(Boolean) + .some(word => /^(mcp|server)$/i.test(word)); +} diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/mockPaths.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/mockPaths.ts index 7ffdd53887..bf217a936f 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/mockPaths.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/mockPaths.ts @@ -7,7 +7,7 @@ import { isSdkSpecifier } from '../../../utils/importUtils'; import { resolveTypesPackage } from '../../../utils/projectAnalyzer'; import type { ImportMapping } from '../mappings/importMap'; import { isAuthImport, lookupImportMapping } from '../mappings/importMap'; -import { isSharedSchemaConst, resolveRenamedName, symbolTargetOverride } from '../mappings/schemaRouting'; +import { isSharedSchemaConst, removedSymbolGuidance, resolveRenamedName, symbolTargetOverride } from '../mappings/schemaRouting'; import { SIMPLE_RENAMES } from '../mappings/symbolMap'; /** @@ -28,7 +28,7 @@ function routeSymbols(symbols: string[], mapping: ImportMapping): { target?: str return { mixed: false }; } -const MOCK_METHODS = new Set([ +export const MOCK_METHODS: ReadonlySet = new Set([ 'mock', 'doMock', 'unmock', @@ -39,7 +39,7 @@ const MOCK_METHODS = new Set([ 'requireMock', 'createMockFromModule' ]); -const MOCK_CALLERS = new Set(['vi', 'jest']); +export const MOCK_CALLERS: ReadonlySet = new Set(['vi', 'jest']); export const mockPathsTransform: Transform = { name: 'Mock and dynamic import path rewrites', @@ -152,6 +152,20 @@ function rewriteMockCall( return 0; } + const removedHits = factorySymbols.filter(sym => removedSymbolGuidance(sym, resolved.mapping) !== undefined); + if (removedHits.length > 0) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + call, + `Mock factory from ${specifier} provides ${removedHits.join(', ')}, which no v2 package exports — rewriting the ` + + `specifier would mock a module that never had the member. Restructure the test instead. ` + + removedHits.map(sym => removedSymbolGuidance(sym, resolved.mapping)!).join(' ') + ) + ); + return 0; + } + let changes = 0; let effectiveTarget = resolved.target; @@ -398,6 +412,20 @@ function rewriteDynamicImports( return; } + const removedHits = destructuredKeys.filter(sym => removedSymbolGuidance(sym, resolved.mapping) !== undefined); + if (removedHits.length > 0) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + node, + `Dynamic import of ${specifier} destructures ${removedHits.join(', ')}, which no v2 package exports — the ` + + `binding would be undefined at runtime. ` + + removedHits.map(sym => removedSymbolGuidance(sym, resolved.mapping)!).join(' ') + ) + ); + return; + } + let effectiveTarget = resolved.target; const allRenames: Record = { ...SIMPLE_RENAMES, ...resolved.mapping.renamedSymbols }; diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/removedApis.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/removedApis.ts index 2fc2573fed..ff8fb15b21 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/removedApis.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/removedApis.ts @@ -3,8 +3,39 @@ import { Node, SyntaxKind } from 'ts-morph'; import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types'; import { renameAllReferences } from '../../../utils/astUtils'; -import { warning } from '../../../utils/diagnostics'; -import { addOrMergeImport, isAnyMcpSpecifier } from '../../../utils/importUtils'; +import { actionRequired, info, warning } from '../../../utils/diagnostics'; +import { addOrMergeImport, hasMcpImports, isAnyMcpSpecifier } from '../../../utils/importUtils'; + +/** + * v2 `finishAuth(code, iss?)` verifies the callback's `iss` when the authorization + * server advertises `authorization_response_iss_parameter_supported` — and the v2 + * server-legacy router advertises it by default. A bare `finishAuth(code)` stays + * type-correct (the parameter is optional) but the verification then has no input, + * so single-argument call sites in files this run changes get a run-log note + * pointing at the guide (not a marker: the one-argument URLSearchParams form is the + * blessed v2 spelling and is statically indistinguishable from the code-string + * form, so the note is advisory-only and stays quiet on already-migrated trees). + */ +function handleFinishAuthAdvisory(sourceFile: SourceFile, diagnostics: Diagnostic[]): void { + if (!hasMcpImports(sourceFile)) return; + for (const call of sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression)) { + const expr = call.getExpression(); + if (!Node.isPropertyAccessExpression(expr) || expr.getName() !== 'finishAuth') continue; + if (call.getArguments().length !== 1) continue; + diagnostics.push({ + ...warning( + sourceFile.getFilePath(), + call.getStartLineNumber(), + 'finishAuth with one argument: if this passes the authorization code string, v2 also accepts ' + + 'finishAuth(new URL(callbackUrl).searchParams) — preferred, since it carries the iss the callback check ' + + 'reads when the authorization server advertises authorization_response_iss_parameter_supported ' + + '(the v2 server-legacy router advertises it by default). A call already passing URLSearchParams needs ' + + "no change. See the migration guide's authorization-server mix-up defense section." + ), + advisoryOnly: true + }); + } +} const REMOVED_ZOD_HELPERS: Record = { schemaToJson: @@ -29,6 +60,7 @@ export const removedApisTransform: Transform = { changesCount += handleRemovedZodHelpers(sourceFile, diagnostics); changesCount += handleIsomorphicHeaders(sourceFile, diagnostics); changesCount += handleStreamableHTTPError(sourceFile, diagnostics); + handleFinishAuthAdvisory(sourceFile, diagnostics); return { changesCount, diagnostics }; } @@ -145,9 +177,9 @@ function handleStreamableHTTPError(sourceFile: SourceFile, diagnostics: Diagnost if (!Node.isIdentifier(expr) || expr.getText() !== localName) continue; hasConstructorCalls = true; diagnostics.push( - warning( + actionRequired( sourceFile.getFilePath(), - node.getStartLineNumber(), + node, 'new StreamableHTTPError(statusCode, statusText, body?) → new SdkHttpError(code, message, data). ' + 'Constructor arguments differ — manual review required. Map the HTTP status to a SdkErrorCode enum value ' + 'and pass the HTTP status via the data argument, e.g. { status, statusText }.' @@ -158,6 +190,8 @@ function handleStreamableHTTPError(sourceFile: SourceFile, diagnostics: Diagnost renameAllReferences(sourceFile, localName, 'SdkHttpError'); changesCount++; + changesCount += rewriteGuardedStatusReads(sourceFile, diagnostics); + foundImport.remove(); if (foundImportDecl.getNamedImports().length === 0 && !foundImportDecl.getDefaultImport() && !foundImportDecl.getNamespaceImport()) { foundImportDecl.remove(); @@ -193,3 +227,110 @@ function resolveTargetModule(sourceFile: SourceFile, originalModule: string): st if (originalModule.includes('/client')) return '@modelcontextprotocol/client'; return '@modelcontextprotocol/server'; } + +/** + * Climb only through positive conjunctions (`&&`) and parentheses: under `||` or `!` + * the other operand evaluates exactly when the instanceof check FAILED, so it is not + * a guarded scope. + */ +function conjunctionRootOf(node: Node): Node { + let top: Node = node; + let parent = top.getParent(); + while (parent !== undefined) { + if (Node.isParenthesizedExpression(parent)) { + top = parent; + } else if (Node.isBinaryExpression(parent) && parent.getOperatorToken().getKind() === SyntaxKind.AmpersandAmpersandToken) { + top = parent; + } else { + break; + } + parent = top.getParent(); + } + return top; +} + +/** True when `name` is rebound by a function parameter between `node` and `scopeRoot`. */ +function isShadowedWithin(node: Node, scopeRoot: Node, name: string): boolean { + let current = node.getParent(); + while (current !== undefined && current !== scopeRoot) { + if ( + (Node.isArrowFunction(current) || + Node.isFunctionExpression(current) || + Node.isFunctionDeclaration(current) || + Node.isMethodDeclaration(current)) && + current.getParameters().some(param => param.getName() === name) + ) { + return true; + } + current = current.getParent(); + } + return false; +} + +/** True when the scope contains any assignment whose left side is `subject`. */ +function subjectReassignedWithin(scope: Node, subject: string): boolean { + return scope.getDescendantsOfKind(SyntaxKind.BinaryExpression).some(bin => { + const op = bin.getOperatorToken().getKind(); + return ( + (op === SyntaxKind.EqualsToken || op === SyntaxKind.QuestionQuestionEqualsToken || op === SyntaxKind.BarBarEqualsToken) && + bin.getLeft().getText() === subject + ); + }); +} + +/** + * v2 carries the HTTP status on `.status`; `.code` is an `SdkErrorCode` string. A + * `.code` read on a value the surrounding check proves is an `SdkHttpError` — + * positive-conjunction siblings of the check, or reads inside the positively guarded + * if-block — rewrites mechanically. Unprovable reads keep the existing file-level + * warning. + */ +function rewriteGuardedStatusReads(sourceFile: SourceFile, diagnostics: Diagnostic[]): number { + let changes = 0; + for (const guard of sourceFile.getDescendantsOfKind(SyntaxKind.BinaryExpression)) { + if (guard.wasForgotten()) continue; + if (guard.getOperatorToken().getKind() !== SyntaxKind.InstanceOfKeyword) continue; + const right = guard.getRight(); + if (!Node.isIdentifier(right) || right.getText() !== 'SdkHttpError') continue; + const subject = guard.getLeft().getText(); + + const conjunctionRoot = conjunctionRootOf(guard); + const scopes: Node[] = [conjunctionRoot]; + // The then-block is guarded only when the check reaches the if condition + // through positive conjunctions alone (no `!`, no `||` on the path). + const ifStmt = guard.getFirstAncestorByKind(SyntaxKind.IfStatement); + if (ifStmt !== undefined && conjunctionRoot === ifStmt.getExpression()) { + const thenBlock = ifStmt.getThenStatement(); + if (!subjectReassignedWithin(thenBlock, subject)) scopes.push(thenBlock); + } + + let rewrote = false; + for (const scope of scopes) { + const reads = scope + .getDescendantsOfKind(SyntaxKind.PropertyAccessExpression) + .filter( + pa => + !pa.wasForgotten() && + pa.getName() === 'code' && + pa.getExpression().getText() === subject && + !isShadowedWithin(pa, scope, subject) + ); + for (const pa of reads.toReversed()) { + pa.getNameNode().replaceWithText('status'); + changes++; + rewrote = true; + } + } + if (rewrote) { + diagnostics.push( + info( + sourceFile.getFilePath(), + guard.getStartLineNumber(), + `Rewrote ${subject}.code to ${subject}.status under the instanceof SdkHttpError check — v2 carries ` + + `the HTTP status on .status (.code is an SdkErrorCode string).` + ) + ); + } + } + return changes; +} diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts index c8fd439eb3..f2688e8fd0 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts @@ -69,10 +69,32 @@ export const schemaParamRemovalTransform: Transform = { // argument too many (TS2554). Drop it only when a third argument follows — a 2-arg // `callTool(params, undefined)` already type-checks, since `undefined` is a valid options arg. if (secondArg.getText() === 'undefined') { - if (fileHasMcpImports && args.length >= 3) { - call.removeArgument(1); - changesCount++; + if (!fileHasMcpImports || args.length < 3) continue; + const firstArg = args[0]!; + if (methodName === 'request') { + // Drop only when the method is provably a literal spec method (the same + // proof the schema-identifier path requires). When the skip leaves an + // explicit undefined behind, the result is a LOUD compile error the user + // resolves with the call's intent in view — strictly better than silently + // changing semantics. The proof also keeps the rewrite off non-SDK + // receivers that happen to be named `request` with a bare-string first + // argument, where deleting the middle argument corrupts the call. + const literal = literalMethodOf(firstArg); + if (literal === undefined || !SPEC_REQUEST_METHODS.has(literal)) continue; + } else if ( + Node.isStringLiteral(firstArg) || + Node.isNoSubstitutionTemplateLiteral(firstArg) || + Node.isTemplateExpression(firstArg) || + Node.isNumericLiteral(firstArg) || + firstArg.getKind() === SyntaxKind.TrueKeyword || + firstArg.getKind() === SyntaxKind.FalseKeyword + ) { + // v1 callTool() takes a params OBJECT first — a primitive first argument + // means a non-SDK receiver sharing the method name. + continue; } + call.removeArgument(1); + changesCount++; continue; } diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/symbolRenames.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/symbolRenames.ts index 4e47c1ee68..ca594a352e 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/symbolRenames.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/symbolRenames.ts @@ -1,10 +1,10 @@ import type { SourceFile } from 'ts-morph'; -import { Node } from 'ts-morph'; +import { Node, SyntaxKind } from 'ts-morph'; import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types'; -import { renameAllReferences } from '../../../utils/astUtils'; -import { info, warning } from '../../../utils/diagnostics'; -import { addOrMergeImport, isAnyMcpSpecifier, removeUnusedImport } from '../../../utils/importUtils'; +import { findFirstIdentifierOutsideImports, renameAllReferences } from '../../../utils/astUtils'; +import { actionRequired, info, warning } from '../../../utils/diagnostics'; +import { addOrMergeImport, isAnyMcpSpecifier, isV2Specifier, removeUnusedImport } from '../../../utils/importUtils'; import { resolveTypesPackage } from '../../../utils/projectAnalyzer'; import { ERROR_CODE_SDK_MEMBERS, SIMPLE_RENAMES } from '../mappings/symbolMap'; @@ -36,6 +36,7 @@ export const symbolRenamesTransform: Transform = { } } + changesCount += renameDynamicImportBindings(sourceFile); changesCount += handleErrorCodeSplit(sourceFile, diagnostics); changesCount += handleRequestHandlerExtra(sourceFile, context, diagnostics); changesCount += handleSchemaInput(sourceFile, context, diagnostics); @@ -67,6 +68,18 @@ function handleErrorCodeSplit(sourceFile: SourceFile, diagnostics: Diagnostic[]) let needsProtocolErrorCode = false; let needsSdkErrorCode = false; + let needsSdkError = false; + + // First pass: rewrite the enum sides and remember which boolean expressions hold + // SDK-member comparisons, protocol-member comparisons, or both. The instanceof + // pairing decision needs the WHOLE expression's membership, so it runs after. + interface BooleanExprInfo { + hasSdkMember: boolean; + hasProtocolMember: boolean; + } + const booleanExprs = new Map(); + const ctorExprs = new Map(); + const matcherSubjects = new Map>(); sourceFile.forEachDescendant(node => { if (!Node.isPropertyAccessExpression(node)) return; @@ -74,17 +87,212 @@ function handleErrorCodeSplit(sourceFile: SourceFile, diagnostics: Diagnostic[]) if (!Node.isIdentifier(expr) || expr.getText() !== errorCodeLocalName) return; const member = node.getName(); - if (ERROR_CODE_SDK_MEMBERS.has(member)) { - needsSdkErrorCode = true; - node.getExpression().replaceWithText('SdkErrorCode'); - } else { - needsProtocolErrorCode = true; - node.getExpression().replaceWithText('ProtocolErrorCode'); - } + const isSdkMember = ERROR_CODE_SDK_MEMBERS.has(member); + node.getExpression().replaceWithText(isSdkMember ? 'SdkErrorCode' : 'ProtocolErrorCode'); + if (isSdkMember) needsSdkErrorCode = true; + else needsProtocolErrorCode = true; changesCount++; + + if (isInComparisonContext(node)) { + const root = booleanExpressionRoot(node); + const entry = booleanExprs.get(root) ?? { hasSdkMember: false, hasProtocolMember: false }; + if (isSdkMember) entry.hasSdkMember = true; + else entry.hasProtocolMember = true; + booleanExprs.set(root, entry); + } + + // Constructor pairing: `new ProtocolError(ErrorCode.RequestTimeout, …)` — the + // SDK-routed codes ride on SdkError, so the class must move with the member. + // Only the FIRST argument (the code) participates, and per-constructor + // membership is tracked so a ternary mixing both enums gets a marker rather + // than a wrong class. + { + const newExpr = node.getFirstAncestorByKind(SyntaxKind.NewExpression); + const classExpr = newExpr?.getExpression(); + const firstArg = newExpr?.getArguments()[0]; + if ( + newExpr !== undefined && + classExpr !== undefined && + firstArg !== undefined && + Node.isIdentifier(classExpr) && + (classExpr.getText() === 'ProtocolError' || classExpr.getText() === 'McpError') && + (node === firstArg || node.getAncestors().includes(firstArg)) + ) { + const entry = ctorExprs.get(newExpr) ?? { hasSdkMember: false, hasProtocolMember: false }; + if (isSdkMember) entry.hasSdkMember = true; + else entry.hasProtocolMember = true; + ctorExprs.set(newExpr, entry); + } + } + + // Matcher pairing: `expect(err.code).toBe(ErrorCode.X)` assertions correlate — + // by asserted subject within the enclosing block — with + // `expect(err).toBeInstanceOf(…)` class assertions handled below. + const subject = matcherSubjectOf(node); + if (subject !== undefined) { + const block: Node = node.getFirstAncestorByKind(SyntaxKind.Block) ?? sourceFile; + let bySubject = matcherSubjects.get(block); + if (!bySubject) { + bySubject = new Map(); + matcherSubjects.set(block, bySubject); + } + const entry = bySubject.get(subject) ?? { hasSdkMember: false, hasProtocolMember: false }; + if (isSdkMember) entry.hasSdkMember = true; + else entry.hasProtocolMember = true; + bySubject.set(subject, entry); + } }); - if (changesCount > 0) { + for (const [newExpr, entry] of ctorExprs) { + if (!entry.hasSdkMember) continue; + if (entry.hasProtocolMember) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + newExpr, + `This constructor's code argument mixes SdkErrorCode and ProtocolErrorCode members, but v2 raises ` + + `them on different error classes (SdkError and ProtocolError). Split the construction per class.` + ) + ); + } else { + newExpr.getExpression().replaceWithText('SdkError'); + needsSdkError = true; + } + } + + // `toBeInstanceOf(ProtocolError)` assertions whose subject is also asserted to + // carry an SDK-routed code in the same block follow the same rule as instanceof + // guards: all-SDK rewrites the class, mixed gets a marker. + { + let unpairedMatcherSites = 0; + for (const call of sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression)) { + const callee = call.getExpression(); + if (!Node.isPropertyAccessExpression(callee) || callee.getName() !== 'toBeInstanceOf') continue; + const classArg = call.getArguments()[0]; + if (classArg === undefined || !Node.isIdentifier(classArg)) continue; + const className = classArg.getText(); + if (className !== 'ProtocolError' && className !== 'McpError') continue; + const subject = instanceofSubjectOf(callee.getExpression()); + const block: Node = call.getFirstAncestorByKind(SyntaxKind.Block) ?? sourceFile; + const entry = subject === undefined ? undefined : matcherSubjects.get(block)?.get(subject); + if (!entry || !entry.hasSdkMember) { + if (needsSdkErrorCode) unpairedMatcherSites++; + continue; + } + if (entry.hasProtocolMember) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + call, + `This subject is asserted with both SdkErrorCode and ProtocolErrorCode members, but v2 raises them ` + + `on different error classes — one toBeInstanceOf cannot cover both. Split the assertions per class.` + ) + ); + } else { + classArg.replaceWithText('SdkError'); + needsSdkError = true; + if (subject !== undefined) { + const fnAncestor = call.getFirstAncestor( + a => + Node.isArrowFunction(a) || + Node.isFunctionExpression(a) || + Node.isFunctionDeclaration(a) || + Node.isMethodDeclaration(a) + ); + const fnBody = + fnAncestor !== undefined && + (Node.isArrowFunction(fnAncestor) || + Node.isFunctionExpression(fnAncestor) || + Node.isFunctionDeclaration(fnAncestor) || + Node.isMethodDeclaration(fnAncestor)) + ? fnAncestor.getBody() + : undefined; + repointSubjectCasts(fnBody ?? block, subject); + } + } + } + // Codes the codemod routed to SdkErrorCode pair with SdkError; class matchers it + // could not correlate (cast/aliased subjects, toMatchObject shapes) may still + // name the wrong class — surface a run-log note, not a marker. + if (unpairedMatcherSites > 0 && needsSdkErrorCode) { + diagnostics.push( + warning( + sourceFile.getFilePath(), + 1, + `This file routes ErrorCode members to SdkErrorCode and also asserts error classes with ` + + `toBeInstanceOf — review those assertions: SDK-routed codes are raised on SdkError, not ProtocolError.` + ) + ); + } + } + + // Second pass: v2 raises the SDK codes on SdkError, not ProtocolError, so a + // guard of `instanceof ProtocolError` paired with an SDK-code comparison never + // matches. When the expression compares ONLY SDK codes, the guard rewrites + // mechanically; when it mixes SDK and protocol codes, one guard cannot cover + // both classes — mark it. Expressions with no instanceof guard are left alone + // (the enclosing guard, if any, may be elsewhere and already correct). + for (const [root, entry] of booleanExprs) { + if (!entry.hasSdkMember) continue; + for (const guard of instanceofGuards(root)) { + if (entry.hasProtocolMember) { + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + guard, + `This check compares both SdkErrorCode and ProtocolErrorCode members, but v2 raises them on ` + + `different error classes (SdkError and ProtocolError) — one instanceof guard cannot cover both. ` + + `Split the check per error class.` + ) + ); + } else if (guardPinnedToSdkCode(guard)) { + guard.getRight().replaceWithText('SdkError'); + needsSdkError = true; + } else { + // `e instanceof X || e.code === SdkErrorCode.Y`, `e instanceof X && + // e.code !== …`, a comparison on another subject: v1's class carried + // BOTH code families, so no single v2 class reproduces this check. + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + guard, + `In v1 this class carried both the protocol codes and the SDK-local codes; in v2 they split into ` + + `ProtocolError and SdkError. The codes compared here route to SdkErrorCode, but this check's shape ` + + `does not pin the guarded value to one class — match SdkError for SDK-raised codes, ProtocolError ` + + `for wire errors, or both.` + ) + ); + } + } + } + + if (changesCount === 0) { + const decl = errorCodeImport.getImportDeclaration(); + // Only a v2 specifier warrants the drop: none of the v2 packages export + // `ErrorCode`, so leaving the named import behind fails at module link time. + // A still-v1 specifier (isolated `--transforms symbols` run) is valid as-is — + // the imports transform handles it when it runs. + if (!isV2Specifier(decl.getModuleSpecifierValue())) return 0; + // Drop it and mark the first remaining use (re-export-only files are handled + // by the export rewrite's own warning). + const usage = findFirstIdentifierOutsideImports(sourceFile, errorCodeLocalName); + diagnostics.push( + actionRequired( + sourceFile.getFilePath(), + usage ?? decl, + `${errorCodeLocalName} is not exported by the v2 packages — it split into ProtocolErrorCode (protocol codes) ` + + `and SdkErrorCode (local SDK codes). No direct member access could be rewritten here; replace the remaining ` + + `uses with the appropriate enum.` + ) + ); + errorCodeImport.remove(); + if (decl.getNamedImports().length === 0 && !decl.getDefaultImport() && !decl.getNamespaceImport()) { + decl.remove(); + } + return 1; + } + + { const errorCodeImportDecl = errorCodeImport.getImportDeclaration(); // Capture target module before removing the import, so we don't lose the original // module specifier when ErrorCode was the only named import in the declaration. @@ -112,6 +320,7 @@ function handleErrorCodeSplit(sourceFile: SourceFile, diagnostics: Diagnostic[]) const newImports: string[] = []; if (needsProtocolErrorCode) newImports.push('ProtocolErrorCode'); if (needsSdkErrorCode) newImports.push('SdkErrorCode'); + if (needsSdkError) newImports.push('SdkError'); if (newImports.length > 0) { const existingImp = sourceFile @@ -140,9 +349,195 @@ function handleErrorCodeSplit(sourceFile: SourceFile, diagnostics: Diagnostic[]) ); } + if (needsSdkError) { + // Class rewrites can leave the original error-class import referenced nowhere. + removeUnusedImport(sourceFile, 'ProtocolError', true); + removeUnusedImport(sourceFile, 'McpError', true); + } + return changesCount; } +const MATCHER_EQUALITY_NAMES = new Set(['toBe', 'toEqual', 'toStrictEqual']); + +/** Strip parentheses and `as`/`satisfies` casts. */ +function unwrapCasts(node: Node): Node { + let current = node; + while (Node.isParenthesizedExpression(current) || Node.isAsExpression(current) || Node.isSatisfiesExpression(current)) { + current = current.getExpression(); + } + return current; +} + +/** + * For `expect(x.code).toBe()`-style assertions where `memberNode` is the + * matcher argument, the asserted subject (`x`) — undefined when the member is not in + * that shape. Only a `.code` / `['code']` property read pairs (that is the premise of + * the class/code correlation); casts around the base are stripped so + * `expect((err as any).code)` pairs with `expect(err)`. + */ +function matcherSubjectOf(memberNode: Node): string | undefined { + const parent = memberNode.getParent(); + if (parent === undefined || !Node.isCallExpression(parent)) return undefined; + if (!parent.getArguments().includes(memberNode)) return undefined; + const callee = parent.getExpression(); + if (!Node.isPropertyAccessExpression(callee) || !MATCHER_EQUALITY_NAMES.has(callee.getName())) return undefined; + const expectArg = expectArgumentOf(callee.getExpression()); + if (expectArg === undefined) return undefined; + const arg = unwrapCasts(expectArg); + if (Node.isPropertyAccessExpression(arg) && arg.getName() === 'code') { + return unwrapCasts(arg.getExpression()).getText(); + } + if (Node.isElementAccessExpression(arg)) { + const key = arg.getArgumentExpression(); + if (key !== undefined && Node.isStringLiteral(key) && key.getLiteralValue() === 'code') { + return unwrapCasts(arg.getExpression()).getText(); + } + } + return undefined; +} + +/** + * The subject a `toBeInstanceOf` assertion is about: the full (cast-stripped) text of + * the expect argument — `expect(err.cause)` asserts `err.cause`, not `err`. + */ +function instanceofSubjectOf(expr: Node): string | undefined { + const arg = expectArgumentOf(expr); + if (arg === undefined) return undefined; + return unwrapCasts(arg).getText(); +} + +/** Walk an expect-chain receiver (`expect(x)`, `expect(x).rejects`, …) to the expect call's first argument. */ +function expectArgumentOf(expr: Node): Node | undefined { + let current: Node | undefined = expr; + while (current !== undefined) { + if (Node.isCallExpression(current)) { + const callee = current.getExpression(); + if (Node.isIdentifier(callee) && callee.getText() === 'expect') { + return current.getArguments()[0]; + } + current = callee; + continue; + } + if (Node.isPropertyAccessExpression(current)) { + current = current.getExpression(); + continue; + } + return undefined; + } + return undefined; +} + +const EQUALITY_OPERATORS = new Set([ + SyntaxKind.EqualsEqualsToken, + SyntaxKind.EqualsEqualsEqualsToken, + SyntaxKind.ExclamationEqualsToken, + SyntaxKind.ExclamationEqualsEqualsToken +]); + +/** True when the member access is one side of an equality comparison or a switch case expression. */ +function isInComparisonContext(node: Node): boolean { + const parent = node.getParent(); + if (parent !== undefined && Node.isBinaryExpression(parent) && EQUALITY_OPERATORS.has(parent.getOperatorToken().getKind())) { + return true; + } + return parent !== undefined && Node.isCaseClause(parent); +} + +const LOGICAL_OPERATORS = new Set([SyntaxKind.AmpersandAmpersandToken, SyntaxKind.BarBarToken]); + +/** + * The outermost expression reachable from `node` through logical operators (&&/||), + * parentheses, and `!` only — assignments, comma sequences, and arithmetic stop the + * climb so a guard stored or reused elsewhere is never claimed. + */ +function booleanExpressionRoot(node: Node): Node { + let top: Node = node; + let parent = top.getParent(); + while (parent !== undefined) { + if (Node.isParenthesizedExpression(parent) || Node.isPrefixUnaryExpression(parent)) { + top = parent; + } else if (Node.isBinaryExpression(parent)) { + const op = parent.getOperatorToken().getKind(); + if (!LOGICAL_OPERATORS.has(op) && !EQUALITY_OPERATORS.has(op)) break; + top = parent; + } else { + break; + } + parent = top.getParent(); + } + return top; +} + +/** + * Whether the class an `instanceof` guard names is pinned by a conjoined SDK-routed + * code comparison ON THE SAME SUBJECT: `e instanceof X && e.code === + * SdkErrorCode.RequestTimeout`, including a negated guard (`!(e instanceof X) && …` + * excludes exactly the SDK-raised codes, which ride on SdkError in v2) and a + * disjunction of same-subject SDK codes (`e instanceof X && (e.code === A || e.code + * === B)`). A code reached only through `||` with the guard, a negated comparison + * (`!==`), another subject's code, or a nested function proves nothing about this + * guard's class. + */ +function guardPinnedToSdkCode(guard: import('ts-morph').BinaryExpression): boolean { + const subject = unwrapCasts(guard.getLeft()).getText(); + + // Conjunction scope of the guard: parentheses, negation of the guard itself, `&&`. + let top: Node = guard; + let parent = top.getParent(); + while (parent !== undefined) { + const isAnd = Node.isBinaryExpression(parent) && parent.getOperatorToken().getKind() === SyntaxKind.AmpersandAmpersandToken; + if (!Node.isParenthesizedExpression(parent) && !Node.isPrefixUnaryExpression(parent) && !isAnd) break; + top = parent; + parent = top.getParent(); + } + return pinsSubjectToSdkCode(top, subject, guard); +} + +/** + * Boolean-shape evaluation of "every way this expression is satisfied constrains + * `subject` to an SDK-routed code": an `&&` pins if either conjunct pins, an `||` + * pins only if every branch pins, a leaf pins only as a positive same-subject + * `SdkErrorCode` equality. The guard itself and anything else (calls, negations, + * other subjects) never pin. + */ +function pinsSubjectToSdkCode(node: Node, subject: string, guard: Node): boolean { + let expr: Node = node; + while (Node.isParenthesizedExpression(expr)) expr = expr.getExpression(); + if (expr === guard) return false; + if (!Node.isBinaryExpression(expr)) return false; + const op = expr.getOperatorToken().getKind(); + if (op === SyntaxKind.AmpersandAmpersandToken) { + return pinsSubjectToSdkCode(expr.getLeft(), subject, guard) || pinsSubjectToSdkCode(expr.getRight(), subject, guard); + } + if (op === SyntaxKind.BarBarToken) { + return pinsSubjectToSdkCode(expr.getLeft(), subject, guard) && pinsSubjectToSdkCode(expr.getRight(), subject, guard); + } + if (op !== SyntaxKind.EqualsEqualsEqualsToken && op !== SyntaxKind.EqualsEqualsToken) return false; + const sides = [expr.getLeft(), expr.getRight()]; + const memberSide = sides.find(side => Node.isPropertyAccessExpression(side) && side.getExpression().getText() === 'SdkErrorCode'); + if (memberSide === undefined) return false; + const valueSide = unwrapCasts(sides.find(side => side !== memberSide)!); + if (!Node.isPropertyAccessExpression(valueSide) && !Node.isElementAccessExpression(valueSide)) return false; + return unwrapCasts(valueSide.getExpression()).getText() === subject; +} + +/** `instanceof ProtocolError`/`instanceof McpError` checks within the expression. */ +function instanceofGuards(root: Node): import('ts-morph').BinaryExpression[] { + const guards: import('ts-morph').BinaryExpression[] = []; + const visit = (candidate: Node): void => { + if (Node.isBinaryExpression(candidate) && candidate.getOperatorToken().getKind() === SyntaxKind.InstanceOfKeyword) { + const right = candidate.getRight(); + if (Node.isIdentifier(right) && (right.getText() === 'ProtocolError' || right.getText() === 'McpError')) { + guards.push(candidate); + } + } + }; + visit(root); + root.forEachDescendant(visit); + return guards; +} + function handleRequestHandlerExtra(sourceFile: SourceFile, context: TransformContext, diagnostics: Diagnostic[]): number { let changesCount = 0; @@ -354,3 +749,72 @@ function handleSchemaInput(sourceFile: SourceFile, context: TransformContext, di return changesCount; } + +/** + * When the pairing moves a subject's asserted class to SdkError, a + * `const err = (await p) as ProtocolError` binding for the same subject becomes a + * type lie (.code typed as a number while the runtime value is an SdkErrorCode + * string) — re-point the cast with the assertion. + */ +function repointSubjectCasts(scope: Node, subject: string): void { + const repointIn = (expr: Node): void => { + const casts = Node.isAsExpression(expr) ? [expr] : expr.getDescendantsOfKind(SyntaxKind.AsExpression); + for (const cast of casts) { + if (cast.wasForgotten()) continue; + const typeNode = cast.getTypeNode(); + const typeText = typeNode?.getText(); + if (typeNode !== undefined && (typeText === 'ProtocolError' || typeText === 'McpError')) { + typeNode.replaceWithText('SdkError'); + } + } + }; + for (const decl of scope.getDescendantsOfKind(SyntaxKind.VariableDeclaration)) { + if (decl.wasForgotten() || decl.getName() !== subject) continue; + const initializer = decl.getInitializer(); + if (initializer !== undefined) repointIn(initializer); + } + // `err = e as McpError` inside a catch block is the dominant test shape — the + // cast lives in an assignment, not a declaration. + for (const bin of scope.getDescendantsOfKind(SyntaxKind.BinaryExpression)) { + if (bin.wasForgotten()) continue; + if (bin.getOperatorToken().getKind() !== SyntaxKind.EqualsToken) continue; + if (bin.getLeft().getText() !== subject) continue; + repointIn(bin.getRight()); + } +} + +/** + * `const { McpError } = await import('@modelcontextprotocol/…')` — the static-import + * rename pass never sees these bindings. Shorthand elements rename binding and + * references; aliased elements (`{ McpError: ME }`) re-point only the property name. + */ +function renameDynamicImportBindings(sourceFile: SourceFile): number { + let changes = 0; + for (const decl of sourceFile.getDescendantsOfKind(SyntaxKind.VariableDeclaration)) { + if (decl.wasForgotten()) continue; + const nameNode = decl.getNameNode(); + if (!Node.isObjectBindingPattern(nameNode)) continue; + let initializer = decl.getInitializer(); + if (initializer !== undefined && Node.isAwaitExpression(initializer)) initializer = initializer.getExpression(); + if (initializer === undefined || !Node.isCallExpression(initializer)) continue; + if (initializer.getExpression().getKind() !== SyntaxKind.ImportKeyword) continue; + const spec = initializer.getArguments()[0]?.asKind(SyntaxKind.StringLiteral)?.getLiteralValue(); + if (spec === undefined || !isAnyMcpSpecifier(spec)) continue; + for (const element of nameNode.getElements()) { + const propertyNode = element.getPropertyNameNode(); + const importedName = propertyNode?.getText() ?? element.getName(); + const newName = SIMPLE_RENAMES[importedName]; + if (newName === undefined) continue; + if (propertyNode === undefined) { + // Text-based like every other rename here — the language-service rename + // resolves to the installed v1 typings and can split the shorthand into + // a dead-property alias or touch node_modules. + renameAllReferences(sourceFile, importedName, newName); + } else { + propertyNode.replaceWithText(newName); + } + changes++; + } + } + return changes; +} diff --git a/packages/codemod/src/runner.ts b/packages/codemod/src/runner.ts index 5398dcc04f..f25543b1fc 100644 --- a/packages/codemod/src/runner.ts +++ b/packages/codemod/src/runner.ts @@ -1,9 +1,15 @@ +import { statSync } from 'node:fs'; +import path from 'node:path'; + +import fg from 'fast-glob'; import type { Node } from 'ts-morph'; import { Project, SyntaxKind } from 'ts-morph'; -import type { Diagnostic, FileResult, Migration, RunnerOptions, RunnerResult } from './types'; +import { MOCK_CALLERS, MOCK_METHODS } from './migrations/v1-to-v2/transforms/mockPaths'; +import type { Diagnostic, FileResult, Migration, PackageJsonChange, RunnerOptions, RunnerResult } from './types'; import { CODEMOD_ERROR_PREFIX, error } from './utils/diagnostics'; -import { updatePackageJson } from './utils/packageJsonUpdater'; +import { isSdkSpecifier, isV2Specifier } from './utils/importUtils'; +import { discoverManifests, ownerManifest, updatePackageJson } from './utils/packageJsonUpdater'; import { analyzeProject } from './utils/projectAnalyzer'; const LITERAL_NODE_KINDS = new Set([ @@ -75,10 +81,6 @@ function insertDiagnosticComments(project: Project, fileResults: FileResult[]): return insertedCount; } -function escapeGlobPath(p: string): string { - return p.replaceAll(/[[\]{}()*?!@#]/g, String.raw`\$&`); -} - export function run(migration: Migration, options: RunnerOptions): RunnerResult { const context = analyzeProject(options.targetDir); @@ -104,7 +106,30 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult } }); - const globPattern = `${escapeGlobPath(options.targetDir)}/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}`; + // A file target scopes the run to that one source file; project context and the + // nearest-manifest rule still derive from its directory. + const targetIsFile = (() => { + try { + return statSync(options.targetDir).isFile(); + } catch { + return false; + } + })(); + + const targetBase = fg.convertPathToPattern(path.resolve(targetIsFile ? path.dirname(options.targetDir) : options.targetDir)); + const globPattern = `${targetBase}/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}`; + // The positive pattern is absolute, so fast-glob compares ignore patterns against + // absolute paths — a bare-relative --ignore would silently match nothing. Rebase + // relative user patterns onto the target directory. + const userIgnores = (options.ignore ?? []).map(pattern => { + // User ignores are PATTERNS, not paths: convertPathToPattern would escape + // their glob metacharacters (`**` → `\*\*`) and they would match nothing. + // fast-glob needs forward slashes, so Windows separators are normalized. + if (pattern.startsWith('**') || path.isAbsolute(pattern)) { + return path.sep === '\\' ? pattern.replaceAll('\\', '/') : pattern; + } + return `${targetBase}/${pattern}`; + }); const ignorePatterns = [ '**/node_modules/**', '**/dist/**', @@ -117,14 +142,24 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult '**/*.d.ts', '**/*.d.mts', '**/*.d.cts', - ...(options.ignore ?? []) + ...userIgnores ]; - const allPatterns = [globPattern]; - for (const ignore of ignorePatterns) { - allPatterns.push(`!${ignore}`); + // Collect files with fast-glob directly instead of ts-morph's glob handling: + // symbolic links are never followed (pnpm node_modules layouts contain symlink + // cycles that ELOOP a following walker) and ignore patterns — including the + // user's --ignore — apply during directory descent, not as a post-filter. + const files = targetIsFile + ? [path.resolve(options.targetDir)] + : fg.sync(globPattern, { + ignore: ignorePatterns, + followSymbolicLinks: false, + suppressErrors: true, + absolute: true + }); + for (const filePath of files) { + project.addSourceFileAtPathIfExists(filePath); } - project.addSourceFilesAtPaths(allPatterns); const sourceFiles = project.getSourceFiles().filter(sf => { const fp = sf.getFilePath(); @@ -134,7 +169,6 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult }); const fileResults: FileResult[] = []; const allDiagnostics: Diagnostic[] = []; - const allUsedPackages = new Set(); const shebangs = new Map(); let totalChanges = 0; let filesChanged = 0; @@ -154,33 +188,11 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult shebangs.set(sourceFile.getFilePath(), shebangMatch[0]); } - const fileClaimedPackages = new Set(); try { for (const transform of enabledTransforms) { const result = transform.apply(sourceFile, context); fileChanges += result.changesCount; fileDiagnostics.push(...result.diagnostics); - if (result.usedPackages) { - for (const pkg of result.usedPackages) { - fileClaimedPackages.add(pkg); - } - } - } - // A transform records a package as "used" when it routes a binding there — but importPaths does - // so the moment it rewrites an import, and later transforms (handlerRegistration, - // schemaParamRemoval) routinely rewrite the schema usage away and delete that very import. - // Honouring a claim whose import did not survive would add an unused dependency to package.json, - // so a claim counts only when the FINAL file still references the specifier. Every claim - // originates from a string literal the transform wrote (an import/export module specifier, or a - // vi.mock()/dynamic import() argument), so a surviving string-literal match is the ground truth. - const survivingSpecifiers = new Set(); - for (const literal of sourceFile.getDescendantsOfKind(SyntaxKind.StringLiteral)) { - survivingSpecifiers.add(literal.getLiteralValue()); - } - for (const pkg of fileClaimedPackages) { - if (survivingSpecifiers.has(pkg)) { - allUsedPackages.add(pkg); - } } } catch (error_) { const filePath = sourceFile.getFilePath(); @@ -188,7 +200,14 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult fileDiagnostics.push(error(filePath, 1, `Transform failed: ${error_ instanceof Error ? error_.message : String(error_)}`)); sourceFile.replaceWithText(originalText); fileChanges = 0; - fileClaimedPackages.clear(); + } + + // Heuristic advisories only flush for files a transform actually changed — + // a re-run over a migrated tree stays quiet. + if (fileChanges === 0) { + const kept = fileDiagnostics.filter(d => !d.advisoryOnly); + fileDiagnostics.length = 0; + fileDiagnostics.push(...kept); } for (const d of fileDiagnostics) { @@ -196,7 +215,10 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult try { d.line = d.resolveCurrentLine(); } catch { - // Node was removed by a later transform; keep snapshot line + // The anchor node was removed by a later rewrite — the snapshot + // line is stale against the final text, so keep the console + // diagnostic but never insert a comment at the wrong place. + delete d.insertComment; } delete d.resolveCurrentLine; } @@ -216,10 +238,68 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult } } + // The v2 dependency set for each manifest is derived from the POST-transform + // import state of the files it owns (longest-prefix match against the + // discovered manifests). Reading the final state — rather than what this run + // rewrote — means an already-migrated package whose v1 dependency is being + // removed still gets the v2 packages its imports need. const hasImportsTransform = enabledTransforms.some(t => t.id === 'imports'); - const packageJsonChanges = hasImportsTransform - ? updatePackageJson(options.targetDir, allUsedPackages, options.dryRun ?? false) - : undefined; + let packageJsonChanges: PackageJsonChange[] | undefined; + if (hasImportsTransform) { + const manifests = discoverManifests(options.targetDir); + const usedByManifest = new Map>(); + for (const sourceFile of sourceFiles) { + const owner = ownerManifest(sourceFile.getFilePath(), manifests); + if (!owner) continue; + let used = usedByManifest.get(owner.path); + if (!used) { + used = new Set(); + usedByManifest.set(owner.path, used); + } + for (const decl of sourceFile.getImportDeclarations()) { + const spec = decl.getModuleSpecifierValue(); + if (spec.startsWith('@modelcontextprotocol/')) used.add(spec); + } + for (const decl of sourceFile.getExportDeclarations()) { + const spec = decl.getModuleSpecifierValue(); + if (spec !== undefined && spec.startsWith('@modelcontextprotocol/')) used.add(spec); + } + for (const call of sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression)) { + const exprText = call.getExpression().getText(); + const dot = exprText.indexOf('.'); + const isMockCall = dot !== -1 && MOCK_CALLERS.has(exprText.slice(0, dot)) && MOCK_METHODS.has(exprText.slice(dot + 1)); + const isModuleRef = call.getExpression().getKind() === SyntaxKind.ImportKeyword || exprText === 'require' || isMockCall; + if (!isModuleRef) continue; + const spec = call.getArguments()[0]?.asKind(SyntaxKind.StringLiteral)?.getLiteralValue(); + if (spec !== undefined && spec.startsWith('@modelcontextprotocol/')) used.add(spec); + } + } + // A single-file run sees one file's imports — applying a manifest edit from that + // view could remove the v1 dependency the rest of the package still needs, so + // manifests are report-only in file mode. + const zodInjectedByManifest = new Map(); + for (const fileResult of fileResults) { + if (!fileResult.diagnostics.some(d => d.tag === 'zod-injected')) continue; + const owner = ownerManifest(fileResult.filePath, manifests); + if (!owner) continue; + let files = zodInjectedByManifest.get(owner.path); + if (!files) { + files = []; + zodInjectedByManifest.set(owner.path, files); + } + files.push(fileResult.filePath); + } + + const changes = updatePackageJson(manifests, usedByManifest, (options.dryRun ?? false) || targetIsFile, zodInjectedByManifest); + packageJsonChanges = changes.length > 0 ? changes : undefined; + } + + const mcpImportFiles = sourceFiles.filter(sf => + sf.getImportDeclarations().some(decl => isV2Specifier(decl.getModuleSpecifierValue())) + ).length; + const v1ImportFiles = sourceFiles.filter(sf => + sf.getImportDeclarations().some(decl => isSdkSpecifier(decl.getModuleSpecifierValue())) + ).length; // Per-file mutations are atomic: if any transform fails, the file is rolled back to its // original state and an error diagnostic is emitted. @@ -253,6 +333,8 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult diagnostics: allDiagnostics, fileResults, packageJsonChanges, + mcpImportFiles, + v1ImportFiles, commentCount }; } diff --git a/packages/codemod/src/types.ts b/packages/codemod/src/types.ts index 6b286f5f49..d2f42c9802 100644 --- a/packages/codemod/src/types.ts +++ b/packages/codemod/src/types.ts @@ -12,6 +12,10 @@ export interface Diagnostic { line: number; message: string; category?: 'v2-gap'; + /** Heuristic "verify this" advisories: dropped by the runner for files no transform changed, so re-runs over migrated trees stay quiet. */ + advisoryOnly?: boolean; + /** Machine-readable marker for cross-stage plumbing (e.g. the runner feeding manifest edits). */ + tag?: 'zod-injected'; insertComment?: boolean; resolveCurrentLine?: () => number; } @@ -56,6 +60,17 @@ export interface PackageJsonChange { added: string[]; removed: string[]; packageJsonPath: string; + /** + * True for the manifest the codemod writes (the nearest one walking up from the + * target directory) — or, under dry-run, the one it would write. False entries + * describe edits to other manifests — typically workspace members — that are + * reported for the user to apply themselves; those are never written. + */ + applied: boolean; + /** Context on how the change set was computed (e.g. hoisted member usage credited to this manifest). */ + notes?: string[]; + /** Manifest-level findings that need the user's attention (e.g. an incompatible zod range). */ + warnings?: string[]; } export interface RunnerResult { @@ -63,6 +78,10 @@ export interface RunnerResult { totalChanges: number; diagnostics: Diagnostic[]; fileResults: FileResult[]; - packageJsonChanges?: PackageJsonChange; + packageJsonChanges?: PackageJsonChange[]; commentCount: number; + /** Source files importing a v2 `@modelcontextprotocol/*` package after the run. */ + mcpImportFiles: number; + /** Source files still importing the v1 `@modelcontextprotocol/sdk` package after the run (possible under `--transforms` subsets). */ + v1ImportFiles: number; } diff --git a/packages/codemod/src/utils/astUtils.ts b/packages/codemod/src/utils/astUtils.ts index 643cc24977..fccac265bb 100644 --- a/packages/codemod/src/utils/astUtils.ts +++ b/packages/codemod/src/utils/astUtils.ts @@ -1,5 +1,5 @@ import type { SourceFile } from 'ts-morph'; -import { Node } from 'ts-morph'; +import { Node, SyntaxKind } from 'ts-morph'; export function isKeyPositionIdentifier(node: import('ts-morph').Node): boolean { const parent = node.getParent(); @@ -38,3 +38,13 @@ export function renameAllReferences(sourceFile: SourceFile, oldName: string, new } }); } + +/** First identifier named `name` that is not part of an import declaration. */ +export function findFirstIdentifierOutsideImports(sourceFile: SourceFile, name: string): Node | undefined { + for (const id of sourceFile.getDescendantsOfKind(SyntaxKind.Identifier)) { + if (id.getText() !== name) continue; + if (id.getFirstAncestorByKind(SyntaxKind.ImportDeclaration)) continue; + return id; + } + return undefined; +} diff --git a/packages/codemod/src/utils/importUtils.ts b/packages/codemod/src/utils/importUtils.ts index ecf3670af5..eba4572c7e 100644 --- a/packages/codemod/src/utils/importUtils.ts +++ b/packages/codemod/src/utils/importUtils.ts @@ -84,6 +84,13 @@ export function addOrMergeImport( } } +/** True when the specifier resolves to one of the published v2 packages (root or subpath). */ +export function isV2Specifier(specifier: string): boolean { + if (V2_PACKAGES.has(specifier)) return true; + const secondSlash = specifier.indexOf('/', specifier.indexOf('/') + 1); + return secondSlash !== -1 && V2_PACKAGES.has(specifier.slice(0, secondSlash)); +} + export function isAnyMcpSpecifier(specifier: string): boolean { if (isSdkSpecifier(specifier)) return true; if (V2_PACKAGES.has(specifier)) return true; diff --git a/packages/codemod/src/utils/packageJsonUpdater.ts b/packages/codemod/src/utils/packageJsonUpdater.ts index 1b3eb07f8a..0a03fc1e52 100644 --- a/packages/codemod/src/utils/packageJsonUpdater.ts +++ b/packages/codemod/src/utils/packageJsonUpdater.ts @@ -1,4 +1,7 @@ -import { readFileSync, writeFileSync } from 'node:fs'; +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; + +import fg from 'fast-glob'; import { V2_PACKAGE_VERSIONS } from '../generated/versions'; import type { PackageJsonChange } from '../types'; @@ -7,72 +10,422 @@ import { findPackageJson } from './projectAnalyzer'; const V1_PACKAGE = '@modelcontextprotocol/sdk'; const PRIVATE_PACKAGES = new Set(['@modelcontextprotocol/core-internal']); -function normalizeToRoot(pkg: string): string { +type ZodSegmentVerdict = 'ok' | 'v3' | 'v4pre42'; + +/** + * Classify one `||` alternative of a zod range by the highest version it can resolve + * to. 'ok' = can reach >=4.2 (satisfies v2's floor); 'v3' = resolves into 3.x; + * 'v4pre42' = resolves into 4.0/4.1 (typings predate `~standard.jsonSchema`, so + * registration calls fail type-checking). + */ +function classifyZodSegment(segment: string): ZodSegmentVerdict { + const seg = segment + .replace(/^(?:workspace:|npm:)/, '') + .replace(/^[=\s]+/, '') + .trim(); + if (seg === '' || seg === '*' || seg === 'x' || seg === 'latest') return 'ok'; + + const classifyUpper = (major: number, minor: number | undefined, inclusive: boolean): ZodSegmentVerdict => { + if (major > 4) return 'ok'; + if (major < 4) return 'v3'; + // major === 4: an upper bound below 4.2 caps resolution at 4.0/4.1; `<4` and + // `<4.0` cap it in 3.x. A bare inclusive major (`<=4`, `… - 4`) is an + // X-range upper bound — it admits every 4.x release and satisfies the floor. + if (minor === undefined) return inclusive ? 'ok' : 'v3'; + if (minor === 0 && inclusive === false) return 'v3'; + if (minor < 2) return 'v4pre42'; + if (minor === 2 && inclusive === false) return 'v4pre42'; + return 'ok'; + }; + + // Hyphen range `A - B`: resolution maxes at B (inclusive). + const hyphen = seg.match(/^\S+\s+-\s+v?(\d+)(?:\.(\d+))?/); + if (hyphen) { + return classifyUpper(Number(hyphen[1]), hyphen[2] === undefined ? undefined : Number(hyphen[2]), true); + } + + // Comparator sets: an upper `<`/`<=` bound caps resolution; a floor with no upper + // bound resolves to the latest release. + const upper = seg.match(/<(=?)\s*v?(\d+)(?:\.(\d+))?/); + if (upper) { + return classifyUpper(Number(upper[2]), upper[3] === undefined ? undefined : Number(upper[3]), upper[1] === '='); + } + if (seg.startsWith('>')) return 'ok'; + + // Caret: `^3.x` cannot reach 4; `^4.x` allows everything below 5. + const caret = seg.match(/^\^\s*v?(\d+)/); + if (caret) return Number(caret[1]) < 4 ? 'v3' : 'ok'; + + // Tilde and bare/exact versions: `~4.1`/`4.1.x`/`=4.0.2` stay below 4.2; a bare + // major (`4`, `~4`, `4.x`) allows the whole major line. + const plain = seg.match(/^~?\s*v?(\d+)(?:\.(x|\*|\d+))?/); + if (plain) { + const major = Number(plain[1]); + if (major < 4) return 'v3'; + if (major > 4) return 'ok'; + const minorRaw = plain[2]; + if (minorRaw === undefined || minorRaw === 'x' || minorRaw === '*') return 'ok'; + return Number(minorRaw) < 2 ? 'v4pre42' : 'ok'; + } + return 'ok'; +} + +export interface ManifestInfo { + /** Directory containing the manifest. */ + dir: string; + /** Absolute path of the package.json. */ + path: string; +} + +export function normalizeToRoot(pkg: string): string { const secondSlash = pkg.indexOf('/', pkg.indexOf('/') + 1); if (secondSlash === -1) return pkg; return pkg.slice(0, secondSlash); } +/** ts-morph standardizes file paths to forward slashes on every platform; manifests must compare the same way. */ +function toPosix(p: string): string { + return p.replaceAll('\\', '/'); +} + function detectIndent(text: string): string { const match = text.match(/\n([ \t]+)/); return match ? match[1]! : ' '; } -export function updatePackageJson(targetDir: string, usedPackages: Set, dryRun: boolean): PackageJsonChange | undefined { - const pkgJsonPath = findPackageJson(targetDir); - if (!pkgJsonPath) return undefined; - - let raw: string; - let pkgJson: Record; +function readJson(p: string): { raw: string; json: Record } | undefined { try { - raw = readFileSync(pkgJsonPath, 'utf8'); - pkgJson = JSON.parse(raw) as Record; + const raw = readFileSync(p, 'utf8'); + return { raw, json: JSON.parse(raw) as Record }; } catch { return undefined; } - const deps = pkgJson.dependencies as Record | undefined; - const devDeps = pkgJson.devDependencies as Record | undefined; +} - const inDeps = deps !== undefined && V1_PACKAGE in deps; - const inDevDeps = devDeps !== undefined && V1_PACKAGE in devDeps; - if (!inDeps && !inDevDeps) return undefined; +/** Parse the `packages:` list out of a pnpm-workspace.yaml without a YAML dependency. */ +function pnpmWorkspaceGlobs(rootDir: string): string[] { + const p = path.join(rootDir, 'pnpm-workspace.yaml'); + if (!existsSync(p)) return []; + const globs: string[] = []; + let inPackages = false; + for (const line of readFileSync(p, 'utf8').split('\n')) { + if (/^packages\s*:/.test(line)) { + inPackages = true; + continue; + } + if (inPackages) { + const item = line.match(/^\s+-\s*['"]?([^'"#]+?)['"]?\s*(?:#.*)?$/); + if (item) { + globs.push(item[1]!); + continue; + } + if (/^\S/.test(line)) inPackages = false; // next top-level key + } + } + return globs; +} - const packagesToAdd = [...new Set([...usedPackages].map(pkg => normalizeToRoot(pkg)))].filter( - pkg => !PRIVATE_PACKAGES.has(pkg) && pkg in V2_PACKAGE_VERSIONS - ); +/** Workspace member globs from the root manifest's `workspaces` field (npm/yarn/bun shape). */ +function npmWorkspaceGlobs(rootJson: Record): string[] { + const ws = rootJson.workspaces; + if (Array.isArray(ws)) return ws.filter((g): g is string => typeof g === 'string'); + if (ws && typeof ws === 'object' && Array.isArray((ws as { packages?: unknown }).packages)) { + return (ws as { packages: unknown[] }).packages.filter((g): g is string => typeof g === 'string'); + } + return []; +} - // Determine which section to add v2 packages to. - // If v1 SDK was in both, prefer dependencies. - const targetSection = inDeps ? 'dependencies' : 'devDependencies'; +/** + * The manifests a migration run may need to update: the nearest package.json walking + * up from the target directory, plus every workspace-member manifest it declares + * (npm/yarn/bun `workspaces` and pnpm-workspace.yaml), so monorepo members do not + * keep a stale v1 dependency the root swap never sees. + */ +export function discoverManifests(targetDir: string): ManifestInfo[] { + const rootManifest = findPackageJson(targetDir); + if (!rootManifest) return []; + const rootDir = path.dirname(rootManifest); + const manifests: ManifestInfo[] = [{ dir: toPosix(rootDir), path: rootManifest }]; - const added: string[] = []; - for (const pkg of packagesToAdd) { - const alreadyInDeps = deps !== undefined && pkg in deps; - const alreadyInDevDeps = devDeps !== undefined && pkg in devDeps; - if (alreadyInDeps || alreadyInDevDeps) continue; + const rootJson = readJson(rootManifest)?.json ?? {}; + const memberGlobs = [...npmWorkspaceGlobs(rootJson), ...pnpmWorkspaceGlobs(rootDir)]; + if (memberGlobs.length === 0) return manifests; - if (!pkgJson[targetSection]) { - pkgJson[targetSection] = {}; + const memberDirs = fg.sync(memberGlobs, { + cwd: rootDir, + onlyDirectories: true, + followSymbolicLinks: false, + suppressErrors: true, + ignore: ['**/node_modules/**'], + absolute: true + }); + // Workspace members live under the root that declares them; a glob that + // resolves elsewhere (an absolute pattern, a parent reference) is outside this + // run's scope and is not reported. + const resolvedRoot = path.resolve(rootDir); + for (const dir of memberDirs) { + if (!path.resolve(dir).startsWith(resolvedRoot + path.sep)) continue; + const manifest = path.join(dir, 'package.json'); + if (existsSync(manifest) && manifest !== rootManifest) { + manifests.push({ dir: toPosix(dir), path: manifest }); } - (pkgJson[targetSection] as Record)[pkg] = V2_PACKAGE_VERSIONS[pkg]!; - added.push(pkg); } + return manifests; +} - if (inDeps) delete deps![V1_PACKAGE]; - if (inDevDeps) delete devDeps![V1_PACKAGE]; - const removed = [V1_PACKAGE]; +/** Longest-prefix owner of a file among the discovered manifest directories. */ +export function ownerManifest(filePath: string, manifests: readonly ManifestInfo[]): ManifestInfo | undefined { + const posixFile = toPosix(filePath); + let best: ManifestInfo | undefined; + for (const m of manifests) { + const dir = toPosix(m.dir); + const prefix = dir.endsWith('/') ? dir : dir + '/'; + if (posixFile.startsWith(prefix) && (!best || dir.length > (best ? toPosix(best.dir).length : 0))) { + best = m; + } + } + return best; +} - if (!dryRun) { - const indent = detectIndent(raw); - const trailingNewline = raw.endsWith('\n'); - let output = JSON.stringify(pkgJson, null, indent); - if (trailingNewline) output += '\n'; - writeFileSync(pkgJsonPath, output); +function zodWarning(...depSections: (Record | undefined)[]): string | undefined { + const range = depSections.find(section => section?.zod !== undefined)?.zod; + if (range === undefined) return undefined; + // Every `||` alternative must fall short of the floor before we warn — `^3.25 || ^4.5` resolves fine. + const verdicts = range.split('||').map(seg => classifyZodSegment(seg)); + if (verdicts.length === 0 || verdicts.includes('ok')) return undefined; + const floor = `zod range '${range}' cannot satisfy v2's floor: zod >=4.2.0 is required. `; + if (verdicts.every(v => v === 'v4pre42')) { + return ( + floor + + `This range resolves to zod 4.0-4.1, which predates ~standard.jsonSchema: registerTool/registerPrompt ` + + `calls fail type-checking (TS2769: no overload matches), and plain-JavaScript projects run through a ` + + `bundled fallback that drops .describe() field descriptions.` + ); } + return ( + floor + + `An older range installs cleanly and then, depending on the zod entry point your code imports, ` + + `fails type-checking (zod/v4 subpath) or only fails at runtime ` + + `(main-entry imports: the server starts normally and the first tools/list reports the failure).` + ); +} + +interface ParsedManifest { + raw: string; + json: Record; + deps?: Record; + devDeps?: Record; + peerDeps?: Record; + optionalDeps?: Record; + declaresV1: boolean; +} +function parseManifest(manifestPath: string): ParsedManifest | undefined { + const parsed = readJson(manifestPath); + if (!parsed) return undefined; + const deps = parsed.json.dependencies as Record | undefined; + const devDeps = parsed.json.devDependencies as Record | undefined; + const peerDeps = parsed.json.peerDependencies as Record | undefined; + const optionalDeps = parsed.json.optionalDependencies as Record | undefined; return { - added: added.toSorted(), - removed, - packageJsonPath: pkgJsonPath + raw: parsed.raw, + json: parsed.json, + deps, + devDeps, + peerDeps, + optionalDeps, + declaresV1: (deps !== undefined && V1_PACKAGE in deps) || (devDeps !== undefined && V1_PACKAGE in devDeps) }; } + +/** + * Swap the v1 SDK dependency for the v2 packages in the **nearest** manifest (the + * first entry of `manifests`), and report — without modifying — the same edit for + * every other manifest that declares the v1 SDK, so workspace members never receive + * writes the user did not point the codemod at. + * + * The v2 additions come from the **post-transform** import state of the files each + * manifest owns (`usedByManifest`), not from what was rewritten in this run — so a + * partially or fully pre-migrated package still gets the v2 packages its imports + * need when its v1 dependency is removed. + */ +const ZOD_RANGE = '^4.2.0'; +const TEST_PATH_RE = /(^|\/)(test|tests|__tests__)\/|\.(test|spec)\.[cm]?[jt]sx?$/; + +function writeManifest(manifestPath: string, raw: string, pkgJson: Record): void { + const indent = detectIndent(raw); + let output = JSON.stringify(pkgJson, null, indent); + if (raw.endsWith('\n')) output += '\n'; + writeFileSync(manifestPath, output); +} + +export function updatePackageJson( + manifests: readonly ManifestInfo[], + usedByManifest: ReadonlyMap>, + dryRun: boolean, + zodInjectedByManifest: ReadonlyMap = new Map() +): PackageJsonChange[] { + const changes: PackageJsonChange[] = []; + const nearestPath = manifests[0]?.path; + + // Each manifest is read and parsed exactly once; every later step (v1 checks, + // roll-up, edits) works from this shared view. + const parsedByPath = new Map(); + for (const m of manifests) { + const parsed = parseManifest(m.path); + if (parsed) parsedByPath.set(m.path, parsed); + } + + // Hoisted-dependency roll-up: a workspace member without its own v1 dependency + // relies on an ancestor manifest (usually the root) for SDK resolution, so its + // usage must count toward the nearest ancestor that DOES declare the v1 SDK — + // otherwise a hoisted monorepo would get the v1 dependency removed from the + // root with none of the v2 replacements added. + const effectiveUsed = new Map>(); + const hoistNotes = new Map(); + for (const m of manifests) { + effectiveUsed.set(m.path, new Set(usedByManifest.get(m.path))); + } + const v1Ancestors = manifests.filter(a => parsedByPath.get(a.path)?.declaresV1 === true); + + // zod-injected files follow the same hoisting rule as usage: a member without its + // own v1 dependency takes part through its nearest v1-declaring ancestor, so its + // injections must surface there or the add is silently lost. + const effectiveZodInjected = new Map(); + for (const [manifestPath, files] of zodInjectedByManifest) { + const declaresV1Here = parsedByPath.get(manifestPath)?.declaresV1 === true; + const home = declaresV1Here ? manifestPath : (ownerManifest(manifestPath, v1Ancestors)?.path ?? manifestPath); + let bucket = effectiveZodInjected.get(home); + if (!bucket) { + bucket = []; + effectiveZodInjected.set(home, bucket); + } + bucket.push(...files); + } + for (const m of manifests) { + if (parsedByPath.get(m.path)?.declaresV1 === true) continue; + const used = effectiveUsed.get(m.path); + if (!used || used.size === 0) continue; + // A member only participates when its imports map to publishable v2 packages. + const contributes = [...used] + .map(pkg => normalizeToRoot(pkg)) + .some(pkg => !PRIVATE_PACKAGES.has(pkg) && pkg in V2_PACKAGE_VERSIONS); + if (!contributes) continue; + const ancestor = ownerManifest(m.path, v1Ancestors); + if (ancestor) { + const target = effectiveUsed.get(ancestor.path); + for (const pkg of used) target?.add(pkg); + let notes = hoistNotes.get(ancestor.path); + if (!notes) { + notes = []; + hoistNotes.set(ancestor.path, notes); + } + notes.push( + `workspace member ${toPosix(path.relative(ancestor.dir, m.dir)) || '.'} has no own ${V1_PACKAGE} dependency ` + + `and resolves the SDK through this manifest; its imports were included when computing the v2 dependency set` + ); + } + } + + for (const manifest of manifests) { + const parsed = parsedByPath.get(manifest.path); + if (!parsed) continue; + const { raw, json: pkgJson, deps, devDeps, peerDeps, optionalDeps } = parsed; + + const inDeps = deps !== undefined && V1_PACKAGE in deps; + const inDevDeps = devDeps !== undefined && V1_PACKAGE in devDeps; + const warning = zodWarning(deps, devDeps, peerDeps, optionalDeps); + + const declaresAnyV2 = Object.keys({ ...deps, ...devDeps }).some(dep => dep in V2_PACKAGE_VERSIONS); + const usesV2 = (effectiveUsed.get(manifest.path)?.size ?? 0) > 0; + const isNearest = manifest.path === nearestPath; + + // The file-level `import { z } from 'zod'` injection promises a manifest edit; + // it must land even when this manifest never declared the v1 SDK (a member + // already on v2, or a sub-tree target whose v1 dependency lives outside it). + const injectedFiles = effectiveZodInjected.get(manifest.path); + // A peer- or optional-declared zod is still a declaration: adding a direct + // dependency on top of it would duplicate (and possibly conflict with) the + // package's own contract. + const declaresZod = [deps, devDeps, peerDeps, optionalDeps].some(section => section !== undefined && 'zod' in section); + const needsInjectedZod = injectedFiles !== undefined && injectedFiles.length > 0 && !declaresZod; + const addInjectedZod = (targetSection: 'dependencies' | 'devDependencies'): 'dependencies' | 'devDependencies' => { + // Classify against the path RELATIVE to the package — a test/ segment in + // some ancestor directory (CI checkout dirs) must not demote the dep. + const testOnly = injectedFiles!.every(file => TEST_PATH_RE.test(toPosix(path.relative(manifest.dir, file)))); + const section = testOnly ? 'devDependencies' : targetSection; + if (!pkgJson[section]) { + pkgJson[section] = {}; + } + (pkgJson[section] as Record)['zod'] = ZOD_RANGE; + return section; + }; + + if (!inDeps && !inDevDeps) { + if (needsInjectedZod) { + addInjectedZod('dependencies'); + if (isNearest && !dryRun) { + writeManifest(manifest.path, raw, pkgJson); + } + changes.push({ added: ['zod'], removed: [], packageJsonPath: manifest.path, applied: isNearest }); + continue; + } + // The zod note only matters for manifests that take part in the migration: + // they declare a v2 package, or their files import one (hoisted members). + if (warning && (declaresAnyV2 || usesV2)) { + changes.push({ added: [], removed: [], packageJsonPath: manifest.path, applied: false, warnings: [warning] }); + } + continue; + } + + const used = effectiveUsed.get(manifest.path) ?? new Set(); + const packagesToAdd = [...new Set([...used].map(pkg => normalizeToRoot(pkg)))].filter( + pkg => !PRIVATE_PACKAGES.has(pkg) && pkg in V2_PACKAGE_VERSIONS + ); + + // If v1 SDK was in both sections, prefer dependencies. + const targetSection = inDeps ? 'dependencies' : 'devDependencies'; + + const added: string[] = []; + for (const pkg of packagesToAdd) { + const alreadyInDeps = deps !== undefined && pkg in deps; + const alreadyInDevDeps = devDeps !== undefined && pkg in devDeps; + if (alreadyInDeps || alreadyInDevDeps) continue; + + if (!pkgJson[targetSection]) { + pkgJson[targetSection] = {}; + } + (pkgJson[targetSection] as Record)[pkg] = V2_PACKAGE_VERSIONS[pkg]!; + added.push(pkg); + } + + // Wrapped raw shapes had `import { z } from 'zod'` injected — under strict + // node_modules layouts the owning package must declare zod itself, so a + // zod-less manifest gets it added (devDependencies when only tests import it). + if (needsInjectedZod) { + addInjectedZod(targetSection); + added.push('zod'); + } + + if (inDeps) delete deps![V1_PACKAGE]; + if (inDevDeps) delete devDeps![V1_PACKAGE]; + + // Only the nearest manifest is written; the others are reported so the user + // applies (or deliberately skips) each workspace-member edit themselves. + if (isNearest && !dryRun) { + writeManifest(manifest.path, raw, pkgJson); + } + + const notes = hoistNotes.get(manifest.path); + changes.push({ + added: added.toSorted(), + removed: [V1_PACKAGE], + packageJsonPath: manifest.path, + applied: isNearest, + ...(notes !== undefined && { notes }), + ...(warning !== undefined && (added.length > 0 || declaresAnyV2 || usesV2) && { warnings: [warning] }) + }); + } + + return changes; +} diff --git a/packages/codemod/test/commentInsertion.test.ts b/packages/codemod/test/commentInsertion.test.ts index 8a71cd00a6..255cb9e581 100644 --- a/packages/codemod/test/commentInsertion.test.ts +++ b/packages/codemod/test/commentInsertion.test.ts @@ -328,3 +328,32 @@ describe('comment insertion', () => { expect(lines[commentIdx + 1]).toContain('setRequestHandler'); }); }); + +describe('markers whose import-declaration anchor is removed by the same pass', () => { + it('inserts the resource-server auth helper marker at the usage site', () => { + const dir = createTempDir(); + writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }) + ); + const file = path.join(dir, 'auth.ts'); + writeFileSync( + file, + [ + `import { requireBearerAuth } from '@modelcontextprotocol/sdk/server/auth/middleware/bearerAuth.js';`, + ``, + `export const guard = requireBearerAuth({ verifier });`, + '' + ].join('\n') + ); + + run(migration, { targetDir: dir, dryRun: false }); + + const output = readFileSync(file, 'utf8'); + expect(output).toContain('@mcp-codemod-error'); + expect(output).toContain('frozen'); + const lines = output.split('\n'); + const markerIndex = lines.findIndex(line => line.includes('@mcp-codemod-error')); + expect(lines[markerIndex + 1]).toContain('requireBearerAuth({ verifier })'); + }); +}); diff --git a/packages/codemod/test/integration.test.ts b/packages/codemod/test/integration.test.ts index 6c1cd0634b..6a149da7f0 100644 --- a/packages/codemod/test/integration.test.ts +++ b/packages/codemod/test/integration.test.ts @@ -322,7 +322,7 @@ describe('integration', () => { // Phantom package from the reverted transform should not leak into package.json expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.added).not.toContain('@modelcontextprotocol/phantom-pkg'); + expect(result.packageJsonChanges![0]!.added).not.toContain('@modelcontextprotocol/phantom-pkg'); }); it('respects transform filter option', () => { @@ -422,9 +422,9 @@ describe('integration', () => { const result = run(migration, { targetDir: dir }); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/server'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/node'); + expect(result.packageJsonChanges![0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/node'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/sdk']).toBeUndefined(); @@ -460,8 +460,8 @@ describe('integration', () => { // So core must not be added; the package actually imported (server) still is. expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/server'); - expect(result.packageJsonChanges!.added).not.toContain('@modelcontextprotocol/core'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.added).not.toContain('@modelcontextprotocol/core'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/core']).toBeUndefined(); @@ -491,7 +491,7 @@ describe('integration', () => { expect(output).toContain('CallToolResultSchema.parse'); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/core'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/core'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/core']).toBeDefined(); @@ -509,7 +509,7 @@ describe('integration', () => { const result = run(migration, { targetDir: dir, dryRun: true }); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/server'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/sdk']).toBe('^1.0.0'); @@ -532,10 +532,10 @@ describe('integration', () => { const result = run(migration, { targetDir: dir }); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/client'); - expect(result.packageJsonChanges!.added).not.toContain('@modelcontextprotocol/server'); - expect(result.packageJsonChanges!.added).not.toContain('@modelcontextprotocol/node'); + expect(result.packageJsonChanges![0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/client'); + expect(result.packageJsonChanges![0]!.added).not.toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.added).not.toContain('@modelcontextprotocol/node'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/sdk']).toBeUndefined(); @@ -568,9 +568,9 @@ describe('integration', () => { const result = run(migration, { targetDir: dir }); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/server'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/client'); + expect(result.packageJsonChanges![0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/client'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/sdk']).toBeUndefined(); @@ -596,9 +596,9 @@ describe('integration', () => { const result = run(migration, { targetDir: dir }); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/express'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/express'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/server'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/sdk']).toBeUndefined(); @@ -644,9 +644,9 @@ describe('integration', () => { const result = run(migration, { targetDir: dir }); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/server'); - expect(result.packageJsonChanges!.added).toContain('@modelcontextprotocol/node'); + expect(result.packageJsonChanges![0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/server'); + expect(result.packageJsonChanges![0]!.added).toContain('@modelcontextprotocol/node'); const pkgJson = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); expect(pkgJson.dependencies['@modelcontextprotocol/sdk']).toBeUndefined(); @@ -688,7 +688,7 @@ describe('integration', () => { const result = run(migration, { targetDir: dir }); expect(result.filesChanged).toBe(0); expect(result.packageJsonChanges).toBeDefined(); - expect(result.packageJsonChanges!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result.packageJsonChanges![0]!.removed).toContain('@modelcontextprotocol/sdk'); }); it('emits info diagnostics for legacy-moved imports', () => { diff --git a/packages/codemod/test/packageJsonUpdater.test.ts b/packages/codemod/test/packageJsonUpdater.test.ts index 9b1e993e2f..463fea5099 100644 --- a/packages/codemod/test/packageJsonUpdater.test.ts +++ b/packages/codemod/test/packageJsonUpdater.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { updatePackageJson } from '../src/utils/packageJsonUpdater'; +import { discoverManifests, updatePackageJson } from '../src/utils/packageJsonUpdater'; let tempDir: string; @@ -36,11 +36,15 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); - expect(result).toBeDefined(); - expect(result!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result!.added).toContain('@modelcontextprotocol/server'); + expect(result).toHaveLength(1); + expect(result[0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result[0]!.added).toContain('@modelcontextprotocol/server'); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; @@ -57,9 +61,13 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/client']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/client'])]]), + false + ); - expect(result).toBeDefined(); + expect(result).toHaveLength(1); const pkg = readPkgJson(dir); const devDeps = pkg.devDependencies as Record; expect(devDeps['@modelcontextprotocol/sdk']).toBeUndefined(); @@ -77,9 +85,13 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); - expect(result).toBeDefined(); + expect(result).toHaveLength(1); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; const devDeps = pkg.devDependencies as Record; @@ -98,11 +110,15 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/server', '@modelcontextprotocol/node']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server', '@modelcontextprotocol/node'])]]), + false + ); - expect(result).toBeDefined(); - expect(result!.added).toContain('@modelcontextprotocol/node'); - expect(result!.added).not.toContain('@modelcontextprotocol/server'); + expect(result).toHaveLength(1); + expect(result[0]!.added).toContain('@modelcontextprotocol/node'); + expect(result[0]!.added).not.toContain('@modelcontextprotocol/server'); }); it('returns undefined when no package.json exists', () => { @@ -110,8 +126,8 @@ describe('updatePackageJson', () => { mkdirSync(path.join(dir, '.git'), { recursive: true }); mkdirSync(path.join(dir, 'src'), { recursive: true }); - const result = updatePackageJson(path.join(dir, 'src'), new Set(['@modelcontextprotocol/server']), false); - expect(result).toBeUndefined(); + const result = updatePackageJson(discoverManifests(path.join(dir, 'src')), new Map(), false); + expect(result).toHaveLength(0); }); it('returns undefined when v1 SDK is not in package.json', () => { @@ -122,8 +138,12 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); - expect(result).toBeUndefined(); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); + expect(result).toHaveLength(0); }); it('does not write file in dry-run mode', () => { @@ -134,10 +154,14 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), true); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + true + ); - expect(result).toBeDefined(); - expect(result!.added).toContain('@modelcontextprotocol/server'); + expect(result).toHaveLength(1); + expect(result[0]!.added).toContain('@modelcontextprotocol/server'); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; @@ -153,11 +177,15 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/core-internal', '@modelcontextprotocol/server']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/core-internal', '@modelcontextprotocol/server'])]]), + false + ); - expect(result).toBeDefined(); - expect(result!.added).not.toContain('@modelcontextprotocol/core-internal'); - expect(result!.added).toContain('@modelcontextprotocol/server'); + expect(result).toHaveLength(1); + expect(result[0]!.added).not.toContain('@modelcontextprotocol/core-internal'); + expect(result[0]!.added).toContain('@modelcontextprotocol/server'); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; @@ -176,7 +204,11 @@ describe('updatePackageJson', () => { 4 ); - updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); + updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); const raw = readFileSync(path.join(dir, 'package.json'), 'utf8'); expect(raw).toContain(' "dependencies"'); @@ -190,7 +222,11 @@ describe('updatePackageJson', () => { } }); - updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); + updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); const raw = readFileSync(path.join(dir, 'package.json'), 'utf8'); expect(raw.endsWith('\n')).toBe(true); @@ -206,11 +242,11 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(), false); + const result = updatePackageJson(discoverManifests(dir), new Map([[path.join(dir, 'package.json'), new Set()]]), false); - expect(result).toBeDefined(); - expect(result!.removed).toContain('@modelcontextprotocol/sdk'); - expect(result!.added).toEqual([]); + expect(result).toHaveLength(1); + expect(result[0]!.removed).toContain('@modelcontextprotocol/sdk'); + expect(result[0]!.added).toEqual([]); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; @@ -226,7 +262,11 @@ describe('updatePackageJson', () => { } }); - updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); + updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; @@ -237,8 +277,12 @@ describe('updatePackageJson', () => { const dir = createTempDir(); writeFileSync(path.join(dir, 'package.json'), '{ invalid json }'); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/server']), false); - expect(result).toBeUndefined(); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/server'])]]), + false + ); + expect(result).toHaveLength(0); }); it('normalizes subpath packages to root before adding to package.json', () => { @@ -249,10 +293,14 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/client/stdio']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/client/stdio'])]]), + false + ); - expect(result).toBeDefined(); - expect(result!.added).toContain('@modelcontextprotocol/client'); + expect(result).toHaveLength(1); + expect(result[0]!.added).toContain('@modelcontextprotocol/client'); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; @@ -267,10 +315,14 @@ describe('updatePackageJson', () => { } }); - const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/client', '@modelcontextprotocol/client/stdio']), false); + const result = updatePackageJson( + discoverManifests(dir), + new Map([[path.join(dir, 'package.json'), new Set(['@modelcontextprotocol/client', '@modelcontextprotocol/client/stdio'])]]), + false + ); - expect(result).toBeDefined(); - expect(result!.added.filter(p => p === '@modelcontextprotocol/client')).toHaveLength(1); + expect(result).toHaveLength(1); + expect(result[0]!.added.filter(p => p === '@modelcontextprotocol/client')).toHaveLength(1); }); it('adds multiple v2 packages', () => { @@ -282,13 +334,18 @@ describe('updatePackageJson', () => { }); const result = updatePackageJson( - dir, - new Set(['@modelcontextprotocol/server', '@modelcontextprotocol/node', '@modelcontextprotocol/express']), + discoverManifests(dir), + new Map([ + [ + path.join(dir, 'package.json'), + new Set(['@modelcontextprotocol/server', '@modelcontextprotocol/node', '@modelcontextprotocol/express']) + ] + ]), false ); - expect(result).toBeDefined(); - expect(result!.added).toEqual(['@modelcontextprotocol/express', '@modelcontextprotocol/node', '@modelcontextprotocol/server']); + expect(result).toHaveLength(1); + expect(result[0]!.added).toEqual(['@modelcontextprotocol/express', '@modelcontextprotocol/node', '@modelcontextprotocol/server']); const pkg = readPkgJson(dir); const deps = pkg.dependencies as Record; diff --git a/packages/codemod/test/v1-to-v2/packageJsonUpdater.test.ts b/packages/codemod/test/v1-to-v2/packageJsonUpdater.test.ts new file mode 100644 index 0000000000..75bd4877d0 --- /dev/null +++ b/packages/codemod/test/v1-to-v2/packageJsonUpdater.test.ts @@ -0,0 +1,690 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { v1ToV2Migration } from '../../src/migrations/v1-to-v2'; +import { run } from '../../src/runner'; +import { discoverManifests, ownerManifest, updatePackageJson } from '../../src/utils/packageJsonUpdater'; + +let dir: string; + +beforeEach(() => { + dir = mkdtempSync(path.join(tmpdir(), 'codemod-manifests-')); +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +function writeJson(rel: string, value: unknown, indent = ' '): string { + const p = path.join(dir, rel); + mkdirSync(path.dirname(p), { recursive: true }); + writeFileSync(p, JSON.stringify(value, null, indent) + '\n'); + return p; +} + +function readJson(p: string): Record { + return JSON.parse(readFileSync(p, 'utf8')) as Record; +} + +describe('discoverManifests', () => { + it('returns the nearest manifest walking up from the target directory', () => { + const root = writeJson('package.json', { name: 'app' }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + const manifests = discoverManifests(path.join(dir, 'src')); + expect(manifests.map(m => m.path)).toEqual([root]); + }); + + it('includes npm/yarn workspace members', () => { + const root = writeJson('package.json', { name: 'mono', workspaces: ['packages/*'] }); + const a = writeJson('packages/a/package.json', { name: 'a' }); + const b = writeJson('packages/b/package.json', { name: 'b' }); + const manifests = discoverManifests(dir); + expect(manifests.map(m => m.path).toSorted()).toEqual([root, a, b].toSorted()); + }); + + it('includes pnpm-workspace.yaml members', () => { + const root = writeJson('package.json', { name: 'mono' }); + writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n - apps/web\n"); + const a = writeJson('packages/a/package.json', { name: 'a' }); + const web = writeJson('apps/web/package.json', { name: 'web' }); + const manifests = discoverManifests(dir); + expect(manifests.map(m => m.path).toSorted()).toEqual([root, a, web].toSorted()); + }); +}); + +describe('ownerManifest', () => { + it('assigns a file to the longest-prefix manifest directory', () => { + writeJson('package.json', { name: 'mono', workspaces: ['packages/*'] }); + writeJson('packages/a/package.json', { name: 'a' }); + const manifests = discoverManifests(dir); + const inMember = ownerManifest(path.join(dir, 'packages/a/src/index.ts'), manifests); + const inRoot = ownerManifest(path.join(dir, 'scripts/build.ts'), manifests); + expect(inMember?.path).toBe(path.join(dir, 'packages/a/package.json')); + expect(inRoot?.path).toBe(path.join(dir, 'package.json')); + }); +}); + +describe('updatePackageJson', () => { + it('swaps the v1 dependency for the used v2 packages in a single manifest', () => { + const manifest = writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', express: '^5.0.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson( + manifests, + new Map([[manifest, new Set(['@modelcontextprotocol/client', '@modelcontextprotocol/client/stdio'])]]), + false + ); + expect(changes).toHaveLength(1); + expect(changes[0]!.removed).toEqual(['@modelcontextprotocol/sdk']); + expect(changes[0]!.added).toEqual(['@modelcontextprotocol/client']); + const after = readJson(manifest); + const deps = after.dependencies as Record; + expect(deps['@modelcontextprotocol/sdk']).toBeUndefined(); + expect(deps['@modelcontextprotocol/client']).toBeDefined(); + expect(deps.express).toBe('^5.0.0'); + }); + + it('reports workspace-member manifests without modifying them', () => { + writeJson('package.json', { name: 'mono', workspaces: ['packages/*'] }); + const member = writeJson('packages/a/package.json', { + name: 'a', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[member, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes).toHaveLength(1); + expect(changes[0]!.packageJsonPath).toBe(member); + expect(changes[0]!.applied).toBe(false); + expect(changes[0]!.removed).toEqual(['@modelcontextprotocol/sdk']); + expect(changes[0]!.added).toEqual(['@modelcontextprotocol/server']); + // The member manifest is reported, never written. + const deps = readJson(member).dependencies as Record; + expect(deps['@modelcontextprotocol/sdk']).toBe('^1.29.0'); + expect(deps['@modelcontextprotocol/server']).toBeUndefined(); + }); + + it('writes the nearest manifest and reports a member that also declares v1', () => { + const root = writeJson('package.json', { + name: 'mono', + workspaces: ['packages/*'], + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + const member = writeJson('packages/a/package.json', { + name: 'a', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson( + manifests, + new Map([ + [root, new Set(['@modelcontextprotocol/client'])], + [member, new Set(['@modelcontextprotocol/server'])] + ]), + false + ); + const rootChange = changes.find(c => c.packageJsonPath === root); + const memberChange = changes.find(c => c.packageJsonPath === member); + expect(rootChange?.applied).toBe(true); + expect(memberChange?.applied).toBe(false); + expect((readJson(root).dependencies as Record)['@modelcontextprotocol/sdk']).toBeUndefined(); + expect((readJson(member).dependencies as Record)['@modelcontextprotocol/sdk']).toBe('^1.29.0'); + expect(memberChange?.added).toEqual(['@modelcontextprotocol/server']); + }); + + it('places additions in devDependencies when v1 lived there', () => { + const manifest = writeJson('package.json', { + name: 'app', + devDependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + updatePackageJson(discoverManifests(dir), new Map([[manifest, new Set(['@modelcontextprotocol/server'])]]), false); + const after = readJson(manifest); + expect((after.devDependencies as Record)['@modelcontextprotocol/server']).toBeDefined(); + expect(after.dependencies).toBeUndefined(); + }); + + it('warns on a zod range below the v2 floor without touching it', () => { + const manifest = writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '^3.25.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('zod'); + expect((readJson(manifest).dependencies as Record).zod).toBe('^3.25.0'); + }); + + it('does not warn on zod ranges that satisfy the floor', () => { + writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '^4.2.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings).toBeUndefined(); + }); + + it('reports a zod warning for v2-declaring manifests without the v1 dependency', () => { + writeJson('package.json', { name: 'app', dependencies: { zod: '~4.1.0', '@modelcontextprotocol/server': '^2.0.0-alpha.3' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes).toHaveLength(1); + expect(changes[0]!.removed).toEqual([]); + expect(changes[0]!.warnings?.[0]).toContain('zod'); + }); + + it('dry run reports without writing', () => { + const manifest = writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + const before = readFileSync(manifest, 'utf8'); + const changes = updatePackageJson(discoverManifests(dir), new Map([[manifest, new Set(['@modelcontextprotocol/client'])]]), true); + expect(changes[0]!.removed).toEqual(['@modelcontextprotocol/sdk']); + // `applied` marks the write target even in a dry run (nothing is written either way). + expect(changes[0]!.applied).toBe(true); + expect(readFileSync(manifest, 'utf8')).toBe(before); + }); + + it('preserves 4-space indentation', () => { + const manifest = writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }, ' '); + updatePackageJson(discoverManifests(dir), new Map(), false); + expect(readFileSync(manifest, 'utf8')).toContain('\n "name"'); + }); +}); + +describe('run() manifest integration', () => { + it('reports the v2 packages an already-migrated workspace member needs without modifying it', () => { + writeJson('package.json', { name: 'mono', workspaces: ['packages/*'] }); + const member = writeJson('packages/a/package.json', { + name: 'a', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + mkdirSync(path.join(dir, 'packages/a/src'), { recursive: true }); + // Source is ALREADY on v2 imports — nothing for the import transform to rewrite. + writeFileSync( + path.join(dir, 'packages/a/src/index.ts'), + "import { Client } from '@modelcontextprotocol/client';\nexport const c = new Client({ name: 'x', version: '1' });\n" + ); + + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: false }); + + const change = result.packageJsonChanges?.find(c => c.packageJsonPath === member); + expect(change).toBeDefined(); + expect(change!.applied).toBe(false); + expect(change!.removed).toEqual(['@modelcontextprotocol/sdk']); + expect(change!.added).toContain('@modelcontextprotocol/client'); + // The member manifest is a workspace member, not the nearest manifest — reported only. + const deps = readJson(member).dependencies as Record; + expect(deps['@modelcontextprotocol/sdk']).toBe('^1.29.0'); + expect(deps['@modelcontextprotocol/client']).toBeUndefined(); + }); + + it('survives a directory symlink cycle without following it', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + writeFileSync( + path.join(dir, 'src/index.ts'), + "import { Client } from '@modelcontextprotocol/sdk/client/index.js';\nexport { Client };\n" + ); + // A cycle: src/loop -> the project root. + symlinkSync(dir, path.join(dir, 'src', 'loop'), 'dir'); + + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: false }); + + // The symlinked re-entry must not be followed: exactly one source file seen. + expect(result.fileResults.map(fr => fr.filePath)).toHaveLength(1); + expect(result.packageJsonChanges?.[0]?.applied).toBe(true); + expect(result.packageJsonChanges?.[0]?.removed).toEqual(['@modelcontextprotocol/sdk']); + }); +}); + +describe('hoisted-dependency roll-up', () => { + it('credits member usage to the root when only the root declares v1, and notes the hoist', () => { + const root = writeJson('package.json', { + name: 'mono', + workspaces: ['packages/*'], + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + const member = writeJson('packages/a/package.json', { name: 'a' }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[member, new Set(['@modelcontextprotocol/client'])]]), false); + expect(changes).toHaveLength(1); + expect(changes[0]!.packageJsonPath).toBe(root); + expect(changes[0]!.applied).toBe(true); + expect(changes[0]!.removed).toEqual(['@modelcontextprotocol/sdk']); + expect(changes[0]!.added).toEqual(['@modelcontextprotocol/client']); + expect(changes[0]!.notes?.[0]).toContain('packages/a'); + }); + + it('does not note a member whose imports map to no publishable v2 package', () => { + const root = writeJson('package.json', { + name: 'mono', + workspaces: ['packages/*'], + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + const member = writeJson('packages/a/package.json', { name: 'a' }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[member, new Set(['@modelcontextprotocol/core-internal'])]]), false); + expect(changes).toHaveLength(1); + expect(changes[0]!.packageJsonPath).toBe(root); + expect(changes[0]!.notes).toBeUndefined(); + }); +}); + +describe('review-round hardening', () => { + it('ownerManifest tolerates mixed path separators (ts-morph emits forward slashes)', () => { + const manifests = [{ dir: 'C:\\repo\\packages\\a', path: 'C:\\repo\\packages\\a\\package.json' }]; + const owner = ownerManifest('C:/repo/packages/a/src/index.ts', manifests); + expect(owner).toBe(manifests[0]); + }); + + it('parses pnpm-workspace.yaml entries with inline comments', () => { + writeJson('package.json', { name: 'mono' }); + writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*' # the libs\n"); + const a = writeJson('packages/a/package.json', { name: 'a' }); + expect(discoverManifests(dir).map(m => m.path)).toContain(a); + }); + + it('does not warn on a zod disjunction with a satisfying alternative', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: '^3.25.0 || ^4.5.0' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings).toBeUndefined(); + }); + + it('describes the compile-time symptom for zod 4.0/4.1 ranges', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: '~4.1.0' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('TS2769'); + }); + + it('describes both symptom paths for zod-3 ranges', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: '^3.25.0' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('zod/v4 subpath'); + expect(changes[0]!.warnings?.[0]).toContain('tools/list'); + }); + + it('does not warn on open-ended floors and caret-4 ranges that can resolve past the floor', () => { + for (const range of ['>=4.0.0', '>=3', '^4.1.5', '4.x', '~4', '>=4.0 <5', '<=4.2', '3.25.0 - 4.5.0']) { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: range } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings, range).toBeUndefined(); + } + }); + + it('uses the zod-3 narrative for upper bounds that cap resolution below 4', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: '<4.0' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('tools/list'); + expect(changes[0]!.warnings?.[0]).not.toContain('TS2769'); + }); + + it('uses the compile-time narrative for hyphen ranges resolving into 4.0/4.1', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: '3.25.0 - 4.1.99' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('TS2769'); + }); + + it('warns on comparator and workspace-protocol ranges below the floor', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.0.0', zod: '>=3 <4' } }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('zod'); + }); + + it('honors a relative --ignore pattern during collection', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + mkdirSync(path.join(dir, 'src/legacy'), { recursive: true }); + writeFileSync( + path.join(dir, 'src/index.ts'), + "import { Client } from '@modelcontextprotocol/sdk/client/index.js';\nexport { Client };\n" + ); + writeFileSync( + path.join(dir, 'src/legacy/old.ts'), + "import { Client } from '@modelcontextprotocol/sdk/client/index.js';\nexport { Client };\n" + ); + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: true, ignore: ['src/legacy/**'] }); + const touched = result.fileResults.map(fr => fr.filePath); + expect(touched.some(f => f.includes('src/index.ts'))).toBe(true); + expect(touched.some(f => f.includes('legacy'))).toBe(false); + }); + + it('counts a vi.doMock specifier toward manifest additions', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + mkdirSync(path.join(dir, 'test'), { recursive: true }); + writeFileSync(path.join(dir, 'test/mocked.test.ts'), "vi.doMock('@modelcontextprotocol/server', () => ({}));\nexport {};\n"); + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: true }); + expect(result.packageJsonChanges?.[0]?.added).toContain('@modelcontextprotocol/server'); + }); +}); + +describe('single-file targets and mcp import counts (B2)', () => { + it('scopes the run to one file and reports (without applying) the owning manifest edit', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + writeFileSync( + path.join(dir, 'src/a.ts'), + "import { Client } from '@modelcontextprotocol/sdk/client/index.js';\nexport { Client };\n" + ); + writeFileSync( + path.join(dir, 'src/b.ts'), + "import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';\nexport { McpServer };\n" + ); + + const result = run(v1ToV2Migration, { targetDir: path.join(dir, 'src/a.ts'), dryRun: false }); + + expect(result.fileResults.map(fr => fr.filePath)).toHaveLength(1); + expect(readFileSync(path.join(dir, 'src/a.ts'), 'utf8')).toContain('@modelcontextprotocol/client'); + // The sibling stays untouched. + expect(readFileSync(path.join(dir, 'src/b.ts'), 'utf8')).toContain('@modelcontextprotocol/sdk/server/mcp.js'); + expect(result.packageJsonChanges?.[0]?.removed).toEqual(['@modelcontextprotocol/sdk']); + // Report-only in file mode: a one-file view must not strip the dependency the + // rest of the package still needs. + expect((readJson(path.join(dir, 'package.json')).dependencies as Record)['@modelcontextprotocol/sdk']).toBe( + '^1.29.0' + ); + }); + + it('reports how many files already import the v2 packages', () => { + writeJson('package.json', { name: 'app' }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + writeFileSync(path.join(dir, 'src/done.ts'), "import { Client } from '@modelcontextprotocol/client';\nexport { Client };\n"); + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: true }); + expect(result.mcpImportFiles).toBe(1); + expect(result.filesChanged).toBe(0); + }); + + it('reports zero mcp import files for a tree that never used the SDK', () => { + writeJson('package.json', { name: 'app' }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + writeFileSync(path.join(dir, 'src/other.ts'), 'export const x = 1;\n'); + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: true }); + expect(result.mcpImportFiles).toBe(0); + }); +}); + +describe('zod warning relevance gate (B4, #40)', () => { + it('suppresses the zod note when the swap removes the dep and adds nothing', () => { + writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '^3.25.0' } + }); + const changes = updatePackageJson(discoverManifests(dir), new Map(), false); + expect(changes[0]!.removed).toEqual(['@modelcontextprotocol/sdk']); + expect(changes[0]!.added).toEqual([]); + expect(changes[0]!.warnings).toBeUndefined(); + }); + + it('suppresses warning-only entries for manifests with no MCP relation', () => { + writeJson('package.json', { name: 'app', dependencies: { zod: '^3.25.0' } }); + const changes = updatePackageJson(discoverManifests(dir), new Map(), false); + expect(changes).toHaveLength(0); + }); +}); + +describe('advisory-only diagnostics at the runner level (B3)', () => { + it('keeps advisories on first runs (file changed by imports) and drops them on re-runs', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + const file = path.join(dir, 'src/server.ts'); + writeFileSync( + file, + [ + `import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`, + `const inputSchema = mySchema;`, + `const server = new McpServer({ name: 'x', version: '1' });`, + `server.registerTool('t', { inputSchema }, cb);`, + '' + ].join('\n') + ); + + // First run: the imports transform changes the file, so the verify-advisory ships. + const first = run(v1ToV2Migration, { targetDir: dir, dryRun: false }); + expect(first.diagnostics.some(d => d.message.includes('Shorthand'))).toBe(true); + + // Second run: nothing changes — the advisory stays quiet. + const second = run(v1ToV2Migration, { targetDir: dir, dryRun: false }); + expect(second.filesChanged).toBe(0); + expect(second.diagnostics.some(d => d.message.includes('Shorthand'))).toBe(false); + }); +}); + +describe('zod added for injection in zod-less manifests (B6, #53)', () => { + it('adds zod as devDependency when injection fired only in test files', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + const manifests = discoverManifests(dir); + const testFile = path.join(dir, 'test', 'unit', 'a.test.ts'); + const changes = updatePackageJson( + manifests, + new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), + false, + new Map([[manifests[0]!.path, [testFile]]]) + ); + expect(changes[0]!.added).toContain('zod'); + const written = JSON.parse(readFileSync(manifests[0]!.path, 'utf8')); + expect(written.devDependencies.zod).toBe('^4.2.0'); + }); + + it('adds zod to the v1 section for source-file injection', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + const manifests = discoverManifests(dir); + const srcFile = path.join(dir, 'src', 'server.ts'); + const changes = updatePackageJson( + manifests, + new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), + false, + new Map([[manifests[0]!.path, [srcFile]]]) + ); + expect(changes[0]!.added).toContain('zod'); + const written = JSON.parse(readFileSync(manifests[0]!.path, 'utf8')); + expect(written.dependencies.zod).toBe('^4.2.0'); + }); + + it('does not add zod when the manifest already declares it', () => { + writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '^4.3.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson( + manifests, + new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), + false, + new Map([[manifests[0]!.path, [path.join(dir, 'src', 'a.ts')]]]) + ); + expect(changes[0]!.added).not.toContain('zod'); + }); +}); + +describe('zod injection roll-up and path classification (B6 review)', () => { + it('rolls injected files up to the v1-declaring ancestor for hoisted members', () => { + writeJson('package.json', { + name: 'root', + workspaces: ['packages/*'], + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + mkdirSync(path.join(dir, 'packages', 'api', 'src'), { recursive: true }); + writeJson('packages/api/package.json', { name: 'api' }); + const manifests = discoverManifests(dir); + const memberPath = manifests.find(m => m.path.includes('api'))!.path; + const rootPath = manifests.find(m => !m.path.includes('api'))!.path; + const srcFile = path.join(dir, 'packages', 'api', 'src', 'server.ts'); + const changes = updatePackageJson( + manifests, + new Map([[memberPath, new Set(['@modelcontextprotocol/server'])]]), + false, + new Map([[memberPath, [srcFile]]]) + ); + const rootChange = changes.find(c => c.packageJsonPath === rootPath); + expect(rootChange!.added).toContain('zod'); + }); + + it('classifies test paths relative to the package, not the checkout', () => { + // The temp dir itself contains no test segment; simulate one by nesting. + const nested = path.join(dir, 'tests', 'build', 'repo'); + mkdirSync(path.join(nested, 'src'), { recursive: true }); + writeFileSync( + path.join(nested, 'package.json'), + JSON.stringify({ name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }) + ); + const manifests = discoverManifests(nested); + const srcFile = path.join(nested, 'src', 'server.ts'); + const changes = updatePackageJson( + manifests, + new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), + false, + new Map([[manifests[0]!.path, [srcFile]]]) + ); + expect(changes[0]!.added).toContain('zod'); + const written = JSON.parse(readFileSync(manifests[0]!.path, 'utf8')); + expect(written.dependencies.zod).toBe('^4.2.0'); + expect(written.devDependencies?.zod).toBeUndefined(); + }); +}); + +describe('zod range upper bounds at the major boundary', () => { + it('accepts inclusive bare-major upper bounds that admit 4.x', () => { + writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '<=4' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings).toBeUndefined(); + }); + + it('accepts hyphen ranges whose upper bound is a bare 4', () => { + writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '3.25.0 - 4' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings).toBeUndefined(); + }); + + it('still warns on exclusive <4 upper bounds', () => { + writeJson('package.json', { + name: 'app', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0', zod: '>=3.25 <4' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('floor'); + }); +}); + +describe('injected zod in manifests that never declared the v1 SDK', () => { + it('adds and applies zod for the nearest non-v1 manifest', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/server': '^2.0.0-alpha.3' } }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson( + manifests, + new Map(), + false, + new Map([[manifests[0]!.path, [path.join(dir, 'src', 'server.ts')]]]) + ); + expect(changes).toHaveLength(1); + expect(changes[0]!.added).toEqual(['zod']); + expect(changes[0]!.applied).toBe(true); + const written = JSON.parse(readFileSync(manifests[0]!.path, 'utf8')); + expect(written.dependencies.zod).toBe('^4.2.0'); + }); + + it('reports without writing in dry-run mode', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/server': '^2.0.0-alpha.3' } }); + const manifests = discoverManifests(dir); + const before = readFileSync(manifests[0]!.path, 'utf8'); + const changes = updatePackageJson( + manifests, + new Map(), + true, + new Map([[manifests[0]!.path, [path.join(dir, 'src', 'server.ts')]]]) + ); + expect(changes[0]!.added).toEqual(['zod']); + expect(readFileSync(manifests[0]!.path, 'utf8')).toBe(before); + }); +}); + +describe('absolute --ignore glob patterns (review round 3)', () => { + it('honors absolute patterns containing wildcards', () => { + writeJson('package.json', { name: 'app', dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } }); + mkdirSync(path.join(dir, 'src', 'legacy'), { recursive: true }); + writeFileSync(path.join(dir, 'src', 'index.ts'), `import { Client } from '@modelcontextprotocol/sdk/client/index.js';\n`); + writeFileSync(path.join(dir, 'src', 'legacy', 'old.ts'), `import { Client } from '@modelcontextprotocol/sdk/client/index.js';\n`); + + const result = run(v1ToV2Migration, { targetDir: dir, dryRun: true, ignore: [path.join(dir, 'src', 'legacy', '**')] }); + const touched = result.fileResults.map(fileResult => fileResult.filePath); + expect(touched.some(filePath => filePath.endsWith('src/index.ts'))).toBe(true); + expect(touched.some(filePath => filePath.includes('legacy'))).toBe(false); + }); +}); + +describe('workspace member discovery scope', () => { + it('ignores workspace globs that resolve outside the target root', () => { + const outside = mkdtempSync(path.join(tmpdir(), 'mcp-codemod-outside-')); + try { + mkdirSync(path.join(outside, 'stray'), { recursive: true }); + writeFileSync(path.join(outside, 'stray', 'package.json'), JSON.stringify({ name: 'stray' })); + mkdirSync(path.join(dir, 'packages', 'api'), { recursive: true }); + writeJson('package.json', { + name: 'root', + workspaces: ['packages/*', path.join(outside, '*')], + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' } + }); + writeJson('packages/api/package.json', { name: 'api' }); + + const manifests = discoverManifests(dir); + const dirs = manifests.map(manifest => manifest.dir); + expect(dirs.some(memberDir => memberDir.includes('packages/api'))).toBe(true); + expect(dirs.some(memberDir => memberDir.includes('stray'))).toBe(false); + } finally { + rmSync(outside, { recursive: true, force: true }); + } + }); +}); + +describe('peer-declared zod (review)', () => { + it('warns on a peer-only zod range below the floor', () => { + writeJson('package.json', { + name: 'lib', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' }, + peerDependencies: { zod: '^3.25.0' } + }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson(manifests, new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), false); + expect(changes[0]!.warnings?.[0]).toContain('floor'); + }); + + it('does not add a direct zod dependency when zod is declared as a peer', () => { + writeJson('package.json', { + name: 'lib', + dependencies: { '@modelcontextprotocol/sdk': '^1.29.0' }, + peerDependencies: { zod: '^4.2.0' } + }); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + const manifests = discoverManifests(dir); + const changes = updatePackageJson( + manifests, + new Map([[manifests[0]!.path, new Set(['@modelcontextprotocol/server'])]]), + false, + new Map([[manifests[0]!.path, [path.join(dir, 'src', 'server.ts')]]]) + ); + expect(changes[0]!.added).not.toContain('zod'); + const written = JSON.parse(readFileSync(manifests[0]!.path, 'utf8')); + expect(written.dependencies.zod).toBeUndefined(); + expect(written.devDependencies?.zod).toBeUndefined(); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/completableNesting.test.ts b/packages/codemod/test/v1-to-v2/transforms/completableNesting.test.ts new file mode 100644 index 0000000000..b8bce346fb --- /dev/null +++ b/packages/codemod/test/v1-to-v2/transforms/completableNesting.test.ts @@ -0,0 +1,138 @@ +import { Project } from 'ts-morph'; +import { describe, expect, it } from 'vitest'; + +import { completableNestingTransform } from '../../../src/migrations/v1-to-v2/transforms/completableNesting'; +import type { TransformContext } from '../../../src/types'; +import { DiagnosticLevel } from '../../../src/types'; + +const ctx: TransformContext = { projectType: 'server' }; + +function apply(code: string): { text: string; result: ReturnType } { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = completableNestingTransform.apply(sourceFile, ctx); + return { text: sourceFile.getFullText(), result }; +} + +const IMPORT = `import { completable } from '@modelcontextprotocol/server';\n`; + +describe('completable optional-nesting inversion', () => { + it('hoists a postfix .optional() outside the completable call', () => { + const { text, result } = apply(IMPORT + `const arg = completable(z.string().optional(), cb);\n`); + expect(text).toContain('completable(z.string(), cb).optional()'); + expect(result.changesCount).toBe(1); + expect(result.diagnostics.some(d => d.level === DiagnosticLevel.Info)).toBe(true); + }); + + it('hoists the z.optional(inner) factory form', () => { + const { text, result } = apply(IMPORT + `const arg = completable(z.optional(z.string()), cb);\n`); + expect(text).toContain('completable(z.string(), cb).optional()'); + expect(result.changesCount).toBe(1); + }); + + it('handles an aliased completable import', () => { + const code = `import { completable as c } from '@modelcontextprotocol/server';\nconst arg = c(z.string().optional(), cb);\n`; + const { text } = apply(code); + expect(text).toContain('c(z.string(), cb).optional()'); + }); + + it('handles namespace-qualified calls', () => { + const code = `import * as mcp from '@modelcontextprotocol/server';\nconst arg = mcp.completable(z.string().optional(), cb);\n`; + const { text } = apply(code); + expect(text).toContain('mcp.completable(z.string(), cb).optional()'); + }); + + it('still matches the v1 specifier when run in isolation', () => { + const code = `import { completable } from '@modelcontextprotocol/sdk/server/completable.js';\nconst arg = completable(z.string().optional(), cb);\n`; + const { text } = apply(code); + expect(text).toContain('completable(z.string(), cb).optional()'); + }); + + it('leaves the already-correct nesting untouched', () => { + const code = IMPORT + `const arg = completable(z.string(), cb).optional();\n`; + const { text, result } = apply(code); + expect(text).toBe(code); + expect(result.changesCount).toBe(0); + }); + + it('leaves plain (non-optional) schema arguments untouched', () => { + const code = IMPORT + `const arg = completable(z.enum(['a', 'b']), cb);\n`; + const { text, result } = apply(code); + expect(text).toBe(code); + expect(result.changesCount).toBe(0); + expect(result.diagnostics).toEqual([]); + }); + + it('flags .nullish() with the concrete nullable+optional rewrite', () => { + const { text, result } = apply(IMPORT + `const arg = completable(z.string().nullish(), cb);\n`); + expect(text).toContain('completable(z.string().nullish(), cb)'); + expect(result.changesCount).toBe(0); + const diag = result.diagnostics.find(d => d.insertComment); + expect(diag?.message).toContain('completable(schema.nullable(), cb).optional()'); + }); + + it('leaves wrappers that keep working in v2 (.default) untouched with no diagnostics', () => { + const code = IMPORT + `const arg = completable(z.enum(['a', 'b']).default('a'), cb);\n`; + const { text, result } = apply(code); + expect(text).toBe(code); + expect(result.changesCount).toBe(0); + expect(result.diagnostics).toEqual([]); + }); + + it('flags an optional buried below the chain tail (.optional().describe())', () => { + const code = IMPORT + `const arg = completable(z.string().optional().describe('country'), cb);\n`; + const { text, result } = apply(code); + expect(text).toBe(code); + const diag = result.diagnostics.find(d => d.insertComment); + expect(diag?.message).toContain('inside its method chain'); + }); + + it('flags a factory-form optional buried below the chain tail', () => { + const code = IMPORT + `const arg = completable(z.optional(z.string()).meta({ a: 1 }), cb);\n`; + const { result } = apply(code); + expect(result.diagnostics.some(d => d.insertComment)).toBe(true); + }); + + it('notes a by-reference schema argument once per file without a marker', () => { + const code = IMPORT + `const arg1 = completable(schemaA, cb);\nconst arg2 = completable(schemaB, cb);\n`; + const { result } = apply(code); + const warnings = result.diagnostics.filter(d => d.level === DiagnosticLevel.Warning); + expect(warnings).toHaveLength(1); + expect(warnings[0]!.insertComment).toBeUndefined(); + expect(warnings[0]!.message).toContain('by reference'); + }); + + it('ignores completable from non-MCP modules', () => { + const code = `import { completable } from 'other-lib';\nconst arg = completable(z.string().optional(), cb);\n`; + const { text, result } = apply(code); + expect(text).toBe(code); + expect(result.changesCount).toBe(0); + }); + + it('rewrites multiple calls in one file', () => { + const code = IMPORT + `const a = completable(z.string().optional(), cbA);\nconst b = completable(z.number().optional(), cbB);\n`; + const { text, result } = apply(code); + expect(text).toContain('completable(z.string(), cbA).optional()'); + expect(text).toContain('completable(z.number(), cbB).optional()'); + expect(result.changesCount).toBe(2); + }); + + it('unwraps parentheses around the first argument', () => { + const { text } = apply(IMPORT + `const arg = completable((z.string().optional()), cb);\n`); + expect(text).toContain('completable(z.string(), cb).optional()'); + }); +}); + +describe('opaque schema note flag', () => { + it('marks the by-reference schema note advisory-only', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [`import { completable } from '@modelcontextprotocol/server';`, `completable(citySchema, complete);`, ''].join('\n') + ); + const result = completableNestingTransform.apply(sourceFile, { projectType: 'server' }); + const note = result.diagnostics.find(d => d.message.includes('by reference')); + expect(note).toBeDefined(); + expect(note?.advisoryOnly).toBe(true); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/contextTypes.test.ts b/packages/codemod/test/v1-to-v2/transforms/contextTypes.test.ts index 039fcf559d..4b41562c1e 100644 --- a/packages/codemod/test/v1-to-v2/transforms/contextTypes.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/contextTypes.test.ts @@ -215,7 +215,7 @@ describe('context-types transform', () => { '' ].join('\n'); const result = applyTransform(input); - expect(result).toContain('ctx.mcpReq.signal'); + expect(result).toContain('ctx?.mcpReq.signal'); expect(result).not.toContain('extra'); }); @@ -382,3 +382,258 @@ describe('context-types transform', () => { expect(result).not.toContain('extra'); }); }); + +describe('trailing context parameter selection (B2)', () => { + it('processes the 3-parameter registerResource template callback', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerResource('r', template, {}, async (uri, variables, extra) => {`, + ` const s = extra.signal;`, + ` return { contents: [] };`, + `});`, + '' + ].join('\n') + ); + contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text).toContain('(uri, variables, ctx)'); + expect(text).toContain('ctx.mcpReq.signal'); + }); + + it('does not flag a destructured middle parameter when the trailing context is already named ctx', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerResource('r', tpl, {}, async (uri, { id }, ctx) => ({ contents: [] }));`, + '' + ].join('\n') + ); + const result = contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + expect(result.diagnostics.some(d => d.message.includes('Destructuring of context parameter'))).toBe(false); + }); + + it('still processes the 2-parameter form via index fallback', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerTool('t', {}, async (args, extra) => {`, + ` const s = extra.signal;`, + ` return { content: [] };`, + `});`, + '' + ].join('\n') + ); + contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text).toContain('(args, ctx)'); + expect(text).toContain('ctx.mcpReq.signal'); + }); +}); + +describe('destructured trailing parameter key gate (B4)', () => { + it('does not flag template-variable destructures', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerResource('r', tpl, {}, async (uri, { owner, repo }) => ({ contents: [] }));`, + '' + ].join('\n') + ); + const result = contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + expect(result.diagnostics.some(d => d.message.includes('Destructuring of context parameter'))).toBe(false); + }); + + it('flags renamed context destructures and requestInfo', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerTool('a', {}, async (args, { signal: abort }) => ({ content: [] }));`, + `server.registerTool('b', {}, async (args, { requestInfo }) => ({ content: [] }));`, + '' + ].join('\n') + ); + const result = contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + const flagged = result.diagnostics.filter(d => d.message.includes('Destructuring of context parameter')); + expect(flagged).toHaveLength(2); + }); + + it('still flags destructures carrying context-like keys', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerTool('t', {}, async (args, { signal, sessionId }) => ({ content: [] }));`, + '' + ].join('\n') + ); + const result = contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + expect(result.diagnostics.some(d => d.message.includes('Destructuring of context parameter'))).toBe(true); + }); +}); + +describe('fallback handler and annotated context params (B5, #152)', () => { + function apply(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = contextTypesTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + const IMPORT = `import { McpServer } from '@modelcontextprotocol/server';\n`; + + it('remaps fallbackRequestHandler assignments', () => { + const code = + IMPORT + + [ + `server.fallbackRequestHandler = async (request, extra) => {`, + ` extra.sendNotification(note);`, + ` return { ok: true };`, + `};`, + '' + ].join('\n'); + const { text } = apply(code); + expect(text).toContain('(request, ctx)'); + expect(text).toContain('ctx.mcpReq.notify(note)'); + }); + + it('remaps accesses on parameters annotated with a context type alias', () => { + const code = + IMPORT + + [ + `type ToolExtra = ServerContext;`, + `class Host {`, + ` private screenParty(args: unknown, extra: ToolExtra) {`, + ` extra.sendRequest(req, schema);`, + ` return extra.authInfo;`, + ` }`, + `}`, + '' + ].join('\n'); + const { text } = apply(code); + expect(text).toContain('extra.mcpReq.send(req, schema)'); + expect(text).toContain('extra.http?.authInfo'); + }); + + it('remaps direct ServerContext annotations on free functions', () => { + const code = IMPORT + [`function helper(extra: ServerContext) {`, ` return extra.signal;`, `}`, ''].join('\n'); + const { text } = apply(code); + expect(text).toContain('extra.mcpReq.signal'); + }); + + it('does not remap shadowed inner parameters with the same name', () => { + const code = + IMPORT + + [`function helper(extra: ServerContext) {`, ` return items.map((extra: ItemMeta) => extra.signal);`, `}`, ''].join('\n'); + const { text } = apply(code); + expect(text).toContain('=> extra.signal'); + expect(text).not.toContain('mcpReq'); + }); + + it('resolves alias-of-alias annotations and nullable widening', () => { + const code = + IMPORT + + [ + `type A = ServerContext;`, + `type B = A;`, + `function h1(extra: B) { return extra.signal; }`, + `function h2(extra: ServerContext | undefined) { return extra?.signal; }`, + '' + ].join('\n'); + const { text } = apply(code); + expect(text).toContain('function h1(extra: B) { return extra.mcpReq.signal; }'); + expect(text).toContain('return extra?.mcpReq.signal;'); + }); + + it('accepts direct aliases whose generic arguments contain unions', () => { + const code = [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `type Extra = RequestHandlerExtra;`, + `function h(extra: Extra) {`, + ` return extra.signal;`, + `}`, + '' + ].join('\n'); + const { text } = apply(code); + expect(text).toContain('extra.mcpReq.signal'); + }); + + it('does not remap intersection aliases involving a context type', () => { + const code = [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `type Augmented = ServerContext & { tenant: string };`, + `function h(extra: Augmented) {`, + ` return extra.signal;`, + `}`, + '' + ].join('\n'); + const { text, result } = apply(code); + expect(text).toContain('return extra.signal;'); + expect(result.diagnostics.some(d => d.message.includes('cannot remap'))).toBe(true); + }); + + it('does not remap wrapper aliases that merely mention a context type', () => { + const code = [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `type ToolCallContext = { mcp: ServerContext; signal: AbortSignal };`, + `function run(extra: ToolCallContext) {`, + ` return extra.signal;`, + `}`, + '' + ].join('\n'); + const { text, result } = apply(code); + expect(text).toContain('return extra.signal;'); + expect(text).not.toContain('mcpReq'); + const advisory = result.diagnostics.find(d => d.message.includes('cannot remap')); + expect(advisory).toBeDefined(); + expect(advisory?.advisoryOnly).toBe(true); + }); + + it('advises on context-mentioning annotations it cannot remap', () => { + const code = IMPORT + [`function pool(extra: Map) { return extra.size; }`, ''].join('\n'); + const { text, result } = apply(code); + expect(text).toContain('return extra.size;'); + const diag = result.diagnostics.find(d => d.message.includes('cannot remap')); + expect(diag).toBeDefined(); + expect(diag?.advisoryOnly).toBe(true); + }); + + it('preserves optional chaining in processed callbacks', () => { + const code = + IMPORT + + [ + `server.registerTool('t', {}, async (args, extra) => {`, + ` return { content: [], note: extra?.sessionId };`, + `});`, + '' + ].join('\n'); + const { text } = apply(code); + expect(text).toContain('ctx?.sessionId'); + }); + + it('notes contexts forwarded wholesale to helpers', () => { + const code = + IMPORT + + [ + `server.registerTool('t', {}, async (args, extra) => {`, + ` if (!hasScope(extra)) throw new Error('denied');`, + ` return { content: [] };`, + `});`, + '' + ].join('\n'); + const { result } = apply(code); + const diag = result.diagnostics.find(d => d.message.includes('forwarded to hasScope')); + expect(diag).toBeDefined(); + expect(diag?.advisoryOnly).toBe(true); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/handlerRegistration.test.ts b/packages/codemod/test/v1-to-v2/transforms/handlerRegistration.test.ts index 443814d7a5..b29f1cefcb 100644 --- a/packages/codemod/test/v1-to-v2/transforms/handlerRegistration.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/handlerRegistration.test.ts @@ -262,3 +262,135 @@ describe('handler-registration transform', () => { expect(result.diagnostics.length).toBe(0); }); }); + +describe('custom and schema-derived handler arguments (B4)', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = handlerRegistrationTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('marks a setNotificationHandler whose first argument is a schema expression', () => { + const code = [ + `import { Server } from '@modelcontextprotocol/sdk/server/index.js';`, + `server.setNotificationHandler(schemas.SelectionChanged, handler);`, + '' + ].join('\n'); + const { result } = applyWithDiagnostics(code); + const diag = result.diagnostics.find(d => d.insertComment && d.message.includes('setNotificationHandler')); + expect(diag).toBeDefined(); + expect(diag?.message).toContain('three-argument custom form'); + expect(diag?.advisoryOnly).toBe(true); + }); + + it('leaves template-literal method arguments alone (valid v2 form)', () => { + const code = [ + `import { Server } from '@modelcontextprotocol/sdk/server/index.js';`, + 'server.setRequestHandler(`${NS}/search`, { params: P }, handler);', + 'client.removeNotificationHandler(`notifications/${kind}`);', + '' + ].join('\n'); + const { result } = applyWithDiagnostics(code); + expect(result.diagnostics).toHaveLength(0); + }); + + it('leaves string-method setNotificationHandler calls alone', () => { + const code = [ + `import { Server } from '@modelcontextprotocol/sdk/server/index.js';`, + `server.setNotificationHandler('notifications/custom', { params: P }, handler);`, + '' + ].join('\n'); + const { result } = applyWithDiagnostics(code); + expect(result.diagnostics).toHaveLength(0); + }); + + it('rewrites removeRequestHandler(Schema.shape.method.value) to the method string', () => { + const code = [ + `import { PingRequestSchema } from '@modelcontextprotocol/sdk/types.js';`, + `server.removeRequestHandler(PingRequestSchema.shape.method.value);`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain(`removeRequestHandler('ping')`); + expect(text).not.toContain('PingRequestSchema'); + }); + + it('marks removeNotificationHandler with an unresolvable schema-derived argument', () => { + const code = [ + `import { Server } from '@modelcontextprotocol/sdk/server/index.js';`, + `server.removeNotificationHandler(localSchema.shape.method.value);`, + '' + ].join('\n'); + const { result } = applyWithDiagnostics(code); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('method string'))).toBe(true); + }); +}); + +describe('same-file identifier schema resolution (B6, #30)', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = handlerRegistrationTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('resolves a local const alias of a spec schema to the method string', () => { + const code = [ + `import { ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const S = ListToolsRequestSchema;`, + `server.setRequestHandler(S, handler);`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).toContain(`setRequestHandler('tools/list', handler)`); + expect(result.diagnostics.some(d => d.message.includes('Custom method handler'))).toBe(false); + }); + + it('still marks local variables that do not resolve to a spec schema', () => { + const code = [ + `import { Server } from '@modelcontextprotocol/sdk/server/index.js';`, + `const S = myCustomSchema;`, + `server.setRequestHandler(S, handler);`, + '' + ].join('\n'); + const { result } = applyWithDiagnostics(code); + expect(result.diagnostics.some(d => d.message.includes('Custom method handler'))).toBe(true); + }); +}); + +describe('variable-hop guard rails (B6 review)', () => { + function applyB6(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = handlerRegistrationTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('routes local aliases of removed task schemas to the removal guidance', () => { + const code = [ + `import { ListTasksRequestSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const T = ListTasksRequestSchema;`, + `server.setRequestHandler(T, handler);`, + '' + ].join('\n'); + const { result } = applyB6(code); + expect(result.diagnostics.some(d => d.message.includes('removed in v2 (SEP-2663)'))).toBe(true); + expect(result.diagnostics.some(d => d.message.includes('Custom method handler'))).toBe(false); + }); + + it('keeps the custom marker when the name is shadowed', () => { + const code = [ + `import { ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const S = ListToolsRequestSchema;`, + `function inner() {`, + ` const S = myCustomSchema;`, + ` server.setRequestHandler(S, handler);`, + `}`, + '' + ].join('\n'); + const { text, result } = applyB6(code); + expect(text).not.toContain(`setRequestHandler('tools/list'`); + expect(result.diagnostics.some(d => d.message.includes('Custom method handler'))).toBe(true); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts b/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts index 4cd43967c0..ee5d889cee 100644 --- a/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts @@ -991,3 +991,155 @@ describe('import-paths transform', () => { }); }); }); + +describe('auth types routing (B3)', () => { + it('routes AuthInfo from server/auth/types.js by context, not to server-legacy', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + `import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js';\nimport { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';\n` + ); + importPathsTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text).toContain('@modelcontextprotocol/server'); + expect(text).not.toContain('server-legacy'); + }); +}); + +describe('symbols with no v2 export (removedSymbols)', () => { + function applyWithDiagnostics(code: string, context: TransformContext = { projectType: 'server' }) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = importPathsTransform.apply(sourceFile, context); + return { text: sourceFile.getFullText(), result }; + } + + it('drops Protocol from a shared/protocol.js import and flags it', () => { + const input = `import { Protocol } from '@modelcontextprotocol/sdk/shared/protocol.js';\n`; + const { text, result } = applyWithDiagnostics(input); + expect(text).not.toContain('Protocol }'); + expect(text).not.toContain('@modelcontextprotocol/sdk'); + const diag = result.diagnostics.find(d => d.insertComment && d.message.includes('Protocol base class')); + expect(diag).toBeDefined(); + expect(diag?.message).toContain('fallbackRequestHandler'); + }); + + it('routes surviving siblings while dropping the removed symbol', () => { + const input = `import { Protocol, type ProtocolOptions } from '@modelcontextprotocol/sdk/shared/protocol.js';\n`; + const { text, result } = applyWithDiagnostics(input); + expect(text).toContain('ProtocolOptions'); + expect(text).toContain('@modelcontextprotocol/server'); + expect(text).not.toMatch(/\bProtocol\b(?!Options)/); + expect(result.diagnostics.some(d => d.message.includes('Protocol base class'))).toBe(true); + }); + + it('flags mergeCapabilities with spread guidance', () => { + const input = `import { mergeCapabilities } from '@modelcontextprotocol/sdk/shared/protocol.js';\n`; + const { result } = applyWithDiagnostics(input); + const diag = result.diagnostics.find(d => d.message.includes('mergeCapabilities')); + expect(diag).toBeDefined(); + expect(diag?.message).toContain('object spread'); + }); + + it('does not resolve a context package for an import of only removed symbols', () => { + const input = `import { Protocol } from '@modelcontextprotocol/sdk/shared/protocol.js';\n`; + const { result } = applyWithDiagnostics(input, { projectType: 'unknown' }); + // projectType 'unknown' would emit a could-not-determine warning if context were resolved. + expect(result.diagnostics.some(d => d.message.includes('could not determine'))).toBe(false); + }); + + it('flags qualified accesses to removed symbols on a namespace import', () => { + const input = `import * as proto from '@modelcontextprotocol/sdk/shared/protocol.js';\nclass Mine extends proto.Protocol {}\n`; + const { text, result } = applyWithDiagnostics(input); + expect(text).toContain('@modelcontextprotocol/server'); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('Protocol base class'))).toBe(true); + }); + + it('warns on a named re-export of Protocol with module-scoped guidance', () => { + const input = `export { Protocol } from '@modelcontextprotocol/sdk/shared/protocol.js';\n`; + const { result } = applyWithDiagnostics(input); + const diag = result.diagnostics.find(d => d.message.includes('Re-exported Protocol has no v2 export')); + expect(diag).toBeDefined(); + expect(diag?.message).toContain('fallbackRequestHandler'); + }); + + it('flags a star re-export of a module with removed symbols', () => { + const input = `export * from '@modelcontextprotocol/sdk/shared/protocol.js';\n`; + const { result } = applyWithDiagnostics(input); + const messages = result.diagnostics.map(d => d.message); + expect(messages.some(m => m.includes('Star re-export') && m.includes('Protocol'))).toBe(true); + expect(messages.some(m => m.includes('Star re-export') && m.includes('mergeCapabilities'))).toBe(true); + }); + + it('flags type-position namespace accesses (QualifiedName) to removed symbols', () => { + const input = `import * as proto from '@modelcontextprotocol/sdk/shared/protocol.js';\nexport function f(p: proto.Protocol): void {}\n`; + const { text, result } = applyWithDiagnostics(input); + expect(text).toContain('@modelcontextprotocol/server'); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('Protocol base class'))).toBe(true); + }); + + it('anchors the dropped-symbol marker to a usage site that survives the rewrite', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + `import { Protocol } from '@modelcontextprotocol/sdk/shared/protocol.js';\n\nexport class Mine extends Protocol {}\n` + ); + const result = importPathsTransform.apply(sourceFile, { projectType: 'server' }); + const diag = result.diagnostics.find(d => d.message.includes('Protocol base class')); + expect(diag).toBeDefined(); + // resolveCurrentLine resolves against the live usage node, not the removed import. + const finalLine = + sourceFile + .getFullText() + .split('\n') + .findIndex(l => l.includes('extends Protocol')) + 1; + expect(diag?.resolveCurrentLine?.()).toBe(finalLine); + }); +}); + +describe('RS-only auth helper markers (B2)', () => { + it('marks requireBearerAuth routed to server-legacy/auth', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + `import { requireBearerAuth } from '@modelcontextprotocol/sdk/server/auth/middleware.js';\n` + ); + const result = importPathsTransform.apply(sourceFile, { projectType: 'server' }); + const diag = result.diagnostics.find(d => d.insertComment && d.message.includes('requireBearerAuth')); + expect(diag).toBeDefined(); + expect(diag?.message).toContain('@modelcontextprotocol/express'); + expect(diag?.message).toContain('OAuthError'); + }); + + it('downgrades type-only RS imports to an info note', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + `import type { OAuthTokenVerifier } from '@modelcontextprotocol/sdk/server/auth/provider.js';\n` + ); + const result = importPathsTransform.apply(sourceFile, { projectType: 'server' }); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('OAuthTokenVerifier'))).toBe(false); + expect(result.diagnostics.some(d => d.message.includes('type-only import'))).toBe(true); + }); + + it('marks RS helpers re-exported from a barrel', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + `export { requireBearerAuth } from '@modelcontextprotocol/sdk/server/auth/middleware.js';\n` + ); + const result = importPathsTransform.apply(sourceFile, { projectType: 'server' }); + const diag = result.diagnostics.find(d => d.insertComment && d.message.includes('Re-exported requireBearerAuth')); + expect(diag).toBeDefined(); + }); + + it('does not mark AS-only helpers routed to server-legacy/auth', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + `import { mcpAuthRouter } from '@modelcontextprotocol/sdk/server/auth/router.js';\n` + ); + const result = importPathsTransform.apply(sourceFile, { projectType: 'server' }); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('@modelcontextprotocol/express'))).toBe(false); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/mcpServerApi.test.ts b/packages/codemod/test/v1-to-v2/transforms/mcpServerApi.test.ts index c9392aa830..52a17e2779 100644 --- a/packages/codemod/test/v1-to-v2/transforms/mcpServerApi.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/mcpServerApi.test.ts @@ -301,7 +301,7 @@ describe('mcp-server-api transform', () => { expect(result).not.toContain('z.object(z.object('); }); - it('emits diagnostic for variable-valued schema in config', () => { + it('flags the variable-valued schema advisory as advisory-only', () => { const input = [ `const promptArgsSchema = { city: z.string() };`, `server.registerPrompt("args-prompt", { argsSchema: promptArgsSchema }, (args) => {`, @@ -312,25 +312,21 @@ describe('mcp-server-api transform', () => { const project = new Project({ useInMemoryFileSystem: true }); const sourceFile = project.createSourceFile('test.ts', MCP_IMPORT + input); const result = mcpServerApiTransform.apply(sourceFile, ctx); - const text = sourceFile.getFullText(); - expect(text).toContain('argsSchema: promptArgsSchema'); - expect(text).not.toContain('z.object(promptArgsSchema)'); - expect(result.diagnostics.some(d => d.message.includes('not an object literal'))).toBe(true); + const advisory = result.diagnostics.find(d => d.message.includes('Could not verify')); + expect(advisory).toBeDefined(); + // The runner drops advisory-only diagnostics for files no transform changed, + // so re-runs over migrated trees stay quiet while first runs keep them. + expect(advisory?.advisoryOnly).toBe(true); }); - it('emits diagnostic for shorthand schema property in config', () => { - const input = [ - `server.registerTool("echo", { inputSchema }, async ({ msg }) => {`, - ` return { content: [{ type: 'text', text: msg }] };`, - `});`, - '' - ].join('\n'); + it('flags the shorthand schema advisory as advisory-only', () => { + const input = [`const inputSchema = mySchema;`, `server.registerTool("t", { inputSchema }, cb);`, ''].join('\n'); const project = new Project({ useInMemoryFileSystem: true }); const sourceFile = project.createSourceFile('test.ts', MCP_IMPORT + input); const result = mcpServerApiTransform.apply(sourceFile, ctx); - const text = sourceFile.getFullText(); - expect(text).toContain('{ inputSchema }'); - expect(result.diagnostics.some(d => d.message.includes('Shorthand'))).toBe(true); + const advisory = result.diagnostics.find(d => d.message.includes('Shorthand')); + expect(advisory).toBeDefined(); + expect(advisory?.advisoryOnly).toBe(true); }); it('leaves .registerTool() without inputSchema unchanged', () => { @@ -404,3 +400,289 @@ describe('mcp-server-api transform', () => { expect(result.diagnostics).toHaveLength(0); }); }); + +describe('zod import injection for wrapped shapes (sweep rollup)', () => { + it('adds the zod import when wrapping in a file without a z binding', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `server.registerTool('t', { inputSchema: { name: nameSchema } }, cb);`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text).toContain('z.object('); + expect(text).toContain(`import { z } from "zod"`); + expect(result.diagnostics.some(d => d.message.includes('Added `import { z }'))).toBe(true); + }); + + it('marks a non-import z binding instead of redeclaring it', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `const { z } = require('zod');`, + `server.registerTool('t', { inputSchema: { name: nameSchema } }, cb);`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, { projectType: 'server' }); + expect(sourceFile.getFullText()).not.toContain(`import { z } from "zod"`); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('not a value import from zod'))).toBe(true); + }); + + it('does not treat a type-only z import as a usable binding', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `import type { z as zt } from 'zod';`, + `server.registerTool('t', { inputSchema: { name: nameSchema } }, cb);`, + '' + ].join('\n') + ); + mcpServerApiTransform.apply(sourceFile, { projectType: 'server' }); + expect(sourceFile.getFullText()).toContain(`import { z } from "zod"`); + }); + + it('does not add a second import when z is already bound', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/server';`, + `import * as z from 'zod/v4';`, + `server.registerTool('t', { inputSchema: { name: z.string() } }, cb);`, + '' + ].join('\n') + ); + mcpServerApiTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text.match(/from ['"]zod/g)?.length).toBe(1); + }); +}); + +describe('nested and harness registrations (B4)', () => { + it('migrates a registration nested inside another handler without crashing', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`, + `server.tool('outer', { x: z.string() }, async (args) => {`, + ` server.tool('inner', { y: z.number() }, async () => ({ content: [] }));`, + ` return { content: [] };`, + `});`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(result.changesCount).toBeGreaterThan(0); + expect(text).toContain(`registerTool('outer'`); + expect(text).toContain(`registerTool('inner'`); + expect(text).not.toContain('server.tool('); + }); + + it('migrates legacy calls on property-access receivers without a direct McpServer import', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { Client } from '@modelcontextprotocol/sdk/client/index.js';`, + `harness.mcp.tool('t', { x: z.string() }, async () => ({ content: [] }));`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + expect(sourceFile.getFullText()).toContain(`registerTool('t'`); + expect(result.changesCount).toBeGreaterThan(0); + }); + + it('migrates cross-category nesting (a .prompt() inside a .tool() handler)', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`, + `server.tool('outer', { x: z.string() }, async () => {`, + ` server.prompt('greet', { name: z.string() }, cb);`, + ` return { content: [] };`, + `});`, + '' + ].join('\n') + ); + mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(text).toContain(`registerTool('outer'`); + expect(text).toContain(`registerPrompt('greet'`); + expect(text).not.toContain('server.prompt('); + }); + + it('stays silent on shape-mismatched calls in fallback mode (2-arg .resource())', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [`import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, `router.resource('users', usersController);`, ''].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + expect(sourceFile.getFullText()).toContain(`router.resource('users', usersController)`); + expect(result.diagnostics.some(d => d.message.includes('Could not automatically migrate'))).toBe(false); + }); + + it('does not rewrite shape-identical non-MCP calls when the receiver type is unknown', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, + `const name = await cli.prompt('What is your name?', validateName);`, + `app.resource('users', '/api/users', usersHandler);`, + `registry.tool('hammer', { weight: 2 }, describeTool);`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(text).toContain(`cli.prompt('What is your name?', validateName)`); + expect(text).toContain(`app.resource('users', '/api/users', usersHandler)`); + expect(text).toContain(`registry.tool('hammer'`); + expect(text).not.toContain('register'); + expect(result.diagnostics).toHaveLength(0); + }); + + it('does not rewrite members hanging off a server-named receiver', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, + `this.server.cli.prompt('What is your name?', validateName);`, + `ctx.server.app.resource('users', '/api/users', usersHandler);`, + `observer.tool('t', { x: z.string() }, cb);`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(text).toContain(`this.server.cli.prompt('What is your name?', validateName)`); + expect(text).toContain(`ctx.server.app.resource('users'`); + expect(text).toContain(`observer.tool('t'`); + expect(result.diagnostics).toHaveLength(0); + }); + + it('migrates wrapped and word-named server receivers without a direct McpServer import', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, + `this.server!.tool('a', { x: z.string() }, cb);`, + `mockServer.prompt('b', cb);`, + `(testServer as any).resource('c', 'uri://c', readCb);`, + '' + ].join('\n') + ); + mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(text).toContain(`registerTool('a'`); + expect(text).toContain(`registerPrompt('b'`); + expect(text).toContain(`registerResource('c'`); + }); + + it('does not wrap register* schemas on non-MCP receivers in fallback mode', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, + `registry.registerTool('hammer', { inputSchema: { weight: z.number() } }, describeTool);`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(text).toContain('inputSchema: { weight: z.number() }'); + expect(text).not.toContain('z.object'); + expect(result.diagnostics).toHaveLength(0); + }); + + it('still wraps register* schemas on mcp-named receivers in fallback mode', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, + `mockServer.registerTool('t', { inputSchema: { a: z.string() } }, cb);`, + '' + ].join('\n') + ); + mcpServerApiTransform.apply(sourceFile, ctx); + expect(sourceFile.getFullText()).toContain('inputSchema: z.object({ a: z.string() })'); + }); + + it('still migrates mcp-named receivers without a direct McpServer import', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import type { Tool } from '@modelcontextprotocol/sdk/types.js';`, + `this.mcpServer.tool('a', { x: z.string() }, cb);`, + `server.prompt('b', cb);`, + '' + ].join('\n') + ); + mcpServerApiTransform.apply(sourceFile, ctx); + const text = sourceFile.getFullText(); + expect(text).toContain(`registerTool('a'`); + expect(text).toContain(`registerPrompt('b'`); + }); + + it('stays silent on non-matching .tool() shapes when the receiver type is unknown', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [`import { Client } from '@modelcontextprotocol/sdk/client/index.js';`, `toolbox.tool('hammer');`, ''].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + expect(sourceFile.getFullText()).toContain(`toolbox.tool('hammer')`); + expect(result.diagnostics.some(d => d.message.includes('Could not automatically migrate'))).toBe(false); + }); +}); + +describe('mock call-shape assertions (B6, #41)', () => { + it('notes objectContaining assertions pinning a registration schema', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`, + `expect(register).toHaveBeenCalledWith('t', expect.objectContaining({ inputSchema: { x: expectAny } }), cb);`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + const diag = result.diagnostics.find(d => d.message.includes('Call-shape assertion')); + expect(diag).toBeDefined(); + expect(diag?.advisoryOnly).toBe(true); + }); + + it('ignores objectContaining without schema keys', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`, + `expect(fn).toHaveBeenCalledWith(expect.objectContaining({ title: 'x' }));`, + '' + ].join('\n') + ); + const result = mcpServerApiTransform.apply(sourceFile, ctx); + expect(result.diagnostics.some(d => d.message.includes('Call-shape assertion'))).toBe(false); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/mockPaths.test.ts b/packages/codemod/test/v1-to-v2/transforms/mockPaths.test.ts index ed613ddc2f..9776135d09 100644 --- a/packages/codemod/test/v1-to-v2/transforms/mockPaths.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/mockPaths.test.ts @@ -569,3 +569,35 @@ describe('mock-paths transform', () => { }); }); }); + +describe('removed symbols in mocks and dynamic imports', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = mockPathsTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('leaves a mock factory providing Protocol unrewritten and flags it', () => { + const input = `vi.mock('@modelcontextprotocol/sdk/shared/protocol.js', () => ({ Protocol: class {} }));\n`; + const { text, result } = applyWithDiagnostics(input); + expect(text).toContain('@modelcontextprotocol/sdk/shared/protocol.js'); + const diag = result.diagnostics.find(d => d.insertComment); + expect(diag?.message).toContain('Protocol'); + expect(diag?.message).toContain('no v2 package exports'); + }); + + it('leaves a dynamic import destructuring Protocol unrewritten and flags it', () => { + const input = `const { Protocol } = await import('@modelcontextprotocol/sdk/shared/protocol.js');\nexport { Protocol };\n`; + const { text, result } = applyWithDiagnostics(input); + expect(text).toContain('@modelcontextprotocol/sdk/shared/protocol.js'); + const diag = result.diagnostics.find(d => d.insertComment); + expect(diag?.message).toContain('undefined at runtime'); + }); + + it('still rewrites protocol.js mocks that only touch surviving symbols', () => { + const input = `vi.mock('@modelcontextprotocol/sdk/shared/protocol.js', () => ({ ProtocolOptions: {} }));\nimport '@modelcontextprotocol/sdk/server/mcp.js';\n`; + const { text } = applyWithDiagnostics(input); + expect(text).toContain(`vi.mock('@modelcontextprotocol/server'`); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/removedApis.test.ts b/packages/codemod/test/v1-to-v2/transforms/removedApis.test.ts index bf1001978a..eb65c0311c 100644 --- a/packages/codemod/test/v1-to-v2/transforms/removedApis.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/removedApis.test.ts @@ -268,3 +268,125 @@ describe('removed-apis transform', () => { }); }); }); + +describe('finishAuth advisory (B2)', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + return removedApisTransform.apply(sourceFile, { projectType: 'client' }); + } + + it('notes a single-argument finishAuth call without a marker', () => { + const code = [`import { Client } from '@modelcontextprotocol/client';`, `await transport.finishAuth(code);`, ''].join('\n'); + const result = applyWithDiagnostics(code); + const diag = result.diagnostics.find(d => d.message.includes('finishAuth')); + expect(diag).toBeDefined(); + // A run-log note, not a marker — the 1-arg URLSearchParams form is valid v2. + expect(diag?.insertComment).toBeUndefined(); + expect(diag?.message).toContain('iss'); + }); + + it('leaves a two-argument finishAuth call alone', () => { + const code = [`import { Client } from '@modelcontextprotocol/client';`, `await transport.finishAuth(code, iss);`, ''].join('\n'); + const result = applyWithDiagnostics(code); + expect(result.diagnostics.some(d => d.message.includes('finishAuth'))).toBe(false); + }); + + it('ignores finishAuth in files with no MCP imports', () => { + const result = applyWithDiagnostics(`await other.finishAuth(code);\n`); + expect(result.diagnostics.some(d => d.message.includes('finishAuth'))).toBe(false); + }); +}); + +describe('SdkHttpError constructor marker (B2)', () => { + it('emits an insertComment diagnostic at each constructor site', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js';`, + `throw new StreamableHTTPError(404, 'not found');`, + '' + ].join('\n') + ); + const result = removedApisTransform.apply(sourceFile, { projectType: 'client' }); + const ctorDiag = result.diagnostics.find(d => d.message.includes('Constructor arguments differ')); + expect(ctorDiag?.insertComment).toBe(true); + }); +}); + +describe('finishAuth advisory flag', () => { + it('marks the one-argument finishAuth note advisory-only so re-runs stay quiet', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [`import { Client } from '@modelcontextprotocol/client';`, `await provider.finishAuth(params);`, ''].join('\n') + ); + const result = removedApisTransform.apply(sourceFile, { projectType: 'client' }); + const note = result.diagnostics.find(d => d.message.includes('finishAuth with one argument')); + expect(note).toBeDefined(); + expect(note?.advisoryOnly).toBe(true); + }); +}); + +describe('guarded .code to .status rewrites (B5, #155)', () => { + function apply(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = removedApisTransform.apply(sourceFile, { projectType: 'client' }); + return { text: sourceFile.getFullText(), result }; + } + const IMPORT = `import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js';\n`; + + it('rewrites .code in the same boolean expression as the instanceof check', () => { + const { text } = apply(IMPORT + `if (failure instanceof StreamableHTTPError && failure.code === 404) { retry(); }\n`); + expect(text).toContain('instanceof SdkHttpError && failure.status === 404'); + expect(text).not.toContain('failure.code'); + }); + + it('rewrites .code reads inside the guarded then-block', () => { + const code = + IMPORT + + [`if (err instanceof StreamableHTTPError) {`, ` if (err.code >= 500) backoff();`, ` log(err.code);`, `}`, ''].join('\n'); + const { text } = apply(code); + expect(text).toContain('err.status >= 500'); + expect(text).toContain('log(err.status)'); + }); + + it('leaves unguarded .code reads alone', () => { + const { text } = apply(IMPORT + `classify(error.code);\n`); + expect(text).toContain('classify(error.code)'); + }); + + it('does not rewrite under negated guards', () => { + const { text } = apply(IMPORT + `if (!(err instanceof StreamableHTTPError)) { classify(err.code); }\n`); + expect(text).toContain('classify(err.code)'); + }); + + it('does not rewrite the other operand of a disjunction', () => { + const { text } = apply(IMPORT + `const retriable = err instanceof StreamableHTTPError || err.code === 'ECONNRESET';\n`); + expect(text).toContain(`err.code === 'ECONNRESET'`); + }); + + it('does not rewrite then-blocks reached through a disjunction', () => { + const { text } = apply(IMPORT + `if (err instanceof StreamableHTTPError || isTimeout(err)) { log(err.code); }\n`); + expect(text).toContain('log(err.code)'); + }); + + it('does not rewrite shadowed same-name variables in the guarded block', () => { + const code = IMPORT + [`if (err instanceof StreamableHTTPError) {`, ` items.forEach(err => log(err.code));`, `}`, ''].join('\n'); + const { text } = apply(code); + expect(text).toContain('log(err.code)'); + }); + + it('skips the whole then-block when the subject is reassigned inside it', () => { + const code = IMPORT + [`if (e instanceof StreamableHTTPError) {`, ` e = unwrap(e);`, ` use(e.code);`, `}`, ''].join('\n'); + const { text } = apply(code); + expect(text).toContain('use(e.code)'); + }); + + it('does not touch other subjects in the same expression', () => { + const { text } = apply(IMPORT + `if (a instanceof StreamableHTTPError && b.code === 404) handle();\n`); + expect(text).toContain('b.code === 404'); + }); +}); diff --git a/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts b/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts index 93a52ecaea..27492389d8 100644 --- a/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts @@ -161,6 +161,46 @@ describe('schema-param-removal transform', () => { expect(result).toContain('someHttpClient.request(payload, undefined, { timeout: 5000 })'); }); + it('does not corrupt a non-SDK .request() helper taking a bare-string method', () => { + const input = [ + `import { Client } from '@modelcontextprotocol/client';`, + `await end.request('ping', undefined, 'fence-after-cancel');`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain(`end.request('ping', undefined, 'fence-after-cancel')`); + }); + + it('keeps an explicit undefined on request() when the method is not provably a spec literal', () => { + const input = [ + `import { Client } from '@modelcontextprotocol/client';`, + `await client.request(reqVar, undefined, { timeout: 5000 });`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('client.request(reqVar, undefined, { timeout: 5000 })'); + }); + + it('does not strip undefined from a callTool() whose first argument is a primitive', () => { + const input = [ + `import { Client } from '@modelcontextprotocol/client';`, + `await runner.callTool('add', undefined, retries);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain(`runner.callTool('add', undefined, retries)`); + }); + + it('does not strip undefined from a callTool() whose first argument is a template with substitutions', () => { + const input = [ + `import { Client } from '@modelcontextprotocol/client';`, + 'await runner.callTool(`${ns}/add`, undefined, retries);', + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('undefined, retries'); + }); + it('leaves a 2-arg callTool(params, undefined) unchanged (already valid as options in v2)', () => { const input = [`await client.callTool({ name: 'add' }, undefined);`, ''].join('\n'); const result = applyTransform(input); diff --git a/packages/codemod/test/v1-to-v2/transforms/symbolRenames.test.ts b/packages/codemod/test/v1-to-v2/transforms/symbolRenames.test.ts index c877d029fa..5ab0ea8178 100644 --- a/packages/codemod/test/v1-to-v2/transforms/symbolRenames.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/symbolRenames.test.ts @@ -498,3 +498,425 @@ describe('symbol-renames transform', () => { expect(result).toContain('export { Foo as McpError }'); }); }); + +describe('ErrorCode split — instanceof pairing (B2)', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('rewrites the paired instanceof class to SdkError and imports it', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `if (e instanceof McpError && e.code === ErrorCode.RequestTimeout) retry();`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('instanceof SdkError'); + expect(text).toContain('SdkErrorCode.RequestTimeout'); + expect(text).toMatch(/import \{[^}]*SdkError[^}]*\}/); + }); + + it('stays silent for an SdkErrorCode comparison with no instanceof guard', () => { + const code = [ + `import { ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `if (e.code === ErrorCode.ConnectionClosed) reconnect();`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).toContain('SdkErrorCode.ConnectionClosed'); + expect(result.diagnostics.some(d => d.insertComment)).toBe(false); + }); + + it('stays silent for switch cases over SDK members', () => { + const code = [ + `import { ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `switch (e.code) { case ErrorCode.RequestTimeout: retry(); }`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).toContain('case SdkErrorCode.RequestTimeout'); + expect(result.diagnostics.some(d => d.insertComment)).toBe(false); + }); + + it('marks a mixed SDK/protocol guard instead of rewriting it', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `if (e instanceof McpError && (e.code === ErrorCode.ConnectionClosed || e.code === ErrorCode.ParseError)) handle();`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).not.toContain('instanceof SdkError'); + expect(text).toContain('SdkErrorCode.ConnectionClosed'); + expect(text).toContain('ProtocolErrorCode.ParseError'); + const diag = result.diagnostics.find(d => d.insertComment); + expect(diag?.message).toContain('Split the check'); + }); + + it('does not claim guards stored through assignments', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `const isProto = e instanceof McpError;`, + `if (e.code === ErrorCode.RequestTimeout) retry();`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).not.toContain('instanceof SdkError'); + }); + + it('rewrites the guard of an all-SDK two-member condition without markers', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `if (e instanceof McpError && (e.code === ErrorCode.RequestTimeout || e.code === ErrorCode.ConnectionClosed)) retry();`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).toContain('instanceof SdkError'); + expect(text).not.toContain('instanceof McpError'); + expect(result.diagnostics.some(d => d.insertComment)).toBe(false); + }); + + it('stays silent for bare member uses outside comparisons', () => { + const code = [`import { ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, `const t = ErrorCode.RequestTimeout;`, ''].join( + '\n' + ); + const { result } = applyWithDiagnostics(code); + expect(result.diagnostics.some(d => d.insertComment)).toBe(false); + }); + + it('leaves ProtocolError untouched for ProtocolErrorCode members', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `if (e instanceof McpError && e.code === ErrorCode.InvalidParams) reject();`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('ProtocolErrorCode.InvalidParams'); + expect(text).not.toContain('instanceof SdkError'); + }); +}); + +describe('matcher and constructor pairing (B3)', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('rewrites a toBeInstanceOf paired with an SDK-code matcher on the same subject', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('times out', () => {`, + ` expect(err).toBeInstanceOf(McpError);`, + ` expect(err.code).toBe(ErrorCode.RequestTimeout);`, + `});`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('toBeInstanceOf(SdkError)'); + expect(text).toContain('SdkErrorCode.RequestTimeout'); + expect(text).toMatch(/import \{[^}]*SdkError[^}]*\}/); + }); + + it('marks a mixed-subject toBeInstanceOf instead of rewriting it', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('mixed', () => {`, + ` expect(err).toBeInstanceOf(McpError);`, + ` expect(err.code).toBe(ErrorCode.RequestTimeout);`, + ` expect(err.code).toBe(ErrorCode.ParseError);`, + `});`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).not.toContain('toBeInstanceOf(SdkError)'); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('Split the assertions'))).toBe(true); + }); + + it('does not touch a toBeInstanceOf whose subject has no SDK-code assertion', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('proto', () => {`, + ` expect(other).toBeInstanceOf(McpError);`, + ` expect(err.code).toBe(ErrorCode.RequestTimeout);`, + `});`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('toBeInstanceOf(ProtocolError)'); + }); + + it('moves the constructor class with an SDK-routed code argument', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `throw new McpError(ErrorCode.ConnectionClosed, 'closed');`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('new SdkError(SdkErrorCode.ConnectionClosed'); + expect(text).not.toContain('new ProtocolError'); + }); + + it('leaves constructors with protocol codes on ProtocolError', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `throw new McpError(ErrorCode.InvalidParams, 'bad');`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('new ProtocolError(ProtocolErrorCode.InvalidParams'); + }); +}); + +describe('pairing redesign (B3 review)', () => { + function applyWithDiagnostics(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('pairs cast subjects: expect((err as any).code)', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('t', () => {`, + ` expect(err).toBeInstanceOf(McpError);`, + ` expect((err as any).code).toBe(ErrorCode.RequestTimeout);`, + `});`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('toBeInstanceOf(SdkError)'); + }); + + it('does not pair an assertion about a different property (err.cause)', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('t', () => {`, + ` expect(err.cause).toBeInstanceOf(McpError);`, + ` expect(err.code).toBe(ErrorCode.RequestTimeout);`, + `});`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('toBeInstanceOf(ProtocolError)'); + expect(text).not.toContain('toBeInstanceOf(SdkError)'); + }); + + it('marks a constructor whose code argument mixes both enums', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `throw new McpError(isTimeout ? ErrorCode.RequestTimeout : ErrorCode.InvalidRequest, 'm');`, + '' + ].join('\n'); + const { text, result } = applyWithDiagnostics(code); + expect(text).not.toContain('new SdkError'); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('Split the construction'))).toBe(true); + }); + + it('ignores SDK members outside the constructor first argument', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `throw new McpError(ErrorCode.InvalidRequest, 'x', { hint: ErrorCode.RequestTimeout });`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('new ProtocolError(ProtocolErrorCode.InvalidRequest'); + }); + + it('removes the stranded error-class import after a sole-use constructor rewrite', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `throw new McpError(ErrorCode.ConnectionClosed, 'closed');`, + '' + ].join('\n'); + const { text } = applyWithDiagnostics(code); + expect(text).toContain('new SdkError('); + expect(text).not.toMatch(/import \{[^}]*ProtocolError\b[^}]*\} from/); + }); + + it('notes unpaired class matchers in files with SDK-routed codes', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('t', async () => {`, + ` await expect(p).rejects.toMatchObject({ code: ErrorCode.RequestTimeout });`, + ` expect(somethingElse).toBeInstanceOf(McpError);`, + `});`, + '' + ].join('\n'); + const { result } = applyWithDiagnostics(code); + expect(result.diagnostics.some(d => d.message.includes('review those assertions'))).toBe(true); + }); +}); + +describe('ErrorCode passthrough imports (sweep rollup)', () => { + it('drops an ErrorCode import with no member accesses and marks the remaining use', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [`import { ErrorCode } from '@modelcontextprotocol/server';`, `registerCodes(ErrorCode);`, ''].join('\n') + ); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text).not.toContain('import { ErrorCode }'); + const diag = result.diagnostics.find(d => d.insertComment); + expect(diag?.message).toContain('ProtocolErrorCode'); + expect(diag?.message).toContain('SdkErrorCode'); + }); + + it('leaves a still-v1 ErrorCode import alone in isolated runs', () => { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile( + 'test.ts', + [ + `import { ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `export function f(code: ErrorCode): boolean { return retryable(code); }`, + '' + ].join('\n') + ); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + expect(sourceFile.getFullText()).toContain(`from '@modelcontextprotocol/sdk/types.js'`); + expect(result.diagnostics.some(d => d.message.includes('not exported by the v2 packages'))).toBe(false); + }); +}); + +describe('cast repointing and dynamic-import bindings (B6)', () => { + function applyB6(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + + it('re-points stale as-casts when the pairing moves the asserted class', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('t', async () => {`, + ` const err = (await settled) as McpError;`, + ` expect(err).toBeInstanceOf(McpError);`, + ` expect(err.code).toBe(ErrorCode.RequestTimeout);`, + `});`, + '' + ].join('\n'); + const { text } = applyB6(code); + expect(text).toContain('(await settled) as SdkError'); + expect(text).toContain('toBeInstanceOf(SdkError)'); + }); + + it('leaves casts alone when the pairing does not move the class', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('t', async () => {`, + ` const err = (await settled) as McpError;`, + ` expect(err.code).toBe(ErrorCode.InvalidRequest);`, + `});`, + '' + ].join('\n'); + const { text } = applyB6(code); + expect(text).toContain('as ProtocolError'); + }); + + it('renames shorthand dynamic-import destructure bindings and references', () => { + const code = [ + `import { Client } from '@modelcontextprotocol/sdk/client/index.js';`, + `async function load() {`, + ` const { McpError } = await import('@modelcontextprotocol/sdk/types.js');`, + ` throw new McpError(1, 'x');`, + `}`, + '' + ].join('\n'); + const { text } = applyB6(code); + expect(text).toContain('const { ProtocolError }'); + expect(text).toContain('new ProtocolError(1'); + }); + + it('re-points only the property name for aliased destructures', () => { + const code = [ + `import { Client } from '@modelcontextprotocol/sdk/client/index.js';`, + `const { McpError: LocalError } = await import('@modelcontextprotocol/sdk/types.js');`, + `throw new LocalError(1, 'x');`, + '' + ].join('\n'); + const { text } = applyB6(code); + expect(text).toContain('{ ProtocolError: LocalError }'); + expect(text).toContain('new LocalError(1'); + }); + + it('ignores destructures of non-SDK dynamic imports', () => { + const code = [ + `import { Client } from '@modelcontextprotocol/sdk/client/index.js';`, + `const { McpError } = await import('./local-errors');`, + '' + ].join('\n'); + const { text } = applyB6(code); + expect(text).toContain(`const { McpError } = await import('./local-errors')`); + }); +}); + +describe('assignment-cast repointing (B6 review)', () => { + it('re-points casts assigned in catch blocks when the matcher moves the class', () => { + const code = [ + `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';`, + `it('t', async () => {`, + ` let err: any;`, + ` try { await op(); } catch (e) { err = e as McpError; }`, + ` expect(err).toBeInstanceOf(McpError);`, + ` expect(err.code).toBe(ErrorCode.RequestTimeout);`, + `});`, + '' + ].join('\n'); + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + const text = sourceFile.getFullText(); + expect(text).toContain('err = e as SdkError'); + expect(text).toContain('toBeInstanceOf(SdkError)'); + }); +}); + +describe('guard polarity in the ErrorCode split (review round 3)', () => { + function applyR3(code: string) { + const project = new Project({ useInMemoryFileSystem: true }); + const sourceFile = project.createSourceFile('test.ts', code); + const result = symbolRenamesTransform.apply(sourceFile, { projectType: 'server' }); + return { text: sourceFile.getFullText(), result }; + } + const IMP = `import { McpError, ErrorCode } from '@modelcontextprotocol/sdk/types.js';\n`; + + it('marks guards joined to the SDK code by a disjunction instead of rewriting', () => { + const { text, result } = applyR3(IMP + `if (e instanceof McpError || e.code === ErrorCode.RequestTimeout) retry();\n`); + expect(text).toContain('e instanceof ProtocolError || e.code === SdkErrorCode.RequestTimeout'); + expect(text).not.toContain('SdkError '); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('does not pin'))).toBe(true); + }); + + it('rewrites negated guards conjoined with a same-subject SDK code', () => { + const { text, result } = applyR3(IMP + `if (!(e instanceof McpError) && e.code === ErrorCode.RequestTimeout) bail();\n`); + expect(text).toContain('!(e instanceof SdkError) && e.code === SdkErrorCode.RequestTimeout'); + expect(result.diagnostics.some(d => d.insertComment)).toBe(false); + }); + + it('rewrites guards conjoined inside a disjunct', () => { + const { text } = applyR3(IMP + `const retriable = (e instanceof McpError && e.code === ErrorCode.RequestTimeout) || isAbort(e);\n`); + expect(text).toContain('(e instanceof SdkError && e.code === SdkErrorCode.RequestTimeout)'); + }); + + it('marks guards whose conjoined SDK code is on another subject or in a nested function', () => { + const code = + IMP + + `if ((e instanceof McpError && retries.every(r => r.code === ErrorCode.RequestTimeout)) || e.code === ErrorCode.ConnectionClosed) requeue(e);\n`; + const { text, result } = applyR3(code); + expect(text).toContain('e instanceof ProtocolError &&'); + expect(text).not.toContain('instanceof SdkError'); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('does not pin'))).toBe(true); + }); + + it('marks guards conjoined with a negated code comparison', () => { + const { text, result } = applyR3(IMP + `if (e instanceof McpError && e.code !== ErrorCode.RequestTimeout) propagate(e);\n`); + expect(text).toContain('e instanceof ProtocolError &&'); + expect(result.diagnostics.some(d => d.insertComment && d.message.includes('does not pin'))).toBe(true); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 256377b6ca..84a2dcbd5e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1284,6 +1284,9 @@ importers: commander: specifier: ^13.0.0 version: 13.1.0 + fast-glob: + specifier: ^3.3.3 + version: 3.3.3 ts-morph: specifier: ^28.0.0 version: 28.0.0