fix(pacquet/config): pick the store on the project's volume#11804
Conversation
Port pnpm's `getStorePath` / `storePathRelativeToHome` cross-volume detection to pacquet's `default_store_dir`. When no `storeDir` is explicitly set (global config.yaml, pnpm-workspace.yaml, or `PNPM_CONFIG_STORE_DIR`), `Config::current` now probes whether the project root can be hardlinked into the user's pnpm home dir. If not, it walks from the filesystem root toward the project to find the volume mount point and falls back to `<mountpoint>/.pnpm-store` — matching pnpm's behaviour at https://github.com/pnpm/pnpm/blob/29a42efc3b/store/path/src/index.ts#L14-L78. Before this fix, a workspace on a separate volume (e.g. `/Volumes/src/` on macOS) installed into the home-volume store (`~/Library/pnpm/store`). When the home volume is case-insensitive and the workspace volume is case-sensitive, typescript-eslint's path-cache canonicalises against the home store and then can't locate the same files in TypeScript's case-sensitive program loaded from the workspace, so `eslint --fix` fails with "TSConfig does not include this file" on every project file. The hardlink probe goes through a new `LinkProbe` capability in `pacquet-config::api`, threaded into `Config::current`'s `Sys` bound so tests can pin the linkability answer without touching disk. The production `Host` impl performs real link attempts via `store_path::host_can_link_between_dirs`. Test fakes get an inert `LinkProbe` impl via the `inert_link_probe!` macro added to the `tests` module — every probe returns `false`, the algorithm walks without finding a mount, and the SmartDefault home store survives unchanged, so existing cascade assertions stay valid.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (1)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis PR adds hardlink-based store directory resolution to the config crate: a new ChangesHardlink-based store directory resolution
Sequence Diagram(s)sequenceDiagram
participant Config as Config::current
participant LinkProbe as Sys: LinkProbe
participant ResolveStore as store_path::resolve_store_dir
participant RootWalk as root_link_target
Config->>LinkProbe: can_link_between_dirs(project, home)?
alt project and home linkable
Config->>Config: return home_default
else cross-volume or same-mount
Config->>ResolveStore: resolve_store_dir(home, pnpm_home, project)
ResolveStore->>RootWalk: find first linkable ancestor
RootWalk->>LinkProbe: can_link_between_dirs(ancestor, project)?
RootWalk-->>ResolveStore: linkable_ancestor or None
ResolveStore->>ResolveStore: compute mountpoint/.pnpm-store
ResolveStore-->>Config: resolved store path or home_default
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11804 +/- ##
==========================================
+ Coverage 87.39% 87.47% +0.07%
==========================================
Files 200 202 +2
Lines 23514 23698 +184
==========================================
+ Hits 20550 20729 +179
- Misses 2964 2969 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Drop the redundant explicit `[LinkProbe][crate::api::LinkProbe]` link target and fully qualify `[StoreDir]` as `[pacquet_store_dir::StoreDir]` so the `Doc` job passes under `-D rustdoc::broken-intra-doc-links` and `-D rustdoc::redundant-explicit-links`. - Gate the `PrefixProbe` fake, its `ALLOW_PREFIXES` static, and the related imports with `#[cfg(unix)]`. Every consumer is already `#[cfg(unix)]`, so on Windows the items were unreferenced and the `Lint and Test (windows-latest)` job's clippy run failed under `-D dead-code`.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pacquet/crates/config/src/store_path/tests.rs`:
- Around line 97-117: The tests use global mutable ALLOW_PREFIXES via
PrefixProbe::set_allow which causes cross-talk when run in parallel; wrap each
test scenario that calls PrefixProbe::set_allow and resolve_store_dir in a
serialized helper (e.g., a with_allow(prefixes, || { ... }) that sets
ALLOW_PREFIXES, runs the closure, then clears/restores it) and replace direct
calls to PrefixProbe::set_allow in the PrefixProbe-linked tests (the cases
around resolve_store_dir and the instances referenced by set_allow /
can_link_between_dirs) with that with_allow helper so each scenario is isolated
and ALLOW_PREFIXES is reset after the closure finishes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 67d95ca9-6a19-49a4-8e04-b153c1656425
📒 Files selected for processing (5)
pacquet/crates/config/src/api.rspacquet/crates/config/src/defaults.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/store_path.rspacquet/crates/config/src/store_path/tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/config/src/defaults.rspacquet/crates/config/src/api.rspacquet/crates/config/src/store_path.rspacquet/crates/config/src/store_path/tests.rspacquet/crates/config/src/lib.rs
🧠 Learnings (2)
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.
Applied to files:
pacquet/crates/config/src/defaults.rspacquet/crates/config/src/api.rspacquet/crates/config/src/store_path.rspacquet/crates/config/src/store_path/tests.rspacquet/crates/config/src/lib.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.
Applied to files:
pacquet/crates/config/src/defaults.rspacquet/crates/config/src/api.rspacquet/crates/config/src/store_path.rspacquet/crates/config/src/store_path/tests.rspacquet/crates/config/src/lib.rs
🔇 Additional comments (13)
pacquet/crates/config/src/api.rs (1)
18-22: LGTM!Also applies to: 82-108, 145-150
pacquet/crates/config/src/store_path.rs (4)
1-50: LGTM!
51-101: LGTM!
103-160: LGTM!
162-201: LGTM!pacquet/crates/config/src/lib.rs (7)
7-11: LGTM!
970-970: LGTM!
1039-1059: LGTM!
1152-1152: LGTM!Also applies to: 1176-1176
1188-1212: LGTM!
1249-1277: LGTM!
1334-1334: LGTM!Also applies to: 1522-1522, 1581-1581, 1846-1846, 1890-1890, 1950-1950, 2002-2002, 2064-2064, 2120-2120, 2161-2161, 2193-2193, 2230-2230, 2268-2268, 2332-2332
pacquet/crates/config/src/defaults.rs (1)
80-92: LGTM!
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.4674120568999998,
"stddev": 0.07802922154642,
"median": 2.4470448147,
"user": 2.7645650399999995,
"system": 3.80627522,
"min": 2.3993040082,
"max": 2.6741052202,
"times": [
2.6741052202,
2.4490012172,
2.4821193672,
2.4945697052,
2.4450884122,
2.3993040082,
2.4209182692,
2.4261437332,
2.4267589942,
2.4561116422000002
]
},
{
"command": "pacquet@main",
"mean": 2.4423776136,
"stddev": 0.04210503361848168,
"median": 2.4478164787,
"user": 2.7573174399999996,
"system": 3.7878310199999996,
"min": 2.3581336582,
"max": 2.5177116362,
"times": [
2.4328508362,
2.4535490142,
2.4337042022,
2.4591469712,
2.4015131562,
2.3581336582,
2.4438100702,
2.4715337042,
2.5177116362,
2.4518228872
]
},
{
"command": "pnpm",
"mean": 4.974697775699999,
"stddev": 0.0895644154153369,
"median": 4.975167923700001,
"user": 8.41047334,
"system": 4.29014522,
"min": 4.8674246442,
"max": 5.1781084032,
"times": [
5.0293435252,
4.9901424092,
4.9827608562000005,
4.9138050052,
4.9675749912,
5.1781084032,
4.8674246442,
4.9341782822,
4.8767793262,
5.0068603142
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.7025594730800001,
"stddev": 0.037475220103137155,
"median": 0.6813734918800001,
"user": 0.39165972,
"system": 1.5658668399999995,
"min": 0.67048800538,
"max": 0.76923668438,
"times": [
0.76923668438,
0.72885360438,
0.68017904338,
0.67048800538,
0.70968956738,
0.6720899153800001,
0.75940616238,
0.6786115783800001,
0.68256794038,
0.6744722293800001
]
},
{
"command": "pacquet@main",
"mean": 0.70620882138,
"stddev": 0.02944395883310541,
"median": 0.69707264788,
"user": 0.39842811999999994,
"system": 1.5700981399999996,
"min": 0.66971366538,
"max": 0.76412812638,
"times": [
0.7073308823800001,
0.66971366538,
0.68281064138,
0.76412812638,
0.68572205738,
0.69039617538,
0.70374912038,
0.68977326738,
0.73926735338,
0.7291969243800001
]
},
{
"command": "pnpm",
"mean": 2.6268179064800004,
"stddev": 0.1151383420255221,
"median": 2.61492279188,
"user": 3.3301411199999995,
"system": 2.26899734,
"min": 2.4527928343800003,
"max": 2.87860446138,
"times": [
2.7280399283800003,
2.60947170938,
2.60638721638,
2.87860446138,
2.62799233438,
2.55138288438,
2.4527928343800003,
2.53697433138,
2.6561594903800003,
2.6203738743800002
]
}
]
} |
… lock `ALLOW_PREFIXES` was only locked for the read/write of its `Vec`, not across "set allowlist then probe" — under nextest's default parallel execution two scenarios could race: scenario A would set its prefixes, scenario B would overwrite them, and A's `resolve_store_dir` would observe B's allowlist. Add `PREFIX_PROBE_SCENARIO_LOCK` and a `PrefixProbe::with_allow(prefixes, body)` helper that holds it across the entire set-and-probe so scenarios serialise cleanly. Per CodeRabbit review on #11804.
…tionist Dylint's `perfectionist::single-letter-generic` flagged the `R` return-type parameter on `PrefixProbe::with_allow`. Rename to `Output`.
Summary
getStorePath/storePathRelativeToHomecross-volume detection to pacquet'sdefault_store_dir.storeDiris explicitly set,Config::currentnow probes whether the project root can be hardlinked into the user's pnpm home dir and falls back to<mountpoint>/.pnpm-storewhen it can't.LinkProbecapability inpacquet-config::apiso tests can answer the linkability question deterministically; productionHostimpl performs the real link attempts.Why
Before this fix, a workspace on a separate volume (e.g.
/Volumes/src/on macOS) installed into the home-volume store (~/Library/pnpm/store). When the home volume is case-insensitive and the workspace volume is case-sensitive, typescript-eslint's path cache canonicalises against the home store and then can't locate the same files in TypeScript's case-sensitive program loaded from the workspace, soeslint --fixfails withParsing error: ESLint was configured to run on <file> using parserOptions.project: tsconfig.lint.json — TSConfig does not include this fileon every project file.Concrete repro: on the
use-pacquet-config-depbranch (which delegates installs to pacquet viaconfigDependencies['@pnpm/pacquet']),pn -w compilefails itseslint --fixstep with ~50 of those errors. After this fix pacquet picks/Volumes/src/.pnpm-store/v11— the same store pnpm 11 picks viagetStorePath— and the lint succeeds.What landed
pacquet/crates/config/src/store_path.rs— port ofgetStorePath/storePathRelativeToHome/rootLinkTarget/canLinkToSubdir/nextPath. Algorithm is generic overLinkProbe; theHostimpl performs the real link attempts viahost_can_link_between_dirs.pacquet/crates/config/src/store_path/tests.rs— 11 unit tests:next_pathwalks,filesystem_rootextraction, same-volume happy path (real fixture), cross-volume to mountpoint (DI probe), mountpoint-parent preference,pkg_root-only fallback, no-mountpoint fallback, missing-pkg_rootfallback, and theHostprimitive itself. Each cites the upstreamindex.tslines it mirrors.pacquet/crates/config/src/api.rs— newLinkProbecapability trait +Hostimpl.pacquet/crates/config/src/lib.rs— addedstore_dir_explicittracking through the cascade (global config.yaml → pnpm-workspace.yaml →PNPM_CONFIG_*env vars), and a late-stageresolve_store_dir::<Sys>call when the user didn't pinstoreDir.Config::current'sSysbound gained+ LinkProbe; aninert_link_probe!macro in the test module wires the bound onto every existing test fake.pacquet/crates/config/src/defaults.rs— doc-comment update ondefault_store_dirpointing at the new re-resolution step.Test plan
cargo nextest run --workspace— 1947/1947 pass, 3 skipped.cargo nextest run -p pacquet-config -E 'test(store_path)'— 11/11 pass.cargo clippy --locked --workspace --all-targets -- --deny warningsclean.just dylintclean.cargo fmt --all -- --checkclean.pacquet install --frozen-lockfilein/Volumes/src/tmp/pacquet-store-test(workspace on case-sensitive volume) now linksnode_modules/is-positive→/Volumes/src/.pnpm-store/v11/links/..., matching whatpnpm installpicks at the same directory.Written by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit
New Features
Documentation
Tests