diff --git a/src/openhuman/memory/ops/documents.rs b/src/openhuman/memory/ops/documents.rs index 9314f81bf3..bb2cca8116 100644 --- a/src/openhuman/memory/ops/documents.rs +++ b/src/openhuman/memory/ops/documents.rs @@ -496,8 +496,53 @@ mod tests { use super::*; - fn ensure_memory_client() { - crate::openhuman::memory::ops::ensure_shared_memory_client(); + /// Pins `OPENHUMAN_WORKSPACE` to the shared memory workspace for a test's + /// duration, holding [`crate::openhuman::config::TEST_ENV_LOCK`] so sibling + /// tests that mutate the env var (e.g. `config::ops`, `update::ops`, + /// autonomy settings) cannot change it mid-run. + /// + /// `documents` tests are the only `memory::ops` tests that resolve the + /// workspace from the env var (`memory_init` → `current_workspace_dir` → + /// `Config::load_or_init`), so without this pin they race those tests and + /// `memory_init` intermittently fails — surfaced under `cargo-llvm-cov` + /// timing. Lock order is `GLOBAL_MEMORY_TEST_LOCK` → `TEST_ENV_LOCK` (the + /// test takes the memory lock first, then this guard takes the env lock); no + /// code path takes them in the opposite order, so there is no deadlock. + struct WorkspaceEnvGuard { + _env_lock: std::sync::MutexGuard<'static, ()>, + previous: Option, + } + + impl WorkspaceEnvGuard { + fn pin(workspace: &std::path::Path) -> Self { + let env_lock = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let previous = std::env::var_os("OPENHUMAN_WORKSPACE"); + std::env::set_var("OPENHUMAN_WORKSPACE", workspace); + Self { + _env_lock: env_lock, + previous, + } + } + } + + impl Drop for WorkspaceEnvGuard { + fn drop(&mut self) { + match self.previous.take() { + Some(value) => std::env::set_var("OPENHUMAN_WORKSPACE", value), + None => std::env::remove_var("OPENHUMAN_WORKSPACE"), + } + } + } + + /// Bind the shared memory client and pin `OPENHUMAN_WORKSPACE` to its + /// workspace for the test (see [`WorkspaceEnvGuard`]). Hold the returned + /// guard for the whole test: `let _env = ensure_memory_client();`. + #[must_use] + fn ensure_memory_client() -> WorkspaceEnvGuard { + let workspace = crate::openhuman::memory::ops::ensure_shared_memory_client(); + WorkspaceEnvGuard::pin(&workspace) } fn unique_namespace(prefix: &str) -> String { diff --git a/src/openhuman/memory/ops/test_support.rs b/src/openhuman/memory/ops/test_support.rs index ce543a93c5..f231a5d59e 100644 --- a/src/openhuman/memory/ops/test_support.rs +++ b/src/openhuman/memory/ops/test_support.rs @@ -10,11 +10,17 @@ use std::path::PathBuf; use std::sync::OnceLock; -/// Binds the process-global memory client to a single shared temp workspace. +/// Binds the process-global memory client to a single shared temp workspace and +/// returns that workspace path. /// /// Safe to call from multiple test threads concurrently — subsequent calls with /// the same workspace path return the existing client without rebinding. -pub(crate) fn ensure_shared_memory_client() { +/// +/// The returned path lets callers whose RPC path *also* resolves the workspace +/// from `OPENHUMAN_WORKSPACE` (notably `memory::ops::documents` via +/// `memory_init` → `current_workspace_dir`) pin the env var to this same path so +/// the env and the bound client agree. See `documents::tests`. +pub(crate) fn ensure_shared_memory_client() -> PathBuf { static WORKSPACE: OnceLock = OnceLock::new(); let workspace = WORKSPACE.get_or_init(|| { let tmp = tempfile::TempDir::new().expect("tempdir"); @@ -25,4 +31,5 @@ pub(crate) fn ensure_shared_memory_client() { }); crate::openhuman::memory::global::init(workspace.clone()) .expect("initialize shared test memory client"); + workspace.clone() }