Skip to content

Commit 6b3d798

Browse files
clydinalan-agius4
authored andcommitted
refactor(@angular/cli): introduce Host abstraction for find_examples MCP tool
This commit introduces a `Host` abstraction layer for the `find_examples` MCP tool, enhancing its testability and modularity by decoupling it from direct Node.js APIs.
1 parent 8d8ba4f commit 6b3d798

File tree

8 files changed

+202
-14
lines changed

8 files changed

+202
-14
lines changed

packages/angular/cli/src/commands/mcp/host.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
import { existsSync as nodeExistsSync } from 'fs';
1717
import { ChildProcess, spawn } from 'node:child_process';
1818
import { Stats } from 'node:fs';
19-
import { stat } from 'node:fs/promises';
19+
import { glob as nodeGlob, readFile as nodeReadFile, stat } from 'node:fs/promises';
20+
import { createRequire } from 'node:module';
2021
import { createServer } from 'node:net';
2122

2223
/**
@@ -50,6 +51,33 @@ export interface Host {
5051
*/
5152
existsSync(path: string): boolean;
5253

54+
/**
55+
* Reads a file and returns its content.
56+
* @param path The path to the file.
57+
* @param encoding The encoding to use.
58+
* @returns A promise that resolves to the file content.
59+
*/
60+
readFile(path: string, encoding: 'utf-8'): Promise<string>;
61+
62+
/**
63+
* Finds files matching a glob pattern.
64+
* @param pattern The glob pattern.
65+
* @param options Options for the glob search.
66+
* @returns An async iterable of file entries.
67+
*/
68+
glob(
69+
pattern: string,
70+
options: { cwd: string },
71+
): AsyncIterable<{ name: string; parentPath: string; isFile(): boolean }>;
72+
73+
/**
74+
* Resolves a module request from a given path.
75+
* @param request The module request to resolve.
76+
* @param from The path from which to resolve the request.
77+
* @returns The resolved module path.
78+
*/
79+
resolveModule(request: string, from: string): string;
80+
5381
/**
5482
* Spawns a child process and returns a promise that resolves with the process's
5583
* output or rejects with a structured error.
@@ -100,6 +128,19 @@ export const LocalWorkspaceHost: Host = {
100128

101129
existsSync: nodeExistsSync,
102130

131+
readFile: nodeReadFile,
132+
133+
glob: function (
134+
pattern: string,
135+
options: { cwd: string },
136+
): AsyncIterable<{ name: string; parentPath: string; isFile(): boolean }> {
137+
return nodeGlob(pattern, { ...options, withFileTypes: true });
138+
},
139+
140+
resolveModule(request: string, from: string): string {
141+
return createRequire(from).resolve(request);
142+
},
143+
103144
runCommand: async (
104145
command: string,
105146
args: readonly string[],

packages/angular/cli/src/commands/mcp/mcp-server.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { join } from 'node:path';
1111
import type { AngularWorkspace } from '../../utilities/config';
1212
import { VERSION } from '../../utilities/version';
1313
import type { DevServer } from './dev-server';
14+
import { LocalWorkspaceHost } from './host';
1415
import { registerInstructionsResource } from './resources/instructions';
1516
import { AI_TUTOR_TOOL } from './tools/ai-tutor';
1617
import { BEST_PRACTICES_TOOL } from './tools/best-practices';
@@ -115,6 +116,7 @@ equivalent actions.
115116
logger,
116117
exampleDatabasePath: join(__dirname, '../../../lib/code-examples.db'),
117118
devServers: new Map<string, DevServer>(),
119+
host: LocalWorkspaceHost,
118120
},
119121
toolDeclarations,
120122
);

packages/angular/cli/src/commands/mcp/testing/mock-host.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ import type { Host } from '../host';
1313
* This class allows spying on host methods and controlling their return values.
1414
*/
1515
export class MockHost implements Host {
16-
runCommand = jasmine.createSpy('runCommand').and.resolveTo({ stdout: '', stderr: '' });
16+
runCommand = jasmine.createSpy('runCommand').and.resolveTo({ logs: [] });
1717
stat = jasmine.createSpy('stat');
1818
existsSync = jasmine.createSpy('existsSync');
19+
readFile = jasmine.createSpy('readFile').and.resolveTo('');
20+
glob = jasmine.createSpy('glob').and.returnValue((async function* () {})());
21+
resolveModule = jasmine.createSpy('resolveRequest').and.returnValue('/dev/null');
1922
spawn = jasmine.createSpy('spawn');
2023
getAvailablePort = jasmine.createSpy('getAvailablePort');
2124
}

packages/angular/cli/src/commands/mcp/tools/examples/database-discovery.ts

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

9-
import { readFile, stat } from 'node:fs/promises';
10-
import { createRequire } from 'node:module';
119
import { dirname, isAbsolute, relative, resolve } from 'node:path';
1210
import type { McpToolContext } from '../tool-registry';
1311

