refactor(pacquet/config): replace env mutations with DI pattern (#11718)

* refactor(pacquet/config): thread default_store_dir through the EnvVar DI seam

Replaces process-environment mutation in the `default_store_dir` tests with
the dependency-injection pattern established in pnpm/pacquet#339 and
consolidated for the in-tree pacquet subtree in #11708. Tracks
pnpm/pacquet#343 — the original issue still applied after pacquet was
merged into this repo because the env-mutating tests came with it.

The four affected unit tests (`test_default_store_dir_with_pnpm_home_env`
and `test_default_store_dir_with_xdg_env` in `defaults::tests`, plus
`should_use_pnpm_home_env_var` and `should_use_xdg_data_home_env_var` in
`lib::tests`) now drive each branch with per-test unit structs that
satisfy `EnvVar`. No `EnvGuard` snapshot, no `unsafe` block, no
process-environment write. The `home_dir` and `current_dir` closures call
`unreachable!` for branches the early `PNPM_HOME` / `XDG_DATA_HOME`
returns short-circuit before consulting them, documenting the
precondition the way the style guide's worked example does.

`default_store_dir` is now generic over `Sys: EnvVar` and takes the
`home_dir` and `current_dir` lookups as `FnOnce` closures, mirroring the
shape of `Config::current`. A thin args-less wrapper
`default_store_dir_host` wires the production `Host` provider together
with `home::home_dir` and `env::current_dir` so the SmartDefault
expression on `Config::store_dir` stays short. The `have_default_values`
wiring assertion now compares against
`default_store_dir::<Host>(home::home_dir, env::current_dir)` instead of
the old args-less helper.

`npmrc_auth::tests::ignores_non_auth_keys` no longer needs to hold the
EnvGuard global lock against the two `Config::new()` snapshots it
compares — nothing in the crate mutates `PNPM_HOME` or `XDG_DATA_HOME`
anymore, so the env-derived `store_dir` is observed identically by both
calls even under nextest's in-process parallelism.

`EnvGuard` itself stays in `pacquet-testing-utils` because the proxy
cascade and `NPM_CONFIG_WORKSPACE_DIR` tests in `lib::tests`, as well as
out-of-crate users in `git-fetcher` and `executor`, still rely on it.
Retiring it entirely is out of scope for this issue.

---
Written by an agent (Claude Code, claude-opus-4-7).

* fix(pacquet/config): add Debug bound on default_store_dir's Error parameter

The Windows branch of `default_store_dir` calls
`current_dir().expect("current directory is unavailable")`, which
requires `Error: Debug` on the `Result<PathBuf, Error>` returned by the
`CurrentDir` closure. The original `where` clause didn't declare that
bound, so the function compiled on Linux/macOS (where the Windows
branch is `#[cfg(windows)]`-gated out) but failed on
`Lint and Test (windows-latest)` once the cfg fired.

All current callers pass `env::current_dir` (Error = `io::Error`) or
pin `Error = std::io::Error` via turbofish on test fakes — both
already satisfy `Debug` — so this is a pure type-bound fix with no
behaviour or call-site change.

---
Written by an agent (Claude Code, claude-opus-4-7).

* docs(pacquet): add shared-process-state DI exception to the style guide

The "Dependency injection for tests" gating list in
`pacquet/CODE_STYLE_GUIDE.md` enumerated four reasons to reach past
real fixtures for the DI seam: filesystem error kinds, deterministic
time, external-service happy paths, and unreachable-by-design
preconditions. It did not cover the case this PR retired from
`default_store_dir`: tests that mutate a single per-process slot
(env vars, cwd, umask, signal handlers, …) and so race with every
other test in the same process.

The pre-existing `EnvGuard` workaround restored correctness only by
holding a binary-wide mutex around `unsafe { env::set_var(...) }`,
which forecloses parallelism inside the affected tests and leaves
`unsafe` in the test source. A capability-trait fake keeps the read
deterministic and the mutation contained to the test that needs it.

Adds that bullet to the style guide between "Deterministic time" and
"External-service happy paths," and updates rule 7 in
`pacquet/AGENTS.md` (and its `CLAUDE.md` / `GEMINI.md` symlink
targets) so the summary mirrors the longer list.

---
Written by an agent (Claude Code, claude-opus-4-7).

* docs(pacquet): drop unfounded set_current_dir example from DI guidance

The previous commit cited `set_current_dir` and "the umask" as
illustrative examples in both the style guide and the AGENTS rule 7
summary. Neither is actually used anywhere in the pacquet codebase
today — only `env::set_var` is. Speculative examples in a style
guide invite contributors to chase patterns that don't exist, so
narrow the wording to the one slot we have a concrete example for
while keeping the principle generalisable (the bullet still says
"any future analogue").

---
Written by an agent (Claude Code, claude-opus-4-7).

* docs(pacquet): revert example narrowing on the shared-process-state bullet

Reviewer feedback: keep the broader `set_current_dir` / umask /
signal-handler / global-allocator examples in the DI gating bullet
and rule 7 summary. They're not used in the codebase today, but the
analogues are useful as forward-looking illustrations so the next
contributor recognises the same shape when it comes up.

Restores the wording introduced in 96040d7; reverses the narrowing in
d930bed.

---
Written by an agent (Claude Code, claude-opus-4-7).

* docs(pacquet): document when library code may read process state

Adds a "Reading process state" section to CODE_STYLE_GUIDE.md, placed
right before "Dependency injection for tests" so the reading order
captures the two questions in dependency order: first ask whether you
should be reaching for `env::var` / `env::current_dir` at all, then
ask how to inject it if the answer is yes.

The section pins:

1. Library code should rarely call `std::env::var`, `std::env::var_os`,
   or `std::env::current_dir` directly. The default fix is a parameter
   (`&Path`, `&str`, an `Option<String>`, or a `FnOnce` closure) so the
   caller decides which value to read; `Config::current` and
   `workspace::find_workspace_dir_from_env_with` already follow this.
2. When the read genuinely needs to happen, route through the DI seam
   (`Sys: EnvVar`, `Sys::var(name)`).
3. The narrow legitimate direct-call case is computing a `Config`
   default that has no caller — `default_store_dir`,
   `default_modules_dir`, `default_virtual_store_dir`. This is the
   "begrudging" use that pnpm/pacquet#343 + pnpm/pnpm#11718 finally
   threaded through the DI seam.
4. Other accepted boundary reads are program-entry knobs
   (`RAYON_NUM_THREADS`, `TRACE`) and the lifecycle-script env snapshot
   in `crates/executor` / `crates/git-fetcher` that forwards the parent
   env to spawned children verbatim.

The section belongs in CODE_STYLE_GUIDE.md (code-level convention),
not CONTRIBUTING.md (PR workflow) or AGENTS.md (agent-specific
operating rules); rule 7 in AGENTS.md already cross-links the style
guide, so no change there.

---
Written by an agent (Claude Code, claude-opus-4-7).

* refactor(pacquet/config): inline default_store_dir's SmartDefault wiring

Reviewer pointed out that the args-less `default_store_dir_host`
wrapper could be inlined into the `#[default(_code = ...)]`
expression on `Config::store_dir` — the SmartDefault macro accepts
the turbofish form `default_store_dir::<Host, _, _, _>(home::home_dir,
env::current_dir)` verbatim, and once the wrapper is gone there's
nothing for the indirection to earn.

Dropping the wrapper also removes a small motivation for the
`Host` import inside `defaults.rs` — adjust the doc comment on
`default_store_dir` to cite `crate::Host` by full path instead.

The `#[cfg(windows)] Error: std::fmt::Debug` bound that the same
reviewer asked about is not feasible: attributes on where-clause
predicates are still unstable (rust-lang/rust#115590), so the bound
stays unconditional with a comment explaining why.

---
Written by an agent (Claude Code, claude-opus-4-7).

* docs(pacquet/config): purge stale default_store_dir_host references

Commit 49c690c inlined the SmartDefault expression for
`Config::store_dir` and dropped the `default_store_dir_host`
wrapper, but three comments still pointed at the wrapper or at the
old single-type-parameter signature of `default_store_dir`.
CodeRabbit and Copilot flagged all three on review:

- `crates/config/src/lib.rs`, `have_default_values`: the comment said
  the SmartDefault resolved "via the thin `default_store_dir_host`
  wrapper". Now reflects the direct
  `default_store_dir::<Host, _, _, _>(home::home_dir,
  env::current_dir)` call.
- `crates/config/src/npmrc_auth/tests.rs`, `ignores_non_auth_keys`:
  the comment cited `default_store_dir::<Host>` with no closure
  parameters. Now spells out the four-parameter turbofish that
  matches the current signature.
- `CODE_STYLE_GUIDE.md`, "Reading process state": the paragraph
  recommended an args-less `default_store_dir_host` wrapper as the
  pattern to follow. Now recommends inlining the production
  composition at the `SmartDefault` site directly.

Copilot also queried whether `pnpm/pnpm#11718` was the right number
in the DI section's `EnvGuard` retirement citation — confirmed it
is. #11708 introduced the DI seam consolidation; this PR is the
one that retires the env-mutation pattern from `default_store_dir`.

---
Written by an agent (Claude Code, claude-opus-4-7).

---------

Co-authored-by: Claude <noreply@anthropic.com>

pacquet/AGENTS.md

@@ -49,7 +49,9 @@ Before writing code for a feature, bug fix, or behavior change:
    binary. Use the DI seam — a capability trait on the `Host`
    provider, threaded as `Sys: <Bounds>` — only for branches a real
    fixture can't reach portably: filesystem error kinds
-   (`PermissionDenied`, `ENOSPC`, …), deterministic time, or the
+   (`PermissionDenied`, `ENOSPC`, …), deterministic time, shared
+   process-global state a test would otherwise mutate
+   (`env::set_var`, `set_current_dir`, the umask, …), or the
    external-service happy paths in features like `pnpm login` (2FA)
    and `pnpm publish` (OIDC / provenance) when those land. See
    [Dependency injection for tests](./CODE_STYLE_GUIDE.md#dependency-injection-for-tests)

pacquet/CODE_STYLE_GUIDE.md

@@ -504,6 +504,15 @@ Do not flatten the tests into a sibling file such as `src/foo_tests.rs`, and do
 
 Prefer `Arc::clone(&x)` / `Rc::clone(&x)` over `x.clone()` for reference-counted types. The qualified form makes the O(1) refcount bump visible at the call site and fails to compile if a refactor changes the binding's type to something whose `Clone` is an arbitrarily expensive deep copy. Enforced by [`perfectionist::arc_rc_clone`](https://github.com/KSXGitHub/perfectionist/blob/0.0.0-rc.15/rules/arc_rc_clone.md).
 
+### Reading process state
+
+Library code should rarely call `std::env::var`, `std::env::var_os`, or `std::env::current_dir` directly. Process state belongs to the running process, not to the operation a function performs, so reaching for it couples the logic to whichever shell invoked pacquet and hides the dependency from the caller. Prefer one of:
+
+- **Take the value as a parameter.** A function that needs a path declares `path: &Path`; one that needs an env-var value takes it as a `&str` argument, an `Option<String>`, or a `FnOnce` closure that produces it. `Config::current` threads the cwd through a caller-supplied `FnOnce() -> Result<PathBuf, _>` closure rather than calling `env::current_dir` itself, and `crates/workspace`'s `find_workspace_dir_from_env_with` does the same for an env-var lookup — both let tests drive the read without touching the process environment.
+- **Thread the read through the DI seam.** Declare `Sys: EnvVar` and call `Sys::var(name)`. See [Dependency injection for tests](#dependency-injection-for-tests) for the convention.
+
+The narrow case where a direct call is acceptable is computing the default of a `Config` field that has no caller to take the value from — `default_store_dir`, `default_modules_dir`, and `default_virtual_store_dir` are the canonical examples; this is the "begrudging" use behind pnpm/pacquet#343. Even there, route the lookup through the DI seam: write the helper as `default_store_dir<Sys: EnvVar>(home_dir, current_dir)` and inline the production composition at the `SmartDefault` site (`#[default(_code = "default_store_dir::<Host, _, _, _>(home::home_dir, env::current_dir)")]`), so the fake-`Sys` test path and the production `Host` path resolve through the same generic. The other accepted boundary reads are program-entry knobs (`RAYON_NUM_THREADS` in `crates/cli`, `TRACE` in `crates/diagnostics`) and the lifecycle-script env snapshot in `crates/executor` and `crates/git-fetcher`, which has to forward the parent environment to spawned children verbatim. If new library code seems to need `env::var` / `env::current_dir` for any other reason, the answer is almost always a `&Path` (or equivalent) parameter, not a process-state read.
+
 ### Dependency injection for tests
 
 Side-effecting code — filesystem access, environment variables, network calls, time, process state — has two testing routes. **The default route is a real fixture:** a `tempfile::TempDir` for filesystem work, the mocked registry (`just registry-mock`) for HTTP, an integration test that spawns the actual pacquet binary in a scratch directory for end-to-end flows. Real fixtures keep tests close to what users see and scale with the codebase without per-call-site plumbing; they are the right tool everywhere except the cases enumerated below.
@@ -512,6 +521,7 @@ The dependency-injection seam described below is the **narrow second route**. Re
 
 - **Filesystem error branches the host OS won't reproduce portably.** `PermissionDenied`, `ENOSPC`, a directory that disappears mid-walk, a chmod that fails after the file exists — provoking these on real disks is platform-specific, racy, or both. A fake that returns the exact `io::ErrorKind` is the only portable way to drive the branch.
 - **Deterministic time.** Asserting that `prunedAt` equals a specific HTTP-date (RFC 7231 IMF-fixdate, what `httpdate::fmt_http_date` emits), or that a throttled emitter fires on the second sample, needs the clock to be a known value. The real `SystemTime::now` makes those assertions flaky.
+- **Shared process-global state that tests would otherwise mutate.** When a branch depends on a single per-process slot — environment variables, the current working directory, the umask, signal handlers, the global allocator — the only way to exercise it without DI is to write to that slot, and the write is observed by every other test in the same process. A serialisation lock (the `EnvGuard` pattern that pnpm/pacquet#343 + pnpm/pnpm#11718 retired from `default_store_dir`) restores correctness only by forcing the affected tests to run single-threaded, and leaves an `unsafe { env::set_var(...) }` block at the call site. A capability-trait fake keeps the read deterministic and the mutation contained to the test that needs it, so nextest's in-process parallelism stays useful and `unsafe` stays out of test code. The reverse follows too: a real-fixture happy path should not be promoted to DI just because it crosses a process-global slot — only the *mutation* side of the slot is the problem; reading whatever the shell already has is fine.
 - **External-service happy paths that can't be staged in CI.** Upstream pnpm has features whose *normal* flow depends on real external systems — `pnpm login`'s 2FA prompt round-trip, the OIDC token exchange and provenance attestation in `pnpm publish`, and similar — where the happy path itself is what needs faking, not just the error path. When pacquet ports those features, DI is the right tool for their tests too. (As of writing, these are not yet ported; this exception is documented so the convention is in place when they land.)
 - **Unreachable-by-design preconditions.** When a function declares a capability bound but a specific test exercises a branch that never reaches that capability, the fake satisfies the bound with `unreachable!` and documents the precondition. See the worked example below.
 

pacquet/crates/config/src/defaults.rs

@@ -1,3 +1,4 @@
+use crate::api::EnvVar;
 use pacquet_store_dir::StoreDir;
 use std::{env, path::PathBuf};
 
@@ -52,30 +53,62 @@ fn default_store_dir_windows(home_dir: &Path, current_dir: &Path) -> PathBuf {
     PathBuf::from(format!("{current_drive}:\\.pnpm-store"))
 }
 
-/// If the $PNPM_HOME env variable is set, then $PNPM_HOME/store
-/// If the $XDG_DATA_HOME env variable is set, then $XDG_DATA_HOME/pnpm/store
-/// On Windows: ~/AppData/Local/pnpm/store
-/// On macOS: ~/Library/pnpm/store
-/// On Linux: ~/.local/share/pnpm/store
-pub fn default_store_dir() -> StoreDir {
+/// If the `$PNPM_HOME` env variable is set, then `$PNPM_HOME/store`.
+/// If the `$XDG_DATA_HOME` env variable is set, then `$XDG_DATA_HOME/pnpm/store`.
+/// On Windows: `~/AppData/Local/pnpm/store` (same drive) or `<drive>:\.pnpm-store` (different drive).
+/// On macOS: `~/Library/pnpm/store`.
+/// On Linux: `~/.local/share/pnpm/store`.
+///
+/// Generic over [`EnvVar`] (and over closures producing the home
+/// and current directories) so unit tests can drive every branch
+/// — `PNPM_HOME` set, `XDG_DATA_HOME` set, neither set — without
+/// mutating the process environment. Mirrors the seam established
+/// in pnpm/pacquet#339 and consolidated in
+/// [pnpm/pnpm#11708](https://github.com/pnpm/pnpm/pull/11708).
+/// Production callers pass [`crate::Host`] for `Sys` together
+/// with `home::home_dir` and `env::current_dir` for the
+/// closures — see the `SmartDefault` expression on
+/// [`crate::Config::store_dir`].
+pub fn default_store_dir<Sys, HomeDir, CurrentDir, Error>(
+    home_dir: HomeDir,
+    current_dir: CurrentDir,
+) -> StoreDir
+where
+    Sys: EnvVar,
+    HomeDir: FnOnce() -> Option<PathBuf>,
+    CurrentDir: FnOnce() -> Result<PathBuf, Error>,
+    // `.expect(...)` on the `current_dir` result inside the Windows
+    // branch needs `Error: Debug`. Production callers pass
+    // `env::current_dir` (Error = `io::Error`, which has `Debug`);
+    // test fakes that don't drive the Windows branch pin
+    // `Error = std::io::Error` via turbofish for the same reason.
+    // The bound would ideally be gated `#[cfg(windows)]` so non-Windows
+    // callers stayed free of the `Debug` requirement, but
+    // `#[cfg(...)]` on where-clause predicates is still unstable
+    // (rust-lang/rust#115590) — keep the bound unconditional until
+    // that stabilises.
+    Error: std::fmt::Debug,
+{
     // TODO: If env variables start with ~, make sure to resolve it into home_dir.
-    if let Ok(pnpm_home) = env::var("PNPM_HOME") {
+    if let Some(pnpm_home) = Sys::var("PNPM_HOME") {
         return PathBuf::from(pnpm_home).join("store").into();
     }
 
-    if let Ok(xdg_data_home) = env::var("XDG_DATA_HOME") {
+    if let Some(xdg_data_home) = Sys::var("XDG_DATA_HOME") {
         return PathBuf::from(xdg_data_home).join("pnpm").join("store").into();
     }
 
     // Using ~ (tilde) for defining home path is not supported in Rust and
     // needs to be resolved into an absolute path.
-    let home_dir = home::home_dir().expect("Home directory is not available");
+    let home_dir = home_dir().expect("Home directory is not available");
 
     #[cfg(windows)]
     if cfg!(windows) {
-        let current_dir = env::current_dir().expect("current directory is unavailable");
+        let current_dir = current_dir().expect("current directory is unavailable");
         return default_store_dir_windows(&home_dir, &current_dir).into();
     }
+    #[cfg(not(windows))]
+    let _ = current_dir;
 
     // https://doc.rust-lang.org/std/env/consts/constant.OS.html
     match env::consts::OS {

pacquet/crates/config/src/defaults/tests.rs

@@ -2,10 +2,9 @@ use super::{
     default_child_concurrency_with_parallelism, default_store_dir, default_unsafe_perm,
     is_unsafe_perm_posix, resolve_child_concurrency, resolve_child_concurrency_with_parallelism,
 };
+use crate::api::EnvVar;
 use pacquet_store_dir::StoreDir;
-use pacquet_testing_utils::env_guard::EnvGuard;
 use pretty_assertions::assert_eq;
-use std::env;
 
 #[cfg(windows)]
 use super::{default_store_dir_windows, get_drive_letter};
@@ -16,39 +15,90 @@ fn display_store_dir(store_dir: &StoreDir) -> String {
     store_dir.display().to_string().replace('\\', "/")
 }
 
+/// Empty env: every lookup returns `None`. Used by the
+/// neither-`PNPM_HOME`-nor-`XDG_DATA_HOME` paths that fall through
+/// to the home/cwd-anchored OS defaults.
+struct NoEnv;
+impl EnvVar for NoEnv {
+    fn var(_: &str) -> Option<String> {
+        None
+    }
+}
+
+/// `default_store_dir`'s `PNPM_HOME` branch wins over everything
+/// else. Exercised through the dependency-injection seam from
+/// pnpm/pacquet#339 + pnpm/pnpm#11708 with a per-test unit struct
+/// that satisfies [`EnvVar`] — no `std::env::set_var`, no
+/// `EnvGuard` lock, no `unsafe` block. Tracks pnpm/pacquet#343.
+///
+/// The `home_dir` and `current_dir` closures call `unreachable!`
+/// because the early `PNPM_HOME` return short-circuits before
+/// either is consumed. Matches the worked example in
+/// `pacquet/CODE_STYLE_GUIDE.md` (Dependency injection for tests):
+/// satisfy the bound, document the precondition.
 #[test]
 fn test_default_store_dir_with_pnpm_home_env() {
-    let _g = EnvGuard::snapshot(["PNPM_HOME"]);
-    // SAFETY: EnvGuard above serializes the test against other env-mutating
-    // tests in this process; no other thread reads these vars concurrently.
-    unsafe {
-        env::set_var("PNPM_HOME", "/tmp/pnpm-home"); // TODO: change this to dependency injection
+    struct EnvWithPnpmHome;
+    impl EnvVar for EnvWithPnpmHome {
+        fn var(name: &str) -> Option<String> {
+            (name == "PNPM_HOME").then(|| "/tmp/pnpm-home".to_owned())
+        }
     }
-    let store_dir = default_store_dir();
+    let store_dir = default_store_dir::<EnvWithPnpmHome, _, _, std::io::Error>(
+        || unreachable!("home_dir must not be called when PNPM_HOME is set"),
+        || unreachable!("current_dir must not be called when PNPM_HOME is set"),
+    );
     assert_eq!(display_store_dir(&store_dir), "/tmp/pnpm-home/store");
 }
 
+/// `default_store_dir`'s `XDG_DATA_HOME` branch fires only when
+/// `PNPM_HOME` is unset. The fake `Sys` here returns a value for
+/// `XDG_DATA_HOME` and `None` for `PNPM_HOME`, so the lookup falls
+/// through to the second branch deterministically — no need to
+/// snapshot-and-restore real process env state to neutralise a
+/// developer's shell that has `PNPM_HOME` set. Tracks
+/// pnpm/pacquet#343.
 #[test]
 fn test_default_store_dir_with_xdg_env() {
-    // `default_store_dir` checks `PNPM_HOME` before `XDG_DATA_HOME`,
-    // so a developer running the test suite with pnpm in their
-    // environment (very common) otherwise sees the `PNPM_HOME`
-    // branch win and the assertion fail. Snapshot-and-restore both
-    // env vars so the test is self-contained even under nextest's
-    // in-process parallelism. Proper fix is dependency injection —
-    // see the TODO — but this is enough for the failure mode this
-    // PR is fixing.
-    let _g = EnvGuard::snapshot(["PNPM_HOME", "XDG_DATA_HOME"]);
-    // SAFETY: EnvGuard above serializes the test against other env-mutating
-    // tests in this process; no other thread reads these vars concurrently.
-    unsafe {
-        env::remove_var("PNPM_HOME");
-        env::set_var("XDG_DATA_HOME", "/tmp/xdg_data_home");
+    struct EnvWithXdgDataHome;
+    impl EnvVar for EnvWithXdgDataHome {
+        fn var(name: &str) -> Option<String> {
+            (name == "XDG_DATA_HOME").then(|| "/tmp/xdg_data_home".to_owned())
+        }
     }
-    let store_dir = default_store_dir();
+    let store_dir = default_store_dir::<EnvWithXdgDataHome, _, _, std::io::Error>(
+        || unreachable!("home_dir must not be called when XDG_DATA_HOME is set"),
+        || unreachable!("current_dir must not be called when XDG_DATA_HOME is set"),
+    );
     assert_eq!(display_store_dir(&store_dir), "/tmp/xdg_data_home/pnpm/store");
 }
 
+/// When neither `PNPM_HOME` nor `XDG_DATA_HOME` is set, the
+/// non-Windows fall-through uses `home_dir()` plus the
+/// `env::consts::OS` switch — `~/.local/share/pnpm/store` on
+/// Linux, `~/Library/pnpm/store` on macOS. Drive the home-dir
+/// closure with a fixed path so the assertion is deterministic on
+/// any host. The `current_dir` closure stays `unreachable!`
+/// because the non-Windows fall-through never consults it. Mirrors
+/// the third branch of pnpm's
+/// [`storePath`](https://github.com/pnpm/pnpm/blob/29a42efc3b/store/path/src/index.ts).
+#[cfg(not(windows))]
+#[test]
+fn test_default_store_dir_falls_back_to_home_dir() {
+    use std::path::PathBuf;
+
+    let store_dir = default_store_dir::<NoEnv, _, _, std::io::Error>(
+        || Some(PathBuf::from("/home/test-user")),
+        || unreachable!("current_dir must not be called on non-Windows fall-through"),
+    );
+    let expected = match std::env::consts::OS {
+        "linux" => "/home/test-user/.local/share/pnpm/store",
+        "macos" => "/home/test-user/Library/pnpm/store",
+        other => panic!("unexpected target OS in test: {other}"),
+    };
+    assert_eq!(display_store_dir(&store_dir), expected);
+}
+
 /// Port of upstream
 /// [`'getDefaultWorkspaceConcurrency: cpu num < 4'`](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/config/reader/src/concurrency.test.ts#L25-L28).
 /// On a 1-core host, the default caps at 1 (not 4).

pacquet/crates/config/src/lib.rs

@@ -15,7 +15,7 @@ use serde::Deserialize;
 use smart_default::SmartDefault;
 use std::{
     collections::{BTreeMap, BTreeSet, HashMap},
-    fs,
+    env, fs,
     path::PathBuf,
 };
 
@@ -184,7 +184,7 @@ pub struct Config {
     pub shamefully_hoist: bool,
 
     /// The location where all the packages are saved on the disk.
-    #[default(_code = "default_store_dir()")]
+    #[default(_code = "default_store_dir::<Host, _, _, _>(home::home_dir, env::current_dir)")]
     pub store_dir: StoreDir,
 
     /// The directory in which dependencies will be installed (instead of node_modules).
@@ -956,7 +956,7 @@ mod tests {
     use pretty_assertions::assert_eq;
     use tempfile::tempdir;
 
-    use super::{Config, Host, NodeLinker, PackageImportMethod, fs};
+    use super::{Config, EnvVar, Host, NodeLinker, PackageImportMethod, fs};
     use crate::defaults::default_store_dir;
     use pacquet_store_dir::StoreDir;
     use pacquet_testing_utils::env_guard::EnvGuard;
@@ -974,7 +974,20 @@ mod tests {
         assert!(value.prefer_frozen_lockfile);
         assert!(value.symlink);
         assert!(value.hoist);
-        assert_eq!(value.store_dir, default_store_dir());
+        // The SmartDefault expression for `store_dir` resolves to
+        // `default_store_dir::<Host, _, _, _>(home::home_dir, env::current_dir)`
+        // directly (no wrapper), so calling the generic helper here
+        // with the same `Host` capability and the same OS closures
+        // must produce the same value — even on a developer machine
+        // with `PNPM_HOME` / `XDG_DATA_HOME` set. This is the wiring
+        // assertion that proves the SmartDefault field still goes
+        // through the production capability; the per-branch behaviour
+        // of `default_store_dir` is exercised with fake-`Sys` structs
+        // in `defaults::tests`.
+        assert_eq!(
+            value.store_dir,
+            default_store_dir::<Host, _, _, _>(home::home_dir, env::current_dir),
+        );
         assert_eq!(value.registry, "https://registry.npmjs.org/");
     }
 
@@ -991,36 +1004,52 @@ mod tests {
         assert_eq!(value.fetch_retry_maxtimeout, 60_000);
     }
 
+    /// `default_store_dir`'s `PNPM_HOME` branch, exercised through the
+    /// generic capability seam — no process-environment mutation, no
+    /// `EnvGuard` lock, no `unsafe` block. The earlier shape of this
+    /// test set `PNPM_HOME` via `std::env::set_var` and called
+    /// `Config::new()`; with the DI seam from pnpm/pacquet#339 +
+    /// pnpm/pnpm#11708 the same precedence is checked by passing a
+    /// per-test unit struct that satisfies [`EnvVar`].
+    ///
+    /// The `home_dir` and `current_dir` closures both call
+    /// `unreachable!` because `default_store_dir` short-circuits on
+    /// `PNPM_HOME` before consulting either — the panic-on-call
+    /// documents that precondition. Tracks pnpm/pacquet#343.
     #[test]
     pub fn should_use_pnpm_home_env_var() {
-        let _g = EnvGuard::snapshot(["PNPM_HOME"]);
-        // SAFETY: EnvGuard above serializes the test against other env-mutating
-        // tests in this process; no other thread reads these vars concurrently.
-        unsafe {
-            env::set_var("PNPM_HOME", "/hello"); // TODO: change this to dependency injection
+        struct EnvWithPnpmHome;
+        impl EnvVar for EnvWithPnpmHome {
+            fn var(name: &str) -> Option<String> {
+                (name == "PNPM_HOME").then(|| "/hello".to_owned())
+            }
         }
-        let value = Config::new();
-        assert_eq!(display_store_dir(&value.store_dir), "/hello/store");
+        let store_dir = default_store_dir::<EnvWithPnpmHome, _, _, std::io::Error>(
+            || unreachable!("home_dir must not be called when PNPM_HOME is set"),
+            || unreachable!("current_dir must not be called when PNPM_HOME is set"),
+        );
+        assert_eq!(display_store_dir(&store_dir), "/hello/store");
     }
 
+    /// Companion to [`should_use_pnpm_home_env_var`]: when
+    /// `PNPM_HOME` is unset, `default_store_dir` falls through to
+    /// `XDG_DATA_HOME`. Exercised through the DI seam with a fake
+    /// `Sys` that only returns a value for `XDG_DATA_HOME`. No
+    /// process-environment mutation, no `EnvGuard`, no `unsafe`.
+    /// Tracks pnpm/pacquet#343.
     #[test]
     pub fn should_use_xdg_data_home_env_var() {
-        // Clear `PNPM_HOME` first — `default_store_dir` checks it
-        // before `XDG_DATA_HOME`, so running the test suite with pnpm
-        // installed (common) would otherwise hit the `PNPM_HOME`
-        // branch and fail the assertion. Snapshot both so the test
-        // cleans up after itself even when parallel peers observe the
-        // temporarily-unset state. See the companion fix in
-        // `defaults::tests::test_default_store_dir_with_xdg_env`.
-        let _g = EnvGuard::snapshot(["PNPM_HOME", "XDG_DATA_HOME"]);
-        // SAFETY: EnvGuard above serializes the test against other env-mutating
-        // tests in this process; no other thread reads these vars concurrently.
-        unsafe {
-            env::remove_var("PNPM_HOME"); // TODO: change this to dependency injection
-            env::set_var("XDG_DATA_HOME", "/hello");
+        struct EnvWithXdgDataHome;
+        impl EnvVar for EnvWithXdgDataHome {
+            fn var(name: &str) -> Option<String> {
+                (name == "XDG_DATA_HOME").then(|| "/hello".to_owned())
+            }
         }
-        let value = Config::new();
-        assert_eq!(display_store_dir(&value.store_dir), "/hello/pnpm/store");
+        let store_dir = default_store_dir::<EnvWithXdgDataHome, _, _, std::io::Error>(
+            || unreachable!("home_dir must not be called when XDG_DATA_HOME is set"),
+            || unreachable!("current_dir must not be called when XDG_DATA_HOME is set"),
+        );
+        assert_eq!(display_store_dir(&store_dir), "/hello/pnpm/store");
     }
 
     #[test]

pacquet/crates/config/src/npmrc_auth/tests.rs

@@ -56,13 +56,18 @@ fn ignores_non_auth_keys() {
     // from pnpm-workspace.yaml now. Writing them to .npmrc should be a
     // no-op.
     //
-    // `Config::new()` reads `PNPM_HOME` / `XDG_DATA_HOME` to compute
-    // `store_dir`, and the env-mutating tests in `defaults`
-    // toggle those vars under `EnvGuard`. Hold the same lock so a
-    // parallel test can't change the env between the two `Config::new()`
-    // snapshots compared below. Proper fix is dependency injection.
-    // See the TODO on `default_store_dir`.
-    let _g = pacquet_testing_utils::env_guard::EnvGuard::snapshot(["PNPM_HOME", "XDG_DATA_HOME"]);
+    // `Config::new()` reads `PNPM_HOME` / `XDG_DATA_HOME` via the
+    // SmartDefault expression on `Config::store_dir` —
+    // `default_store_dir::<Host, _, _, _>(home::home_dir,
+    // env::current_dir)` — to compute `store_dir`. Both values come
+    // from the real process environment, but no other test in this
+    // crate mutates them anymore — the per-branch tests in
+    // `defaults::tests` and `lib::tests` drive `default_store_dir`
+    // through the dependency-injection seam (pnpm/pacquet#339,
+    // pnpm/pnpm#11708, pnpm/pacquet#343) with fake `Sys` providers,
+    // so the two `Config::new()` snapshots compared below observe
+    // the same env-derived `store_dir` even under nextest's
+    // in-process parallelism without an `EnvGuard` lock.
     let ini = "
 store-dir=/should/not/apply
 lockfile=false