Skip to content

feat(@angular/cli): add "test" and "e2e" MCP tools, and some test cleanup #32066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amishne wants to merge 3 commits into
base: main
Choose a base branch
from
+786 −67
Open

feat(@angular/cli): add "test" and "e2e" MCP tools, and some test cleanup #32066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
626a430
feat(@angular/cli): Add "all" as an experimental tool group
amishne Dec 3, 2025
c61bfb9
feat(@angular/cli): add 'test' and 'e2e' MCP tools
amishne Dec 3, 2025
04d6890
refactor(@angular/cli): clean up MCP tests and use real project name …
amishne Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions packages/angular/cli/src/commands/mcp/devserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ export interface Devserver {
port: number;
}

export function devserverKey(project?: string) {
return project ?? '<default>';
}

/**
* A local Angular development server managed by the MCP server.
*/
Expand Down
29 changes: 19 additions & 10 deletions packages/angular/cli/src/commands/mcp/mcp-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,19 @@ import { DEVSERVER_START_TOOL } from './tools/devserver/devserver-start';
import { DEVSERVER_STOP_TOOL } from './tools/devserver/devserver-stop';
import { DEVSERVER_WAIT_FOR_BUILD_TOOL } from './tools/devserver/devserver-wait-for-build';
import { DOC_SEARCH_TOOL } from './tools/doc-search';
import { E2E_TOOL } from './tools/e2e';
import { FIND_EXAMPLE_TOOL } from './tools/examples/index';
import { MODERNIZE_TOOL } from './tools/modernize';
import { ZONELESS_MIGRATION_TOOL } from './tools/onpush-zoneless-migration/zoneless-migration';
import { LIST_PROJECTS_TOOL } from './tools/projects';
import { TEST_TOOL } from './tools/test';
import { type AnyMcpToolDeclaration, registerTools } from './tools/tool-registry';

/**
* Tools to manage devservers. Should be bundled together, then added to experimental or stable as a group.
*/
const DEVSERVER_TOOLS = [DEVSERVER_START_TOOL, DEVSERVER_STOP_TOOL, DEVSERVER_WAIT_FOR_BUILD_TOOL];

/**
* Experimental tools that are grouped together under a single name.
*
* Used for enabling them as a group.
*/
export const EXPERIMENTAL_TOOL_GROUPS = {
'devserver': DEVSERVER_TOOLS,
};

/**
* The set of tools that are enabled by default for the MCP server.
* These tools are considered stable and suitable for general use.
Expand All @@ -57,7 +50,23 @@ const STABLE_TOOLS = [
* The set of tools that are available but not enabled by default.
* These tools are considered experimental and may have limitations.
*/
export const EXPERIMENTAL_TOOLS = [BUILD_TOOL, MODERNIZE_TOOL, ...DEVSERVER_TOOLS] as const;
export const EXPERIMENTAL_TOOLS = [
BUILD_TOOL,
E2E_TOOL,
MODERNIZE_TOOL,
TEST_TOOL,
...DEVSERVER_TOOLS,
] as const;

/**
* Experimental tools that are grouped together under a single name.
*
* Used for enabling them as a group.
*/
export const EXPERIMENTAL_TOOL_GROUPS = {
'all': EXPERIMENTAL_TOOLS,
'devserver': DEVSERVER_TOOLS,
};

