fix: make unknown-harness tests hermetic against ambient adapter env

[BunsDev]

Context

• Summary: Two tests assert hermes is an unknown harness but read real adapter config, so they fail on any host with a hermes adapter installed — notably under Coven Cave, which exports COVEN_HARNESS_ADAPTER_DIRS into every spawned session. One genuine panic under the shared env lock then poisons it, cascading into ~28 secondary PoisonError failures. Full root-cause analysis in Unknown-harness tests are not hermetic against ambient adapter env vars / trust store #442.
• Files changed: crates/coven-cli/src/{test_env.rs (new), harness.rs, api.rs, main.rs}
• Closes Unknown-harness tests are not hermetic against ambient adapter env vars / trust store #442

Implementation

• Approach:
  1. New #[cfg(test)] mod test_env: one crate-wide env_lock(), a poison-recovering lock_env(), and a shared EnvVarGuard (RAII set/remove + restore).
  2. harness.rs/main.rs test modules now use the shared lock/guard instead of per-module duplicates — per-module locks could not serialize cross-module env mutation (main.rs COVEN_HOME tests raced harness.rs adapter reads).
  3. Both unknown-harness tests neutralize all three ambient adapter sources under the lock: remove COVEN_HARNESS_ADAPTER_MANIFEST + COVEN_HARNESS_ADAPTER_DIRS, point COVEN_HOME at an empty tempdir (trust store).
  4. All 52 env_lock().lock().unwrap* call sites mechanically converted to lock_env() — one failed env test now reports one failure, not a poison cascade.
• User-visible behavior: none — test-only change; #[cfg(test)] module ships no production code.
• Compatibility notes: privacy.rs keeps its own disjoint lock (only touches COVEN_PERSIST_RAW_ARTIFACTS); intentionally untouched. Complements fix: make env-mutating unit tests hermetic #440 (hermeticity via DI seams for TUI/HOME vars) — zero file overlap in test scope.

Verification

• cargo fmt --check
• cargo clippy --workspace --all-targets -- -D warnings
• cargo test --workspace --locked — exit 0, 1268 coven-cli unit tests + all workspace suites green
• python3 scripts/check-secrets.py — clean
• Additional manual checks: red baseline reproduced on pristine main (2850c53) with COVEN_HARNESS_ADAPTER_DIRS=~/.coven/adapters set → exactly the 2 diagnosed failures (hermes resolved as adapter hermes-coven; API returned 201 not 400). Entire verification run with that var still exported — the tests are now hermetic against it.

Risk and Rollback

• Risk level: Low — test-only; no production code path touched.
• Rollback plan: revert the single commit; tests return to env-sensitive behavior.

Agent Handoff

• Current state: complete and gate-green; claim issue-442 released after PR creation.
• Follow-ups: Cave-side — stop exporting COVEN_HARNESS_ADAPTER_DIRS once released CLIs trust COVEN_HOME/adapters natively (tracked in coven-cave, not here).
• Known gaps: other env-reading tests remain hermetic-by-lock only; converting them to guards is a possible later chore, deliberately out of scope.

[BunsDev]

Review

