Skip to content

Commit d23a24c

Browse files
committed
fixes
1 parent f513c38 commit d23a24c

7 files changed

Lines changed: 144 additions & 3 deletions

File tree

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Node, SyntaxKind } from 'ts-morph';
44
import { SPEC_SCHEMA_NAMES, specSchemaToTypeName } from '../../../generated/specSchemaMap.js';
55
import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types.js';
66
import { warning } from '../../../utils/diagnostics.js';
7-
import { addOrMergeImport, isAnyMcpSpecifier } from '../../../utils/importUtils.js';
7+
import { addOrMergeImport, isAnyMcpSpecifier, removeUnusedImport } from '../../../utils/importUtils.js';
88

99
export const specSchemaAccessTransform: Transform = {
1010
name: 'Spec schema standalone usage',
@@ -27,6 +27,7 @@ export const specSchemaAccessTransform: Transform = {
2727
const result = handleReference(ref, localName, typeName, sourceFile, diagnostics);
2828
if (result) changesCount++;
2929
}
30+
removeUnusedImport(sourceFile, localName, true);
3031
}
3132

3233
return { changesCount, diagnostics };
@@ -130,6 +131,31 @@ function handleReference(
130131
return false;
131132
}
132133

134+
if (parent && Node.isExportSpecifier(parent)) {
135+
diagnostics.push(
136+
warning(
137+
sourceFile.getFilePath(),
138+
ref.getStartLineNumber(),
139+
`Re-export of ${localName} requires manual update: replace with specTypeSchemas.${typeName} or remove.`
140+
)
141+
);
142+
return false;
143+
}
144+
145+
if (parent && Node.isShorthandPropertyAssignment(parent)) {
146+
const line = ref.getStartLineNumber();
147+
parent.replaceWithText(`${localName}: specTypeSchemas.${typeName}`);
148+
ensureImport(sourceFile, 'specTypeSchemas');
149+
diagnostics.push(
150+
warning(
151+
sourceFile.getFilePath(),
152+
line,
153+
`Replaced ${localName} with specTypeSchemas.${typeName}. Note: typed as StandardSchemaV1, not ZodType — Zod methods like .safeParse()/.parse() are not available.`
154+
)
155+
);
156+
return true;
157+
}
158+
133159
// Value position: replace identifier with specTypeSchemas.X
134160
const line = ref.getStartLineNumber();
135161
ref.replaceWithText(`specTypeSchemas.${typeName}`);

packages/codemod/src/runner.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ export function run(migration: Migration, options: RunnerOptions): RunnerResult
9494
fileDiagnostics.push(error(filePath, 1, `Transform failed: ${error_ instanceof Error ? error_.message : String(error_)}`));
9595
sourceFile.replaceWithText(originalText);
9696
fileChanges = 0;
97+
fileUsedPackages.clear();
9798
}
9899

