diff --git a/.changeset/codemod-preserve-file-header.md b/.changeset/codemod-preserve-file-header.md new file mode 100644 index 0000000000..4603f82631 --- /dev/null +++ b/.changeset/codemod-preserve-file-header.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +Keep a file's leading comment block in place when `v1-to-v2` rewrites its imports. A rewritten import was inserted against the full start of the declaration it replaced — before that declaration's leading trivia — so a license/SPDX header stopped being the first thing in the file and the blank line separating it from the code was consumed, leaving the header attached to the next declaration as a doc comment. When the header was a multi-line `//` run and the SDK import was not the first import, the new import landed between two header lines instead. Neither is corrected by the formatter the migration guide points at. The block is now detached for the duration of the rewrite and restored verbatim; a comment above a mid-file import still travels with that import. diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts index 1d05a307f1..5324183aea 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/importPaths.ts @@ -21,6 +21,43 @@ const REEXPORT_WARNINGS: Record = { 'Re-exported StreamableHTTPError was renamed to SdkHttpError in v2 with a different constructor. Update this re-export manually.' }; +/** + * Detach a comment block anchored at byte 0 — a license/SPDX header or a file-level note. + * + * ts-morph positions an index-inserted import against the *full* start of the declaration at that + * index, i.e. ahead of that declaration's leading trivia. So a re-emitted import lands above the + * header (which then stops being the first thing in the file, and swallows the blank line below it, + * leaving the header attached to the next declaration as a doc comment), or — when the header is a + * multi-line `//` run and the SDK import is not the first import — between two of the header's own + * lines. Removing the block for the duration of the rewrite removes the trivia those positions are + * derived from, so neither can happen. + * + * Only a block starting at byte 0 is a file header. A comment above a mid-file import documents that + * import and must travel with it, which the leading-comment capture inside `apply` still handles. + * + * Returns the exact detached bytes, including the whitespace that followed the block so the blank + * line between header and code survives the round trip, or `''` when there is no leading block. + */ +function detachFileHeader(sourceFile: SourceFile): string { + const firstStatement = sourceFile.getStatements()[0]; + if (!firstStatement) return ''; + + const ranges = firstStatement.getLeadingCommentRanges(); + if (ranges.length === 0 || ranges[0]!.getPos() !== 0) return ''; + + const fullText = sourceFile.getFullText(); + let end = ranges.at(-1)!.getEnd(); + while (end < fullText.length && /\s/.test(fullText[end]!)) end++; + + const header = fullText.slice(0, end); + sourceFile.removeText(0, end); + return header; +} + +function reattachFileHeader(sourceFile: SourceFile, header: string): void { + if (header) sourceFile.insertText(0, header); +} + export const importPathsTransform: Transform = { name: 'Import path rewrites', id: 'imports', @@ -29,9 +66,14 @@ export const importPathsTransform: Transform = { const usedPackages = new Set(); let changesCount = 0; + // Detached before the AST is read: removeText() invalidates existing node references, so this + // cannot wait until after getSdkImports(). Restored at each of the three exits below. + const fileHeader = detachFileHeader(sourceFile); + const sdkImports = getSdkImports(sourceFile); const sdkExports = getSdkExports(sourceFile); if (sdkImports.length === 0 && sdkExports.length === 0) { + reattachFileHeader(sourceFile, fileHeader); return { changesCount: 0, diagnostics: [] }; } @@ -40,6 +82,7 @@ export const importPathsTransform: Transform = { changesCount += rewriteExportDeclarations(sdkExports, sourceFile, filePath, context, diagnostics, usedPackages); if (sdkImports.length === 0) { + reattachFileHeader(sourceFile, fileHeader); return { changesCount, diagnostics, usedPackages }; } @@ -362,6 +405,8 @@ export const importPathsTransform: Transform = { sourceFile.insertText(anchor ? anchor.getStart() : 0, `${leadingCommentText}\n`); } + reattachFileHeader(sourceFile, fileHeader); + return { changesCount, diagnostics, usedPackages }; } }; diff --git a/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts b/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts index 933cf02e29..1083179c50 100644 --- a/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/importPaths.test.ts @@ -204,6 +204,74 @@ describe('import-paths transform', () => { expect(result).toContain('@modelcontextprotocol/server'); }); + it('keeps a license header above the rewritten import, with its blank line intact', () => { + // #2575: the header survived in the text but the re-emitted import was inserted above it, so + // the header stopped being the first thing in the file (breaking eslint-plugin-header / + // SPDX scanners) and the blank line separating it from the code was consumed, turning the + // header into a doc comment for the next declaration. Content-only assertions miss both. + const input = [ + `// Copyright (c) 2026 Example Corp.`, + `// SPDX-License-Identifier: Apache-2.0`, + ``, + `import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js';`, + ``, + `export const ok = (): CallToolResult => ({ content: [] });`, + '' + ].join('\n'); + const result = applyTransform(input, { projectType: 'server' }); + const lines = result.split('\n'); + + expect(lines[0]).toBe('// Copyright (c) 2026 Example Corp.'); + expect(lines[1]).toBe('// SPDX-License-Identifier: Apache-2.0'); + // the blank line between header and code must survive, or the header reads as a doc comment + expect(lines[2]).toBe(''); + expect(lines.findIndex(l => l.startsWith('import'))).toBeGreaterThan(1); + expect(result).toContain('@modelcontextprotocol/server'); + }); + + it('does not split a multi-line header run when the SDK import is not the first import', () => { + // #2575, second shape: the rewritten import was inserted *inside* the leading `//` run, + // stranding line 1 above an unrelated import. + const input = [ + `// page_to_markdown tool: fetches a URL and returns clean Markdown.`, + `// Uses @page2ai/core under the hood - inherits SSRF protection and a 10MB cap.`, + `// Static tab discovery emits per-tab sections for docs sites.`, + ``, + `import { fetchAndConvert } from '@page2ai/core';`, + `import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js';`, + ``, + `export const x = (r: CallToolResult) => fetchAndConvert(r);`, + '' + ].join('\n'); + const result = applyTransform(input, { projectType: 'server' }); + const lines = result.split('\n'); + + // the three header lines stay contiguous at the top of the file + expect(lines[0]).toContain('page_to_markdown tool'); + expect(lines[1]).toContain('@page2ai/core under the hood'); + expect(lines[2]).toContain('Static tab discovery'); + expect(lines.findIndex(l => l.startsWith('import'))).toBeGreaterThan(2); + expect(result).toContain('@modelcontextprotocol/server'); + }); + + it('leaves a comment above a mid-file import attached to that import', () => { + // Guard on the scope of the header detach: only a block anchored at byte 0 is a file header. + // A comment above a later import documents that import and must not be hoisted to the top. + const input = [ + `import { fetchAndConvert } from '@page2ai/core';`, + ``, + `// Result type returned to the caller.`, + `import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js';`, + ``, + `export const x = (r: CallToolResult) => fetchAndConvert(r);`, + '' + ].join('\n'); + const result = applyTransform(input, { projectType: 'server' }); + + expect(result.split('// Result type returned to the caller.').length - 1).toBe(1); + expect(result.indexOf('// Result type returned to the caller.')).toBeGreaterThan(result.indexOf('@page2ai/core')); + }); + it('routes OAuth *Schema from sdk/shared/auth.js to core; the TYPE resolves by context', () => { // OAuthTokensSchema is a Zod schema re-exported by core (AUTH_SCHEMA_NAMES), so route it // there — `OAuthTokensSchema.parse(...)` keeps working. OAuthTokens (the type) has no schema-name