Description

modified_manifests_match_lockfile() checks lockfile satisfaction using an override-applied
manifest but then runs linked_packages_are_up_to_date() against the raw on-disk manifest, so
overrides that change linkability can be missed. This can incorrectly return Decision::UpToDate
and skip a needed relink, leaving node_modules inconsistent with the effective configuration.

Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R411-433]

+    let linked_ctx = LinkedPackagesContext::new(config, project_manifests);
+    for project in to_check {
+        let importer_id =
+            pacquet_workspace::importer_id_from_root_dir(workspace_root, project.root_dir);
+        if let Err(error) = crate::install::check_importer_satisfies(
+            wanted,
+            project.manifest,
+            &importer_id,
+            config,
+            parsed_overrides.as_deref(),
+        ) {
+            tracing::debug!(target: "pacquet::install", %error, importer_id, "repeat-install content check: manifest no longer satisfied");
+            return Err("a modified manifest is no longer satisfied by the lockfile");
+        }
+        let Some(importer) = wanted.importers.get(&importer_id) else {
+            return Err("a modified project has no importer entry in the lockfile");
+        };
+        if !linked_packages_are_up_to_date(
+            &linked_ctx,
+            project.root_dir,
+            project.manifest,
+            importer,
+        ) {

Evidence

The code applies pnpm.overrides when checking whether the lockfile satisfies the manifest, but the
linked-package freshness check uses the unmodified manifest’s dependency specifiers, so it can
disagree with the effective (override-applied) dependency graph that the lockfile represents.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[402-433]
pacquet/crates/package-manager/src/install.rs[1303-1308]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[537-607]

Agent prompt

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

## Issue description
During the modified-manifests content re-check, overrides are applied for the lockfile/manifest specifier comparison, but `linked_packages_are_up_to_date()` still reads dependency specifiers from `project.manifest` (the unmodified on-disk manifest). If `pnpm.overrides` changes a dependency’s effective specifier in a way that affects whether it should be linked to a workspace package, the linked-package check can make the wrong decision and allow the optimistic fast path to return `UpToDate`.
### Issue Context
- `check_importer_satisfies()` explicitly applies overrides because lockfile specifiers are written with overrides already applied.
- `linked_packages_are_up_to_date()` reads the manifest’s dependency strings directly from `manifest.value()`.
### Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[402-436]
- pacquet/crates/package-manager/src/install.rs[1303-1322]
### Suggested fix
1. Ensure `linked_packages_are_up_to_date()` receives an override-applied manifest (and, if applicable, the same effective view used for lockfile satisfaction).
2. Options:
- Refactor `check_importer_satisfies()` to optionally return the effective manifest (or a lightweight effective dependency-spec map) so the caller can reuse it for the linked check.
- Or duplicate the override-application logic in `modified_manifests_match_lockfile()` (build `VersionsOverrider` once, apply to a cloned manifest per project) and pass the cloned manifest into `linked_packages_are_up_to_date()`.
3. Add a regression test where an override flips linkability (e.g., manifest has `"pkg-a": "^1.0.0"`, workspace has `pkg-a@2.0.0`, override forces `workspace:*`, and the fast path must *not* return `UpToDate` unless the installed state matches the override-implied linking).

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

Description

In linked_packages_are_up_to_date(), workspace package lookup uses
dep.version.as_regular().to_string(), which can include a peer suffix (e.g. "1.0.0(react@...)"), so
it won’t match the workspace index keyed by package.json’s plain "version". This can incorrectly
treat a registry-resolved dep as not linkable and return Decision::UpToDate when a full install is
needed to re-link to the workspace package.

Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R586-593]

+            let linked_dir: Option<std::borrow::Cow<'_, Path>> = match link_target {
+                Some(target) => Some(std::borrow::Cow::Owned(project_dir.join(target))),
+                None => dep
+                    .version
+                    .as_regular()
+                    .map(|ver| ver.to_string())
+                    .and_then(|version| ctx.workspace_packages.get(&dep_name)?.get(&version))
+                    .map(|dir| std::borrow::Cow::Borrowed(*dir)),

Evidence

The repeat-install linked-package check indexes workspace packages by manifest "version" strings,
but uses PkgVerPeer::to_string() (which includes the peer suffix) when looking up a
lockfile-resolved version, causing the lookup to miss peer-variant versions.
PkgVerPeer::without_peer() exists to normalize this exact mismatch.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[492-507]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[586-594]
pacquet/crates/lockfile/src/pkg_ver_peer.rs[49-59]
pacquet/crates/lockfile/src/pkg_ver_peer.rs[137-149]

Agent prompt

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

## Issue description
`linked_packages_are_up_to_date()` tries to detect when a dependency that was previously resolved from the registry should now resolve to a workspace package (or vice versa). In the non-linked case it looks up a candidate workspace package by using the lockfile-resolved version string as a key, but that string can include a peer suffix (e.g. `1.0.0(react@17.0.2)`), while the workspace index is built from `package.json`'s plain `"version"` field (e.g. `1.0.0`). This mismatch can cause the lookup to fail and the fast path to incorrectly return `UpToDate`.
### Issue Context
`PkgVerPeer`’s `Display` includes the peer suffix, and it provides `without_peer()` specifically for normalization.
### Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[586-593]
- pacquet/crates/lockfile/src/pkg_ver_peer.rs[49-59]
- pacquet/crates/lockfile/src/pkg_ver_peer.rs[137-149]
### Proposed fix
When building the version key for `ctx.workspace_packages` lookup, normalize away the peer suffix (and keep the base version) before indexing, e.g.:
- replace `ver.to_string()` with `ver.without_peer().to_string()` (or equivalent extraction of the base `VersionPart`), so peer-variant lockfile entries still map to the workspace manifest’s `version`.
Add/adjust a test that covers a workspace package with `version: 1.0.0` and an importer dep resolved as `1.0.0(<peer suffix>)`, ensuring the repeat-install content check falls through when it should relink.

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

Description

In workspace mode, modified_manifests_match_lockfile only runs linked_packages_are_up_to_date
for projects whose manifests are newer than lastValidatedTimestamp, so an unchanged dependent
importer can be skipped even if a linked workspace package it depends on changed (e.g., version bump
outside the declared range). This can incorrectly return Decision::UpToDate and skip a needed full
install, leaving node_modules inconsistent with workspace link semantics.

Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R285-358]