export async function createMcpServer(
options: {
Expand Down
91 changes: 91 additions & 0 deletions packages/angular/cli/src/commands/mcp/testing/test-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/

import { workspaces } from '@angular-devkit/core';
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
import { AngularWorkspace } from '../../../utilities/config';
import { type Devserver } from '../devserver';
import { Host } from '../host';
import { McpToolContext } from '../tools/tool-registry';
import { MockHost } from './mock-host';

/**
* Creates a mock implementation of the Host interface for testing purposes.
* Each method is a Jasmine spy that can be configured.
*/
export function createMockHost(): MockHost {
Copy link
Collaborator

@dgp1130 dgp1130 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: Is there a path to a "fake" host instead of a "mock" host? Could we provide fake implementations for stat and existsSync if we accept a virtual file system (or create a real one in a temp directly from inputs)?

Faking implementation of runCommand and spawn is likely a bit harder, but we could either delegate the command to an input function or actually run a real subprocess to get more accurate error behavior. I'm thinking something along the lines of:

import * as fs from 'fs/promises';

class FakeHost implements Host {
  private nextPort = 0;

  private constructor(private readonly cmdImpls: Record<string, (...args: string[]) => CommandResult>, private readonly root: string) {}

  static async from(cmdImpls: Record<string, (...args: string[]) => CommandResult>, fs: Record<string, string | Buffer> = {}): Promise<FakeHost> {
    const tempDir = await fs.mkdtemp();

    for (const [path, content] of Object.entries(fs)) {
      await fs.writeFile(path, content);
    }

    return new FakeHost(cmdImpls, tempDir);
  }

  stat() { /* Check in `root` */ }
  existsSync() { /* Check in `root` */ }

  getAvailablePort(): number {
    return this.nextPort++;
  }

  runCommand(binary: string, args: string[]): /* ... */ {
    return this.cmdImpls[binary](args);
  }
  spawn() { /* Similar to `runCommand`... */ }

  await dispose(): Promise<void> {
    await fs.rmdir(this.root);
  }
}

I'm thinking this can avoid the need to define spies unique to each test and increase overall test confidence. If we can make this specific to each test as suggested in a separate comment, it can potentially remove the need to mutate the environment after the beforeEach within a test.

Downside is that we do need to explicitly call dispose to clean up the temp directory if we're using a real file system. If we can run these tests on Node 24, we can potentially get away with using and Symbol.dispose which would help.

it('does a thing', () => {
  await using host = await FakeHost.from(...);
  await runE2e(host, ...);
  // Host implicitly disposed at end of scope.
});

Copy link
Contributor Author

@amishne amishne Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not that fond of creating actual temp files or folders here, especially since the the tests are intended to test how our local code behaves, not how ng responds to it. I think the right approach to improve confidence would be to add end-to-end tests to this, that actually invoke the real ng on a real toy app and verify the results. I prefer to do that in a follow-up though, especially after the workspace/project lookup change.

return {
runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }),
stat: jasmine.createSpy<Host['stat']>('stat'),
existsSync: jasmine.createSpy<Host['existsSync']>('existsSync'),
spawn: jasmine.createSpy<Host['spawn']>('spawn'),
getAvailablePort: jasmine
.createSpy<Host['getAvailablePort']>('getAvailablePort')
.and.resolveTo(0),
Comment on lines +27 to +29
Copy link
Collaborator

@dgp1130 dgp1130 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: Could we put the auto-incrementing port number implementation here and potentially reuse it across all affected tests?

Copy link
Contributor Author

@amishne amishne Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one test currently looks at this, I prefer to not introduce more here until we need it more widely.

} as unknown as MockHost;
}

/**
* Options for configuring the mock MCP tool context.
*/
export interface MockContextOptions {
/** An optional pre-configured mock host. If not provided, a default mock host will be created. */
host?: MockHost;

/** Initial set of projects to populate the mock workspace with. */
projects?: Record<string, workspaces.ProjectDefinition>;
}

/**
* Creates a comprehensive mock for the McpToolContext, including a mock Host,
* an AngularWorkspace, and a ProjectDefinitionCollection. This simplifies testing
* MCP tools by providing a consistent and configurable testing environment.
* @param options Configuration options for the mock context.
* @returns An object containing the mock host, context, projects collection, and workspace instance.
*/
export function createMockContext(options: MockContextOptions = {}): {
host: MockHost;
context: McpToolContext;
projects: workspaces.ProjectDefinitionCollection;
workspace: AngularWorkspace;
} {
const host = options.host ?? createMockHost();
const projects = new workspaces.ProjectDefinitionCollection(options.projects);
const workspace = new AngularWorkspace({ projects, extensions: {} }, '/test/angular.json');

const context: McpToolContext = {
server: {} as unknown as McpServer,
workspace,
logger: { warn: () => {} },
devservers: new Map<string, Devserver>(),
host,
};

return { host, context, projects, workspace };
}

