-
Notifications
You must be signed in to change notification settings - Fork 11.9k
feat(@angular/cli): support custom port in MCP devserver start tool #32855
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,13 @@ import { type McpToolContext, type McpToolDeclaration, declareTool } from '../to | |
|
|
||
| const devserverStartToolInputSchema = z.object({ | ||
| ...workspaceAndProjectOptions, | ||
| port: z | ||
| .number() | ||
| .optional() | ||
| .describe( | ||
| 'The port number to run the server on. If not provided, a random available port will be chosen. ' + | ||
| 'It is recommended to reuse port numbers across calls within the same workspace to maintain consistency.', | ||
| ), | ||
| }); | ||
|
|
||
| export type DevserverStartToolInput = z.infer<typeof devserverStartToolInputSchema>; | ||
|
|
@@ -53,7 +60,17 @@ export async function startDevserver(input: DevserverStartToolInput, context: Mc | |
| }); | ||
| } | ||
|
|
||
| const port = await context.host.getAvailablePort(); | ||
| let port: number; | ||
| if (input.port) { | ||
| if (!(await context.host.isPortAvailable(input.port))) { | ||
| throw new Error( | ||
| `Port ${input.port} is unavailable. Try calling this tool again without the 'port' parameter to auto-assign a free port.`, | ||
| ); | ||
| } | ||
| port = input.port; | ||
| } else { | ||
| port = await context.host.getAvailablePort(); | ||
| } | ||
|
Comment on lines
+63
to
+73
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Consider moving this into a function to reduce mutations. const port = getPort(input, context); |
||
|
|
||
| devserver = new LocalDevserver({ | ||
| host: context.host, | ||
|
|
@@ -87,14 +104,18 @@ the first build completes. | |
| background. | ||
| * **Get Initial Build Logs:** Once a dev server has started, use the "devserver.wait_for_build" tool to ensure it's alive. If there are any | ||
| build errors, "devserver.wait_for_build" would provide them back and you can give them to the user or rely on them to propose a fix. | ||
| * **Get Updated Build Logs:** Important: as long as a devserver is alive (i.e. "devserver.stop" wasn't called), after every time you make a | ||
| change to the workspace, re-run "devserver.wait_for_build" to see whether the change was successfully built and wait for the devserver to | ||
| be updated. | ||
| * **Get Updated Build Logs:** Important: as long as a devserver is alive (i.e. "devserver.stop" wasn't called), after every time you | ||
| make a change to the workspace, re-run "devserver.wait_for_build" to see whether the change was successfully built and wait for the | ||
| devserver to be updated. | ||
| </Use Cases> | ||
| <Operational Notes> | ||
| * This tool manages development servers by itself. It maintains at most a single dev server instance for each project in the monorepo. | ||
| * This is an asynchronous operation. Subsequent commands can be ran while the server is active. | ||
| * Use 'devserver.stop' to gracefully shut down the server and access the full log output. | ||
| * **Keeping the Server Alive**: It is often better to keep the server alive between tool calls if you expect the user to request more | ||
| changes or run more tests, as it saves time on restarts and maintains the file watcher state. You must still call | ||
| 'devserver.wait_for_build' after every change to see whether the change was successfully built and be sure that that app was updated. | ||
| * **Consistent Ports**: If making multiple calls, it is recommended to reuse the port you got from the first call for subsequent ones. | ||
| </Operational Notes> | ||
| `, | ||
| isReadOnly: true, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,31 @@ describe('Serve Tools', () => { | |
| expect(mockProcess.kill).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should use the provided port number', async () => { | ||
| const startResult = await startDevserver({ port: 54321 }, mockContext); | ||
| expect(startResult.structuredContent.message).toBe( | ||
| `Development server for project 'my-app' started and watching for workspace changes.`, | ||
| ); | ||
| expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', 'my-app', '--port=54321'], { | ||
| stdio: 'pipe', | ||
| cwd: '/test', | ||
| }); | ||
| expect(mockHost.getAvailablePort).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should throw an error if the provided port is taken', async () => { | ||
| mockHost.isPortAvailable.and.resolveTo(false); | ||
|
|
||
| try { | ||
| await startDevserver({ port: 55555 }, mockContext); | ||
| fail('Should have thrown an error'); | ||
| } catch (e) { | ||
| expect((e as Error).message).toContain( | ||
| "Port 55555 is unavailable. Try calling this tool again without the 'port' parameter to auto-assign a free port.", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: The agent can also just pick a different point. Maybe "Try using a different port or omitting the port parameter to auto-assign a free port." |
||
| ); | ||
| } | ||
| }); | ||
|
|
||
| it('should wait for a build to complete', async () => { | ||
| await startDevserver({}, mockContext); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.