Skip to content

Commit 44ac609

Browse files
committed
claude review fixes
1 parent 05eadea commit 44ac609

13 files changed

Lines changed: 855 additions & 31 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import type { SourceFile } from 'ts-morph';
2+
import { Node, SyntaxKind } from 'ts-morph';
3+
4+
import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types.js';
5+
import { info } from '../../../utils/diagnostics.js';
6+
7+
export const expressMiddlewareTransform: Transform = {
8+
name: 'Express middleware signature migration',
9+
id: 'express-middleware',
10+
apply(sourceFile: SourceFile, _context: TransformContext): TransformResult {
11+
const diagnostics: Diagnostic[] = [];
12+
let changesCount = 0;
13+
14+
changesCount += rewriteHostHeaderValidation(sourceFile, diagnostics);
15+
16+
return { changesCount, diagnostics };
17+
}
18+
};
19+
20+
function rewriteHostHeaderValidation(sourceFile: SourceFile, diagnostics: Diagnostic[]): number {
21+
let changesCount = 0;
22+
23+
const calls = sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression);
24+
25+
for (const call of calls) {
26+
const expr = call.getExpression();
27+
if (!Node.isIdentifier(expr) || expr.getText() !== 'hostHeaderValidation') continue;
28+
29+
const args = call.getArguments();
30+
if (args.length !== 1) continue;
31+
32+
const firstArg = args[0]!;
33+
if (!Node.isObjectLiteralExpression(firstArg)) continue;
34+
35+
const allowedHostsProp = firstArg.getProperty('allowedHosts');
36+
if (!allowedHostsProp || !Node.isPropertyAssignment(allowedHostsProp)) continue;
37+
38+
const initializer = allowedHostsProp.getInitializer();
39+
if (!initializer) continue;
40+
41+
const arrayText = initializer.getText();
42+
firstArg.replaceWithText(arrayText);
43+
changesCount++;
44+
45+
diagnostics.push(
46+
info(
47+
sourceFile.getFilePath(),
48+
call.getStartLineNumber(),
49+
'hostHeaderValidation({ allowedHosts: [...] }) simplified to hostHeaderValidation([...]). Verify the migration.'
50+
)
51+
);
52+
}
53+
54+
return changesCount;
55+
}

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { SourceFile } from 'ts-morph';
22