/**
* Adds a project to the provided mock ProjectDefinitionCollection.
* This is a helper function to easily populate a mock Angular workspace.
* @param projects The ProjectDefinitionCollection to add the project to.
* @param name The name of the project.
* @param targets A record of target definitions for the project (e.g., build, test, e2e).
* @param root The root path of the project, relative to the workspace root. Defaults to `projects/${name}`.
*/
export function addProjectToWorkspace(
projects: workspaces.ProjectDefinitionCollection,
name: string,
targets: Record<string, workspaces.TargetDefinition> = {},
root = `projects/${name}`,
) {
projects.set(name, {
root,
extensions: {},
targets: new workspaces.TargetDefinitionCollection(targets),
});
}
10 changes: 2 additions & 8 deletions packages/angular/cli/src/commands/mcp/tools/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { z } from 'zod';
import { CommandError, type Host } from '../host';
import { createStructuredContentOutput } from '../utils';
import { createStructuredContentOutput, getCommandErrorLogs } from '../utils';
import { type McpToolDeclaration, declareTool } from './tool-registry';

const DEFAULT_CONFIGURATION = 'development';
Expand Down Expand Up @@ -55,13 +55,7 @@ export async function runBuild(input: BuildToolInput, host: Host) {
logs = (await host.runCommand('ng', args)).logs;
} catch (e) {
status = 'failure';
if (e instanceof CommandError) {
logs = e.logs;
} else if (e instanceof Error) {
logs = [e.message];
} else {
logs = [String(e)];
}
logs = getCommandErrorLogs(e);
}

