From 3aa4f69b7eeb08890c483546455409fbd5673e91 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Date: Sun, 26 Jul 2026 12:28:51 -0400 Subject: [PATCH] test(desktop): make runtime subprocess tests PATH-independent Wren and Dawn both reproduced two more parallel-test flakes at the merge head, same class as the grandchild_inherits_pgid fix one commit earlier: claude_spawn_uses_the_probed_cli_executable holds lock_path_mutex while swapping PATH to a tempdir, and any sibling test doing a bare-name PATH lookup in that window fails with NotFound. - own_group_grandchild_detected_by_ancestor_walk: bare `sh` -> absolute /bin/sh, and the script's inner `sleep` -> /bin/sleep (a child spawned during the swap window inherits the poisoned PATH for life, so inner lookups must be absolute too) - make_pair_runtime_placeholder (kill_stale_live_pair_is_not_touched): bare `true` -> /usr/bin/true on unix (present on macOS and Linux); Windows keeps the PATH lookup since no test there swaps PATH - grandchild_inherits_pgid_of_process_group_leader: inner `sleep` lookups also made absolute, closing the same residual window Chose PATH-independence over taking lock_path_mutex (Wren's noted preference): no new lock scope, no serialization of unrelated tests. Test-only. Repro at 0ec684477: 5/8 scoped parallel runs failed (cargo test --lib managed_agents::runtime::tests -- --test-threads=16). After: 10/10 scoped runs green, 4/4 full managed_agents parallel runs green (805/805 each). Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../src/managed_agents/runtime/tests.rs | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 95a78256d..362775326 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -710,15 +710,20 @@ fn grandchild_inherits_pgid_of_process_group_leader() { // getpgid returned -1). The group is killed in cleanup, so the sleep // never runs to term. // - // `/bin/sh` (absolute) rather than `sh`: parallel tests holding - // `lock_path_mutex` legitimately swap PATH to a tempdir, and this test - // doesn't need the lock — but a PATH lookup during the swap window - // fails with NotFound (observed flake). + // Absolute `/bin/sh` and `/bin/sleep` rather than bare names: parallel + // tests holding `lock_path_mutex` legitimately swap PATH to a tempdir, + // and this test doesn't need the lock — but a PATH lookup during the + // swap window fails with NotFound, and a child spawned during it + // inherits the poisoned PATH for its lifetime, so the script's inner + // lookups must be absolute too (observed flake). let mut harness = { let mut cmd = Command::new("/bin/sh"); - cmd.args(["-c", "/bin/sh -c 'sleep 10 & echo $!' & wait $!; sleep 10"]) - .stdout(std::process::Stdio::piped()) - .process_group(0); + cmd.args([ + "-c", + "/bin/sh -c '/bin/sleep 10 & echo $!' & wait $!; /bin/sleep 10", + ]) + .stdout(std::process::Stdio::piped()) + .process_group(0); cmd.spawn().expect("spawn harness") }; @@ -802,9 +807,15 @@ fn own_group_grandchild_detected_by_ancestor_walk() { // The test process is the "harness". Spawn an intermediate with its own // process group (mirrors the node shim). It backgrounds a grandchild // (sleep 30) and prints the grandchild PID so we can inspect it. + // + // Absolute `/bin/sh` and `/bin/sleep` — no PATH lookups anywhere in this + // tree. Parallel tests holding `lock_path_mutex` legitimately swap PATH to + // a tempdir; the outer spawn during that window fails with NotFound, and a + // child spawned during it inherits the poisoned PATH for its lifetime, so + // inner lookups must be absolute too (observed flake). let mut intermediate = { - let mut cmd = Command::new("sh"); - cmd.args(["-c", "sleep 30 & echo $!; wait"]) + let mut cmd = Command::new("/bin/sh"); + cmd.args(["-c", "/bin/sleep 30 & echo $!; wait"]) .stdout(std::process::Stdio::piped()) .process_group(0); cmd.spawn().expect("spawn intermediate") @@ -1232,7 +1243,16 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun use std::process::{Command, Stdio}; // Spawn a real child so ManagedAgentProcess's Child field is satisfied. // `true` exits immediately with 0 — just a handle we need for type purposes. - let child = Command::new("true") + // + // Absolute `/usr/bin/true` on unix (present on both macOS and Linux): + // parallel tests holding `lock_path_mutex` swap PATH to a tempdir, and a + // bare `true` lookup during that window fails with NotFound (observed + // flake). Windows keeps the PATH lookup — no test there swaps PATH. + #[cfg(unix)] + let program = "/usr/bin/true"; + #[cfg(windows)] + let program = "true"; + let child = Command::new(program) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null())