Walkthrough

This PR fixes a critical bug where pnpm.overrides were not applied during fresh-lockfile resolution in pacquet. The changes refactor VersionsOverrider to use owned storage, parse and apply overrides transitively through the resolver via composed manifest hooks, tighten lockfile reuse validation, and add comprehensive test coverage.

Changes

Override Application During Fresh-Lockfile Resolution

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• pnpm update: an active override referencing a catalog is applied with the pre-update catalog during resolution (graph stays stale) #12159: Related override parsing and resolution logic that feeds into the same resolved-override application path during fresh-lockfile generation.

Possibly related PRs

• pnpm/pnpm#11793: Introduces the VersionsOverrider and override-resolution foundation that this PR builds upon and refactors for owned storage and composition.
• pnpm/pnpm#11820: Ensures catalog: override values are resolved before validation, aligning with the catalog-expansion logic added in this PR's fresh-lockfile options.

Poem

🐰 Hops through the resolver with springs of cheer,
Overrides now applied, both far and near—
Catalogs expand, transitive paths glow,
Fresh locks now fresh from the top to the toe! 🎉

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

• X
• Mastodon
• Reddit
• LinkedIn

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

Description

overrides_match compares overrides as an unordered key/value set, so reordering pnpm.overrides
can still allow wanted-lockfile subtree reuse even though override selection can depend on entry
order when multiple candidates are equally specific. This can reuse stale transitive subtrees and
yield a resolution that doesn’t reflect the newly ordered overrides.

Code

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[R1827-1843]

+fn overrides_match(
+    lockfile: Option<&IndexMap<String, String>>,
+    config: Option<&IndexMap<String, String>>,
+) -> bool {
+    let lockfile = lockfile.filter(|map| !map.is_empty());
+    let config = config.filter(|map| !map.is_empty());
+    match (lockfile, config) {
+        (None, None) => true,
+        (Some(lockfile), Some(config)) => {
+            lockfile.len() == config.len()
+                && lockfile.iter().all(|(key, value)| {
+                    config.get(key).is_some_and(|config_value| config_value == value)
+                })
+        }
+        _ => false,
+    }
+}

Evidence

The reuse gate uses overrides_match to decide whether to pass the prior lockfile for subtree
reuse, but overrides_match ignores ordering. Meanwhile, override selection sorts candidates and
then takes the first; because the comparator can return Equal for some pairs, the original
iteration order can affect which override is chosen, so reordering can change behavior and should
invalidate reuse.

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1003-1016]
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1827-1843]
pacquet/crates/package-manager/src/overrides.rs[226-263]
pacquet/crates/package-manager/src/overrides.rs[278-299]

Agent prompt

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

### Issue description
`overrides_match()` currently ignores insertion order by checking only that both maps contain the same key/value pairs. However, `VersionsOverrider` can select an override based on the relative order of equally-specific matching candidates (it sorts, but can return `Ordering::Equal` and then takes the first match), so reordering overrides can change behavior.

This means the fresh-resolution reuse gate may incorrectly reuse the prior lockfile’s resolved subtrees after an override reordering, producing stale/incorrect resolutions.

### Issue Context
- `wanted_lockfile` reuse is gated on `overrides_match(...)`.
- Override selection takes the first candidate after a specificity sort; ties are possible and then input order becomes the tie-breaker.

### Fix Focus Areas
- pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[1827-1843]

### Suggested fix
Change `overrides_match` to be order-sensitive (since both config and lockfile use `IndexMap` specifically to preserve insertion order):
- After filtering empty maps, compare via `lockfile == config` (IndexMap equality is order-sensitive).

(Alternative, if you *want* ordering to be irrelevant: make override selection deterministic and order-independent by adding a strict tie-breaker, then keep the unordered comparison. But today, selection can depend on order, so the reuse gate should too.)

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

Description

The fresh resolver’s overrides ManifestHook calls VersionsOverrider::apply_to_arc(..., None), so
relative link:/file: override values cannot be re-relativized and are rendered as absolute
paths. This can make overridden transitive dependency specs machine-specific and inconsistent with
the overrider’s documented local-target behavior.

Code

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[R830-838]

