test: keep the write guard, pid handshakes and store cases reliable under load (HACK-1439)

tests/checkout-write-guard.test.ts

@@ -12,6 +12,7 @@ import { join, resolve } from "node:path";
 import { ClosedCliError } from "../src/lib/cli-result.ts";
 import {
   assertNoCheckoutWrites,
+  CHECKOUT_GIT_BUDGET_MS,
   checkoutStatusEntries,
   newStatusEntries,
 } from "./helpers/checkout-write-guard.ts";
@@ -34,6 +35,14 @@ const HELPERS = resolve(import.meta.dir, "helpers");
 const CLI_RESULT = resolve(import.meta.dir, "..", "src/lib/cli-result.ts");
 /** Each case starts a fresh `bun` child; a cold start under suite load is slow. */
 const CHILD_TEST_TIMEOUT_MS = 60_000;
+/**
+ * Bun timeout of a guarded fixture's keep-alive case. That case only has to
+ * outlive the timed-out case's late call, but a late `spawnSync` blocks the
+ * fixture's event loop while its cold `bun` child starts, which under host
+ * load passed Bun's 5 s default (HACK-1439). It stays inside the outer case's
+ * budget.
+ */
+const KEEP_ALIVE_CASE_TIMEOUT_MS = 30_000;
 
 let root = "";
 
