fix(test): pin OPENHUMAN_WORKSPACE under TEST_ENV_LOCK in memory documents tests

[sanil-23]

Summary

• Fix the intermittent Rust Core Coverage (cargo-llvm-cov) failure on memory::ops::documents::tests::envelope_memory_handlers_report_counts_and_statuses (a 9637 passed; 1 failed flake that recurs under coverage timing).
• documents test helper now pins OPENHUMAN_WORKSPACE under TEST_ENV_LOCK for the test's duration, so sibling tests can't change the env var mid-run.
• Test-only change (2 files, both #[cfg(test)] / test-support).

Problem

memory::ops::documents tests resolve the workspace from the OPENHUMAN_WORKSPACE env var (memory_init → current_workspace_dir → Config::load_or_init), but they only held GLOBAL_MEMORY_TEST_LOCK — not TEST_ENV_LOCK, the lock that config::ops / update::ops / autonomy-settings tests take when they set_var/remove_var OPENHUMAN_WORKSPACE. Under cargo-llvm-cov's instrumented timing, a sibling test can change the env between a documents test's setup and its memory_init call, so memory_init resolves a stale/removed workspace and fails intermittently. documents is the only memory::ops module that reads the workspace from the env (kv_graph/sync/tool_memory don't), so they're unaffected.

Solution

• ensure_memory_client() now returns a WorkspaceEnvGuard that holds TEST_ENV_LOCK and pins OPENHUMAN_WORKSPACE to the shared memory workspace for the test's duration (restoring the prior value on drop). The two documents tests already capture it as let _env = ensure_memory_client();.
• ensure_shared_memory_client() returns the shared workspace path so the env var and the bound client agree.
• Lock order is GLOBAL_MEMORY_TEST_LOCK → TEST_ENV_LOCK (the test takes the memory lock first, then the guard takes the env lock); no code path takes them in the reverse order, so there is no deadlock. This mirrors the existing update::ops tests, which take TEST_ENV_LOCK for the same reason.

Submission Checklist

• Tests added or updated — this is a test-isolation fix; documents tests now serialize on TEST_ENV_LOCK and pass deterministically. Verified the full parallel cargo test --lib run with documents green and no deadlock.
• Diff coverage ≥ 80% — changed lines are entirely test / test-support code, exercised by the documents tests themselves.
• N/A Coverage matrix updated — test-infrastructure-only change; no feature behavior added/removed/renamed.
• N/A All affected feature IDs listed — no feature change.
• No new external network dependencies introduced — no deps changed.
• N/A Manual smoke checklist — test-only change, no release-cut surface.
• N/A Linked issue closed via Closes #NNN — no tracked issue; this is a CI flake fix found while landing feat(approval): consolidate on ApprovalGate + persistent "Always allow" list #2706 (its coverage job recurs on this test).

Impact

• CI only — makes Rust Core Coverage deterministic for this test; unblocks every PR that currently inherits this flake (e.g. feat(approval): consolidate on ApprovalGate + persistent "Always allow" list #2706 once rebased on top). No runtime/product code touched.

Related

• Closes: —
• Follow-up PR(s)/TODOs: there are separate, unrelated global-state flakes (composio::action_tool / composio::ops mock state) observed in the same suite — out of scope here; can be a follow-up.

---

AI Authored PR Metadata

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: fix/documents-env-race-test-isolation
• Commit SHA: d492b1ba

Validation Run

• pnpm --filter openhuman-app format:check — N/A (no app/ changes); cargo fmt --check clean
• N/A pnpm typecheck — no TypeScript changed
• Focused tests: cargo test --lib openhuman::memory::ops::documents::tests (2 pass); full cargo test --lib run with documents green, no deadlock
• Rust fmt/check: cargo fmt --check clean; cargo check --tests clean; cargo clippy --lib clean (changed files produce zero findings)
• N/A Tauri fmt/check: shell unchanged

Validation Blocked

• command: full cargo test --lib shows pre-existing unrelated flakes (composio::*) and cargo clippy --tests shows pre-existing approx_constant lints in rpc_log/pformat/vectors/store_tests
• error: unrelated to this change
• impact: none on this fix; noted as separate follow-ups

Behavior Changes

• Intended: none at runtime — test isolation only.
• User-visible effect: none.

Parity Contract

• Legacy behavior preserved: ensure_shared_memory_client() still binds the same shared workspace; callers that ignore the new return value are unaffected.
• Guard/fallback/dispatch parity checks: lock order documented (GLOBAL → ENV), no deadlock.

Duplicate / Superseded PR Handling

• Duplicate PR(s): none
• Canonical PR: this
• Resolution: n/a

Summary by CodeRabbit

• Tests
  • Improved test infrastructure to prevent race conditions in shared-memory tests by better managing environment variable handling across concurrent test execution.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 095a58ea-6608-4a45-b346-65e421651483

📥 Commits

Reviewing files that changed from the base of the PR and between 6d0657e and d492b1b.

📒 Files selected for processing (2)

• src/openhuman/memory/ops/documents.rs
• src/openhuman/memory/ops/test_support.rs

---

📝 Walkthrough

Walkthrough

This PR updates test infrastructure for shared-memory RPC tests to prevent race conditions on the OPENHUMAN_WORKSPACE environment variable. The workspace path is now returned and pinned via a new guard type held for test execution.

Changes

Shared-memory RPC test isolation

Layer / File(s)	Summary
Test helper workspace path return src/openhuman/memory/ops/test_support.rs	ensure_shared_memory_client() returns the shared temp workspace PathBuf after binding the global memory client, with expanded doc comments explaining callers should use this path to align OPENHUMAN_WORKSPACE with the bound client across tests.
Environment guard for shared-memory tests src/openhuman/memory/ops/documents.rs	Introduced WorkspaceEnvGuard to acquire TEST_ENV_LOCK, pin OPENHUMAN_WORKSPACE to the shared workspace path, and restore/remove it on drop. Replaced ensure_memory_client() helper with #[must_use] ensure_memory_client() -> WorkspaceEnvGuard so tests hold the guard for proper environment synchronization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#2649: Related test-isolation effort for GLOBAL_MEMORY_TEST_LOCK serialization in memory tests, coordinated with this PR's WorkspaceEnvGuard changes.

Suggested labels

memory, rust-core, bug

Suggested reviewers

• graycyrus

Poem

🐰 A guard to hold the env so tight,
No races in the test-lit night,
The workspace path is pinned with care,
Until the drop, it's anchored there!
✨ Test harmony achieved!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: introducing a mechanism to pin the OPENHUMAN_WORKSPACE environment variable under TEST_ENV_LOCK to prevent race conditions in memory documents tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}