test: prune abandoned test-run tmp directories at run setup

A run killed before its teardown (tool-timeout SIGKILL, OOM, cancelled job)
left /tmp/agent-device-test-run-<pid>-* behind, and check:tmpdir-leaks — which
runs after test:unit in check:unit — flagged every dead-pid directory it
found. It could not tell this run's leak from a historical one, so one killed
run made every later, otherwise-green gate on the host fail.

Both TMPDIR redirection entry points (the Vitest global setup and the
node --test wrapper) now prune dead-pid run directories before creating their
own, printing one [tmpdir] line when they did; the post-run check keeps its
semantics and can now only ever name the run that just finished. Live owners
(a concurrent run in another worktree) are never touched.

The root/prefix constants move into check-tmpdir-leaks-model.ts, next to the
liveness classification, so the setup can import the prune without a cycle.

Author: thymikee

docs/agents/testing.md

@@ -78,6 +78,14 @@ cleanup for a directory they created — that's the global teardown's job, and p
 already existed for other reasons should stay (it's the fallback global sweep that's new, not a
 replacement for tests being tidy).
 
+A run killed before its teardown (a tool timeout's SIGKILL, OOM, a cancelled job) leaves its
+`/tmp/agent-device-test-run-<pid>-*` directory behind. The next run on the host prunes every such
+directory whose owning pid is dead (`pruneAbandonedRunDirectories`, called by both the Vitest global
+setup and the `node --test` wrapper) and prints one `[tmpdir] pruned …` line, so `check:tmpdir-leaks`
+after `test:unit` can only ever name the run that just finished. Directories owned by a live pid —
+a concurrent run in another worktree — are never touched. If the check fails, the leak is this
+run's: a teardown that did not execute, not history.
+
 Keep tests behavioral. Do not assert shapes or cases TypeScript already proves.
 
 A test added as a regression pin must be shown to fail without the change it pins — vacuity is the

scripts/check-tmpdir-leaks-model.test.ts

@@ -3,7 +3,10 @@ import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import { test } from 'node:test';
-import { findLeakedRunDirectories } from './check-tmpdir-leaks-model.ts';
+import {
+  findLeakedRunDirectories,
+  pruneAbandonedRunDirectories,
+} from './check-tmpdir-leaks-model.ts';
 
 function withScratchRoot(fn: (root: string) => void): void {
   const root = fs.mkdtempSync(path.join(os.tmpdir(), 'check-tmpdir-leaks-model-test-'));
@@ -68,3 +71,39 @@ test('non-matching directories and files are ignored', () => {
     assert.deepEqual(leaks, []);
   });
 });
+
+test('pruning removes only abandoned directories and returns their names', () => {
+  withScratchRoot((root) => {
+    fs.mkdirSync(path.join(root, 'agent-device-test-run-1111-aaaaaa')); // alive
+    fs.mkdirSync(path.join(root, 'agent-device-test-run-3333-cccccc', 'nested'), {
+      recursive: true,
+    }); // exited, non-empty
+    fs.mkdirSync(path.join(root, 'agent-device-test-run-not-a-pid')); // no owner
+    fs.mkdirSync(path.join(root, 'unrelated-directory'));
+    const pruned = pruneAbandonedRunDirectories(root, (pid) => pid === 1111);
+    assert.deepEqual(pruned.sort(), [
+      'agent-device-test-run-3333-cccccc',
+      'agent-device-test-run-not-a-pid',
+    ]);
+    assert.deepEqual(fs.readdirSync(root).sort(), [
+      'agent-device-test-run-1111-aaaaaa',
+      'unrelated-directory',
+    ]);
+    // Once pruned, the post-run check has nothing historical left to report.
+    assert.deepEqual(
+      findLeakedRunDirectories(root, (pid) => pid === 1111),
+      [],
+    );
+  });
+});
+
+test('pruning nothing is a no-op that reports nothing', () => {
+  withScratchRoot((root) => {
+    fs.mkdirSync(path.join(root, 'agent-device-test-run-1111-aaaaaa'));
+    assert.deepEqual(
+      pruneAbandonedRunDirectories(root, () => true),
+      [],
+    );
+    assert.deepEqual(fs.readdirSync(root), ['agent-device-test-run-1111-aaaaaa']);
+  });
+});

scripts/check-tmpdir-leaks-model.ts

@@ -1,5 +1,22 @@
 import fs from 'node:fs';