+    let to_check: &[&ManifestStat<'_>] = if is_workspace_install {
+        // Workspace branch: a wanted lockfile newer than the last
+        // validation must equal what the previous install materialized.
+        // Mirrors <https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L283-L289>.
+        if wanted_mtime_ms > state.last_validated_timestamp {
+            assert_wanted_lockfile_equals_current(wanted, config)?;
+        }
+        modified
+    } else {
+        // Single-project branch keys off the lockfile mtimes instead of
+        // `lastValidatedTimestamp`. Mirrors
+        // <https://github.com/pnpm/pnpm/blob/cc4ff817aa/deps/status/src/checkDepsStatus.ts#L407-L462>.
+        let current_mtime_ms =
+            mtime_ms(&config.virtual_store_dir.join(Lockfile::CURRENT_FILE_NAME));
+        if let Some(current_mtime_ms) = current_mtime_ms
+            && wanted_mtime_ms > current_mtime_ms
+        {
+            assert_wanted_lockfile_equals_current(wanted, config)?;
+        }
+        let root = modified.first().expect("modified-manifests branch requires a modified project");
+        if root.mtime_ms > wanted_mtime_ms {
+            modified
+        } else if current_mtime_ms.is_some() {
+            // "The manifest file is not newer than the lockfile.
+            // Exiting check."
+            &[]
+        } else if wanted.packages.as_ref().is_some_and(|packages| !packages.is_empty()) {
+            // RUN_CHECK_DEPS_NO_DEPS: the lockfile requires
+            // dependencies but nothing was ever installed.
+            return Err("the lockfile requires dependencies but none were installed");
+        } else {
+            &[]
+        }
+    };
+
+    if to_check.is_empty() {
+        return Ok(());
+    }
+
+    let parsed_overrides = crate::install::parse_config_overrides(config, catalogs)
+        .map_err(|_| "pnpm.overrides cannot be parsed")?;
+    if let Err(error) =
+        crate::install::check_lockfile_settings_drift(wanted, config, parsed_overrides.as_deref())
+    {
+        tracing::debug!(target: "pacquet::install", %error, "repeat-install content check: lockfile settings drift");
+        return Err("a lockfile setting drifted from the current configuration");
+    }
+
+    let linked_ctx = LinkedPackagesContext::new(config, project_manifests);
+    for project in to_check {
+        let importer_id =
+            pacquet_workspace::importer_id_from_root_dir(workspace_root, project.root_dir);
+        if let Err(error) = crate::install::check_importer_satisfies(
+            wanted,
+            project.manifest,
+            &importer_id,
+            config,
+            parsed_overrides.as_deref(),
+        ) {
+            tracing::debug!(target: "pacquet::install", %error, importer_id, "repeat-install content check: manifest no longer satisfied");
+            return Err("a modified manifest is no longer satisfied by the lockfile");
+        }
+        let Some(importer) = wanted.importers.get(&importer_id) else {
+            return Err("a modified project has no importer entry in the lockfile");
+        };
+        if !linked_packages_are_up_to_date(
+            &linked_ctx,
+            project.root_dir,
+            project.manifest,
+            importer,
+        ) {
+            return Err("a linked package is out of date");
+        }
}

