Walkthrough

Tracks which importer first resolved each package and records first-walk missing-peer names; introduces HoistMissingScope and threads it into peer-resolution hoist passes so missing-peer diagnostics can be suppressed per-importer in shared subtrees.

Changes

Hoist-scoped peer suppression with first-importer tracking

Sequence Diagram

sequenceDiagram
  participant Importer
  participant resolve_importer
  participant resolve_peers
  participant WorkspaceTreeCtx
  Importer->>resolve_importer: start importer walk (with importer_id)
  resolve_importer->>resolve_peers: call resolve_peers (hoist_peers_opts)
  resolve_peers->>WorkspaceTreeCtx: read first_importer_by_pkg / first_walk_missing_by_pkg
  resolve_peers->>WorkspaceTreeCtx: return missing_names_by_pkg
  resolve_importer->>WorkspaceTreeCtx: record_first_walk_missing(...)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• pacquet: lockfile-parity gaps vs pnpm CLI (npm: aliases, minimumReleaseAge, peer suffixes) #12266: Both changes modify peer-resolution hoisting and missing-peer diagnostics, introducing first-importer tracking and hoist_missing_scope to control when missing peers are emitted in shared subtrees.

Possibly related PRs

• pnpm/pnpm#11784: Related work on the hoist/auto-install-peers loop that this PR scopes per-importer.
• pnpm/pnpm#11906: Overlaps in resolve_peers.rs around missing-peer computation and cached issue emission.
• pnpm/pnpm#12022: Also extends ResolvePeersOptions and touches hoist/peer id handling in related code paths.

Poem

🐰 I hopped through shared subtrees, leaving tiny marks,
First importers signed the maps, quiet in the dark.
When peers went missing near, I hushed the second cries—
No echoes of the same old call beneath those skies.
🥕

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

• X
• Mastodon
• Reddit
• LinkedIn

Description

ResolvePeersResult::missing_names_by_pkg is built as a union of missing peer names across all
occurrences of a package in the walk, but it is persisted as the package’s “first-walk” missing set
and later used to decide hoist suppression. This can make later importers incorrectly *not* suppress
a peer miss (because some later occurrence in the first importer missed it), diverging peer-hoist
behavior from the intended “first occurrence wins” semantics.

Code

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[R613-621]

+        let mut missing_names_by_pkg: HashMap<String, std::collections::HashSet<String>> =
+            HashMap::new();
+        for (node_id, missing) in &self.node_missing_peers {
+            let Some(tree_node) = self.tree.dependencies_tree.get(node_id) else { continue };
+            missing_names_by_pkg
+                .entry(tree_node.resolved_package_id.clone())
+                .or_default()
+                .extend(missing.keys().cloned());
+        }

Evidence

missing_names_by_pkg is populated by extending the set for a pkg_id for every node occurrence,
which creates union semantics; this map is then persisted as the “first walk” record via
record_first_walk_missing (first write wins).

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[613-621]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[604-612]

Agent prompt

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

## Issue description
`missing_names_by_pkg` is currently computed by unioning missing peer names across *all* occurrences of a resolved package within a single `resolve_peers` walk, and `WorkspaceTreeCtx::record_first_walk_missing` persists that union as the package’s “first walk” missing set.
This defeats the intended “first occurrence / first walker wins” behavior: if the first importer sees the same `pkg_id` multiple times under different peer contexts, a peer that was satisfied in the first occurrence but missing in a later occurrence will still end up recorded as “missing in the first walk”, preventing suppression for later importers.
## Issue Context
- `missing_names_by_pkg` is fed into `WorkspaceTreeCtx::record_first_walk_missing(...)` and then used by `HoistMissingScope::suppresses(...)` to decide whether to suppress a missing-peer issue under a foreign-claimed ancestor.
- The current implementation builds `missing_names_by_pkg` from `self.node_missing_peers` at the end of the walk, which inherently loses traversal order and forces union semantics.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[578-628]
- pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[701-1002]
### Implementation direction
- Track “first occurrence” missing-peer names per `pkg_id` during the DFS walk (in deterministic visit order), e.g.:
- Add a `first_missing_names_by_pkg: HashMap<String, HashSet<String>>` field to `Walker`.
- When a node is resolved (both cache-hit and non-cache-hit), if `first_missing_names_by_pkg` has no entry for `pkg.id`, insert a `HashSet` of the current occurrence’s missing peer names (may be empty).
- Return that map as `ResolvePeersResult::missing_names_by_pkg`.
- Avoid building this from iterating a `HashMap` after the walk, because `HashMap` iteration order is not tied to first-visit order and forces union behavior.

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

Description

