From 07f8e86c58a0aa5378537a3daa4610361fdbad55 Mon Sep 17 00:00:00 2001 From: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz> Date: Thu, 10 Sep 2026 16:43:01 -0600 Subject: [PATCH] test(desktop): isolate login-shell probe measurements Run the counter-owning discovery tests in bounded, exact-filtered libtest processes. Keep process-wide counting so auth workers remain covered, require a completed assertion body, and reject unisolated resets. Add parent-isolation and own-worker controls without changing production discovery. Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz> --- .../discovery/login_shell_spawn_probe.rs | 87 +++++++++++++++++++ .../discovery/tests/forced_discovery.rs | 9 +- .../tests/managed_path_resolution.rs | 11 ++- 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs b/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs index a716dee9f56..ad3a4dad23d 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs @@ -13,9 +13,96 @@ pub(crate) fn record() { } pub(crate) fn reset() { + #[cfg(unix)] + assert!( + is_isolated_process(), + "counter measurements must run in an isolated test process" + ); COUNT.store(0, Ordering::SeqCst); } pub(crate) fn count() -> usize { COUNT.load(Ordering::SeqCst) } + +#[cfg(unix)] +const CHILD_TEST: &str = "BUZZ_LOGIN_SHELL_PROBE_TEST"; + +#[cfg(unix)] +fn is_isolated_process() -> bool { + std::thread::current() + .name() + .is_some_and(|name| std::env::var(CHILD_TEST).as_deref() == Ok(name)) +} + +/// Run a counter-owning test body alone in a fresh libtest process. +/// +/// The counter deliberately stays process-wide: discovery's auth workers must +/// count too. A voluntary PATH lock cannot exclude unrelated, unlocked probe +/// callers in the full suite. Process isolation excludes those callers without +/// changing production code or requiring new workers to inherit test context. +/// Call while holding the PATH lock so the child inherits stable environment. +#[cfg(unix)] +pub(crate) fn run_in_isolated_process(test: impl FnOnce()) { + use std::process::Command; + use std::time::Duration; + + let thread = std::thread::current(); + let name = thread.name().expect("libtest names its test threads"); + let completed = format!("completed isolated login-shell probe test: {name}"); + if is_isolated_process() { + test(); + // Receipt only after the assertion body returns, never on entry. + println!("{completed}"); + return; + } + + let mut command = Command::new(std::env::current_exe().expect("test executable")); + command + .args(["--exact", name, "--nocapture", "--test-threads=1"]) + .env(CHILD_TEST, name); + let output = super::bounded_command::output_with_timeout(command, Duration::from_secs(300)) + .expect("isolated probe test must finish within five minutes and the output cap"); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success() && stdout.lines().any(|line| line.contains(&completed)), + "isolated probe test {name} failed or did not run: {}\n{stdout}\n{stderr}", + output.status + ); +} + +#[cfg(unix)] +#[test] +fn isolated_counter_excludes_parent_probes_but_counts_own_workers() { + let _guard = crate::managed_agents::lock_path_mutex(); + let probe_on_worker = || { + std::thread::spawn(|| { + // A real production probe, independent of the shared PATH cache. + super::find_via_login_shell("buzz-absent-probe-isolation-xyzzy") + }) + .join() + .expect("probe worker must finish") + }; + if !is_isolated_process() { + assert!(probe_on_worker().is_none()); + assert!(count() >= 1, "parent worker must reach the real probe"); + } + run_in_isolated_process(|| { + // Before reset: unrelated parent probes must not enter this process. + assert_eq!(count(), 0, "the child must start with its own counter"); + reset(); + assert!(probe_on_worker().is_none()); + }); + if is_isolated_process() { + // Outside the closure so omitting its invocation cannot pass this test. + assert_eq!(count(), 1, "the child's assertion body and worker must run"); + } +} + +#[cfg(unix)] +#[test] +#[should_panic(expected = "counter measurements must run in an isolated test process")] +fn reset_rejects_unisolated_measurement() { + reset(); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs index cfbad365e3a..4b2ed0e908f 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs @@ -108,6 +108,14 @@ fn forced_discovery_probes_auth_but_cheap_discovery_reuses_cached_status() { #[cfg(unix)] #[test] fn cheap_discovery_reports_absent_before_any_forced_probe() { + let _path_guard = crate::managed_agents::lock_path_mutex(); + super::super::login_shell_spawn_probe::run_in_isolated_process( + cheap_discovery_reports_absent_before_any_forced_probe_body, + ); +} + +#[cfg(unix)] +fn cheap_discovery_reports_absent_before_any_forced_probe_body() { use crate::managed_agents::custom_harnesses::registry_test_lock; use crate::managed_agents::discovery::{ clear_resolve_cache, discover_acp_runtimes_from, login_shell_spawn_probe, @@ -115,7 +123,6 @@ fn cheap_discovery_reports_absent_before_any_forced_probe() { use crate::managed_agents::{AcpAvailabilityStatus, AuthStatus}; use std::os::unix::fs::PermissionsExt; - let _path_guard = crate::managed_agents::lock_path_mutex(); let _registry_guard = registry_test_lock(); let dir = tempfile::tempdir().expect("tempdir"); diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs index aab5cd45298..dce4c2910cb 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs @@ -126,6 +126,14 @@ fn resolve_command_prefers_buzz_managed_npm_shim_over_path() { #[cfg(unix)] #[test] fn cheap_discovery_never_spawns_login_shell_even_when_cold() { + let _path_guard = crate::managed_agents::lock_path_mutex(); + super::super::login_shell_spawn_probe::run_in_isolated_process( + cheap_discovery_never_spawns_login_shell_even_when_cold_body, + ); +} + +#[cfg(unix)] +fn cheap_discovery_never_spawns_login_shell_even_when_cold_body() { use crate::managed_agents::custom_harnesses::registry_test_lock; use crate::managed_agents::discovery::{ clear_resolve_cache, discover_acp_runtimes_from, login_shell_spawn_probe, @@ -133,9 +141,6 @@ fn cheap_discovery_never_spawns_login_shell_even_when_cold() { use std::fs; use tempfile::tempdir; - // Serialize with every other test that spawns a login shell: the spawn - // counter and the PATH/login-shell caches are process-global. - let _path_guard = crate::managed_agents::lock_path_mutex(); let _registry = registry_test_lock(); // A custom harness whose command cannot resolve anywhere, so the resolver