-import { TEST_RUN_TMP_PREFIX } from './vitest-tmpdir-global-setup.ts';
+import path from 'node:path';
+
+// Every test run (Vitest via scripts/vitest-tmpdir-global-setup.ts, node --test
+// via scripts/node-test-tmpdir.ts) redirects TMPDIR into one disposable,
+// pid-tagged directory under this root and removes it at teardown.
+//
+// Rooted at /tmp rather than nested inside the current os.tmpdir(): macOS's
+// per-user TMPDIR (/var/folders/.../T/) is already close to the 104-byte
+// sun_path limit AF_UNIX sockets need, and tests that bind real sockets
+// (e.g. runner-usbmux.test.ts) started hitting EINVAL once nested one level
+// deeper. /tmp is short enough to leave headroom for those.
+//
+// Both redirection mechanisms and check-tmpdir-leaks.ts import these two from
+// here rather than recomputing them, so they can't drift onto different
+// directories (os.tmpdir() != /tmp on macOS, where TMPDIR is a deep per-user
+// path).
+export const TEST_RUN_TMP_ROOT = '/tmp';
+export const TEST_RUN_TMP_PREFIX = 'agent-device-test-run-';
 
 const PID_SUFFIX = new RegExp(`^${TEST_RUN_TMP_PREFIX}(\\d+)-`);
 
@@ -14,7 +31,7 @@ function isProcessAlive(pid: number): boolean {
 }
 
 /**
- * Directories owned by a still-running process are a concurrent vitest run
+ * Directories owned by a still-running process are a concurrent test run
  * (e.g. another worktree), not a leak — only report the ones whose owning
  * process has already exited without cleaning up after itself.
  */
@@ -33,3 +50,37 @@ export function findLeakedRunDirectories(
     })
     .map((entry) => entry.name);
 }
+
+/**
+ * Removes the run directories an earlier, already-exited run left behind and
+ * returns their names. A run's setup calls this before creating its own
+ * directory, so the post-run leak check (check-tmpdir-leaks.ts) can only ever
+ * report the run that just finished: a directory abandoned by an earlier run
+ * that was killed before its teardown (SIGKILL on a tool timeout, OOM, a
+ * cancelled CI job) is by construction the same thing that teardown would have
+ * removed, and leaving it in place made every later, otherwise-green gate on
+ * the host fail for a run it never ran. Live owners are never touched, so a
+ * concurrent run in another worktree keeps its directory.
+ */
+export function pruneAbandonedRunDirectories(
+  root: string,
+  isAlive: (pid: number) => boolean = isProcessAlive,
+): string[] {
+  const abandoned = findLeakedRunDirectories(root, isAlive);
+  for (const name of abandoned) {
+    fs.rmSync(path.join(root, name), { recursive: true, force: true });
+  }
+  return abandoned;
+}
+
+/**
+ * One stderr line, only when something was pruned: an earlier run on this
+ * host died before its teardown, which the operator should know (a tool
+ * timeout killed it, say) without it being a failure of this run.
+ */
+export function reportPrunedRunDirectories(pruned: readonly string[]): void {
+  if (pruned.length === 0) return;
+  process.stderr.write(
+    `[tmpdir] pruned ${pruned.length} abandoned ${TEST_RUN_TMP_PREFIX}* director${pruned.length === 1 ? 'y' : 'ies'} left by an earlier killed run\n`,
+  );
+}

scripts/check-tmpdir-leaks.ts

@@ -9,11 +9,12 @@
 // Covers both redirection mechanisms sharing this root/prefix: Vitest's
 // globalSetup/globalTeardown (scripts/vitest-tmpdir-global-setup.ts) and the
 // node --test wrapper (scripts/node-test-tmpdir.ts, #1595) that every
-// `node --test` package.json script now runs through.
+// `node --test` package.json script now runs through. Both prune what an
+// earlier killed run left behind before creating their own directory, so a
+// failure here names the run that just finished — never a historical one.
 
 import path from 'node:path';
-import { findLeakedRunDirectories } from './check-tmpdir-leaks-model.ts';
-import { TEST_RUN_TMP_ROOT } from './vitest-tmpdir-global-setup.ts';
+import { TEST_RUN_TMP_ROOT, findLeakedRunDirectories } from './check-tmpdir-leaks-model.ts';
 
 const leaks = findLeakedRunDirectories(TEST_RUN_TMP_ROOT);
 

scripts/node-test-tmpdir.test.ts

@@ -6,7 +6,7 @@ import path from 'node:path';
 import { test } from 'node:test';
 import { fileURLToPath } from 'node:url';
 import { runCmd } from '../src/utils/exec.ts';
-import { TEST_RUN_TMP_PREFIX, TEST_RUN_TMP_ROOT } from './vitest-tmpdir-global-setup.ts';
+import { TEST_RUN_TMP_PREFIX, TEST_RUN_TMP_ROOT } from './check-tmpdir-leaks-model.ts';
 
 const REPOSITORY_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
 const WRAPPER = path.join(REPOSITORY_ROOT, 'scripts', 'node-test-tmpdir.ts');
