fix(codemod): only count real module specifiers in project-type inference (#2765)

Co-authored-by: Claude <noreply@anthropic.com>

.changeset/codemod-string-literal-imports.md

@@ -0,0 +1,5 @@
+---
+'@modelcontextprotocol/codemod': patch
+---
+
+Project-type inference no longer counts bare SDK paths that appear only in ordinary string data. The v1→v2 codemod's source scanner matched any quoted `@modelcontextprotocol/sdk/client|server` subpath anywhere in a file, so a server path stored as data (example text, a log message, a config value) misclassified a client-only project as `both` — rewriting shared type imports to `@modelcontextprotocol/server` and adding a server dependency the project never uses. The scanner now requires a module-specifier position: static imports and re-exports (`from '...'`), side-effect imports, dynamic `import('...')` (including webpack magic comments), `require('...')` / `require.resolve('...')`, and the `vi.`/`jest.` mock-method calls the mock-paths transform rewrites. Known limitation: the scan is lexical, so a string whose text embeds a complete import statement still counts.

packages/codemod/src/migrations/v1-to-v2/transforms/mockPaths.ts

@@ -3,7 +3,7 @@ import { Node, SyntaxKind } from 'ts-morph';
 
 import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types';
 import { actionRequired, v2Gap, warning } from '../../../utils/diagnostics';
-import { isSdkSpecifier } from '../../../utils/importUtils';
+import { isSdkSpecifier, MOCK_CALLERS, MOCK_METHODS } from '../../../utils/importUtils';
 import { resolveTypesPackage } from '../../../utils/projectAnalyzer';
 import type { ImportMapping } from '../mappings/importMap';
 import { isAuthImport, lookupImportMapping } from '../mappings/importMap';
@@ -28,18 +28,10 @@ function routeSymbols(symbols: string[], mapping: ImportMapping): { target?: str
     return { mixed: false };
 }
 
-export const MOCK_METHODS: ReadonlySet<string> = new Set([
-    'mock',
-    'doMock',
-    'unmock',
-    'dontMock',
-    'deepUnmock',
-    'requireActual',
-    'importActual',
-    'requireMock',
-    'createMockFromModule'
-]);
-export const MOCK_CALLERS: ReadonlySet<string> = new Set(['vi', 'jest']);
+// Defined in utils/importUtils (shared with the project analyzer, which cannot import from this
+// module without a cycle — this module imports resolveTypesPackage from utils/projectAnalyzer);
+// re-exported here for existing consumers (runner.ts).
+export { MOCK_CALLERS, MOCK_METHODS } from '../../../utils/importUtils';
 
 export const mockPathsTransform: Transform = {
     name: 'Mock and dynamic import path rewrites',

packages/codemod/src/utils/importUtils.ts

@@ -16,6 +16,25 @@ export function isSdkSpecifier(specifier: string): boolean {
     return specifier === SDK_PREFIX || specifier.startsWith(SDK_PREFIX + '/');
 }
 
+/**
+ * Mock-framework methods whose first string argument is a module specifier. The single source of
+ * truth shared by the mock-paths transform (which rewrites these specifiers), the runner (which
+ * detects them), and the project analyzer (which counts them toward project-type inference) —
+ * keep the three consumers in sync by editing only this set.
+ */
+export const MOCK_METHODS: ReadonlySet<string> = new Set([
+    'mock',
+    'doMock',
+    'unmock',
+    'dontMock',
+    'deepUnmock',
+    'requireActual',
+    'importActual',
+    'requireMock',
+    'createMockFromModule'
+]);
+export const MOCK_CALLERS: ReadonlySet<string> = new Set(['vi', 'jest']);
+
 export function getSdkImports(sourceFile: SourceFile): ImportDeclaration[] {
     return sourceFile.getImportDeclarations().filter(imp => {
         return isSdkSpecifier(imp.getModuleSpecifierValue());

packages/codemod/src/utils/projectAnalyzer.ts

@@ -3,6 +3,7 @@ import path from 'node:path';
 
 import type { Diagnostic, TransformContext } from '../types';
 import { info, warning } from './diagnostics';
+import { MOCK_CALLERS, MOCK_METHODS } from './importUtils';
 
 const PROJECT_ROOT_MARKERS = ['.git', 'node_modules'];
 
@@ -10,14 +11,33 @@ const SCAN_EXTENSIONS = new Set(['.ts', '.tsx', '.mts', '.cts', '.js', '.jsx', '
 const SCAN_SKIP_DIRS = new Set(['node_modules', 'dist', '.git', 'build', '.next', '.nuxt', 'coverage']);
 const SCAN_FILE_BUDGET = 5000;
 
-// Matches a quoted v1 SDK client/server subpath import specifier — e.g.
+// Matches a quoted v1 SDK client/server subpath — e.g.
 //   '@modelcontextprotocol/sdk/client/index.js'   "@modelcontextprotocol/sdk/server/mcp.js"
 //   '@modelcontextprotocol/sdk/client'            (extensionless / bare subpath; see the extensionless
 //                                                  import matching the codemod already supports)
-// Anchored to the opening quote and a trailing `/` or closing quote so that comments or prose that
-// merely mention the path do not count, and `…/client` is not confused with `…/clientfoo`.
-const CLIENT_IMPORT_RE = /['"`]@modelcontextprotocol\/sdk\/client(?:\/|['"`])/;
-const SERVER_IMPORT_RE = /['"`]@modelcontextprotocol\/sdk\/server(?:\/|['"`])/;
+// — but only in a module-specifier position: after `from` (static imports and re-exports), `import`
+// (side-effect and dynamic imports, tolerating webpack-style /* magic comments */ inside `import(`),
+// `require(` / `require.resolve(`, or the vi./jest. mock-method calls the mock-paths transform
+// rewrites (MOCK_CALLERS/MOCK_METHODS). A bare SDK path in ordinary string data (example text, log
+// messages, config values) no longer counts toward project-type inference (#2760).
+//
+// Known limitation: the scan is lexical, not a parser, so a string whose TEXT embeds a full import
+// statement (e.g. help text quoting `from '@modelcontextprotocol/sdk/server/mcp.js'`) still counts —
+// the inner `from '` is indistinguishable from a real specifier position without parsing, which the
+// budget-bounded scan deliberately avoids.
+//
+// The tail is anchored to a trailing `/` or closing quote so `…/client` is not confused with
+// `…/clientfoo`.
+const MOCK_CALL = String.raw`(?:${[...MOCK_CALLERS].join('|')})\s*\.\s*(?:${[...MOCK_METHODS].join('|')})`;
+const SPECIFIER_POSITION =
+    String.raw`(?:\bfrom\s*` + // static import / re-export
+    String.raw`|\bimport\s*\(\s*(?:\/\*[\s\S]*?\*\/\s*)*` + // dynamic import(), optional magic comments
+    String.raw`|\bimport\s*` + // side-effect import
+    String.raw`|\brequire\s*(?:\.\s*resolve\s*)?\(\s*` + // require() / require.resolve()
+    String.raw`|\b${MOCK_CALL}\s*\(\s*` + // vi.mock(...), jest.requireActual(...), ...
+    `)`;
+const CLIENT_IMPORT_RE = new RegExp(SPECIFIER_POSITION + /['"`]@modelcontextprotocol\/sdk\/client(?:\/|['"`])/.source);
+const SERVER_IMPORT_RE = new RegExp(SPECIFIER_POSITION + /['"`]@modelcontextprotocol\/sdk\/server(?:\/|['"`])/.source);
 
 export function findPackageJson(startDir: string): string | undefined {
     let dir = path.resolve(startDir);

packages/codemod/test/projectAnalyzer.test.ts

@@ -180,6 +180,74 @@ describe('analyzeProject', () => {
             expect(analyzeProject(dir).projectType).toBe('client');
         });
 
+        it('ignores an SDK subpath that appears only in a string literal (not a module specifier)', () => {
+            // A real client import plus a server subpath stored as data in an ordinary string
+            // literal. Counting quoted paths anywhere would flip this to "both" (rewriting shared
+            // imports to the server package and adding a server dependency to a client-only
+            // project); only genuine module specifiers may contribute to inference (#2760).
+            const dir = v1Project({
+                'a.ts': [
+                    `import { Client } from '@modelcontextprotocol/sdk/client/index.js';`,
+                    `const example = '@modelcontextprotocol/sdk/server/mcp.js';`,
+                    ''
+                ].join('\n')
+            });
+            expect(analyzeProject(dir).projectType).toBe('client');
+        });
+
+        it('still infers from dynamic import() and require() specifiers', () => {
+            const dir = v1Project({
+                'a.ts': `const { Client } = await import('@modelcontextprotocol/sdk/client/index.js');`,
+                'b.cjs': `const { McpServer } = require('@modelcontextprotocol/sdk/server/mcp.js');`
+            });
+            expect(analyzeProject(dir).projectType).toBe('both');
+        });
+
+        it('still infers from a side-effect import and an export-from re-export', () => {
+            const dir = v1Project({
+                'a.ts': `import '@modelcontextprotocol/sdk/client/index.js';`,
+                'b.ts': `export { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`
+            });
+            expect(analyzeProject(dir).projectType).toBe('both');
+        });
+
+        it('still infers from vi./jest. mock-method specifiers (the forms the mock-paths transform rewrites)', () => {
+            const dir = v1Project({
+                'a.test.ts': `vi.mock('@modelcontextprotocol/sdk/client/index.js');`,
+                'b.test.ts': `const actual = jest.requireActual('@modelcontextprotocol/sdk/server/mcp.js');`
+            });
+            expect(analyzeProject(dir).projectType).toBe('both');
+        });
+
+        it('still infers from a dynamic import() carrying a webpack magic comment', () => {
+            const dir = v1Project({
+                'a.ts': `const mod = await import(/* webpackChunkName: "mcp-client" */ '@modelcontextprotocol/sdk/client/index.js');`
+            });
+            expect(analyzeProject(dir).projectType).toBe('client');
+        });
+
+        it('still infers from require.resolve()', () => {
+            const dir = v1Project({
+                'a.cjs': `const p = require.resolve('@modelcontextprotocol/sdk/server/mcp.js');`
+            });
+            expect(analyzeProject(dir).projectType).toBe('server');
+        });
+
+        it('documents a known limitation: a string whose text embeds a full import statement still counts', () => {
+            // The scan is lexical: the inner `from '` puts the quoted path in a specifier position
+            // even though it sits inside string data. Distinguishing that from a real import needs
+            // parsing, which the budget-bounded scan deliberately avoids. If the analyzer ever gets
+            // smart enough to make this fail, flip the expectation to 'client'.
+            const dir = v1Project({
+                'a.ts': [
+                    `import { Client } from '@modelcontextprotocol/sdk/client/index.js';`,
+                    `const help = "import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'";`,
+                    ''
+                ].join('\n')
+            });
+            expect(analyzeProject(dir).projectType).toBe('both');
+        });
+
         it('infers from source even without a package.json', () => {
             const dir = createTempDir();
             mkdirSync(path.join(dir, 'src'), { recursive: true });