Skip to content

fix(pacquet): close four lockfile-parity gaps (importer peer+dev merge, peer-vs-own-dep, peer overrides, optional-peer hoist)#12345

Merged
zkochan merged 7 commits into
mainfrom
lockfile-parity2
Jun 12, 2026
Merged

fix(pacquet): close four lockfile-parity gaps (importer peer+dev merge, peer-vs-own-dep, peer overrides, optional-peer hoist)#12345
zkochan merged 7 commits into
mainfrom
lockfile-parity2

Conversation

@zkochan

@zkochan zkochan commented Jun 12, 2026

Copy link
Copy Markdown
Member

Summary

Four lockfile-parity fixes for pacquet, continuing #12266. Measured on the monorepo itself (fresh state, install --lockfile-only, back-to-back against the live registry vs pnpm 11.6.0), the real-lockfile document diff drops from 806 to 461 changed lines. Two consecutive pacquet runs are byte-identical before and after.

1. Importer's regular dep wins over its own peer range (fix(deps-resolver))

An importer that declares the same alias in both peerDependencies and a regular group (e.g. @pnpm/logger: workspace:* devDependency + catalog: peer — 67 importers in this repo) resolved both specs, and the peer's registry resolution overwrote the workspace link in the importers section (version: 1001.0.1 instead of link:../../core/logger, 182 diff lines). importer_direct_wanted_specs now merges the groups the way pnpm spreads them in getWantedDependencies: peers first, then dependencies < devDependencies < optionalDependencies, one wanted spec per alias. The @pnpm/logger resolution tallies now match pnpm's exactly.

2. A package's peer survives when it also lists the name as a dependency (fix(deps-resolver))

Under autoInstallPeers, pnpm removes peer-shadowed names from a resolved package's dependencies before extracting peers (resolveDependencies.ts#L1527-L1542), so the peer edge supplies the package and the depPath carries the suffix. pacquet did the inverse (dropped the peer, walked the own dep), so @babel/parser — whose packageExtensions-added @babel/types peer is also its regular dependency — lost its (@babel/types@7.29.7) suffix and its peerDependencies: block. Also aligns extract_peer_dependencies with peerDependenciesWithoutOwn: the package's own name counts as an own dep, and a peerDependenciesMeta-only entry becomes a peer only when optional: true. The non-autoInstallPeers arm (omit only peers resolvable from the parent scope) is not ported — pacquet's per-package children cache has no parent context; behavior there matches pnpm whenever the peer is not in scope.

3. Overrides apply to peerDependencies (fix(package-manager))

Ports the peer arm of overrideDepsOfPkg: a matched peer is deleted on -, rewritten in place when the override value is a valid peer range, otherwise written into dependencies while the declared peer stays. Fixes e.g. ajv@>=7.0.0-alpha.0 <8.18.0 → >=8.18.0 not rewriting peer ranges and @yarnpkg/libzip losing its (@yarnpkg/fslib@3.x) suffix.

4. Optional-peer hoist: run-extended preferred versions, meta-only peers excluded (fix(deps-resolver))

Corrects the model from #12267. pnpm folds every run-resolved version into ctx.allPreferredVersions (resolveDependencies.ts#L1483-L1488, since #7812) and getHoistableOptionalPeers reads that map after each wave — so an optional peer with a real peerDependencies entry (eslint's jiti) resolves against a provider anywhere in the freshly resolved tree. What keeps debug's supports-color bare is not a static map but the missing-peer set: getMissingPeers iterates peerDependencies only, so a meta-only peer never feeds the hoist. Both behaviors verified empirically against pnpm 11.6.0 (eslint + cosmiconfig-typescript-loader hoists jiti@2.6.1; debug + concurrently stays bare). pacquet's static-snapshot approach got the meta-only case right by the wrong mechanism and under-hoisted every real-entry optional peer — the missing (jiti@2.6.1), (typanion@3.14.0), (conventional-commits-parser@6.4.0), (@types/node@…) suffixes and their cascades on the monorepo. The 12267-era regression test asserted the anti-pnpm behavior for the real-entry shape and was inverted; a new test pins the meta-only shape.

Verification

  • New unit tests in pacquet-resolving-deps-resolver (importer group merge ×4, peer-vs-own-dep shadowing ×3, optional-peer hoist real-entry/meta-only ×2) and pacquet-package-manager (peer overrides ×4).
  • Full workspace suite: 3218 tests pass; clippy --deny warnings clean; rustfmt + typos clean.
  • Whole-monorepo lockfile diff vs fresh pnpm 11.6.0: 806 → 461 changed lines; @pnpm/logger, jiti, typanion, @babel/types, ajv categories eliminated. Two consecutive pacquet runs byte-identical.

Remaining gaps (tracked in #12266)

The surviving 461 lines are dominated by a hoisted optional-peer version-pick divergence (@types/node 22.19.21 vs 25.9.3, spdx-expression-parse 4.0.0 vs 3.0.1 — pacquet also emits duplicate suffix variants of the same snapshot), plus small items: es-abstract/varint version picks, the array-form engines quirk, and the env-lockfile document's missing packageManagerDependencies block and libc: fields.


Written by an agent (Claude Code, claude-fable-5).

Summary by CodeRabbit

  • New Features

    • Added support for peer dependency overrides compatible with pnpm configuration.
    • Added auto-install peers configuration option for automatic peer dependency resolution.
  • Improvements

    • Enhanced peer dependency hoisting logic for optional and metadata-only peer entries.
    • Improved peer dependency resolution and metadata handling.

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (2)

Context used

Grey Divider


Action required

1. Optional peers hoisted as direct deps 📎 Requirement gap ≡ Correctness
Description
get_hoistable_optional_peers results are converted into new wanted specs with optional=false and
then installed at the importer level, materializing optional peers as regular direct dependencies.
This can cause lockfile entries to diverge from pnpm’s representation for optional peers, which
should rely on peer suffixes and transitivePeerDependencies instead of direct dependencies
entries.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[R383-387]

+        let hoisted_optional =
+            get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions);
    if hoisted_optional.is_empty() {
        break;
    }
Evidence
PR Compliance ID 3 forbids representing optional peers as regular direct dependencies in the
lockfile. The new code explicitly turns hoistable optional peers into importer wanted specs with
optional=false and extends the tree with them, which materializes them as direct deps.

Do not materialize resolved optional peers as direct dependencies (match pnpm representation)
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The optional-peer hoist path currently installs hoisted optional peers as importer-level direct dependencies (`optional=false`), which violates the requirement to not materialize resolved optional peers as direct `dependencies` entries.
## Issue Context
The compliance requirement (PR Compliance ID 3) expects optional peers to be represented via peer suffixes and `transitivePeerDependencies`, matching pnpm’s lockfile representation, rather than being added as regular direct dependencies.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Dash override resurrects peer 🐞 Bug ≡ Correctness
Description
override_peer_group removes a peer from peerDependencies when the override value is "-" but
does not remove the corresponding peerDependenciesMeta entry, so the peer can reappear as a
meta-only optional "*" peer. This defeats the intended meaning of a dash override and can change
the resolved graph/lockfile output.
Code

pacquet/crates/package-manager/src/overrides.rs[R249-255]

+            if chosen.inner.new_bare_specifier == "-" {
+                if let Some(peers) =
+                    value.get_mut("peerDependencies").and_then(Value::as_object_mut)
+                {
+                    peers.remove(&name);
+                }
+                continue;
Evidence
The override code deletes only from peerDependencies, while peer extraction explicitly recreates a
peer from peerDependenciesMeta (optional:true) when no peerDependencies entry exists, so a dash
override can be negated by leftover metadata.

pacquet/crates/package-manager/src/overrides.rs[244-256]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When a peer override is `"-"`, `override_peer_group` removes the entry from `peerDependencies` but leaves any corresponding `peerDependenciesMeta[name]` entry intact.
Because `extract_peer_dependencies` treats `peerDependenciesMeta[name].optional == true` without a matching `peerDependencies[name]` as a real meta-only optional peer (`version: "*"`), the deleted peer can reappear later.
## Issue Context
- `override_peer_group` handles dash deletion by removing from `peerDependencies` only.
- Peer extraction will materialize optional meta-only peers as `PeerDep { version: "*", optional: true, meta_only: true }`.
## Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[244-256]
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]
### Concrete fix
In the `new_bare_specifier == "-"` branch, also remove the key from `peerDependenciesMeta` (and optionally remove the whole `peerDependenciesMeta` object if it becomes empty). This ensures the dash override fully deletes the peer requirement rather than turning it into a meta-only optional peer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Workspace peer override mismatch 🐞 Bug ≡ Correctness
Description
Peer overrides are matched via semver range intersection, but peer dependency ranges may be
workspace:-prefixed; those specs won’t parse as semver so choose_override won’t match and the
override will silently not apply.
Code

pacquet/crates/package-manager/src/overrides.rs[R329-331]