Evidence

The workspace branch sets to_check to only the manifests with mtimes newer than
lastValidatedTimestamp, and then runs linked_packages_are_up_to_date only for `project in
to_check. Since linked_packages_are_up_to_date` validates link correctness for the specific
importer snapshot it is passed, skipping an unchanged dependent importer means its workspace links
are never revalidated when only the linked package’s manifest changed.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[192-206]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[285-358]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[451-531]
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs[48-55]

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 workspace installs, the modified-manifests content re-check only validates linked-package freshness for `to_check` (which is set to `modified`), so it may fail to detect when an *unchanged* importer’s workspace link is no longer valid (e.g., a sibling package’s version changed and no longer satisfies the dependent’s semver range).
## Issue Context
- `modified` is derived purely from manifest mtimes, and in the workspace branch `to_check = modified`.
- `linked_packages_are_up_to_date(...)` validates link correctness for a specific importer by comparing that importer’s manifest spec against the linked package’s current version.
- If only a linked package’s own manifest changes (version bump) while the dependent importer’s manifest is untouched, the dependent importer won’t be in `modified`, so its links won’t be validated.
## Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[252-360]
### Suggested fix approach
- Keep the per-importer `check_importer_satisfies(...)` gate limited to the `modified` set (to preserve the perf intent).
- But in **workspace mode**, run `linked_packages_are_up_to_date(...)` for **all importers/projects** in `project_manifests` (or all `wanted.importers` that have corresponding manifests), not just `modified`.
- Build/lookup `importer_id` for each project via `importer_id_from_root_dir(...)`.
- Fetch the corresponding `ProjectSnapshot` from `wanted.importers` and validate.
- If any importer fails the linked-package check, return the same `Decision::Skipped` reason (e.g. "a linked package is out of date").
### Add/adjust tests
- Add a workspace test where:
1) root depends on `pkg-a` via a link in the lockfile,
2) only `pkg-a/package.json` is rewritten with a version bump outside `^1.0.0`,
3) root `package.json` is *not* touched,
4) assert the decision is `Skipped` (linked package out of date).

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

Description

missingWantedLockfileStandIn() reads the current lockfile from /node_modules/.pnpm, ignoring
virtualStoreDir, so installs using a custom virtual store location won’t restore a deleted
pnpm-lock.yaml before the optimistic repeat-install short-circuit reports “Already up to date”.
This leaves the lockfile missing even though /lock.yaml may exist and could have been used for
restoration.

Code

deps/status/src/checkDepsStatus.ts[R637-641]