+        let overrides_hook: Option<ManifestHook> =
+            versions_overrider.as_ref().and_then(|overrider| {
+                if overrider.is_empty() {
+                    None
+                } else {
+                    let overrider = Arc::clone(overrider);
+                    Some(Arc::new(move |manifest| overrider.apply_to_arc(manifest, None))
+                        as ManifestHook)
+                }

Evidence

The overrides hook explicitly passes None as the manifest directory, and the overrider’s local
override rendering falls back to emitting the absolute path when no directory is provided.

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[825-838]
pacquet/crates/package-manager/src/overrides.rs[105-111]
pacquet/crates/package-manager/src/overrides.rs[329-372]

Agent prompt

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

## Issue description
Fresh resolution applies overrides via a `ManifestHook` but passes `manifest_dir=None` into `VersionsOverrider::apply_to_arc`, which forces relative `link:`/`file:` override values to serialize as absolute paths when applied transitively.
## Issue Context
`resolve_local_override_spec` only re-relativizes when `pkg_dir` is `Some(_)`; otherwise it emits the absolute path stored in `LocalTarget`.
## Fix Focus Areas
- pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[825-839]
- pacquet/crates/package-manager/src/overrides.rs[329-372]
## Suggested fix approach
Prefer a contained fix in `VersionsOverrider`:
1. When parsing local override targets, also store the original normalized relative path component (or a root-dir-relative form) in `LocalTarget` when `specified_via_relative_path == true`.
2. Update `resolve_local_override_spec` so that when `pkg_dir` is `None` and the target was specified via a relative path, it returns a stable relative specifier (e.g. the stored root-relative form) instead of the absolute path.
This avoids changing the `ManifestHook` signature while preventing machine-specific absolute paths from being introduced by the hook path.

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

Description

VersionsOverrider::apply_to_arc scans dependency maps to decide whether any override applies and
then scans again to apply overrides after cloning, duplicating override-selection work for matching
manifests. Since ManifestHook runs on every resolved package manifest during fresh resolution,
this extra pass can add noticeable overhead on large graphs or large override sets.

Code

pacquet/crates/package-manager/src/overrides.rs[R134-171]

+    /// Apply overrides to the resolver's shared manifest value,
+    /// cloning only when at least one configured override can rewrite
+    /// the manifest.
+    pub fn apply_to_arc(&self, manifest: Arc<Value>, manifest_dir: Option<&Path>) -> Arc<Value> {
+        if self.is_empty() || !self.has_applicable_override(&manifest) {
+            return manifest;
       }
+        let mut cloned = (*manifest).clone();
+        self.apply_to_value(&mut cloned, manifest_dir);
+        Arc::new(cloned)
+    }
+
+    fn applicable_parent_scoped<'b>(&'b self, manifest: &Value) -> Vec<&'b ResolvedOverride> {
+        let manifest_name = manifest.get("name").and_then(Value::as_str);
+        let manifest_version = manifest.get("version").and_then(Value::as_str);
+
+        self.parent_scoped
+            .iter()
+            .filter(|entry| {
+                let Some(parent) = entry.inner.parent_pkg.as_ref() else { return false };
+                let name_matches = manifest_name == Some(parent.name.as_str());
+                let range_matches = match (parent.bare_specifier.as_deref(), manifest_version) {
+                    (None, _) => true,
+                    (Some(_), None) => false,
+                    (Some(range), Some(version)) => semver_satisfies(version, range),
+                };
+                name_matches && range_matches
+            })
+            .collect()
+    }
+
+    fn has_applicable_override(&self, value: &Value) -> bool {
+        let applicable_parent_scoped = self.applicable_parent_scoped(value);
+        [DependencyGroup::Prod, DependencyGroup::Optional, DependencyGroup::Dev]
+            .iter()
+            .copied()
+            .any(|group| self.group_has_override(value, group, &applicable_parent_scoped))
+    }

Evidence

The resolver applies manifest_hook to every resolved package manifest, and apply_to_arc performs
a full applicability scan before cloning and applying, which repeats override selection logic for
manifests where overrides match.

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[117-128]
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[904-944]
pacquet/crates/package-manager/src/overrides.rs[134-187]

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 new `apply_to_arc` path runs an applicability scan (`has_applicable_override`) and, if true, clones and applies overrides, which repeats similar per-dependency override selection work. This is on the resolver hot path because the resolver applies `manifest_hook` to every resolved manifest.
## Issue Context
The resolver applies `manifest_hook` before any downstream consumers read the manifest. Any extra per-manifest scanning scales with package count.
## Fix Focus Areas
- pacquet/crates/package-manager/src/overrides.rs[134-234]
- pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs[889-944]
## Suggested fix approach
1. Pre-index overrides by target package name during `VersionsOverrider::new` (e.g. `HashMap<String, Vec<ResolvedOverride>>` for generic and parent-scoped). This makes both the applicability check and application avoid scanning unrelated overrides.
2. Make the applicability check cheaper than full selection (avoid building/sorting vectors just to detect a match), or restructure `apply_to_arc` to compute the needed information once (e.g. compute applicable parent-scoped set once and reuse it for both the “any match?” decision and mutation).
3. Keep the existing “clone only on match” behavior, but ensure the “match” determination is close to O(#deps * #candidate-overrides-for-those-deps), not O(#deps * #all-overrides).

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

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