Skip to content

Add build/serve MCP tools #31667

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add build/serve MCP tools #31667

wants to merge 3 commits into from

Conversation

@amishne
Copy link
Contributor

@amishne amishne commented Oct 31, 2025

New MCP experimental tools:

  • "build" tool: A new MCP tool that allows for one-off, non-watched builds of Angular projects, similar to
    ng build.
  • "serve" tools: A group of new MCP tools to manage the development server:
    • "start_devserver": Starts the Angular development server as a background process.
    • "stop_devserver": Stops a running development server.
    • "wait_for_devserver_build": Waits for a build to complete and reports the logs.

Notes:

  • Implementation mostly follows the design doc for build/serve/test workflow.
  • The currently-alive devservers are stored as part of the MCP context, and so are maintained across tool invocations.
  • Build progress detection relies on hardcoding expected strings to match - see "dev-server.ts".
  • Beyond the unit tests, also tested manually in Gemini CLI.
  • A bunch of tools, included these new ones, just return basic unweighted structured content, so added a utility function in a "utils.ts" file for that functionality.

@amishne amishne requested review from clydin and dgp1130 October 31, 2025 20:50
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: @angular/cli labels Oct 31, 2025
dgp1130
dgp1130 reviewed Oct 31, 2025
View reviewed changes
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together @amishne! Hopefully my comments should be internally consistent, but there might be a few which you addressed in later commits.

I know I left a lot of feedback, feel free to ack and ignore any comments you don't agree with or would rather follow up on separately from this PR. We definitely don't need to expand the scope any more than it already is.

I think probably my most important concern is just about whether we can reduce the dependency on specific log output and hopefully make this a little more durable to internal changes there.

packages/angular/cli/src/commands/mcp/tools/wait-for-devserver-build.ts Outdated
timeout: z
.number()
.optional()
.default(30000)
Copy link
Collaborator

@dgp1130 dgp1130 Oct 31, 2025

Choose a reason for hiding this comment

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

Question: @clydin, I'm curious to know what you think a good timeout would be here. I wonder if 30s might be too short such that some larger projects will hit this 100% of the time and the AI might not know to configure it.

Copy link
Contributor Author

@amishne amishne Nov 5, 2025

Choose a reason for hiding this comment

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

Waiting for feedback from @clydin but changing to 3 minutes for now.

Copy link
Member

@clydin clydin Nov 5, 2025

Choose a reason for hiding this comment

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

We can leave it at 3 minutes for now and adjust as we perform more testing on various size projects.

@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Nov 3, 2025
dgp1130
dgp1130 approved these changes Nov 5, 2025
View reviewed changes
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

Don't forget to squash or switch to fixup! commits before merging, so "Address review comments" doesn't end up in our permanent history (it's fine for a PR, just don't want it being merged to main).

packages/angular/cli/src/commands/mcp/tools/build.ts
if (configuration.args) {
args.push(configuration.args);
}
args.push(`-c ${input.configuration ?? DEFAULT_CONFIGURATION}`);
Copy link
Collaborator

@dgp1130 dgp1130 Nov 5, 2025

Choose a reason for hiding this comment

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

Bug: I think we want -c and input.configuration to be two different arguments?

args.push('-c', input.configuration ?? DEFAULT_CONFIGURATION);

packages/angular/cli/src/commands/mcp/tools/build.ts
stderr = e.stderr;
} else if (e instanceof Error) {
stderr = e.message;
}
Copy link
Collaborator

@dgp1130 dgp1130 Nov 5, 2025

Choose a reason for hiding this comment

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

I think this might have gotten dropped?

packages/angular/cli/src/commands/mcp/tools/wait-for-devserver-build.ts
timeout: z
.number()
.default(30000)
.default(180000)
Copy link
Collaborator

@dgp1130 dgp1130 Nov 5, 2025

Choose a reason for hiding this comment

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

Suggestion: Use an underscore separator to make the zeroes a little easier to read. I also suggest including units somewhere like:

z.default(180_000 /* ms */)

packages/angular/cli/src/commands/mcp/tools/modernize_spec.ts
return p === join(projectDir, 'angular.json');
}),
} as Partial<Host> as Host;
} as Partial<MockHost> as MockHost;
Copy link
Collaborator

@dgp1130 dgp1130 Nov 5, 2025

Choose a reason for hiding this comment

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

Consider: Is it worth using MockHost here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dgp1130 dgp1130 dgp1130 approved these changes

@clydin clydin Awaiting requested review from clydin

Assignees

No one assigned

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: @angular/cli detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amishne @dgp1130 @clydin @alan-agius4