fix: handle spawn failure of the MCP test server in smoke:web:app

Copilot's third pass (suppressed comment) caught the remaining hole in the
child-process handling: `spawn()` reports a failure to start via an `error`
event, not `exit`. With no `error` listener Node throws it uncaught, replacing
the smoke's diagnostic with a raw stack — and because `exit` never fires in that
case, the readiness poll would otherwise spin for the full 30s before reporting
a timeout that misattributes the cause. `close` is now listened to alongside
`exit` for the same reason: it fires in cases `exit` does not.

prod-web-server.mjs already documents this exact hazard for the launcher child;
this brings the test-server child in line.

Both new branches verified by hand:
  - unspawnable executable → "could not spawn the MCP test server (…): spawn
    /nonexistent/node-binary ENOENT", immediately, instead of an uncaught throw
  - missing module → "MCP test server exited early" with the loader stack, fast,
    with no 30s hang

Full `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAR2Nr9kXbrywFNUVoTe9F
This commit is contained in:
cliffhall 2026-08-06 09:52:40 -04:00
commit 5401e527e6

View file

@ -166,12 +166,25 @@ async function startMcpServer() {
child.stdout.on("data", (d) => (out += d));
child.stderr.on("data", (d) => (out += d));
let exited = false;
let spawnError = null;
// A spawn failure (e.g. an unbuilt/renamed entry) emits `error`, NOT `exit` —
// and with no `error` listener Node throws it uncaught, replacing this smoke's
// diagnostic with a raw stack. `close` is listened to alongside `exit` for the
// same reason: it fires in cases `exit` does not, so a child that dies without
// an exit event can't leave the poll below spinning for the full 30s.
child.on("error", (err) => (spawnError = err));
child.on("exit", () => (exited = true));
child.on("close", () => (exited = true));
for (let attempt = 0; attempt < 120; attempt++) {
// Take the port the server actually bound, not the one we asked for.
const announced = out.match(/listening at (http:\/\/\S+)/i);
if (announced) return { child, url: announced[1] };
if (spawnError) {
throw new Error(
`could not spawn the MCP test server (${composableServer}): ${spawnError.message}`,
);
}
if (exited) throw new Error(`MCP test server exited early:\n${out}`);
await delay(250);
}