Skip to content

fix

36a1774
Select commit
Loading
Failed to load commit list.
Draft

feat: add v2 codemod draft #1950

fix
36a1774
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 24, 2026 in 11m 48s

Code review found 5 potential issues

Found 10 candidates, confirmed 5. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 3
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit packages/codemod/package.json:32-34 New codemod package missing README.md
🟡 Nit packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts:45-47 contextTypes silently skips param-level destructured context with no diagnostic
🟡 Nit packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts:206-208 rewriteExportDeclarations ignores renamedSymbols — re-exports break

Annotations

Check warning on line 34 in packages/codemod/package.json

See this annotation in the file changed.

@claude claude / Claude Code Review

New codemod package missing README.md

The new `@modelcontextprotocol/codemod` package is added to the publish workflow but ships with no `README.md`, while every other published package in the repo (client, server, middleware/*) has one. The PR description's CLI usage, programmatic API, and transform table won't be visible on the npm package page — worth adding a `packages/codemod/README.md` before this lands (npm auto-includes README regardless of the `files` array, so no `package.json` change is needed).

Check warning on line 47 in packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

contextTypes silently skips param-level destructured context with no diagnostic

When the second handler param is destructured directly in the signature (e.g., `async (request, { signal, authInfo }) => {...}`), `extraParam.getName()` returns the binding-pattern text `'{ signal, authInfo }'`, which fails the `=== 'extra'` check at line 47 and silently `continue`s with no diagnostic. This is internally inconsistent with the body-level destructuring case (lines 74-88), which *does* push a 'Manual refactoring required' warning for the same situation. Consider adding `if (Node.is

Check warning on line 208 in packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

rewriteExportDeclarations ignores renamedSymbols — re-exports break

`rewriteExportDeclarations` only calls `exp.setModuleSpecifier(targetPackage)` and never inspects `mapping.renamedSymbols`, so a barrel re-export like `export { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'` becomes `export { StreamableHTTPServerTransport } from '@modelcontextprotocol/node'` — which doesn't export that name (TS2305), with no diagnostic. Iterate `exp.getNamedExports()` and emit `{ NodeStreamableHTTPServerTransport as StreamableHTTPServe