Background

This is a follow-up to #339, which establishes a dependency-injection pattern for side-effecting code paths in this repository. Once that pattern lands, the same pattern should be extended to environment-variable lookups in crates/config so the unit tests for default_store_dir stop mutating shared process state. This issue tracks that extension.

Environment variable access in tests

Four unit tests in crates/config exercise default_store_dir by mutating the process environment:

• crates/config/src/lib.rs, in should_use_pnpm_home_env_var and should_use_xdg_data_home_env_var.
• crates/config/src/custom_deserializer.rs, in test_default_store_dir_with_pnpm_home_env and test_default_store_dir_with_xdg_env.

The process environment is a single global map shared by every test running inside the same test binary. Mutating it from one test is observable to every other test running concurrently. Three failure modes follow.

1. Race conditions. A parallel test reading PNPM_HOME while another test writes it can observe a stale or inconsistent value, producing flaky failures unrelated to the code under test. This is the same shared-state hazard that any global mutable resource exhibits. A shared file, a binary-wide cache, or a global counter would show the same pattern, and the hazard does not depend on the storage being unsafe. The unsoundness of std::env::set_var and std::env::remove_var in Rust 2024 is a separate, narrower problem. The C-level environ block has no internal synchronization, so a concurrent getenv against a setenv is memory-unsoundness on top of the logical race. Removing the unsafe calls without removing the shared mutation would close the soundness hole but leave the shared-state hazard untouched. Removing the shared mutation closes both at once.

2. Cascading failures. If a test sets PNPM_HOME and then panics or fails an assertion before its cleanup runs, the value persists into the process environment. Every subsequent test that reads PNPM_HOME then fails for a reason unrelated to the change under test, hiding the original failure behind a trail of false ones. One genuine bug surfaces as N noisy failures, which is the worst possible debugging signal.

3. Forced serialization. The mitigation in crates/config/src/test_env_guard.rs is a binary-wide Mutex that every env-mutating test must hold for its full duration, plus a snapshot-and-restore pass on Drop to undo the mutations. The four tests therefore cannot run in parallel with each other or with any other code that depends on the same variables, and parallelism within the npmrc test suite is artificially capped. The unsafe block at test_env_guard.rs:76 exists only because the restore step itself calls set_var and remove_var.

Each test site already carries a // TODO: change this to dependency injection comment, and test_env_guard.rs:17-19 states that the proper fix is to thread environment lookups through dependency injection.

The pattern documented in #339 is that fix. A new capability trait describes a single environment lookup. Its name should match whatever convention is chosen in #339 for the existing traits. Reasonable candidates include EnvVar, GetEnvVar, and env::Read if the trait-by-module form is adopted. The production provider implements the trait by delegating to std::env::var. Tests provide a fake that returns hardcoded values for the keys they care about. The function default_store_dir in crates/config/src/custom_deserializer.rs:54 becomes generic on the new capability and queries it instead of calling std::env::var directly. Its caller chain in crates/config/src/lib.rs propagates the same generic.

Once that lands, the four tests stop touching the process environment entirely. The test_env_guard module, its binary-wide Mutex, and the snapshot-and-restore machinery are all deleted. The five unsafe blocks fall out as a consequence rather than a goal: with no calls to set_var or remove_var left, there is no unsafe API to invoke. The cascading failures and forced serialization described above also disappear, because the tests no longer share any mutable state. Tests run in parallel without coordination. The new capability composes onto the same provider as the filesystem capabilities, which is exactly what principle 2 of the pattern in #339 prescribes.

Tasks

Add an environment-variable capability to the dependency-injection provider established in #339. Refactor default_store_dir in crates/config/src/custom_deserializer.rs to consume it instead of calling std::env::var directly, and propagate the new generic up its caller chain in crates/config/src/lib.rs. Rewrite the four affected unit tests in crates/config/src/lib.rs and crates/config/src/custom_deserializer.rs to use the new capability with test fakes. Delete the test_env_guard module once nothing depends on it. After this step, the unit tests for default_store_dir no longer mutate the process environment, the test_env_guard module and its binary-wide mutex are gone, and the repository contains no unsafe blocks as a consequence.