resolve_peers now always allocates and populates ResolvePeersResult::missing_names_by_pkg by
iterating node_missing_peers, even in call paths where it is not consumed (e.g. final per-importer
and workspace-wide peer passes). This adds avoidable CPU/memory overhead to every resolve_peers
invocation and can regress performance on large graphs/workspaces.

Code

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[R617-625]

+        let mut missing_names_by_pkg: HashMap<String, std::collections::HashSet<String>> =
+            HashMap::new();
+        for (node_id, missing) in &self.node_missing_peers {
+            let Some(tree_node) = self.tree.dependencies_tree.get(node_id) else { continue };
+            missing_names_by_pkg
+                .entry(tree_node.resolved_package_id.clone())
+                .or_default()
+                .extend(missing.keys().cloned());
+        }

Evidence

The PR adds an unconditional pass that allocates/fills missing_names_by_pkg for every
resolve_peers result. But both the final per-importer peer pass and the workspace-wide peer pass
explicitly run with hoist_missing_scope: None, meaning this new map is typically unused in those
paths.

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[617-631]
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[287-294]
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[426-428]
pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs[242-252]

Agent prompt

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

## Issue description
`Walker::walk()` always computes `missing_names_by_pkg`, but most call sites do not need it. This adds extra iteration and allocation to all `resolve_peers` calls.
## Issue Context
The field is needed for the importer hoist loop to persist the first walk’s missing-peer names, but the final per-importer pass and workspace-wide pass set `hoist_missing_scope: None` and do not record the per-package missing set.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[617-631]
### Suggested approach
- Make `missing_names_by_pkg` computation conditional (e.g., behind a new option like `ResolvePeersOptions::collect_missing_names_by_pkg: bool`), or change the result field to `Option<...>` and only populate when requested.
- Update the hoist loop call site to request/populate it, leaving other call sites to skip the work.

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

Description

The hoist loop constructs HoistMissingScope by cloning the full first_importer_by_pkg and
first_walk_missing_by_pkg maps on every resolve_peers(...) call. In large workspaces and/or
multiple hoist iterations, this adds avoidable O(packages) cloning overhead per iteration.

Code

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[R323-330]