for (const line of logs) {
Expand Down
11 changes: 4 additions & 7 deletions packages/angular/cli/src/commands/mcp/tools/build_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,16 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { CommandError, Host } from '../host';
import { CommandError } from '../host';
import type { MockHost } from '../testing/mock-host';
import { createMockHost } from '../testing/test-utils';
import { runBuild } from './build';

describe('Build Tool', () => {
let mockHost: MockHost;

beforeEach(() => {
mockHost = {
runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }),
stat: jasmine.createSpy<Host['stat']>('stat'),
existsSync: jasmine.createSpy<Host['existsSync']>('existsSync'),
} as MockHost;
mockHost = createMockHost();
});

it('should construct the command correctly with default configuration', async () => {
Expand Down Expand Up @@ -82,7 +79,7 @@ describe('Build Tool', () => {
'production',
]);
expect(structuredContent.status).toBe('failure');
expect(structuredContent.logs).toEqual(buildLogs);
expect(structuredContent.logs).toEqual([...buildLogs, 'Build failed']);
expect(structuredContent.path).toBeUndefined();
});

Expand Down
20 changes: 13 additions & 7 deletions packages/angular/cli/src/commands/mcp/tools/devserver/devserver-start.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
*/

import { z } from 'zod';
import { LocalDevserver, devserverKey } from '../../devserver';
import { createStructuredContentOutput } from '../../utils';
import { LocalDevserver } from '../../devserver';
import { createStructuredContentOutput, getDefaultProjectName } from '../../utils';
import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry';

const devserverStartToolInputSchema = z.object({
Expand Down Expand Up @@ -39,12 +39,18 @@ function localhostAddress(port: number) {
}

export async function startDevserver(input: DevserverStartToolInput, context: McpToolContext) {
const projectKey = devserverKey(input.project);
const projectName = input.project ?? getDefaultProjectName(context);

let devserver = context.devservers.get(projectKey);
if (!projectName) {
return createStructuredContentOutput({
message: ['Project name not provided, and no default project found.'],
});
}

let devserver = context.devservers.get(projectName);
if (devserver) {
return createStructuredContentOutput({
message: `Development server for project '${projectKey}' is already running.`,
message: `Development server for project '${projectName}' is already running.`,
address: localhostAddress(devserver.port),
});
}
Expand All @@ -54,10 +60,10 @@ export async function startDevserver(input: DevserverStartToolInput, context: Mc
devserver = new LocalDevserver({ host: context.host, project: input.project, port });
devserver.start();

context.devservers.set(projectKey, devserver);
context.devservers.set(projectName, devserver);

return createStructuredContentOutput({
message: `Development server for project '${projectKey}' started and watching for workspace changes.`,
message: `Development server for project '${projectName}' started and watching for workspace changes.`,
address: localhostAddress(port),
});
}
Expand Down
33 changes: 26 additions & 7 deletions packages/angular/cli/src/commands/mcp/tools/devserver/devserver-stop.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
*/

import { z } from 'zod';
import { devserverKey } from '../../devserver';
import { createStructuredContentOutput } from '../../utils';
import { createStructuredContentOutput, getDefaultProjectName } from '../../utils';
import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry';

const devserverStopToolInputSchema = z.object({
Expand All @@ -30,21 +29,41 @@ const devserverStopToolOutputSchema = z.object({
export type DevserverStopToolOutput = z.infer<typeof devserverStopToolOutputSchema>;

export function stopDevserver(input: DevserverStopToolInput, context: McpToolContext) {
const projectKey = devserverKey(input.project);
const devServer = context.devservers.get(projectKey);
if (context.devservers.size === 0) {
return createStructuredContentOutput({
message: ['No development servers are currently running.'],
logs: undefined,
});
}

let projectName = input.project ?? getDefaultProjectName(context);

if (!projectName) {
// This should not happen. But if there's just a single running devserver, stop it.
Copy link
Collaborator

@dgp1130 dgp1130 Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: If this shouldn't happen, do we want to just throw and escalate the issue? I'd worry a bit that this might mask a legitimate bug elsewhere in the program.

Based on the error message below, it seems weird that it would be ignored if there happens to be just one devserver running.

Copy link
Contributor Author

@amishne amishne Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure of this myself. In general I think it's preferred to throw when something weird is going on than to hide it, but in this specific case of trying to stop a running server, I prefer to err on the side of stopping it.

if (context.devservers.size === 1) {
projectName = Array.from(context.devservers.keys())[0];
} else {
return createStructuredContentOutput({
message: ['Project name not provided, and no default project found.'],
logs: undefined,
});
}
}

const devServer = context.devservers.get(projectName);

if (!devServer) {
return createStructuredContentOutput({
message: `Development server for project '${projectKey}' was not running.`,
message: `Development server for project '${projectName}' was not running.`,
logs: undefined,
});
}

devServer.stop();
context.devservers.delete(projectKey);
context.devservers.delete(projectName);

return createStructuredContentOutput({
message: `Development server for project '${projectKey}' stopped.`,
message: `Development server for project '${projectName}' stopped.`,
logs: devServer.getServerLogs(),
});
}
Expand Down
31 changes: 26 additions & 5 deletions packages/angular/cli/src/commands/mcp/tools/devserver/devserver-wait-for-build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
*/

import { z } from 'zod';
import { devserverKey } from '../../devserver';
import { createStructuredContentOutput } from '../../utils';
import { createStructuredContentOutput, getDefaultProjectName } from '../../utils';
import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry';

/**
Expand Down Expand Up @@ -60,21 +59,43 @@ export async function waitForDevserverBuild(
input: DevserverWaitForBuildToolInput,
context: McpToolContext,
) {
const projectKey = devserverKey(input.project);
const devServer = context.devservers.get(projectKey);
const deadline = Date.now() + input.timeout;
if (context.devservers.size === 0) {
return createStructuredContentOutput({
status: 'no_devserver_found',
logs: undefined,
});
}

let projectName = input.project ?? getDefaultProjectName(context);

if (!projectName) {
// This should not happen. But if there's just a single running devserver, wait for it.
if (context.devservers.size === 1) {
projectName = Array.from(context.devservers.keys())[0];
} else {
return createStructuredContentOutput({
status: 'no_devserver_found',
logs: undefined,
});
}
}

const devServer = context.devservers.get(projectName);

if (!devServer) {
return createStructuredContentOutput<DevserverWaitForBuildToolOutput>({
status: 'no_devserver_found',
logs: undefined,
});
}

const deadline = Date.now() + input.timeout;
await wait(WATCH_DELAY);
while (devServer.isBuilding()) {
if (Date.now() > deadline) {
return createStructuredContentOutput<DevserverWaitForBuildToolOutput>({
status: 'timeout',
logs: undefined,
});
}
await wait(WATCH_DELAY);
Expand Down
Loading