Skip to content

Commit 3e50511

Browse files
committed
claude fixes
1 parent 5bcf7dc commit 3e50511

5 files changed

Lines changed: 60 additions & 34 deletions

File tree

packages/codemod/src/cli.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ for (const [name, migration] of listMigrations()) {
143143
if (result.commentCount > 0) {
144144
console.log(
145145
`${result.commentCount} location(s) marked with ${CODEMOD_ERROR_PREFIX} comments — search your code to find them:\n` +
146-
` grep -r '${CODEMOD_ERROR_PREFIX}' ${resolvedDir}\n`
146+
` grep -r '${CODEMOD_ERROR_PREFIX}' "${resolvedDir}"\n`
147147
);
148148
}
149149

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Node, SyntaxKind } from 'ts-morph';
33

44
import type { Diagnostic, Transform, TransformContext, TransformResult } from '../../../types.js';
55
import { actionRequired } from '../../../utils/diagnostics.js';
6-
import { isImportedFromMcp, removeUnusedImport, resolveOriginalImportName } from '../../../utils/importUtils.js';
6+
import { hasMcpImports, isImportedFromMcp, removeUnusedImport, resolveOriginalImportName } from '../../../utils/importUtils.js';
77
import { NOTIFICATION_SCHEMA_TO_METHOD, SCHEMA_TO_METHOD } from '../mappings/schemaToMethodMap.js';
88

