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