From 0b9c5822f78453c8678e82ecfd59c5badc688fa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 05:15:12 +0000 Subject: [PATCH 1/2] fix(codemod): only count real module specifiers in project-type inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v1→v2 source scanner matched any quoted @modelcontextprotocol/sdk/client|server subpath anywhere in a scanned file, so an SDK path appearing in an ordinary string literal (example text, a log message, a config value) counted as an import. A client-only project carrying such a string was classified 'both': shared type imports were rewritten to @modelcontextprotocol/server and a server dependency the project never uses was added to package.json. The client/server detection regexes are now anchored to genuine module-specifier positions — after 'from' (static imports and re-exports), 'import' (side-effect and dynamic imports), or 'require(' — so arbitrary string occurrences no longer affect inference. Fixes #2760. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KWwxVxEvDmfaFhr7EPAWpR --- .changeset/codemod-string-literal-imports.md | 5 +++ packages/codemod/src/utils/projectAnalyzer.ts | 14 ++++++--- packages/codemod/test/projectAnalyzer.test.ts | 31 +++++++++++++++++++ 3 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 .changeset/codemod-string-literal-imports.md diff --git a/.changeset/codemod-string-literal-imports.md b/.changeset/codemod-string-literal-imports.md new file mode 100644 index 0000000000..3c293f14e9 --- /dev/null +++ b/.changeset/codemod-string-literal-imports.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +Project-type inference no longer counts SDK paths that appear only in ordinary string literals. 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 only counts genuine module specifiers: static imports and re-exports (`from '...'`), side-effect imports, dynamic `import('...')`, and `require('...')`. diff --git a/packages/codemod/src/utils/projectAnalyzer.ts b/packages/codemod/src/utils/projectAnalyzer.ts index 1a836021c7..f9b549b3fe 100644 --- a/packages/codemod/src/utils/projectAnalyzer.ts +++ b/packages/codemod/src/utils/projectAnalyzer.ts @@ -10,14 +10,18 @@ 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 genuine module-specifier position: after `from` (static imports and re-exports), +// `import` (side-effect and dynamic imports), or `require(`. An SDK path that merely appears in an +// ordinary string literal (example text, log messages, config values) is not an import and must not +// count toward project-type inference (#2760). The tail is anchored to a trailing `/` or closing +// quote so `…/client` is not confused with `…/clientfoo`. +const SPECIFIER_POSITION = /(?:\bfrom\s*|\bimport\s*\(\s*|\bimport\s*|\brequire\s*\(\s*)/.source; +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); diff --git a/packages/codemod/test/projectAnalyzer.test.ts b/packages/codemod/test/projectAnalyzer.test.ts index 222894aef7..1045c58532 100644 --- a/packages/codemod/test/projectAnalyzer.test.ts +++ b/packages/codemod/test/projectAnalyzer.test.ts @@ -180,6 +180,37 @@ 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('infers from source even without a package.json', () => { const dir = createTempDir(); mkdirSync(path.join(dir, 'src'), { recursive: true }); From 8652eee92f0113c694796c9966fd9085bc9c9e33 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 05:28:19 +0000 Subject: [PATCH 2/2] fix(codemod): count mock-method, magic-comment and require.resolve specifiers in inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: the specifier-position anchor missed forms the codemod itself treats as SDK module specifiers — vi./jest. mock-method calls (the ones the mock-paths transform rewrites), dynamic import() carrying a webpack magic comment, and require.resolve() — so a project whose only signal for one SDK side used such a form degraded to 'unknown'/one-sided inference vs base. MOCK_CALLERS/MOCK_METHODS move to utils/importUtils as the single source of truth (projectAnalyzer cannot import them from the mock-paths transform without a cycle; mockPaths re-exports them for runner.ts) and the analyzer builds its specifier-position alternatives from them. Fail-first tests added for all three forms. Also narrows the changeset and inline-comment claims to what the lexical scan actually guarantees — bare SDK paths in string data no longer count — and adds a test documenting the known remaining case: a string whose text embeds a full import statement still matches, since only a real parser could tell the inner `from '` apart from a genuine specifier position. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KWwxVxEvDmfaFhr7EPAWpR --- .changeset/codemod-string-literal-imports.md | 2 +- .../v1-to-v2/transforms/mockPaths.ts | 18 +++------ packages/codemod/src/utils/importUtils.ts | 19 ++++++++++ packages/codemod/src/utils/projectAnalyzer.ts | 28 +++++++++++--- packages/codemod/test/projectAnalyzer.test.ts | 37 +++++++++++++++++++ 5 files changed, 84 insertions(+), 20 deletions(-) diff --git a/.changeset/codemod-string-literal-imports.md b/.changeset/codemod-string-literal-imports.md index 3c293f14e9..9d96bcded8 100644 --- a/.changeset/codemod-string-literal-imports.md +++ b/.changeset/codemod-string-literal-imports.md @@ -2,4 +2,4 @@ '@modelcontextprotocol/codemod': patch --- -Project-type inference no longer counts SDK paths that appear only in ordinary string literals. 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 only counts genuine module specifiers: static imports and re-exports (`from '...'`), side-effect imports, dynamic `import('...')`, and `require('...')`. +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. 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 bf217a936f..a7c02451ef 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/mockPaths.ts +++ b/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 = new Set([ - 'mock', - 'doMock', - 'unmock', - 'dontMock', - 'deepUnmock', - 'requireActual', - 'importActual', - 'requireMock', - 'createMockFromModule' -]); -export const MOCK_CALLERS: ReadonlySet = 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', diff --git a/packages/codemod/src/utils/importUtils.ts b/packages/codemod/src/utils/importUtils.ts index eba4572c7e..e2e6c70a49 100644 --- a/packages/codemod/src/utils/importUtils.ts +++ b/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 = new Set([ + 'mock', + 'doMock', + 'unmock', + 'dontMock', + 'deepUnmock', + 'requireActual', + 'importActual', + 'requireMock', + 'createMockFromModule' +]); +export const MOCK_CALLERS: ReadonlySet = new Set(['vi', 'jest']); + export function getSdkImports(sourceFile: SourceFile): ImportDeclaration[] { return sourceFile.getImportDeclarations().filter(imp => { return isSdkSpecifier(imp.getModuleSpecifierValue()); diff --git a/packages/codemod/src/utils/projectAnalyzer.ts b/packages/codemod/src/utils/projectAnalyzer.ts index f9b549b3fe..96cc86ad71 100644 --- a/packages/codemod/src/utils/projectAnalyzer.ts +++ b/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']; @@ -14,12 +15,27 @@ const SCAN_FILE_BUDGET = 5000; // '@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) -// — but only in a genuine module-specifier position: after `from` (static imports and re-exports), -// `import` (side-effect and dynamic imports), or `require(`. An SDK path that merely appears in an -// ordinary string literal (example text, log messages, config values) is not an import and must not -// count toward project-type inference (#2760). The tail is anchored to a trailing `/` or closing -// quote so `…/client` is not confused with `…/clientfoo`. -const SPECIFIER_POSITION = /(?:\bfrom\s*|\bimport\s*\(\s*|\bimport\s*|\brequire\s*\(\s*)/.source; +// — 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); diff --git a/packages/codemod/test/projectAnalyzer.test.ts b/packages/codemod/test/projectAnalyzer.test.ts index 1045c58532..8e02436733 100644 --- a/packages/codemod/test/projectAnalyzer.test.ts +++ b/packages/codemod/test/projectAnalyzer.test.ts @@ -211,6 +211,43 @@ describe('analyzeProject', () => { 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 });