Skip to content

Commit 79e9f65

Browse files
committed
refactor(@angular/cli): Change modernize to use a host interface for OS/FS operations
1 parent aac3736 commit 79e9f65

File tree

4 files changed

+253
-78
lines changed

4 files changed

+253
-78
lines changed
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
/**
10+
* @fileoverview
11+
* This file defines an abstraction layer for operating-system or file-system operations, such as
12+
* command execution. This allows for easier testing by enabling the injection of mock or
13+
* test-specific implementations.
14+
*/
15+
16+
import { spawn } from 'node:child_process';
17+
import { existsSync as nodeExistsSync } from 'fs';
18+
import { Stats } from 'node:fs';
19+
import { stat } from 'node:fs/promises';
20+
21+
/**
22+
* An error thrown when a command fails to execute.
23+
*/
24+
export class CommandError extends Error {
25+
constructor(
26+
message: string,
27+
public readonly stdout: string,
28+
public readonly stderr: string,
29+
public readonly code: number | null,
30+
) {
31+
super(message);
32+
}
33+
}
34+
35+
/**
36+
* An abstraction layer for side-effectful operations.
37+
*/
38+
export interface Host {
39+
/**
40+
* Gets the stats of a file or directory.
41+
* @param path The path to the file or directory.
42+
* @returns A promise that resolves to the stats.
43+
*/
44+
stat(path: string): Promise<Stats>;
45+
46+
/**
47+
* Checks if a path exists on the file system.
48+
* @param path The path to check.
49+
* @returns A boolean indicating whether the path exists.
50+
*/
51+
existsSync(path: string): boolean;
52+
53+
/**
54+
* Spawns a child process and returns a promise that resolves with the process's
55+
* output or rejects with a structured error.
56+
* @param command The command to run.
57+
* @param args The arguments to pass to the command.
58+
* @param options Options for the child process.
59+
* @returns A promise that resolves with the standard output and standard error of the command.
60+
*/
61+
runCommand(
62+
command: string,
63+
args: readonly string[],
64+
options?: {
65+
timeout?: number;
66+
stdio?: 'pipe' | 'ignore';
67+
cwd?: string;
68+
env?: Record<string, string>;
69+
},
70+
): Promise<{ stdout: string; stderr: string }>;
71+
}
72+
73+
/**
74+
* A concrete implementation of the `Host` interface that uses the Node.js APIs.
75+
*/
76+
export const LocalWorkspaceHost: Host = {
77+
stat,
78+
existsSync: nodeExistsSync,
79+
runCommand: async (
80+
command: string,
81+
args: readonly string[],
82+
options: {
83+
timeout?: number;
84+
stdio?: 'pipe' | 'ignore';
85+
cwd?: string;
86+
env?: Record<string, string>;
87+
} = {},
88+
): Promise<{ stdout: string; stderr: string }> => {
89+
const signal = options.timeout ? AbortSignal.timeout(options.timeout) : undefined;
90+
91+
return new Promise((resolve, reject) => {
92+
const childProcess = spawn(command, args, {
93+
shell: false,
94+
stdio: options.stdio ?? 'pipe',
95+
signal,
96+
cwd: options.cwd,
97+
env: {
98+
...process.env,
99+
...options.env,
100+
},
101+
});
102+
103+
let stdout = '';
104+
childProcess.stdout?.on('data', (data) => (stdout += data.toString()));
105+
106+
let stderr = '';
107+
childProcess.stderr?.on('data', (data) => (stderr += data.toString()));
108+
109+
childProcess.on('close', (code) => {
110+
if (code === 0) {
111+
resolve({ stdout, stderr });
112+
} else {
113+
const message = `Process exited with code ${code}.`;
114+
reject(new CommandError(message, stdout, stderr, code));
115+
}
116+
});
117+
118+
childProcess.on('error', (err) => {
119+
if (err.name === 'AbortError') {
120+
const message = `Process timed out.`;
121+
reject(new CommandError(message, stdout, stderr, null));
122+
123+
return;
124+
}
125+
const message = `Process failed with error: ${err.message}`;
126+
reject(new CommandError(message, stdout, stderr, null));
127+
});
128+
});
129+
},
130+
};

