test: isolate install-family tests from process-global env bleed#283
Merged
Conversation
The install path resolves profile root via TRACEDECAY_DATA_DIR when set, falling back to <home>/.tracedecay only when unset. Under in-process cargo test, a sibling test that pins the env made concurrent unpinned installs resolve to the shared path and race the memory_digest_targets atomic rename. Adds an AgentEnvLock RAII helper (locks PROCESS_ENV_LOCK + pins the env to the test's own home — the exact unset-fallback value, so no assertions change) and applies it to the install/uninstall tests that lacked isolation. Test-only; verified full workspace green + stress-stable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
home is &Path (dir.path()) at these sites, so pin(&home) is a needless borrow (clippy, blocking policy); PathBuf sites keep the borrow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Migrate the two call sites from the local install_env_lock() + pinned_profile_storage() pair to crate::common::AgentEnvLock::pin (the canonical bundle this PR introduced), and delete the now-duplicate local helpers and their now-unused imports. Same lock+pin behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract lock_recovering_poison/lock_global_db_env for poison-tolerant GLOBAL_DB_ENV_LOCK acquires, migrate Hermes update-plugin tests to AgentEnvLock, and drop the one-off hermes_env_guard helper.
Parallel lib unit tests that store MCP response handles under the process-global TRACEDECAY_DATA_DIR profile tree raced on profile-sharded response-handles/ directories, causing intermittent store failures.
Parallel lib tests used separate mutexes for TRACEDECAY_DATA_DIR pins, letting hook analytics and profile resolution tests race and flake.
Add lock_test_env alias, Claude install profile pinning, and route remaining config/daemon/handler env overrides through the shared lock with poison recovery.
Handle-store lib tests must serialize with TRACEDECAY_DATA_DIR mutators; otherwise profile resolution races and truncation handles fail to persist.
TraceDecay init/index tests shared TRACEDECAY_DATA_DIR with parallel env mutators; pin an isolated profile under the shared test lock.
This was referenced Jul 5, 2026
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a pre-existing test flake (install/uninstall tests race on the
memory_digest_targets.jsonatomic rename under in-processcargo test).Root cause
The install path resolves the profile root via
TRACEDECAY_DATA_DIRwhen set, falling back to<home>/.tracedecayonly when unset (profile_root_for_agent_home, skill_targets.rs:74). Undercargo test(one process, shared env), a sibling test that pins the env makes concurrent unpinned installs resolve to the shared path and race writing<shared>/agent_managed/memory_digest_targets.json— the atomic sibling-rename colliding with a peer's dir removal. Reproduced at ~36% in-process; nextest (CI harness) is immune because it runs each test in its own process, which is why CI never caught it.Fix (test-only, zero product change)
Adds an
AgentEnvLockRAII helper (locks the process-globalPROCESS_ENV_LOCKand pinsTRACEDECAY_DATA_DIRto the test's own home — the exact value the unset fallback produces, so no assertions change) and applies it to the install/uninstall tests that lacked isolation. Verified: full workspace green + heavy in-process stress stable.Note: the task also mentioned a git_watch debounce flake, but
git_watch.rsdoesn't exist on master (it's in PR #280's auto-sync work, already green there) — out of scope for this master-based fix.Recovery
Session
2c51d204-3565-4a10-833d-d8fbd51620c3· workflowwf_bae0c1a3-0b3· fact 41 (CI-gate lesson)🤖 Generated with Claude Code