codemod: rewrite moved error-code semantics, fix StreamableHTTPError target, preserve file headers

.changeset/codemod-error-code-semantics.md

@@ -0,0 +1,10 @@
+---
+'@modelcontextprotocol/codemod': minor
+---
+
+Add an `error-code-semantics` transform that fixes `instanceof` guards around the
+`ErrorCode.RequestTimeout` / `ErrorCode.ConnectionClosed` members, which moved from the
+protocol error enum to `SdkErrorCode` (raised on `SdkError`) in v2. Without this, migrated
+checks like `e instanceof ProtocolError && e.code === SdkErrorCode.RequestTimeout` compile
+but never match at runtime. The transform also flags switches and maps that mix the two
+enums, since `SdkErrorCode` is a string enum.

.changeset/codemod-preserve-file-headers.md

@@ -0,0 +1,6 @@
+---
+'@modelcontextprotocol/codemod': patch
+---
+
+Preserve file-leading shebangs and banner comments when import rewrites remove the first
+import declaration in a file. Previously the attached leading trivia was silently deleted.

.changeset/codemod-sdkhttperror-rewrite.md

@@ -0,0 +1,7 @@
+---
+'@modelcontextprotocol/codemod': patch
+---
+
+Rewrite `StreamableHTTPError` to `SdkHttpError` (the documented v2 replacement with typed
+`.status`/`.statusText`) instead of `SdkError`, and correct the migration guidance the
+codemod emits for it.

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