packages/angular/cli/src/commands/mcp/tools/modernize.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,9 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { existsSync } from 'fs';
10-
import { stat } from 'fs/promises';
119
import { dirname, join, relative } from 'path';
1210
import { z } from 'zod';
13-
import { execAsync } from './process-executor';
11+
import { CommandError, Host, LocalWorkspaceHost } from '../host';
1412
import { McpToolDeclaration, declareTool } from './tool-registry';
1513

1614
interface Transformation {
@@ -102,10 +100,10 @@ function createToolOutput(structuredContent: ModernizeOutput) {
102100
};
103101
}
104102

105-
function findAngularJsonDir(startDir: string): string | null {
103+
function findAngularJsonDir(startDir: string, host: Host): string | null {
106104
let currentDir = startDir;
107105
while (true) {
108-
if (existsSync(join(currentDir, 'angular.json'))) {
106+
if (host.existsSync(join(currentDir, 'angular.json'))) {
109107
return currentDir;
110108
}
111109
const parentDir = dirname(currentDir);
@@ -116,7 +114,7 @@ function findAngularJsonDir(startDir: string): string | null {
116114
}
117115
}
118116

119-
export async function runModernization(input: ModernizeInput) {
117+
export async function runModernization(input: ModernizeInput, host: Host) {
120118
const transformationNames = input.transformations ?? [];
121119
const directories = input.directories ?? [];
122120

@@ -138,9 +136,9 @@ export async function runModernization(input: ModernizeInput) {
138136
}
139137

140138
const firstDir = directories[0];
141-
const executionDir = (await stat(firstDir)).isDirectory() ? firstDir : dirname(firstDir);
139+
const executionDir = (await host.stat(firstDir)).isDirectory() ? firstDir : dirname(firstDir);
142140

143-
const angularProjectRoot = findAngularJsonDir(executionDir);
141+
const angularProjectRoot = findAngularJsonDir(executionDir, host);
144142
if (!angularProjectRoot) {
145143
return createToolOutput({
146144
instructions: ['Could not find an angular.json file in the current or parent directories.'],
@@ -164,9 +162,12 @@ export async function runModernization(input: ModernizeInput) {
164162
// Simple case, run the command.
165163
for (const dir of directories) {
166164
const relativePath = relative(angularProjectRoot, dir) || '.';
167-
const command = `ng generate @angular/core:${transformation.name} --path ${relativePath}`;
165+
const command = 'ng';
166+
const args = ['generate', `@angular/core:${transformation.name}`, '--path', relativePath];
168167
try {
169-
const { stdout, stderr } = await execAsync(command, { cwd: angularProjectRoot });
168+
const { stdout, stderr } = await host.runCommand(command, args, {
169+
cwd: angularProjectRoot,
170+
});
170171
if (stdout) {
171172
stdoutMessages.push(stdout);
172173
}
@@ -177,6 +178,14 @@ export async function runModernization(input: ModernizeInput) {
177178
`Migration ${transformation.name} on directory ${relativePath} completed successfully.`,
178179
);
179180
} catch (e) {
181+
if (e instanceof CommandError) {
182+
if (e.stdout) {
183+
stdoutMessages.push(e.stdout);
184+
}
185+
if (e.stderr) {
186+
stderrMessages.push(e.stderr);
187+
}
188+
}
180189
stderrMessages.push((e as Error).message);
181190
instructions.push(
182191
`Migration ${transformation.name} on directory ${relativePath} failed.`,
@@ -228,5 +237,5 @@ ${TRANSFORMATIONS.map((t) => ` * ${t.name}: ${t.description}`).join('\n')}
228237
outputSchema: modernizeOutputSchema.shape,
229238
isLocalOnly: true,
230239
isReadOnly: false,
231-
factory: () => runModernization,
240+
factory: () => (input) => runModernization(input, LocalWorkspaceHost),
232241
});

0 commit comments

Comments
 (0)