@@ -36,28 +34,29 @@ const KNOWN_EXAMPLE_PACKAGES = ['@angular/core', '@angular/aria', '@angular/form
3634
*
3735
* @param workspacePath The absolute path to the user's `angular.json` file.
3836
* @param logger The MCP tool context logger for reporting warnings.
37+
* @param host The host interface for file system and module resolution operations.
3938
* @returns A promise that resolves to an array of objects, each containing a database path and source.
4039
*/
4140
export async function getVersionSpecificExampleDatabases(
4241
workspacePath: string,
4342
logger: McpToolContext['logger'],
43+
host: McpToolContext['host'],
4444
): Promise<{ dbPath: string; source: string }[]> {
45-
const workspaceRequire = createRequire(workspacePath);
4645
const databases: { dbPath: string; source: string }[] = [];
4746

4847
for (const packageName of KNOWN_EXAMPLE_PACKAGES) {
4948
// 1. Resolve the path to package.json
5049
let pkgJsonPath: string;
5150
try {
52-
pkgJsonPath = workspaceRequire.resolve(`${packageName}/package.json`);
51+
pkgJsonPath = host.resolveModule(`${packageName}/package.json`, workspacePath);
5352
} catch (e) {
5453
// This is not a warning because the user may not have all known packages installed.
5554
continue;
5655
}
5756

5857
// 2. Read and parse package.json, then find the database.
5958
try {
60-
const pkgJsonContent = await readFile(pkgJsonPath, 'utf-8');
59+
const pkgJsonContent = await host.readFile(pkgJsonPath, 'utf-8');
6160
const pkgJson = JSON.parse(pkgJsonContent);
6261
const examplesInfo = pkgJson['angular']?.examples;
6362

@@ -81,7 +80,7 @@ export async function getVersionSpecificExampleDatabases(
8180
}
8281

8382
// Check the file size to prevent reading a very large file.
84-
const stats = await stat(dbPath);
83+
const stats = await host.stat(dbPath);
8584
if (stats.size > 10 * 1024 * 1024) {
8685
// 10MB
8786
logger.warn(
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
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+
import type { Stats } from 'node:fs';
10+
import { Host } from '../../host';
11+
import { getVersionSpecificExampleDatabases } from './database-discovery';
12+
13+
describe('getVersionSpecificExampleDatabases', () => {
14+
let mockHost: jasmine.SpyObj<Host>;
15+
let mockLogger: { warn: jasmine.Spy };
16+
17+
beforeEach(() => {
18+
mockHost = jasmine.createSpyObj('Host', ['resolveModule', 'readFile', 'stat']);
19+
mockLogger = {
20+
warn: jasmine.createSpy('warn'),
21+
};
22+
});
23+
24+
it('should find a valid example database from a package', async () => {
25+
mockHost.resolveModule.and.callFake((specifier) => {
26+
if (specifier === '@angular/core/package.json') {
27+
return '/path/to/node_modules/@angular/core/package.json';
28+
}
29+
throw new Error(`Unexpected module specifier: ${specifier}`);
30+
});
31+
mockHost.readFile.and.resolveTo(
32+
JSON.stringify({
33+
name: '@angular/core',
34+
version: '18.1.0',
35+
angular: {
36+
examples: {
37+
format: 'sqlite',
38+
path: './resources/code-examples.db',
39+
},
40+
},
41+
}),
42+
);
43+
mockHost.stat.and.resolveTo({ size: 1024 } as Stats);
44+
45+
const databases = await getVersionSpecificExampleDatabases(
46+
'/path/to/workspace',
47+
mockLogger,
48+
mockHost,
49+
);
50+
51+
expect(databases.length).toBe(1);
52+
expect(databases[0].dbPath).toBe(
53+
'/path/to/node_modules/@angular/core/resources/code-examples.db',
54+
);
55+
expect(databases[0].source).toBe('package @angular/core@18.1.0');
56+
expect(mockLogger.warn).not.toHaveBeenCalled();
57+
});
58+
59+
it('should skip packages without angular.examples metadata', async () => {
60+
mockHost.resolveModule.and.returnValue('/path/to/node_modules/@angular/core/package.json');
61+
mockHost.readFile.and.resolveTo(JSON.stringify({ name: '@angular/core', version: '18.1.0' }));
62+
63+
const databases = await getVersionSpecificExampleDatabases(
64+
'/path/to/workspace',
65+
mockLogger,
66+
mockHost,
67+
);
68+
69+
expect(databases.length).toBe(0);
70+
});
71+
72+
it('should handle packages that are not found', async () => {
73+
mockHost.resolveModule.and.throwError(new Error('Cannot find module'));
74+
75+
const databases = await getVersionSpecificExampleDatabases(
76+
'/path/to/workspace',
77+
mockLogger,
78+
mockHost,
79+
);
80+
81+
expect(databases.length).toBe(0);
82+
expect(mockLogger.warn).not.toHaveBeenCalled();
83+
});
84+
85+
it('should reject database paths that attempt path traversal', async () => {
86+
mockHost.resolveModule.and.returnValue('/path/to/node_modules/@angular/core/package.json');
87+
mockHost.readFile.and.resolveTo(
88+
JSON.stringify({
89+
name: '@angular/core',
90+
version: '18.1.0',
91+
angular: {
92+
examples: {
93+
format: 'sqlite',
94+
path: '../outside-package/danger.db',
95+
},
96+
},
97+
}),
98+
);
99+
100+
const databases = await getVersionSpecificExampleDatabases(
101+
'/path/to/workspace',
102+
mockLogger,
103+
mockHost,
104+
);
105+
106+
expect(databases.length).toBe(0);
107+
expect(mockLogger.warn).toHaveBeenCalledWith(
108+
jasmine.stringMatching(/Detected a potential path traversal attempt/),
109+
);
110+
});
111+
112+
it('should skip database files larger than 10MB', async () => {
113+
mockHost.resolveModule.and.returnValue('/path/to/node_modules/@angular/core/package.json');
114+
mockHost.readFile.and.resolveTo(
115+
JSON.stringify({
116+
name: '@angular/core',
117+
version: '18.1.0',
118+
angular: {
119+
examples: {
120+
format: 'sqlite',
121+
path: './resources/code-examples.db',
122+
},
123+
},
124+
}),
125+
);
126+
mockHost.stat.and.resolveTo({ size: 11 * 1024 * 1024 } as Stats); // 11MB
127+
128+
const databases = await getVersionSpecificExampleDatabases(
129+
'/path/to/workspace',
130+
mockLogger,
131+
mockHost,
132+
);
133+
134+
expect(databases.length).toBe(0);
135+
expect(mockLogger.warn).toHaveBeenCalledWith(jasmine.stringMatching(/is larger than 10MB/));
136+
});
137+
});

packages/angular/cli/src/commands/mcp/tools/examples/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ new or evolving features.
7171
factory: createFindExampleHandler,
7272
});
7373

74-
async function createFindExampleHandler({ logger, exampleDatabasePath }: McpToolContext) {
74+
async function createFindExampleHandler({ logger, exampleDatabasePath, host }: McpToolContext) {
7575
const runtimeDb = process.env['NG_MCP_EXAMPLES_DIR']
76-
? await setupRuntimeExamples(process.env['NG_MCP_EXAMPLES_DIR'])
76+
? await setupRuntimeExamples(process.env['NG_MCP_EXAMPLES_DIR'], host)
7777
: undefined;
7878

7979
suppressSqliteWarning();
@@ -91,6 +91,7 @@ async function createFindExampleHandler({ logger, exampleDatabasePath }: McpTool
9191
const versionSpecificDbs = await getVersionSpecificExampleDatabases(
9292
input.workspacePath,
9393
logger,
94+
host,
9495
);
9596
for (const db of versionSpecificDbs) {
9697
resolvedDbs.push({ path: db.dbPath, source: db.source });

packages/angular/cli/src/commands/mcp/tools/examples/runtime-database.ts

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

9-
import { glob, readFile } from 'node:fs/promises';
109
import { join } from 'node:path';
1110
import type { DatabaseSync } from 'node:sqlite';
1211
import { z } from 'zod';
12+
import type { McpToolContext } from '../tool-registry';
1313

1414
/**
1515
* A simple YAML front matter parser.
@@ -80,7 +80,10 @@ function parseFrontmatter(content: string): Record<string, unknown> {
8080
return data;
8181
}
8282

83-
export async function setupRuntimeExamples(examplesPath: string): Promise<DatabaseSync> {
83+
export async function setupRuntimeExamples(
84+
examplesPath: string,
85+
host: McpToolContext['host'],
86+
): Promise<DatabaseSync> {
8487
const { DatabaseSync } = await import('node:sqlite');
8588
const db = new DatabaseSync(':memory:');
8689

@@ -156,12 +159,12 @@ export async function setupRuntimeExamples(examplesPath: string): Promise<Databa
156159
});
157160

158161
db.exec('BEGIN TRANSACTION');
159-
for await (const entry of glob('**/*.md', { cwd: examplesPath, withFileTypes: true })) {
162+
for await (const entry of host.glob('**/*.md', { cwd: examplesPath })) {
160163
if (!entry.isFile()) {
161164
continue;
162165
}
163166

164-
const content = await readFile(join(entry.parentPath, entry.name), 'utf-8');
167+
const content = await host.readFile(join(entry.parentPath, entry.name), 'utf-8');
165168
const frontmatter = parseFrontmatter(content);
166169

167170
const validation = frontmatterSchema.safeParse(frontmatter);

packages/angular/cli/src/commands/mcp/tools/tool-registry.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ import type { ToolAnnotations } from '@modelcontextprotocol/sdk/types';
1111
import type { ZodRawShape } from 'zod';
1212
import type { AngularWorkspace } from '../../../utilities/config';
1313
import type { DevServer } from '../dev-server';
14+
import type { Host } from '../host';
1415

1516
export interface McpToolContext {
1617
server: McpServer;
1718
workspace?: AngularWorkspace;
1819
logger: { warn(text: string): void };
1920
exampleDatabasePath?: string;
2021
devServers: Map<string, DevServer>;
22+
host: Host;
2123
}
2224

2325
export type McpToolFactory<TInput extends ZodRawShape> = (

0 commit comments

Comments
 (0)