test: fail and contain process-state leaks between test files

.ai/skills/hack-repo-verify/SKILL.md

@@ -63,6 +63,18 @@ Host and container `dist` remain separate. This checks compatibility, not exact
 source provenance. Raw `bun test <paths>` remains available for focused tests;
 it bypasses the suite preflight and does not prepare compiler-dependent fixtures.
 
+All test files share one Bun process. `bunfig.toml` preloads
+[the global-state guard](../../../tests/helpers/global-state-guard-preload.ts) for
+every `bun test` run: a test or file that leaves the environment, cwd, umask,
+exit code, a guarded global function or a process listener changed fails with a
+`[global-state-guard]` message naming the file. The guard restores the
+environment, cwd, umask and exit code so later files run clean; it keeps
+replaced functions and listeners, which may still protect unsettled work. Fix
+the named file's restore (`try`/`finally` or `afterEach`), not the later failures.
+Restore environment keys with `restoreEnv` from `tests/helpers/env.ts`: Bun keeps
+a deleted `HTTP_PROXY`-style key for `fetch` and `Bun.spawn` without `env`, and
+the guard reports that as `native env <KEY> changed`.
+
 For release workflow edits, run the pinned syntax/context check also enforced by CI:
 `mise x actionlint@1.7.12 -- actionlint -shellcheck= -pyflakes= .github/workflows/ci.yml .github/workflows/release.yml .github/workflows/prerelease.yml .github/workflows/bun-crash-experiment.yml`.
 This validates event globs and reusable workflow contexts; Bun YAML parsing alone does not.

bunfig.toml

@@ -1,4 +1,10 @@
 [test]
 root = "./tests"
-# Fails the run when a test creates or changes a file in the checkout.
-preload = ["./tests/helpers/checkout-write-guard-preload.ts"]
+# global-state-guard: fails a test or file that leaves PATH, HOME, cwd or other
+# process state changed, and restores it so later files run clean (HACK-1327).
+# checkout-write-guard: fails the run when a test creates or changes a file in
+# the checkout (HACK-1343).
+preload = [
+  "./tests/helpers/global-state-guard-preload.ts",
+  "./tests/helpers/checkout-write-guard-preload.ts",
+]

src/ui/loki-logs.ts

@@ -408,10 +408,6 @@ async function lokiTail(opts: LokiLogsParams): Promise<number> {
     }
   };
 
