Skip to content

fix(codemod): batch real-world v1→v2 migration bugs #2376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
felixweinberger wants to merge 1 commit into from
Closed

fix(codemod): batch real-world v1→v2 migration bugs #2376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/codemod-real-world-migration-gaps.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@modelcontextprotocol/codemod': patch
---

Fix a batch of v1→v2 codemod gaps surfaced by real-world migrations:

- Directory walk no longer follows symlinks and prunes ignored directories during descent, so a pnpm workspace with a cyclic intra-workspace dev-dependency no longer crashes with `ELOOP`, and `--ignore` actually skips a tree.
- Every workspace-member `package.json` that depends on the v1 SDK is now updated (not just the one closest to the target directory); `RunnerResult.packageJsonChanges` is now an array.
- A `zod@^3` dependency is reported (not rewritten) with guidance to bump to `^4` or pin `^3.25+` and import from `zod/v4`.
- New `--prefer client|server` flag overrides the hard-coded `server` default when no client/server signal is found.
- Leading file-header comments (JSDoc and `//` blocks) are preserved across the full transform pipeline, including when a later transform removes the rewritten first import.
- Type-position `import('@modelcontextprotocol/sdk/...').<Name>` qualifiers are rewritten and routed per-symbol.
- The `extra → ctx` remap now reaches `as`-cast/parenthesized callbacks, `fallbackRequestHandler` assignments, and single-parameter (schema-less) register callbacks; parameters typed `ServerContext`/`ClientContext` that still access v1 properties are marked with an `@mcp-codemod-error` comment.
- `error.code` accesses on an `instanceof`-checked `StreamableHTTPError`/`SdkHttpError` are marked (the HTTP status moved to `.status`).
- `require()`/`require.resolve()` of a v1 SDK path is marked (v2 subpath exports have no `require` condition).
- Wrapping a raw shape with `z.object()` now adds `import { z } from 'zod'` when missing and warns about the `zod` dependency.
37 changes: 24 additions & 13 deletions packages/codemod/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ for (const [name, migration] of listMigrations()) {
.option('-t, --transforms <ids>', 'Comma-separated transform IDs to run (default: all)')
.option('-v, --verbose', 'Show detailed per-change output')
.option('--ignore <patterns...>', 'Additional glob patterns to ignore')
.option('--prefer <side>', 'Default package (client|server) for shared types when no signal is found')
.option('--list', 'List available transforms for this migration')
.action((targetDir: string | undefined, opts: Record<string, unknown>) => {
try {
Expand Down Expand Up @@ -87,12 +88,20 @@ for (const [name, migration] of listMigrations()) {

const transforms = opts['transforms'] ? (opts['transforms'] as string).split(',').map(s => s.trim()) : undefined;

const preferRaw = opts['prefer'] as string | undefined;
if (preferRaw !== undefined && preferRaw !== 'client' && preferRaw !== 'server') {
console.error(`\nError: --prefer must be "client" or "server" (got "${preferRaw}").\n`);
process.exitCode = 1;
return;
}

const result = run(migration, {
targetDir: resolvedDir,
dryRun: opts['dryRun'] as boolean | undefined,
verbose: opts['verbose'] as boolean | undefined,
transforms,
ignore: opts['ignore'] as string[] | undefined
ignore: opts['ignore'] as string[] | undefined,
prefer: preferRaw
});

if (result.filesChanged === 0 && result.diagnostics.length === 0 && !result.packageJsonChanges) {
Expand Down Expand Up @@ -150,19 +159,21 @@ for (const [name, migration] of listMigrations()) {
}

if (result.packageJsonChanges) {
const pc = result.packageJsonChanges;
if (opts['dryRun']) {
console.log('package.json changes (dry run — not applied):');
} else {
console.log('package.json updated:');
const verb = opts['dryRun'] ? 'changes (dry run — not applied)' : 'updated';
for (const pc of result.packageJsonChanges) {
const rel = path.relative(process.cwd(), pc.packageJsonPath) || pc.packageJsonPath;
console.log(`${rel} ${verb}:`);
if (pc.removed.length > 0) {
console.log(` Removed: ${pc.removed.join(', ')}`);
}
if (pc.added.length > 0) {
console.log(` Added: ${pc.added.join(', ')}`);
}
for (const note of pc.notes ?? []) {
console.log(` Note: ${note}`);
}
console.log('');
}
if (pc.removed.length > 0) {
console.log(` Removed: ${pc.removed.join(', ')}`);
}
if (pc.added.length > 0) {
console.log(` Added: ${pc.added.join(', ')}`);
}
console.log('');
}

if (result.commentCount > 0) {
Expand Down
93 changes: 90 additions & 3 deletions packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,29 @@ const HANDLER_METHODS = new Set(['setRequestHandler', 'setNotificationHandler'])

const REGISTER_METHODS = new Set(['registerTool', 'registerPrompt', 'registerResource', 'tool', 'prompt', 'resource']);

const FALLBACK_HANDLER_PROPS = new Set(['fallbackRequestHandler', 'fallbackNotificationHandler']);

/** v1 context properties whose presence on a non-remapped parameter should be flagged. */
const V1_CONTEXT_PROPS = new Set(CONTEXT_PROPERTY_MAP.filter(m => m.from !== m.to).map(m => m.from.slice(1)));

/**
* Unwrap `as`-cast and parenthesized callback expressions so the underlying arrow/function is reached.
* v1 code commonly typed a `registerTool` callback via `(async (args, extra) => …) as Parameters<…>[2]`;
* without the unwrap the remap silently skips it and `extra.authInfo` etc. survive into v2 — which
* type-checks (the cast carries its own annotation) and only fails at runtime.
*/
function unwrapCallback(node: Node | undefined): Node | undefined {
let current = node;
while (current) {
if (Node.isAsExpression(current) || Node.isParenthesizedExpression(current) || Node.isSatisfiesExpression(current)) {
current = current.getExpression();
continue;
}
return current;
}
return current;
}

/**
* Attempt to rename the second parameter of a callback from 'extra' to 'ctx'
* and rewrite context property accesses in its body.
Expand All @@ -27,9 +50,15 @@ function processCallback(
return -1;

const params = callbackNode.getParameters();
if (params.length < 2) return -1;

const extraParam = params[1]!;
// For `setRequestHandler`/`setNotificationHandler` and the McpServer register/tool/prompt/resource
// methods the context is the SECOND parameter when an args/request param is present. A schema-less
// tool/prompt callback (`registerTool('name', {}, async (extra) => …)`) receives the context as its
// ONLY parameter — fall back to params[0] when the sole parameter is the v1 `extra` name.
let extraParam = params[1];
if (!extraParam && params.length === 1 && params[0]!.getName() === EXTRA_PARAM_NAME) {
extraParam = params[0];
}
if (!extraParam) return -1;
const paramNameNode = extraParam.getNameNode();
if (Node.isObjectBindingPattern(paramNameNode)) {
diagnostics.push(
Expand Down Expand Up @@ -219,6 +248,7 @@ export const contextTypesTransform: Transform = {
callbackArg = args.at(-1);
}

callbackArg = unwrapCallback(callbackArg);
if (!callbackArg) continue;

// Handle ObjectLiteralExpression callback containers (handler maps)
Expand Down Expand Up @@ -254,6 +284,63 @@ export const contextTypesTransform: Transform = {
}
}

// `protocol.fallbackRequestHandler = async (request, extra) => …` — the v1 fallback handler
// is assigned, not registered, so the call-site scan above never sees it.
for (const be of sourceFile.getDescendantsOfKind(SyntaxKind.BinaryExpression)) {
if (be.getOperatorToken().getKind() !== SyntaxKind.EqualsToken) continue;
const lhs = be.getLeft();
if (!Node.isPropertyAccessExpression(lhs) || !FALLBACK_HANDLER_PROPS.has(lhs.getName())) continue;
const cb = unwrapCallback(be.getRight());
if (!cb) continue;
const result = processCallback(cb, sourceFile, diagnostics, lhs.getName(), be.getStartLineNumber());
if (result > 0) changesCount += result;
}

// Catch-all marker for context parameters the remap could not reach: any function parameter
// annotated `ServerContext` / `ClientContext` (the symbol-renames transform has already
// rewritten v1's `RequestHandlerExtra` to those) whose body still accesses a v1-shaped
// property. Covers handlers typed via a local alias, helpers a callback forwards its context
// to, and any callback shape not enumerated above. Marking (not rewriting) keeps the
// compile-time/runtime regression visible without risking a wrong rewrite on a non-MCP type
// that happens to share the property name.
for (const param of sourceFile.getDescendantsOfKind(SyntaxKind.Parameter)) {
const typeNode = param.getTypeNode();
if (!typeNode || !Node.isTypeReference(typeNode)) continue;
const typeName = typeNode.getTypeName().getText();
if (typeName !== 'ServerContext' && typeName !== 'ClientContext') continue;
const nameNode = param.getNameNode();
if (!Node.isIdentifier(nameNode)) continue;
const paramName = nameNode.getText();
if (paramName === CTX_PARAM_NAME) continue; // already migrated form

const fn = param.getParent();
const body =
Node.isArrowFunction(fn) || Node.isFunctionExpression(fn) || Node.isFunctionDeclaration(fn) || Node.isMethodDeclaration(fn)
? fn.getBody()
: undefined;
if (!body) continue;

const stale = new Set<string>();
body.forEachDescendant(node => {
if (!Node.isPropertyAccessExpression(node)) return;
const obj = node.getExpression();
if (!Node.isIdentifier(obj) || obj.getText() !== paramName) return;
if (V1_CONTEXT_PROPS.has(node.getName())) stale.add(node.getName());
});
if (stale.size === 0) continue;

diagnostics.push(
actionRequired(
sourceFile.getFilePath(),
param,
`Parameter \`${paramName}: ${typeName}\` accesses v1 context propert${stale.size === 1 ? 'y' : 'ies'} ` +
`(${[...stale].map(p => `.${p}`).join(', ')}) that moved in v2 ` +
`(e.g. .signal → .mcpReq.signal, .authInfo → .http?.authInfo). The codemod could not remap this ` +
`parameter automatically — update the property accesses manually.`
)
);
}

return { changesCount, diagnostics };
}
};
43 changes: 40 additions & 3 deletions packages/codemod/src/migrations/v1-to-v2/transforms/mcpServerApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,39 @@ import type { CallExpression, SourceFile } from 'ts-morph';
import { Node, SyntaxKind } from 'ts-morph';

import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types';
import { actionRequired, info } from '../../../utils/diagnostics';
import { actionRequired, info, warning } from '../../../utils/diagnostics';
import { isOriginalNameImportedFromMcp, resolveLocalImportName } from '../../../utils/importUtils';

/** True when `z` is already bound in the file (named, default, or namespace) by an import from `zod`. */
function hasZodImport(sourceFile: SourceFile): boolean {
return sourceFile.getImportDeclarations().some(imp => {
const spec = imp.getModuleSpecifierValue();
if (spec !== 'zod' && !spec.startsWith('zod/')) return false;
if (imp.getDefaultImport()?.getText() === 'z') return true;
if (imp.getNamespaceImport()?.getText() === 'z') return true;
return imp.getNamedImports().some(n => (n.getAliasNode()?.getText() ?? n.getName()) === 'z');
});
}

/**
* Ensure the `z` binding exists when this transform has just emitted a `z.object(...)` wrapper.
* Without it the wrap turns a working v1 file into a TS2304 (`Cannot find name 'z'`). Adds
* `import { z } from 'zod'` and warns that `zod` may need adding to the package's dependencies — the
* package.json updater bumps an existing zod range but does not add zod to a package that never had it.
*/
function ensureZodImport(sourceFile: SourceFile, diagnostics: Diagnostic[]): void {
if (hasZodImport(sourceFile)) return;
sourceFile.insertImportDeclaration(0, { moduleSpecifier: 'zod', namedImports: ['z'] });
diagnostics.push(
warning(
sourceFile.getFilePath(),
1,
"Added `import { z } from 'zod'` for the z.object() wrapper(s). " +
'If this package does not already depend on zod, add `zod@^4` to its dependencies.'
)
);
}

export const mcpServerApiTransform: Transform = {
name: 'McpServer API migration',
id: 'mcpserver-api',
Expand Down Expand Up @@ -126,6 +156,11 @@ export const mcpServerApiTransform: Transform = {

flagRemovedTaskOptions(sourceFile, diagnostics);

// The wrap helpers emit a fixed info note whenever they emit a `z.object(...)` wrapper; use
// that as the signal so the helpers don't need to thread an extra out-param.
const wrappedRawShape = diagnostics.some(d => d.message.startsWith(RAW_WRAP_INFO_PREFIX));
if (wrappedRawShape) ensureZodImport(sourceFile, diagnostics);

return { changesCount, diagnostics };
}
};
Expand All @@ -141,6 +176,8 @@ function isZodObjectCall(node: Node): boolean {
return expr.getName() === 'object' && expr.getExpression().getText() === 'z';
}

const RAW_WRAP_INFO_PREFIX = 'Raw object literal';

function wrapWithZObject(schemaText: string): string {
return `z.object(${schemaText})`;
}
Expand All @@ -159,7 +196,7 @@ function emitWrapDiagnostic(node: Node, sourceFile: SourceFile, call: CallExpres
info(
sourceFile.getFilePath(),
call.getStartLineNumber(),
'Raw object literal wrapped with z.object(). Verify that zod (z) is imported in this file.'
`${RAW_WRAP_INFO_PREFIX} wrapped with z.object(). Verify that zod (z) is imported in this file.`
)
);
} else if (!isZodObjectCall(node)) {
Expand Down Expand Up @@ -218,7 +255,7 @@ function wrapSchemaInConfig(call: CallExpression, schemaPropertyName: string, so
info(
sourceFile.getFilePath(),
call.getStartLineNumber(),
`Raw object literal in ${schemaPropertyName} wrapped with z.object(). Verify that zod (z) is imported in this file.`
`${RAW_WRAP_INFO_PREFIX} in ${schemaPropertyName} wrapped with z.object(). Verify that zod (z) is imported in this file.`
)
);
return true;
Expand Down
Loading
Loading