From 7ba492b9651b154e1f48a0fc69b6d71e1aa28c3d Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 9 Aug 2026 18:59:46 +0200 Subject: [PATCH 1/2] move hive-sh4re's harness_dir() into hive-agent-sock, drop the dev-fallback derivation --- hive-agent-sock/src/lib.rs | 2 ++ hive-agent-sock/src/paths.rs | 30 ++++++++++++++++++++++++++++++ hive-agent/src/paths.rs | 10 +++++----- hive-bash-mcp/src/paths.rs | 8 ++++---- hive-sh4re/src/paths.rs | 35 +++++------------------------------ nix/agent-modules/mcp.nix | 6 +++--- 6 files changed, 49 insertions(+), 42 deletions(-) create mode 100644 hive-agent-sock/src/paths.rs diff --git a/hive-agent-sock/src/lib.rs b/hive-agent-sock/src/lib.rs index 76d8fb5b..83d92b34 100644 --- a/hive-agent-sock/src/lib.rs +++ b/hive-agent-sock/src/lib.rs @@ -18,6 +18,8 @@ use serde::{Deserialize, Serialize}; use hive_sh4re::LooseEnd; +pub mod paths; + /// In-container path of the harness-served in-agent socket. The harness /// binds it on boot; the in-container producers dial it for todo ops. /// (Placeholder default — the harness + producers resolve the real path diff --git a/hive-agent-sock/src/paths.rs b/hive-agent-sock/src/paths.rs new file mode 100644 index 00000000..59816483 --- /dev/null +++ b/hive-agent-sock/src/paths.rs @@ -0,0 +1,30 @@ +//! In-container harness-directory resolution, shared by the harness +//! itself and every producer daemon that dials the in-agent socket +//! (`hive-bash-mcp`, and any future producer that writes artifacts +//! alongside the harness's own state). + +use std::path::PathBuf; + +/// Base harness directory for the current agent. Reads +/// `HYPERHIVE_HARNESS_DIR`, always injected by the meta flake's +/// `systemd.globalEnvironment` for every in-container service +/// (`nix/host-modules` — see `hive-c0re/src/meta.rs`'s per-agent +/// environment block). Every process this crate's wire types serve +/// (the harness, `hive-bash-daemon`, ...) runs under that same +/// environment, so there is no legitimate runtime path where it's +/// unset — a missing var means the container is misconfigured, not +/// that a fallback derivation should paper over it. +/// +/// # Panics +/// +/// Panics if `HYPERHIVE_HARNESS_DIR` is not set. Deliberate: a wrong +/// silently-derived path here means task files, sqlite stores, and the +/// in-agent socket itself could resolve to the wrong directory — a +/// loud crash at startup beats a quiet cross-agent path collision. +#[must_use] +pub fn harness_dir() -> PathBuf { + PathBuf::from( + std::env::var_os("HYPERHIVE_HARNESS_DIR") + .expect("HYPERHIVE_HARNESS_DIR must be set — the meta flake injects it for every in-container service; a missing value means the container's environment is misconfigured"), + ) +} diff --git a/hive-agent/src/paths.rs b/hive-agent/src/paths.rs index f8acefb4..45158138 100644 --- a/hive-agent/src/paths.rs +++ b/hive-agent/src/paths.rs @@ -31,13 +31,13 @@ pub fn state_dir() -> PathBuf { /// (`hyperhive-events.sqlite`, `hyperhive-turn-stats.sqlite`, /// `hyperhive-model`) so they do not appear inside the agent-visible /// `/agents/{label}/state` tree. Delegates to the shared canonical -/// resolver in `hive_sh4re::paths` so the harness + every out-of-process -/// MCP daemon resolve this identically (reads `HYPERHIVE_HARNESS_DIR`, -/// then a `harness/` sibling of `HYPERHIVE_STATE_DIR`, then -/// `/agents/{HIVE_LABEL}/harness`). +/// resolver in `hive_agent_sock::paths` so the harness + every +/// out-of-process MCP daemon resolve this identically (reads +/// `HYPERHIVE_HARNESS_DIR`, always injected by the meta flake — panics +/// if it's unset rather than silently deriving a fallback path). #[must_use] pub fn harness_dir() -> PathBuf { - hive_sh4re::paths::harness_dir() + hive_agent_sock::paths::harness_dir() } /// Consolidated harness-local state db — todos + reminders + the questions diff --git a/hive-bash-mcp/src/paths.rs b/hive-bash-mcp/src/paths.rs index a68f6cb5..a75725bc 100644 --- a/hive-bash-mcp/src/paths.rs +++ b/hive-bash-mcp/src/paths.rs @@ -6,12 +6,12 @@ use std::path::PathBuf; /// Base harness directory. Shared resolution lives in -/// `hive_sh4re::paths::harness_dir` so the harness + every MCP daemon -/// agree on the layout. Re-exported here as the base for the per-agent -/// artifact paths below. +/// `hive_agent_sock::paths::harness_dir` so the harness + every MCP +/// daemon agree on the layout. Re-exported here as the base for the +/// per-agent artifact paths below. #[must_use] pub fn harness_dir() -> PathBuf { - hive_sh4re::paths::harness_dir() + hive_agent_sock::paths::harness_dir() } /// Base directory for task files. diff --git a/hive-sh4re/src/paths.rs b/hive-sh4re/src/paths.rs index a869e2a0..c73ba9ba 100644 --- a/hive-sh4re/src/paths.rs +++ b/hive-sh4re/src/paths.rs @@ -1,33 +1,8 @@ -//! Shared in-container filesystem-path resolution. -//! -//! Every process that runs inside an agent container - the harness plus -//! the out-of-process MCP daemons (bash, matrix, ...) - must resolve the -//! harness directory layout identically. These helpers live here so the -//! resolution exists in exactly one place rather than being mirrored -//! across crates. - -use std::path::PathBuf; - -/// Base harness directory for the current agent. Uses `HYPERHIVE_HARNESS_DIR` -/// if set (injected by the hive-c0re meta flake after the harness/state -/// split). For pre-split / dev deployments where it isn't set, falls back to -/// a `harness/` sibling of `HYPERHIVE_STATE_DIR`, and finally to -/// `/agents/{HIVE_LABEL}/harness` when neither dir env var is present — the -/// shape the harness derives from its label alone. -#[must_use] -pub fn harness_dir() -> PathBuf { - if let Some(p) = std::env::var_os("HYPERHIVE_HARNESS_DIR") { - return PathBuf::from(p); - } - if let Some(state) = std::env::var_os("HYPERHIVE_STATE_DIR") { - let state_path = PathBuf::from(&state); - if let Some(parent) = state_path.parent() { - return parent.join("harness"); - } - } - let label = std::env::var("HIVE_LABEL").unwrap_or_default(); - PathBuf::from(format!("/agents/{label}/harness")) -} +//! Shared filename constants for in-container filesystem paths. Harness +//! *directory* resolution itself lives in `hive_agent_sock::paths` +//! (every in-container producer already depends on that crate for the +//! socket wire types) — this module only keeps the bits that also need +//! to be reachable from the host side (`hive-c0re`). /// File name of the pause marker inside the harness dir. Re-exported from /// `hive-priv-sock`, which owns the definition because hive-priv (root) is diff --git a/nix/agent-modules/mcp.nix b/nix/agent-modules/mcp.nix index dafeda83..24a0f939 100644 --- a/nix/agent-modules/mcp.nix +++ b/nix/agent-modules/mcp.nix @@ -261,9 +261,9 @@ in # injected via systemd.globalEnvironment by the meta flake # (set to /agents//harness and /agents//state # respectively). The daemon uses these to derive its task + - # loose-ends dir paths; without them it falls back to deriving - # harness/ as a sibling of state/, which produces the same - # value but is less robust if the two vars ever diverge. + # loose-ends dir paths; `hive_agent_sock::paths::harness_dir` + # panics loudly if HYPERHIVE_HARNESS_DIR is unset rather than + # deriving a fallback, since every service here always gets it. }; serviceConfig = { ExecStart = "${config.hyperhive.packages.hive-bash-daemon}/bin/hive-bash-daemon --http 127.0.0.1:${toString config.hyperhive.mcp.bashHttpPort}"; From 22f8805a77dc1e56a4faa186cfb86dc2d71b7768 Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 9 Aug 2026 19:41:47 +0200 Subject: [PATCH 2/2] fix ci: gate harness_dir-dependent tests behind a dummy env var --- hive-bash-mcp/src/mcp.rs | 48 +++++++++++++++++++---- hive-bash-mcp/src/runner.rs | 77 +++++++++++++++++++++++++++---------- 2 files changed, 98 insertions(+), 27 deletions(-) diff --git a/hive-bash-mcp/src/mcp.rs b/hive-bash-mcp/src/mcp.rs index 59545a0a..20efddc0 100644 --- a/hive-bash-mcp/src/mcp.rs +++ b/hive-bash-mcp/src/mcp.rs @@ -289,6 +289,36 @@ 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: 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"), + } + } + } fn task(status: TaskStatus) -> TaskFile { TaskFile { @@ -307,16 +337,20 @@ mod status_hint_tests { #[test] fn running_task_after_wait_appends_idle_hint() { - let t = task(TaskStatus::Running); - let mut out = format_task(&t); - out.push_str(BASH_IDLE_WAIT_HINT); - assert!(out.contains(BASH_IDLE_WAIT_HINT)); + with_harness_dir(|| { + let t = task(TaskStatus::Running); + let mut out = format_task(&t); + out.push_str(BASH_IDLE_WAIT_HINT); + assert!(out.contains(BASH_IDLE_WAIT_HINT)); + }); } #[test] fn done_task_formats_without_hint() { - let out = format_task(&task(TaskStatus::Done)); - assert!(!out.contains(BASH_IDLE_WAIT_HINT)); - assert!(out.contains("status=done")); + with_harness_dir(|| { + let out = format_task(&task(TaskStatus::Done)); + assert!(!out.contains(BASH_IDLE_WAIT_HINT)); + assert!(out.contains("status=done")); + }); } } diff --git a/hive-bash-mcp/src/runner.rs b/hive-bash-mcp/src/runner.rs index 789d4633..e2f1843b 100644 --- a/hive-bash-mcp/src/runner.rs +++ b/hive-bash-mcp/src/runner.rs @@ -797,6 +797,37 @@ fn done_summary(id: &str, summary: &str, output: Option<(bool, bool)>) -> String mod tests { use super::validate_task_name; 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: 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() { @@ -881,35 +912,41 @@ mod tests { #[test] fn done_summary_stdout_only_is_unflagged() { use super::done_summary; - let body = done_summary("t1", "exit=0", Some((true, false))); - assert!(body.contains("output captured — read the full text with:")); - assert!(body.contains("Read(")); - assert!(body.contains(".out")); - assert!(!body.contains(".err")); - assert!(!body.contains('\u{26a0}'), "no flag without stderr"); + with_harness_dir(|| { + let body = done_summary("t1", "exit=0", Some((true, false))); + assert!(body.contains("output captured — read the full text with:")); + assert!(body.contains("Read(")); + assert!(body.contains(".out")); + assert!(!body.contains(".err")); + assert!(!body.contains('\u{26a0}'), "no flag without stderr"); + }); } #[test] fn done_summary_stderr_present_is_flagged_regardless_of_summary_text() { use super::done_summary; - // exit=0 in the summary — a swallowed mid-chain failure that - // still exits clean. The flag must fire anyway, keyed on - // has_stderr, not on the summary text. - let body = done_summary("t1", "exit=0", Some((true, true))); - assert!( - body.contains("⚠️ stderr present"), - "stderr present must flag even on a clean exit code" - ); - let err_pos = body.find(".err").expect("err pointer present"); - let out_pos = body.find(".out").expect("out pointer present"); - assert!(err_pos < out_pos, ".err must be listed before .out"); + with_harness_dir(|| { + // exit=0 in the summary — a swallowed mid-chain failure that + // still exits clean. The flag must fire anyway, keyed on + // has_stderr, not on the summary text. + let body = done_summary("t1", "exit=0", Some((true, true))); + assert!( + body.contains("⚠️ stderr present"), + "stderr present must flag even on a clean exit code" + ); + let err_pos = body.find(".err").expect("err pointer present"); + let out_pos = body.find(".out").expect("out pointer present"); + assert!(err_pos < out_pos, ".err must be listed before .out"); + }); } #[test] fn done_summary_stderr_only_omits_out_pointer() { use super::done_summary; - let body = done_summary("t1", "exit=1", Some((false, true))); - assert!(body.contains(".err")); - assert!(!body.contains(".out"), "no stdout ⇒ no stdout pointer"); + with_harness_dir(|| { + let body = done_summary("t1", "exit=1", Some((false, true))); + assert!(body.contains(".err")); + assert!(!body.contains(".out"), "no stdout ⇒ no stdout pointer"); + }); } }