@@ -0,0 +1,254 @@
+import type { BinaryExpression, SourceFile } from 'ts-morph';
+import { Node, SyntaxKind } from 'ts-morph';
+
+import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types.js';
+import { error, warning } from '../../../utils/diagnostics.js';
+import { addOrMergeImport, isImportedFromMcp, removeUnusedImport } from '../../../utils/importUtils.js';
+import { ERROR_CODE_SDK_MEMBERS } from '../mappings/symbolMap.js';
+
+/**
+ * In v1, `ErrorCode.RequestTimeout` and `ErrorCode.ConnectionClosed` were numeric
+ * members of the protocol error enum and were raised on `McpError`. In v2 they moved
+ * to `SdkErrorCode` — a STRING enum — and are raised on `SdkError`, while protocol
+ * errors became `ProtocolError` with `ProtocolErrorCode`.
+ *
+ * The symbol rename transform updates the enum references, but a check like
+ *
+ *     e instanceof ProtocolError && e.code === SdkErrorCode.RequestTimeout
+ *
+ * still compiles under loose typing and NEVER matches at runtime, because v2 raises
+ * timeouts/disconnects as `SdkError`, not `ProtocolError`. This transform rewrites the
+ * `instanceof` side of such checks to `SdkError` where that is safe, and emits
+ * diagnostics where it is not:
+ *
+ * - `subject instanceof ProtocolError/McpError` guarding a comparison against a moved
+ *   member → guard rewritten to `subject instanceof SdkError` (import added).
+ * - comparison against a moved member with no detectable `instanceof` guard → warning
+ *   to verify the subject is checked as `SdkError`.
+ * - `instanceof` guard against an unrecognized class → error with the exact manual fix.
+ * - `switch` over `x.code` mixing `SdkErrorCode` and `ProtocolErrorCode` cases → error
+ *   (the enums live on different error classes in v2; the switch must be split).
+ * - object literals keyed by a moved member → warning (`SdkErrorCode` is a string enum;
+ *   previously-numeric map keys are now strings).
+ *
+ * Both the post-rename (`SdkErrorCode.X`) and pre-rename (`ErrorCode.X`) spellings are
+ * recognized, so the transform composes with the symbol rename transform in either
+ * order; it never rewrites the enum reference itself.
+ */
+
+const MOVED_MEMBER_ENUMS = new Set(['SdkErrorCode', 'ErrorCode']);
+const V1_ERROR_CLASSES = new Set(['ProtocolError', 'McpError']);
+
+export const errorCodeSemanticsTransform: Transform = {
+    name: 'Error code semantics',
+    id: 'error-code-semantics',
+    apply(sourceFile: SourceFile, context: TransformContext): TransformResult {
+        const diagnostics: Diagnostic[] = [];
+        const filePath = sourceFile.getFilePath();
+        let changesCount = 0;
+        let needsSdkErrorImport = false;
+
+        // Collect first, mutate after — mutating while iterating invalidates descendants.
+        const movedMemberAccesses: Node[] = [];
+        sourceFile.forEachDescendant(node => {
+            if (!Node.isPropertyAccessExpression(node)) return;
+            const expr = node.getExpression();
+            if (!Node.isIdentifier(expr)) return;
+            if (!MOVED_MEMBER_ENUMS.has(expr.getText())) return;
+            if (!ERROR_CODE_SDK_MEMBERS.has(node.getName())) return;
+            movedMemberAccesses.push(node);
+        });
+
+        const guardsToRewrite = new Set<BinaryExpression>();
+        const flaggedObjectLiterals = new Set<Node>();
+        const flaggedSwitches = new Set<Node>();
+
+        for (const access of movedMemberAccesses) {
+            const memberName = access.getChildAtIndex(2)?.getText() ?? access.getText();
+            const line = access.getStartLineNumber();
+
+            const comparison = getComparisonContext(access);
+            if (comparison) {
+                const guard = findInstanceofGuard(access, comparison.subjectText);
+                if (guard) {
+                    const className = guard.getRight().getText();
+                    if (V1_ERROR_CLASSES.has(className)) {
+                        guardsToRewrite.add(guard);
+                    } else if (className !== 'SdkError') {
+                        diagnostics.push(
+                            error(
+                                filePath,
+                                line,
+                                `\`${comparison.subjectText}.code === SdkErrorCode.${memberName}\` is guarded by ` +
+                                    `\`instanceof ${className}\`, but v2 raises ${memberName} on SdkError. ` +
+                                    `Manual fix: \`${comparison.subjectText} instanceof SdkError && ` +
+                                    `${comparison.subjectText}.code === SdkErrorCode.${memberName}\`.`
+                            )
+                        );
+                    }
+                } else {
+                    diagnostics.push(
+                        warning(
+                            filePath,
+                            line,
+                            `Comparison against SdkErrorCode.${memberName}: in v2 this code is raised on SdkError ` +
+                                `(not McpError/ProtocolError as in v1). Verify \`${comparison.subjectText}\` is checked ` +
+                                `with \`instanceof SdkError\` before this comparison.`
+                        )
+                    );
+                }
+                continue;
+            }
+
+            const caseClause = access.getFirstAncestorByKind(SyntaxKind.CaseClause);
+            if (caseClause && caseClause.getExpression() === access) {
+                const switchStmt = caseClause.getFirstAncestorByKind(SyntaxKind.SwitchStatement);
+                if (switchStmt && !flaggedSwitches.has(switchStmt)) {
+                    flaggedSwitches.add(switchStmt);
+                    const mixesProtocolCodes = switchStmt
+                        .getDescendantsOfKind(SyntaxKind.PropertyAccessExpression)
+                        .some(p => Node.isIdentifier(p.getExpression()) && p.getExpression().getText() === 'ProtocolErrorCode');
+                    if (mixesProtocolCodes) {
+                        diagnostics.push(
+                            error(
+                                filePath,
+                                line,
+                                `switch mixes SdkErrorCode and ProtocolErrorCode cases. In v2 these enums live on ` +
+                                    `different error classes (SdkError vs ProtocolError), so a single switch over ` +
+                                    `\`.code\` cannot handle both. Split into an \`instanceof SdkError\` branch and an ` +
+                                    `\`instanceof ProtocolError\` branch.`
+                            )
+                        );
+                    } else {
+                        diagnostics.push(
+                            warning(
+                                filePath,
+                                line,
+                                `switch case on SdkErrorCode.${memberName}: in v2 this code is raised on SdkError. ` +
+                                    `Verify the switch subject is checked with \`instanceof SdkError\`.`
+                            )
+                        );
+                    }
+                }
+                continue;
+            }
+
+            const computedName = access.getFirstAncestorByKind(SyntaxKind.ComputedPropertyName);
+            if (computedName && computedName.getExpression() === access) {
+                const objectLiteral = computedName.getFirstAncestorByKind(SyntaxKind.ObjectLiteralExpression);
+                if (objectLiteral && !flaggedObjectLiterals.has(objectLiteral)) {
+                    flaggedObjectLiterals.add(objectLiteral);
+                    const mixesProtocolCodes = objectLiteral
+                        .getDescendantsOfKind(SyntaxKind.PropertyAccessExpression)
+                        .some(p => Node.isIdentifier(p.getExpression()) && p.getExpression().getText() === 'ProtocolErrorCode');
+                    diagnostics.push(
+                        warning(
+                            filePath,
+                            line,
+                            `Map keyed by SdkErrorCode.${memberName}: SdkErrorCode is a string enum in v2, so this key ` +
+                                `changed from a number to a string.` +
+                                (mixesProtocolCodes
+                                    ? ` This literal also has ProtocolErrorCode keys (numeric) — split it into separate maps.`
+                                    : ` Update any \`Record<number, ...>\` typing and numeric lookups accordingly.`)
+                        )
+                    );
+                }
+            }
+        }
+
+        const rewrittenGuardClasses = new Set<string>();
+        for (const guard of guardsToRewrite) {
+            rewrittenGuardClasses.add(guard.getRight().getText());
+            guard.getRight().replaceWithText('SdkError');
+            needsSdkErrorImport = true;
+            changesCount++;
+        }
+
+        if (needsSdkErrorImport && !isImportedFromMcp(sourceFile, 'SdkError')) {
+            const targetModule = resolveTargetModule(sourceFile, context);
+            addOrMergeImport(sourceFile, targetModule, ['SdkError'], false, sourceFile.getImportDeclarations().length);
+            changesCount++;
+        }
+
+        // Retargeting a guard may have removed the last usage of the v1 error
+        // class — drop the now-unused import specifier so migrated output does
+        // not carry unused-import lint debt.
+        for (const className of rewrittenGuardClasses) {
+            if (removeUnusedImport(sourceFile, className, true)) {
+                changesCount++;
+            }
+        }
+
+        return { changesCount, diagnostics };
+    }
+};
+
+interface ComparisonContext {
+    subjectText: string;
+}
+
+/**
+ * If `access` is one side of a `===`/`!==`/`==`/`!=` comparison whose other side is a
+ * `<subject>.code` property access, return the subject expression text.
+ */
+function getComparisonContext(access: Node): ComparisonContext | undefined {
+    const parent = access.getParent();
+    if (!parent || !Node.isBinaryExpression(parent)) return undefined;
+    const op = parent.getOperatorToken().getKind();
+    if (
+        op !== SyntaxKind.EqualsEqualsEqualsToken &&
+        op !== SyntaxKind.ExclamationEqualsEqualsToken &&
+        op !== SyntaxKind.EqualsEqualsToken &&
+        op !== SyntaxKind.ExclamationEqualsToken
+    ) {
+        return undefined;
+    }
+    const other = parent.getLeft() === access ? parent.getRight() : parent.getLeft();
+    if (!Node.isPropertyAccessExpression(other)) return undefined;
+    if (other.getName() !== 'code') return undefined;
+    return { subjectText: other.getExpression().getText() };
+}
+
+/**
+ * Search the enclosing conditions for `subject instanceof <Class>`. Covers the same
+ * logical-AND chain (`e instanceof X && e.code === ...`) and enclosing if/ternary
+ * conditions (`if (e instanceof X) { if (e.code === ...) ... }`).
+ */
+function findInstanceofGuard(access: Node, subjectText: string): BinaryExpression | undefined {
+    let scope: Node | undefined = access;
+    while (scope) {
+        if (Node.isBinaryExpression(scope) || Node.isIfStatement(scope) || Node.isConditionalExpression(scope)) {
+            const searchRoot = Node.isIfStatement(scope)
+                ? scope.getExpression()
+                : Node.isConditionalExpression(scope)
+                  ? scope.getCondition()
+                  : scope;
+            for (const candidate of [searchRoot, ...searchRoot.getDescendantsOfKind(SyntaxKind.BinaryExpression)]) {
+                if (!Node.isBinaryExpression(candidate)) continue;
+                if (candidate.getOperatorToken().getKind() !== SyntaxKind.InstanceOfKeyword) continue;
+                if (candidate.getLeft().getText() !== subjectText) continue;
+                return candidate;
+            }
+        }
+        if (
+            Node.isFunctionDeclaration(scope) ||
+            Node.isFunctionExpression(scope) ||
+            Node.isArrowFunction(scope) ||
+            Node.isSourceFile(scope)
+        ) {
+            return undefined;
+        }
+        scope = scope.getParent();
+    }
+    return undefined;
+}
+
+function resolveTargetModule(sourceFile: SourceFile, context: TransformContext): string {
+    const existing = sourceFile.getImportDeclarations().find(i => {
+        const spec = i.getModuleSpecifierValue();
+        return spec === '@modelcontextprotocol/client' || spec === '@modelcontextprotocol/server';
+    });
+    if (existing) return existing.getModuleSpecifierValue();
+    if (context.projectType === 'client') return '@modelcontextprotocol/client';
+    return '@modelcontextprotocol/server';
+}

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