@@ -250,3 +250,49 @@ test('every node --test package.json script routes through scripts/node-test-tmp
       `NODE_TEST_WRAPPER_BYPASS_ALLOWLIST above with a reason if one must legitimately bypass it.`,
   );
 });
+
+// INT32_MAX exceeds every platform's pid range (Linux pid_max caps at 2^22,
+// macOS at 99999), so kill(pid, 0) is ESRCH by construction — an owner that
+// is dead and can never be reused mid-test, unlike a freshly exited child's pid.
+const NEVER_A_PID = 2_147_483_647;
+
+test('the wrapper prunes a run directory abandoned by an earlier killed run and keeps a live one', async () => {
+  const stamp = crypto.randomUUID();
+  const abandoned = path.join(
+    TEST_RUN_TMP_ROOT,
+    `${TEST_RUN_TMP_PREFIX}${NEVER_A_PID}-planted-${stamp}`,
+  );
+  const live = path.join(
+    TEST_RUN_TMP_ROOT,
+    `${TEST_RUN_TMP_PREFIX}${process.pid}-planted-${stamp}`,
+  );
+  const evidenceRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'node-test-tmpdir-prune-'));
+  const probePath = writeProbe(path.join(evidenceRoot, 'child-tmpdir.txt'));
+  fs.mkdirSync(path.join(abandoned, 'nested'), { recursive: true });
+  fs.writeFileSync(path.join(abandoned, 'nested', 'leftover.txt'), 'from a killed run');
+  fs.mkdirSync(live);
+
+  try {
+    const result = await runCmd(
+      process.execPath,
+      ['--experimental-strip-types', WRAPPER, '--experimental-strip-types', '--test', probePath],
+      { cwd: REPOSITORY_ROOT, timeoutMs: 30_000 },
+    );
+    assert.equal(result.exitCode, 0, `probe run failed:\n${result.stdout}\n${result.stderr}`);
+    assert.equal(
+      fs.existsSync(abandoned),
+      false,
+      'the wrapper must prune the abandoned run directory',
+    );
+    assert.equal(
+      fs.existsSync(live),
+      true,
+      'the wrapper must never touch a live owner’s run directory',
+    );
+  } finally {
+    fs.rmSync(abandoned, { recursive: true, force: true });
+    fs.rmSync(live, { recursive: true, force: true });
+    fs.rmSync(probePath, { force: true });
+    fs.rmSync(evidenceRoot, { recursive: true, force: true });
+  }
+});

scripts/node-test-tmpdir.ts

@@ -20,14 +20,20 @@
 // process forwards to a still-running child (Ctrl-C locally, or a CI job
 // cancellation): the signal handlers below call `process.exit()`, which
 // triggers 'exit' synchronously before the process actually terminates.
-// Only a SIGKILL against this wrapper itself bypasses all of that — the same
-// residual case check-tmpdir-leaks-model.ts already tolerates via its
-// pid-liveness check, since it shares this directory's root and prefix.
+// Only a SIGKILL against this wrapper itself bypasses all of that; the next
+// run on the host prunes what such a kill left behind (see
+// pruneAbandonedRunDirectories), since both lanes share this directory's root
+// and prefix.
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import { runCmdBackground } from '../src/utils/exec.ts';
-import { TEST_RUN_TMP_PREFIX, TEST_RUN_TMP_ROOT } from './vitest-tmpdir-global-setup.ts';
+import {
+  TEST_RUN_TMP_PREFIX,
+  TEST_RUN_TMP_ROOT,
+  pruneAbandonedRunDirectories,
+  reportPrunedRunDirectories,
+} from './check-tmpdir-leaks-model.ts';
 
 const forwardedArgs = process.argv.slice(2);
 if (forwardedArgs.length === 0) {
@@ -36,6 +42,7 @@ if (forwardedArgs.length === 0) {
   );
 }
 
+reportPrunedRunDirectories(pruneAbandonedRunDirectories(TEST_RUN_TMP_ROOT));
 const testRunTmpDir = fs.mkdtempSync(
   path.join(TEST_RUN_TMP_ROOT, `${TEST_RUN_TMP_PREFIX}${process.pid}-`),
 );

scripts/vitest-tmpdir-global-setup.test.ts

@@ -1,11 +1,12 @@
 import assert from 'node:assert/strict';
+import crypto from 'node:crypto';
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import { test } from 'node:test';
 import { fileURLToPath } from 'node:url';
 import { runCmd } from '../src/utils/exec.ts';
-import { TEST_RUN_TMP_PREFIX, TEST_RUN_TMP_ROOT } from './vitest-tmpdir-global-setup.ts';
+import { TEST_RUN_TMP_PREFIX, TEST_RUN_TMP_ROOT } from './check-tmpdir-leaks-model.ts';
 
 const REPOSITORY_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
 
