From 7a1391fbc1e3bccb3050c8607e0387cd4b286437 Mon Sep 17 00:00:00 2001 From: reacechalmers Date: Thu, 13 Aug 2026 14:49:16 -0600 Subject: [PATCH] fix(test): isolate tests from the machine's real user config and kernel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite asserted against whatever the developer's box happened to be configured as. Two independent leaks, both surfaced by a real `~/.claw`: 1. `parse_args` resolves defaults — notably `permissions.defaultMode` — through the user-scope config, which `default_config_home()` reads from `CLAW_CONFIG_HOME` falling back to `$HOME/.claw`. Nothing overrode it, so a `defaultMode` of `dontAsk` on the host turned 12 assertions from `WorkspaceWrite` into `DangerFullAccess`. `env_lock()` now returns an `EnvGuard` that points `CLAW_CONFIG_HOME` at a fresh empty directory and restores the previous value on drop, so "no user config" is the state under test. All 42 call sites already bind the guard, so this fixes them at once; tests that want a populated config home can still set the var themselves afterwards. `removed_login_and_logout_subcommands_error_helpfully` asserted a Default-sourced permission mode without taking the guard at all, and now does. 2. `direct_resume_safe_slash_commands_route_to_local_json_actions_831` hardcoded `/sandbox` => "warn", which is only true where namespace isolation is unsupported and just the filesystem sandbox is active (#731). On a host with working user namespaces the correct answer is "ok". That entry now accepts either; the test is about #831 routing — local JSON action instead of `interactive_only` — not about the kernel under the runner. Verified with the real ~/.claw/settings.json in place (defaultMode=dontAsk): cargo test --workspace 0 failures, clippy --workspace --all-targets -D warnings 0 findings, scripts/fmt.sh clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Q9McEEH32ZtyM62GSXVSyJ (cherry picked from commit 67aa4530c22f6fdf95a29d6a46992af137134c67) --- rust/crates/rusty-claude-cli/src/main.rs | 51 +++++++++++++++++-- .../tests/output_format_contract.rs | 21 +++++--- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 4c1d847a..9f0dd044 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -14823,11 +14823,54 @@ mod tests { ); } - fn env_lock() -> MutexGuard<'static, ()> { + /// Serialises tests that mutate process-global state, and isolates them from the + /// developer's real `~/.claw`. + /// + /// `parse_args` resolves defaults — notably `permissions.defaultMode` — through the + /// user-scope config, which `default_config_home()` reads from `CLAW_CONFIG_HOME`, + /// falling back to `$HOME/.claw`. Without an override, these tests therefore assert + /// against whatever the machine running them happens to have configured: a + /// `defaultMode` of `dontAsk` there resolves to `DangerFullAccess` and turns a dozen + /// `WorkspaceWrite` assertions red. Point the config home at a fresh empty directory + /// so "no user config" is the state under test, and restore the caller's value on drop. + /// + /// Tests that need a *populated* config home can still set `CLAW_CONFIG_HOME` + /// themselves after taking this guard; the guard restores the real value afterwards. + struct EnvGuard { + _lock: MutexGuard<'static, ()>, + config_home: PathBuf, + previous_config_home: Option, + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + match self.previous_config_home.take() { + Some(previous) => std::env::set_var("CLAW_CONFIG_HOME", previous), + None => std::env::remove_var("CLAW_CONFIG_HOME"), + } + // Best effort: a leftover temp dir must never fail a test. + let _ = std::fs::remove_dir_all(&self.config_home); + } + } + + fn env_lock() -> EnvGuard { static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) + let lock = LOCK + .get_or_init(|| Mutex::new(())) .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) + .unwrap_or_else(std::sync::PoisonError::into_inner); + + // Deliberately empty: the point is that no user settings.json is discoverable. + let config_home = temp_dir(); + std::fs::create_dir_all(&config_home).expect("isolated config home should be creatable"); + let previous_config_home = std::env::var_os("CLAW_CONFIG_HOME"); + std::env::set_var("CLAW_CONFIG_HOME", &config_home); + + EnvGuard { + _lock: lock, + config_home, + previous_config_home, + } } fn with_current_dir(cwd: &Path, f: impl FnOnce() -> T) -> T { @@ -15554,6 +15597,8 @@ mod tests { #[test] fn removed_login_and_logout_subcommands_error_helpfully() { + // Asserts a `Default`-sourced permission mode, so it must not see a user config. + let _guard = env_lock(); let login = parse_args(&["login".to_string()]).expect_err("login should be removed"); assert!(login.contains("ANTHROPIC_API_KEY")); let logout = parse_args(&["logout".to_string()]).expect_err("logout should be removed"); diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index 02a8eccb..f620797e 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -451,11 +451,16 @@ fn direct_resume_safe_slash_commands_route_to_local_json_actions_831() { .output() .expect("git init should launch"); - for (command, expected_kind, expected_status) in [ - ("/version", "version", "ok"), - ("/sandbox", "sandbox", "warn"), - ("/diff", "diff", "ok"), - ("/status", "status", "ok"), + // `/sandbox`'s status reflects the host's real sandbox capability, so it is the one + // entry with more than one legitimate value: "ok" where namespace isolation works, + // "warn" where it is unsupported and only the filesystem sandbox is active (#731). + // This test is about #831 routing — that these commands produce a local JSON action + // instead of `interactive_only` — not about the kernel the suite happens to run on. + for (command, expected_kind, expected_statuses) in [ + ("/version", "version", &["ok"][..]), + ("/sandbox", "sandbox", &["ok", "warn"][..]), + ("/diff", "diff", &["ok"][..]), + ("/status", "status", &["ok"][..]), ] { let output = run_claw(&root, &["--output-format", "json", command], &[]); assert!( @@ -470,9 +475,9 @@ fn direct_resume_safe_slash_commands_route_to_local_json_actions_831() { .unwrap_or_else(|_| panic!("{command} must emit JSON (#831), got: {stdout:?}")); assert_eq!(parsed["kind"], expected_kind, "{command} kind: {parsed}"); - assert_eq!( - parsed["status"], expected_status, - "{command} status: {parsed}" + assert!( + expected_statuses.contains(&parsed["status"].as_str().unwrap_or_default()), + "{command} status should be one of {expected_statuses:?}: {parsed}" ); assert_ne!( parsed["error_kind"], "interactive_only",