vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

hive-bash-mcp: share one HARNESS_DIR_ENV_LOCK across test modules

Browse source
This commit is contained in:
damocles 2026-08-10 23:35:52 +02:00
commit 4fd25b7f45
4 changed files with 54 additions and 61 deletions
Download patch file Download diff file Expand all files Collapse all files

2
hive-bash-mcp/src/lib.rs
View file

@ -9,3 +9,5 @@ pub mod paths;
pub mod protocol;
pub mod runner;
pub mod stats;
#[cfg(test)]
mod test_util;

31
hive-bash-mcp/src/mcp.rs
View file

@ -289,36 +289,7 @@ pub async fn serve_http(addr: std::net::SocketAddr) -> anyhow::Result<()> {
mod status_hint_tests {
use super::{BASH_IDLE_WAIT_HINT, format_task};
use crate::protocol::{TaskFile, TaskStatus};
use std::sync::Mutex;
/// `format_task` unconditionally resolves `crate::paths::task_out`/
/// `task_err` (to check captured-output length), which panics if
/// `HYPERHIVE_HARNESS_DIR` is unset — exactly cargo's sandboxed test
/// environment. Same helper shape as `runner::tests::with_harness_dir`
/// (module-scope mutex to serialise against parallel test threads
/// mutating the process-wide env var, save/restore on the way out);
/// duplicated rather than shared because the two test modules live in
/// separate files with no existing shared test-util module.
static HARNESS_DIR_ENV_LOCK: Mutex<()> = Mutex::new(());
fn with_harness_dir<F: FnOnce()>(f: F) {
let _guard = HARNESS_DIR_ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let prev = std::env::var("HYPERHIVE_HARNESS_DIR").ok();
// SAFETY: serialised by HARNESS_DIR_ENV_LOCK above; restored below
// in the same scope before the guard drops.
unsafe {
std::env::set_var("HYPERHIVE_HARNESS_DIR", "/tmp/hive-bash-mcp-test-harness");
}
f();
unsafe {
match prev {
Some(v) => std::env::set_var("HYPERHIVE_HARNESS_DIR", v),
None => std::env::remove_var("HYPERHIVE_HARNESS_DIR"),
}
}
}
use crate::test_util::with_harness_dir;
fn task(status: TaskStatus) -> TaskFile {
TaskFile {

32
hive-bash-mcp/src/runner.rs
View file

@ -796,38 +796,8 @@ fn done_summary(id: &str, summary: &str, output: Option<(bool, bool)>) -> String
#[cfg(test)]
mod tests {
use super::validate_task_name;
use crate::test_util::with_harness_dir;
use hive_types::Ident;
use std::sync::Mutex;
/// `done_summary`'s stderr/stdout-pointer tests format real paths via
/// `crate::paths::task_out`/`task_err`, which resolve through
/// `hive_agent_sock::paths::harness_dir` — that panics if
/// `HYPERHIVE_HARNESS_DIR` is unset (see its doc comment), which is
/// exactly cargo's sandboxed test environment (no real container, no
/// meta-flake-injected env). Set a dummy value for the duration of
/// those tests, serialised on a module mutex so parallel test threads
/// don't race the process-wide env var, and restore whatever was there
/// before on the way out.
static HARNESS_DIR_ENV_LOCK: Mutex<()> = Mutex::new(());
fn with_harness_dir<F: FnOnce()>(f: F) {
let _guard = HARNESS_DIR_ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let prev = std::env::var("HYPERHIVE_HARNESS_DIR").ok();
// SAFETY: serialised by HARNESS_DIR_ENV_LOCK above; restored below
// in the same scope before the guard drops.
unsafe {
std::env::set_var("HYPERHIVE_HARNESS_DIR", "/tmp/hive-bash-mcp-test-harness");
}
f();
unsafe {
match prev {
Some(v) => std::env::set_var("HYPERHIVE_HARNESS_DIR", v),
None => std::env::remove_var("HYPERHIVE_HARNESS_DIR"),
}
}
}
#[test]
fn accepts_ident_names() {

50
hive-bash-mcp/src/test_util.rs Normal file
View file

@ -0,0 +1,50 @@
//! Test-only helper shared by `mcp::status_hint_tests` and `runner::tests`.
//!
//! Both modules need `HYPERHIVE_HARNESS_DIR` set for the duration of a test
//! (`crate::paths::harness_dir` panics if it's unset — deliberately, see
//! its doc comment — and that's exactly cargo's sandboxed test env with no
//! meta-flake-injected value). The env var is one process-global, and both
//! modules land in the *same* test binary (same crate) with tests running
//! on parallel threads, so a single shared lock here is required — two
//! separate per-module mutexes serialise nothing against each other — that
//! caused a CI-only flake, since a container always has the var set so a
//! losing race still finds a valid restored value, while the nix sandbox
//! strips it, so a racing thread can delete the var out from under another
//! mid-test.
//!
//! ⚠️ **This asymmetry also breaks local reproduction**: a plain `cargo
//! test` inside an agent container can never fail this test, fixed or
//! broken — `HYPERHIVE_HARNESS_DIR` is always ambient-set there, so the
//! restore branch always takes `set_var`, never `remove_var`. To actually
//! exercise the sandbox shape (and prove a fix works, or reproduce the
//! original bug), unset the var first: `env -u HYPERHIVE_HARNESS_DIR
//! cargo test -p hive-bash-mcp`. Any *new* helper that reaches for the env
//! var in a test should route through `with_harness_dir` below rather than
//! rolling its own save/restore — a second lock reintroduces exactly this
//! bug.
use std::sync::Mutex;
static HARNESS_DIR_ENV_LOCK: Mutex<()> = Mutex::new(());
/// Run `f` with `HYPERHIVE_HARNESS_DIR` set to a dummy path, serialised
/// against every other caller of this helper in the process so parallel
/// test threads can't race the env var out from under each other.
/// Restores whatever value (or absence) was there before on the way out.
pub fn with_harness_dir<F: FnOnce()>(f: F) {
let _guard = HARNESS_DIR_ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let prev = std::env::var("HYPERHIVE_HARNESS_DIR").ok();
// SAFETY: serialised by HARNESS_DIR_ENV_LOCK above; restored below in
// the same scope before the guard drops.
unsafe {
std::env::set_var("HYPERHIVE_HARNESS_DIR", "/tmp/hive-bash-mcp-test-harness");
}
f();
unsafe {
match prev {
Some(v) => std::env::set_var("HYPERHIVE_HARNESS_DIR", v),
None => std::env::remove_var("HYPERHIVE_HARNESS_DIR"),
}
}
}