fixup! refactor(@angular/cli): add a get Zoneless/OnPush MCP tool

Author: atscott

packages/angular/cli/BUILD.bazel

@@ -70,7 +70,6 @@ ts_project(
         "//:node_modules/@types/semver",
         "//:node_modules/@types/yargs",
         "//:node_modules/@types/yarnpkg__lockfile",
-        "//:node_modules/fast-glob",
         "//:node_modules/listr2",
         "//:node_modules/semver",
         "//:node_modules/typescript",
@@ -129,7 +128,6 @@ ts_project(
         ":node_modules/yargs",
         "//:node_modules/@types/semver",
         "//:node_modules/@types/yargs",
-        "//:node_modules/fast-glob",
         "//:node_modules/semver",
     ],
 )

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/analyze_for_unsupported_zone_uses.ts

@@ -6,23 +6,25 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import ts from 'typescript';
+import type { SourceFile, Node, ImportSpecifier } from 'typescript';
 import { createUnsupportedZoneUsagesMessage } from './prompts';
-import { getImportSpecifier } from './ts_utils';
+import { getImportSpecifier, loadTypescript } from './ts_utils';
 import { MigrationResponse } from './types';
 
-export function analyzeForUnsupportedZoneUses(sourceFile: ts.SourceFile): MigrationResponse | null {
-  const ngZoneImport = getImportSpecifier(sourceFile, '@angular/core', 'NgZone');
+export async function analyzeForUnsupportedZoneUses(
+  sourceFile: SourceFile,
+): Promise<MigrationResponse | null> {
+  const ngZoneImport = await getImportSpecifier(sourceFile, '@angular/core', 'NgZone');
   if (!ngZoneImport) {
     return null;
   }
-  const unsupportedUsages = findUnsupportedZoneUsages(sourceFile, ngZoneImport);
+  const unsupportedUsages = await findUnsupportedZoneUsages(sourceFile, ngZoneImport);
 
   if (unsupportedUsages.length === 0) {
     return null;
   }
 
-  const locations = unsupportedUsages.map((node: ts.Node) => {
+  const locations = unsupportedUsages.map((node: Node) => {
     const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart());
 
     return `line ${line + 1}, character ${character + 1}: ${node.getText()}`;
@@ -37,11 +39,11 @@ export function analyzeForUnsupportedZoneUses(sourceFile: ts.SourceFile): Migrat
  * @param ngZoneImport The import specifier for `NgZone`.
  * @returns A list of nodes that are unsupported `NgZone` usages.
  */
-export function findUnsupportedZoneUsages(
-  sourceFile: ts.SourceFile,
-  ngZoneImport: ts.ImportSpecifier,
-): ts.Node[] {
-  const unsupportedUsages: ts.Node[] = [];
+export async function findUnsupportedZoneUsages(
+  sourceFile: SourceFile,
+  ngZoneImport: ImportSpecifier,
+): Promise<Node[]> {
+  const unsupportedUsages: Node[] = [];
   const ngZoneClassName = ngZoneImport.name.text;
 
   const staticMethods = new Set([
@@ -51,6 +53,7 @@ export function findUnsupportedZoneUsages(
   ]);
   const instanceMethods = new Set(['onMicrotaskEmpty', 'onStable']);
 
+  const ts = await loadTypescript();
   ts.forEachChild(sourceFile, function visit(node) {
     if (ts.isPropertyAccessExpression(node)) {
       const propertyName = node.name.text;

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file.ts

@@ -8,39 +8,40 @@
 
 import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol';
 import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types';
-import ts from 'typescript';
+import type { SourceFile } from 'typescript';
 import { analyzeForUnsupportedZoneUses } from './analyze_for_unsupported_zone_uses';
 import { migrateTestFile } from './migrate_test_file';
 import { generateZonelessMigrationInstructionsForComponent } from './prompts';
 import { sendDebugMessage } from './send_debug_message';
-import { getImportSpecifier } from './ts_utils';
+import { getImportSpecifier, loadTypescript } from './ts_utils';
 import { MigrationResponse } from './types';
 
 export async function migrateSingleFile(
-  sourceFile: ts.SourceFile,
+  sourceFile: SourceFile,
   extras: RequestHandlerExtra<ServerRequest, ServerNotification>,
 ): Promise<MigrationResponse | null> {
-  const testBedSpecifier = getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed');
+  const testBedSpecifier = await getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed');
   const isTestFile = sourceFile.fileName.endsWith('.spec.ts') || !!testBedSpecifier;
   if (isTestFile) {
     return migrateTestFile(sourceFile);
   }
 
-  const unsupportedZoneUseResponse = analyzeForUnsupportedZoneUses(sourceFile);
+  const unsupportedZoneUseResponse = await analyzeForUnsupportedZoneUses(sourceFile);
   if (unsupportedZoneUseResponse) {
     return unsupportedZoneUseResponse;
   }
 
   let detectedStrategy: 'OnPush' | 'Default' | undefined;
   let hasComponentDecorator = false;
 
-  const componentSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Component');
+  const componentSpecifier = await getImportSpecifier(sourceFile, '@angular/core', 'Component');
   if (!componentSpecifier) {
     sendDebugMessage(`No component decorator found in file: ${sourceFile.fileName}`, extras);
 
     return null;
   }
 
+  const ts = await loadTypescript();
   ts.forEachChild(sourceFile, function visit(node) {
     if (detectedStrategy) {
       return; // Already found, no need to traverse further

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file.ts

@@ -6,16 +6,16 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import { glob } from 'fast-glob';
-import * as fs from 'fs';
-import { dirname, join } from 'path';
-import ts from 'typescript';
+import { glob } from 'node:fs/promises';
+import * as fs from 'node:fs';
+import { dirname, join } from 'node:path';
+import type { SourceFile } from 'typescript';
 import { createFixResponseForZoneTests, createProvideZonelessForTestsSetupPrompt } from './prompts';
 import { MigrationResponse } from './types';
+import { loadTypescript } from './ts_utils';
 
-export async function migrateTestFile(
-  sourceFile: ts.SourceFile,
-): Promise<MigrationResponse | null> {
+export async function migrateTestFile(sourceFile: SourceFile): Promise<MigrationResponse | null> {
+  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);
   if (!testsUseZonelessChangeDetection) {
@@ -51,7 +51,7 @@ export async function searchForGlobalZoneless(startPath: string): Promise<boolea
 
   try {
     const files = await glob(`${angularJsonDir}/**/*.ts`);
-    for (const file of files) {
+    for await (const file of files) {
       const content = fs.readFileSync(file, 'utf-8');
       if (
         content.includes('initTestEnvironment') &&

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/prompts.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import ts from 'typescript';
+import type { Node, SourceFile } from 'typescript';
 import { MigrationResponse } from './types';
+import { loadTypescript } from './ts_utils';
 
 /* eslint-disable max-len */
 
@@ -28,15 +29,9 @@ export function createProvideZonelessForTestsSetupPrompt(testFilePath: string):
     + import { provideZonelessChangeDetection } from '@angular/core';
       
       describe('MyComponent', () => {
-        beforeEach(() => {
-      TestBed.configureTestingModule({
-        // ... other configuration
-        providers: [
-    +         provideZonelessChangeDetection(),
-          // ... other providers
-        ]
-      });
-        });
+   +    beforeEach(() => {
+   +      TestBed.configureTestingModule({providers: [provideZonelessChangeDetection()]});
+   +    });
       });
     \`\`\`
 
@@ -54,26 +49,17 @@ export function createProvideZonelessForTestsSetupPrompt(testFilePath: string):
       });
       // ... rest of the test
     });
-
-    // Example for a group of failing tests
-    describe('tests that require Zone.js', () => {
-      beforeEach(() => {
-        TestBed.configureTestingModule({
-      providers: [provideZoneChangeDetection()]
-        });
-      });
-
-      // ... all failing tests go here
-    });
     \`\`\`
 
     ### IMPORTANT: Rules and Constraints
 
     You must follow these rules without exception:
-    1.  **DO** add \`provideZonelessChangeDetection()\` to the top-level \`beforeEach\` block as instructed in Step 1.
-    2.  **DO** add \`provideZoneChangeDetection()\` to the providers for each individual test or group of tests that fail after the change in Step 1.
-    3.  **DO NOT** attempt to fix the logic of the failing tests at this stage. The goal is only to enable zoneless for the suite and isolate the incompatible tests.
-    4.  **DO NOT** make any other changes to the test file or any application code.
+    1.  **DO** add \`provideZonelessChangeDetection()\` _once_ to the top-most \`describe\` in a \`beforeEach\` block as instructed in Step 1.
+    2.  **DO** run the tests after adding \`provideZonelessChangeDetection\` to see which ones fail. **DO NOT** make assumptions about which tests will might fail.
+    3.  **DO** add \`provideZoneChangeDetection()\` to the providers for each individual test or group of tests that fail after the change in Step 1.
+    4.  **DO NOT** attempt to fix the logic of the failing tests at this stage. The goal is only to enable zoneless for the suite and isolate the incompatible tests.
+    5.  **DO NOT** make any other changes to the test file or any application code.
+    6.  **DO NOT** move tests around or create new describe blocks if it can be avoided. Changes are easier to review when code moves around less.
 
     ### Final Step
     After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process.`;
@@ -189,47 +175,26 @@ export function generateZonelessMigrationInstructionsForComponent(
   return createResponse(text);
 }
 
-export function createTestDebuggingGuide(fileOrDirPath: string): MigrationResponse {
+export function createTestDebuggingGuideForNonActionableInput(
+  fileOrDirPath: string,
+): MigrationResponse {
   const text = `You are an expert Angular developer assisting with a migration to zoneless.
 
 No actionable migration steps were found in the application code for \`${fileOrDirPath}\`. However, if the tests for this code are failing with zoneless enabled, the tests themselves likely need to be updated.
 
 Your task is to investigate and fix any failing tests related to the code in \`${fileOrDirPath}\`.
 
-### Test Debugging Guide
-
-When running tests in a zoneless environment, you may encounter new types of failures. Here are common issues and how to resolve them:
-
-1.  **\`ExpressionChangedAfterItHasBeenCheckedError\`**:
-  *   **Cause**: This error indicates that a value in a component's template was updated, but Angular was not notified to run change detection.
-  *   **Solution**:
-    *   If the value is in a test-only wrapper component, update the property to be a signal.
-    *   For application components, either convert the property to a signal or call \`ChangeDetectorRef.markForCheck()\` immediately after the property is updated.
-
-2.  **Asynchronous Operations and Timing**:
-  *   **Cause**: Without Zone.js, change detection is always scheduled asynchronously. Tests that previously relied on synchronous updates might now fail. The \`fixture.whenStable()\` utility also no longer waits for timers (like \`setTimeout\` or \`setInterval\`).
-  *   **Solution**:
-    *   Avoid relying on synchronous change detection.
-    *   To wait for asynchronous operations to complete, you may need to poll for an expected state, use \`fakeAsync\` with \`tick()\`, or use a mock clock to flush timers.
-
-3.  **Indirect Dependencies**:
-  *   **Cause**: The component itself might be zoneless-compatible, but it could be using a service or another dependency that is not.
-  *   **Solution**: Investigate the services and dependencies used by the component and its tests. Run this tool on those dependencies to identify and fix any issues.
-
-### IMPORTANT: Rules and Constraints
-
-You must follow these rules without exception:
-1.  **DO** focus only on fixing the tests for the code in \`${fileOrDirPath}\`.
-2.  **DO** apply the solutions described in the debugging guide to fix test failures.
-3.  **DO NOT** make changes to application code unless it is to fix a bug revealed by the zoneless migration (e.g., converting a property to a signal to fix an \`ExpressionChangedAfterItHasBeenCheckedError\`).
-4.  **DO NOT** re-introduce \`provideZoneChangeDetection()\` into tests that are already using \`provideZonelessChangeDetection()\`.
+${testDebuggingGuideText(fileOrDirPath)}
 `;
 
   return createResponse(text);
 }
 
-export function createFixResponseForZoneTests(sourceFile: ts.SourceFile): MigrationResponse | null {
-  const usages: ts.Node[] = [];
+export async function createFixResponseForZoneTests(
+  sourceFile: SourceFile,
+): Promise<MigrationResponse | null> {
+  const ts = await loadTypescript();
+  const usages: Node[] = [];
   ts.forEachChild(sourceFile, function visit(node) {
     if (
       ts.isCallExpression(node) &&
@@ -255,7 +220,16 @@ export function createFixResponseForZoneTests(sourceFile: ts.SourceFile): Migrat
 
       The following usages of \`provideZoneChangeDetection\` must be removed:
       ${locations.map((loc) => `- ${loc}`).join('\n')}
+      ${testDebuggingGuideText(sourceFile.fileName)}
 
+      ### Final Step
+      After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process.`;
+
+  return createResponse(text);
+}
+
+function testDebuggingGuideText(fileName: string) {
+  return `
       ### Test Debugging Guide
 
       After removing \`provideZoneChangeDetection\`, the tests will likely fail. Use this guide to diagnose and fix the failures.
@@ -279,15 +253,13 @@ export function createFixResponseForZoneTests(sourceFile: ts.SourceFile): Migrat
       ### IMPORTANT: Rules and Constraints
 
       You must follow these rules without exception:
-      1.  **DO** remove all usages of \`provideZoneChangeDetection\` from the test file.
-      2.  **DO** apply the solutions described in the debugging guide to fix any resulting test failures.
-      3.  **DO NOT** make changes to application code unless it is to fix a bug revealed by the zoneless migration (e.g., converting a property to a signal to fix an \`ExpressionChangedAfterItHasBeenCheckedError\`).
-      4.  **DO NOT** make any changes unrelated to fixing the failing tests in \`${sourceFile.fileName}\`.
-
-      ### Final Step
-      After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process.`;
-
-  return createResponse(text);
+      1.  **DO** focus only on fixing the tests for the code in \`${fileName}\`.
+      2.  **DO** remove all usages of \`provideZoneChangeDetection\` from the test file.
+      3.  **DO** apply the solutions described in the debugging guide to fix any resulting test failures.
+      4.  **DO** update properties of test components and directives to use signals. Tests often use plain objects and values and update the component state directly before calling \`fixture.detectChanges\`. This will not work and will result in \`ExpressionChangedAfterItHasBeenCheckedError\` because Angular was not notifed of the change.
+      5.  **DO NOT** make changes to application code unless it is to fix a bug revealed by the zoneless migration (e.g., converting a property to a signal to fix an \`ExpressionChangedAfterItHasBeenCheckedError\`).
+      6.  **DO NOT** make any changes unrelated to fixing the failing tests in \`${fileName}\`.
+      7.  **DO NOT** re-introduce \`provideZoneChangeDetection()\` into tests that are already using \`provideZonelessChangeDetection()\`.`;
 }
 
 /* eslint-enable max-len */