LGTM. Correct diagnosis (#442), right-sized fix: one crate-wide #[cfg(test)] test_env module (single env_lock, poison-recovering lock_env(), shared RAII EnvVarGuard), the two unknown-harness tests neutralize all three ambient adapter sources (COVEN_HARNESS_ADAPTER_MANIFEST, COVEN_HARNESS_ADAPTER_DIRS, COVEN_HOME → empty tempdir), and the mechanical lock_env() conversion kills the poison cascade. Per-module locks genuinely couldn't serialize main.rs↔harness.rs env races, so the shared module is the correct shape; privacy.rs staying on its disjoint lock is right since COVEN_PERSIST_RAW_ARTIFACTS is touched nowhere else.

Patched during review

🔧 #443 landed after this branch forked and added one more env_lock().lock().unwrap() call site in harness.rs (hermes_recipe_forwards_selected_models_before_the_shim_prompt). The merge was textually clean but failed to compile (E0425: cannot find function env_lock) — reproduced by trial-merging origin/main. Pushed the fix: merged current main and converted the missed call site to lock_env() (0bfcac8); guard usage unchanged since the shared EnvVarGuard has the same API.

Verification (post-patch, local macOS)

• cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace --locked — all green (1269 unit tests + suites).
• Hermeticity re-confirmed empirically: with a hostile COVEN_HARNESS_ADAPTER_DIRS pointing at a real hermes manifest exported, command_parts_reject_unknown_harnesses, launch_request_with_unknown_harness_returns_400_upfront_no_session_row, and #443's hermes model test all pass.

[Copilot]

Pull request overview

This PR makes coven-cli unit tests hermetic against ambient harness-adapter configuration by introducing a single crate-wide env coordination module and updating env-touching tests to use it, preventing cross-module env races and avoiding mutex-poison cascades.

Changes:

• Add a new #[cfg(test)] test_env module providing a shared env lock (lock_env()) and an RAII env-var guard (EnvVarGuard).
• Replace per-module env locks with the shared lock across test modules to properly serialize process-env mutation/reads across the whole test binary.
• Update the two “unknown harness” tests to neutralize all ambient adapter sources (COVEN_HARNESS_ADAPTER_MANIFEST, COVEN_HARNESS_ADAPTER_DIRS, and COVEN_HOME trust store) under the shared lock.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
crates/coven-cli/src/test_env.rs	Introduces crate-wide test-only env coordination primitives (shared mutex + poison recovery + RAII env guards).
crates/coven-cli/src/main.rs	Wires in test_env under #[cfg(test)] and migrates main’s env-touching tests to the shared lock/guard.
crates/coven-cli/src/harness.rs	Migrates harness tests to the shared env lock and makes unknown-harness tests hermetic against ambient adapter config.
crates/coven-cli/src/api.rs	Hardens the unknown-harness API test by locking and neutralizing adapter-related env/trust-store inputs.

Comments suppressed due to low confidence (3)

crates/coven-cli/src/harness.rs:2448

• This test mutates the adapter-manifest env var and restores it manually. If any later assertion panics, the restore call won’t run and the env var will leak into subsequent tests, reintroducing cross-test flakiness. Prefer EnvVarGuard so restoration is panic-safe.

        let _guard = lock_env();
        let previous = env::var_os(EXTERNAL_ADAPTER_MANIFEST_ENV);
        env::set_var(EXTERNAL_ADAPTER_MANIFEST_ENV, &manifest);
        let parts =
            command_parts_for_harness("hermes", "audit repo", HarnessLaunchMode::NonInteractive);

crates/coven-cli/src/harness.rs:2509

• This test sets EXTERNAL_ADAPTER_MANIFEST_ENV and restores it manually at the end. If any assertion fails before the restore call, the env var remains set and can affect later tests. Use EnvVarGuard to ensure the env is always restored on unwind.

        let _guard = lock_env();
        let previous = env::var_os(EXTERNAL_ADAPTER_MANIFEST_ENV);
        env::set_var(EXTERNAL_ADAPTER_MANIFEST_ENV, &manifest);
        let supports_stream = harness_supports_stream_mode("streamy");
        let supports_session = harness_supports_preassigned_session_id("streamy");

crates/coven-cli/src/harness.rs:2806

• This test mutates EXTERNAL_ADAPTER_MANIFEST_ENV/EXTERNAL_ADAPTER_DIRS_ENV and restores them manually. A panic between mutation and the restore calls will leak env state into later tests. Prefer EnvVarGuard (remove/set) so restoration is guaranteed even on unwind.

        let _guard = lock_env();
        let previous_manifest = env::var_os(EXTERNAL_ADAPTER_MANIFEST_ENV);
        let previous_dirs = env::var_os(EXTERNAL_ADAPTER_DIRS_ENV);
        env::remove_var(EXTERNAL_ADAPTER_MANIFEST_ENV);
        env::set_var(EXTERNAL_ADAPTER_DIRS_ENV, temp_dir.path());

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.