/// Mirrors upstream's `targetPkg.name === name && isIntersectingRange(targetPkg.bareSpecifier, bareSpecifier)`.
fn matches_target(target: &PackageSelector, dep_name: &str, dep_spec: &str) -> bool {
   target.name == dep_name && is_intersecting_range(target.bare_specifier.as_deref(), dep_spec)
Evidence
The codebase explicitly documents and implements workspace: prefix stripping for peer semver
matching, but the overrides matcher passes the raw dep spec into a semver parser; therefore
workspace:-prefixed peer specs won’t match version-scoped overrides.

pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs[162-181]
pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[1066-1069]
pacquet/crates/package-manager/src/overrides.rs[329-379]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`VersionsOverrider::override_peer_group` relies on `choose_override(...)`, which matches using `matches_target(...)` → `is_intersecting_range(...)`. That intersection code semver-parses the dependency spec string. However, peer dependency specs may legitimately start with `workspace:` (and the peer resolver already strips this prefix before semver checks). As a result, peer overrides won’t apply when a peer is declared as `workspace:^1.2.3`, causing incorrect override behavior and lockfile parity drift.
### Issue Context
- Peer resolution treats `workspace:` as a prefix that should be removed for semver matching.
- The override matcher does not perform the same normalization, so semver parsing fails and the override is skipped.
### Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[219-280]
- pacquet/crates/package-manager/src/overrides.rs[329-379]
### Suggested fix
Before calling `choose_override(...)` from the peer-deps path (or inside `matches_target` / `is_intersecting_range`), normalize the peer spec used for intersection matching, e.g.:
- Strip a leading `workspace:` prefix from `dep_spec` when matching overrides.
- Keep the original raw spec for actual mutation decisions if needed; only the *matching* input should be normalized.
Add/extend a unit test to cover:
- peerDependencies: { "foo": "workspace:^1.0.0" }
- override selector: "foo@<2" → new spec ">=2" (or any intersecting selector)
- assert the peerDependencies entry is rewritten / promoted as expected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Tests reference pnpm 11.6.0 📎 Requirement gap ≡ Correctness
Description
The updated optional-peer hoist tests explicitly state behavior was verified against pnpm 11.6.0,
but the compliance target requires lockfile parity with pnpm 11.5.2. This weakens assurance that the
implemented behavior matches the required pnpm version and may allow remaining semantic parity gaps
to slip through.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[R726-738]

+/// An optional peer with a real `peerDependencies` entry whose
+/// provider is resolved anywhere in the tree (here: deep under a
+/// sibling) IS hoisted: upstream folds every run-resolved version into
+/// `ctx.allPreferredVersions` and `getHoistableOptionalPeers` resolves
+/// the peer against it after the wave (verified against pnpm 11.6.0 —
+/// the `eslint` + `cosmiconfig-typescript-loader` shape, where
+/// `eslint` gains `(jiti@x)`). For
+/// <https://github.com/pnpm/pnpm/issues/12266>.
#[tokio::test]
-async fn optional_peer_only_in_resolved_tree_is_not_hoisted() {
+async fn optional_peer_with_real_entry_is_hoisted_from_resolved_tree() {
  let mut table = HashMap::new();
  table.insert(
      ("needs-opt".to_string(), "1.0.0".to_string()),
Evidence
PR Compliance ID 1 requires parity with pnpm 11.5.2, but the modified tests explicitly cite pnpm
11.6.0 as the verification baseline for the intended behavior, which does not satisfy the stated
compliance target version.

Lockfile-only output matches pnpm 11.5.2 for the same clean state (content parity, excluding env-doc)
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[726-733]
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[785-790]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires `pacquet install --lockfile-only` parity with pnpm **11.5.2**, but the newly added/updated tests document verification against pnpm **11.6.0**. This creates a compliance gap because the behavior may differ between pnpm versions.
## Issue Context
The checklist explicitly targets pnpm 11.5.2 for content parity, so tests and documentation should either:
- verify and cite 11.5.2 behavior, or
- include additional assertions/fixtures that lock in 11.5.2 semantics.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[726-790]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Optional bit mis-tagged 🐞 Bug ≡ Correctness
Description
importer_direct_wanted_specs sets WantedSpec.optional by checking whether an alias exists in
optionalDependencies, even when DependencyGroup::Optional is not included (e.g. --no-optional
runs with [Prod]). This can mis-classify a non-optional direct dep as optional, affecting the
wanted-dep cache key and optionality propagation during resolution.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[R338-361]

  let optional_names = importer_optional_dependency_names(manifest);
  let injected_names = importer_injected_dependency_names(manifest);
-    let mut wanted: Vec<WantedSpec> = Vec::new();
+    let mut order: Vec<&str> = Vec::new();
+    let mut ranges: HashMap<&str, &str> = HashMap::new();
  for (name, range) in manifest.dependencies(groups) {
      if !crate::is_valid_dependency_alias(name) {
          return Err(ResolveDependencyTreeError::InvalidDependencyName {
              parent: "The current package".to_string(),
              alias: name.to_string(),
          });
      }
-        let optional = optional_names.contains(name);
-        let injected = injected_names.contains(name);
-        wanted.push((name.to_string(), range.to_string(), optional, injected));
+        if ranges.insert(name, range).is_none() {
+            order.push(name);
+        }
  }
+    let wanted: Vec<WantedSpec> = order
+        .into_iter()
+        .map(|name| {
+            (
+                name.to_string(),
+                ranges[name].to_string(),
+                optional_names.contains(name),
+                injected_names.contains(name),
+            )
Evidence
The function always builds optional_names from the manifest’s optionalDependencies and then uses
it for every wanted alias, regardless of which dependency groups are included for the current run.
The repo also demonstrates real executions where optional deps are excluded by passing only
[DependencyGroup::Prod] (the --no-optional shape), making this mis-tagging reachable in
practice; additionally wanted.optional is part of the wanted-dep cache key so incorrect tagging
changes resolution/caching behavior.

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[264-274]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[328-363]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[377-383]
pacquet/crates/package-manager/src/install/tests.rs[3229-3242]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`importer_direct_wanted_specs` marks a wanted dep as `optional` if its alias exists anywhere in `optionalDependencies`, even if the current install excluded optional dependencies (i.e. `DependencyGroup::Optional` is not in `dependency_groups`). This can incorrectly set `WantedDependency.optional` for deps that are actually coming from `dependencies`/`devDependencies` in that run.
### Issue Context
This is observable when an alias appears in both `dependencies` (or `devDependencies`) and `optionalDependencies`, and the install is run with an included-group set that does **not** include `Optional` (the codebase explicitly uses `[DependencyGroup::Prod]` for `--no-optional`).
### Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[328-365]
### Suggested fix
Compute the `optional` flag from the group that actually contributed the final chosen specifier (i.e., track the “winning” group per alias while iterating groups in precedence order), or at minimum gate `optional_names` behind `included.contains(&DependencyGroup::Optional)` so optional tagging can’t happen when optional deps are excluded.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 46d2cc9

Results up to commit 69535c8


🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (2)

Context used

Action required
1. Optional peers hoisted as direct deps 📎 Requirement gap ≡ Correctness
Description
get_hoistable_optional_peers results are converted into new wanted specs with optional=false and
then installed at the importer level, materializing optional peers as regular direct dependencies.
This can cause lockfile entries to diverge from pnpm’s representation for optional peers, which
should rely on peer suffixes and transitivePeerDependencies instead of direct dependencies
entries.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[R383-387]

+        let hoisted_optional =
+            get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions);
     if hoisted_optional.is_empty() {
         break;
     }
Evidence
PR Compliance ID 3 forbids representing optional peers as regular direct dependencies in the
lockfile. The new code explicitly turns hoistable optional peers into importer wanted specs with
optional=false and extends the tree with them, which materializes them as direct deps.

Do not materialize resolved optional peers as direct dependencies (match pnpm representation)
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The optional-peer hoist path currently installs hoisted optional peers as importer-level direct dependencies (`optional=false`), which violates the requirement to not materialize resolved optional peers as direct `dependencies` entries.
## Issue Context
The compliance requirement (PR Compliance ID 3) expects optional peers to be represented via peer suffixes and `transitivePeerDependencies`, matching pnpm’s lockfile representation, rather than being added as regular direct dependencies.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Dash override resurrects peer 🐞 Bug ≡ Correctness
Description
override_peer_group removes a peer from peerDependencies when the override value is "-" but
does not remove the corresponding peerDependenciesMeta entry, so the peer can reappear as a
meta-only optional "*" peer. This defeats the intended meaning of a dash override and can change
the resolved graph/lockfile output.
Code

pacquet/crates/package-manager/src/overrides.rs[R249-255]

+            if chosen.inner.new_bare_specifier == "-" {
+                if let Some(peers) =
+                    value.get_mut("peerDependencies").and_then(Value::as_object_mut)
+                {
+                    peers.remove(&name);
+                }
+                continue;
Evidence
The override code deletes only from peerDependencies, while peer extraction explicitly recreates a
peer from peerDependenciesMeta (optional:true) when no peerDependencies entry exists, so a dash
override can be negated by leftover metadata.

pacquet/crates/package-manager/src/overrides.rs[244-256]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When a peer override is `"-"`, `override_peer_group` removes the entry from `peerDependencies` but leaves any corresponding `peerDependenciesMeta[name]` entry intact.
Because `extract_peer_dependencies` treats `peerDependenciesMeta[name].optional == true` without a matching `peerDependencies[name]` as a real meta-only optional peer (`version: "*"`), the deleted peer can reappear later.
## Issue Context
- `override_peer_group` handles dash deletion by removing from `peerDependencies` only.
- Peer extraction will materialize optional meta-only peers as `PeerDep { version: "*", optional: true, meta_only: true }`.
## Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[244-256]
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]
### Concrete fix
In the `new_bare_specifier == "-"` branch, also remove the key from `peerDependenciesMeta` (and optionally remove the whole `peerDependenciesMeta` object if it becomes empty). This ensures the dash override fully deletes the peer requirement rather than turning it into a meta-only optional peer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
3. Workspace peer override mismatch 🐞 Bug ≡ Correctness ⭐ New
Description
Peer overrides are matched via semver range intersection, but peer dependency ranges may be
workspace:-prefixed; those specs won’t parse as semver so choose_override won’t match and the
override will silently not apply.
Code

pacquet/crates/package-manager/src/overrides.rs[R329-331]

/// Mirrors upstream's `targetPkg.name === name && isIntersectingRange(targetPkg.bareSpecifier, bareSpecifier)`.
fn matches_target(target: &PackageSelector, dep_name: &str, dep_spec: &str) -> bool {
    target.name == dep_name && is_intersecting_range(target.bare_specifier.as_deref(), dep_spec)
Evidence
The codebase explicitly documents and implements workspace: prefix stripping for peer semver
matching, but the overrides matcher passes the raw dep spec into a semver parser; therefore
workspace:-prefixed peer specs won’t match version-scoped overrides.

pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs[162-181]
pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[1066-1069]
pacquet/crates/package-manager/src/overrides.rs[329-379]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`VersionsOverrider::override_peer_group` relies on `choose_override(...)`, which matches using `matches_target(...)` → `is_intersecting_range(...)`. That intersection code semver-parses the dependency spec string. However, peer dependency specs may legitimately start with `workspace:` (and the peer resolver already strips this prefix before semver checks). As a result, peer overrides won’t apply when a peer is declared as `workspace:^1.2.3`, causing incorrect override behavior and lockfile parity drift.

### Issue Context
- Peer resolution treats `workspace:` as a prefix that should be removed for semver matching.
- The override matcher does not perform the same normalization, so semver parsing fails and the override is skipped.

### Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[219-280]
- pacquet/crates/package-manager/src/overrides.rs[329-379]

### Suggested fix
Before calling `choose_override(...)` from the peer-deps path (or inside `matches_target` / `is_intersecting_range`), normalize the peer spec used for intersection matching, e.g.:
- Strip a leading `workspace:` prefix from `dep_spec` when matching overrides.
- Keep the original raw spec for actual mutation decisions if needed; only the *matching* input should be normalized.

Add/extend a unit test to cover:
- peerDependencies: { "foo": "workspace:^1.0.0" }
- override selector: "foo@<2" → new spec ">=2" (or any intersecting selector)
- assert the peerDependencies entry is rewritten / promoted as expected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Tests reference pnpm 11.6.0 📎 Requirement gap ≡ Correctness
Description
The updated optional-peer hoist tests explicitly state behavior was verified against pnpm 11.6.0,
but the compliance target requires lockfile parity with pnpm 11.5.2. This weakens assurance that the
implemented behavior matches the required pnpm version and may allow remaining semantic parity gaps
to slip through.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[R726-738]

+/// An optional peer with a real `peerDependencies` entry whose
+/// provider is resolved anywhere in the tree (here: deep under a
+/// sibling) IS hoisted: upstream folds every run-resolved version into
+/// `ctx.allPreferredVersions` and `getHoistableOptionalPeers` resolves
+/// the peer against it after the wave (verified against pnpm 11.6.0 —
+/// the `eslint` + `cosmiconfig-typescript-loader` shape, where
+/// `eslint` gains `(jiti@x)`). For
+/// <https://github.com/pnpm/pnpm/issues/12266>.
#[tokio::test]
-async fn optional_peer_only_in_resolved_tree_is_not_hoisted() {
+async fn optional_peer_with_real_entry_is_hoisted_from_resolved_tree() {
   let mut table = HashMap::new();
   table.insert(
       ("needs-opt".to_string(), "1.0.0".to_string()),
Evidence
PR Compliance ID 1 requires parity with pnpm 11.5.2, but the modified tests explicitly cite pnpm
11.6.0 as the verification baseline for the intended behavior, which does not satisfy the stated
compliance target version.

Lockfile-only output matches pnpm 11.5.2 for the same clean state (content parity, excluding env-doc)
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[726-733]
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[785-790]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires `pacquet install --lockfile-only` parity with pnpm **11.5.2**, but the newly added/updated tests document verification against pnpm **11.6.0**. This creates a compliance gap because the behavior may differ between pnpm versions.
## Issue Context
The checklist explicitly targets pnpm 11.5.2 for content parity, so tests and documentation should either:
- verify and cite 11.5.2 behavior, or
- include additional assertions/fixtures that lock in 11.5.2 semantics.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[726-790]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Optional bit mis-tagged 🐞 Bug ≡ Correctness
Description
importer_direct_wanted_specs sets WantedSpec.optional by checking whether an alias exists in
optionalDependencies, even when DependencyGroup::Optional is not included (e.g. --no-optional
runs with [Prod]). This can mis-classify a non-optional direct dep as optional, affecting the
wanted-dep cache key and optionality propagation during resolution.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[R338-361]

   let optional_names = importer_optional_dependency_names(manifest);
   let injected_names = importer_injected_dependency_names(manifest);
-    let mut wanted: Vec<WantedSpec> = Vec::new();
+    let mut order: Vec<&str> = Vec::new();
+    let mut ranges: HashMap<&str, &str> = HashMap::new();
   for (name, range) in manifest.dependencies(groups) {
       if !crate::is_valid_dependency_alias(name) {
           return Err(ResolveDependencyTreeError::InvalidDependencyName {
               parent: "The current package".to_string(),
               alias: name.to_string(),
           });
       }
-        let optional = optional_names.contains(name);
-        let injected = injected_names.contains(name);
-        wanted.push((name.to_string(), range.to_string(), optional, injected));
+        if ranges.insert(name, range).is_none() {
+            order.push(name);
+        }
   }
+    let wanted: Vec<WantedSpec> = order
+        .into_iter()
+        .map(|name| {
+            (
+                name.to_string(),
+                ranges[name].to_string(),
+                optional_names.contains(name),
+                injected_names.contains(name),
+            )
Evidence
The function always builds optional_names from the manifest’s optionalDependencies and then uses
it for every wanted alias, regardless of which dependency groups are included for the current run.
The repo also demonstrates real executions where optional deps are excluded by passing only
[DependencyGroup::Prod] (the --no-optional shape), making this mis-tagging reachable in
practice; additionally wanted.optional is part of the wanted-dep cache key so incorrect tagging
changes resolution/caching behavior.

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[264-274]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[328-363]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[377-383]
pacquet/crates/package-manager/src/install/tests.rs[3229-3242]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`importer_direct_wanted_specs` marks a wanted dep as `optional` if its alias exists anywhere in `optionalDependencies`, even if the current install excluded optional dependencies (i.e. `DependencyGroup::Optional` is not in `dependency_groups`). This can incorrectly set `WantedDependency.optional` for deps that are actually coming from `dependencies`/`devDependencies` in that run.
### Issue Context
This is observable when an alias appears in both `dependencies` (or `devDependencies`) and `optionalDependencies`, and the install is run with an included-group set that does **not** include `Optional` (the codebase explicitly uses `[DependencyGroup::Prod]` for `--no-optional`).
### Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[328-365]
### Suggested fix
Compute the `optional` flag from the group that actually contributed the final chosen specifier (i.e., track the “winning” group per alias while iterating groups in precedence order), or at minimum gate `optional_names` behind `included.contains(&DependencyGroup::Optional)` so optional tagging can’t happen when optional deps are excluded.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 43fd70f


🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (2)

Context used

Action required
1. Optional peers hoisted as direct deps 📎 Requirement gap ≡ Correctness
Description
get_hoistable_optional_peers results are converted into new wanted specs with optional=false and
then installed at the importer level, materializing optional peers as regular direct dependencies.
This can cause lockfile entries to diverge from pnpm’s representation for optional peers, which
should rely on peer suffixes and transitivePeerDependencies instead of direct dependencies
entries.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[R383-387]

+        let hoisted_optional =
+            get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions);
      if hoisted_optional.is_empty() {
          break;
      }
Evidence
PR Compliance ID 3 forbids representing optional peers as regular direct dependencies in the
lockfile. The new code explicitly turns hoistable optional peers into importer wanted specs with
optional=false and extends the tree with them, which materializes them as direct deps.

Do not materialize resolved optional peers as direct dependencies (match pnpm representation)
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The optional-peer hoist path currently installs hoisted optional peers as importer-level direct dependencies (`optional=false`), which violates the requirement to not materialize resolved optional peers as direct `dependencies` entries.
## Issue Context
The compliance requirement (PR Compliance ID 3) expects optional peers to be represented via peer suffixes and `transitivePeerDependencies`, matching pnpm’s lockfile representation, rather than being added as regular direct dependencies.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Dash override resurrects peer 🐞 Bug ≡ Correctness
Description
override_peer_group removes a peer from peerDependencies when the override value is "-" but
does not remove the corresponding peerDependenciesMeta entry, so the peer can reappear as a
meta-only optional "*" peer. This defeats the intended meaning of a dash override and can change
the resolved graph/lockfile output.
Code

pacquet/crates/package-manager/src/overrides.rs[R249-255]

+            if chosen.inner.new_bare_specifier == "-" {
+                if let Some(peers) =
+                    value.get_mut("peerDependencies").and_then(Value::as_object_mut)
+                {
+                    peers.remove(&name);
+                }
+                continue;
Evidence
The override code deletes only from peerDependencies, while peer extraction explicitly recreates a
peer from peerDependenciesMeta (optional:true) when no peerDependencies entry exists, so a dash
override can be negated by leftover metadata.

pacquet/crates/package-manager/src/overrides.rs[244-256]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When a peer override is `"-"`, `override_peer_group` removes the entry from `peerDependencies` but leaves any corresponding `peerDependenciesMeta[name]` entry intact.
Because `extract_peer_dependencies` treats `peerDependenciesMeta[name].optional == true` without a matching `peerDependencies[name]` as a real meta-only optional peer (`version: "*"`), the deleted peer can reappear later.
## Issue Context
- `override_peer_group` handles dash deletion by removing from `peerDependencies` only.
- Peer extraction will materialize optional meta-only peers as `PeerDep { version: "*", optional: true, meta_only: true }`.
## Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[244-256]
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]
### Concrete fix
In the `new_bare_specifier == "-"` branch, also remove the key from `peerDependenciesMeta` (and optionally remove the whole `peerDependenciesMeta` object if it becomes empty). This ensures the dash override fully deletes the peer requirement rather than turning it into a meta-only optional peer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
3. Tests reference pnpm 11.6.0 📎 Requirement gap ≡ Correctness ⭐ New
Description
The updated optional-peer hoist tests explicitly state behavior was verified against pnpm 11.6.0,
but the compliance target requires lockfile parity with pnpm 11.5.2. This weakens assurance that the
implemented behavior matches the required pnpm version and may allow remaining semantic parity gaps
to slip through.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[R726-738]

+/// An optional peer with a real `peerDependencies` entry whose
+/// provider is resolved anywhere in the tree (here: deep under a
+/// sibling) IS hoisted: upstream folds every run-resolved version into
+/// `ctx.allPreferredVersions` and `getHoistableOptionalPeers` resolves
+/// the peer against it after the wave (verified against pnpm 11.6.0 —
+/// the `eslint` + `cosmiconfig-typescript-loader` shape, where
+/// `eslint` gains `(jiti@x)`). For
+/// <https://github.com/pnpm/pnpm/issues/12266>.
#[tokio::test]
-async fn optional_peer_only_in_resolved_tree_is_not_hoisted() {
+async fn optional_peer_with_real_entry_is_hoisted_from_resolved_tree() {
    let mut table = HashMap::new();
    table.insert(
        ("needs-opt".to_string(), "1.0.0".to_string()),
Evidence
PR Compliance ID 1 requires parity with pnpm 11.5.2, but the modified tests explicitly cite pnpm
11.6.0 as the verification baseline for the intended behavior, which does not satisfy the stated
compliance target version.

Lockfile-only output matches pnpm 11.5.2 for the same clean state (content parity, excluding env-doc)
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[726-733]
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[785-790]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires `pacquet install --lockfile-only` parity with pnpm **11.5.2**, but the newly added/updated tests document verification against pnpm **11.6.0**. This creates a compliance gap because the behavior may differ between pnpm versions.

## Issue Context
The checklist explicitly targets pnpm 11.5.2 for content parity, so tests and documentation should either:
- verify and cite 11.5.2 behavior, or
- include additional assertions/fixtures that lock in 11.5.2 semantics.

## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs[726-790]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Optional bit mis-tagged 🐞 Bug ≡ Correctness ⭐ New
Description
importer_direct_wanted_specs sets WantedSpec.optional by checking whether an alias exists in
optionalDependencies, even when DependencyGroup::Optional is not included (e.g. --no-optional
runs with [Prod]). This can mis-classify a non-optional direct dep as optional, affecting the
wanted-dep cache key and optionality propagation during resolution.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[R338-361]

    let optional_names = importer_optional_dependency_names(manifest);
    let injected_names = importer_injected_dependency_names(manifest);
-    let mut wanted: Vec<WantedSpec> = Vec::new();
+    let mut order: Vec<&str> = Vec::new();
+    let mut ranges: HashMap<&str, &str> = HashMap::new();
    for (name, range) in manifest.dependencies(groups) {
        if !crate::is_valid_dependency_alias(name) {
            return Err(ResolveDependencyTreeError::InvalidDependencyName {
                parent: "The current package".to_string(),
                alias: name.to_string(),
            });
        }
-        let optional = optional_names.contains(name);
-        let injected = injected_names.contains(name);
-        wanted.push((name.to_string(), range.to_string(), optional, injected));
+        if ranges.insert(name, range).is_none() {
+            order.push(name);
+        }
    }
+    let wanted: Vec<WantedSpec> = order
+        .into_iter()
+        .map(|name| {
+            (
+                name.to_string(),
+                ranges[name].to_string(),
+                optional_names.contains(name),
+                injected_names.contains(name),
+            )
Evidence
The function always builds optional_names from the manifest’s optionalDependencies and then uses
it for every wanted alias, regardless of which dependency groups are included for the current run.
The repo also demonstrates real executions where optional deps are excluded by passing only
[DependencyGroup::Prod] (the --no-optional shape), making this mis-tagging reachable in
practice; additionally wanted.optional is part of the wanted-dep cache key so incorrect tagging
changes resolution/caching behavior.

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[264-274]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[328-363]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[377-383]
pacquet/crates/package-manager/src/install/tests.rs[3229-3242]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`importer_direct_wanted_specs` marks a wanted dep as `optional` if its alias exists anywhere in `optionalDependencies`, even if the current install excluded optional dependencies (i.e. `DependencyGroup::Optional` is not in `dependency_groups`). This can incorrectly set `WantedDependency.optional` for deps that are actually coming from `dependencies`/`devDependencies` in that run.

### Issue Context
This is observable when an alias appears in both `dependencies` (or `devDependencies`) and `optionalDependencies`, and the install is run with an included-group set that does **not** include `Optional` (the codebase explicitly uses `[DependencyGroup::Prod]` for `--no-optional`).

### Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[328-365]

### Suggested fix
Compute the `optional` flag from the group that actually contributed the final chosen specifier (i.e., track the “winning” group per alias while iterating groups in precedence order), or at minimum gate `optional_names` behind `included.contains(&DependencyGroup::Optional)` so optional tagging can’t happen when optional deps are excluded.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 5327f7b


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1)

Context used

Action required
1. Optional peers hoisted as direct deps 📎 Requirement gap ≡ Correctness
Description
get_hoistable_optional_peers results are converted into new wanted specs with optional=false and
then installed at the importer level, materializing optional peers as regular direct dependencies.
This can cause lockfile entries to diverge from pnpm’s representation for optional peers, which
should rely on peer suffixes and transitivePeerDependencies instead of direct dependencies
entries.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[R383-387]

+        let hoisted_optional =
+            get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions);
       if hoisted_optional.is_empty() {
           break;
       }
Evidence
PR Compliance ID 3 forbids representing optional peers as regular direct dependencies in the
lockfile. The new code explicitly turns hoistable optional peers into importer wanted specs with
optional=false and extends the tree with them, which materializes them as direct deps.

Do not materialize resolved optional peers as direct dependencies (match pnpm representation)
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The optional-peer hoist path currently installs hoisted optional peers as importer-level direct dependencies (`optional=false`), which violates the requirement to not materialize resolved optional peers as direct `dependencies` entries.
## Issue Context
The compliance requirement (PR Compliance ID 3) expects optional peers to be represented via peer suffixes and `transitivePeerDependencies`, matching pnpm’s lockfile representation, rather than being added as regular direct dependencies.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[383-400]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Dash override resurrects peer 🐞 Bug ≡ Correctness
Description
override_peer_group removes a peer from peerDependencies when the override value is "-" but
does not remove the corresponding peerDependenciesMeta entry, so the peer can reappear as a
meta-only optional "*" peer. This defeats the intended meaning of a dash override and can change
the resolved graph/lockfile output.
Code

pacquet/crates/package-manager/src/overrides.rs[R249-255]

+            if chosen.inner.new_bare_specifier == "-" {
+                if let Some(peers) =
+                    value.get_mut("peerDependencies").and_then(Value::as_object_mut)
+                {
+                    peers.remove(&name);
+                }
+                continue;
Evidence
The override code deletes only from peerDependencies, while peer extraction explicitly recreates a
peer from peerDependenciesMeta (optional:true) when no peerDependencies entry exists, so a dash
override can be negated by leftover metadata.

pacquet/crates/package-manager/src/overrides.rs[244-256]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When a peer override is `"-"`, `override_peer_group` removes the entry from `peerDependencies` but leaves any corresponding `peerDependenciesMeta[name]` entry intact.
Because `extract_peer_dependencies` treats `peerDependenciesMeta[name].optional == true` without a matching `peerDependencies[name]` as a real meta-only optional peer (`version: "*"`), the deleted peer can reappear later.
## Issue Context
- `override_peer_group` handles dash deletion by removing from `peerDependencies` only.
- Peer extraction will materialize optional meta-only peers as `PeerDep { version: "*", optional: true, meta_only: true }`.
## Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[244-256]
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[1863-1873]
### Concrete fix
In the `new_bare_specifier == "-"` branch, also remove the key from `peerDependenciesMeta` (and optionally remove the whole `peerDependenciesMeta` object if it becomes empty). This ensures the dash override fully deletes the peer requirement rather than turning it into a meta-only optional peer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 5327f7b


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1)

Context used

Action required
1. Optional peers hoisted as direct deps 📎 Requirement gap ≡ Correctness
Description
get_hoistable_optional_peers results are converted into new wanted specs with optional=false and
then installed at the importer level, materializing optional peers as regular direct dependencies.
This can cause lockfile entries to diverge from pnpm’s representation for optional peers, which
should rely on peer suffixes and transitivePeerDependencies instead of direct dependencies
entries.
Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[R383-387]

+        let hoisted_optional =
+            get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions);
        if hoisted_optional.is_empty() {
            break;
        }
Evidence
PR Com...

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ece52f5-e2b8-4dcd-9ef3-b8de5e4f66c0

📥 Commits

Reviewing files that changed from the base of the PR and between 69535c8 and 46d2cc9.

📒 Files selected for processing (14)
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
✅ Files skipped from review due to trivial changes (1)
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
🚧 Files skipped from review as they are similar to previous changes (12)
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization
Derive simple conversions for branded strings using #[derive(derive_more::From)] and #[derive(derive_more::Into)] instead of handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like `${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
🧠 Learnings (5)
📚 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/package-manager/src/dependencies_graph_to_lockfile/tests.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.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/package-manager/src/dependencies_graph_to_lockfile/tests.rs
📚 Learning: 2026-06-06T18:58:37.156Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12243
File: pacquet/crates/package-manager/src/install_package_by_snapshot.rs:319-322
Timestamp: 2026-06-06T18:58:37.156Z
Learning: When reviewing Rust code, do not assume `matches!(expr, Pattern(_))` will move out of `expr` if `Pattern(_)` contains no by-value bindings. `matches!` desugars to a `match` that auto-borrows the scrutinee for discrimination, so even if `expr` is a non-`Copy` value behind a shared reference (e.g., `&T`), the macro should not move-out of the borrowed data purely due to `matches!`. Treat `matches!(&expr, Pattern(_))` as a readability/clarity improvement, not a correctness requirement. Only flag potential move-out-of-borrow risks when the pattern includes by-value bindings that would require moving the matched value.

Applied to files:

  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
📚 Learning: 2026-06-12T07:04:03.703Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12227
File: pacquet/crates/package-manager/src/install.rs:708-719
Timestamp: 2026-06-12T07:04:03.703Z
Learning: In this pnpm-based repo’s package-manager Rust code, `ThrottledClient` uses a two-class semaphore scheduling policy (“reserved half” for tarball/download work and a higher-priority “latency class” for lockfile-verification metadata fetches). Therefore, when reviewing, do NOT flag reuse of the same `Arc<ThrottledClient>` for both downloads and verification as a budget/contention issue—this reuse is intentional and handled by the internal two-class “reserved permits” design. Creating a separate client instance would break the EMFILE guard behavior that relies on shared throttling.

Applied to files:

  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
🔇 Additional comments (1)
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs (1)

557-560: LGTM!


📝 Walkthrough

Walkthrough

This PR adds auto_install_peers threading through the resolver, extends the data model with meta_only flags to distinguish peers declared only via peerDependenciesMeta, refines peer extraction and wanted-spec selection, omits peer-shadowed dependencies, excludes meta-only peers from optional hoisting, and adds peer-dependency override support to VersionsOverrider.

Changes

Auto-install peers and meta-only peer dependency feature

Layer / File(s) Summary
Data model: add meta_only field to peer types
pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs, pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs, pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
PeerDep, MissingPeer, and MissingPeerInfo structs gain a meta_only: bool field to distinguish peers declared only via peerDependenciesMeta.
Thread auto_install_peers through resolver options and contexts
pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs, pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
WorkspaceResolveOptions and ResolveDependencyTreeOptions add auto_install_peers field; wired into WorkspaceTreeCtx and TreeCtx with new setter methods; workspace-level setting overrides per-importer values.
Integrate fresh lockfile with auto_install_peers
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
Fresh-lockfile install path propagates workspace auto_install_peers config into WorkspaceResolveOptions.
Extract meta-only peers and refine wanted specs
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
importer_direct_wanted_specs refactored to conditionally include peer group and merge by first-seen alias; extract_peer_dependencies treats package's own name as skip set and marks peerDependenciesMeta entries as meta_only peers with version: "*".
Omit peer-shadowed dependencies
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
When auto_install_peers is enabled, resolved manifests post-processed via omit_peer_shadowed_dependencies to remove dependencies entries shadowed by peerDependencies.
Propagate meta_only through peer resolution
pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs, pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
meta_only flag threaded through cached missing-peer hits and newly computed missing peers; stored in MissingPeerInfo and propagated to issue entries.
Refine optional-peer hoisting to exclude meta_only peers
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
Optional-peer hoisting aligned with run-extended preferred-versions map; meta_only peers filtered out and excluded from hoist candidate selection.
Extend VersionsOverrider to handle peerDependencies overrides
pacquet/crates/package-manager/src/overrides.rs
apply_to_value pipeline extended to call override_peer_group; peer overrides update peerDependencies in-place for valid peer ranges or redirect concrete results to dependencies; - value deletes peer entries; is_valid_peer_range classifies specifier as peer-compatible.
Test coverage: overrides, hoisting, and wanted specs
pacquet/crates/package-manager/src/overrides/tests.rs, pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs, pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs, pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs, pacquet/crates/resolving-deps-resolver/src/tests.rs, pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
Adds tests for peer-override outcomes, optional-peer hoisting (real vs. meta-only), wanted-spec selection logic, and peer shadowing behavior; updates existing tests to explicitly set auto_install_peers: false; new test modules for importer_wanted_specs and peer_own_dep_shadowing.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

  • pnpm/pnpm#12266: Changes directly address peer-suffix and peer-hoisting behaviors by adding meta_only tracking, auto-install-peers plumbing, and refined optional-peer hoisting logic described in the pnpm parity issue.

Possibly related PRs

  • pnpm/pnpm#11784: Main PR extends the auto-install-peers and peer-hoisting pipeline by adding and threading the meta_only peer flag, building directly on the same hoist_peers algorithm introduced in #11784.
  • pnpm/pnpm#11906: Both PRs modify the cached peer/missing-peer resolution flow in resolve_peers.rs; main PR extends that flow to propagate meta_only through cached and newly computed missing-peer diagnostics.
  • pnpm/pnpm#12273: Both PRs modify peer-resolution logic in resolve_peers.rs; main PR adds meta_only propagation into missing-peer tracking while the retrieved PR restructures peer parent-context and edge patching.

Poem

🐰 Meta-only peers now marked with care,
Auto-install hoists what's truly there,
Shadowed deps stripped clean and neat,
Overrides for peers complete—our review is sweet! 🌱

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically summarizes the four main fixes implemented: importer peer+dev merge, peer-vs-own-dep handling, peer overrides, and optional-peer hoist behavior, directly matching the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lockfile-parity2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Fix pacquet lockfile parity for peer merge/shadowing, peer overrides, optional-peer hoist
🐞 Bug fix 🧪 Tests ✨ Enhancement 🕐 40+ Minutes

Grey Divider

Walkthroughs

Description
• Align importer wanted-spec merging with pnpm so regular deps override same-name peers.
• Match pnpm peer/own-dep shadowing and optional-peer hoist behavior (incl. meta-only peers).
• Apply pnpm-style overrides to peerDependencies and expand regression coverage.
Diagram
graph TD
  A["package-manager"] --> D["resolve_workspace"] --> E["resolve_importer"] --> F["resolve_dependency_tree"] --> G["resolve_peers & hoist"] --> H[("lockfile output")]
  A --> B["VersionsOverrider"] --> C["manifest rewrite"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Thread parent-scope context to fully port non-autoInstallPeers peer omission
  • ➕ Would match pnpm’s non-autoInstallPeers behavior in more cases (omit only in-scope peers).
  • ➕ Reduces remaining parity gaps around peer-vs-own-dep resolution in scoped contexts.
  • ➖ Requires reworking pacquet’s per-package children cache to carry parent context, increasing complexity and memory pressure.
  • ➖ Higher risk of perf regressions and subtle cache invalidation bugs.
2. Port a real semver-range intersection library for peer range merges
  • ➕ Closer fidelity to pnpm’s safeIntersect behavior for multi-consumer peer ranges.
  • ➕ Avoids conservative drop/join behavior in edge cases.
  • ➖ Adds dependency surface area (or non-trivial port) for a comparatively rare case.
  • ➖ More complex to test across npm semver corner-cases.
3. Reuse upstream peer-range validation logic via shared crate/module
  • ➕ Keeps isValidPeerRange perfectly aligned as pnpm evolves.
  • ➕ Reduces ongoing maintenance of peer-range special cases (workspace:/catalog:).
  • ➖ May be hard to share code across ecosystems; could increase build/packaging complexity.
  • ➖ Current heuristic is already targeted and covered by tests.

Recommendation: The current approach is appropriate for lockfile-parity work: it directly ports pnpm’s decision points where pacquet has equivalent context (importer wanted-spec merge order, autoInstallPeers shadowing, optional-peer hoist inputs, peer overrides) and explicitly documents the one upstream arm that cannot be faithfully replicated without redesign (non-autoInstallPeers parent-scope omission). Consider the parent-context threading alternative only if remaining parity gaps prove material and justify the additional architectural complexity.

Grey Divider

File Changes

Enhancement (2)
overrides.rs Apply pnpm overrides to peerDependencies (delete/rewrite/promote) +81/-19

Apply pnpm overrides to peerDependencies (delete/rewrite/promote)

• Implements the peerDependencies arm of pnpm’s override logic: '-' deletes peers, valid peer ranges rewrite peers in-place, and non-peer specs are added to dependencies while retaining the declared peer. Adds peer-range validity check mirroring pnpm’s isValidPeerRange behavior.

pacquet/crates/package-manager/src/overrides.rs


resolved_tree.rs Extend PeerDep model with meta-only marker +7/-0

Extend PeerDep model with meta-only marker

• Adds PeerDep.meta_only to represent peers declared only through peerDependenciesMeta, aligning downstream missing-peer/hoist behavior with pnpm’s peerDependencies-only sourcing for optional-peer hoists.

pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs


Bug fix (6)
install_with_fresh_lockfile.rs Thread auto_install_peers into workspace resolve options +1/-0

Thread auto_install_peers into workspace resolve options

• Propagates config.auto_install_peers into WorkspaceResolveOptions so resolver behavior (peer shadowing, wanted spec merge) matches the install configuration.

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs


dependencies_graph.rs Record meta-only flag on missing-peer issues +7/-0

Record meta-only flag on missing-peer issues

• Extends MissingPeer with meta_only to distinguish peers declared only via peerDependenciesMeta, enabling optional-peer hoist to match pnpm’s missing-peer sourcing rules.

pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs


resolve_dependency_tree.rs Merge importer wanted specs like pnpm and add autoInstallPeers to context +125/-22

Merge importer wanted specs like pnpm and add autoInstallPeers to context

• Adds auto_install_peers to ResolveDependencyTreeOptions and threads it into WorkspaceTreeCtx/TreeCtx. Updates importer_direct_wanted_specs to merge duplicate aliases across peer/prod/dev/optional groups in pnpm order so later regular deps override earlier peer ranges.

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs


resolve_importer.rs Fix optional-peer hoist inputs and exclude meta-only peers +20/-16

Fix optional-peer hoist inputs and exclude meta-only peers

• Uses the run-extended preferred-versions map for optional-peer hoisting (matching pnpm) and filters meta-only peers out of the optional-hoist missing set. Threads auto_install_peers into the shared workspace context.

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs


resolve_peers.rs Propagate meta-only state through missing-peer computation +6/-1

Propagate meta-only state through missing-peer computation

• Adds meta_only to MissingPeerInfo and threads it from PeerDep into missing-peer reporting, enabling downstream partitioning to enforce pnpm-equivalent hoist eligibility.

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs


resolve_workspace.rs Make workspace resolution aware of autoInstallPeers +10/-1

Make workspace resolution aware of autoInstallPeers

• Adds auto_install_peers to WorkspaceResolveOptions and stores it on the shared WorkspaceTreeCtx so tree walking can drop peer-shadowed dependencies consistently across importers.

pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs


Tests (6)
tests.rs Update lockfile key tests for meta-only peer field +4/-2

Update lockfile key tests for meta-only peer field

• Adjusts test fixtures to construct PeerDep with the new meta_only flag, keeping snapshot/package-key assertions consistent with the extended peer model.

pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs


tests.rs Add regression tests for peerDependency overrides +70/-0

Add regression tests for peerDependency overrides

• Adds tests covering peer-range rewrites, dependency promotion for non-peer ranges, dash deletion, and apply_to_arc cloning when only peers match overrides.

pacquet/crates/package-manager/src/overrides/tests.rs


tests.rs Correct optional-peer hoist regression coverage (real-entry vs meta-only) +67/-16

Correct optional-peer hoist regression coverage (real-entry vs meta-only)

• Inverts the prior regression expectation: optional peers with real peerDependencies entries can be hoisted from newly resolved in-tree providers. Adds a new test ensuring meta-only optional peers are not hoisted even when a provider exists in the resolved tree.

pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs


tests.rs Update peer test helpers for meta-only PeerDep field +4/-1

Update peer test helpers for meta-only PeerDep field

• Adjusts helper constructors to set meta_only: false for standard peers to satisfy the expanded PeerDep struct.

pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs


tests.rs Update workspace option fixtures for auto_install_peers field +1/-0

Update workspace option fixtures for auto_install_peers field

• Extends workspace test option builders to include auto_install_peers with a default false value.

pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs


tests.rs Update resolver test option structs for auto_install_peers +240/-0

Update resolver test option structs for auto_install_peers

• Plumbs the new auto_install_peers field through multiple test option initializations to keep compilation and behavior consistent.

pacquet/crates/resolving-deps-resolver/src/tests.rs


Grey Divider

Qodo Logo

Comment thread pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
Comment thread pacquet/crates/package-manager/src/overrides.rs
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.02      7.7±0.30ms   561.3 KB/sec    1.00      7.6±0.22ms   569.9 KB/sec

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs (1)

252-300: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep WorkspaceTreeCtx.auto_install_peers and ResolveImporterOptions.auto_install_peers in sync here.

resolve_importer() stamps a fresh WorkspaceTreeCtx with opts.auto_install_peers, but this overload never reconciles the same flag on the caller-supplied workspace. That lets one call pass auto_install_peers: true while the shared workspace still says false, so importer_direct_wanted_specs() and the TreeCtx-driven walk can run under different peer-install settings and build the wrong tree.

Based on learnings, "follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly."

🤖 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 `@pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs` around lines
252 - 300, The workspace's auto_install_peers flag must be synchronized with the
options passed into resolve_importer_with_workspace: before creating the TreeCtx
(and before calling importer_direct_wanted_specs), set the caller-supplied
WorkspaceTreeCtx.auto_install_peers to the
ResolveImporterOptions.auto_install_peers value so the shared workspace and the
TreeCtx/walk use the same peer-install behavior; update
resolve_importer_with_workspace to apply that change (or call the workspace's
setter) right after destructuring opts and before TreeCtx::with_workspace /
importer_direct_wanted_specs, ensuring resolve_importer and
resolve_importer_with_workspace behave consistently.

Source: Learnings

🧹 Nitpick comments (1)
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs (1)

78-92: ⚡ Quick win

Update the all_preferred_versions contract comment.

This doc block still says optional-peer hoisting reads a pre-run snapshot, but Lines 302-310 and 383-384 now intentionally consult the run-extended map. Leaving the old text here makes the public API contract wrong.

As per coding guidelines, "Doc comments (///, //!) document the contract — preconditions, postconditions, panics, and the reason the function exists; they are not a re-narration of the body."

🤖 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 `@pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs` around lines
78 - 92, The doc comment for the struct field `all_preferred_versions` is
outdated and must be rewritten to reflect the current contract: explain that the
orchestrator extends the preferred-versions map in place as packages are walked
(so run-time extensions are visible), and that both the `hoist_peers`
(required-peer) picker and the `get_hoistable_optional_peers` logic consult this
run-extended map rather than a pre-run snapshot; state the caller should pass
the result of `get_preferred_versions_from_lockfile_and_manifests` (or an empty
map when none) and document this precondition/intent clearly on
`all_preferred_versions`.

Source: Coding guidelines

🤖 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/resolving-deps-resolver/src/resolve_dependency_tree.rs`:
- Around line 1035-1039: Reused manifests are not normalized like fresh
resolves, causing peers listed also as dependencies to be dropped when
ctx.workspace.auto_install_peers is enabled; update resolve_reused_node() to run
the same normalization as the fresh-path by applying
omit_peer_shadowed_dependencies(...) to result_inner.manifest when
ctx.workspace.auto_install_peers is true and a manifest exists (mirror the
pattern used where result_inner.manifest is taken and then replaced with
Some(omit_peer_shadowed_dependencies(manifest))). This ensures
extract_peer_dependencies(...) and snapshot_child_refs(...) see the normalized
manifest and prevents reintroducing peer-vs-own-dep mismatches.

In `@pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs`:
- Around line 507-508: The meta_only flag on MissingPeerInfo should be combined
with AND semantics when merging multiple occurrences (so meta_only stays false
if any occurrence was false) instead of last-writer-wins; locate where
MissingPeerInfo entries are merged (places using HashMap::insert or the map
merge for peer names) and change the merge to set meta_only = existing.meta_only
&& new.meta_only (or equivalent) so that resolve_node and
partition_missing_peers see meta_only==true only when all occurrences were
meta-only.

In `@pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs`:
- Around line 163-170: The workspace-level auto_install_peers value in
resolve_workspace is the single source of truth but importer_opts still keep
their callback-returned values, which can cause mixed policies; after building
each ResolveImporterOptions (the importer_opts instances) and before calling
compute_time_based_cutoff or entering the resolve loop (and before
resolve_importer_with_workspace/shadow pruning), assign
importer_opts.auto_install_peers = auto_install_peers so every importer uses the
WorkspaceTreeCtx auto_install_peers; update references where importer_opts are
constructed in resolve_workspace to normalize this field immediately after
construction.

---

Outside diff comments:
In `@pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs`:
- Around line 252-300: The workspace's auto_install_peers flag must be
synchronized with the options passed into resolve_importer_with_workspace:
before creating the TreeCtx (and before calling importer_direct_wanted_specs),
set the caller-supplied WorkspaceTreeCtx.auto_install_peers to the
ResolveImporterOptions.auto_install_peers value so the shared workspace and the
TreeCtx/walk use the same peer-install behavior; update
resolve_importer_with_workspace to apply that change (or call the workspace's
setter) right after destructuring opts and before TreeCtx::with_workspace /
importer_direct_wanted_specs, ensuring resolve_importer and
resolve_importer_with_workspace behave consistently.

---

Nitpick comments:
In `@pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs`:
- Around line 78-92: The doc comment for the struct field
`all_preferred_versions` is outdated and must be rewritten to reflect the
current contract: explain that the orchestrator extends the preferred-versions
map in place as packages are walked (so run-time extensions are visible), and
that both the `hoist_peers` (required-peer) picker and the
`get_hoistable_optional_peers` logic consult this run-extended map rather than a
pre-run snapshot; state the caller should pass the result of
`get_preferred_versions_from_lockfile_and_manifests` (or an empty map when none)
and document this precondition/intent clearly on `all_preferred_versions`.
🪄 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: d5de30b4-8e47-41a7-abce-676d28ff393b

📥 Commits

Reviewing files that changed from the base of the PR and between 01b3d45 and 5327f7b.

📒 Files selected for processing (14)
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/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). (7)
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Code Coverage
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: ubuntu-latest / Node.js 24 / Test
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization
Derive simple conversions for branded strings using #[derive(derive_more::From)] and #[derive(derive_more::Into)] instead of handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like `${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
🧠 Learnings (6)
📚 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/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.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/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
📚 Learning: 2026-05-24T21:10:50.292Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: When implementing dependency “revisit”/optional-folding in pacquet, mirror pnpm’s `resolveDependencies.ts` behavior: on revisit, update the `optional` flag only for the directly visited package, and do not automatically change `optional` for transitive descendants. pnpm corrects stale optional flags later via `copyDependencySubGraph` (a BFS in `lockfile/pruner/src/index.ts`/`copyDependencySubGraph`). Until pacquet has an equivalent pruner/copy-subgraph step, be aware that the raw `node.optional` can flow through to the lockfile/virtual store via `dependencies_graph_to_lockfile.rs` → `create_virtual_store.rs` → `installability.rs`, so review changes that affect optional flag propagation for this mismatch.

Applied to files:

  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
📚 Learning: 2026-05-24T21:10:50.292Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: In pacquet’s Rust resolver layer, the behavior of `ResolvedPackage.optional` “AND-folding”/revisit updates is intentionally aligned with pnpm: when a package is directly revisited, only that package’s `optional` flag is updated; transitive descendants are not re-walked/rescored in this stage. pnpm later corrects any stale optional flags downstream via its pruner/BFS logic (`copyDependencySubGraph` + `nonOptional` reachability stamping in the lockfile pruner). Since pacquet does not yet have that pruner equivalent, review should NOT flag the downstream/stale optional propagation as a bug in pacquet PRs; treat the resolver-layer optional propagation gap as expected parity until the pruning/stamping step is ported.

Applied to files:

  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
📚 Learning: 2026-06-06T18:58:37.156Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12243
File: pacquet/crates/package-manager/src/install_package_by_snapshot.rs:319-322
Timestamp: 2026-06-06T18:58:37.156Z
Learning: When reviewing Rust code, do not assume `matches!(expr, Pattern(_))` will move out of `expr` if `Pattern(_)` contains no by-value bindings. `matches!` desugars to a `match` that auto-borrows the scrutinee for discrimination, so even if `expr` is a non-`Copy` value behind a shared reference (e.g., `&T`), the macro should not move-out of the borrowed data purely due to `matches!`. Treat `matches!(&expr, Pattern(_))` as a readability/clarity improvement, not a correctness requirement. Only flag potential move-out-of-borrow risks when the pattern includes by-value bindings that would require moving the matched value.

Applied to files:

  • pacquet/crates/resolving-deps-resolver/src/dependencies_graph.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
  • pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs
  • pacquet/crates/package-manager/src/overrides/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
  • pacquet/crates/package-manager/src/overrides.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs
  • pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
🔇 Additional comments (5)
pacquet/crates/package-manager/src/overrides.rs (1)

119-119: LGTM!

Also applies to: 156-165, 219-280, 321-327

pacquet/crates/package-manager/src/overrides/tests.rs (1)

279-348: LGTM!

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)

1030-1030: LGTM!

pacquet/crates/resolving-deps-resolver/src/resolved_tree.rs (1)

175-181: LGTM!

pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs (1)

557-560: LGTM!

Comment thread pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs
Comment thread pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs
@codecov-commenter

codecov-commenter commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.38562% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.97%. Comparing base (66a9078) to head (46d2cc9).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...lving-deps-resolver/src/resolve_dependency_tree.rs 97.18% 2 Missing ⚠️
pacquet/crates/package-manager/src/overrides.rs 98.30% 1 Missing ⚠️
...rates/resolving-deps-resolver/src/resolve_peers.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12345      +/-   ##
==========================================
+ Coverage   87.85%   87.97%   +0.11%     
==========================================
  Files         291      292       +1     
  Lines       36246    36752     +506     
==========================================
+ Hits        31844    32332     +488     
- Misses       4402     4420      +18     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD.

Scenario: Isolated linker: fresh restore, cold cache + cold store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 4.003 ± 0.137 3.786 4.144 1.99 ± 0.15
pacquet@main 3.861 ± 0.138 3.665 4.192 1.91 ± 0.14
pnpr@HEAD 2.111 ± 0.207 1.895 2.454 1.05 ± 0.12
pnpr@main 2.017 ± 0.134 1.845 2.243 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 4.0032156271,
      "stddev": 0.1369877319390789,
      "median": 4.0733646266,
      "user": 3.838639379999999,
      "system": 2.0902702999999994,
      "min": 3.7863705106000003,
      "max": 4.1435850846,
      "times": [
        4.1302483476,
        3.8909674806,
        3.8342614256000003,
        4.1435850846,
        4.0889988396,
        4.1231176896,
        4.0721401406,
        3.8878776396,
        3.7863705106000003,
        4.0745891126
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 3.8611097349999994,
      "stddev": 0.1384283523961935,
      "median": 3.8353241250999996,
      "user": 3.8672411799999997,
      "system": 2.1010458,
      "min": 3.6647599186,
      "max": 4.1919642276,
      "times": [
        3.9265130986,
        3.9198103726,
        3.7763333976,
        3.8609228236,
        4.1919642276,
        3.6647599186,
        3.7811864906,
        3.8397446316,
        3.8189587706,
        3.8309036186
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 2.1107412765,
      "stddev": 0.20663987806188877,
      "median": 2.1139132441,
      "user": 2.8714842800000002,
      "system": 1.8099260999999995,
      "min": 1.8954830076000002,
      "max": 2.4543430156,
      "times": [
        2.2509941196,
        2.2265077626,
        2.4543430156,
        1.8998095266,
        1.8997653826,
        2.2450662326,
        2.0013187256,
        2.3010868166,
        1.8954830076000002,
        1.9330381756000001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 2.0166318477000003,
      "stddev": 0.1344524853793379,
      "median": 1.9710337966,
      "user": 2.8846097800000003,
      "system": 1.7942183,
      "min": 1.8448647026,
      "max": 2.2431163486,
      "times": [
        2.2431163486,
        1.9083000826,
        2.2044482156,
        2.0808328456,
        1.8448647026,
        2.0954881516,
        1.9328903156,
        2.0091772776,
        1.9144011456,
        1.9327993916000001
      ]
    }
  ]
}

Scenario: Isolated linker: fresh restore, hot cache + hot store

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 476.0 ± 10.4 460.2 488.5 1.02 ± 0.02
pacquet@main 466.4 ± 5.1 456.8 476.1 1.00
pnpr@HEAD 631.1 ± 114.4 554.2 866.5 1.35 ± 0.25
pnpr@main 569.5 ± 10.1 557.5 588.0 1.22 ± 0.03
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.47601746240000004,
      "stddev": 0.010360026211877808,
      "median": 0.48038782860000007,
      "user": 0.36009028,
      "system": 0.77886516,
      "min": 0.4601961411,
      "max": 0.48852341610000005,
      "times": [
        0.48556262710000003,
        0.4601961411,
        0.48852341610000005,
        0.46136778910000004,
        0.47051558610000005,
        0.4811633551,
        0.48193042610000003,
        0.47961230210000005,
        0.4670308651,
        0.48427211610000004
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.4664263024000001,
      "stddev": 0.00506208457728723,
      "median": 0.4663320166,
      "user": 0.36435867999999993,
      "system": 0.7662863599999999,
      "min": 0.4568073931,
      "max": 0.47614995810000005,
      "times": [
        0.46663088710000006,
        0.46844601310000006,
        0.4628476771,
        0.4633628281,
        0.4568073931,
        0.46550580910000006,
        0.47614995810000005,
        0.46878960710000006,
        0.4696897051,
        0.4660331461
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.6311199821000001,
      "stddev": 0.11442703337121747,
      "median": 0.5664890836,
      "user": 0.3831332800000001,
      "system": 0.7808810599999999,
      "min": 0.5541722770999999,
      "max": 0.8665066641,
      "times": [
        0.5612345680999999,
        0.5826301101,
        0.5544405481,
        0.5576472621,
        0.7284599991,
        0.7731302251,
        0.8665066641,
        0.5649782461,
        0.5679999211,
        0.5541722770999999
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.5694930658999999,
      "stddev": 0.010072859598796656,
      "median": 0.5672568306,
      "user": 0.38128038000000003,
      "system": 0.77477356,
      "min": 0.5575124571,
      "max": 0.5879829911,
      "times": [
        0.5622547211,
        0.5575124571,
        0.5653931641,
        0.5691204971,
        0.5635468321,
        0.5724727950999999,
        0.5582816701,
        0.5797727261,
        0.5785928051,
        0.5879829911
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + cold store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 3.642 ± 0.054 3.590 3.752 1.88 ± 0.14
pacquet@main 3.581 ± 0.045 3.500 3.659 1.85 ± 0.14
pnpr@HEAD 1.949 ± 0.148 1.776 2.166 1.01 ± 0.11
pnpr@main 1.934 ± 0.144 1.687 2.177 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 3.6419463663399996,
      "stddev": 0.05406740713885515,
      "median": 3.6298304312400003,
      "user": 3.81648676,
      "system": 2.05583934,
      "min": 3.59034103174,
      "max": 3.75222100674,
      "times": [
        3.6438254547400004,
        3.59042868674,
        3.63686217974,
        3.70606413074,
        3.6093862957400003,
        3.75222100674,
        3.59034103174,
        3.59420261574,
        3.6733335787400003,
        3.62279868274
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 3.58143873464,
      "stddev": 0.04453026798341445,
      "median": 3.5849252327400003,
      "user": 3.7923357599999994,
      "system": 2.02559684,
      "min": 3.50046543174,
      "max": 3.6589476707400004,
      "times": [
        3.6085614197400004,
        3.5823790797400004,
        3.5874713857400002,
        3.54172386974,
        3.6589476707400004,
        3.61795314674,
        3.55444701974,
        3.5603618737400002,
        3.60207644874,
        3.50046543174
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 1.9485671475399997,
      "stddev": 0.14763820363675065,
      "median": 1.96361353874,
      "user": 2.6657021600000004,
      "system": 1.74973834,
      "min": 1.77588141574,
      "max": 2.16587809874,
      "times": [
        2.06521083174,
        1.77588141574,
        2.02676152774,
        1.78060047274,
        1.8229414387399998,
        1.90933467274,
        2.01789240474,
        2.11675853974,
        1.80441207274,
        2.16587809874
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 1.93427089934,
      "stddev": 0.14426032720175566,
      "median": 1.92837602174,
      "user": 2.68368836,
      "system": 1.73297144,
      "min": 1.68678222774,
      "max": 2.17682233874,
      "times": [
        1.96871603374,
        1.9202637487399998,
        1.93648829474,
        2.12418499874,
        1.91988350974,
        1.82415275374,
        1.9762974867399998,
        1.8091176007399998,
        1.68678222774,
        2.17682233874
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, hot cache + hot store

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 914.9 ± 9.4 903.4 927.8 1.86 ± 0.04
pacquet@main 873.9 ± 6.2 861.5 882.7 1.78 ± 0.03
pnpr@HEAD 491.7 ± 8.6 479.1 512.8 1.00
pnpr@main 553.5 ± 87.2 489.5 690.3 1.13 ± 0.18
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.91487379036,
      "stddev": 0.009438542323202431,
      "median": 0.9142862350600001,
      "user": 1.0866176199999997,
      "system": 0.98614426,
      "min": 0.90344718806,
      "max": 0.9277659050600001,
      "times": [
        0.9277659050600001,
        0.90633293406,
        0.9159467330600001,
        0.9265878040600001,
        0.91262573706,
        0.92270929406,
        0.90789683606,
        0.90344718806,
        0.9035144140600001,
        0.9219110580600001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.8739124996600001,
      "stddev": 0.006188935529207131,
      "median": 0.87358729106,
      "user": 1.04246792,
      "system": 0.9938700599999999,
      "min": 0.86151902006,
      "max": 0.8827077400600001,
      "times": [
        0.8728278700600001,
        0.86151902006,
        0.8678856740600001,
        0.8716124340600001,
        0.8798774450600001,
        0.8724283450600001,
        0.8827077400600001,
        0.87831201006,
        0.87434671206,
        0.87760774606
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.4916676650599999,
      "stddev": 0.008621818651969567,
      "median": 0.49111128706000007,
      "user": 0.32034931999999994,
      "system": 0.74975846,
      "min": 0.47911009506,
      "max": 0.51282091606,
      "times": [
        0.49464101906,
        0.51282091606,
        0.49155798006000007,
        0.49198797606000005,
        0.47911009506,
        0.48828812406000005,
        0.49236606806000005,
        0.48547164106000007,
        0.49066459406000007,
        0.48976823706000006
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.55349647646,
      "stddev": 0.08720245377931576,
      "median": 0.49749953856000007,
      "user": 0.33026131999999997,
      "system": 0.74952416,
      "min": 0.48946253706000004,
      "max": 0.6903189160600001,
      "times": [
        0.49374843706000004,
        0.49199395206,
        0.49065125906000007,
        0.49852962106000004,
        0.48946253706000004,
        0.67957237806,
        0.6630222870600001,
        0.6903189160600001,
        0.54119592106,
        0.49646945606000004
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + hot store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.425 ± 0.022 2.388 2.469 4.90 ± 0.07
pacquet@main 2.386 ± 0.026 2.348 2.419 4.82 ± 0.07
pnpr@HEAD 0.496 ± 0.006 0.488 0.505 1.00 ± 0.02
pnpr@main 0.495 ± 0.005 0.485 0.502 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.4246799796199996,
      "stddev": 0.021978469821676797,
      "median": 2.4203496838199996,
      "user": 1.56368774,
      "system": 1.19268896,
      "min": 2.38818841332,
      "max": 2.46851744032,
      "times": [
        2.4272266443199997,
        2.42126454232,
        2.38818841332,
        2.4194348253199998,
        2.41699774132,
        2.43350069332,
        2.4476892923199998,
        2.46851744032,
        2.40650638732,
        2.41747381632
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.38581368792,
      "stddev": 0.025802242155046836,
      "median": 2.38450042882,
      "user": 1.53637414,
      "system": 1.18744416,
      "min": 2.3476503523199996,
      "max": 2.41859792632,
      "times": [
        2.38684620632,
        2.40638802932,
        2.3821546513199996,
        2.4173694383199997,
        2.40696484932,
        2.3476503523199996,
        2.37006554732,
        2.35379841432,
        2.41859792632,
        2.36830146432
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.49571366382000004,
      "stddev": 0.0063968860810277965,
      "median": 0.49488380332000004,
      "user": 0.33234144,
      "system": 0.7574323599999999,
      "min": 0.48771032632,
      "max": 0.5052212853200001,
      "times": [
        0.5052212853200001,
        0.49400543532,
        0.49065084932,
        0.48982059932,
        0.50270862232,
        0.49028211132,
        0.49576217132,
        0.48771032632,
        0.5041304123200001,
        0.49684482532
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.49493324432,
      "stddev": 0.005344274226303749,
      "median": 0.49644137832,
      "user": 0.32577284,
      "system": 0.7506447600000001,
      "min": 0.48536882732000003,
      "max": 0.5024571103200001,
      "times": [
        0.49865738232,
        0.5024571103200001,
        0.49504026932,
        0.49905155132,
        0.48536882732000003,
        0.49851078532000004,
        0.49118391632,
        0.49784248732,
        0.49225684232,
        0.48896327132
      ]
    }
  ]
}

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12345
Testbedpacquet
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
isolated-linker.fresh-install.cold-cache.cold-store📈 view plot
🚷 view threshold
3,641.95 ms
(-56.22%)Baseline: 8,319.07 ms
9,982.88 ms
(36.48%)
isolated-linker.fresh-install.cold-cache.hot-store📈 view plot
🚷 view threshold
2,424.68 ms
(-47.26%)Baseline: 4,597.06 ms
5,516.48 ms
(43.95%)
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
🚷 view threshold
914.87 ms
(-32.63%)Baseline: 1,357.97 ms
1,629.57 ms
(56.14%)
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
🚷 view threshold
4,003.22 ms
(-56.45%)Baseline: 9,192.32 ms
11,030.79 ms
(36.29%)
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
🚷 view threshold
476.02 ms
(-28.36%)Baseline: 664.44 ms
797.33 ms
(59.70%)
🐰 View full continuous benchmarking report in Bencher

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12345
Testbedpnpr

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
BenchmarkLatencymilliseconds (ms)
isolated-linker.fresh-install.cold-cache.cold-store📈 view plot
⚠️ NO THRESHOLD
1,948.57 ms
isolated-linker.fresh-install.cold-cache.hot-store📈 view plot
⚠️ NO THRESHOLD
495.71 ms
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
⚠️ NO THRESHOLD
491.67 ms
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
⚠️ NO THRESHOLD
2,110.74 ms
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
⚠️ NO THRESHOLD
631.12 ms
🐰 View full continuous benchmarking report in Bencher

@zkochan zkochan marked this pull request as draft June 12, 2026 05:22
@zkochan zkochan marked this pull request as ready for review June 12, 2026 06:04
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5327f7b

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 43fd70f

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 69535c8

zkochan added 7 commits June 12, 2026 09:43
…er range

An importer that declares the same alias in both peerDependencies and a
regular group (e.g. a workspace:* devDependency plus a catalog: peer)
resolved both specs, and the peer's registry resolution overwrote the
workspace link in the importers lockfile section.

Merge the groups the way pnpm spreads them in getWantedDependencies
(https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/getWantedDependencies.ts#L32-L43):
peerDependencies first (when autoInstallPeers), then dependencies <
devDependencies < optionalDependencies, a later group's range replacing
an earlier one, so each alias yields a single wanted spec.

Part of #12266 (the @pnpm/logger
link-vs-registry divergence, 182 lines of the monorepo lockfile diff).
… as a dependency

Under autoInstallPeers, pnpm removes peer-shadowed names from a resolved
package's dependencies before extracting its peers
(https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1527-L1542),
so the peer edge supplies the package and the depPath carries the peer
suffix. pacquet did the inverse — peerDependenciesWithoutOwn-style
filtering dropped the peer and walked the own dependency — so e.g.
@babel/parser (whose packageExtensions-added @babel/types peer is also
its regular dependency) lost its (@babel/types@x) suffix and the
peerDependencies block in the lockfile.

Also align extract_peer_dependencies with upstream's
peerDependenciesWithoutOwn: the package's own name counts as an own
dep, and a peerDependenciesMeta entry without a matching
peerDependencies entry only becomes a peer when optional: true.

The non-autoInstallPeers arm (omit only peers resolvable from the
parent scope) is not ported: pacquet's per-package children cache has
no parent context, and the own dependency keeps winning there, which
matches upstream whenever the peer is not in scope.

Part of #12266 (~80 lines of the
monorepo lockfile diff).
Port the peerDependencies arm of upstream's overrideDepsOfPkg
(https://github.com/pnpm/pnpm/blob/01b3d45ddb/hooks/read-package-hook/src/createVersionsOverrider.ts#L68-L129):
a matched peer is deleted on '-', rewritten in place when the override
value is a valid peer range (semver, or a workspace:/catalog:
expression), and otherwise written into dependencies while the declared
peer range stays. The apply_to_arc clone gate now also fires for
manifests whose only match is a peer.

Without this, e.g. the ajv@<8.18.0 → >=8.18.0 override kept a
package's peerDependencies at the stale range and @yarnpkg/libzip lost
its (@yarnpkg/fslib@3.x) suffix relative to pnpm's lockfile.

Part of #12266.
…s, except meta-only peers

pnpm folds every run-resolved package version into
ctx.allPreferredVersions
(https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1483-L1488)
and getHoistableOptionalPeers reads that map after each wave — so an
optional peer with a real peerDependencies entry (e.g. eslint's jiti)
resolves against a provider anywhere in the freshly resolved tree.
What keeps a peer like debug's supports-color bare is not the map but
the missing-peer set: getMissingPeers
(https://github.com/pnpm/pnpm/blob/01b3d45ddb/installing/deps-resolver/src/resolveDependencies.ts#L1773-L1782)
iterates peerDependencies only, so a peer declared solely via
peerDependenciesMeta never feeds the hoist. Both behaviors verified
against pnpm 11.6.0 (eslint + cosmiconfig-typescript-loader hoists
jiti; debug + concurrently stays bare).

pacquet modeled this with a static lockfile-seeded map, which got the
meta-only case right by the wrong mechanism and under-hoisted every
real-entry optional peer (jiti, typanion, conventional-commits-parser,
@types/node suffixes on the monorepo). Feed the run-extended map to
the optional hoist and track meta-only-ness on PeerDep/MissingPeer so
partition_missing_peers can exclude meta-only peers from the hoist
bucket.

Part of #12266.
… for auto_install_peers

The setting is workspace-wide (pnpm's autoInstallPeers is one config
value per install), so the workspace options now override each
per-importer value instead of trusting the callback to agree — the
importer hoist loop and the tree walk's shadow pruning can no longer
diverge. Addresses a review comment on the PR.
@zkochan zkochan force-pushed the lockfile-parity2 branch from 69535c8 to 46d2cc9 Compare June 12, 2026 08:02
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 46d2cc9

@zkochan zkochan merged commit a751c7f into main Jun 12, 2026
20 of 21 checks passed
@zkochan zkochan deleted the lockfile-parity2 branch June 12, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants