From 3dead1f83d5e5b1bbd4c9e5894f744ca0f882e9a Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Fri, 10 Jul 2026 18:32:03 -0400
Subject: [PATCH] fix(pack-verify): actually clean up on failure (fail()
 bypasses finally)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Re-review caught that the previous fix was inert: fail() calls process.exit(1),
which skips finally blocks — so `cleanupTarball = false` never took effect and,
worse, the temp work dir (a full node_modules from the real install) leaked on
every failed run.

Move cleanup into fail() itself — the single failure-exit point, called from
everywhere including deep inside the run. On failure it removes the heavy work
dir and leaves the packed tarball in place for post-mortem inspection; the
success path removes both inline before the OK log. Drop the dead
`cleanupTarball` flag entirely.

Verified both paths: success removes work dir + tarball (exit 0); a forced
failure removes the work dir, retains the tarball, and exits 1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
---
 scripts/pack-and-verify.mjs | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/scripts/pack-and-verify.mjs b/scripts/pack-and-verify.mjs
index a49e5e7a..a460daa5 100644
--- a/scripts/pack-and-verify.mjs
+++ b/scripts/pack-and-verify.mjs
@@ -51,8 +51,21 @@ const testServer = join(repoRoot, "test-servers", "build", "test-server-stdio.js
 // literal because this plain .mjs script can't import the TS source.
 const TOKEN_GLOBAL = "__INSPECTOR_API_TOKEN__";
 
+// Set once the throwaway consumer dir exists. `fail()` is the single
+// failure-exit point (called from everywhere, including deep inside the run),
+// and it calls process.exit(), which skips any `finally` — so cleanup has to
+// happen here, not in a trailing `finally`. On failure we remove the heavy work
+// dir (a full node_modules from the real install) but leave the packed tarball
+// in place for post-mortem inspection.
+let workDir = null;
+
 function fail(message) {
   console.error(`\npack:verify FAILED — ${message}`);
+  if (workDir) {
+    rmSync(workDir, { recursive: true, force: true });
+    // `tarball` is initialized before `workDir` is ever set, so this is safe.
+    console.error(`pack:verify — tarball retained for inspection at ${tarball}`);
+  }
   process.exit(1);
 }
 
@@ -151,9 +164,7 @@ console.log(
 //    runs the package's postinstall).
 // ---------------------------------------------------------------------------
 const work = mkdtempSync(join(tmpdir(), "pack-verify-"));
-// Remove the packed tarball on success; retain it on failure so a packaging
-// problem can be inspected (unpack it, diff the file list).
-let cleanupTarball = true;
+workDir = work; // from now on, fail() cleans this up
 try {
   ensureTestServer();
   step(`installing the tarball into a clean consumer at ${work} (pulls runtime deps)...`);
@@ -250,18 +261,20 @@ try {
   //     it) and inject the auth token. Run non-blocking and poll `/`.
   await verifyWeb(bin, work);
 
+  // Success: clean up both the work dir and the tarball. (This is reached only
+  // on success — every failure path goes through fail() → process.exit(), which
+  // does its own cleanup above and never returns here.)
+  rmSync(work, { recursive: true, force: true });
+  rmSync(tarball, { force: true });
+
   console.log(
     "\npack:verify OK — published tarball installs clean and the real bin drives " +
       "web (prod / served dist), cli (stdio tools/list), and tui (help) end to end.",
   );
 } catch (err) {
-  // Keep the tarball around for post-mortem inspection of the packaging failure.
-  cleanupTarball = false;
-  console.error(`pack:verify — tarball retained for inspection at ${tarball}`);
+  // Unexpected throw (not via fail()) — route through fail() for consistent
+  // cleanup + exit.
   fail(err instanceof Error ? err.message : String(err));
-} finally {
-  rmSync(work, { recursive: true, force: true });
-  if (cleanupTarball) rmSync(tarball, { force: true });
 }
 
 /**