fix(tests): pin the spawned wt binary against concurrent cargo uplifts

src/git/reap.rs

@@ -67,7 +67,18 @@ pub struct CwdProcess {
 
 /// Timeout for the `lsof` / `ps` probes. Discovery is opt-in and off the hot
 /// path, but a hung probe should still not stall removal indefinitely.
-const PROBE_TIMEOUT: Duration = Duration::from_secs(5);
+///
+/// Tests override via `WORKTRUNK_TEST_PROBE_TIMEOUT_MS` (set generously in
+/// `STATIC_TEST_ENV_VARS`): under suite load a probe's spawn alone can stall
+/// past the production bound, and the timeout's fail-safe empty result then
+/// reads as "no processes to reap" — a load-dependent test outcome.
+fn probe_timeout() -> Duration {
+    let ms = std::env::var("WORKTRUNK_TEST_PROBE_TIMEOUT_MS")
+        .ok()
+        .and_then(|s| s.parse().ok())
+        .unwrap_or(5_000);
+    Duration::from_millis(ms)
+}
 
 /// Parse `lsof -d cwd -F pcn` field output into one [`CwdProcess`] per process.
 ///
@@ -153,7 +164,7 @@ fn without_controlling_terminal(pids: &[u32]) -> HashSet<u32> {
     // partial-output shape `lsof` has in `processes_under`).
     let Ok(output) = Cmd::new("ps")
         .args(["-o", "pid=,tty=", "-p", &pid_list])
-        .timeout(PROBE_TIMEOUT)
+        .timeout(probe_timeout())
         .run()
     else {
         return HashSet::new();
@@ -182,7 +193,7 @@ pub fn processes_under(worktree_path: &Path) -> Vec<CwdProcess> {
     // exit status. Only a spawn failure (lsof missing) means "no data".
     let Ok(output) = Cmd::new("lsof")
         .args(["-d", "cwd", "-F", "pcn"])
-        .timeout(PROBE_TIMEOUT)
+        .timeout(probe_timeout())
         .run()
     else {
         return Vec::new();
@@ -360,10 +371,11 @@ not-a-pid tty
     }
 
     /// `reap_pids` against a real process: `SIGTERM` terminates it and the
-    /// count comes back confirmed. Discovery and the controlling-terminal
-    /// guard are covered in-process by the `test_remove_reap_kills_process`
-    /// integration test (which calls `processes_under` / `collect_reapable`
-    /// directly), so this focuses on the signalling half.
+    /// count comes back confirmed. Discovery is covered in-process by the
+    /// `test_remove_reap_kills_process` integration test (which polls
+    /// `processes_under` directly); the controlling-terminal guard's
+    /// classification has the `parse_ps_tty` unit tests, and the spawned `wt`
+    /// exercises the assembled gate — so this focuses on the signalling half.
     #[test]
     fn reap_pids_terminates_a_process() {
         use std::process::Command;

src/testing/mod.rs

@@ -66,7 +66,7 @@ use path_slash::PathExt;
 
 use self::mock_commands::{MockConfig, MockResponse, tea_api_include_stderr};
 
-/// Path to the `wt` binary built by Cargo.
+/// Path to the `wt` binary built by Cargo, pinned against concurrent builds.
 ///
 /// Resolved at runtime from `CARGO_BIN_EXE_wt`, which cargo and nextest both
 /// provide to integration-test processes, naming the binary the same
@@ -76,13 +76,93 @@ use self::mock_commands::{MockConfig, MockResponse, tea_api_include_stderr};
 /// code that spawns `wt` — `mock_commands` included — belongs in integration
 /// tests.
 ///
+/// The returned path is not `CARGO_BIN_EXE_wt` itself but a hardlink to it
+/// under `target/<profile>/wt-test-bin/<key>/` (see [`pin_test_binary`]):
+/// cargo uplifts `target/debug/wt` by removing the path and recreating it, so
+/// a concurrent `cargo build` in the same tree leaves the uplifted path absent
+/// for a fraction of a millisecond per rebuild, failing whatever spawn is in
+/// flight with `NotFound`. The hardlink keeps the observed binary's inode
+/// alive whatever a concurrent cargo does to the uplifted path. Pinned once
+/// per test process; every spawn helper routes through here, so no test
+/// spawns the unlinkable path directly (`test_wt_spawns_are_pinned`).
+///
 /// Panics when the variable is absent (the test binary run outside a cargo
 /// runner) rather than deriving a path that could name a stale binary.
 pub fn wt_bin() -> PathBuf {
-    PathBuf::from(
-        std::env::var("CARGO_BIN_EXE_wt")
-            .expect("CARGO_BIN_EXE_wt not set — only available during `cargo test`"),
-    )
+    static PINNED: std::sync::OnceLock<PathBuf> = std::sync::OnceLock::new();
+    PINNED
+        .get_or_init(|| {
+            let uplifted = PathBuf::from(
+                std::env::var("CARGO_BIN_EXE_wt")
+                    .expect("CARGO_BIN_EXE_wt not set — only available during `cargo test`"),
+            );
+            pin_test_binary(&uplifted)
+        })
+        .clone()
+}
+
+/// Pin `src` where a concurrent cargo can't unlink it, returning the pinned
+/// path.
+///
+/// Hardlinks `src` into a sibling `wt-test-bin/<mtime>-<len>/` directory named
+/// by the observed binary's identity, keeping `src`'s basename. The hardlink
+/// pins the inode: cargo's uplift only unlinks the *path*, so the pinned entry
+/// keeps serving the observed binary through any number of concurrent
+/// rebuilds. A link costs no space while the `deps/` artifact it shares an
+/// inode with exists, and everything under `wt-test-bin/` dies with
+/// `cargo clean`, so nothing sweeps it.
+///
+/// Two runs observing the same binary converge on the same entry (`link`
+/// returning `AlreadyExists` is success — the key names the content); a run
+/// observing a rebuilt binary creates a new entry beside the old one, exactly
+/// as it would have spawned the rebuilt binary before pinning existed. A
+/// `NotFound` from `stat` or `link` is the uplift window itself — the binary
+/// is absent for well under a millisecond — so it's polled through rather than
+/// surfaced (bounded; a genuinely missing binary still panics, with the path).
+pub fn pin_test_binary(src: &Path) -> PathBuf {
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
+    loop {
+        match try_pin(src) {
+            Ok(pinned) => return pinned,
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+                assert!(
+                    std::time::Instant::now() < deadline,
+                    "binary to pin stayed absent for 10s: {}",
+                    src.display()
+                );
+                std::thread::sleep(std::time::Duration::from_millis(10));
+            }
+            Err(e) => panic!("failed to pin test binary {}: {e}", src.display()),
+        }
+    }
+}
+
+/// One pin attempt. `NotFound` means `src` was observed mid-uplift (or the
+/// pin directory's ancestors vanished under a `cargo clean`); the caller
+/// retries those.
+fn try_pin(src: &Path) -> std::io::Result<PathBuf> {
+    let meta = std::fs::metadata(src)?;
+    let mtime = meta
+        .modified()?
+        .duration_since(std::time::UNIX_EPOCH)
+        .unwrap_or_default()
+        .as_nanos();
+    let dir = src
+        .parent()
+        .expect("binary path has a parent")
+        .join("wt-test-bin")
+        .join(format!("{mtime:x}-{:x}", meta.len()));
+    let pinned = dir.join(src.file_name().expect("binary path has a file name"));
+    if pinned.exists() {
+        return Ok(pinned);
+    }
+    std::fs::create_dir_all(&dir)?;
+    match std::fs::hard_link(src, &pinned) {
+        // A concurrent test process pinned the same key first; the key names
+        // the content, so its entry is ours.
+        Err(e) if e.kind() != std::io::ErrorKind::AlreadyExists => Err(e),
+        _ => Ok(pinned),
+    }
 }
 
 use tempfile::TempDir;
@@ -472,6 +552,11 @@ pub const STATIC_TEST_ENV_VARS: &[(&str, &str)] = &[
     // Disable delayed streaming for deterministic output across platforms.
     // Without this, slow CI triggers progress messages that don't appear on faster systems.
     ("WORKTRUNK_TEST_DELAYED_STREAM_MS", "-1"),
+    // Give the `--reap` probes (`git::reap::probe_timeout`) a load-proof
+    // bound. At the production 5s, a probe spawn stalling under suite load
+    // trips the timeout, whose fail-safe empty result turns "reap the child"
+    // into "No processes to reap" — a load-dependent outcome.
+    ("WORKTRUNK_TEST_PROBE_TIMEOUT_MS", "60000"),
     // Treat shells as not installed by default so the "Skipped …; rc not found"
     // filter in scan_shell_configs is deterministic across hosts. Tests that need
     // a shell to count as installed (e.g., to assert the Skipped path) set "1".

tests/CLAUDE.md

@@ -12,7 +12,7 @@ cargo test --test integration                                      # integration
 cargo test --test integration --features shell-integration-tests   # + shell tests
 ```
 
-Every binary the suite spawns is `wt` itself — the mock commands are the same binary linked under other names, dispatching on argv[0] (`testing::mock_stub`) — so no run can spawn missing or stale code: cargo rebuilds a package's own binaries whenever its integration tests build, under every runner and filter, and the `CARGO_BIN_EXE_wt` the harness resolves at runtime names that just-built binary; outside a cargo runner the suite panics ("CARGO_BIN_EXE_wt not set") rather than guessing a path. `cargo build --bin wt` recompiling right after a test run is the bin-only build being a separate cached unit (a different feature graph), not evidence the tests ran stale code.
+Every binary the suite spawns is `wt` itself — the mock commands are the same binary linked under other names, dispatching on argv[0] (`testing::mock_stub`) — so no run can spawn missing or stale code: cargo rebuilds a package's own binaries whenever its integration tests build, under every runner and filter, and `wt_bin()` resolves `CARGO_BIN_EXE_wt` — naming that just-built binary — into a hardlink pinned under `target/debug/wt-test-bin/`, which a concurrent `cargo build`'s uplift can't unlink mid-run (see No Retries); outside a cargo runner the suite panics ("CARGO_BIN_EXE_wt not set") rather than guessing a path. `cargo build --bin wt` recompiling right after a test run is the bin-only build being a separate cached unit (a different feature graph), not evidence the tests ran stale code.
 
 **Claude Code web:** `task setup-web` installs zsh/fish/nushell, `gh`, and dev tools. Install `task` first if needed: `sh -c "$(curl --location https://taskfile.dev/install.sh)" -- -d -b ~/bin` then `export PATH="$HOME/bin:$PATH"`. The permission tests (`test_permission_error_prevents_save`, `test_approval_prompt_permission_error`) skip automatically when running as root.
 
@@ -116,7 +116,7 @@ cmd.args(["switch", "--create", "feature"])
 
 ```rust
 // ❌ BAD: Missing isolation - inherits host environment
-let output = Command::new(env!("CARGO_BIN_EXE_wt"))
+let output = Command::new(wt_bin())
     .args(["switch", "--create", "feature"])
     .current_dir(repo.root_path())
     .output()?;
@@ -389,6 +389,7 @@ Tests run once. Worktrunk configures no nextest `retries` and writes no retry lo
 - A racy assertion is a timing bug. Make it deterministic, per Timing Tests above: poll for the event, or drive it causally through a callback.
 - Resource pressure is a concurrency bug. Windows process creation intermittently fails with STATUS_DLL_INIT_FAILED (exit `-1073741502`) when many tests spawn git/wt children at once. Bound how many heavy tests run together; that removes the pressure instead of retrying past it.
 - A shared channel with more than one producer is an attribution bug. Counting events drained from a channel between steps only measures the step that produced them if nothing *else* can produce them: a background task's event landing after its step's drain is charged to the next step, so the assertion that fails names the wrong step and the wrong cause. The events are usually indistinguishable (skim's `Event::RunPreview` carries no payload), so identity assertions aren't available — quiesce the other producers instead, before the step sequence arms whatever they'd match. `on_update_pokes_run_preview_only_when_the_visible_pane_changes` is the worked example: `PreviewOrchestrator::wait_for_idle()` before each `note_awaiting`, so the skeleton precompute lands while no key matches it.
+- A spawn that fails `NotFound` is a concurrent-build bug. Cargo uplifts `target/debug/wt` by removing the path and recreating it, so a second `cargo` against the same target directory leaves the binary absent for a fraction of a millisecond per rebuild, failing whatever spawn is in flight with `Os { code: 2, kind: NotFound }` — anywhere: an `insta_cmd` snapshot, a PTY wrapper's `wt config shell init`, a hook whose marker file then never appears — and passing on re-run, which is the signature. `wt_bin()` closes the window by spawning a hardlink pinned under `target/debug/wt-test-bin/` instead of the uplifted path (`testing::pin_test_binary`), so route every `wt` spawn through it or a helper that does; `test_wt_spawns_are_pinned` makes a direct `CARGO_BIN_EXE_wt` spawn fail the suite.
 - A shared namespace is a collision bug. **Never `NamedTempFile::new()`** for a file a test needs by name: `tempfile` retries a name collision only when it surfaces as `AlreadyExists`, and on Windows `create_new` against a name already held by a *directory* — or by a file in delete-pending state — comes back `PermissionDenied`, which it hands straight back to the caller. A full suite run leaves the temp directory full of `.tmpXXXXXX` entries (every `TestRepo` makes one), and under that load the call fails ~1% of the time with `Access is denied.` — a panic that has nothing to do with what the test asserts. Take a `TempDir` and give the files fixed names inside it (`worktrunk::testing::directive_files` is the pattern); a directory collision surfaces as `AlreadyExists`, which tempfile retries.
 
 **Reproducing a Windows-only flake** means reproducing its *neighbours*, not starving it: run the suspect tests in a loop on a Windows runner with a full `cargo nextest run` going alongside. Pinning the CPU instead models the wrong thing whenever the failing run's own timing was normal (compare its duration against the same test passing — nextest prints it, and the `test` job uploads `junit.xml` with every test's time). Measured both ways on the same five tests: 80 iterations under a concurrent full suite reproduced a 1-in-100 Windows failure that no local run had ever shown, while pinning all four cores with busy loops instead just pushed nearly every iteration past the 30s waits — artificial failures that say nothing about the flake.