Skip to content

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

Open
claude[bot] wants to merge 2 commits into
mainfrom
claude/codemod-string-literal-imports
Open

fix(codemod): only count real module specifiers in project-type inference#2765
claude[bot] wants to merge 2 commits into
mainfrom
claude/codemod-string-literal-imports

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown

Requested via Slack thread

Fixes #2760.

Before: the v1→v2 codemod's project-type inference regex-matched any quoted @modelcontextprotocol/sdk/client|server subpath anywhere in a scanned file, so an SDK path that only appears inside an ordinary string literal — example text, a log message, a config value — counted as an import. A client-only project carrying one server path as data was classified as using both packages: its shared type imports (e.g. types.js) were rewritten to @modelcontextprotocol/server, and package.json gained both @modelcontextprotocol/client and @modelcontextprotocol/server, including a server dependency the project never uses.

After: only genuine module specifiers affect inference — static imports and re-exports (from '...'), side-effect imports (import '...'), dynamic import('...'), and require('...'). The reported fixture is now classified client, its shared type import moves to @modelcontextprotocol/client, and no server dependency is added. String occurrences of SDK paths are ignored, matching how the imports transform itself (which is already specifier-aware and never rewrote the literal) treats them.

How: the two detection regexes in packages/codemod/src/utils/projectAnalyzer.ts now require a module-specifier position — the quoted path must follow from, import / import(, or require( — instead of matching any quoted occurrence. Three tests added to packages/codemod/test/projectAnalyzer.test.ts: the string-literal regression (fails on main with both, passes with client after the fix) plus two guard tests pinning that dynamic import()/require() and side-effect/export ... from specifiers still count. Full codemod suite: 632/632 passing; pnpm run check (tsgo typecheck + ESLint + Prettier) clean; patch changeset for @modelcontextprotocol/codemod included and Prettier-clean. Verified end-to-end with the issue's fixture via tsx src/cli.ts v1-to-v2 <fixture> --transforms imports --verbose.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KWwxVxEvDmfaFhr7EPAWpR


Generated by Claude Code

…ence

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWwxVxEvDmfaFhr7EPAWpR
@claude
claude Bot requested a review from a team as a code owner September 7, 2026 05:19
@changeset-bot

changeset-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8652eee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/core-internal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 7, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2765

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2765

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2765

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2765

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2765

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2765

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2765

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2765

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2765

commit: 8652eee

Comment thread packages/codemod/src/utils/projectAnalyzer.ts Outdated
Comment thread packages/codemod/src/utils/projectAnalyzer.ts Outdated
…ecifiers in inference

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWwxVxEvDmfaFhr7EPAWpR

@claude claude Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new blocking issues. 1 optional suggestion (a nit or a note on pre-existing code) was found and not posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v2] Codemod treats SDK paths in string literals as imports

1 participant