@@ -54,10 +63,14 @@ const fixture = (): string =>
 async function run(opts: {
   readonly argv: readonly string[];
   readonly cwd: string;
+  readonly env?: Readonly<Record<string, string>>;
 }): Promise<{ readonly exitCode: number; readonly output: string }> {
   const child = Bun.spawn([...opts.argv], {
     cwd: opts.cwd,
-    env: privateCliEnv({ root: fixture(), env: { NO_COLOR: "1" } }),
+    env: privateCliEnv({
+      root: fixture(),
+      env: { NO_COLOR: "1", ...opts.env },
+    }),
     stdin: "ignore",
     stdout: "pipe",
     stderr: "pipe",
@@ -276,6 +289,29 @@ test(
   CHILD_TEST_TIMEOUT_MS
 );
 
+test(
+  "the guard kills and refuses a git status that reaches its bound instead of waiting on it",
+  async () => {
+    const checkout = await gitCheckout("hanging-status");
+    const bin = join(fixture(), "hanging-bin");
+    await mkdir(bin);
+    const hanging = join(bin, "git");
+    await writeFile(hanging, "#!/bin/sh\nexec sleep 30\n", { mode: 0o700 });
+    const started = performance.now();
+    expectClosedThrow(
+      () =>
+        checkoutStatusEntries(
+          { git: hanging, env: gitRunner().env, budgetMs: 200 },
+          checkout
+        ),
+      "checkout write guard: git status did not finish within 200 ms"
+    );
+    // Killed at its bound, not left to finish its 30 s sleep.
+    expect(performance.now() - started).toBeLessThan(20_000);
+  },
+  CHILD_TEST_TIMEOUT_MS
+);
+
 /**
  * A bun-test project in a fresh git checkout, preloading the guard exactly as
  * the repository's bunfig.toml does.
@@ -328,7 +364,7 @@ test(
         });
         test("keeps the run alive past the late call", async () => {
           await Bun.sleep(1000);
-        });
+        }, ${KEEP_ALIVE_CASE_TIMEOUT_MS});
       `,
     });
     const result = await run({
@@ -372,6 +408,60 @@ test(
   CHILD_TEST_TIMEOUT_MS
 );
 
+/** Longer than Bun's 5 s default hook timeout, far inside the status bound. */
+const SLOW_STATUS_SECONDS = 6;
+
+test(
+  "an end-of-run git status slower than Bun's default hook timeout still passes a clean run",
+  async () => {
+    // HACK-1439: under host load the end-of-run status passed 5 s, Bun's
+    // default hook timeout killed git mid-read ("killed 1 dangling process"),
+    // and the guard refused a run whose cases all passed. A git wrapper makes
+    // only the end-of-run status slow: the case arms it after the baseline.
+    expect(SLOW_STATUS_SECONDS * 1000).toBeLessThan(CHECKOUT_GIT_BUDGET_MS);
+    const realGit = gitRunner().git;
+    const bin = join(fixture(), "slow-bin");
+    await mkdir(bin);
+    const armed = join(fixture(), "slow-status-armed");
+    const slept = join(fixture(), "slow-status-slept");
+    await writeFile(
+      join(bin, "git"),
+      [
+        "#!/bin/sh",
+        `if [ "$1" = status ] && [ -e ${JSON.stringify(armed)} ]; then`,
+        `  sleep ${SLOW_STATUS_SECONDS}`,
+        `  : > ${JSON.stringify(slept)}`,
+        "fi",
+        `exec ${JSON.stringify(realGit)} "$@"`,
+        "",
+      ].join("\n"),
+      { mode: 0o700 }
+    );
+    const checkout = await guardedProject({
+      name: "slow-status",
+      testSource: `
+        import { expect, test } from "bun:test";
+        import { writeFileSync } from "node:fs";
+        test("arms the slow end-of-run status", () => {
+          writeFileSync(${JSON.stringify(armed)}, "");
+          expect(true).toBe(true);
+        });
+      `,
+    });
+    const result = await run({
+      argv: [process.execPath, "test"],
+      cwd: checkout,
+      env: { PATH: `${bin}:${process.env.PATH ?? ""}` },
+    });
+    expect(result.output).not.toContain("checkout write guard");
+    expect(result.output).not.toContain("dangling process");
+    expect(result.exitCode).toBe(0);
+    // The hook waited out the slow status rather than skipping it.
+    expect(await Bun.file(slept).exists()).toBe(true);
+  },
+  CHILD_TEST_TIMEOUT_MS
+);
+
 test(
   "a late spawn with a reset cwd that rewrites a tracked file fails the run, named",
   async () => {
@@ -401,7 +491,7 @@ test(
         });
         test("keeps the run alive past the late spawn", async () => {
           await Bun.sleep(1000);
-        });
+        }, ${KEEP_ALIVE_CASE_TIMEOUT_MS});
       `,
     });
     const result = await run({

tests/helpers/checkout-write-guard.ts

@@ -19,32 +19,61 @@ import { ClosedCliError } from "../../src/lib/cli-result.ts";
 /** At most this many paths are named in a failure. */
 const MAX_NAMED = 50;
 
+/**
+ * Wall-clock bound on one guard `git` call. A read-only `git status` of this
+ * checkout takes well under a second on an idle host, but its cost is file
+ * system work that grows with host load: at a load of 56-118 on an 18-CPU
+ * host it passed 5 s (HACK-1439). The bound is a hang guard, not a speed
+ * assertion; a call that reaches it is killed and refused, closed.
+ */
+export const CHECKOUT_GIT_BUDGET_MS = 60_000;
+
+/**
+ * Bun timeout of the end-of-run `afterAll` hook. It outlasts the status
+ * call's own bound, so a slow status ends as the guard's named refusal rather
+ * than Bun's 5 s default hook timeout killing `git` mid-read, which failed
+ * runs whose cases all passed (HACK-1439).
+ */
+export const CHECKOUT_GUARD_HOOK_BUDGET_MS = CHECKOUT_GIT_BUDGET_MS + 30_000;
+
 type GitRunner = {
   readonly git: string;
   readonly env: Readonly<Record<string, string>>;
+  /** Bound on each call; `CHECKOUT_GIT_BUDGET_MS` when omitted. */
+  readonly budgetMs?: number;
 };
 
 function runGit(
   runner: GitRunner,
   cwd: string,
   args: readonly string[]
-): { readonly exitCode: number; readonly stdout: string } {
+): {
+  readonly exitCode: number;
+  readonly timedOut: boolean;
+  readonly stdout: string;
+} {
   const result = Bun.spawnSync([runner.git, ...args], {
     cwd,
     env: runner.env,
     stdin: "ignore",
     stdout: "pipe",
     stderr: "ignore",
+    timeout: runner.budgetMs ?? CHECKOUT_GIT_BUDGET_MS,
+    killSignal: "SIGKILL",
   });
-  return { exitCode: result.exitCode, stdout: result.stdout.toString() };
+  return {
+    exitCode: result.exitCode,
+    timedOut: result.exitedDueToTimeout === true,
+    stdout: result.stdout.toString(),
+  };
 }
 
 /**
  * The `git status` entries of `checkout`: every untracked, non-ignored file
  * and every changed tracked file, each as its two-letter status and its
  * checkout-relative path (`?? new.txt`, ` M package.json`). A failed
- * `git status` is refused, closed: the guard never treats an unreadable
- * checkout as clean.
+ * `git status`, or one that reaches its bound, is refused, closed: the guard
+ * never treats an unreadable checkout as clean.
  */
 export function checkoutStatusEntries(
   runner: GitRunner,
@@ -56,6 +85,11 @@ export function checkoutStatusEntries(
     "-z",
     "--untracked-files=all",
   ]);
+  if (status.timedOut) {
+    throw new ClosedCliError(
+      `checkout write guard: git status did not finish within ${runner.budgetMs ?? CHECKOUT_GIT_BUDGET_MS} ms; the checkout could not be checked for files written by tests.`
+    );
+  }
   if (status.exitCode !== 0) {
     throw new ClosedCliError(
       "checkout write guard: git status failed; the checkout could not be checked for files written by tests."
@@ -110,7 +144,8 @@ export function assertNoCheckoutWrites(opts: {
 
 /**
  * Records the baseline for the git checkout that contains `start` and
- * registers the end-of-run check through `afterAll`.
+ * registers the end-of-run check through `afterAll`, with the named
+ * `CHECKOUT_GUARD_HOOK_BUDGET_MS` timeout instead of Bun's 5 s default.
  *
  * When `start` is not in a git work tree (an export, or a container mount of a
  * linked worktree) there is nothing to compare, so the guard stays inactive
@@ -119,7 +154,7 @@ export function assertNoCheckoutWrites(opts: {
  */
 export function installCheckoutWriteGuard(opts: {
   readonly start: string;
-  readonly afterAll: (hook: () => void) => void;
+  readonly afterAll: (hook: () => void, timeoutMs: number) => void;
   readonly env?: Readonly<Record<string, string | undefined>>;
 }): void {
   const source = opts.env ?? process.env;
@@ -158,5 +193,5 @@ export function installCheckoutWriteGuard(opts: {
         checkoutStatusEntries(runner, checkout)
       ),
     });
-  });
+  }, CHECKOUT_GUARD_HOOK_BUDGET_MS);
 }

tests/helpers/wait-for-file-content.ts

@@ -0,0 +1,37 @@
+/**
+ * Wait, bounded, until a fixture file at `path` holds complete content, and
+ * return what `complete` makes of it.
+ *
+ * A fixture's writeFileSync or Bun.write creates the file before it writes the
+ * bytes, so a wait for existence alone can return while the file is still ""
+ * (HACK-1422, HACK-1439). Use this for any handshake whose content is read
+ * afterwards. A file that is renamed into place is already complete, so it does
+ * not need this helper; neither does a barrier whose content is never read.
+ *
+ * `complete` returns undefined for content that is not complete yet. Take the
+ * value before signaling the writer: a writer killed between creating the file
+ * and writing it leaves the file empty for good.
+ */
+export async function waitForFileContent<T>(opts: {
+  readonly path: string;
+  readonly complete: (text: string) => T | undefined;
+  readonly what: string;
+  readonly budgetMs: number;
+}): Promise<T> {
+  const deadline = performance.now() + opts.budgetMs;
+  for (;;) {
+    const file = Bun.file(opts.path);
+    if (await file.exists()) {
+      const value = opts.complete(await file.text());
+      if (value !== undefined) {
+        return value;
+      }
+    }
+    if (performance.now() >= deadline) {
+      throw new Error(
+        `Fixture ${opts.what} did not complete within ${opts.budgetMs} ms.`
+      );
+    }
+    await Bun.sleep(10);
+  }
+}

tests/native-compose-adoption-job-worktrees.test.ts

@@ -14,6 +14,7 @@ import {
   createCompletedJobFixturePhases,
   createCompletedJobFixtureSettlement,
 } from "./e2e/scenarios/native-compose-adoption-job-worktrees.ts";
+import { waitForFileContent } from "./helpers/wait-for-file-content.ts";
 
 const ID = "a".repeat(64);
 const IMAGE = `sha256:${"d".repeat(64)}`;
@@ -118,8 +119,10 @@ test("dependent attempts correspond exactly to successful job attempts rather th
   }
 });
 
+const READY_BUDGET_MS = 2000;
+/** A barrier only: a case that reads the file uses waitForFileContent. */
 async function waitForFile(path: string) {
-  const deadline = Date.now() + 2000;
+  const deadline = Date.now() + READY_BUDGET_MS;
   while (Date.now() < deadline) {
     if (await Bun.file(path).exists()) {
       return;
@@ -492,8 +495,12 @@ test("inherited-pipe descendants withhold cancellation settlement without signal
         finished = true;
       }
     );
-    await waitForFile(ready);
-    const descendantPid = Number(await Bun.file(ready).text());
+    const descendantPid = await waitForFileContent({
+      path: ready,
+      budgetMs: READY_BUDGET_MS,
+      what: "descendant pid",
+      complete: (text) => (text === "" ? undefined : Number(text)),
+    });
     expect(Number.isSafeInteger(descendantPid) && descendantPid > 0).toBe(true);
     expect(Number.isSafeInteger(leaderPid) && leaderPid > 0).toBe(true);
     await waitForAbsent(leaderPid);

tests/native-compose-route-claims.test.ts

@@ -10,7 +10,15 @@ import {
   type NativeComposeRouteOwner,
   openNativeComposeRouteClaims,
 } from "../src/lib/native-compose-route-claims.ts";
+import { waitForFileContent } from "./helpers/wait-for-file-content.ts";
 
+const BARRIER_BUDGET_MS = 5000;
+/**
+ * Gap between creating the rollback barrier and writing its reference. It makes
+ * the empty-file window happen on every run, so a case that waits for existence
+ * alone and then kills the writer fails every time.
+ */
+const TORN_BARRIER_GAP_MS = 50;
 const BINDING: NativeComposeRouteBinding = {
   engineId: "fixture-engine:1",
   proxyId: "a".repeat(64),
@@ -91,8 +99,9 @@ async function complete(
   await result.markEffectsPossible(attempt);
   await result.complete({ attempt, assertTransition: async () => {} });
 }
+/** A barrier only: a case that reads the file uses waitForFileContent. */
 async function waitForFile(path: string) {
-  const deadline = performance.now() + 5000;
+  const deadline = performance.now() + BARRIER_BUDGET_MS;
   while (!(await Bun.file(path).exists())) {
     if (performance.now() >= deadline) {
       throw new Error("fixture barrier timed out");
@@ -604,6 +613,7 @@ test("interrupted rollback before deletion-intent publication never hides its ex
   ).href;
   const child = spawn(`
     import {spyOn} from "bun:test";
+    import {writeFileSync} from "node:fs";
     import * as fs from "node:fs/promises";
     import {openNativeComposeRouteClaims} from ${JSON.stringify(module)};
     const root = ${JSON.stringify(root)};
@@ -612,20 +622,31 @@ test("interrupted rollback before deletion-intent publication never hides its ex
     const original = fs.open;
     spyOn(fs, "open").mockImplementation(async (...args) => {
       if (String(args[0]).includes("/releases/") && String(args[0]).endsWith(".tmp")) {
+        writeFileSync(root + "/rollback-barrier", "");
+        await Bun.sleep(${TORN_BARRIER_GAP_MS});
         await Bun.write(root + "/rollback-barrier", JSON.stringify(attempt.reference));
         await Bun.sleep(60_000);
       }
       return await original(...args);
     });
     await store.rollback(attempt);
   `);
-  await waitForFile(join(root, "rollback-barrier"));
+  const reference = await waitForFileContent({
+    path: join(root, "rollback-barrier"),
+    budgetMs: BARRIER_BUDGET_MS,
+    what: "rollback barrier",
+    complete: (text) => {
+      try {
+        return JSON.parse(text) as NativeComposeRouteAttempt["reference"];
+      } catch {
+        return undefined;
+      }
+    },
+  });
   child.kill("SIGKILL");
   await child.exited;
   const reopened = await store(root);
-  const saved = await reopened.reopen(
-    await Bun.file(join(root, "rollback-barrier")).json()
-  );
+  const saved = await reopened.reopen(reference);
   expect(["reserved", "aborted"]).toContain(saved.phase);
   let observed: readonly string[] = [];
   await reopened.release({