Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions src/openhuman/memory/ops/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::ffi::OsString>,
}

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 {
Expand Down
11 changes: 9 additions & 2 deletions src/openhuman/memory/ops/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf> = OnceLock::new();
let workspace = WORKSPACE.get_or_init(|| {
let tmp = tempfile::TempDir::new().expect("tempdir");
Expand All @@ -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()
}
Loading