@@ -3,7 +3,14 @@ import type { SourceFile } from 'ts-morph';
 import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types.js';
 import { renameAllReferences } from '../../../utils/astUtils.js';
 import { actionRequired, info, v2Gap, warning } from '../../../utils/diagnostics.js';
-import { addOrMergeImport, getSdkExports, getSdkImports, isTypeOnlyImport } from '../../../utils/importUtils.js';
+import {
+    addOrMergeImport,
+    captureLeadingFileTrivia,
+    getSdkExports,
+    getSdkImports,
+    isTypeOnlyImport,
+    restoreLeadingFileTrivia
+} from '../../../utils/importUtils.js';
 import { resolveTypesPackage } from '../../../utils/projectAnalyzer.js';
 import { IMPORT_MAP, isAuthImport } from '../mappings/importMap.js';
 import { SIMPLE_RENAMES } from '../mappings/symbolMap.js';
@@ -14,7 +21,7 @@ const REEXPORT_WARNINGS: Record<string, string> = {
         'Re-exported RequestHandlerExtra was renamed to ServerContext/ClientContext in v2. Update this re-export manually.',
     IsomorphicHeaders: 'Re-exported IsomorphicHeaders was removed in v2 (replaced by standard Headers API). Remove this re-export.',
     StreamableHTTPError:
-        'Re-exported StreamableHTTPError was renamed to SdkError in v2 with different constructor. Update this re-export manually.'
+        'Re-exported StreamableHTTPError was replaced by SdkHttpError in v2 with a different constructor. Update this re-export manually.'
 };
 
 export const importPathsTransform: Transform = {
@@ -49,6 +56,9 @@ export const importPathsTransform: Transform = {
         });
 
         const insertIndex = sourceFile.getImportDeclarations().indexOf(sdkImports[0]!);