99100
if (fileChanges > 0 || fileDiagnostics.length > 0) {

packages/codemod/src/utils/importUtils.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ export function addOrMergeImport(
6262
}
6363

6464
export function isAnyMcpSpecifier(specifier: string): boolean {
65-
return isSdkSpecifier(specifier) || V2_PACKAGES.has(specifier);
65+
if (isSdkSpecifier(specifier)) return true;
66+
if (V2_PACKAGES.has(specifier)) return true;
67+
const secondSlash = specifier.indexOf('/', specifier.indexOf('/') + 1);
68+
return secondSlash !== -1 && V2_PACKAGES.has(specifier.slice(0, secondSlash));
6669
}
6770

6871
export function hasMcpImports(sourceFile: SourceFile): boolean {

packages/codemod/src/utils/packageJsonUpdater.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ import { findPackageJson } from './projectAnalyzer.js';
77
const V1_PACKAGE = '@modelcontextprotocol/sdk';
88
const PRIVATE_PACKAGES = new Set(['@modelcontextprotocol/core']);
99

10+
function normalizeToRoot(pkg: string): string {
11+
const secondSlash = pkg.indexOf('/', pkg.indexOf('/') + 1);
12+
if (secondSlash === -1) return pkg;
13+
return pkg.slice(0, secondSlash);
14+
}
15+
1016
function detectIndent(text: string): string {
1117
const match = text.match(/\n([ \t]+)/);
1218
return match ? match[1]! : ' ';
@@ -31,7 +37,9 @@ export function updatePackageJson(targetDir: string, usedPackages: Set<string>,
3137
const inDevDeps = devDeps !== undefined && V1_PACKAGE in devDeps;
3238
if (!inDeps && !inDevDeps) return undefined;
3339

34-
const packagesToAdd = [...usedPackages].filter(pkg => !PRIVATE_PACKAGES.has(pkg) && pkg in V2_PACKAGE_VERSIONS);
40+
const packagesToAdd = [...new Set([...usedPackages].map(pkg => normalizeToRoot(pkg)))].filter(
41+
pkg => !PRIVATE_PACKAGES.has(pkg) && pkg in V2_PACKAGE_VERSIONS
42+
);
3543

3644
// Determine which section to add v2 packages to.
3745
// If v1 SDK was in both, prefer dependencies.

packages/codemod/test/packageJsonUpdater.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,38 @@ describe('updatePackageJson', () => {
241241
expect(result).toBeUndefined();
242242
});
243243

244+
it('normalizes subpath packages to root before adding to package.json', () => {
245+
const dir = createTempDir();
246+
writePkgJson(dir, {
247+
dependencies: {
248+
'@modelcontextprotocol/sdk': '^1.0.0'
249+
}
250+
});
251+
252+
const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/client/stdio']), false);
253+
254+
expect(result).toBeDefined();
255+
expect(result!.added).toContain('@modelcontextprotocol/client');
256+
257+
const pkg = readPkgJson(dir);
258+
const deps = pkg.dependencies as Record<string, string>;
259+
expect(deps['@modelcontextprotocol/client']).toBeDefined();
260+
});
261+
262+
it('deduplicates root and subpath packages', () => {
263+
const dir = createTempDir();
264+
writePkgJson(dir, {
265+
dependencies: {
266+
'@modelcontextprotocol/sdk': '^1.0.0'
267+
}
268+
});
269+
270+
const result = updatePackageJson(dir, new Set(['@modelcontextprotocol/client', '@modelcontextprotocol/client/stdio']), false);
271+
272+
expect(result).toBeDefined();
273+
expect(result!.added.filter(p => p === '@modelcontextprotocol/client')).toHaveLength(1);
274+
});
275+
244276
it('adds multiple v2 packages', () => {
245277
const dir = createTempDir();
246278
writePkgJson(dir, {

packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,17 @@ describe('import-paths transform', () => {
399399
expect(result).not.toContain('@modelcontextprotocol/sdk');
400400
});
401401

402+
it('includes subpath target in usedPackages for stdio-only file', () => {
403+
const project = new Project({ useInMemoryFileSystem: true });
404+
const sourceFile = project.createSourceFile(
405+
'test.ts',
406+
`import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js';\n`
407+
);
408+
const result = importPathsTransform.apply(sourceFile, { projectType: 'client' });
409+
expect(result.usedPackages).toBeDefined();
410+
expect(result.usedPackages!.has('@modelcontextprotocol/client/stdio')).toBe(true);
411+
});
412+
402413
it('resolves InMemoryTransport based on sibling client imports', () => {
403414
const input = [
404415
`import { Client } from '@modelcontextprotocol/sdk/client/index.js';`,

packages/codemod/test/v1-to-v2/transforms/specSchemaAccess.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,66 @@ describe('spec-schema-access transform', () => {
140140
});
141141
});
142142

143+
describe('import cleanup after transform', () => {
144+
it('removes original schema import after all refs are auto-transformed', () => {
145+
const input = [
146+
`import { CallToolRequestSchema } from '@modelcontextprotocol/server';`,
147+
`const valid = CallToolRequestSchema.safeParse(data).success;`,
148+
''
149+
].join('\n');
150+
const { text } = applyTransform(input);
151+
expect(text).toContain('isSpecType.CallToolRequest(data)');
152+
expect(text).not.toMatch(/import\s*\{[^}]*CallToolRequestSchema[^}]*\}/);
153+
});
154+
155+
it('keeps original schema import when some refs are diagnostic-only', () => {
156+
const input = [
157+
`import { CallToolRequestSchema } from '@modelcontextprotocol/server';`,
158+
`const valid = CallToolRequestSchema.safeParse(data).success;`,
159+
`const parsed = CallToolRequestSchema.parse(data);`,
160+
''
161+
].join('\n');
162+
const { text } = applyTransform(input);
163+
expect(text).toContain('isSpecType.CallToolRequest(data)');
164+
expect(text).toContain('CallToolRequestSchema.parse');
165+
expect(text).toMatch(/import\s*\{[^}]*CallToolRequestSchema[^}]*\}/);
166+
});
167+
168+
it('removes schema specifier from import that also has other symbols', () => {
169+
const input = [
170+
`import { CallToolRequestSchema, McpError } from '@modelcontextprotocol/server';`,
171+
`const valid = CallToolRequestSchema.safeParse(data).success;`,
172+
`throw new McpError(1, 'fail');`,
173+
''
174+
].join('\n');
175+
const { text } = applyTransform(input);
176+
expect(text).not.toMatch(/import\s*\{[^}]*CallToolRequestSchema[^}]*\}/);
177+
expect(text).toContain('McpError');
178+
expect(text).toContain(`@modelcontextprotocol/server`);
179+
});
180+
});
181+
182+
describe('parent-kind guards', () => {
183+
it('emits diagnostic for re-exported schema (ExportSpecifier)', () => {
184+
const input = [
185+
`import { CallToolRequestSchema } from '@modelcontextprotocol/server';`,
186+
`export { CallToolRequestSchema };`,
187+
''
188+
].join('\n');
189+
const { text, result } = applyTransform(input);
190+
expect(text).toContain('export { CallToolRequestSchema }');
191+
expect(result.diagnostics.some(d => d.message.includes('Re-export'))).toBe(true);
192+
expect(result.changesCount).toBe(0);
193+
});
194+
195+
it('expands shorthand property assignment', () => {
196+
const input = [`import { ToolSchema } from '@modelcontextprotocol/server';`, `const schemas = { ToolSchema };`, ''].join('\n');
197+
const { text, result } = applyTransform(input);
198+
expect(text).toContain('ToolSchema: specTypeSchemas.Tool');
199+
expect(result.changesCount).toBeGreaterThan(0);
200+
});
201+
});
202+
143203
describe('aliased imports', () => {
144204
it('handles aliased import and references original name in diagnostic', () => {
145205
const input = [

0 commit comments

Comments
 (0)