feat(@angular/cli): support custom port in MCP devserver start tool#32855
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to specify a port number for the development server and adds a utility to check port availability. The feedback identifies a potential race condition in the port check implementation where the promise resolves before the test server is fully closed, and points out an unused import in the test suite.
| 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(); | ||
| } |
There was a problem hiding this comment.
Nit: Consider moving this into a function to reduce mutations.
const port = getPort(input, context);| 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.", |
There was a problem hiding this comment.
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."
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Agents tend to recreate the devserver frequently instead of keeping it alive, plus getting a random port each time can be annoying. This change slightly changes the descriptions, and adds an optional
portparameter, to prevent these two problems.This is a "feature" but a very small one, plus done to an experimental capability to start with, so patch is okay.