From 167150402f17edc24b4b127450c9ebc134d1ef8e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 5 May 2026 16:04:23 -0400 Subject: [PATCH] refactor(@angular/cli): migrate MCP zoneless tool to host abstraction Migrate the zoneless migration tool to use the Host abstraction for file system operations including reading files, checking stats, and globbing. This ensures that the tool adheres to the allowed roots configured in the MCP server. By removing direct dependencies on native Node.js fs modules, the tool now benefits from the centralized injection and restriction model provided by the Host implementation. --- packages/angular/cli/src/commands/mcp/host.ts | 4 +- .../migrate-single-file.ts | 4 +- .../migrate-single-file_spec.ts | 19 ++++++---- .../migrate-test-file.ts | 21 +++++----- .../migrate-test-file_spec.ts | 8 ++-- .../onpush-zoneless-migration/ts-utils.ts | 6 +-- .../zoneless-migration.ts | 38 ++++++++++--------- 7 files changed, 57 insertions(+), 43 deletions(-) diff --git a/packages/angular/cli/src/commands/mcp/host.ts b/packages/angular/cli/src/commands/mcp/host.ts index 40586bdcd8ac..c1665be98b2f 100644 --- a/packages/angular/cli/src/commands/mcp/host.ts +++ b/packages/angular/cli/src/commands/mcp/host.ts @@ -58,7 +58,7 @@ export interface Host { * @param encoding The encoding to use. * @returns A promise that resolves to the file content. */ - readFile(path: string, encoding: 'utf-8'): Promise; + readFile(path: string, encoding: BufferEncoding): Promise; /** * Finds files matching a glob pattern. @@ -348,7 +348,7 @@ export function createRootRestrictedHost( return baseHost.existsSync(path); }, - readFile(path: string, encoding: 'utf-8') { + readFile(path: string, encoding: BufferEncoding) { checkPath(path); return baseHost.readFile(path, encoding); diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file.ts index afd6316da83d..ebbc254d4d69 100644 --- a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file.ts +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file.ts @@ -9,6 +9,7 @@ import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; import type { SourceFile } from 'typescript'; +import type { Host } from '../../host'; import { analyzeForUnsupportedZoneUses } from './analyze-for-unsupported-zone-uses'; import { migrateTestFile } from './migrate-test-file'; import { generateZonelessMigrationInstructionsForComponent } from './prompts'; @@ -20,12 +21,13 @@ const supportedStrategies: ReadonlySet = new Set(['OnPush', 'Default', ' export async function migrateSingleFile( sourceFile: SourceFile, + host: Host, extras: RequestHandlerExtra, ): Promise { const testBedSpecifier = await getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed'); const isTestFile = sourceFile.fileName.endsWith('.spec.ts') || !!testBedSpecifier; if (isTestFile) { - return migrateTestFile(sourceFile); + return migrateTestFile(sourceFile, host); } const unsupportedZoneUseResponse = await analyzeForUnsupportedZoneUses(sourceFile); diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file_spec.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file_spec.ts index 442dc2c68378..4f6337d8e434 100644 --- a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file_spec.ts +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-single-file_spec.ts @@ -9,6 +9,7 @@ import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; import ts from 'typescript'; +import { createMockHost } from '../../testing/test-utils'; import { migrateSingleFile } from './migrate-single-file'; const fakeExtras = { @@ -17,11 +18,13 @@ const fakeExtras = { } as unknown as RequestHandlerExtra; describe('migrateSingleFile', () => { + const mockHost = createMockHost(); + it('should identify test files by extension', async () => { const fileName = 'test.spec.ts'; const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result?.content[0].text).toContain( 'The test file `test.spec.ts` is not yet configured for zoneless change detection.' + @@ -34,7 +37,7 @@ describe('migrateSingleFile', () => { const content = `import { TestBed } from '@angular/core/testing';`; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result?.content[0].text).toContain( 'The test file `test.ts` is not yet configured for zoneless change detection.' + @@ -59,7 +62,7 @@ describe('migrateSingleFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result?.content[0].text).toContain( 'The component uses NgZone APIs that are incompatible with zoneless applications', @@ -80,7 +83,7 @@ describe('migrateSingleFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result).toBeNull(); }); @@ -99,7 +102,7 @@ describe('migrateSingleFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result).toBeNull(); }); @@ -117,7 +120,7 @@ describe('migrateSingleFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result?.content[0].text).toContain( 'The component does not currently use a change detection strategy, which means it may rely on Zone.js', @@ -134,7 +137,7 @@ describe('migrateSingleFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result).toBeNull(); }); @@ -144,7 +147,7 @@ describe('migrateSingleFile', () => { const content = ``; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateSingleFile(sourceFile, fakeExtras); + const result = await migrateSingleFile(sourceFile, mockHost, fakeExtras); expect(result).toBeNull(); }); diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-test-file.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-test-file.ts index e0c62eb23dc9..79b5acbec70e 100644 --- a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-test-file.ts +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate-test-file.ts @@ -6,19 +6,21 @@ * found in the LICENSE file at https://angular.dev/license */ -import { existsSync, readFileSync } from 'node:fs'; -import { glob } from 'node:fs/promises'; -import { dirname, join } from 'node:path'; +import { join } from 'node:path'; import type { SourceFile } from 'typescript'; +import { type Host } from '../../host'; import { findAngularJsonDir } from '../../workspace-utils'; import { createFixResponseForZoneTests, createProvideZonelessForTestsSetupPrompt } from './prompts'; import { loadTypescript } from './ts-utils'; import { MigrationResponse } from './types'; -export async function migrateTestFile(sourceFile: SourceFile): Promise { +export async function migrateTestFile( + sourceFile: SourceFile, + host: Host, +): Promise { const ts = await loadTypescript(); // Check if tests use zoneless either by default through `initTestEnvironment` or by explicitly calling `provideZonelessChangeDetection`. - let testsUseZonelessChangeDetection = await searchForGlobalZoneless(sourceFile.fileName); + let testsUseZonelessChangeDetection = await searchForGlobalZoneless(sourceFile.fileName, host); if (!testsUseZonelessChangeDetection) { ts.forEachChild(sourceFile, function visit(node) { if ( @@ -42,8 +44,8 @@ export async function migrateTestFile(sourceFile: SourceFile): Promise { - const angularJsonDir = findAngularJsonDir(startPath); +export async function searchForGlobalZoneless(startPath: string, host: Host): Promise { + const angularJsonDir = findAngularJsonDir(startPath, host); if (!angularJsonDir) { // Cannot determine project root, fallback to original behavior or assume false. // For now, let's assume no global setup if angular.json is not found. @@ -51,9 +53,10 @@ export async function searchForGlobalZoneless(startPath: string): Promise { + const mockHost = createMockHost(); it('should return setup prompt when zoneless is not detected', async () => { const fileName = 'test.spec.ts'; const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ESNext, true); - const result = await migrateTestFile(sourceFile); + const result = await migrateTestFile(sourceFile, mockHost); expect(result?.content[0].text).toContain( 'The test file `test.spec.ts` is not yet configured for zoneless change detection.', @@ -33,7 +35,7 @@ describe('migrateTestFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateTestFile(sourceFile); + const result = await migrateTestFile(sourceFile, mockHost); expect(result).toBeNull(); }); @@ -60,7 +62,7 @@ describe('migrateTestFile', () => { `; const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); - const result = await migrateTestFile(sourceFile); + const result = await migrateTestFile(sourceFile, mockHost); expect(result?.content[0].text).toContain( 'You must refactor these tests to work in a zoneless environment and remove the `provideZoneChangeDetection` calls.', diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts-utils.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts-utils.ts index 08a3f6f0dcfd..0c79dc31f2e0 100644 --- a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts-utils.ts +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts-utils.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.dev/license */ -import { readFileSync } from 'node:fs'; import type ts from 'typescript'; +import type { Host } from '../../host'; let typescriptModule: typeof ts; @@ -116,8 +116,8 @@ export function findImportSpecifier( } /** Creates a TypeScript source file from a file path. */ -export async function createSourceFile(file: string) { - const content = readFileSync(file, 'utf8'); +export async function createSourceFile(file: string, host: Host) { + const content = await host.readFile(file, 'utf8'); const ts = await loadTypescript(); diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts index 5e9c9db972e0..c0e1499fb0ba 100644 --- a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts @@ -8,10 +8,10 @@ import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; -import { existsSync, statSync } from 'node:fs'; -import { glob } from 'node:fs/promises'; +import { join } from 'node:path'; import type { SourceFile } from 'typescript'; import { z } from 'zod'; +import type { Host } from '../../host'; import { declareTool } from '../tool-registry'; import { analyzeForUnsupportedZoneUses } from './analyze-for-unsupported-zone-uses'; import { migrateSingleFile } from './migrate-single-file'; @@ -61,19 +61,20 @@ most important action to take in the migration journey. ), }, factory: - () => + ({ host }) => ({ fileOrDirPath }, requestHandlerExtra) => - registerZonelessMigrationTool(fileOrDirPath, requestHandlerExtra), + registerZonelessMigrationTool(fileOrDirPath, host, requestHandlerExtra), }); export async function registerZonelessMigrationTool( fileOrDirPath: string, + host: Host, extras: RequestHandlerExtra, ) { let filesWithComponents, componentTestFiles, zoneFiles, categorizationErrors; try { ({ filesWithComponents, componentTestFiles, zoneFiles, categorizationErrors } = - await discoverAndCategorizeFiles(fileOrDirPath, extras)); + await discoverAndCategorizeFiles(fileOrDirPath, host, extras)); } catch (e) { return createResponse( `Error: Could not access the specified path. Please ensure the following path is correct ` + @@ -97,7 +98,7 @@ export async function registerZonelessMigrationTool( : Array.from(filesWithComponents); for (const file of rankedFiles) { - const result = await migrateSingleFile(file, extras); + const result = await migrateSingleFile(file, host, extras); if (result !== null) { return result; } @@ -105,7 +106,7 @@ export async function registerZonelessMigrationTool( } for (const file of componentTestFiles) { - const result = await migrateTestFile(file); + const result = await migrateTestFile(file, host); if (result !== null) { return result; } @@ -124,6 +125,7 @@ export async function registerZonelessMigrationTool( async function discoverAndCategorizeFiles( fileOrDirPath: string, + host: Host, extras: RequestHandlerExtra, ) { const filePaths: string[] = []; @@ -134,19 +136,20 @@ async function discoverAndCategorizeFiles( let isDirectory: boolean; try { - isDirectory = statSync(fileOrDirPath).isDirectory(); + isDirectory = (await host.stat(fileOrDirPath)).isDirectory(); } catch (e) { // Re-throw to be handled by the main function as a user input error throw new Error(`Failed to access path: ${fileOrDirPath}`, { cause: e }); } if (isDirectory) { - for await (const file of glob(`${fileOrDirPath}/**/*.ts`)) { - filePaths.push(file); + const files = host.glob('**/*.ts', { cwd: fileOrDirPath }); + for await (const file of files) { + filePaths.push(join(file.parentPath, file.name)); } } else { filePaths.push(fileOrDirPath); - const maybeTestFile = await getTestFilePath(fileOrDirPath); + const maybeTestFile = await getTestFilePath(fileOrDirPath, host); if (maybeTestFile) { // Eagerly add the test file path for categorization. filePaths.push(maybeTestFile); @@ -159,8 +162,8 @@ async function discoverAndCategorizeFiles( const batch = filesToProcess.splice(0, CONCURRENCY_LIMIT); const results = await Promise.allSettled( batch.map(async (filePath) => { - const sourceFile = await createSourceFile(filePath); - await categorizeFile(sourceFile, extras, { + const sourceFile = await createSourceFile(filePath, host); + await categorizeFile(sourceFile, host, extras, { filesWithComponents, componentTestFiles, zoneFiles, @@ -183,6 +186,7 @@ async function discoverAndCategorizeFiles( async function categorizeFile( sourceFile: SourceFile, + host: Host, extras: RequestHandlerExtra, categorizedFiles: { filesWithComponents: Set; @@ -214,9 +218,9 @@ async function categorizeFile( ); } - const testFilePath = await getTestFilePath(sourceFile.fileName); + const testFilePath = await getTestFilePath(sourceFile.fileName, host); if (testFilePath) { - componentTestFiles.add(await createSourceFile(testFilePath)); + componentTestFiles.add(await createSourceFile(testFilePath, host)); } } else if (zoneSpecifier) { zoneFiles.add(sourceFile); @@ -271,9 +275,9 @@ async function rankComponentFilesForMigration( return componentFiles; // Fallback to original order if the response fails } -async function getTestFilePath(filePath: string): Promise { +async function getTestFilePath(filePath: string, host: Host): Promise { const testFilePath = filePath.replace(/\.ts$/, '.spec.ts'); - if (existsSync(testFilePath)) { + if (host.existsSync(testFilePath)) { return testFilePath; }