+    let hoist_peers_opts = || {
+        let mut opts = peers_opts();
+        opts.hoist_missing_scope = Some(crate::resolve_peers::HoistMissingScope {
+            importer_id: importer_id.to_string(),
+            first_importer_by_pkg: ctx.workspace().first_importer_by_pkg(),
+            first_walk_missing_by_pkg: ctx.workspace().first_walk_missing_by_pkg(),
+        });
+        opts

Evidence

The hoist options closure pulls first_importer_by_pkg() and first_walk_missing_by_pkg() (both
cloning) for every peer-resolution attempt in the hoist loop.

pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[318-336]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[597-619]

Agent prompt

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

## Issue description
Inside the nested hoist loops, each `resolve_peers(...)` invocation recreates `HoistMissingScope` by calling `ctx.workspace().first_importer_by_pkg()` and `ctx.workspace().first_walk_missing_by_pkg()`, both of which clone entire `HashMap`s (and nested `HashSet`s). This can become a noticeable overhead on large dependency graphs.
## Issue Context
- For a given importer, the *ownership* map (`first_importer_by_pkg`) is stable.
- For non-first importers, `first_walk_missing_by_pkg` is typically stable as well (it was populated by earlier importers).
- The current design still clones both maps per hoist iteration.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[318-336]
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[597-619]
- pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[101-148]
### Implementation direction
- Prefer making `ResolvePeersOptions::hoist_missing_scope` cheap to clone:
- Change it to `Option<Arc<HoistMissingScope>>` (or equivalent), and update `missing_issue_suppressed` to use `as_deref()`.
- Build the `Arc<HoistMissingScope>` once per importer (or once per outer hoist loop) instead of re-cloning the maps.
- If you want to keep the current type, at minimum snapshot the maps once per importer and reuse them, refreshing only when `record_first_walk_missing` could have changed the relevant data.

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

Description

On peersCache hits, the walker deep-clones parent_pkg_ids_chain into a new Vec even when
hoist_missing_scope is None (the common case outside the hoist loop). This introduces avoidable
per-cache-hit allocations and string clones on a hot path.

Code

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[R891-903]

+                let mut chain_with_self = parent_pkg_ids_chain.to_vec();
+                chain_with_self.push(pkg.id.clone());
+                for (peer_name, info) in &missing {
+                    if self.missing_issue_suppressed(&chain_with_self, peer_name) {
+                        continue;
+                    }
+                    self.issues.missing.entry(peer_name.clone()).or_default().push(MissingPeer {
+                        wanted_range: info.range.clone(),
+                        optional: info.optional,
+                        meta_only: info.meta_only,
+                        parents: parents_from_chain(parent_chain_names, &pkg_name),
+                    });
+                }

Evidence

The cache-hit path clones the chain unconditionally, while the suppression helper immediately
returns false when hoist_missing_scope is absent—so the clone work is unnecessary whenever
suppression is disabled (which is explicitly the case in common call sites).

pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[878-904]
pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[1139-1142]
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs[287-294]
pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs[242-252]

Agent prompt

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

## Issue description
In the cache-hit branch, `parent_pkg_ids_chain.to_vec()` clones the entire chain (including all `String`s) before calling `missing_issue_suppressed`. When `hoist_missing_scope` is `None`, `missing_issue_suppressed` returns `false` immediately, so the clone is wasted.
## Issue Context
Most `resolve_peers` executions run with `hoist_missing_scope: None` (final per-importer pass and workspace pass). The hoist loop is the only path that needs suppression.
## Fix Focus Areas
- pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[878-904]
- pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs[1139-1142]
### Suggested approach
- In the cache-hit branch, branch on `self.opts.hoist_missing_scope.is_some()`:
- If `None`: emit missing issues without building `chain_with_self`.
- If `Some`: build `chain_with_self` and apply suppression.
- This preserves behavior while removing the clone/allocation from unscoped runs.

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

• Preserve first-importer peer context for shared package subtrees to match pnpm hoisting.
• Scope hoist-loop missing-peer issues using first-walk missing-peer snapshots (warnings unchanged).
• Record array-form engines as index-keyed entries in generated lockfile metadata.

The following are alternative approaches to this PR:

Recommendation: Current approach is best for parity: explicitly capturing first-importer ownership and first-walk missing-peer names matches pnpm’s shared-subtree caching behavior while keeping the final workspace-wide peer pass unscoped so diagnostics remain complete.

lib.rs Export 'HoistMissingScope' from resolving-deps-resolver +2/-2

Export 'HoistMissingScope' from resolving-deps-resolver

• Updates public re-exports to include 'HoistMissingScope', enabling scoped hoist behavior to be configured from callers.

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

---

dependencies_graph_to_lockfile.rs Preserve array-form 'engines' when writing lockfile metadata +23/-10

Preserve array-form 'engines' when writing lockfile metadata

• Extends 'engines' extraction to handle both object and array forms. Array entries are serialized as index-keyed map entries (matching 'Object.entries()' behavior) and '"*"' ranges are filtered out.

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

---

resolve_dependency_tree.rs Track first importer and first-walk missing peers for shared packages +60/-0

Track first importer and first-walk missing peers for shared packages

• Adds workspace-level maps to record which importer first resolved each package and the first peer-walk’s missing-peer names per package. Persists the first importer on first resolution (including reused-node paths) and exposes snapshot APIs consumed by peer-hoist scoping.

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

---

resolve_importer.rs Scope hoist-loop peer resolution using first-importer subtree context +18/-1

Scope hoist-loop peer resolution using first-importer subtree context

• Threads the importer id into the tree context and constructs scoped 'ResolvePeersOptions' for the hoist loop. Records the first-walk missing-peer names from each 'resolve_peers' call so later importers can suppress satisfied peers under foreign-owned subtrees, matching pnpm’s behavior.

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

---

resolve_peers.rs Suppress foreign-owned subtree missing peers during hoist input computation +107/-14

Suppress foreign-owned subtree missing peers during hoist input computation

• Introduces 'hoist_missing_scope' and 'HoistMissingScope' to conditionally suppress missing-peer issues under subtrees first resolved by other importers when the first walk did not report that peer missing. Also returns 'missing_names_by_pkg' so the first walk’s missing-peer set can be persisted for later importers.

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

---

resolve_workspace.rs Initialize new peer-resolution option for workspace peer pass +1/-0

Initialize new peer-resolution option for workspace peer pass

• Sets 'hoist_missing_scope: None' for the final workspace-wide peer pass to keep missing-peer warnings complete across importers.

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

---

tests.rs Add regression tests for shared-subtree peer context stability +194/-0

Add regression tests for shared-subtree peer context stability

• Adds two workspace-level tests covering both directions: (1) later importers must not hoist optional peers satisfied in the first importer’s context for shared subtrees, and (2) misses unsatisfied by the first importer remain visible and hoisted per importer.

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

---

tests.rs Update peer-resolution tests to include 'hoist_missing_scope' +1/-0

Update peer-resolution tests to include 'hoist_missing_scope'

• Adjusts test construction of 'ResolvePeersOptions' to populate the new 'hoist_missing_scope' field (set to 'None') to keep tests compiling and behavior unchanged.

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

---

• ❄️ 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.