99
const ALL_SCHEMA_TO_METHOD: Record<string, string> = {
@@ -15,6 +15,10 @@ export const handlerRegistrationTransform: Transform = {
1515
name: 'Handler registration migration',
1616
id: 'handlers',
1717
apply(sourceFile: SourceFile, _context: TransformContext): TransformResult {
18+
if (!hasMcpImports(sourceFile)) {
19+
return { changesCount: 0, diagnostics: [] };
20+
}
21+
1822
let changesCount = 0;
1923
const diagnostics: Diagnostic[] = [];
2024

packages/codemod/src/runner.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ function insertDiagnosticComments(project: Project, fileResults: FileResult[]):
3030
// Insertions below mutate sf, but we process in descending line order, so
3131
// each insertText only shifts positions above the next insertion point —
3232
// prior byte offsets stay valid.
33-
const sourceText = sf.getFullText().replaceAll('\r\n', '\n');
33+
const sourceText = sf.getFullText();
3434
const lines = sourceText.split('\n');
35+
const lineEnding = sourceText.includes('\r\n') ? '\r\n' : '\n';
3536

3637
for (const diag of merged) {
3738
const lineIndex = diag.line - 1;
@@ -44,7 +45,7 @@ function insertDiagnosticComments(project: Project, fileResults: FileResult[]):
4445
const comment = `${indent}/* ${CODEMOD_ERROR_PREFIX} ${safeMessage} */`;
4546

4647
const lineStart = lines.slice(0, lineIndex).reduce((sum, l) => sum + l.length + 1, 0);
47-
sf.insertText(lineStart, comment + '\n');
48+
sf.insertText(lineStart, comment + lineEnding);
4849
insertedCount++;
4950
}
5051
}

packages/codemod/test/commentInsertion.test.ts

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
22
import { tmpdir } from 'node:os';
33
import path from 'node:path';
4-
import { Project } from 'ts-morph';
54
import { afterEach, describe, expect, it } from 'vitest';
65

76
import { getMigration } from '../src/migrations/index.js';
87
import { run } from '../src/runner.js';
9-
import type { FileResult } from '../src/types.js';
108
import { CODEMOD_ERROR_PREFIX } from '../src/utils/diagnostics.js';
119

1210
const migration = getMigration('v1-to-v2')!;
@@ -189,33 +187,40 @@ describe('comment insertion', () => {
189187
});
190188

191189
it('merges same-line diagnostics into a single comment', () => {
192-
// Test the merge logic directly with synthetic data via the Project API.
193-
// Two diagnostics on the same line should produce one comment with joined messages.
194-
const project = new Project({ useInMemoryFileSystem: true });
195-
const sf = project.createSourceFile('test.ts', 'const a = 1;\nconst b = 2;\n');
196-
197-
const fileResults: FileResult[] = [
198-
{
199-
filePath: sf.getFilePath(),
200-
changes: 0,
201-
diagnostics: [
202-
{ level: 'warning' as never, file: sf.getFilePath(), line: 2, message: 'First issue', insertComment: true },
203-
{ level: 'warning' as never, file: sf.getFilePath(), line: 2, message: 'Second issue', insertComment: true }
204-
]
205-
}
206-
];
207-
208-
// Import and call insertDiagnosticComments indirectly by checking the runner behavior.
209-
// Since insertDiagnosticComments is not exported, we verify via integration:
210-
// construct a scenario that produces two diagnostics for the same node.
211-
// Instead, we verify the output format by checking the source file directly.
212-
// For a true unit test, we'd need to export the function. For now, verify the
213-
// merge behavior via the runner with a crafted input.
214-
215-
// Actually, we can use a file that triggers two actionRequired diagnostics on the same line.
216-
// This is hard to construct naturally, so we test the runner output instead.
217-
// The key invariant is: if we ever get same-line diagnostics, only one comment appears.
218-
// The earlier tests already cover the single-comment case. This test documents the intent.
219-
expect(fileResults[0]!.diagnostics.filter(d => d.line === 2).length).toBe(2);
190+
const dir = createTempDir();
191+
const input = [
192+
`import { CallToolRequestSchema, ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js';`,
193+
`const a = CallToolRequestSchema.parse(data1); const b = ListToolsRequestSchema.parse(data2);`,
194+
``
195+
].join('\n');
196+
writeFileSync(path.join(dir, 'server.ts'), input);
197+
198+
const result = run(migration, { targetDir: dir });
199+
200+
const output = readFileSync(path.join(dir, 'server.ts'), 'utf8');
201+
const commentLines = output.split('\n').filter(l => l.includes(CODEMOD_ERROR_PREFIX));
202+
expect(commentLines.length).toBe(1);
203+
expect(commentLines[0]).toContain(' | ');
204+
expect(result.commentCount).toBe(1);
205+
});
206+
207+
it('handles CRLF line endings without corrupting the file', () => {
208+
const dir = createTempDir();
209+
const input = [
210+
`import { CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js';`,
211+
`const a = CallToolRequestSchema.parse(data);`,
212+
``
213+
].join('\r\n');
214+
writeFileSync(path.join(dir, 'server.ts'), input);
215+
216+
run(migration, { targetDir: dir });
217+
218+
const output = readFileSync(path.join(dir, 'server.ts'), 'utf8');
219+
expect(output).toContain(CODEMOD_ERROR_PREFIX);
220+
const lines = output.split(/\r?\n/);
221+
const commentIdx = lines.findIndex(l => l.includes(CODEMOD_ERROR_PREFIX));
222+
expect(commentIdx).toBeGreaterThan(-1);
223+
expect(lines[commentIdx]!.trim()).toMatch(/^\/\*.*\*\/$/);
224+
expect(lines[commentIdx + 1]).toContain('.parse(data)');
220225
});
221226
});

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ describe('handler-registration transform', () => {
162162

163163
it('emits diagnostic for custom method schema (not in spec map)', () => {
164164
const input = [
165+
`import { Server } from '@modelcontextprotocol/sdk/server/index.js';`,
165166
`const AcmeSearch = z.object({ method: z.literal('acme/search'), params: z.object({ query: z.string() }) });`,
166167
`server.setRequestHandler(AcmeSearch, async (request) => {`,
167168
` return { items: [] };`,
@@ -179,6 +180,7 @@ describe('handler-registration transform', () => {
179180

180181
it('emits diagnostic for custom notification schema', () => {
181182
const input = [
183+
`import { Server } from '@modelcontextprotocol/sdk/server/index.js';`,
182184
`const CustomNotification = z.object({ method: z.literal('acme/notify') });`,
183185
`server.setNotificationHandler(CustomNotification, async () => {});`,
184186
''
@@ -192,6 +194,20 @@ describe('handler-registration transform', () => {
192194
expect(result.diagnostics[0]!.message).toContain('CustomNotification');
193195
});
194196

197+
it('skips files with no MCP imports', () => {
198+
const input = [
199+
`import { EventBus } from 'some-other-library';`,
200+
`const CustomSchema = z.object({ method: z.literal('custom/op') });`,
201+
`bus.setRequestHandler(CustomSchema, async (req) => {});`,
202+
''
203+
].join('\n');
204+
const project = new Project({ useInMemoryFileSystem: true });
205+
const sourceFile = project.createSourceFile('test.ts', input);
206+
const result = handlerRegistrationTransform.apply(sourceFile, ctx);
207+
expect(result.changesCount).toBe(0);
208+
expect(result.diagnostics.length).toBe(0);
209+
});
210+
195211
it('replaces ListTasksRequestSchema with method string', () => {
196212
const input = [
197213
`import { ListTasksRequestSchema } from '@modelcontextprotocol/sdk/types.js';`,

0 commit comments

Comments
 (0)