Conversation
Add a `satisfies_package_manifest` check to pacquet's frozen-lockfile dispatcher so a stale `pnpm-lock.yaml` no longer silently installs the wrong shape of `node_modules`. Mirrors upstream's gate at [pkg-manager/core/src/install/index.ts:808-832](https://github.com/pnpm/pnpm/blob/94240bc046/pkg-manager/core/src/install/index.ts#L808-L832): before linking, compare the lockfile importer entry against the on-disk `package.json` and fail with `ERR_PNPM_OUTDATED_LOCKFILE` when they diverge. The check (ported from upstream's [`satisfiesPackageManifest`](https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/verification/src/satisfiesPackageManifest.ts)) runs two passes: 1. Flat-record specifier diff over `dependencies ∪ devDependencies ∪ optionalDependencies`. Catches added / removed / modified deps in one bucket; renders as a `SpecDiff` listing each entry per category so the user sees exactly what drifted. 2. Per-field name-set + specifier match. Catches the same-name-same-specifier-moved-between-buckets case the flat- record diff doesn't see (e.g. `react` moved from `dependencies` to `devDependencies`). Reasons are surfaced as typed `StalenessReason` variants so `pacquet-package-manager`'s `InstallError::OutdatedLockfile` forwards a structured detail rather than a parsed string. The new `InstallError::NoImporter` distinguishes "lockfile file is missing" (`NoLockfile`) from "lockfile is present but has no importer entry for this project." Scoped to what pacquet supports today — no catalogs, no auto- install-peers pre-pass, no `excludeLinksFromLockfile`, no semver- range satisfies check (covered in pnpm's `localTarballDepsAreUpToDate` which lives outside this gate). Multi-importer support comes when workspace install (#431) lands. Closes #447.
📝 WalkthroughWalkthroughAdds a lockfile freshness verifier (StalenessReason, SpecDiff, satisfies_package_manifest), exports it from the lockfile crate, gates frozen-lockfile installs with an early freshness check returning new InstallError variants, and adds unit + integration tests covering mismatches and formatting. ChangesLockfile Freshness Verification
Sequence Diagram(s)sequenceDiagram
participant Manifest as PackageManifest
participant Importer as Lockfile.importers["."]
participant Checker as satisfies_package_manifest
participant Install as Install::run (frozen)
participant Result as InstallError / Ok
Install->>Checker: validate importer vs manifest
Checker->>Importer: read dependency specifiers, publishDirectory, dependenciesMeta
Checker->>Manifest: read dependencies/dev/optional and publishConfig, dependenciesMeta
Checker->>Checker: compute flat-union diff via diff_flat_records
alt diff non-empty
Checker-->>Install: Err(OutdatedLockfile { SpecifiersDiffer })
else
Checker->>Checker: publishDirectory / dependenciesMeta / per-field checks
alt mismatch
Checker-->>Install: Err(OutdatedLockfile { reason })
else
Checker-->>Install: Ok(())
Install->>Install: proceed with frozen install
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
There was a problem hiding this comment.
Pull request overview
Adds a lockfile “freshness” gate to the --frozen-lockfile install path so pacquet rejects stale pnpm-lock.yaml when package.json drifts, mirroring pnpm’s ERR_PNPM_OUTDATED_LOCKFILE behavior to prevent silently installing the wrong node_modules shape.
Changes:
- Introduces
pacquet_lockfile::satisfies_package_manifest+StalenessReasonto detect manifest/lockfile drift. - Wires the freshness check into
Install::runfor--frozen-lockfile, addingOutdatedLockfileandNoImporterinstall errors. - Updates/extends package-manager integration tests and adds lockfile-crate unit tests for the new verification behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/package-manager/src/install.rs | Adds frozen-lockfile freshness dispatch check and new error variants/diagnostics. |
| crates/package-manager/src/install/tests.rs | Updates existing fixtures for the new check and adds integration coverage for outdated lockfile / missing importer. |
| crates/lockfile/src/lib.rs | Exposes the new freshness module from the lockfile crate. |
| crates/lockfile/src/freshness.rs | Implements the manifest-vs-importer verification logic and user-facing diff formatting. |
| crates/lockfile/src/freshness/tests.rs | Adds unit tests covering specifier diffs, field-move detection, and diff display formatting. |
Comments suppressed due to low confidence (3)
crates/package-manager/src/install/tests.rs:855
- This test mutates the manifest in-memory via
add_dependency(...)but never callsmanifest.save(). Other tests in this file save after mutation, and the comment mentions the on-disk manifest matching the lockfile; consider saving to keep the fixture accurate and avoid future regressions if code starts re-readingpackage.jsonfrom disk.
let manifest_path = dir.path().join("package.json");
let mut manifest = PackageManifest::create_if_needed(manifest_path).unwrap();
// Manifest must match `PARTIAL_INSTALL_LOCKFILE` — the freshness
// check (#447) rejects any drift between the on-disk manifest and
// the lockfile importer entry.
manifest.add_dependency("placeholder", "1.0.0", DependencyGroup::Prod).unwrap();
crates/package-manager/src/install/tests.rs:946
- This test mutates the manifest in-memory via
add_dependency(...)but never callsmanifest.save(). Other tests in this file save after mutation, and the comment mentions the on-disk manifest matching the lockfile; consider saving to keep the fixture accurate and avoid future regressions if code starts re-readingpackage.jsonfrom disk.
let manifest_path = dir.path().join("package.json");
let mut manifest = PackageManifest::create_if_needed(manifest_path).unwrap();
// Manifest must match the fixture lockfile below — the freshness
// check (#447) rejects any drift between the on-disk manifest and
// the lockfile importer entry.
manifest.add_dependency("placeholder", "1.0.0", DependencyGroup::Prod).unwrap();
crates/package-manager/src/install/tests.rs:1081
- This test mutates the manifest in-memory via
add_dependency(...)but never callsmanifest.save(). Other tests in this file save after mutation, and the comment mentions the on-disk manifest matching the lockfile; consider saving to keep the fixture accurate and avoid future regressions if code starts re-readingpackage.jsonfrom disk.
let manifest_path = dir.path().join("package.json");
let mut manifest = PackageManifest::create_if_needed(manifest_path).unwrap();
// Manifest must match `PARTIAL_INSTALL_LOCKFILE` — the freshness
// check (#447) rejects any drift between the on-disk manifest and
// the lockfile importer entry.
manifest.add_dependency("placeholder", "1.0.0", DependencyGroup::Prod).unwrap();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #450 +/- ##
==========================================
- Coverage 87.45% 87.29% -0.16%
==========================================
Files 100 106 +6
Lines 7865 8674 +809
==========================================
+ Hits 6878 7572 +694
- Misses 987 1102 +115 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Micro-Benchmark ResultsLinux |
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.5327158177399998,
"stddev": 0.07497244829459591,
"median": 2.5149506419399996,
"user": 2.55801202,
"system": 3.53160684,
"min": 2.44438707394,
"max": 2.6794006069400003,
"times": [
2.55301756394,
2.6098870019400002,
2.44438707394,
2.58478866494,
2.5083779969399997,
2.45699999294,
2.6794006069400003,
2.51353692994,
2.46039799194,
2.51636435394
]
},
{
"command": "pacquet@main",
"mean": 2.47458971224,
"stddev": 0.049289145065392274,
"median": 2.4643615524399998,
"user": 2.5250587199999996,
"system": 3.53162684,
"min": 2.41128639294,
"max": 2.56478246994,
"times": [
2.41708554794,
2.51161734394,
2.43764689494,
2.56478246994,
2.46919996094,
2.45952314394,
2.41128639294,
2.49028027494,
2.52798003994,
2.45649505294
]
},
{
"command": "pnpm",
"mean": 6.10968167944,
"stddev": 0.0505406393246956,
"median": 6.128259635940001,
"user": 8.96442782,
"system": 4.47755274,
"min": 6.025029534940001,
"max": 6.17159623894,
"times": [
6.17159623894,
6.1664798129400005,
6.133540940940001,
6.136848956940001,
6.13362521094,
6.03521384494,
6.08854835594,
6.082955566940001,
6.1229783309400005,
6.025029534940001
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6900262016200001,
"stddev": 0.05793580520588494,
"median": 0.67194470112,
"user": 0.35179118000000004,
"system": 1.4698748399999997,
"min": 0.6427830146200001,
"max": 0.83853711362,
"times": [
0.83853711362,
0.71770209762,
0.6808156816200001,
0.66307372062,
0.6427830146200001,
0.7063021556200001,
0.6552067876200001,
0.65191260862,
0.65244541962,
0.69148341662
]
},
{
"command": "pacquet@main",
"mean": 0.6967051121200001,
"stddev": 0.07556155265138503,
"median": 0.6683400421200001,
"user": 0.35486848000000004,
"system": 1.4752084399999998,
"min": 0.6412476796200001,
"max": 0.8854265506200001,
"times": [
0.76425755762,
0.6412476796200001,
0.70637077962,
0.65407070162,
0.67776452862,
0.65062533562,
0.8854265506200001,
0.66975009062,
0.6506079036200001,
0.66692999362
]
},
{
"command": "pnpm",
"mean": 2.5884886469199997,
"stddev": 0.10032379400323758,
"median": 2.55955194762,
"user": 3.10201578,
"system": 2.2463415400000004,
"min": 2.51085352562,
"max": 2.82839096762,
"times": [
2.55168971662,
2.57568341662,
2.66029580962,
2.64705971962,
2.5124757396199997,
2.51196928762,
2.56741417862,
2.51085352562,
2.51905410762,
2.82839096762
]
}
]
} |
Seven fixes from Copilot review on #450: - Per-field check now runs unconditionally so it catches same- cardinality cross-field swaps (e.g. lockfile prod={a}, dev={b} vs manifest prod={b}, dev={a}). The flat-union diff matches in this case because the union stays identical — only the per-field comparison sees the field-level mismatch. - Implemented `publishDirectory` and `dependenciesMeta` checks the module had declared in `StalenessReason` but never emitted. Mirrors upstream's `satisfiesPackageManifest.ts:51-58`. `dependenciesMeta` treats absent and empty-object as equivalent (matching upstream's `?? {}` coercion). - `SpecDiff::Display` now uses singular wording when n==1 ("1 dependency was added" instead of "1 dependencies were added"). - `OutdatedLockfile`'s help text no longer suggests `pacquet install --no-frozen-lockfile` (which doesn't exist); points users at `pnpm install --lockfile-only` to regenerate. - `NoImporter` messages now use `importers["."]` formatting instead of `{:?}` debug-format quoting that rendered short keys awkwardly as `importers."."`. Applied to both the `StalenessReason::NoImporter` and `InstallError::NoImporter` display impls. - Test sites that mutate the manifest in-memory via `add_dependency` now also call `manifest.save()`, in case future code paths start re-reading `package.json` from disk. Added three new tests pinning the new behaviors: `cross_field_swap_with_same_cardinalities_caught_by_per_field_check`, `publish_directory_mismatch_returns_publish_directory_mismatch`, `dependencies_meta_mismatch_returns_dependencies_meta_mismatch`, `no_importer_message_uses_bracket_quoted_id`, `spec_diff_display_uses_singular_for_count_of_one`. Updated the existing `spec_diff_display_lists_added_removed_modified` test to use n=2 for the plural arm so the pluralization wording is also pinned.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/lockfile/src/freshness/tests.rs (2)
203-209: ⚡ Quick winUpdate pnpm reference to
blob/mainThe upstream link in this block is SHA-pinned. Please point it to
https://github.com/pnpm/pnpm/blob/main/...to keep references aligned with the repo’s pnpm-porting convention.As per coding guidelines: "If reading on GitHub, open files from
https://github.com/pnpm/pnpm/blob/main/...... rather than from a permalinked SHA."🤖 Prompt for 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. In `@crates/lockfile/src/freshness/tests.rs` around lines 203 - 209, Update the upstream pnpm link in the test comment (the block describing per-field follow-up loop and referencing satisfiesPackageManifest) to use the main branch URL by replacing "https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/verification/src/satisfiesPackageManifest.ts#L67-L100" with "https://github.com/pnpm/pnpm/blob/main/lockfile/verification/src/satisfiesPackageManifest.ts#L67-L100" so the reference follows the repo convention; ensure only the URL is changed in that comment near the test declaration.
192-201: ⚡ Quick winAlign this test’s name with what it actually verifies
missing_importer_returns_no_importercurrently only assertsroot_project().is_none()and does not validate aNoImporterreason mapping path. Either rename the test to match behavior or extend it to assert the caller-levelNoImporteroutcome.🤖 Prompt for 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. In `@crates/lockfile/src/freshness/tests.rs` around lines 192 - 201, The test name missing_importer_returns_no_importer is misleading because it only checks lockfile.root_project().is_none(); rename the test function to missing_importer_returns_none (or another name that reflects it simply asserts None) by updating the function identifier in the test, so the name matches the actual assertion; alternatively, if you prefer to keep the original name, extend the test to simulate the caller-level behavior (callers that convert None into a NoImporter reason) and assert that the resulting reason equals NoImporter.crates/lockfile/src/freshness.rs (1)
11-176: ⚡ Quick winUse
pnpmblob/mainlinks in Rustdoc referencesSeveral upstream references (for example at Line 12 and Line 169) are pinned to a specific SHA. Please switch them to
https://github.com/pnpm/pnpm/blob/main/...so future ports always point to the current source-of-truth.As per coding guidelines: "When porting features, bug fixes, or behavior changes from pnpm... open files from
https://github.com/pnpm/pnpm/blob/main/...rather than from a permalinked SHA."🤖 Prompt for 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. In `@crates/lockfile/src/freshness.rs` around lines 11 - 176, Update the hardcoded SHA links in the module-level Rustdoc to use the canonical blob/main paths (replace occurrences like the `satisfiesPackageManifest` upstream link and any other `https://github.com/pnpm/pnpm/blob/<SHA>/...` references with `https://github.com/pnpm/pnpm/blob/main/...`); search for the doc comments that reference `satisfiesPackageManifest` and the lines around the `StalenessReason` and function comments to update every permalinked URL to the `blob/main` form so future ports reference the current upstream source.
🤖 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.
Nitpick comments:
In `@crates/lockfile/src/freshness.rs`:
- Around line 11-176: Update the hardcoded SHA links in the module-level Rustdoc
to use the canonical blob/main paths (replace occurrences like the
`satisfiesPackageManifest` upstream link and any other
`https://github.com/pnpm/pnpm/blob/<SHA>/...` references with
`https://github.com/pnpm/pnpm/blob/main/...`); search for the doc comments that
reference `satisfiesPackageManifest` and the lines around the `StalenessReason`
and function comments to update every permalinked URL to the `blob/main` form so
future ports reference the current upstream source.
In `@crates/lockfile/src/freshness/tests.rs`:
- Around line 203-209: Update the upstream pnpm link in the test comment (the
block describing per-field follow-up loop and referencing
satisfiesPackageManifest) to use the main branch URL by replacing
"https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/verification/src/satisfiesPackageManifest.ts#L67-L100"
with
"https://github.com/pnpm/pnpm/blob/main/lockfile/verification/src/satisfiesPackageManifest.ts#L67-L100"
so the reference follows the repo convention; ensure only the URL is changed in
that comment near the test declaration.
- Around line 192-201: The test name missing_importer_returns_no_importer is
misleading because it only checks lockfile.root_project().is_none(); rename the
test function to missing_importer_returns_none (or another name that reflects it
simply asserts None) by updating the function identifier in the test, so the
name matches the actual assertion; alternatively, if you prefer to keep the
original name, extend the test to simulate the caller-level behavior (callers
that convert None into a NoImporter reason) and assert that the resulting reason
equals NoImporter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 50b49b07-65c9-4a5d-b9ab-6ebeefa9d164
📒 Files selected for processing (4)
crates/lockfile/src/freshness.rscrates/lockfile/src/freshness/tests.rscrates/package-manager/src/install.rscrates/package-manager/src/install/tests.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/package-manager/src/install.rs
- crates/package-manager/src/install/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). (5)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Run benchmark on ubuntu-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/lockfile/src/freshness.rscrates/lockfile/src/freshness/tests.rs
🧠 Learnings (2)
📚 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/lockfile/src/freshness.rscrates/lockfile/src/freshness/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 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/freshness/tests.rs
🔇 Additional comments (2)
crates/lockfile/src/freshness.rs (1)
177-377: LGTM!crates/lockfile/src/freshness/tests.rs (1)
1-191: LGTM!Also applies to: 210-392
`[SpecDiff::Display]` doesn't resolve — `Display` is a trait impl, not an associated item — so rustdoc fails with `unresolved link` under `-D rustdoc::broken-intra-doc-links` (caught by the Doc CI job, not `just ready` locally). Refer to it as "[SpecDiff]'s Display impl" instead.
Surveyed upstream's `satisfiesPackageManifest` test file (<https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/verification/test/satisfiesPackageManifest.ts>) and found one behavioral gap and three uncovered cases. **Behavioral gap.** Upstream's per-field check filters a dep out of the `devDependencies` check when it also appears in `dependencies` or `optionalDependencies` (precedence: optional > prod > dev). My port did not, so a manifest listing the same dep in both prod and dev fields would incorrectly fail the dev-field check against a lockfile that records it under prod only. Add the filter at the per-field loop in `freshness.rs`. **Three previously-uncovered upstream test cases ported:** - `dependencies_meta_empty_object_equivalent_to_absent` — `dependenciesMeta: {}` on manifest with no `dependenciesMeta` on importer satisfies (the absent/empty equivalence already worked, just wasn't pinned by a test). - `publish_directory_match_satisfies` — happy path for publishDirectory (previously only the mismatch case was tested). - `same_dep_in_prod_and_dev_counts_under_prod` and `same_dep_in_prod_and_optional_counts_under_optional` — the precedence-filter behavior fixed above. - `importer_empty_dev_dependencies_equivalent_to_absent` — the importer side's `Some(empty)` vs `None` equivalence. Tests upstream covers that remain out of scope for pacquet: `link:` deps (#431), `autoInstallPeers` peer-folding (pacquet has no separate auto-install-peers mode), `excludeLinksFromLockfile`, semver-range satisfies check (lives in upstream's `localTarballDepsAreUpToDate`, out of scope per #447). 722 tests now run (+5 from the porting). All checks green (`just ready`, `just dylint`, `cargo doc -D warnings`, `taplo`).
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 #447. Adds a
satisfies_package_manifestcheck to pacquet's frozen-lockfile dispatcher so a stalepnpm-lock.yamlno longer silently installs the wrong shape ofnode_modules.Before this,
pacquet install --frozen-lockfilewould happily materialize whatever the lockfile said, even when a dev had editedpackage.json(added, removed, or bumped a dep) without re-running the resolver. Upstream pnpm catches this at the dispatch site withERR_PNPM_OUTDATED_LOCKFILE(pkg-manager/core/src/install/index.ts:808-832); this PR ports the same gate.What's checked
Ported from upstream's
satisfiesPackageManifest. Four phases, short-circuiting on the first failure:dependencies ∪ devDependencies ∪ optionalDependencies. Catches added/removed/modified deps in one bucket; rendered as aSpecDiffwith per-bucket lists.publishDirectoryparity between the importer entry and the manifest'spublishConfig.directory.dependenciesMetaJSON equality between the importer and the manifest (with absent ≡ empty-object equivalence to match upstream's?? {}coercion).reactmoved fromdependenciestodevDependencies). Applies upstream's precedence rule (optional>prod>dev): a dep that appears in a higher-precedence manifest field is filtered out of the lower-precedence field's check, so a manifest listing the same dep in both prod and dev still satisfies a lockfile that records it under prod only.Reasons are surfaced as typed
StalenessReasonvariants (SpecifiersDiffer,DepSpecifierMismatch,PublishDirectoryMismatch,DependenciesMetaMismatch,NoImporter) so callers match on the discriminant rather than parsing format strings, and tests assert on shape rather than wording. TheSpecDiff::Displayimpl handles singular/plural ("1 dependency was added" vs "2 dependencies were added") so user-facing CI output reads cleanly.New errors
InstallError::OutdatedLockfile { reason: StalenessReason }— surfaced asERR_PNPM_OUTDATED_LOCKFILE(miette codepacquet_package_manager::outdated_lockfile). Hint points atpnpm install --lockfile-onlyto regenerate.InstallError::NoImporter { importer_id }— distinguishes "lockfile file is missing" (NoLockfile) from "lockfile is present but has no importer entry for this project." Renders asimporters["{id}"]for readability.Performance
Confirmed by hyperfine on a 1352-package fixture with 110-dep manifest (warm reinstall,
--warmup 2 --runs 10):Ratio 1.00 ± 0.05 — within noise. The check is pure-CPU map/set operations on string keys with no syscalls or async; ~50-200 μs for the alot7 manifest.
Out of scope (matching the issue)
auto-install-peerspre-pass (pacquet has no separate auto-install-peers mode).excludeLinksFromLockfileandlink:resolutions (Add workspace support topacquet install --frozen-lockfile#431 territory).localTarballDepsAreUpToDate, outside this gate).pacquet install --frozen-lockfile#431 workspace install — single-importer-only here).Test plan
just ready— 722 tests pass, lint clean.just dylint— perfectionist clean.taplo format --check— clean.RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --workspace --locked— clean.crates/lockfile/src/freshness/tests.rs(13 tests) covering:dependenciesMeta: {}≡ absent,publishDirectorymatch, importer empty-vs-absent dev-deps equivalence, same dep inprod+dev(precedence), same dep inprod+optional(precedence).publishDirectorymismatch,dependenciesMetamismatch.SpecDiff::Displayplural and singular wording,NoImporterbracket-quoted id formatting.crates/package-manager/src/install/tests.rs(2 new):frozen_lockfile_errors_when_manifest_drifts_from_lockfile— drifted manifest fails withOutdatedLockfilebefore any fetch attempt. Fixture uses a bogus tarball URL so a fetch attempt would also fail — only the earlyOutdatedLockfilemakes the test pass cleanly.frozen_lockfile_errors_when_lockfile_has_no_root_importer— well-formed lockfile with emptyimportersmap surfaces asNoImporterfor..PARTIAL_INSTALL_LOCKFILEupdated to populate the manifest with the matchingplaceholderdep so they don't trip the new check.Upstream test coverage audit
Cross-checked against upstream's
satisfiesPackageManifest.test.ts(21 assertions). All applicable cases ported; cases left out are the ones in "Out of scope" above (link:,autoInstallPeers, semver-range satisfies). Caught one behavioral gap in the process: my port was missing the precedence filter on the per-field check, which would have failed a manifest listing the same dep in bothdependenciesanddevDependencies. Fixed before merge.Written by an agent (Claude Code, claude-opus-4-7).