diff --git a/.ai/skills/hack-repo-verify/SKILL.md b/.ai/skills/hack-repo-verify/SKILL.md index bd075c08..2c83dba6 100644 --- a/.ai/skills/hack-repo-verify/SKILL.md +++ b/.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 ` 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 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. diff --git a/bunfig.toml b/bunfig.toml index a2584df8..269a735d 100644 --- a/bunfig.toml +++ b/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", +] diff --git a/src/ui/loki-logs.ts b/src/ui/loki-logs.ts index eb095aef..8820980c 100644 --- a/src/ui/loki-logs.ts +++ b/src/ui/loki-logs.ts @@ -408,10 +408,6 @@ async function lokiTail(opts: LokiLogsParams): Promise { } }; - process.once("SIGINT", () => { - stop(); - }); - ws.addEventListener("error", () => { exitCode = 1; if (opts.json && opts.streamContext) { @@ -485,7 +481,7 @@ async function lokiTail(opts: LokiLogsParams): Promise { } }); - await new Promise((resolve) => { + const closedSocket = new Promise((resolve) => { ws.addEventListener("close", () => { closed = true; clearNoEntriesTimer(); @@ -493,16 +489,27 @@ async function lokiTail(opts: LokiLogsParams): Promise { }); }); - 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 { diff --git a/tests/e2e-launchd-guard.test.ts b/tests/e2e-launchd-guard.test.ts index a669b8d1..6e2f6ca7 100644 --- a/tests/e2e-launchd-guard.test.ts +++ b/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); +}); diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index c8339f60..e64fed10 100644 --- a/tests/e2e/harness.ts +++ b/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. diff --git a/tests/fzf.test.ts b/tests/fzf.test.ts index f44e7e12..6aaa28c3 100644 --- a/tests/fzf.test.ts +++ b/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 = 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(); }); diff --git a/tests/global-state-guard.test.ts b/tests/global-state-guard.test.ts new file mode 100644 index 00000000..e3ff41fc --- /dev/null +++ b/tests/global-state-guard.test.ts @@ -0,0 +1,332 @@ +import { afterEach, expect, test } from "bun:test"; +import { + chmod, + mkdir, + mkdtemp, + realpath, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { restoreEnv } from "./helpers/env.ts"; +import { + captureGlobalState, + diffGlobalState, + diffNativeEnv, + restoreGlobalState, +} from "./helpers/global-state-guard.ts"; + +const PRELOAD = resolve( + import.meta.dir, + "helpers/global-state-guard-preload.ts" +); +const KEY = "HACK_GLOBAL_STATE_GUARD_TEST"; +const owned: string[] = []; + +afterEach(async () => { + await Promise.all( + owned.splice(0).map((path) => rm(path, { recursive: true, force: true })) + ); +}); + +async function tempDir(): Promise { + const dir = await realpath( + await mkdtemp(join(tmpdir(), "hack-global-state-guard-")) + ); + owned.push(dir); + return dir; +} + +test("diff names added, changed and removed env keys and a cwd change, without values", async () => { + const dir = await tempDir(); + const before = captureGlobalState(); + const savedPath = process.env.PATH; + const savedKey = process.env[KEY]; + const cwd = process.cwd(); + try { + process.env[KEY] = "secret-shaped-value"; + process.env.PATH = "/nonexistent"; + process.chdir(dir); + const changes = diffGlobalState(before); + expect(changes).toContain("cwd changed"); + expect(changes).toContain( + `env ${KEY} ${savedKey === undefined ? "added" : "changed"}` + ); + expect(changes).toContain("env PATH changed"); + expect(changes.join("\n")).not.toContain("secret-shaped-value"); + expect(changes.join("\n")).not.toContain("/nonexistent"); + expect( + diffGlobalState(before, captureGlobalState(), { + allowedEnvChanges: new Map([[KEY, "test allowlist"]]), + }) + ).not.toContain(`env ${KEY} added`); + } finally { + process.chdir(cwd); + restoreEnv("PATH", savedPath); + restoreEnv(KEY, savedKey); + } + expect(diffGlobalState(before)).toEqual([]); +}); + +test("restore puts back env keys, the env object, cwd and exitCode in place", async () => { + const dir = await tempDir(); + const before = captureGlobalState(); + const envObject = process.env; + const savedPath = process.env.PATH; + const savedHome = process.env.HOME; + const cwd = process.cwd(); + const exitCode = process.exitCode; + try { + process.env.PATH = "/nonexistent"; + Reflect.deleteProperty(process.env, "HOME"); + process.env[KEY] = "1"; + process.chdir(dir); + process.exitCode = 3; + process.env = { PATH: "/replaced" }; + expect(diffGlobalState(before)).toContain("process.env object replaced"); + + restoreGlobalState(before); + + expect(process.env).toBe(envObject); + expect(process.env.PATH).toBe(savedPath); + expect(process.env.HOME).toBe(savedHome); + expect(process.env[KEY]).toBeUndefined(); + expect(process.cwd()).toBe(cwd); + expect(process.exitCode ?? 0).toBe(exitCode ?? 0); + expect(diffGlobalState(before)).toEqual([]); + } finally { + process.env = envObject; + process.chdir(cwd); + process.exitCode = exitCode ?? 0; + restoreEnv("PATH", savedPath); + restoreEnv("HOME", savedHome); + restoreEnv(KEY, undefined); + } +}); + +test("a replaced function or listener is reported but left for its owner", () => { + const before = captureGlobalState(); + const fetchDescriptor = Object.getOwnPropertyDescriptor(globalThis, "fetch"); + const listener = () => undefined; + const count = process.listenerCount("SIGUSR2"); + const fake = (() => + Promise.reject(new Error("refused"))) as unknown as typeof fetch; + try { + globalThis.fetch = fake; + process.on("SIGUSR2", listener); + const changes = diffGlobalState(before); + expect(changes).toContain("globalThis.fetch replaced"); + expect(changes).toContain( + `process 'SIGUSR2' listeners changed (${count} -> ${count + 1})` + ); + restoreGlobalState(before); + expect(globalThis.fetch).toBe(fake); + expect(process.listeners("SIGUSR2")).toContain(listener); + } finally { + process.removeListener("SIGUSR2", listener); + if (fetchDescriptor) { + Object.defineProperty(globalThis, "fetch", fetchDescriptor); + } + } + expect(diffGlobalState(before)).toEqual([]); +}); + +const LEAK_IN_TEST = `import { test } from "bun:test"; +test("sets PATH and fails before restoring", () => { + const saved = process.env.PATH; + process.env.PATH = "/nonexistent"; + if (saved !== "never") throw new Error("early failure before restore"); + process.env.PATH = saved; +}); +`; + +const LEAK_IN_HOOKS = `import { beforeAll, test } from "bun:test"; +beforeAll(() => { + process.env.HACK_HOME = "/nonexistent/hack-home"; + process.chdir("/"); +}); +test("passes but never restores", () => {}); +`; + +// Port 9 on loopback refuses connections, so a request sent through this +// proxy fails at once instead of waiting for DNS or a timeout. +const LEAK_NATIVE_PROXY = `import { test } from "bun:test"; +test("sets a proxy and deletes it, which Bun's fetch never sees", () => { + process.env.HTTP_PROXY = "http://127.0.0.1:9"; + delete process.env.HTTP_PROXY; +}); +`; + +const LATER = `import { expect, test } from "bun:test"; +test("a later file still finds executables on PATH", () => { + const child = Bun.spawnSync(["hack-guard-probe"], { env: { ...process.env } }); + expect(child.exitCode).toBe(0); + expect(process.env.HACK_HOME).toBeUndefined(); + console.log("LATER-FILE-PASSED"); +}); +test("a later file still reaches a loopback server directly", async () => { + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch: () => new Response("direct"), + }); + try { + const response = await fetch("http://127.0.0.1:" + server.port + "/", { + signal: AbortSignal.timeout(5000), + }); + expect(await response.text()).toBe("direct"); + console.log("LATER-FETCH-PASSED"); + } finally { + server.stop(true); + } +}); +`; + +async function runNestedSuite(opts: { + readonly preload: boolean; +}): Promise<{ readonly code: number; readonly output: string }> { + const dir = await tempDir(); + const bin = join(dir, "bin"); + const home = join(dir, "home"); + const suite = join(dir, "suite"); + await mkdir(bin); + await mkdir(home); + await mkdir(suite); + await writeFile(join(bin, "hack-guard-probe"), "#!/bin/sh\nexit 0\n"); + await chmod(join(bin, "hack-guard-probe"), 0o755); + await writeFile( + join(suite, "bunfig.toml"), + opts.preload ? `[test]\npreload = [${JSON.stringify(PRELOAD)}]\n` : "" + ); + await writeFile(join(suite, "a-leak-in-test.test.ts"), LEAK_IN_TEST); + await writeFile(join(suite, "b-leak-in-hooks.test.ts"), LEAK_IN_HOOKS); + await writeFile( + join(suite, "c-leak-native-proxy.test.ts"), + LEAK_NATIVE_PROXY + ); + await writeFile(join(suite, "d-later.test.ts"), LATER); + const child = Bun.spawn( + [ + process.execPath, + "test", + "./a-leak-in-test.test.ts", + "./b-leak-in-hooks.test.ts", + "./c-leak-native-proxy.test.ts", + "./d-later.test.ts", + ], + { + cwd: suite, + env: { PATH: bin, HOME: home, TMPDIR: dir, NO_COLOR: "1" }, + stdout: "pipe", + stderr: "pipe", + } + ); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + return { code, output: `${stdout}\n${stderr}` }; +} + +test("without the guard, an early leak cascades into a later file", async () => { + const run = await runNestedSuite({ preload: false }); + expect(run.code).not.toBe(0); + expect(run.output).toContain("Executable not found in $PATH"); + expect(run.output).toContain( + "(fail) a later file still finds executables on PATH" + ); + // The deleted proxy still routes a later file's loopback fetch. + expect(run.output).toContain( + "(fail) a later file still reaches a loopback server directly" + ); +}, 60_000); + +test("the guard names each leaking file, restores state, and later files pass", async () => { + const run = await runNestedSuite({ preload: true }); + expect(run.code).not.toBe(0); + expect(run.output).toContain( + "[global-state-guard] a test in a-leak-in-test.test.ts left process state changed: env PATH changed." + ); + expect(run.output).toContain( + "b-leak-in-hooks.test.ts left process state changed after the file finished: cwd changed; env HACK_HOME added" + ); + expect(run.output).toContain( + "c-leak-native-proxy.test.ts left Bun's native environment changed after the file finished: native env HTTP_PROXY changed." + ); + expect(run.output).toContain("2 test file(s) leaked process state"); + expect(run.output).not.toContain("127.0.0.1:9"); + expect(run.output).not.toContain("Executable not found"); + expect(run.output).toContain("LATER-FILE-PASSED"); + expect(run.output).toContain("LATER-FETCH-PASSED"); +}, 60_000); + +test("the native diff names changed keys only and treats empty as unset", () => { + const before = new Map([ + ["PATH", "/bin"], + ["HTTP_PROXY", ""], + ]); + const current = new Map([ + ["PATH", "/bin"], + ["HTTPS_PROXY", "http://user:secret-shaped@127.0.0.1:9"], + ]); + expect(diffNativeEnv(before, current)).toEqual(["HTTPS_PROXY"]); + expect(diffNativeEnv(before, new Map([["PATH", "/bin"]]))).toEqual([]); + expect(diffNativeEnv(before, current).join("\n")).not.toContain("secret"); +}); + +// Runs in its own process: once a proxy key is deleted, Bun ignores later +// assignments to it, so checking this in the shared test process would +// depend on which files ran before. +const RESTORE_ENV_PROBE = `import { restoreEnv } from ${JSON.stringify(resolve(import.meta.dir, "helpers/env.ts"))}; +import { captureNativeEnv, diffNativeEnv } from ${JSON.stringify(resolve(import.meta.dir, "helpers/global-state-guard.ts"))}; +const before = captureNativeEnv(); +process.env.HTTP_PROXY = "http://127.0.0.1:9"; +const set = captureNativeEnv(); +if (process.argv[2] === "bare-delete") delete process.env.HTTP_PROXY; +else restoreEnv("HTTP_PROXY", undefined); +const after = captureNativeEnv(); +console.log(JSON.stringify({ + js: process.env.HTTP_PROXY ?? null, + set: before && set ? diffNativeEnv(before, set) : null, + after: before && after ? diffNativeEnv(before, after) : null, +})); +`; + +async function runRestoreEnvProbe(mode: string): Promise { + const dir = await tempDir(); + const script = join(dir, "probe.ts"); + await writeFile(script, RESTORE_ENV_PROBE); + const child = Bun.spawn([process.execPath, script, mode], { + cwd: dir, + env: { PATH: "/usr/bin:/bin", HOME: dir, TMPDIR: dir }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + if (code !== 0) { + throw new Error(`restoreEnv probe exited ${code}: ${stderr}`); + } + return JSON.parse(stdout); +} + +test("restoreEnv leaves no proxy in Bun's native environment; a bare delete does", async () => { + expect(await runRestoreEnvProbe("restore-env")).toEqual({ + js: null, + set: ["HTTP_PROXY"], + after: [], + }); + // If a Bun upgrade makes deletes reach the native environment, this fails + // and the empty-assignment workaround in restoreEnv can go. + expect(await runRestoreEnvProbe("bare-delete")).toEqual({ + js: null, + set: ["HTTP_PROXY"], + after: ["HTTP_PROXY"], + }); +}, 30_000); diff --git a/tests/helpers/env.ts b/tests/helpers/env.ts index 9421edf6..4c271fd4 100644 --- a/tests/helpers/env.ts +++ b/tests/helpers/env.ts @@ -2,9 +2,16 @@ * Sets one environment variable, or removes it when `value` is undefined. Assigning `undefined` * to a `process.env` key stores the string "undefined" in Bun 1.4 (as in Node), which then leaks * into later tests in the same process, so a missing value must be deleted instead. + * + * Before a delete, the key is set to "". Bun 1.4 copies assignments to the proxy keys + * (`HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` and their lower-case forms) into the environment that + * `fetch` and `Bun.spawn` without `env` use, but a delete never reaches it. Without the empty + * assignment, a proxy a test set and then deleted keeps routing every later `fetch` in the + * process, including loopback requests, through that proxy (HACK-1327). An empty proxy is no proxy. */ export function restoreEnv(key: string, value: string | undefined): void { if (value === undefined) { + process.env[key] = ""; Reflect.deleteProperty(process.env, key); } else { process.env[key] = value; diff --git a/tests/helpers/global-state-guard-preload.ts b/tests/helpers/global-state-guard-preload.ts new file mode 100644 index 00000000..a7041435 --- /dev/null +++ b/tests/helpers/global-state-guard-preload.ts @@ -0,0 +1,154 @@ +/** + * `bun test` preload (see `bunfig.toml`): fails loudly when a test or a test + * file leaves process-global state changed, names the file, and restores the + * environment, working directory, umask and exit code so later files do not + * fail for an unrelated reason. Replaced functions and process listeners are + * reported but kept; `restoreGlobalState` explains why. + * + * Two checks run: + * - Per test: the state before the global `beforeEach` must match the state + * after the global `afterEach`. Bun runs preload `beforeEach` hooks before a + * file's own hooks and preload `afterEach` hooks after them, so a file's + * `beforeEach`/`afterEach` pair that restores correctly passes. A mismatch + * fails that test, restores the pre-test environment and cwd, and names the file. + * - Per file: when Bun loads the next test file, the state must match the + * state before the previous file loaded. This catches module-scope and + * `beforeAll`/`afterAll` leaks, including an `afterAll` that never ran its + * restore. A mismatch is reported at once, the pre-file environment is restored, + * and the run fails from the preload `afterAll` with every leaking file. + * This check also compares Bun's native environment (one short `/usr/bin/env` + * child per file), which keeps a proxy key that a test set and then deleted + * although `process.env` no longer shows it. + * + * Bun calls a runtime plugin's `onLoad` for a test file after the previous + * file's `afterAll` hooks and before the new file's module code, which makes + * it the file boundary. The plugin returns the file's source unchanged. + */ +import { afterAll, afterEach, beforeEach } from "bun:test"; +import { readFileSync } from "node:fs"; +import { relative } from "node:path"; + +import { + captureGlobalState, + captureNativeEnv, + diffGlobalState, + diffNativeEnv, + disableNativeProxies, + type GlobalStateSnapshot, + type NativeEnv, + restoreGlobalState, +} from "./global-state-guard.ts"; + +/** + * Environment keys a test file may change without a restore. Every entry + * needs a reason. Keep this list empty unless a change is outside the tests' + * control. + */ +const ALLOWED_ENV_CHANGES: ReadonlyMap = new Map(); + +const ROOT = process.cwd(); +const TEST_FILE = /\.test\.[cm]?[jt]sx?$/; + +let fileBaseline: GlobalStateSnapshot = captureGlobalState(); +let nativeBaseline: NativeEnv | undefined = captureNativeEnv(); +let currentFile: string | undefined; +const fileLeaks: string[] = []; + +function displayPath(path: string | undefined): string { + return path === undefined ? "(preload)" : relative(ROOT, path); +} + +function reportFileLeak(message: string, repair: string): void { + fileLeaks.push(message); + console.error(`\n[global-state-guard] ${message}. ${repair}\n`); +} + +/** + * Checks Bun's native environment after `process.env` was compared and + * restored, so only a change that `process.env` cannot show remains: a proxy + * key that was set and then deleted (see `captureNativeEnv`). + */ +function checkNativeEnv(): void { + const current = captureNativeEnv(); + if (nativeBaseline === undefined || current === undefined) { + nativeBaseline = current; + return; + } + const keys = diffNativeEnv(nativeBaseline, current); + if (keys.length > 0) { + reportFileLeak( + `${displayPath(currentFile)} left Bun's native environment changed after the file finished: ${keys.map((key) => `native env ${key} changed`).join("; ")}. process.env no longer shows it, but fetch proxies and Bun.spawn without env still use it; restore proxy keys with restoreEnv from tests/helpers/env.ts`, + "Bun cannot take a deleted key back, so the guard set NO_PROXY=* in Bun's native environment for the rest of the run: later fetches go direct." + ); + disableNativeProxies(fileBaseline); + } + nativeBaseline = captureNativeEnv(); +} + +function checkFileBoundary(): void { + const changes = diffGlobalState(fileBaseline, captureGlobalState(), { + allowedEnvChanges: ALLOWED_ENV_CHANGES, + }); + if (changes.length > 0) { + reportFileLeak( + `${displayPath(currentFile)} left process state changed after the file finished: ${changes.join("; ")}`, + "Restored the environment and cwd from before the file so later files run clean." + ); + restoreGlobalState(fileBaseline); + } + checkNativeEnv(); + fileBaseline = captureGlobalState(); +} + +Bun.plugin({ + name: "global-state-guard", + setup(build) { + build.onLoad({ filter: TEST_FILE }, (args) => { + if (args.path === Bun.main && args.path !== currentFile) { + checkFileBoundary(); + currentFile = args.path; + } + const loader = args.path.endsWith("x") + ? args.path.endsWith(".jsx") + ? "jsx" + : "tsx" + : args.path.endsWith(".ts") + ? "ts" + : "js"; + return { contents: readFileSync(args.path, "utf8"), loader }; + }); + }, +}); + +let testBaseline: GlobalStateSnapshot | undefined; + +beforeEach(() => { + testBaseline = captureGlobalState(); +}); + +afterEach(() => { + const before = testBaseline; + testBaseline = undefined; + if (before === undefined) { + return; + } + const changes = diffGlobalState(before, captureGlobalState(), { + allowedEnvChanges: ALLOWED_ENV_CHANGES, + }); + if (changes.length === 0) { + return; + } + restoreGlobalState(before); + throw new Error( + `[global-state-guard] a test in ${displayPath(Bun.main)} left process state changed: ${changes.join("; ")}. Restore it in try/finally or afterEach; the guard restored the pre-test environment and cwd.` + ); +}); + +afterAll(() => { + checkFileBoundary(); + if (fileLeaks.length > 0) { + throw new Error( + `[global-state-guard] ${fileLeaks.length} test file(s) leaked process state:\n${fileLeaks.join("\n")}` + ); + } +}); diff --git a/tests/helpers/global-state-guard.ts b/tests/helpers/global-state-guard.ts new file mode 100644 index 00000000..948de7af --- /dev/null +++ b/tests/helpers/global-state-guard.ts @@ -0,0 +1,386 @@ +/** + * Snapshot, compare and restore the process-global state that Bun shares + * between every test file in one `bun test` run. + * + * Bun runs all files in one process. A test that changes `PATH`, `HOME`, + * `HACK_HOME`, the working directory or another process global and then fails + * before its own restore leaks that change into every later file. Those files + * then fail for unrelated reasons (for example "Executable not found in $PATH: + * git"), which hides the first, real failure. The preload in + * `tests/helpers/global-state-guard-preload.ts` uses this module to name the + * file that leaked and to put the environment and working directory back so + * later files run clean. + * + * Reports name keys and kinds only. They never include environment values, + * because fixtures may hold secret-shaped values. + */ + +/** Process events whose listener lists are compared (reported, not restored). */ +export const GUARDED_PROCESS_EVENTS = [ + "SIGINT", + "SIGTERM", + "SIGHUP", + "SIGQUIT", + "SIGUSR1", + "SIGUSR2", + "exit", + "beforeExit", + "uncaughtException", + "unhandledRejection", +] as const; + +type GuardedProcessEvent = (typeof GUARDED_PROCESS_EVENTS)[number]; +type Listener = (...args: unknown[]) => void; + +interface GlobalBinding { + readonly name: string; + readonly owner: object; + readonly key: PropertyKey; +} + +/** + * Globals that tests replace with fakes. Each entry is compared by identity and + * written back on restore. + */ +const GUARDED_BINDINGS: readonly GlobalBinding[] = [ + { name: "globalThis.fetch", owner: globalThis, key: "fetch" }, + { name: "globalThis.setTimeout", owner: globalThis, key: "setTimeout" }, + { name: "globalThis.clearTimeout", owner: globalThis, key: "clearTimeout" }, + { name: "globalThis.setInterval", owner: globalThis, key: "setInterval" }, + { name: "globalThis.clearInterval", owner: globalThis, key: "clearInterval" }, + { name: "globalThis.Date", owner: globalThis, key: "Date" }, + { name: "Bun.spawn", owner: Bun, key: "spawn" }, + { name: "Bun.spawnSync", owner: Bun, key: "spawnSync" }, + { name: "Bun.which", owner: Bun, key: "which" }, + { name: "process.exit", owner: process, key: "exit" }, + { name: "process.kill", owner: process, key: "kill" }, +]; + +/** The guarded state; {@link restoreGlobalState} puts back its process data. */ +export interface GlobalStateSnapshot { + readonly cwd: string; + readonly umask: number; + /** `process.exitCode`, with unset read as 0: Bun ignores assigning `undefined`. */ + readonly exitCode: NonNullable; + readonly envObject: NodeJS.ProcessEnv; + readonly env: Readonly>; + readonly listeners: ReadonlyMap; + readonly bindings: ReadonlyMap; + readonly platform: PropertyDescriptor | undefined; +} + +function readUmask(): number { + // process.umask() with no argument reads the mask without changing it. + return process.umask(); +} + +function copyEnv(env: NodeJS.ProcessEnv): Record { + const copy: Record = {}; + for (const key of Object.keys(env)) { + const value = env[key]; + if (value !== undefined) { + copy[key] = value; + } + } + return copy; +} + +/** Captures the guarded process state. Cheap enough to run around every test. */ +export function captureGlobalState(): GlobalStateSnapshot { + const listeners = new Map(); + for (const event of GUARDED_PROCESS_EVENTS) { + listeners.set(event, process.listeners(event) as Listener[]); + } + const bindings = new Map(); + for (const binding of GUARDED_BINDINGS) { + bindings.set( + binding.name, + Object.getOwnPropertyDescriptor(binding.owner, binding.key) + ); + } + return { + cwd: process.cwd(), + umask: readUmask(), + exitCode: process.exitCode ?? 0, + envObject: process.env, + env: copyEnv(process.env), + listeners, + bindings, + platform: Object.getOwnPropertyDescriptor(process, "platform"), + }; +} + +function sameDescriptor( + left: PropertyDescriptor | undefined, + right: PropertyDescriptor | undefined +): boolean { + if (left === undefined || right === undefined) { + return left === right; + } + return ( + left.value === right.value && + left.get === right.get && + left.set === right.set && + left.writable === right.writable + ); +} + +function sameListeners( + left: readonly Listener[], + right: readonly Listener[] +): boolean { + return ( + left.length === right.length && + left.every((listener, index) => listener === right[index]) + ); +} + +/** Options for {@link diffGlobalState}. */ +export interface DiffGlobalStateOptions { + /** Environment keys that may differ, each with the reason it may. */ + readonly allowedEnvChanges?: ReadonlyMap; +} + +function diffProcessData( + before: GlobalStateSnapshot, + current: GlobalStateSnapshot +): string[] { + const changes: string[] = []; + if (current.cwd !== before.cwd) { + changes.push("cwd changed"); + } + if (current.umask !== before.umask) { + changes.push("umask changed"); + } + if (current.exitCode !== before.exitCode) { + changes.push("process.exitCode changed"); + } + if (current.envObject !== before.envObject) { + changes.push("process.env object replaced"); + } + return changes; +} + +function envChange( + was: string | undefined, + now: string | undefined +): string | undefined { + if (was === now) { + return; + } + if (was === undefined) { + return "added"; + } + return now === undefined ? "removed" : "changed"; +} + +function diffEnv( + before: GlobalStateSnapshot, + current: GlobalStateSnapshot, + allowed: ReadonlyMap | undefined +): string[] { + const keys = new Set([ + ...Object.keys(before.env), + ...Object.keys(current.env), + ]); + const changes: string[] = []; + for (const key of [...keys].sort()) { + const change = allowed?.has(key) + ? undefined + : envChange(before.env[key], current.env[key]); + if (change !== undefined) { + changes.push(`env ${key} ${change}`); + } + } + return changes; +} + +function diffProcessHooks( + before: GlobalStateSnapshot, + current: GlobalStateSnapshot +): string[] { + const changes: string[] = []; + for (const event of GUARDED_PROCESS_EVENTS) { + const was = before.listeners.get(event) ?? []; + const now = current.listeners.get(event) ?? []; + if (!sameListeners(was, now)) { + // The counts tell a listener left behind (n -> n+1) from one consumed + // here that an earlier file left behind (n+1 -> n): a `once` listener + // removes itself when the test emits that signal. + changes.push( + `process '${event}' listeners changed (${was.length} -> ${now.length})` + ); + } + } + for (const binding of GUARDED_BINDINGS) { + if ( + !sameDescriptor( + before.bindings.get(binding.name), + current.bindings.get(binding.name) + ) + ) { + changes.push(`${binding.name} replaced`); + } + } + if (!sameDescriptor(before.platform, current.platform)) { + changes.push("process.platform replaced"); + } + return changes; +} + +/** + * Lists how `current` differs from `before`, as value-free lines such as + * `env PATH changed` or `cwd changed`. An empty list means no leak. + */ +export function diffGlobalState( + before: GlobalStateSnapshot, + current: GlobalStateSnapshot = captureGlobalState(), + opts: DiffGlobalStateOptions = {} +): string[] { + return [ + ...diffProcessData(before, current), + ...diffEnv(before, current, opts.allowedEnvChanges), + ...diffProcessHooks(before, current), + ]; +} + +/** + * Removes one key. The empty assignment first clears Bun's own copy of a proxy + * key, which a delete alone never reaches (see {@link captureNativeEnv}). + */ +function deleteEnvKey(env: NodeJS.ProcessEnv, key: string): void { + env[key] = ""; + Reflect.deleteProperty(env, key); +} + +function restoreEnvObject(snapshot: GlobalStateSnapshot): void { + if (process.env !== snapshot.envObject) { + process.env = snapshot.envObject; + } + const env = process.env; + for (const key of Object.keys(env)) { + if (!Object.hasOwn(snapshot.env, key)) { + deleteEnvKey(env, key); + } + } + for (const [key, value] of Object.entries(snapshot.env)) { + if (env[key] !== value) { + env[key] = value; + } + } +} + +/** + * Puts the process data back to `snapshot`: the environment, working + * directory, umask and `process.exitCode`. The original `process.env` object + * is reinstated and edited in place, so `Bun.env` and child-process defaults + * keep seeing the same environment. + * + * Replaced functions (`Bun.spawn`, `process.kill`, `fetch`, timers), process + * listeners and `process.platform` are reported by {@link diffGlobalState} + * but deliberately not restored. A leak there usually comes from a test that + * timed out while its work is still running, and that fake is what keeps the + * leftover work away from real processes, signals, the network and the + * Docker socket. Restoring the real function under that work could let it + * reach the host. + */ +export function restoreGlobalState(snapshot: GlobalStateSnapshot): void { + restoreEnvObject(snapshot); + if (process.cwd() !== snapshot.cwd) { + process.chdir(snapshot.cwd); + } + if (readUmask() !== snapshot.umask) { + process.umask(snapshot.umask); + } + process.exitCode = snapshot.exitCode; +} + +/** + * The environment Bun itself uses for `fetch` proxies and for `Bun.spawn` + * without an `env` option, as key to value. + */ +export type NativeEnv = ReadonlyMap; + +// Bound at load so a leaked `Bun.spawnSync` fake can never answer for Bun. +const realSpawnSync = Bun.spawnSync.bind(Bun); + +/** + * Reads Bun's native environment by running `/usr/bin/env -0` without an + * `env` option, or returns undefined when that is not possible. + * + * `process.env` does not show all of it. Bun 1.4 starts this environment + * from the startup environment and copies later assignments to the proxy + * keys (`HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` and their lower-case forms) + * into it, but a delete never reaches it. A test that sets a proxy and then + * deletes it leaves `process.env` looking clean while every later `fetch` in + * the process, loopback included, still goes through that proxy (HACK-1327). + * `restoreEnv` in `tests/helpers/env.ts` avoids this by assigning "" first. + * Other keys never change here: Bun does not copy assignments to them. + * Costs one short child process; the preload runs it once per test file. + */ +export function captureNativeEnv(): NativeEnv | undefined { + try { + const child = realSpawnSync(["/usr/bin/env", "-0"], { + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + }); + if (child.exitCode !== 0 || !child.stdout) { + return; + } + const env = new Map(); + for (const entry of child.stdout.toString().split("\0")) { + const separator = entry.indexOf("="); + if (separator > 0) { + env.set(entry.slice(0, separator), entry.slice(separator + 1)); + } + } + return env; + } catch { + return; + } +} + +/** An empty value and an absent key mean the same to Bun: no proxy. */ +function nativeValue(env: NativeEnv, key: string): string | undefined { + const value = env.get(key); + return value === "" ? undefined : value; +} + +/** + * Lists the keys whose native value differs from `before`, sorted. An empty + * value and an absent key compare equal. Never includes values. + */ +export function diffNativeEnv(before: NativeEnv, current: NativeEnv): string[] { + const keys = new Set([...before.keys(), ...current.keys()]); + return [...keys] + .filter((key) => nativeValue(before, key) !== nativeValue(current, key)) + .sort(); +} + +const NO_PROXY_KEYS = ["NO_PROXY", "no_proxy"] as const; + +/** + * Turns proxies off in Bun's native environment for the rest of the process, + * and leaves `process.env` as `snapshot` had it. + * + * A stale native key cannot be put back: once a key is deleted from + * `process.env`, Bun 1.4 ignores every later assignment to it as well. The + * only repair left is `NO_PROXY=*`: it is assigned (Bun copies it) and deleted + * again (Bun keeps the copy), so later `fetch` calls go direct while + * `process.env` is unchanged. Children spawned without `env` still inherit + * the stale proxy key next to `NO_PROXY=*`. If a no-proxy key was itself + * deleted earlier, Bun ignores this too; the next file check then reports it + * again. + */ +export function disableNativeProxies(snapshot: GlobalStateSnapshot): void { + const env = process.env; + for (const key of NO_PROXY_KEYS) { + env[key] = "*"; + Reflect.deleteProperty(env, key); + const value = snapshot.env[key]; + if (value !== undefined) { + env[key] = value; + } + } +} diff --git a/tests/helpers/native-compose-vm-files.ts b/tests/helpers/native-compose-vm-files.ts index f19c9bf2..aa826d84 100644 --- a/tests/helpers/native-compose-vm-files.ts +++ b/tests/helpers/native-compose-vm-files.ts @@ -15,8 +15,18 @@ import { restoreEnv } from "./env.ts"; const cleanups: (() => Promise)[] = []; /** Register this in each test file; Bun caches imports but scopes test hooks. */ export async function cleanupVmFileFixtures() { + // Run every cleanup even when one fails, so a failed `rm` cannot skip the + // environment restore of another fixture. + const failures: unknown[] = []; for (const cleanup of cleanups.splice(0).reverse()) { - await cleanup(); + try { + await cleanup(); + } catch (error: unknown) { + failures.push(error); + } + } + if (failures.length > 0) { + throw failures[0]; } } const environmentKeys = [ @@ -54,6 +64,14 @@ export async function vmFileFixture() { const compiler = resolve( process.env.HACK_CONFIG_COMPILER_BINARY ?? "dist/hack-config-compiler" ); + // Register the restore before the first change, so a setup step that fails + // below still puts PATH, HOME and the Docker keys back. + cleanups.push(async () => { + for (const key of environmentKeys) { + restoreEnv(key, saved[key]); + } + await rm(root, { recursive: true, force: true }); + }); for (const key of environmentKeys) { Reflect.deleteProperty(process.env, key); } @@ -98,10 +116,6 @@ export async function vmFileFixture() { }); cleanups.push(async () => { server.stop(true); - for (const key of environmentKeys) { - restoreEnv(key, saved[key]); - } - await rm(root, { recursive: true, force: true }); }); const docker = join(root, "docker"); await writeFile( diff --git a/tests/helpers/retained-build-adoption.ts b/tests/helpers/retained-build-adoption.ts index 2fcbd728..e213a12d 100644 --- a/tests/helpers/retained-build-adoption.ts +++ b/tests/helpers/retained-build-adoption.ts @@ -132,7 +132,14 @@ else red(); await chmod(binary, 0o700); const previousPath = process.env.PATH; process.env.PATH = outer; - const compiler = await managedEnvCompilerFixture(join(outer, "compiler")); + let compiler: string; + try { + compiler = await managedEnvCompilerFixture(join(outer, "compiler")); + } catch (error: unknown) { + // No fixture (and so no cleanup) reaches the caller; restore PATH here. + restoreEnv("PATH", previousPath); + throw error; + } function isModel(value: unknown): value is typeof model { return ( isRecord(value) && diff --git a/tests/helpers/retained-routing-adoption.ts b/tests/helpers/retained-routing-adoption.ts index d8d63090..f9ad4ac7 100644 --- a/tests/helpers/retained-routing-adoption.ts +++ b/tests/helpers/retained-routing-adoption.ts @@ -673,14 +673,22 @@ else { throw error; } }, + /** + * Restores HACK_HOME only. Safe while the lifetime is unknown: a HACK_HOME + * left pointing at this fixture leaks into every later file in the Bun + * process (HACK-1327). The spies and root stay until {@link cleanup}. + */ + releaseEnvironment: () => { + restoreEnv("HACK_HOME", previousHome); + }, cleanup: async () => { + restoreEnv("HACK_HOME", previousHome); if (!lifetime.canRestore()) { return refuse(); } for (const spy of spies) { spy.mockRestore(); } - restoreEnv("HACK_HOME", previousHome); await rm(outer, { recursive: true, force: true }); }, }; diff --git a/tests/helpers/retained-source-bind-adoption.ts b/tests/helpers/retained-source-bind-adoption.ts index 4b7001c0..c7074f3c 100644 --- a/tests/helpers/retained-source-bind-adoption.ts +++ b/tests/helpers/retained-source-bind-adoption.ts @@ -87,7 +87,14 @@ else if(kind==='network'&&selected===network)console.log(JSON.stringify({id:netw await chmod(binary, 0o700); const previousPath = process.env.PATH; process.env.PATH = outer; - const compiler = await managedEnvCompilerFixture(join(outer, "compiler")); + let compiler: string; + try { + compiler = await managedEnvCompilerFixture(join(outer, "compiler")); + } catch (error: unknown) { + // No fixture (and so no cleanup) reaches the caller; restore PATH here. + restoreEnv("PATH", previousPath); + throw error; + } return { root, outer, diff --git a/tests/loki-short-lived-logs.test.ts b/tests/loki-short-lived-logs.test.ts index 74092a96..f5a5eb43 100644 --- a/tests/loki-short-lived-logs.test.ts +++ b/tests/loki-short-lived-logs.test.ts @@ -514,3 +514,62 @@ test("Loki follow stays quiet when an entry arrives before the notice delay", as } ); }); + +// --- Loki follow: SIGINT listener ownership (HACK-1392) ---------------------- +// All test files share one Bun process. A follow that leaves its `once` +// SIGINT listener behind makes the next SIGINT in the process run a stale +// stop, and the global-state guard then blames whichever test emits it. + +function sameListeners(left: readonly T[], right: readonly T[]): boolean { + return ( + left.length === right.length && + left.every((listener, index) => listener === right[index]) + ); +} + +const FOLLOW_PARAMS = { + query: '{project="demo",service="api"}', + follow: true, + tail: 10, + pretty: false, + showProjectPrefix: true, +} as const; + +test("Loki follow removes its SIGINT listener once the tail closes", async () => { + const before = process.listeners("SIGINT"); + await withFakeLokiTail( + () => undefined, + async ({ baseUrl, sockets }) => { + captureStdout(); + const exit = lokiLogs({ baseUrl, ...FOLLOW_PARAMS }); + await waitFor(() => sockets.length === 1, "the tail connection"); + expect(process.listenerCount("SIGINT")).toBe(before.length + 1); + for (const socket of sockets) { + socket.close(); + } + expect(await exit).toBe(0); + expect(sameListeners(process.listeners("SIGINT"), before)).toBe(true); + } + ); +}); + +test("Loki follow stops on SIGINT and leaves no listener behind", async () => { + const before = process.listeners("SIGINT"); + await withFakeLokiTail( + () => undefined, + async ({ baseUrl, sockets }) => { + captureStdout(); + const exit = lokiLogs({ baseUrl, ...FOLLOW_PARAMS }); + await waitFor(() => sockets.length === 1, "the tail connection"); + const added = process + .listeners("SIGINT") + .filter((listener) => !before.includes(listener)); + expect(added).toHaveLength(1); + // Run the follow's own handler instead of emitting SIGINT, which would + // reach every listener in the shared test process. + added[0]?.("SIGINT"); + expect(await exit).toBe(0); + expect(sameListeners(process.listeners("SIGINT"), before)).toBe(true); + } + ); +}); diff --git a/tests/native-compose-adoption-file-generation.test.ts b/tests/native-compose-adoption-file-generation.test.ts index cc663286..cbb04dae 100644 --- a/tests/native-compose-adoption-file-generation.test.ts +++ b/tests/native-compose-adoption-file-generation.test.ts @@ -70,6 +70,7 @@ let root: string; let projectRoot: string; let binary: string; let priorPath: string | undefined; +let pathReplaced = false; let fixture: { running: boolean; failed: boolean; @@ -168,9 +169,17 @@ else {const source=JSON.parse(await Bun.stdin.text());const {schema_version,...p ` ); await chmod(binary, 0o700); + pathReplaced = true; process.env.PATH = root; }); afterEach(async () => { + // PATH is restored even when the lifetime is unknown: a PATH left pointing + // at this fixture root hides git and bun from every later file in the Bun + // process (HACK-1327). The root and spawn spy stay retained. + if (pathReplaced) { + pathReplaced = false; + restoreEnv("PATH", priorPath); + } if (!lifetime?.canRestore()) { uncertain = true; throw new Error( @@ -179,7 +188,6 @@ afterEach(async () => { } spawnSpy?.mockRestore(); spawnSpy = undefined; - restoreEnv("PATH", priorPath); await rm(root, { recursive: true, force: true }); }); async function save() { diff --git a/tests/native-compose-adoption-routing-generation.test.ts b/tests/native-compose-adoption-routing-generation.test.ts index 96d48add..c9fa5a88 100644 --- a/tests/native-compose-adoption-routing-generation.test.ts +++ b/tests/native-compose-adoption-routing-generation.test.ts @@ -1,6 +1,6 @@ -import { test as boundedTest, expect, spyOn } from "bun:test"; +import { test as boundedTest, expect, mock, spyOn } from "bun:test"; import * as fs from "node:fs/promises"; -import { readFile } from "node:fs/promises"; +import { readFile, rm } from "node:fs/promises"; import { join } from "node:path"; import { DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS } from "../src/lib/compose-startup-budget.ts"; import { isRecord } from "../src/lib/guards.ts"; @@ -9,6 +9,7 @@ import { LegacyComposeAdoptedGenerationError } from "../src/lib/native-compose-a import { legacyComposePublicationRefusal } from "../src/lib/native-compose-adoption-publication-diagnostics.ts"; import { runLegacyComposeRetainedRoutingOperation } from "../src/lib/native-compose-adoption-routing-execution.ts"; import * as privateState from "../src/lib/native-compose-private-state.ts"; +import { restoreEnv } from "./helpers/env.ts"; import { type RetainedRoutingStoreTimeoutMs, ROUTING_CANARY, @@ -75,6 +76,11 @@ const test = ( } finally { if (!lifetime.canRestore()) { fixtureUncertain = true; + // Keep the spies and root for the unsettled work, but never leave + // HACK_HOME pointing at this fixture for later files (HACK-1327). + if (issued) { + h.releaseEnvironment(); + } if (!failed) { failed = true; failure = new Error( @@ -858,6 +864,29 @@ boundedTest( }, 1000 ); +boundedTest( + "a refused teardown still restores HACK_HOME so later files never inherit the fixture home", + async () => { + const before = process.env.HACK_HOME; + const lifetime = retainedRoutingFixtureLifetime(Date.now() + 5000); + const fixture = await retainedRoutingFixture(lifetime); + try { + expect(process.env.HACK_HOME).toBe(fixture.home); + fixture.releaseEnvironment(); + expect(process.env.HACK_HOME).toBe(before); + process.env.HACK_HOME = fixture.home; + lifetime.retain(); + await expect(fixture.cleanup()).rejects.toThrow(); + expect(process.env.HACK_HOME).toBe(before); + } finally { + restoreEnv("HACK_HOME", before); + // The refused teardown kept the fixture spies; nothing else is mocked here. + mock.restore(); + await rm(fixture.outer, { recursive: true, force: true }); + } + }, + 5000 +); boundedTest( "known callback settlement permits fixture teardown before its unchanged deadline", async () => { diff --git a/tests/native-config-protected-files-fixture.test.ts b/tests/native-config-protected-files-fixture.test.ts index aaa19076..fde295a4 100644 --- a/tests/native-config-protected-files-fixture.test.ts +++ b/tests/native-config-protected-files-fixture.test.ts @@ -770,7 +770,7 @@ test.skipIf(process.platform !== "darwin")( } ); -// This intentional-unknown negative is last: its root and global spy remain retained until process exit. +// This intentional-unknown negative is last: its root remains retained until process exit. test("closed-pipe survivor refuses completion and permanently vetoes restoration without a former-group signal", async () => { const pidPath = join(directory, "descendant"), child = join(directory, "child.ts"), @@ -847,4 +847,8 @@ test("closed-pipe survivor refuses completion and permanently vetoes restoration expect(descendantJoined).toBe(true); expect(lifetime!.canRestore()).toBe(false); expectedUnknown = true; + // The descendant has joined, so this pass-through spy guards nothing more. + // Restore it here: the vetoed teardown would otherwise leave process.kill + // replaced for every later file in the Bun process (HACK-1327). + signal.mockRestore(); });