33
import type { Transform, TransformContext, TransformResult } from '../../../types.js';
4+
import { renameAllReferences } from '../../../utils/astUtils.js';
45
import { warning } from '../../../utils/diagnostics.js';
56
import { addOrMergeImport, getSdkExports, getSdkImports, isTypeOnlyImport } from '../../../utils/importUtils.js';
67
import { resolveTypesPackage } from '../../../utils/projectAnalyzer.js';
@@ -53,9 +54,11 @@ export const importPathsTransform: Transform = {
5354

5455
for (const imp of sdkImports) {
5556
const specifier = imp.getModuleSpecifierValue();
56-
const namedImports = imp.getNamedImports().map(n => n.getName());
57+
const namedImports = imp.getNamedImports();
5758
const typeOnly = isTypeOnlyImport(imp);
5859
const line = imp.getStartLineNumber();
60+
const defaultImport = imp.getDefaultImport();
61+
const namespaceImport = imp.getNamespaceImport();
5962

6063
let mapping = IMPORT_MAP[specifier];
6164

@@ -84,9 +87,31 @@ export const importPathsTransform: Transform = {
8487
targetPackage = resolveTypesPackage(context, hasClientImport, hasServerImport);
8588
}
8689

87-
let resolvedNames = namedImports;
8890
if (mapping.renamedSymbols) {
89-
resolvedNames = namedImports.map(name => mapping.renamedSymbols?.[name] ?? name);
91+
for (const [oldName, newName] of Object.entries(mapping.renamedSymbols)) {
92+
renameAllReferences(sourceFile, oldName, newName);
93+
}
94+
}
95+
96+
const hasAlias = namedImports.some(n => n.getAliasNode() !== undefined);
97+
if (defaultImport || namespaceImport || hasAlias) {
98+
imp.setModuleSpecifier(targetPackage);
99+
if (mapping.renamedSymbols) {
100+
for (const n of namedImports) {
101+
const newName = mapping.renamedSymbols[n.getName()];
102+
if (newName) {
103+
n.setName(newName);
104+
}
105+
}
106+
}
107+
changesCount++;
108+
continue;
109+
}
110+
111+
const names = namedImports.map(n => n.getName());
112+
let resolvedNames = names;
113+
if (mapping.renamedSymbols) {
114+
resolvedNames = names.map(name => mapping.renamedSymbols?.[name] ?? name);
90115
}
91116

92117
addPending(targetPackage, resolvedNames, typeOnly);
Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import type { Transform } from '../../../types.js';
22
import { contextTypesTransform } from './contextTypes.js';
3+
import { expressMiddlewareTransform } from './expressMiddleware.js';
34
import { handlerRegistrationTransform } from './handlerRegistration.js';
45
import { importPathsTransform } from './importPaths.js';
56
import { mcpServerApiTransform } from './mcpServerApi.js';
67
import { mockPathsTransform } from './mockPaths.js';
8+
import { removedApisTransform } from './removedApis.js';
79
import { schemaParamRemovalTransform } from './schemaParamRemoval.js';
810
import { symbolRenamesTransform } from './symbolRenames.js';
911

@@ -14,23 +16,30 @@ import { symbolRenamesTransform } from './symbolRenames.js';
1416
// transforms depend on the rewritten import declarations.
1517
//
1618
// 2. symbolRenames runs early: renames imported symbols (e.g., McpError →
17-
// ProtocolError) that later transforms may reference.
19+
// ProtocolError) and rewrites type references (e.g., SchemaInput<T> →
20+
// StandardSchemaWithJSON.InferInput<T>).
1821
//
19-
// 3. mcpServerApi SHOULD run before contextTypes: it rewrites .tool() etc.
22+
// 3. removedApis runs after symbolRenames: handles removed Zod helpers,
23+
// IsomorphicHeaders, and StreamableHTTPError. Conceptually different
24+
// from renames — these are removals with diagnostic guidance.
25+
//
26+
// 4. mcpServerApi SHOULD run before contextTypes: it rewrites .tool() etc.
2027
// to .registerTool() etc. contextTypes handles both old and new names,
2128
// but running mcpServerApi first ensures consistent argument structure.
2229
//
23-
// 4. handlerRegistration and schemaParamRemoval are independent of each
24-
// other but both depend on importPaths having run.
30+
// 5. handlerRegistration, schemaParamRemoval, and expressMiddleware are
31+
// independent of each other but all depend on importPaths having run.
2532
//
26-
// 5. mockPaths runs last: handles test mocks and dynamic imports,
33+
// 6. mockPaths runs last: handles test mocks and dynamic imports,
2734
// independent of the other transforms.
2835
export const v1ToV2Transforms: Transform[] = [
2936
importPathsTransform,
3037
symbolRenamesTransform,
38+
removedApisTransform,
3139
mcpServerApiTransform,
3240
handlerRegistrationTransform,
3341
schemaParamRemovalTransform,
42+
expressMiddlewareTransform,
3443
contextTypesTransform,
3544
mockPathsTransform
3645
];

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,11 @@ function rewriteDynamicImports(sourceFile: SourceFile, context: TransformContext
173173
const bindingName = element.getName();
174174
const newName = resolved.renamedSymbols[bindingName];
175175
if (newName) {
176-
element.getNameNode().replaceWithText(newName);
176+
if (element.getPropertyNameNode()) {
177+
element.getPropertyNameNode()!.replaceWithText(newName);
178+
} else {
179+
element.replaceWithText(`${newName}: ${bindingName}`);
180+
}
177181
changes++;
178182
}
179183
}
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import type { SourceFile } from 'ts-morph';
2+
import { Node, SyntaxKind } from 'ts-morph';
3+
4+
import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types.js';
5+
import { renameAllReferences } from '../../../utils/astUtils.js';
6+
import { warning } from '../../../utils/diagnostics.js';
7+
import { addOrMergeImport, isAnyMcpSpecifier } from '../../../utils/importUtils.js';
8+
9+
const REMOVED_ZOD_HELPERS: Record<string, string> = {
10+
schemaToJson:
11+
"Removed in v2. Use `fromJsonSchema()` from @modelcontextprotocol/server for JSON Schema, or your schema library's native conversion.",
12+
parseSchemaAsync: "Removed in v2. Use your schema library's validation directly (e.g., Zod's `.safeParseAsync()`).",
13+
getSchemaShape: "Removed in v2. These Zod-specific introspection helpers have no v2 equivalent. Use your schema library's native API.",
14+
getSchemaDescription:
15+
"Removed in v2. These Zod-specific introspection helpers have no v2 equivalent. Use your schema library's native API.",
16+
isOptionalSchema:
17+
"Removed in v2. These Zod-specific introspection helpers have no v2 equivalent. Use your schema library's native API.",
18+
unwrapOptionalSchema:
19+
"Removed in v2. These Zod-specific introspection helpers have no v2 equivalent. Use your schema library's native API."
20+
};
21+
22+
export const removedApisTransform: Transform = {
23+
name: 'Removed API handling',
24+
id: 'removed-apis',
25+
apply(sourceFile: SourceFile, _context: TransformContext): TransformResult {
26+
const diagnostics: Diagnostic[] = [];
27+
let changesCount = 0;
28+
29+
changesCount += handleRemovedZodHelpers(sourceFile, diagnostics);
30+
changesCount += handleIsomorphicHeaders(sourceFile, diagnostics);
31+
changesCount += handleStreamableHTTPError(sourceFile, diagnostics);
32+
33+
return { changesCount, diagnostics };
34+
}
35+
};
36+
37+
function handleRemovedZodHelpers(sourceFile: SourceFile, diagnostics: Diagnostic[]): number {
38+
interface Removal {
39+
importName: string;
40+
message: string;
41+
line: number;
42+
}
43+
44+
const removals: Removal[] = [];
45+
46+
for (const imp of sourceFile.getImportDeclarations()) {
47+
if (!isAnyMcpSpecifier(imp.getModuleSpecifierValue())) continue;
48+
const line = imp.getStartLineNumber();
49+
for (const namedImport of imp.getNamedImports()) {
50+
const name = namedImport.getName();
51+
const message = REMOVED_ZOD_HELPERS[name];
52+
if (message) {
53+
removals.push({ importName: name, message, line });
54+
}
55+
}
56+
}
57+
58+
for (const removal of removals) {
59+
for (const imp of sourceFile.getImportDeclarations()) {
60+
for (const namedImport of imp.getNamedImports()) {
61+
if (namedImport.getName() === removal.importName) {
62+
namedImport.remove();
63+
if (imp.getNamedImports().length === 0 && !imp.getDefaultImport() && !imp.getNamespaceImport()) {
64+
imp.remove();
65+
}
66+
break;
67+
}
68+
}
69+
}
70+
diagnostics.push(warning(sourceFile.getFilePath(), removal.line, `${removal.importName}: ${removal.message}`));
71+
}
72+
73+
return removals.length;
74+
}
75+
76+
function handleIsomorphicHeaders(sourceFile: SourceFile, diagnostics: Diagnostic[]): number {
77+
let changesCount = 0;
78+
let foundImport: ReturnType<ReturnType<SourceFile['getImportDeclarations']>[0]['getNamedImports']>[0] | undefined;
79+
let foundImportDecl: ReturnType<SourceFile['getImportDeclarations']>[0] | undefined;
80+
81+
for (const imp of sourceFile.getImportDeclarations()) {
82+
if (!isAnyMcpSpecifier(imp.getModuleSpecifierValue())) continue;
83+
for (const namedImport of imp.getNamedImports()) {
84+
if (namedImport.getName() === 'IsomorphicHeaders') {
85+
foundImport = namedImport;
86+
foundImportDecl = imp;
87+
break;
88+
}
89+
}
90+
if (foundImport) break;
91+
}
92+
93+
if (!foundImport || !foundImportDecl) return 0;
94+
95+
const line = foundImportDecl.getStartLineNumber();
96+
97+
renameAllReferences(sourceFile, 'IsomorphicHeaders', 'Headers');
98+
changesCount++;
99+
100+
foundImport.remove();
101+
if (foundImportDecl.getNamedImports().length === 0 && !foundImportDecl.getDefaultImport() && !foundImportDecl.getNamespaceImport()) {
102+
foundImportDecl.remove();
103+
}
104+
changesCount++;
105+
106+
diagnostics.push(
107+
warning(
108+
sourceFile.getFilePath(),
109+
line,
110+
'IsomorphicHeaders replaced with standard Web Headers API. Note: Headers uses .get()/.set() methods, not bracket access.'
111+
)
112+
);
113+
114+
return changesCount;
115+
}
116+
117+
function handleStreamableHTTPError(sourceFile: SourceFile, diagnostics: Diagnostic[]): number {
118+
let changesCount = 0;
119+
let foundImport: ReturnType<ReturnType<SourceFile['getImportDeclarations']>[0]['getNamedImports']>[0] | undefined;
120+
let foundImportDecl: ReturnType<SourceFile['getImportDeclarations']>[0] | undefined;
121+
122+
for (const imp of sourceFile.getImportDeclarations()) {
123+
if (!isAnyMcpSpecifier(imp.getModuleSpecifierValue())) continue;
124+
for (const namedImport of imp.getNamedImports()) {
125+
if (namedImport.getName() === 'StreamableHTTPError') {
126+
foundImport = namedImport;
127+
foundImportDecl = imp;
128+
break;
129+
}
130+
}
131+
if (foundImport) break;
132+
}
133+
134+
if (!foundImport || !foundImportDecl) return 0;
135+
136+
const line = foundImportDecl.getStartLineNumber();
137+
const moduleSpec = foundImportDecl.getModuleSpecifierValue();
138+
139+
for (const node of sourceFile.getDescendantsOfKind(SyntaxKind.NewExpression)) {
140+
const expr = node.getExpression();
141+
if (!Node.isIdentifier(expr) || expr.getText() !== 'StreamableHTTPError') continue;
142+
diagnostics.push(
143+
warning(
144+
sourceFile.getFilePath(),
145+
node.getStartLineNumber(),
146+
'new StreamableHTTPError(statusCode, statusText, body?) → new SdkError(code, message, data?). ' +
147+
'Constructor arguments differ — manual review required. Map HTTP status to SdkErrorCode enum value.'
148+
)
149+
);
150+
}
151+
152+
renameAllReferences(sourceFile, 'StreamableHTTPError', 'SdkError');
153+
changesCount++;
154+
155+
foundImport.remove();
156+
if (foundImportDecl.getNamedImports().length === 0 && !foundImportDecl.getDefaultImport() && !foundImportDecl.getNamespaceImport()) {
157+
foundImportDecl.remove();
158+
}
159+
160+
const targetModule = resolveTargetModule(sourceFile, moduleSpec);
161+
const insertIndex = sourceFile.getImportDeclarations().length;
162+
addOrMergeImport(sourceFile, targetModule, ['SdkError', 'SdkErrorCode'], false, insertIndex);
163+
changesCount++;
164+
165+
diagnostics.push(
166+
warning(
167+
sourceFile.getFilePath(),
168+
line,
169+
'StreamableHTTPError replaced with SdkError. Constructor arguments differ — manual review required. ' +
170+
'HTTP status is now in error.data?.status.'
171+
)
172+
);
173+
174+
return changesCount;
175+
}
176+
177+
function resolveTargetModule(sourceFile: SourceFile, originalModule: string): string {
178+
const imp = sourceFile.getImportDeclarations().find(i => {
179+
const spec = i.getModuleSpecifierValue();
180+
return spec === '@modelcontextprotocol/client' || spec === '@modelcontextprotocol/server';
181+
});
182+
if (imp) return imp.getModuleSpecifierValue();
183+
184+
if (originalModule.includes('/client')) return '@modelcontextprotocol/client';
185+
return '@modelcontextprotocol/server';
186+
}

0 commit comments

Comments
 (0)