-  process.once("SIGINT", () => {
-    stop();
-  });
-
   ws.addEventListener("error", () => {
     exitCode = 1;
     if (opts.json && opts.streamContext) {
@@ -485,24 +481,35 @@ async function lokiTail(opts: LokiLogsParams): Promise<number> {
     }
   });
 
-  await new Promise<void>((resolve) => {
+  const closedSocket = new Promise<void>((resolve) => {
     ws.addEventListener("close", () => {
       closed = true;
       clearNoEntriesTimer();
       resolve();
     });
   });
 
-  if (opts.json && opts.streamContext) {
-    writeLogStreamEvent({
-      event: buildLogStreamEndEvent({
-        context: opts.streamContext,
-        reason: exitCode === 0 ? "closed" : "error",
-      }),
-    });
+  // Installed only while the tail is open and removed on every exit path. A
+  // `once` listener that never fires would otherwise outlive the follow and
+  // the next SIGINT in the process would run a stale stop (HACK-1392).
+  const onSigInt = () => {
+    stop();
+  };
+  process.once("SIGINT", onSigInt);
+  try {
+    await closedSocket;
+    if (opts.json && opts.streamContext) {
+      writeLogStreamEvent({
+        event: buildLogStreamEndEvent({
+          context: opts.streamContext,
+          reason: exitCode === 0 ? "closed" : "error",
+        }),
+      });
+    }
+    return exitCode;
+  } finally {
+    process.off("SIGINT", onSigInt);
   }
-
-  return exitCode;
 }
 
 function safeJsonParse(text: string): unknown {

tests/e2e-launchd-guard.test.ts

@@ -12,6 +12,7 @@ import {
   buildCliEnv,
   describeLaunchAgentChange,
   fingerprintLaunchAgentPlist,
+  releaseLaunchctlGuard,
 } from "./e2e/harness.ts";
 
 /*
@@ -24,6 +25,9 @@ import {
 let cleanupRoot: string | null = null;
 
 afterEach(async () => {
+  // The shim and its process exit listener are per process in the e2e runner;
+  // here every test file shares one Bun process, so release them per test.
+  releaseLaunchctlGuard();
   if (cleanupRoot) {
     await rm(cleanupRoot, { recursive: true, force: true });
     cleanupRoot = null;
@@ -129,3 +133,16 @@ test("the LaunchAgent canary reports content, mtime, creation and removal by has
     })
   ).toEndWith("-> absent");
 });
+
+test("releasing the launchctl guard removes its shim and exit listener", async () => {
+  const listeners = process.listenerCount("exit");
+  const env = buildCliEnv({ hackHome: "/tmp/hack-e2e-home-guard" });
+  const guardDir = (env.PATH ?? "").split(":")[0] ?? "";
+  expect(process.listenerCount("exit")).toBe(listeners + 1);
+  expect((await stat(guardDir)).isDirectory()).toBe(true);
+  releaseLaunchctlGuard();
+  expect(process.listenerCount("exit")).toBe(listeners);
+  expect(await stat(guardDir).catch(() => null)).toBeNull();
+  const next = buildCliEnv({ hackHome: "/tmp/hack-e2e-home-guard" });
+  expect((next.PATH ?? "").split(":")[0]).not.toBe(guardDir);
+});

tests/e2e/harness.ts

@@ -272,7 +272,11 @@ function applyLaunchdGuard(
   return guarded;
 }
 
-type LaunchctlGuard = { readonly dir: string; readonly logPath: string };
+type LaunchctlGuard = {
+  readonly dir: string;
+  readonly logPath: string;
+  readonly removeOnExit: () => void;
+};
 let launchctlGuard: LaunchctlGuard | null = null;
 
 function ensureLaunchctlGuard(): LaunchctlGuard {
@@ -299,13 +303,31 @@ function ensureLaunchctlGuard(): LaunchctlGuard {
     ].join("\n")
   );
   chmodSync(shim, 0o755);
-  process.once("exit", () => {
+  const removeOnExit = () => {
     rmSync(dir, { recursive: true, force: true });
-  });
-  launchctlGuard = { dir, logPath };
+  };
+  process.once("exit", removeOnExit);
+  launchctlGuard = { dir, logPath, removeOnExit };
   return launchctlGuard;
 }
 
+/**
+ * Removes the launchctl guard shim and its process `exit` listener now. The
+ * e2e runner keeps one shim for its whole process; a `bun test` file that
+ * builds CLI env calls this in `afterEach`, because every test file shares
+ * one Bun process and a leftover listener would leak into later files. The
+ * next `buildCliEnv` call creates a fresh shim.
+ */
+export function releaseLaunchctlGuard(): void {
+  if (!launchctlGuard) {
+    return;
+  }
+  const { removeOnExit } = launchctlGuard;
+  launchctlGuard = null;
+  process.removeListener("exit", removeOnExit);
+  removeOnExit();
+}
+
 /**
  * launchctl verbs the guard shim refused during this process, in call order.
  * Any entry means a CLI invocation tried to manage the real hackd service.

tests/fzf.test.ts

@@ -9,16 +9,24 @@ import {
   isFzfAvailable,
   resetFzfPathCacheForTests,
 } from "../src/ui/fzf.ts";
+import { restoreEnv } from "./helpers/env.ts";
+
+const TOUCHED_ENV = ["HACK_FZF_PATH", "HACK_HOME", "HOME", "PATH"] as const;
 
 describe("fzf", () => {
-  const originalEnv = { ...process.env };
+  let savedEnv: Map<string, string | undefined> = new Map();
 
   beforeEach(() => {
+    // Edit the live process.env in place and restore only the keys these
+    // cases touch; replacing the object would detach it from Bun.env.
+    savedEnv = new Map(TOUCHED_ENV.map((key) => [key, process.env[key]]));
     resetFzfPathCacheForTests();
   });
 
   afterEach(() => {
-    process.env = { ...originalEnv };
+    for (const [key, value] of savedEnv) {
+      restoreEnv(key, value);
+    }
     resetFzfPathCacheForTests();
   });