Conversation
Read `<virtual_store_dir>/lock.yaml` at install start and diff each wanted snapshot against it. A snapshot whose `dependencies`, `optionalDependencies`, and `resolution.integrity` match the current lockfile *and* whose virtual-store slot exists on disk is dropped from the install graph entirely — no fetch, no extract, no relink. When the slot is gone but the cache key still matches, pacquet emits `pnpm:_broken_node_modules` and falls through to the full install path for that snapshot. At end-of-install pacquet writes the wanted lockfile to `<virtual_store_dir>/lock.yaml` (deleting the file when the lockfile is empty), so the next install sees a fresh diff target. Mirrors upstream pnpm's `readCurrentLockfile` / `writeCurrentLockfile` loop and the per-`depPath` skip in `lockfileToDepGraph` at pnpm v11 `94240bc046`. Closes #433 sections A (read), B (per-snapshot skip), C (write), and D (reporter signal). `pnpm:context.currentLockfileExists` now reflects reality (was hard-coded `false`) and `pnpm:stats.added` reports the post-skip delta rather than the total snapshot count, matching upstream.
📝 WalkthroughWalkthroughReads/writes a current lockfile at the virtual-store root, adds per-snapshot equality checks to skip unchanged snapshots when possible, emits a BrokenModules event for missing cached slots, and threads current-lockfile state through frozen-lockfile and install flows with tests. ChangesVirtual-store lockfile persistence and snapshot skip filtering
Sequence Diagram(s)sequenceDiagram
participant Installer
participant CreateVirtualStore
participant FileSystem
participant Reporter
Installer->>CreateVirtualStore: provide current_snapshots/current_packages
CreateVirtualStore->>CreateVirtualStore: snapshot_deps_equal / integrity_equal checks
CreateVirtualStore->>FileSystem: stat virtual-store slot path
FileSystem-->>CreateVirtualStore: exists / missing
CreateVirtualStore->>Reporter: emit pnpm:_broken_node_modules when missing
Installer->>FileSystem: save_current_to_virtual_store_dir (atomic write or delete)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
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)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
==========================================
+ Coverage 86.98% 88.49% +1.50%
==========================================
Files 93 93
Lines 6647 6822 +175
==========================================
+ Hits 5782 6037 +255
+ Misses 865 785 -80 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements pnpm-style “current lockfile” tracking (<virtual_store_dir>/lock.yaml) to enable warm, partial installs under --frozen-lockfile by skipping snapshots that are unchanged and still present on disk, while preserving pnpm reporter wire compatibility (including _broken_node_modules).
Changes:
- Add load/save support for the current lockfile (
lock.yaml) inpacquet_lockfile, includingLockfile::is_emptyand atomic-ish persistence behavior. - Thread current-lockfile state through the frozen-lockfile install pipeline to skip unchanged snapshots, emit
_broken_node_moduleswhen a slot is missing, and reportpnpm:stats.addedbased on the post-skip delta. - Add/extend unit + integration coverage for skip behavior, broken-modules event shape, and current-lockfile read/write semantics.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/reporter/src/tests.rs | Adds a serialization test to pin pnpm:_broken_node_modules wire shape. |
| crates/reporter/src/lib.rs | Introduces LogEvent::BrokenModules + payload type for _broken_node_modules. |
| crates/package-manager/src/install/tests.rs | Adds integration tests covering skip path, broken-modules emission, context flag flip, and stats/progress behavior. |
| crates/package-manager/src/install.rs | Loads current lockfile for context + skip inputs; writes current lockfile at end of frozen installs. |
| crates/package-manager/src/install_frozen_lockfile.rs | Threads current snapshots/packages into the frozen-lockfile install runner. |
| crates/package-manager/src/create_virtual_store/tests.rs | Adds unit tests for snapshot dep equality + integrity equality helpers. |
| crates/package-manager/src/create_virtual_store.rs | Implements per-snapshot skip filtering and adjusts pnpm:stats.added based on filtered set. |
| crates/lockfile/src/save_lockfile/tests.rs | Adds tests for current-lockfile round-trip, ENOENT-as-None, and empty-lockfile delete semantics. |
| crates/lockfile/src/save_lockfile.rs | Implements writing/deleting <virtual_store_dir>/lock.yaml with a temp+rename helper. |
| crates/lockfile/src/load_lockfile.rs | Adds load_current_from_virtual_store_dir and refactors shared load-from-path logic. |
| crates/lockfile/src/lib.rs | Adds CURRENT_FILE_NAME constant and Lockfile::is_empty helper for suppressing stale current lockfiles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Micro-Benchmark ResultsLinux |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@crates/package-manager/src/create_virtual_store.rs`:
- Around line 252-257: The loop currently calls snapshot_cache_key for every
entry in snapshots before applying the current-lockfile skip filter, causing
UnsupportedResolution for unchanged directory-backed snapshots; change the logic
so you first run the skip filter to produce the subset of snapshots that
survive, then derive cache keys only for those survivors (i.e., apply
snapshot_cache_key only after filtering). Update the code that builds
all_snapshot_entries (the mapping over snapshots and the call to
snapshot_cache_key) to operate on the filtered collection so existing
materialized slots can be reused instead of failing early.
In `@crates/package-manager/src/install.rs`:
- Around line 244-257: The current code persists the current lockfile early (the
if frozen_lockfile && let Some(lockfile) {
lockfile.save_current_to_virtual_store_dir(...) ... }) which can leave a new
lock.yaml on disk if subsequent steps like write_modules_manifest fail; move
this persistence so it runs only after the install is fully committed (i.e.,
after write_modules_manifest returns successfully and any final commit steps
complete) and keep the same error mapping to InstallError::SaveCurrentLockfile;
ensure the condition still checks frozen_lockfile && let Some(lockfile) and
invoke lockfile.save_current_to_virtual_store_dir(&config.virtual_store_dir)
only on the successful path.
In `@crates/package-manager/src/install/tests.rs`:
- Around line 941-1054: The test currently bypasses the write path by using an
empty Lockfile first and then pre-seeding the virtual store before the second
Install::run; change the first run to use a non-empty fixture (e.g., parse
PARTIAL_INSTALL_LOCKFILE into the initial Lockfile instead of the empty one) so
the install must persist a non-empty current lockfile, and remove the manual
call to partial.save_current_to_virtual_store_dir(&virtual_store_dir) (and any
unnecessary seed_placeholder_virtual_store_slot call) so the second Install::run
exercises the read-after-write path rather than relying on pre-seeded state.
🪄 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: 9b4f9a4b-6503-4319-941d-2b0e2283b27e
📒 Files selected for processing (11)
crates/lockfile/src/lib.rscrates/lockfile/src/load_lockfile.rscrates/lockfile/src/save_lockfile.rscrates/lockfile/src/save_lockfile/tests.rscrates/package-manager/src/create_virtual_store.rscrates/package-manager/src/create_virtual_store/tests.rscrates/package-manager/src/install.rscrates/package-manager/src/install/tests.rscrates/package-manager/src/install_frozen_lockfile.rscrates/reporter/src/lib.rscrates/reporter/src/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). (6)
- GitHub Check: Agent
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Code Coverage
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Preserve existing method chains andpipe-traitchains; do not break them into intermediateletbindings unless there is a concrete justification such as a compilation failure, borrow checker rejection, meaningful performance improvement, or other technical necessity. Refactoring for style alone is not sufficient justification.
Choose owned vs. borrowed parameters to minimize copies; prefer borrowed types (&Pathover&PathBuf,&strover&String) when it does not force extra copies.
PreferArc::clone(&x)andRc::clone(&x)overx.clone()for reference-counted types to make the cost visible at the call site.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;for any glob whose target is a module you control. External-crate preludes (e.g.,use rayon::prelude::*;) and root-of-module re-exports (e.g.,pub use submodule::*;inlib.rs) are exceptions.
Follow Rust API Guidelines for naming, as documented in https://rust-lang.github.io/api-guidelines/naming.html.
Declare a newtype wrapper for any branded string type being ported from TypeScript pnpm. Do not collapse the brand into a plainStringor&str; give the type its own struct so misuse is a type error.
When porting branded string types where upstream TypeScript always validates before construction, validate in the Rust port too. Construct the wrapper only viaTryFrom<String>and/orFromStr; do not provide an infallible public constructor that takes an arbitrary string.
For branded string types where upstream TypeScript never validates (used purely for type-safety to prevent confusion between string slots), expose an infallibleFrom<String>andFrom<&str>constructor in the Rust wrapper.
When upstream TypeScript occasionally constructs a branded type without validation (via bareasassertion), add afrom_str_unchecked(or similarly named) constructor on the Rust side. Keep the validating constructor as well; `from_str_u...
Files:
crates/reporter/src/lib.rscrates/lockfile/src/lib.rscrates/package-manager/src/install_frozen_lockfile.rscrates/lockfile/src/save_lockfile/tests.rscrates/package-manager/src/create_virtual_store/tests.rscrates/reporter/src/tests.rscrates/package-manager/src/install.rscrates/lockfile/src/load_lockfile.rscrates/lockfile/src/save_lockfile.rscrates/package-manager/src/install/tests.rscrates/package-manager/src/create_virtual_store.rs
🧠 Learnings (3)
📚 Learning: 2026-05-07T23:19:08.272Z
Learnt from: KSXGitHub
Repo: pnpm/pacquet PR: 401
File: tasks/integrated-benchmark/src/work_env.rs:343-344
Timestamp: 2026-05-07T23:19:08.272Z
Learning: When reviewing Rust code in pnpm/pacquet for deprecated API usage, do not automatically treat `serde_saphyr::to_string` as deprecated. In `serde-saphyr` v0.0.25, `serde_saphyr::to_string` has no `#[deprecated]` attribute (the `#[deprecated]` later in `serde-saphyr-0.0.25/src/lib.rs` applies to a different function). Only flag `serde_saphyr::to_string` as deprecated if the resolved dependency version’s source shows `#[deprecated]` on that specific function.
Applied to files:
crates/reporter/src/lib.rscrates/lockfile/src/lib.rscrates/package-manager/src/install_frozen_lockfile.rscrates/lockfile/src/save_lockfile/tests.rscrates/package-manager/src/create_virtual_store/tests.rscrates/reporter/src/tests.rscrates/package-manager/src/install.rscrates/lockfile/src/load_lockfile.rscrates/lockfile/src/save_lockfile.rscrates/package-manager/src/install/tests.rscrates/package-manager/src/create_virtual_store.rs
📚 Learning: 2026-05-01T10:01:33.766Z
Learnt from: zkochan
Repo: pnpm/pacquet PR: 349
File: crates/reporter/src/tests.rs:121-121
Timestamp: 2026-05-01T10:01:33.766Z
Learning: In Rust test code, follow the repo’s CODE_STYLE_GUIDE test-logging rule: add logging (e.g., `eprintln!`/`eprintln!(...)`) so that useful diagnostic values are printed when a test fails, unless the assertion is `assert_eq!` (where the differing values are already included). Concretely, if you use assertions like `assert!`, `assert_ne!`, etc., ensure the test logs the relevant actual/expected values (or context) before/around the assertion so failures can be diagnosed without rerunning.
Applied to files:
crates/lockfile/src/save_lockfile/tests.rscrates/package-manager/src/create_virtual_store/tests.rscrates/reporter/src/tests.rscrates/package-manager/src/install/tests.rs
📚 Learning: 2026-05-01T10:01:33.766Z
Learnt from: zkochan
Repo: pnpm/pacquet PR: 349
File: crates/reporter/src/tests.rs:121-121
Timestamp: 2026-05-01T10:01:33.766Z
Learning: In this repo’s Rust tests (run via `cargo nextest`), stdout/stderr from passing tests is captured and is only shown when a test fails. As a result, `eprintln!` (and similar stderr logging) in these tests should generally not be flagged as CI “noise” on the happy path; CI output should only appear for failing tests.
Applied to files:
crates/reporter/src/tests.rs
🔇 Additional comments (9)
crates/reporter/src/lib.rs (1)
166-177: LGTM!Also applies to: 600-608
crates/reporter/src/tests.rs (1)
8-12: LGTM!Also applies to: 639-658
crates/lockfile/src/lib.rs (1)
78-85: LGTM!Also applies to: 95-108
crates/lockfile/src/load_lockfile.rs (1)
8-9: LGTM!Also applies to: 33-34, 36-54, 56-63
crates/lockfile/src/save_lockfile.rs (2)
4-99: LGTM!Also applies to: 102-117
118-123:std::fs::renameon Windows correctly replaces existing destination files—no issue here.The Rust standard library explicitly documents that
rename(src, dst)replacesdstif it already exists; on Windows this is implemented viaMoveFileExor equivalent APIs withMOVEFILE_REPLACE_EXISTINGsemantics. Warm installs (whenlock.yamlalready exists) will succeed as intended. The atomic write pattern used here (write to temp, rename in place) matches best practices and pnpm's approach.Note: The caveat on Windows is that
dstmust not be a directory; this is not an issue for a file likelock.yaml.crates/lockfile/src/save_lockfile/tests.rs (1)
102-175: LGTM!crates/package-manager/src/install_frozen_lockfile.rs (1)
39-47: LGTM!Also applies to: 101-103, 128-129
crates/package-manager/src/create_virtual_store/tests.rs (1)
1-4: LGTM!Also applies to: 6-6, 8-36, 78-168
- Harden `write_atomic` in `save_lockfile.rs`: open the temp file with `O_CREAT | O_EXCL` (`create_new(true)`) and retry on AlreadyExists, matching the pattern in `pacquet_fs::ensure_file::write_atomic`. Was using `fs::write` which followed symlinks and clobbered colliding temp files (Copilot review). - Run the per-snapshot skip filter before deriving cache keys in `CreateVirtualStore::run` so unchanged directory-backed snapshots don't fail with `UnsupportedResolution` ahead of the skip check. Cache-key derivation now only happens for survivors (CodeRabbit). - Move `save_current_to_virtual_store_dir` after `write_modules_manifest` so a manifest-write failure can't leave a fresh current lockfile pointing at incomplete install state (CodeRabbit). - Fix stale comment on `current_lockfile` in `Install::run` that referenced a non-existent `current_lockfile_load_error` and claimed None-on-corruption; the code actually propagates `LoadLockfileError` via `?` and fails the install (Copilot). - Restructure `context_log_reflects_current_lockfile_after_first_install` so the false→true assertion really exercises the install's write path: same non-empty fixture across both runs, no manual `save_current_to_virtual_store_dir` seeding before the second run, plus a positive assertion that `lock.yaml` is on disk after the first install. Skip-path side effects (`added: 0`, zero `imported` events) are split into their own test (CodeRabbit).
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.56256568508,
"stddev": 0.0649922598002834,
"median": 2.55277171568,
"user": 2.55223742,
"system": 3.6237814799999994,
"min": 2.44498583968,
"max": 2.71194252668,
"times": [
2.57929243468,
2.55732040168,
2.54889673568,
2.58620045168,
2.71194252668,
2.55144151668,
2.55191891268,
2.55362451868,
2.54003351268,
2.44498583968
]
},
{
"command": "pacquet@main",
"mean": 2.52900869648,
"stddev": 0.049042433018348444,
"median": 2.5190278686800003,
"user": 2.5496613200000007,
"system": 3.5801888799999992,
"min": 2.46488060768,
"max": 2.6039298626800003,
"times": [
2.51841525568,
2.51964048168,
2.5068641076800002,
2.5719856826800003,
2.57080810368,
2.49190751668,
2.6039298626800003,
2.46488060768,
2.57617166268,
2.46548368368
]
},
{
"command": "pnpm",
"mean": 6.13421727698,
"stddev": 0.03853284854194687,
"median": 6.14298718268,
"user": 8.965209519999998,
"system": 4.53997508,
"min": 6.06143088668,
"max": 6.17841032168,
"times": [
6.12231688868,
6.14744765568,
6.17412955168,
6.11826475568,
6.15252013368,
6.08328323668,
6.06143088668,
6.16584262968,
6.17841032168,
6.13852670968
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.7002537486200001,
"stddev": 0.06893507490107097,
"median": 0.6766295647200001,
"user": 0.37144222000000005,
"system": 1.48132798,
"min": 0.6500868692200001,
"max": 0.8846893432200001,
"times": [
0.8846893432200001,
0.6713135302200001,
0.6732437792200001,
0.6629906412200001,
0.6862435972200001,
0.6711381622200001,
0.6800153502200001,
0.6833265392200001,
0.6500868692200001,
0.7394896742200001
]
},
{
"command": "pacquet@main",
"mean": 0.6958914008200001,
"stddev": 0.07201546264807367,
"median": 0.68032391822,
"user": 0.34649251999999997,
"system": 1.48458168,
"min": 0.6491565092200001,
"max": 0.8920035132200002,
"times": [
0.8920035132200002,
0.6774449012200001,
0.6518815692200001,
0.6845115092200001,
0.7113252122200001,
0.6533833732200001,
0.6491565092200001,
0.6582650392200001,
0.6977394462200001,
0.6832029352200001
]
},
{
"command": "pnpm",
"mean": 2.7017339414199997,
"stddev": 0.09653537302219489,
"median": 2.7058586492199996,
"user": 3.16220812,
"system": 2.27416038,
"min": 2.55458438822,
"max": 2.90393533522,
"times": [
2.74079652522,
2.55458438822,
2.6174519722199996,
2.7574112172199996,
2.6231852502199997,
2.70216760822,
2.70954969022,
2.6631325312199996,
2.90393533522,
2.74512489622
]
}
]
} |
Without this, the skip filter bypasses the side-effects-cache prefetch for any snapshot it skips, so `BuildModules`'s `is_built` gate misses on warm reinstalls and every package with `allowBuilds: true` re-executes its lifecycle scripts. On a fixture with 1352 packages and 3 allowed builds, warm reinstalls regressed from ~0.9 s (baseline / `main`) to ~3.0 s. Fix: derive cache keys for *all* snapshots, not just survivors. Survivors go through the strict path so a malformed wanted snapshot still surfaces. Skipped snapshots get a lenient pass: any per-snapshot error collapses to no prefetch entry (same outcome as if the skip filter had not engaged). The combined cache keys feed the prefetch and `package_manifests` / `side_effects_maps_by_snapshot` get populated over the full snapshot set. Warm/cold partitioning still operates only on survivors — the skip filter's effect on the link work is unchanged. After the fix, warm reinstall on a 1352-package fixture: - PR: 896 ms ± 66 ms (sys 349 ms) - main: 969 ms ± 39 ms (sys 985 ms) - PR is 1.08x faster, with ~65% less kernel work
Benchmark resultsRan a warm-reinstall benchmark to verify the PR's performance impact. Fixture: a 1352-package lockfile (one of the larger real-world fixtures I have), warm CAFS, warm node_modules, hyperfine Round 1 — found a regression
PR was 3.25× slower. Bisected the cause: Root cause: the skip filter ran before Round 2 — after the fixCommit 1603f94 derives cache keys for all snapshots (lenient for skipped ones), so the prefetch covers everyone the build phase might need to gate on. Warm/cold partitioning still operates only on survivors.
PR is 1.08× faster than baseline on warm reinstall, with ~65% less kernel work. The big sys-time drop is the skip filter avoiding 1352 per-snapshot syscalls in the warm-batch CAFS link path. The win is modest on this workload because pacquet's warm-batch was already cheap (~330 ms) when every per-snapshot operation collapses to
Test suiteAll 633 tests still pass, lint + dylint + taplo clean. The fix didn't require any test changes — the existing Written by an agent (Claude Code, claude-opus-4-7). |
Copilot review on PR #442 noted that two doc comments hard-coded `node_modules/.pnpm/...` even though pacquet's `virtual_store_dir` is configurable (default `.pnpm` to match pnpm v11, but tests and custom configs override it). Switch to the `<virtual_store_dir>` placeholder used elsewhere in the codebase so the inline docs match the actual on-disk layout regardless of how the user configures it.
Additional benchmark: non-up-to-date node_modulesFollowing up on the question of how the PR behaves when
Interpretation
Caveats
Bottom line: the PR is consistently a small win and never a loss across the damage spectrum, in addition to the pure-warm 1.08× win in the original benchmark. Written by an agent (Claude Code, claude-opus-4-7). |
Two cleanups while rebasing on top of #442 (partial frozen install): - `install.rs`: the comment at the `<virtual_store_dir>/lock.yaml` write site referenced #431 as future work. With workspace install now landing in this PR, refresh the wording to call out that we *do* ship every importer's section unchanged, and the remaining narrowing (selected importers × engine filter) is gated on `--filter` (Stage 2 of #299). - `root_finder/tests.rs`: rustfmt collapses the if/else closures introduced in a0d4df8 to single-line `if … else …` form. Apply it once here so the diff doesn't keep returning across runs. --- Written by an agent (Claude Code, claude-opus-4-7).
After rebasing onto current main (#442 — partial install with --frozen-lockfile) the CI clippy gate fires two errors I missed locally: - `get_subgraph_to_build` had 8 parameters after the `skipped` thread-through (clippy::too_many_arguments, threshold 7). Bundle the 4 read-only inputs (`children`, `requires_build`, `patches`, `skipped`) into a `GetSubgraphCtx` struct; the 3 `&mut` accumulators stay as plain params. 5 total now. - `Some(list) if list.is_empty()` in `platform_axis_meaningful` is clippy::redundant_guards. Use the empty-slice pattern (`Some([])`) and the single-element pattern (`Some([only]) if only == "any"`) so each pattern says what it matches without a separate guard clause. The rebase itself: main's #442 added a per-snapshot "skip already-installed-and-still-on-disk" pass in `CreateVirtualStore::run`. Layered the installability skip on top — it filters BEFORE that pass (installability-skipped snapshots should never enter the install graph at all, including the `skipped_entries` cache-key lane that #442 keeps warm for the build-cache lookup).
Two cleanups while rebasing on top of #442 (partial frozen install): - `install.rs`: the comment at the `<virtual_store_dir>/lock.yaml` write site referenced #431 as future work. With workspace install now landing in this PR, refresh the wording to call out that we *do* ship every importer's section unchanged, and the remaining narrowing (selected importers × engine filter) is gated on `--filter` (Stage 2 of #299). - `root_finder/tests.rs`: rustfmt collapses the if/else closures introduced in a0d4df8 to single-line `if … else …` form. Apply it once here so the diff doesn't keep returning across runs. --- Written by an agent (Claude Code, claude-opus-4-7).
After rebasing onto current main (#442 — partial install with --frozen-lockfile) the CI clippy gate fires two errors I missed locally: - `get_subgraph_to_build` had 8 parameters after the `skipped` thread-through (clippy::too_many_arguments, threshold 7). Bundle the 4 read-only inputs (`children`, `requires_build`, `patches`, `skipped`) into a `GetSubgraphCtx` struct; the 3 `&mut` accumulators stay as plain params. 5 total now. - `Some(list) if list.is_empty()` in `platform_axis_meaningful` is clippy::redundant_guards. Use the empty-slice pattern (`Some([])`) and the single-element pattern (`Some([only]) if only == "any"`) so each pattern says what it matches without a separate guard clause. The rebase itself: main's #442 added a per-snapshot "skip already-installed-and-still-on-disk" pass in `CreateVirtualStore::run`. Layered the installability skip on top — it filters BEFORE that pass (installability-skipped snapshots should never enter the install graph at all, including the `skipped_entries` cache-key lane that #442 keeps warm for the build-cache lookup).
Two cleanups while rebasing on top of #442 (partial frozen install): - `install.rs`: the comment at the `<virtual_store_dir>/lock.yaml` write site referenced #431 as future work. With workspace install now landing in this PR, refresh the wording to call out that we *do* ship every importer's section unchanged, and the remaining narrowing (selected importers × engine filter) is gated on `--filter` (Stage 2 of #299). - `root_finder/tests.rs`: rustfmt collapses the if/else closures introduced in a0d4df8 to single-line `if … else …` form. Apply it once here so the diff doesn't keep returning across runs. --- Written by an agent (Claude Code, claude-opus-4-7).
After rebasing onto current main (#442 — partial install with --frozen-lockfile) the CI clippy gate fires two errors I missed locally: - `get_subgraph_to_build` had 8 parameters after the `skipped` thread-through (clippy::too_many_arguments, threshold 7). Bundle the 4 read-only inputs (`children`, `requires_build`, `patches`, `skipped`) into a `GetSubgraphCtx` struct; the 3 `&mut` accumulators stay as plain params. 5 total now. - `Some(list) if list.is_empty()` in `platform_axis_meaningful` is clippy::redundant_guards. Use the empty-slice pattern (`Some([])`) and the single-element pattern (`Some([only]) if only == "any"`) so each pattern says what it matches without a separate guard clause. The rebase itself: main's #442 added a per-snapshot "skip already-installed-and-still-on-disk" pass in `CreateVirtualStore::run`. Layered the installability skip on top — it filters BEFORE that pass (installability-skipped snapshots should never enter the install graph at all, including the `skipped_entries` cache-key lane that #442 keeps warm for the build-cache lookup).
Two issues flagged by Copilot on #449: 1. `build_modules::virtual_store_dir_for_key` was calling `layout.slot_dir(&bare_key)` after peer-stripping the snapshot key. `VirtualStoreLayout` keys its precomputed GVS suffixes by the *full* snapshot key (with the peer suffix), so the lookup missed for peer-resolved snapshots and fell through to the legacy flat-name path — pointing at a directory `CreateVirtualDirBySnapshot` never created. Effect: build scripts silently skipped for peer-resolved packages. Switched the lookup to the full `key`; the package-name segment still comes from the peer-stripped key. 2. `CreateVirtualStore::run`'s partial-install skip probe (added in #442) still hard-coded `config.virtual_store_dir.join(to_virtual_store_name)`, which is the legacy layout. Under GVS-on the slot lives at `layout.slot_dir(snapshot_key)`, so warm slots were being classified as missing and `BrokenModules` emitted for the wrong path. Routed through the layout to match what the actual install / link steps use. #442's three partial-install tests (`warm_reinstall_skips_snapshot_when_current_lockfile_matches`, `warm_reinstall_emits_broken_modules_when_dir_is_missing`, `warm_reinstall_reports_added_zero_and_emits_no_imported_events`) seeded `<virtual_store_dir>/<flat-name>` slots manually — the legacy shape. With the probe now routed through the layout under default GVS-on, those legacy seeds no longer match. Each test sets `enable_global_virtual_store = false` so the seeded layout matches the probe; the partial-install behaviour under test is independent of the GVS layout, and the GVS-on dispatch is exercised by the `frozen_lockfile_*_gvs_*` tests. All 704 workspace tests pass; taplo / dylint / cargo doc clean.
…incompatibles (#434 slice 1) (#439) Slice 1 of #434 — foundational installability gate. Ports [`@pnpm/config.package-is-installable`](https://github.com/pnpm/pnpm/tree/94240bc046/config/package-is-installable) and threads the resulting skip-set through every phase of the frozen-lockfile install pipeline. Closes #266 once shipped (covers the "install respects every snapshot — no os/cpu/libc filter" gap). Does **not** close #434 — that umbrella has six more slices to follow. Upstream reference: pnpm/pnpm@94240bc046. ## What landed ### New crate: `pacquet-package-is-installable` Ports the upstream `config/package-is-installable` package's three helpers: - `check_platform` (`Option<&[String]>` for each `os`/`cpu`/`libc` axis, plus a `SupportedArchitectures` override) — returns `Option<UnsupportedPlatformError>` matching upstream's `ERR_PNPM_UNSUPPORTED_PLATFORM` code, message shape, and JSON payload. Handles negation entries (`!foo`), the `any` sentinel, the `current` placeholder, and the `currentLibc !== 'unknown'` skip from `checkPlatform.ts:38`. - `check_engine` — evaluates `engines.node` / `engines.pnpm` via `node-semver`. Approximates npm-semver's `includePrerelease: true` via a strip-prerelease fallback; one over-acceptance edge case (`>=X.Y.Z` against `X.Y.Z-rc1`) is pinned in the known-failures integration test for follow-up. - `package_is_installable` — composes the two, returns the tri-state verdict matching upstream's `boolean | null` (Installable / SkipOptional / ProceedWithWarning), plus an `Err` arm for `engine_strict` aborts and `ERR_PNPM_INVALID_NODE_VERSION`. `InstallabilityOptions<'a>` borrows its host strings so a caller running through many snapshots in a row can build the host part once and only toggle `optional` per snapshot. `WantedPlatformRef<'a>` plays the same role for the manifest axes so `check_platform` runs the happy path without any allocation. ### New module: `pacquet_package_manager::installability` `compute_skipped_snapshots` is the per-install entry point. For each snapshot: 1. Look up the matching `PackageMetadata`. 2. Run `check_package` (cached per peer-stripped `metadata_key` so peer-resolved variants of the same package share one verdict). 3. Dispatch on `(verdict, snapshot.optional, host.engine_strict)`: - `Installable`: nothing to do. - `SkipOptional` + `optional`: add to `SkippedSnapshots`, emit `pnpm:skipped-optional-dependency` (deduped per metadata key, matching upstream's emit-per-pkgId). - `Incompatible` + non-optional + `engine_strict`: abort. - `Incompatible` + non-optional + non-strict: `tracing::warn!` and proceed. (Upstream's `pnpm:install-check` channel isn't wired into pacquet's reporter yet — slice 1 follow-up.) `any_installability_constraint(packages)` is the caller-side fast path: if no metadata row declares an `engines.{node,pnpm}` or a non-empty / non-`["any"]` `cpu`/`os`/`libc`, the entire installability pass is skipped. The probe runs synchronously in `install_frozen_lockfile` *before* the `tokio::task::spawn_blocking` that would invoke `node --version` — so the common no-constraints lockfile pays nothing for the new pipeline, restoring main's overlap of node-binary startup with extraction. ### Install-pipeline plumbing The `SkippedSnapshots` set is threaded into every downstream phase of `InstallFrozenLockfile::run`: - `CreateVirtualStore`: installability-skipped snapshots are dropped from both `survivors` (no virtual-store slot extracted) and `skipped_entries` (no warm-cache row). Layered ahead of main's #442 already-installed-and-on-disk skip filter. - `SymlinkDirectDependencies`: a direct dep whose resolved snapshot is in the skip set is omitted from `node_modules/<name>` (no symlink, no `pnpm:root added` event, no bin link). - `LinkVirtualStoreBins`: per-slot bin link skips slots whose snapshot is installability-skipped (their virtual-store directories don't exist). - `BuildModules` via `build_sequence`: `get_subgraph_to_build` consults `skipped` *before* recursion, so a skipped snapshot's subtree doesn't contribute to the build graph via that edge. Descendants reachable from a non-skipped root still build normally. ### Performance CI integrated-benchmark on the 1352-package fixture, latest run: | Scenario | `pacquet@HEAD` | `pacquet@main` | Relative | |---|---|---|---| | Frozen Lockfile (cold) | 2.476 ± 0.083 s | 2.442 ± 0.071 s | 1.01 ± 0.05 | | Frozen Lockfile (Hot Cache) | 685.8 ± 59.3 ms | 700.2 ± 47.4 ms | 1.00 | Earlier iterations of this PR showed a ~5% cold-install regression from the `node --version` spawn landing on the extraction critical path. Closed by hoisting the no-constraints fast-path probe to the caller (commit `cf47ce51`) so the spawn is gated on actual constraint presence. Other perf passes folded in: - `compute_skipped_snapshots` caches the per-metadata-row check verdict so peer-resolved variants share one `check_package` call. - `check_platform` borrows its three wanted axes through `WantedPlatformRef<'a>`; the owned `WantedPlatform` only materialises in the error path. ## Tests | Suite | Count | What it covers | |---|---|---| | `pacquet-package-is-installable::tests::check_platform` | 16 | Port of upstream `checkPlatform.ts` — `any`/`current` sentinels, negation, `supportedArchitectures` override, libc unknown-skip | | `pacquet-package-is-installable::tests::check_engine` | 7 | Port of upstream `checkEngine.ts` — node/pnpm range checks, prerelease cases, `ERR_PNPM_INVALID_NODE_VERSION` | | `pacquet-package-is-installable::tests::package_is_installable` | 6 | Tri-state verdict + optional/engine-strict dispatch | | `pacquet-package-is-installable::tests::known_failures` | 1 | The `>=X.Y.Z` vs `X.Y.Z-rc1` over-acceptance, picked up by `just known-failures` | | `pacquet_package_manager::installability::tests` | 11 | Per-install skip-set computation: skip on bad OS, skip on bad node engine, dedup events across peer variants, fast-path triggers, constraint predicate's edge cases (`engines.npm` only, `["any"]` sentinel, empty lists) | | `pacquet_package_manager::build_sequence::tests` | 3 (new) | Skipped+patched doesn't enter build queue; skipped parent doesn't drag descendants in; descendant with non-skipped parent still builds | All ported tests verified to catch regressions by temporarily breaking the subject under test, observing the failure, then reverting. The "test the tests" workflow from CLAUDE.md. ## Deferred to follow-up slices - `.modules.yaml.skipped` write/read + headless re-check (slice 3). - `supportedArchitectures` config + `--cpu` / `--os` / `--libc` CLI flags (slice 2). - `pnpm:install-check` warn channel on the reporter side (currently `tracing::warn!`). - Real libc detection — `host_libc()` returns `"unknown"` today; matches non-Linux host behavior, but on Alpine/musl this over-installs glibc-only optional packages. Slice 2. - `engine_strict` config wiring — defaults to false today, so the error path is unreachable from production. Wired through end-to-end so the slice that flips the config doesn't churn the error enum.
Two cleanups while rebasing on top of #442 (partial frozen install): - `install.rs`: the comment at the `<virtual_store_dir>/lock.yaml` write site referenced #431 as future work. With workspace install now landing in this PR, refresh the wording to call out that we *do* ship every importer's section unchanged, and the remaining narrowing (selected importers × engine filter) is gated on `--filter` (Stage 2 of #299). - `root_finder/tests.rs`: rustfmt collapses the if/else closures introduced in a0d4df8 to single-line `if … else …` form. Apply it once here so the diff doesn't keep returning across runs. --- Written by an agent (Claude Code, claude-opus-4-7).
Two issues flagged by Copilot on #449: 1. `build_modules::virtual_store_dir_for_key` was calling `layout.slot_dir(&bare_key)` after peer-stripping the snapshot key. `VirtualStoreLayout` keys its precomputed GVS suffixes by the *full* snapshot key (with the peer suffix), so the lookup missed for peer-resolved snapshots and fell through to the legacy flat-name path — pointing at a directory `CreateVirtualDirBySnapshot` never created. Effect: build scripts silently skipped for peer-resolved packages. Switched the lookup to the full `key`; the package-name segment still comes from the peer-stripped key. 2. `CreateVirtualStore::run`'s partial-install skip probe (added in #442) still hard-coded `config.virtual_store_dir.join(to_virtual_store_name)`, which is the legacy layout. Under GVS-on the slot lives at `layout.slot_dir(snapshot_key)`, so warm slots were being classified as missing and `BrokenModules` emitted for the wrong path. Routed through the layout to match what the actual install / link steps use. (`warm_reinstall_skips_snapshot_when_current_lockfile_matches`, `warm_reinstall_emits_broken_modules_when_dir_is_missing`, `warm_reinstall_reports_added_zero_and_emits_no_imported_events`) seeded `<virtual_store_dir>/<flat-name>` slots manually — the legacy shape. With the probe now routed through the layout under default GVS-on, those legacy seeds no longer match. Each test sets `enable_global_virtual_store = false` so the seeded layout matches the probe; the partial-install behaviour under test is independent of the GVS layout, and the GVS-on dispatch is exercised by the `frozen_lockfile_*_gvs_*` tests. All 704 workspace tests pass; taplo / dylint / cargo doc clean.
Two cleanups while rebasing on top of #442 (partial frozen install): - `install.rs`: the comment at the `<virtual_store_dir>/lock.yaml` write site referenced #431 as future work. With workspace install now landing in this PR, refresh the wording to call out that we *do* ship every importer's section unchanged, and the remaining narrowing (selected importers × engine filter) is gated on `--filter` (Stage 2 of #299). - `root_finder/tests.rs`: rustfmt collapses the if/else closures introduced in a0d4df8 to single-line `if … else …` form. Apply it once here so the diff doesn't keep returning across runs. --- Written by an agent (Claude Code, claude-opus-4-7).
Two cleanups while rebasing on top of #442 (partial frozen install): - `install.rs`: the comment at the `<virtual_store_dir>/lock.yaml` write site referenced #431 as future work. With workspace install now landing in this PR, refresh the wording to call out that we *do* ship every importer's section unchanged, and the remaining narrowing (selected importers × engine filter) is gated on `--filter` (Stage 2 of #299). - `root_finder/tests.rs`: rustfmt collapses the if/else closures introduced in a0d4df8 to single-line `if … else …` form. Apply it once here so the diff doesn't keep returning across runs. --- Written by an agent (Claude Code, claude-opus-4-7).
Two issues flagged by Copilot on #449: 1. `build_modules::virtual_store_dir_for_key` was calling `layout.slot_dir(&bare_key)` after peer-stripping the snapshot key. `VirtualStoreLayout` keys its precomputed GVS suffixes by the *full* snapshot key (with the peer suffix), so the lookup missed for peer-resolved snapshots and fell through to the legacy flat-name path — pointing at a directory `CreateVirtualDirBySnapshot` never created. Effect: build scripts silently skipped for peer-resolved packages. Switched the lookup to the full `key`; the package-name segment still comes from the peer-stripped key. 2. `CreateVirtualStore::run`'s partial-install skip probe (added in #442) still hard-coded `config.virtual_store_dir.join(to_virtual_store_name)`, which is the legacy layout. Under GVS-on the slot lives at `layout.slot_dir(snapshot_key)`, so warm slots were being classified as missing and `BrokenModules` emitted for the wrong path. Routed through the layout to match what the actual install / link steps use. (`warm_reinstall_skips_snapshot_when_current_lockfile_matches`, `warm_reinstall_emits_broken_modules_when_dir_is_missing`, `warm_reinstall_reports_added_zero_and_emits_no_imported_events`) seeded `<virtual_store_dir>/<flat-name>` slots manually — the legacy shape. With the probe now routed through the layout under default GVS-on, those legacy seeds no longer match. Each test sets `enable_global_virtual_store = false` so the seeded layout matches the probe; the partial-install behaviour under test is independent of the GVS layout, and the GVS-on dispatch is exercised by the `frozen_lockfile_*_gvs_*` tests. All 704 workspace tests pass; taplo / dylint / cargo doc clean.
Two issues flagged by Copilot on #449: 1. `build_modules::virtual_store_dir_for_key` was calling `layout.slot_dir(&bare_key)` after peer-stripping the snapshot key. `VirtualStoreLayout` keys its precomputed GVS suffixes by the *full* snapshot key (with the peer suffix), so the lookup missed for peer-resolved snapshots and fell through to the legacy flat-name path — pointing at a directory `CreateVirtualDirBySnapshot` never created. Effect: build scripts silently skipped for peer-resolved packages. Switched the lookup to the full `key`; the package-name segment still comes from the peer-stripped key. 2. `CreateVirtualStore::run`'s partial-install skip probe (added in #442) still hard-coded `config.virtual_store_dir.join(to_virtual_store_name)`, which is the legacy layout. Under GVS-on the slot lives at `layout.slot_dir(snapshot_key)`, so warm slots were being classified as missing and `BrokenModules` emitted for the wrong path. Routed through the layout to match what the actual install / link steps use. (`warm_reinstall_skips_snapshot_when_current_lockfile_matches`, `warm_reinstall_emits_broken_modules_when_dir_is_missing`, `warm_reinstall_reports_added_zero_and_emits_no_imported_events`) seeded `<virtual_store_dir>/<flat-name>` slots manually — the legacy shape. With the probe now routed through the layout under default GVS-on, those legacy seeds no longer match. Each test sets `enable_global_virtual_store = false` so the seeded layout matches the probe; the partial-install behaviour under test is independent of the GVS layout, and the GVS-on dispatch is exercised by the `frozen_lockfile_*_gvs_*` tests. All 704 workspace tests pass; taplo / dylint / cargo doc clean.
## Summary Threads `VirtualStoreLayout` (from #444) through the frozen-lockfile install pipeline so packages installed with `enableGlobalVirtualStore: true` (pacquet's default since #444) land at the upstream-shaped `<store_dir>/links/<scope>/<name>/<version>/<hash>/node_modules/<pkg>` path. Closes the rest of #432 (Section B + C activation + D entry tests). ## Pipeline changes (frozen-lockfile path) - `InstallFrozenLockfile::run` constructs the install-scoped `VirtualStoreLayout` from the lockfile + engine name + config, then threads `&VirtualStoreLayout` through `CreateVirtualStore`, `CreateVirtualDirBySnapshot`, `create_symlink_layout`, `SymlinkDirectDependencies`, `InstallPackageBySnapshot`, `BuildModules`, and `LinkVirtualStoreBins`. Every site that used to compute `virtual_store_dir.join(key.to_virtual_store_name())` now goes through one `layout.slot_dir(key)` lookup. - `Install::run` calls `pacquet_store_dir::register_project` **once per importer** when `enable_global_virtual_store` is on, writing `<store_dir>/projects/<short-hash>` per workspace package so a future `pacquet store prune` can find every reachable consumer of `<store_dir>/links/...`. Best-effort: registry write failures degrade to `tracing::warn!` and the install continues. The walk runs *after* `InstallFrozenLockfile::run` succeeds so importer keys have already been validated. - `Config::current` now applies `apply_global_virtual_store_derivation` after yaml — pacquet's variant of upstream's [`extendInstallOptions.ts:343-355`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/extendInstallOptions.ts#L343-L355). An explicit `globalVirtualStoreDir:` in yaml wins over the derivation; otherwise the field falls back to the user's pinned `virtualStoreDir` (under GVS-on) or to `<store_dir>/links`. ## Field-split deviation from upstream Upstream pnpm *mutates* `virtualStoreDir` in place under GVS, so every consumer that reads `virtualStoreDir` sees `<storeDir>/links`. Pacquet keeps `virtual_store_dir` at its project-local value and writes the GVS path into the separate `global_virtual_store_dir` field. The frozen-lockfile path consumes `global_virtual_store_dir` through `VirtualStoreLayout`; the legacy non-frozen `InstallWithoutLockfile` keeps reading `virtual_store_dir` (now via `VirtualStoreLayout::legacy`). The split is a pacquet-only concern: upstream has no without-lockfile install path and so doesn't need the second field. See the doc on `Config::apply_global_virtual_store_derivation` for the reasoning. ## Benchmarks The integrated-benchmark fixture pins `enableGlobalVirtualStore: false` so existing scenarios continue to compare apples-to-apples against the project-local `<modules>/.pnpm/<flat-name>` shape. Without the pin, every revision newer than this PR would silently switch to the GVS layout, while `pacquet@main` and the pnpm side of the comparison would still use the isolated layout — different shape, unfair comparison. GVS-specific benchmark runs are available by passing `--fixture-dir <dir>` at a copy of the fixture with the flag flipped. ## Tests - `install::tests::frozen_lockfile_under_gvs_registers_project_and_runs_clean` pins the registry write under default GVS-on (single-project case). - `install::tests::frozen_lockfile_under_gvs_registers_each_workspace_importer` pins per-importer registration: workspace root + `packages/web` each end up with their own symlink in `<store_dir>/projects/`. - `install::tests::frozen_lockfile_with_gvs_off_skips_project_registry` pins that GVS-off opts out of the registry write. - `config::tests::yaml_global_virtual_store_dir_wins_over_derivation` pins the yaml-override precedence. - All **829 workspace tests** pass under the new layout-threaded path. - Existing per-snapshot legacy tests construct `VirtualStoreLayout::legacy(...)` explicitly so they keep asserting the flat-name layout regardless of any global default. Partial-install tests from #442 pin `enable_global_virtual_store = false` so their seeded legacy-shape slots match the probe path. End-to-end port of upstream's [`installing/deps-installer/test/install/globalVirtualStore.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/test/install/globalVirtualStore.ts) is tracked as a follow-up — those tests need a small frozen-lockfile fixture against the registry-mock that doesn't exist yet.
Pull in 28 commits from upstream main, including the `pacquet-npmrc` → `pacquet-config` rename (#420) plus features: - supportedArchitectures + --cpu/--os/--libc (#456) - frozen-lockfile (#442, #443, #447, #450) - git-fetcher (#436 / #446, #451, #454) - side-effects cache (#421 / #422, #423, #424) - real-hoist + global-virtual-store (#432 / #438 / #444, #449, #452) - patchedDependencies + allow-builds (#425, #427, #428) - engine/platform installability (#434 / #439) Conflicts resolved: - `crates/npmrc/` files migrated under the renamed `crates/config/` directory; `Npmrc` → `Config` everywhere except `NpmrcAuth` (which keeps the `.npmrc`-domain name). - `Config::current` reads the env-var DI generic `Api: EnvVar` for ${VAR}-substitution in `.npmrc`. Production turbofish in `cli_args.rs` is `Config::current::<RealApi, _, _, _, _>(...)`. - Two-phase `NpmrcAuth::apply_*` retained so default-registry creds key at the yaml-resolved registry URL. - New `Config::auth_headers` field plumbed through `install_package_by_snapshot`'s `DownloadTarballToStore`. - Tests under `crates/config/src/workspace_yaml/tests.rs` pick up the new ParseYaml unit test added on this branch.
Summary
Closes #433. Implements partial install with
--frozen-lockfile:<virtual_store_dir>/lock.yamlat install start (Lockfile::load_current_from_virtual_store_dir), mirroring upstream pnpm v11'sreadCurrentLockfile.dependencies,optionalDependencies, andresolution.integritymatch the current lockfile and whose virtual-store slot exists on disk. No fetch, no extract, no relink for skipped snapshots.<virtual_store_dir>/lock.yamlat end-of-install (atomically; deletes the file when the lockfile is empty), so the next install sees a fresh diff target.pnpm:_broken_node_modules(BrokenModulesLog) when the slot is gone despite the cache key matching, then fall through to a full install for that snapshot.pnpm:context.currentLockfileExistsnow reflects reality (was hard-codedfalse);pnpm:stats.addedreports the post-skip delta instead of the total snapshot count, matching upstream.BuildModules'sis_builtgate fires on warm reinstalls — without this, packages withallowBuilds: truere-execute their lifecycle scripts every install (added in 1603f94 after benchmarking caught the regression — see benchmark comment).Tracks pnpm v11
94240bc046forreadCurrentLockfile/writeCurrentLockfile/lockfileToDepGraph's per-depPathskip andisIntegrityEqualhelper.Performance
Two scenarios benchmarked against a 1352-package fixture, hyperfine
--warmup 2 --runs 10.Pure warm reinstall (every snapshot intact, full
lock.yamlon disk):mainHEAD~2)PR is 1.08× faster, with ~65% less kernel work — the skip filter bypasses the warm-batch's per-snapshot syscalls (1352 → 0). Full breakdown + the regression we caught + fixed.
Non-up-to-date
node_modules(N virtual-store slots deleted per iteration):PR is consistently a small win and never a loss across the damage spectrum;
_broken_node_modulesdebug events fire correctly for each missing slot. Full sweep + caveats.Test plan
just ready— 633 tests pass, lint clean.just dylint— perfectionist clean.taplo format --check— clean.snapshot_deps_equal,integrity_equal,BrokenModulesLogwire shape,Lockfile::is_empty,save_current_to_virtual_store_dirround-trip / empty-deletes-file / ENOENT-as-none.install/tests.rs:warm_reinstall_skips_snapshot_when_current_lockfile_matches— proves the skip path short-circuits the fetch (the fixture points at a bogus URL; only the skip path makes the install succeed).warm_reinstall_emits_broken_modules_when_dir_is_missing— asserts the_broken_node_modulesdebug event fires before the fetch.warm_reinstall_reports_added_zero_and_emits_no_imported_events— assertspnpm:stats.added: 0and zeropnpm:progress importedevents when the skip path drops every snapshot from the install graph.context_log_reflects_current_lockfile_after_first_install—pnpm:context.currentLockfileExistsflipsfalse→trueoncelock.yamlis on disk, andlock.yamlis asserted present on disk after the first install (exercises the real write path).Out of scope (tracked in #433)
pacquet install --frozen-lockfile#431): once landed, the write site must narrow to the filtered lockfile (selected importers × engine filter). For now we write the wanted lockfile unmodified — a checkbox in the issue description.pacquet install --frozen-lockfile#432): the per-snapshot skip gains a.pnpm-needs-buildmarker check. The skip helper is structured so adding it is a small extension.pruneVirtualStore).Written by an agent (Claude Code, claude-opus-4-7).