+        // Removing the first import would silently delete a file-leading shebang or
+        // banner comment attached to it — capture it up front and restore at the end.
+        const leadingTrivia = captureLeadingFileTrivia(sdkImports[0]!);
 
         interface PendingImport {
             names: string[];
@@ -204,6 +214,8 @@ export const importPathsTransform: Transform = {
             }
         }
 
+        restoreLeadingFileTrivia(sourceFile, leadingTrivia);
+
         return { changesCount, diagnostics, usedPackages };
     }
 };

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

@@ -1,5 +1,6 @@
 import type { Transform } from '../../../types.js';
 import { contextTypesTransform } from './contextTypes.js';
+import { errorCodeSemanticsTransform } from './errorCodeSemantics.js';
 import { expressMiddlewareTransform } from './expressMiddleware.js';
 import { handlerRegistrationTransform } from './handlerRegistration.js';
 import { importPathsTransform } from './importPaths.js';
@@ -20,26 +21,33 @@ import { symbolRenamesTransform } from './symbolRenames.js';
 //    ProtocolError) and rewrites type references (e.g., SchemaInput<T> →
 //    StandardSchemaWithJSON.InferInput<T>).
 //
-// 3. removedApis runs after symbolRenames: handles removed Zod helpers,
+// 3. errorCodeSemantics runs after symbolRenames: the rename turns
+//    ErrorCode.RequestTimeout/ConnectionClosed into SdkErrorCode members; this
+//    transform then fixes the surrounding semantics (instanceof guards that
+//    still reference ProtocolError, switches/maps mixing the two enums). It
+//    also recognizes the pre-rename spelling, so it is safe standalone.
+//
+// 4. removedApis runs after symbolRenames: handles removed Zod helpers,
 //    IsomorphicHeaders, and StreamableHTTPError. Conceptually different
 //    from renames — these are removals with diagnostic guidance.
 //
-// 4. mcpServerApi SHOULD run before contextTypes: it rewrites .tool() etc.
+// 5. mcpServerApi SHOULD run before contextTypes: it rewrites .tool() etc.
 //    to .registerTool() etc. contextTypes handles both old and new names,
 //    but running mcpServerApi first ensures consistent argument structure.
 //
-// 5. handlerRegistration, schemaParamRemoval, and expressMiddleware are
+// 6. handlerRegistration, schemaParamRemoval, and expressMiddleware are
 //    independent of each other but all depend on importPaths having run.
 //
-// 6. specSchemaAccess runs after handlerRegistration and schemaParamRemoval:
+// 7. specSchemaAccess runs after handlerRegistration and schemaParamRemoval:
 //    those transforms remove spec schema references they handle. specSchemaAccess
 //    then processes remaining standalone usages (safeParse, parse, z.infer, etc.).
 //
-// 7. mockPaths runs last: handles test mocks and dynamic imports,
+// 8. mockPaths runs last: handles test mocks and dynamic imports,
 //    independent of the other transforms.
 export const v1ToV2Transforms: Transform[] = [
     importPathsTransform,
     symbolRenamesTransform,
+    errorCodeSemanticsTransform,
     removedApisTransform,
     mcpServerApiTransform,
     handlerRegistrationTransform,