From 9324f9f21c3240ce54abb201256d4963265bb270 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 10 Jul 2026 18:35:46 -0400 Subject: [PATCH] fix(pack-verify): kill the --web child on failure too (same exit-skips-finally) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review found the same process.exit()-skips-finally pattern in verifyWeb: a failure during the --web phase goes through fail() → process.exit(1), skipping its `finally { stop() }`, so the spawned web server was orphaned — left on the port, where a later run could fetch a stale false-200 and mask a regression. Track the live child in a module `webChild` and have fail() SIGTERM it before exiting, alongside the workDir cleanup; stop() clears it on the success path. Verified: a forced web-phase failure exits 1, retains the tarball, and leaves no listener on the port (child killed, not orphaned). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- scripts/pack-and-verify.mjs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/pack-and-verify.mjs b/scripts/pack-and-verify.mjs index a460daa5..79bfc33b 100644 --- a/scripts/pack-and-verify.mjs +++ b/scripts/pack-and-verify.mjs @@ -58,9 +58,17 @@ const TOKEN_GLOBAL = "__INSPECTOR_API_TOKEN__"; // dir (a full node_modules from the real install) but leave the packed tarball // in place for post-mortem inspection. let workDir = null; +// The live `--web` child during verifyWeb(), if any. fail() kills it so a +// failure in the web phase doesn't orphan the server — which would linger on +// the port and could even serve a stale false-200 to a later run (verifyWeb's +// own `finally { stop() }` is skipped when fail() calls process.exit()). +let webChild = null; function fail(message) { console.error(`\npack:verify FAILED — ${message}`); + if (webChild && webChild.exitCode === null) { + webChild.kill("SIGTERM"); + } if (workDir) { rmSync(workDir, { recursive: true, force: true }); // `tarball` is initialized before `workDir` is ever set, so this is safe. @@ -299,6 +307,9 @@ async function verifyWeb(bin, cwd) { stdio: ["ignore", "inherit", "inherit"], shell: process.platform === "win32", }); + // Expose the child so fail() can kill it if a check below exits the process + // (process.exit skips the `finally { stop() }`). + webChild = child; let exited = false; let exitCode = null; @@ -308,6 +319,7 @@ async function verifyWeb(bin, cwd) { }); const stop = () => { if (!exited) child.kill("SIGTERM"); + webChild = null; }; try {