fixes

Author: KKonstantinov

docs/migration-SKILL.md

@@ -90,6 +90,7 @@ Notes:
 | `isJSONRPCError`                            | `isJSONRPCErrorResponse`                                                                                                                                         |
 | `isJSONRPCResponse` (deprecated in v1)      | `isJSONRPCResultResponse` (**not** v2's new `isJSONRPCResponse`, which correctly matches both result and error)                                                  |
 | `JSONRPCResponseSchema` (result-only in v1) | `JSONRPCResultResponseSchema` (from `@modelcontextprotocol/core`; **not** v2's new `JSONRPCResponseSchema`, a `z.union` that also accepts error responses) |
+| `JSONRPCResponse` (type, result-only in v1) | `JSONRPCResultResponse` (**not** v2's new `JSONRPCResponse` type, which is the result\|error union)                                                              |
 | `ResourceReference`                         | `ResourceTemplateReference`                                                                                                                                      |
 | `ResourceReferenceSchema`                   | `ResourceTemplateReferenceSchema`                                                                                                                                |
 | `IsomorphicHeaders`                         | REMOVED (use Web Standard `Headers`)                                                                                                                             |

docs/migration.md

@@ -610,6 +610,9 @@ All other symbols exported from `@modelcontextprotocol/sdk/types.js` retain thei
 > **Note on `JSONRPCResponseSchema`:** the Zod schema follows the same pattern. v1's `JSONRPCResponseSchema` validated only _result_ responses; v2 reuses the name for a `z.union([JSONRPCResultResponseSchema, JSONRPCErrorResponseSchema])` that also accepts error responses. If you
 > are migrating v1 code that called `JSONRPCResponseSchema.parse()`/`.safeParse()`, rename it to `JSONRPCResultResponseSchema` (re-exported by `@modelcontextprotocol/core`) to preserve the original validation. The codemod performs this rename automatically.
 
+> **Note on the `JSONRPCResponse` type:** the TypeScript type follows the same pattern. v1's `JSONRPCResponse` was the _result-only_ response type; v2 reuses the name for the result\|error union (`Infer<typeof JSONRPCResponseSchema>`). If you are migrating v1 code that annotated values with
+> `JSONRPCResponse`, rename it to `JSONRPCResultResponse` (re-exported by `@modelcontextprotocol/client` and `@modelcontextprotocol/server`) to preserve the original narrower type. The codemod performs this rename automatically.
+
 **Before (v1):**
 
 ```typescript

packages/codemod/src/migrations/v1-to-v2/mappings/symbolMap.ts

@@ -4,12 +4,14 @@ export const SIMPLE_RENAMES: Record<string, string> = {
     JSONRPCErrorSchema: 'JSONRPCErrorResponseSchema',
     isJSONRPCError: 'isJSONRPCErrorResponse',
     isJSONRPCResponse: 'isJSONRPCResultResponse',
-    // v1's JSONRPCResponseSchema validated only *result* responses. v2 reuses the name for a
-    // z.union([JSONRPCResultResponseSchema, JSONRPCErrorResponseSchema]) that also accepts error
-    // responses, so a migrated `JSONRPCResponseSchema.parse(...)` would silently widen. Rename to the
-    // result-only schema to preserve v1 behavior — mirroring the isJSONRPCResponse guard rename above.
-    // (The TYPE JSONRPCResponse/JSONRPCResultResponse is not part of the public v2 surface, so only the
-    // schema constant — re-exported by core — is renamed here.)
+    // v1's JSONRPCResponse type / JSONRPCResponseSchema constant both validated only *result*
+    // responses. v2 reuses each name for the result|error form — the type becomes
+    // `Infer<z.union([JSONRPCResultResponseSchema, JSONRPCErrorResponseSchema])>` and the schema the
+    // matching `z.union([...])` — so a migrated `JSONRPCResponseSchema.parse(...)` (or a typed
+    // `JSONRPCResponse` value) would silently widen. Rename both to the result-only equivalents to
+    // preserve v1 behavior — mirroring the isJSONRPCResponse guard rename above. Both the type and the
+    // schema are public in v2 (re-exported from core via @modelcontextprotocol/client | /server).
+    JSONRPCResponse: 'JSONRPCResultResponse',
     JSONRPCResponseSchema: 'JSONRPCResultResponseSchema',
     ResourceReference: 'ResourceTemplateReference',
     ResourceReferenceSchema: 'ResourceTemplateReferenceSchema'

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 { symbolTargetOverride } from '../mappings/schemaRouting';
+import { isSharedSchemaConst, resolveRenamedName, symbolTargetOverride } from '../mappings/schemaRouting';
 import { SIMPLE_RENAMES } from '../mappings/symbolMap';
 
 /**
@@ -73,6 +73,7 @@ function resolveTarget(
     specifier: string,
     context: TransformContext,
     sourceFile: SourceFile,
+    symbols: string[],
     diagnosticSink?: { filePath: string; line: number; diagnostics: Diagnostic[] }
 ): { target: string; mapping: ImportMapping } | { removed: true; isV2Gap?: boolean; removalMessage?: string } | null {
     const mapping = lookupImportMapping(specifier);
@@ -93,7 +94,15 @@ function resolveTarget(
             const s = i.getModuleSpecifierValue();
             return s.includes('/server/') || s === '@modelcontextprotocol/server';
         });
-        target = resolveTypesPackage(context, hasClient, hasServer, diagnosticSink);
+        // Resolve lazily: only pass the diagnostic sink to resolveTypesPackage when the routed target
+        // actually falls back to the context package. A factory/destructuring whose symbols all route
+        // elsewhere (e.g. only `*Schema` constants → core) never uses the context package, so emitting a
+        // "could not determine project type" warning (or a 'both'-project info note) for it would be
+        // spurious. Mirrors the lazy `needsContext` guard in the static import transform. A
+        // non-destructured/non-routable binding has no symbols, so `routeSymbols` returns no target and
+        // context is (correctly) treated as needed.
+        const needsContext = routeSymbols(symbols, mapping).target === undefined;
+        target = resolveTypesPackage(context, hasClient, hasServer, needsContext ? diagnosticSink : undefined);
         if (mapping.subpathSuffix) {
             target = `${target}${mapping.subpathSuffix}`;
         }
@@ -121,7 +130,8 @@ function rewriteMockCall(
     const specifier = firstArg.getLiteralValue();
     if (!isSdkSpecifier(specifier)) return 0;
 
-    const resolved = resolveTarget(specifier, context, sourceFile, {
+    const factorySymbols = args.length >= 2 ? collectFactorySymbols(args[1]!) : [];
+    const resolved = resolveTarget(specifier, context, sourceFile, factorySymbols, {
         filePath: sourceFile.getFilePath(),
         line: call.getStartLineNumber(),
         diagnostics
@@ -150,7 +160,7 @@ function rewriteMockCall(
         // only `*Schema` constants (from sdk/types.js or sdk/shared/auth.js) moves to core; a factory
         // of only `StreamableHTTPServerTransport` moves to @modelcontextprotocol/node. A single mock path
         // can't be split, so a mix of packages is flagged for manual migration.
-        const { target: routedTarget, mixed } = routeSymbols(collectFactorySymbols(args[1]!), resolved.mapping);
+        const { target: routedTarget, mixed } = routeSymbols(factorySymbols, resolved.mapping);
         if (routedTarget) {
             effectiveTarget = routedTarget;
         } else if (mixed) {
@@ -241,6 +251,80 @@ function renameSymbolsInFactory(factoryArg: import('ts-morph').Node, renamedSymb
     return changes;
 }
 
+/**
+ * The destructured binding keys of an `await import()` assigned to an object binding pattern (e.g.
+ * `const { CallToolResultSchema, McpError } = await import('…')`), or `[]` for a non-destructured
+ * binding, a `.then()` chain, or an unassigned `await import()`. The keys feed per-symbol routing —
+ * the specifier itself can't be split.
+ */
+function getDestructuredKeys(node: import('ts-morph').CallExpression): string[] {
+    const parent = node.getParent();
+    if (!parent || !Node.isAwaitExpression(parent)) return [];
+    const grandParent = parent.getParent();
+    if (!grandParent || !Node.isVariableDeclaration(grandParent)) return [];
+    const nameNode = grandParent.getNameNode();
+    if (!Node.isObjectBindingPattern(nameNode)) return [];
+    return nameNode.getElements().map(el => el.getPropertyNameNode()?.getText() ?? el.getName());
+}
+
+/**
+ * For a dynamic import whose module binding is NOT a destructurable object pattern — a non-destructured
+ * `const mod = await import('…')` or a `.then(m => …)` chain — collect the Zod schema constants
+ * accessed off that binding (e.g. `mod.OAuthTokensSchema`). The destructured form is routed/renamed
+ * elsewhere; these forms can't be split per-symbol, so the schema accesses are surfaced as a diagnostic
+ * (mirroring the namespace-import branch of the static import transform — see `importPaths.ts`). Returns
+ * deduped `[v1Name, v2Name]` pairs (a schema may be renamed, e.g. JSONRPCResponseSchema →
+ * JSONRPCResultResponseSchema, and core only exports the v2 name). Empty unless the mapping carries a
+ * `schemaSymbolTarget`.
+ */
+function collectModuleSchemaAccesses(
+    node: import('ts-morph').CallExpression,
+    mapping: ImportMapping,
+    sourceFile: SourceFile
+): Array<readonly [string, string]> {
+    if (!mapping.schemaSymbolTarget) return [];
+
+    let bindingName: string | undefined;
+    let scope: import('ts-morph').Node | undefined;
+
+    const parent = node.getParent();
+    if (parent && Node.isAwaitExpression(parent)) {
+        // const mod = await import('…')  → `mod` is in scope for the rest of the file.
+        const grandParent = parent.getParent();
+        if (grandParent && Node.isVariableDeclaration(grandParent)) {
+            const nameNode = grandParent.getNameNode();
+            if (Node.isIdentifier(nameNode)) {
+                bindingName = nameNode.getText();
+                scope = sourceFile;
+            }
+        }
+    } else if (parent && Node.isPropertyAccessExpression(parent) && parent.getName() === 'then') {
+        // import('…').then(m => m.XxxSchema…)  → the module is the `.then` callback's first parameter.
+        const thenCall = parent.getParent();
+        if (thenCall && Node.isCallExpression(thenCall)) {
+            const cb = thenCall.getArguments()[0];
+            if (cb && (Node.isArrowFunction(cb) || Node.isFunctionExpression(cb))) {
+                const paramName = cb.getParameters()[0]?.getNameNode();
+                if (paramName && Node.isIdentifier(paramName)) {
+                    bindingName = paramName.getText();
+                    scope = cb;
+                }
+            }
+        }
+    }
+
+    if (!bindingName || !scope) return [];
+
+    return [
+        ...new Map(
+            scope
+                .getDescendantsOfKind(SyntaxKind.PropertyAccessExpression)
+                .filter(pa => pa.getExpression().getText() === bindingName && isSharedSchemaConst(pa.getName(), mapping))
+                .map(pa => [pa.getName(), resolveRenamedName(pa.getName(), mapping)] as const)
+        )
+    ];
+}
+
 function rewriteDynamicImports(
     sourceFile: SourceFile,
     context: TransformContext,
@@ -264,7 +348,8 @@ function rewriteDynamicImports(
         const specifier = firstArg.getLiteralValue();
         if (!isSdkSpecifier(specifier)) return;
 
-        const resolved = resolveTarget(specifier, context, sourceFile, {
+        const destructuredKeys = getDestructuredKeys(node);
+        const resolved = resolveTarget(specifier, context, sourceFile, destructuredKeys, {
             filePath: sourceFile.getFilePath(),
             line: node.getStartLineNumber(),
             diagnostics
@@ -294,28 +379,39 @@ function rewriteDynamicImports(
         // of only `*Schema` constants (e.g. `const { CallToolResultSchema } = await import('…/types.js')`)
         // moves to core, and `StreamableHTTPServerTransport` moves to @modelcontextprotocol/node. A
         // single import() specifier can't be split, so a mix of packages is flagged for manual migration.
-        const parentExpr = node.getParent();
-        if (parentExpr && Node.isAwaitExpression(parentExpr)) {
-            const grandParent = parentExpr.getParent();
-            if (grandParent && Node.isVariableDeclaration(grandParent)) {
-                const nameNode = grandParent.getNameNode();
-                if (Node.isObjectBindingPattern(nameNode)) {
-                    const keys = nameNode.getElements().map(el => el.getPropertyNameNode()?.getText() ?? el.getName());
-                    const { target: routedTarget, mixed } = routeSymbols(keys, resolved.mapping);
-                    if (routedTarget) {
-                        effectiveTarget = routedTarget;
-                    } else if (mixed) {
-                        diagnostics.push(
-                            actionRequired(
-                                sourceFile.getFilePath(),
-                                node,
-                                `Dynamic import of ${specifier} destructures symbols that belong to different v2 packages. ` +
-                                    `Split the import manually so each symbol targets the correct package.`
-                            )
-                        );
-                    }
-                }
-            }
+        const { target: routedTarget, mixed } = routeSymbols(destructuredKeys, resolved.mapping);
+        if (routedTarget) {
+            effectiveTarget = routedTarget;
+        } else if (mixed) {
+            diagnostics.push(
+                actionRequired(
+                    sourceFile.getFilePath(),
+                    node,
+                    `Dynamic import of ${specifier} destructures symbols that belong to different v2 packages. ` +
+                        `Split the import manually so each symbol targets the correct package.`
+                )
+            );
+        }
+
+        // A non-destructured binding (`const mod = await import('…')`) or a `.then(m => …)` chain can't be
+        // routed per-symbol, so the specifier moves to the context package — which does NOT export the
+        // Zod `*Schema` constants (those live in `schemaSymbolTarget`/core). Any `mod.<Name>Schema` /
+        // `m.<Name>Schema` accesses would silently break, so flag them (mirroring the namespace-import
+        // branch of the static import transform). The destructured form is handled by `routeSymbols` above.
+        const schemaAccesses = collectModuleSchemaAccesses(node, resolved.mapping, sourceFile);
+        if (schemaAccesses.length > 0) {
+            const accessed = schemaAccesses.map(([v1]) => v1).join(', ');
+            const importName = schemaAccesses[0]![1];
+            const renamed = schemaAccesses.filter(([v1, v2]) => v1 !== v2);
+            const renameNote = renamed.length > 0 ? ` Renamed in v2: ${renamed.map(([v1, v2]) => `${v1} → ${v2}`).join(', ')}.` : '';
+            diagnostics.push(
+                actionRequired(
+                    sourceFile.getFilePath(),
+                    node,
+                    `Dynamic import of ${specifier} is used to access Zod schema(s) (${accessed}) that moved to ${resolved.mapping.schemaSymbolTarget}.${renameNote} ` +
+                        `Import them with a named import (e.g. \`import { ${importName} } from '${resolved.mapping.schemaSymbolTarget}'\`) and update the qualified usages.`
+                )
+            );
         }
 
         usedPackages.add(effectiveTarget);

packages/codemod/test/detectFormatter.test.ts

@@ -3,7 +3,7 @@ import { tmpdir } from 'node:os';
 import path from 'node:path';
 import { describe, it, expect, afterEach } from 'vitest';
 
-import { detectFormatter } from '../src/utils/detectFormatter.js';
+import { detectFormatter } from '../src/utils/detectFormatter';
 
 let tempDir: string;
 

packages/codemod/test/projectAnalyzer.test.ts

@@ -202,15 +202,15 @@ describe('analyzeProject', () => {
 
 describe('resolveTypesPackage', () => {
     it('emits an info note (not a warning) for a both-project ambiguous file', () => {
-        const sink = { filePath: 'f.ts', line: 1, diagnostics: [] as import('../src/types.js').Diagnostic[] };
+        const sink = { filePath: 'f.ts', line: 1, diagnostics: [] as import('../src/types').Diagnostic[] };
         const target = resolveTypesPackage({ projectType: 'both' }, false, false, sink);
         expect(target).toBe('@modelcontextprotocol/server');
         expect(sink.diagnostics).toHaveLength(1);
         expect(sink.diagnostics[0]!.level).toBe('info');
     });
 
     it('emits an action-required warning for a genuinely unknown project', () => {
-        const sink = { filePath: 'f.ts', line: 1, diagnostics: [] as import('../src/types.js').Diagnostic[] };
+        const sink = { filePath: 'f.ts', line: 1, diagnostics: [] as import('../src/types').Diagnostic[] };
         resolveTypesPackage({ projectType: 'unknown' }, false, false, sink);
         expect(sink.diagnostics).toHaveLength(1);
         expect(sink.diagnostics[0]!.level).toBe('warning');

packages/codemod/test/v1-to-v2/authSchemaNames.test.ts

@@ -3,7 +3,7 @@ import { fileURLToPath } from 'node:url';
 
 import { describe, expect, it } from 'vitest';
 
-import { AUTH_SCHEMA_NAMES, AUTH_SCHEMA_NAMES_NO_V2_PUBLIC_EXPORT } from '../../src/migrations/v1-to-v2/mappings/authSchemaNames.js';
+import { AUTH_SCHEMA_NAMES, AUTH_SCHEMA_NAMES_NO_V2_PUBLIC_EXPORT } from '../../src/migrations/v1-to-v2/mappings/authSchemaNames';
 
 describe('AUTH_SCHEMA_NAMES (codemod auth schema-routing allowlist)', () => {
     it('routes only auth schemas that @modelcontextprotocol/core exports (drift guard)', () => {

packages/codemod/test/v1-to-v2/specSchemaNames.test.ts

@@ -3,7 +3,7 @@ import { fileURLToPath } from 'node:url';
 
 import { describe, expect, it } from 'vitest';
 
-import { SPEC_SCHEMA_NAMES } from '../../src/migrations/v1-to-v2/mappings/specSchemaNames.js';
+import { SPEC_SCHEMA_NAMES } from '../../src/migrations/v1-to-v2/mappings/specSchemaNames';
 
 describe('SPEC_SCHEMA_NAMES (codemod schema-routing allowlist)', () => {
     it("matches @modelcontextprotocol/core's exported schema set exactly (drift guard)", () => {