@@ -66,3 +67,49 @@ test('worker inherits the run-owned temp directory', () => {
     fs.rmSync(evidenceRoot, { recursive: true, force: true });
   }
 });
+
+// INT32_MAX exceeds every platform's pid range (Linux pid_max caps at 2^22,
+// macOS at 99999), so kill(pid, 0) is ESRCH by construction — an owner that
+// is dead and can never be reused mid-test, unlike a freshly exited child's pid.
+const NEVER_A_PID = 2_147_483_647;
+
+test('global setup prunes a run directory abandoned by an earlier killed run and keeps a live one', async () => {
+  const stamp = crypto.randomUUID();
+  const abandoned = path.join(
+    TEST_RUN_TMP_ROOT,
+    `${TEST_RUN_TMP_PREFIX}${NEVER_A_PID}-planted-${stamp}`,
+  );
+  // Owned by this test process, which is alive for the whole nested run: the
+  // same shape as a concurrent run in another worktree, and must survive.
+  const live = path.join(
+    TEST_RUN_TMP_ROOT,
+    `${TEST_RUN_TMP_PREFIX}${process.pid}-planted-${stamp}`,
+  );
+  const probeName = `vitest-tmpdir-prune-probe-${process.pid}-${stamp}.test.ts`;
+  const probePath = path.join(REPOSITORY_ROOT, 'src', '__tests__', probeName);
+  fs.mkdirSync(path.join(abandoned, 'nested'), { recursive: true });
+  fs.writeFileSync(path.join(abandoned, 'nested', 'leftover.txt'), 'from a killed run');
+  fs.mkdirSync(live);
+  fs.writeFileSync(
+    probePath,
+    `import { test } from 'vitest';
+
+test('noop probe: the run itself is the subject', () => {});
+`,
+  );
+
+  try {
+    const result = await runCmd(
+      path.join(REPOSITORY_ROOT, 'node_modules', '.bin', 'vitest'),
+      ['run', '--project', 'unit-core', probePath],
+      { cwd: REPOSITORY_ROOT, timeoutMs: 30_000 },
+    );
+    assert.equal(result.exitCode, 0, `probe run failed:\n${result.stdout}\n${result.stderr}`);
+    assert.equal(fs.existsSync(abandoned), false, 'setup must prune the abandoned run directory');
+    assert.equal(fs.existsSync(live), true, 'setup must never touch a live owner’s run directory');
+  } finally {
+    fs.rmSync(abandoned, { recursive: true, force: true });
+    fs.rmSync(live, { recursive: true, force: true });
+    fs.rmSync(probePath, { force: true });
+  }
+});

scripts/vitest-tmpdir-global-setup.ts

@@ -1,6 +1,12 @@
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
+import {
+  TEST_RUN_TMP_PREFIX,
+  TEST_RUN_TMP_ROOT,
+  pruneAbandonedRunDirectories,
+  reportPrunedRunDirectories,
+} from './check-tmpdir-leaks-model.ts';
 
 // os.tmpdir() reads TMPDIR on every call, so redirecting it here covers every
 // mkdtemp call site — test and production — without touching any of them.
@@ -12,17 +18,8 @@ import path from 'node:path';
 // first; it proved unreliable (some workers were torn down before running
 // it), which is why this is a single run-level hook instead.
 //
-// Rooted at /tmp rather than nested inside the current os.tmpdir(): macOS's
-// per-user TMPDIR (/var/folders/.../T/) is already close to the 104-byte
-// sun_path limit AF_UNIX sockets need, and tests that bind real sockets
-// (e.g. runner-usbmux.test.ts) started hitting EINVAL once nested one level
-// deeper. /tmp is short enough to leave headroom for those.
-//
-// check-tmpdir-leaks.ts imports these two rather than recomputing them, so
-// the two can't drift onto different directories (os.tmpdir() != /tmp on
-// macOS, where TMPDIR is a deep per-user path).
-export const TEST_RUN_TMP_ROOT = '/tmp';
-export const TEST_RUN_TMP_PREFIX = 'agent-device-test-run-';
+// The root/prefix live in check-tmpdir-leaks-model.ts, next to the liveness
+// classification this setup and the post-run leak check both rely on.
 
 let testRunTmpDir: string;
 let previousTmpDir: string | undefined;
@@ -45,6 +42,7 @@ export function setup(): void {
       'agent-device-swift-cache',
     );
   }
+  reportPrunedRunDirectories(pruneAbandonedRunDirectories(TEST_RUN_TMP_ROOT));
   // The pid is embedded so check-tmpdir-leaks.ts can tell a directory that's
   // still in active use (its vitest process is alive — a concurrent run in
   // another worktree, say) apart from one actually abandoned by a killed