+async function missingWantedLockfileStandIn (lockfileDir: string): Promise<CheckDepsStatusResult['wantedLockfileToRestore']> {
+  if (safeStatSync(path.join(lockfileDir, WANTED_LOCKFILE)) != null) return undefined
+  const currentLockfile = await readCurrentLockfile(path.join(lockfileDir, 'node_modules/.pnpm'), { ignoreIncompatible: false })
+  if (currentLockfile == null) return undefined
+  return { lockfile: currentLockfile, lockfileDir }

Evidence

The fallback hard-codes node_modules/.pnpm as the current lockfile directory, but the lockfile FS
layer stores the current lockfile at virtualStoreDir/lock.yaml (and virtualStoreDir is a
supported config option passed through install). Therefore, with a non-default virtualStoreDir,
the restore path will look in the wrong place and fail to restore the wanted lockfile.

deps/status/src/checkDepsStatus.ts[631-642]
installing/commands/src/installDeps.ts[181-186]
config/reader/src/Config.ts[170-176]
lockfile/fs/src/read.ts[26-35]
lockfile/fs/src/write.ts[49-60]

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 “missing wanted lockfile fallback” restores `pnpm-lock.yaml` by reading the **current** lockfile from a hard-coded directory (`<lockfileDir>/node_modules/.pnpm`). When `virtualStoreDir` is configured to a different location, the current lockfile is written to `<virtualStoreDir>/lock.yaml`, so the fallback fails to find it and won’t restore `pnpm-lock.yaml`.
### Issue Context
- `installDeps()` forwards the CLI/config options (including `virtualStoreDir`) into `checkDepsStatus()`.
- `@pnpm/lockfile.fs` reads/writes the **current lockfile** under the configured virtual store directory.
### Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[631-642]
- deps/status/src/checkDepsStatus.ts[293-320]
### Implementation notes
- Thread `virtualStoreDir` into `CheckDepsStatusOptions` (it is already part of `Config` and is optional), and use `opts.virtualStoreDir` when calling `readCurrentLockfile()` / constructing the current-lockfile path.
- Keep the current behavior as a fallback when `opts.virtualStoreDir` is undefined (default to `<lockfileDir>/node_modules/.pnpm`).
- Apply the same path selection in both:
- `missingWantedLockfileStandIn()`
- the `sharedWorkspaceLockfile` branch where `pnpm-lock.yaml` is missing and the current lockfile stands in.

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

Description

check_importer_satisfies() resolves relative link:/file: override targets from
manifest.path().parent() rather than the lockfile/workspace root, so workspace installs can apply
overrides to the wrong path during the modified-manifests content check. This can wrongly force a
full install (or fail offline) even when the lockfile is actually still consistent with the
effective overrides.

Code

pacquet/crates/package-manager/src/install.rs[R1311-1317]

+    let manifest_for_freshness: &PackageManifest = if let Some(parsed) = parsed_overrides {
+        let root_dir = manifest.path().parent().unwrap_or_else(|| Path::new("."));
+        let overrider = crate::VersionsOverrider::new(parsed, root_dir);
+        overrider_manifest_holder = {
+            let mut cloned: PackageManifest = manifest.clone();
+            overrider.apply(&mut cloned, Some(root_dir));
+            cloned

Evidence

The freshness check applies overrides using the manifest directory as the override root, but
VersionsOverrider’s API explicitly separates the override root (used to resolve relative local
targets) from the manifest directory (used to re-relativize), and the fresh-resolve pipeline uses
the lockfile directory as the override root.

pacquet/crates/package-manager/src/install.rs[1303-1317]
pacquet/crates/package-manager/src/overrides.rs[81-116]
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[795-811]

Agent prompt

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

## Issue description
`check_importer_satisfies()` constructs `VersionsOverrider` using `root_dir = manifest.path().parent()`, and also passes that same directory as `manifest_dir` when applying the override. For workspace installs, overrides with relative `link:`/`file:` targets should be resolved relative to the lockfile/workspace root (the same base used in the fresh-resolve hook chain), while `manifest_dir` should remain the importer’s own directory.
### Issue Context
- `VersionsOverrider::new(..., root_dir)` uses `root_dir` to resolve relative local override targets.
- `VersionsOverrider::apply(..., manifest_dir)` uses `manifest_dir` to re-relativize the resolved target for the manifest being rewritten.
- The fresh-resolve path uses `lockfile_dir` as `root_dir` and `manifest.path().parent()` as `manifest_dir`, which indicates the intended semantics.
### Fix Focus Areas
- pacquet/crates/package-manager/src/install.rs[1291-1322]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[411-421]
### Suggested fix
1. Change `check_importer_satisfies()` to accept an additional `lockfile_dir`/`workspace_root` parameter (the base directory for resolving overrides).
2. Build the `VersionsOverrider` with that base directory.
3. When applying, pass `manifest.path().parent()` (importer dir) as `manifest_dir`.
4. Update call sites:
- `check_lockfile_freshness()` should pass the manifest dir (single-project) as both base and importer dir.
- `modified_manifests_match_lockfile()` should pass `workspace_root` as the override base, and each project’s `manifest.path().parent()` as the importer dir.

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

Walkthrough

This PR refactors the optimistic repeat-install fast-path to accept a struct input, replaces the mtime-unchanged shortcut with manifest-stat + content rechecks against wanted/current lockfiles (with optional regeneration), and extracts override parsing and per-importer satisfiability into pub(crate) helpers; tests updated for new flows and offline scenarios.

Changes

Optimistic Repeat-Install Refactor

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• pnpm install is a no-op when only pnpm-lock.yaml is changed (e.g., git checkout) — regression from v10 #12100 — Addresses the same optimistic-repeat-install skipping/validation behavior and is likely related to the stat+content recheck flow.
• Partial install with --frozen-lockfile: read+write node_modules/.pnpm/lock.yaml and skip unchanged snapshots pacquet#433 — Both use the virtual-store node_modules/.pnpm/lock.yaml as an effective wanted lockfile and touch restoration logic.

Possibly related PRs

• pnpm/pnpm#11811: Related by skipping per-importer manifest checks when ignore_manifest_check is enabled.
• pnpm/pnpm#11820: Similar refactor of parsing pnpm.overrides before freshness/drift checks.
• pnpm/pnpm#11945: Overlaps in restructuring optimistic repeat-install inputs and call sites to use a struct-based API.

Suggested reviewers

• anonrig

Poem

🐰 A struct arrives to tidy the run,

mtimes checked and lockfiles spun.
Overrides parsed, importers won,
Tests hop in — the job is done. 🥕

_{✏️ 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.