Skip to content

fix: detect changes inside file: dependencies on repeat install (pacquet + pnpm)#12317

Merged
zkochan merged 20 commits into
pnpm:mainfrom
truffle-dev:fix/repeat-install-file-deps
Jun 16, 2026
Merged

fix: detect changes inside file: dependencies on repeat install (pacquet + pnpm)#12317
zkochan merged 20 commits into
pnpm:mainfrom
truffle-dev:fix/repeat-install-file-deps

Conversation

@truffle-dev

@truffle-dev truffle-dev commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • pnpm install reports "Already up to date" after edits inside a file: dependency's directory or after repacking a file: tarball. This is a v11 regression from the optimisticRepeatInstall default flip in feat: set default minimumReleaseAge to 1 day (1440 minutes) #11158. Fixes pnpm install no longer detects changes in file: directory dependencies (regression from v10) #11795.
  • checkDepsStatus gains a treatLocalFileDepsAsOutdated option: when set, any project manifest declaring a local file dependency makes the check report not up to date. installDeps sets it on the optimistic fast path, so projects with local file dependencies always run a real install, which refetches those dependencies (the v10 behavior).
  • The predicate covers file: specs, path-prefixed specs (./, ../, ~/, absolute POSIX paths, and Windows drive paths including drive-relative ones like C:dir, matching the local resolver's isFilespec), and bare tarball file names (vendor/pkg.tgz). It is deliberately narrower than the local resolver's bare-path matching: a bare user/repo is statically indistinguishable from a git shorthand at this layer, and matching it would kill the fast path for every project with git dependencies, so protocol-carrying and URL specs stay on the fast path.
  • pnpm.overrides entries are scanned with the same predicate: an override mapping to a local file spec redirects every matching dependency in the graph to that directory, so it has the same blind spot as a direct local file dependency. Registry and link: overrides keep the fast path.
  • The option is caller-scoped on purpose. verifyDepsBeforeRun also consumes checkDepsStatus, and treating file: deps as always stale there would force a reinstall before every pnpm run. Its behavior is unchanged, and a regression test pins that.
  • pacquet port in the same commit: check_optimistic_repeat_install bails unconditionally on file: specifiers, because its only caller is the install command, the one consumer that sets the flag upstream. link: specifiers are excluded on both sides: they are symlinked, so changes inside them flow through without a reinstall.

Why

Both branches of checkDepsStatus are blind to content changes inside a file: dependency. The workspace branch exits early with upToDate: true when no project manifest's mtime moved, without ever reaching linkedPackagesAreUpToDate. The non-workspace branch exits at the manifest-vs-lockfile mtime gate the same way. Editing a source file inside a file: dependency bumps neither, so the fast path can never see it; the fix has to bail before those gates rather than refine them. This is the fix shape (a) I proposed in my diagnosis on the issue thread (comment): the cost is a full resolution on repeat installs only for projects that declare file: dependencies, which is exactly what v10 did.

The manifest-only comparison in @pnpm/lockfile.verification (allProjectsAreUpToDate) is intentional for the install-proper path and asserted by its tests, so this PR leaves it untouched.

Checks

  • pnpm --filter @pnpm/deps.status test test/checkDepsStatus.test.ts (31 passed, 13 new)
  • pnpm --filter @pnpm/deps.status run compile and pnpm --filter @pnpm/installing.commands run compile (tsgo + eslint clean)
  • cargo test -p pacquet-package-manager optimistic_repeat_install (51 passed, 7 new; run in a rust:1.95.0 container)
  • cargo fmt --check -p pacquet-package-manager
  • RUSTDOCFLAGS="-D warnings" cargo doc -p pacquet-package-manager --no-deps

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

Summary by CodeRabbit

  • Bug Fixes

    • pnpm install now detects edits inside file: (local) dependencies and will re-run a full install to refetch them (no longer reports "Already up to date"), aligning behavior with pnpm v10.
  • Tests

    • Added coverage ensuring local file-style dependency specs prevent the optimistic "up-to-date" fast path and are treated as outdated when appropriate.
  • Chores

    • Updated spell check allowlist with two additional terms.

@truffle-dev truffle-dev requested a review from zkochan as a code owner June 10, 2026 21:32
@welcome

welcome Bot commented Jun 10, 2026

Copy link
Copy Markdown

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

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

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (10) 📘 Rule violations (0) 📎 Requirement gaps (1) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

Grey Divider


Action required

1. file: refresh integration test missing 📎 Requirement gap ☼ Reliability
Description
The PR adds unit-level coverage for the new treatLocalFileDepsAsOutdated bail-out, but it does not
add a regression test that installs a file: *directory* dependency, modifies a file inside it,
reruns pnpm install, and asserts the updated content appears in node_modules. This leaves PR
Compliance ID 4’s required end-to-end change-detection behavior unverified.
Code

deps/status/test/checkDepsStatus.test.ts[R794-848]

+describe('checkDepsStatus - treatLocalFileDepsAsOutdated', () => {
+  beforeEach(() => {
+    jest.resetModules()
+    jest.clearAllMocks()
+  })
+
+  const currentLockfile = {
+    lockfileVersion: '9.0',
+    importers: { '.': { specifiers: {} } },
+  } as unknown as LockfileObject
+
+  const mockWorkspaceState = (lastValidatedTimestamp: number): WorkspaceState => ({
+    lastValidatedTimestamp,
+    pnpmfiles: [],
+    settings: {
+      excludeLinksFromLockfile: false,
+      linkWorkspacePackages: true,
+      preferWorkspacePackages: true,
+    },
+    projects: {},
+    filteredInstall: false,
+  })
+
+  function mockUpToDateSingleProjectStats (lastValidatedTimestamp: number): void {
+    jest.mocked(fsUtils.safeStat).mockImplementation(async () => ({
+      mtime: new Date(lastValidatedTimestamp - 10_000),
+      mtimeMs: lastValidatedTimestamp - 10_000,
+    } as Stats))
+    jest.mocked(fsUtils.safeStatSync).mockReturnValue(undefined)
+    jest.mocked(statManifestFileUtils.statManifestFile).mockImplementation(async () => ({
+      mtime: new Date(lastValidatedTimestamp - 20_000),
+      mtimeMs: lastValidatedTimestamp - 20_000,
+    } as Stats))
+    jest.mocked(lockfileFs.readCurrentLockfile).mockImplementation(async () => currentLockfile)
+    jest.mocked(lockfileFs.readWantedLockfile).mockResolvedValue(null)
+  }
+
+  it('returns upToDate: false when the root manifest has a file: dependency', async () => {
+    const lastValidatedTimestamp = Date.now() - 10_000
+    jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState(lastValidatedTimestamp))
+
+    const opts: CheckDepsStatusOptions = {
+      rootProjectManifest: {
+        dependencies: { foo: 'file:../foo' },
+      },
+      rootProjectManifestDir: '/project',
+      pnpmfile: [],
+      treatLocalFileDepsAsOutdated: true,
+      ...mockWorkspaceState(lastValidatedTimestamp).settings,
+    }
+    const result = await checkDepsStatus(opts)
+
+    expect(result.upToDate).toBe(false)
+    expect(result.issue).toBe('The dependency "foo" is a local file dependency and its contents may have changed')
+  })
Evidence
PR Compliance ID 4 requires a regression test that verifies node_modules content refresh after
editing files inside a file: directory dependency. The added tests only assert checkDepsStatus()
returns upToDate: false (and pacquet returns Decision::Skipped) when local file specs are
present; they do not perform the install→edit→reinstall→assert-updated-contents sequence.

Regression coverage: v11 behavior for file: directory dependencies matches v10 change detection expectations
deps/status/test/checkDepsStatus.test.ts[794-1172]
pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs[158-507]

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

## Issue description
PR Compliance ID 4 requires automated regression coverage that reproduces the reported v11 regression: install with a `file:` directory dependency, modify a file in the source directory, rerun `pnpm install`, and assert the dependency content in `node_modules` updates. The current PR only tests that the fast-path is skipped/marked outdated; it does not assert that `node_modules/<dep>` actually refreshes.
## Issue Context
The new logic forces a full install for projects with local file dependencies, but without an integration-style test that reads `node_modules/<dep>/...` before/after a source edit, the core expected behavior remains unpinned.
## Fix Focus Areas
- installing/deps-installer/test/install/local.ts[82-97]
- deps/status/test/checkDepsStatus.test.ts[794-1172]

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


2. Root manifest not scanned 🐞 Bug ≡ Correctness
Description
When treatLocalFileDepsAsOutdated is enabled and allProjects is provided, _checkDepsStatus()
only scans manifests from allProjects and ignores rootProjectManifest, so a local file
dependency declared only in the root manifest can be missed and the optimistic repeat-install fast
path may still report up-to-date.
Code

deps/status/src/checkDepsStatus.ts[R164-167]

+  if (opts.treatLocalFileDepsAsOutdated) {
+    const manifests = allProjects?.map(({ manifest }) => manifest) ??
+      (rootProjectManifest ? [rootProjectManifest] : [])
+    const localFileDep = findLocalFileDep(manifests)
Evidence
The bailout’s scan input is allProjects?.map(... ) ?? [rootProjectManifest], so
rootProjectManifest is skipped whenever allProjects is set. The root manifest is tracked
separately in ConfigContext, and the CLI can exclude the workspace root from the recursive project
list when includeWorkspaceRoot is false, making it possible for allProjects to not contain the
root project even when rootProjectManifest exists.

deps/status/src/checkDepsStatus.ts[164-175]
config/reader/src/Config.ts[38-49]
pnpm/src/main.ts[252-274]

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 `treatLocalFileDepsAsOutdated` is enabled, the local-file dependency scan ignores `rootProjectManifest` whenever `allProjects` is present. If a caller provides `allProjects` that does not include the workspace root project, a root-only local file dependency will not trigger the intended bailout and the optimistic repeat-install fast path can incorrectly short-circuit.
## Issue Context
`ConfigContext` carries `rootProjectManifest` separately from `allProjects`, and in recursive mode the workspace root can be excluded from the projects list depending on filters and `includeWorkspaceRoot`.
## Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[164-167]
## Suggested fix
Build the `manifests` list to include:
- all manifests from `allProjects` (if present)
- plus `rootProjectManifest` when defined and not already represented by an entry in `allProjects` (e.g., by checking `project.rootDir === rootProjectManifestDir`).

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


3. Local override deps missed ✓ Resolved 🐞 Bug ≡ Correctness
Description
When treatLocalFileDepsAsOutdated is enabled, checkDepsStatus() only scans dependency fields in
project manifests, so local file:/path overrides can still allow the optimistic repeat-install
fast path to report upToDate: true and skip reinstalling after the overridden local package
changes.
Code

deps/status/src/checkDepsStatus.ts[R164-175]

+  if (opts.treatLocalFileDepsAsOutdated) {
+    const manifests = allProjects?.map(({ manifest }) => manifest) ??
+      (rootProjectManifest ? [rootProjectManifest] : [])
+    const localFileDep = findLocalFileDep(manifests)
+    if (localFileDep != null) {
+      return {
+        upToDate: false,
+        issue: `The dependency "${localFileDep}" is a local file dependency and its contents may have changed`,
+        workspaceState,
+      }
+    }
+  }
Evidence
installDeps() enables the new option to prevent optimistic short-circuits; however, the
implementation only checks DEPENDENCIES_FIELDS in manifests and checkDepsStatus() can still
return upToDate: true based on manifest mtimes alone. Separately, overrides are shown (by existing
tests) to support file: specifiers and to produce pkg@file:... resolutions, meaning they can
create local file dependencies without being present in any manifest dependency field—so the new
bail-out can miss them.

installing/commands/src/installDeps.ts[181-196]
deps/status/src/checkDepsStatus.ts[164-175]
deps/status/src/checkDepsStatus.ts[296-307]
deps/status/src/checkDepsStatus.ts[655-663]
installing/deps-installer/test/install/overrides.ts[198-223]

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

## Issue description
`treatLocalFileDepsAsOutdated` currently only checks manifest dependency fields for local file specs. Local file dependencies can also be introduced via `overrides` (e.g. `overrides: { foo: 'file:./overrides/pkg' }`). When such an override’s target directory/tarball changes, none of the tracked mtimes necessarily move, so the optimistic repeat-install path can still short-circuit and leave `node_modules` stale.
### Issue Context
- `installDeps()` enables `treatLocalFileDepsAsOutdated` specifically to prevent incorrect optimistic short-circuits.
- `checkDepsStatus()` can return `upToDate: true` purely when no manifest mtimes changed.
- `overrides` supports `file:` specifiers and produces `pkg@file:...` resolutions.
### Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[164-175]
- deps/status/src/checkDepsStatus.ts[655-688]
### Suggested fix
- Extend the `treatLocalFileDepsAsOutdated` guard to also scan `opts.overrides` values for local file specs using the same `isLocalFileSpec()` predicate.
- Exclude `link:` overrides (same reasoning as for dependencies).
- Update/add a unit test that sets `treatLocalFileDepsAsOutdated: true` with `overrides` containing a `file:` spec but no manifest `file:` deps, and asserts `upToDate: false` with a clear issue message.

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


View more (1)
4. Pacquet misses bare tarballs ✓ Resolved 🐞 Bug ≡ Correctness
Description
pacquet’s local file dependency detection only matches file:-prefixed specs, so the optimistic
repeat-install fast path can still incorrectly report UpToDate/short-circuit when manifests use
supported local tarball path dependencies like ./vendor/pkg.tgz that may change without
manifest/lockfile mtime updates. This means repacked local tarballs can be missed even though
pacquet/pnpm resolve bare ./*.tgz specs as local tarball dependencies.
Code

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

+fn has_local_file_dep(project_manifests: &[(PathBuf, &PackageManifest)]) -> bool {
+    const FIELDS: [&str; 3] = ["dependencies", "devDependencies", "optionalDependencies"];
+    project_manifests.iter().any(|(_, manifest)| {
+        FIELDS.iter().any(|field| {
+            manifest.value().get(*field).and_then(|value| value.as_object()).is_some_and(|deps| {
+                deps.values()
+                    .any(|spec| spec.as_str().is_some_and(|spec| spec.starts_with("file:")))
+            })
+        })
Evidence
Both reports point out that the bailout/detection logic is implemented as a simple prefix check
(spec.startsWith('file:') / spec.starts_with("file:")), so only explicit file: dependencies
trigger treatLocalFileDepsAsOutdated or the optimistic repeat-install guard. The cited resolver
behavior/tests show that bare local tarball paths like ./something.tgz are a supported input form
and are routed through the local-path resolver, ultimately resolving to a local file: tarball id;
therefore, if the optimistic path skips install based on the narrow file:-only check, it can miss
changes when the tarball is repacked while the manifest/lockfile mtimes remain unchanged.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[278-295]
pacquet/crates/resolving-local-resolver/tests/resolve.rs[220-250]
deps/status/src/checkDepsStatus.ts[648-665]
resolving/default-resolver/src/index.ts[132-152]
resolving/local-resolver/src/parseBareSpecifier.ts[57-71]
resolving/local-resolver/src/index.ts[79-96]

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 optimistic repeat-install/dep-status logic currently treats a project as safe to short-circuit unless a local file dependency is detected, but the detection only matches dependency specs that start with `file:`. pacquet/pnpm also support local tarball dependencies specified as bare filesystem paths (e.g. `./vendor/pkg.tgz`), which can change without manifest/lockfile mtime changes, so the fast path can still incorrectly report `UpToDate`/skip work.
## Issue Context
- The bailout is intended to run before mtime-only gates.
- The resolver already recognizes local tarball filenames/paths and routes bare `./*.tgz` specs through the local-path resolver (i.e., they are effectively local file/tarball deps even without an explicit `file:` prefix).
- To avoid false positives for remote tarball URLs, detection should distinguish local paths from URL-like specs.
## Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[278-295]
- pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs[133-196]
- deps/status/src/checkDepsStatus.ts[654-665]
- deps/status/test/checkDepsStatus.test.ts[793-926]

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



Remediation recommended

5. Extension local deps missed 🐞 Bug ≡ Correctness
Description
When treatLocalFileDepsAsOutdated is enabled, _checkDepsStatus() only scans project manifests
and pnpm.overrides, so a local file dependency injected via packageExtensions can still lead to
upToDate: true and skip the reinstall/refetch. This can leave stale contents in node_modules for
the injected local file dependency even after its directory/tarball contents change.
Code

deps/status/src/checkDepsStatus.ts[R173-198]

+  if (opts.treatLocalFileDepsAsOutdated) {
+    const manifests = allProjects?.map(({ manifest }) => manifest) ?? []
+    // `rootProjectManifest` is tracked separately from `allProjects` and the
+    // recursive project list can omit the workspace root (for example when
+    // `includeWorkspaceRoot` is false), so scan it too unless `allProjects`
+    // already covers it.
+    if (rootProjectManifest != null && !allProjects?.some(({ rootDir }) => rootDir === rootProjectManifestDir)) {
+      manifests.push(rootProjectManifest)
+    }
+    const localFileDep = findLocalFileDep(manifests, opts.include, catalogs)
+    if (localFileDep != null) {
+      return {
+        upToDate: false,
+        issue: `The dependency "${localFileDep}" is a local file dependency and its contents may have changed`,
+        workspaceState,
+      }
+    }
+    const localFileOverride = findLocalFileOverride(opts.overrides, catalogs)
+    if (localFileOverride != null) {
+      return {
+        upToDate: false,
+        issue: `The override "${localFileOverride}" maps to a local file dependency and its contents may have changed`,
+        workspaceState,
+      }
+    }
+  }
Evidence
The new guard only checks manifests and overrides, but packageExtensions are applied later to extend
manifests with additional dependency fields, so local file specs can exist in the actual install
graph without being visible to the guard.

deps/status/src/checkDepsStatus.ts[168-198]
deps/status/src/checkDepsStatus.ts[681-713]
hooks/read-package-hook/src/createPackageExtender.ts[12-45]

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

## Issue description
`checkDepsStatus()`'s new local-file-dependency bail-out (enabled by `treatLocalFileDepsAsOutdated`) does not consider dependencies introduced via `packageExtensions`. Because `packageExtensions` are applied as a read-package hook during install, they can inject `file:`/local-path/tarball specs into dependency manifests without those specs appearing in any project manifest or overrides, allowing the optimistic repeat-install fast path to incorrectly return up-to-date.
### Issue Context
- `_checkDepsStatus()` only checks `findLocalFileDep(manifests, ...)` + `findLocalFileOverride(overrides, ...)` under `treatLocalFileDepsAsOutdated`.
- `packageExtensions` can add entries into `dependencies`/`optionalDependencies` of manifests at install-time.
### Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[168-198]
- deps/status/src/checkDepsStatus.ts[681-713]
### What to change
- Add a lightweight scan of `opts.packageExtensions` under `treatLocalFileDepsAsOutdated`.
- Treat any `packageExtensions[selector].dependencies` entry with an `isLocalFileSpec()` (and optionally catalog-dereferenced `catalog:`) as grounds to return `{ upToDate: false, issue: ... }`.
- Respect `opts.include?.optionalDependencies === false` when checking `optionalDependencies` from packageExtensions (peer deps can be ignored since they aren’t installed).

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


6. Pacquet extension deps missed 🐞 Bug ≡ Correctness
Description
Pacquet’s optimistic repeat-install check skips the fast path based on local file deps found in
project manifests/overrides, but it does not consider local file dependencies injected via
config.package_extensions. Since pacquet applies package_extensions as a manifest hook during
the full install path, the optimistic check can still return Decision::UpToDate even though a
local file dependency is present only after extension application and may have changed contents.
Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R163-179]

+    // Unconditional where upstream gates it behind
+    // `treatLocalFileDepsAsOutdated`: the only caller here is the
+    // install command — the one consumer that sets the flag upstream.
+    if has_local_file_dep(project_manifests, included, catalogs) {
+        return Decision::Skipped {
+            reason: "a dependency is a local file dependency and its contents may have changed",
+        };
+    }
+    match has_local_file_override(config, catalogs) {
+        Ok(true) => {
+            return Decision::Skipped {
+                reason: "an override maps to a local file dependency and its contents may have changed",
+            };
+        }
+        Err(reason) => return Decision::Skipped { reason },
+        Ok(false) => {}
+    }
Evidence
Pacquet applies package extensions during install-time manifest processing, but the optimistic
repeat-install local-file guard only examines the raw project manifests and overrides, so
extension-injected local file deps can be invisible to the guard.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[163-179]
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs[840-879]

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_optimistic_repeat_install()` bails on local file deps by scanning `project_manifests` and `pnpm.overrides`, but it does not account for local file dependencies introduced through `Config::package_extensions`, which are applied as a manifest hook during full resolution. This means the fast path can still conclude `UpToDate` in setups where a local file dep exists only via package extensions.
### Issue Context
- Full installs apply `config.package_extensions` to manifests before resolution.
- The optimistic repeat-install precheck runs earlier and currently doesn’t scan `config.package_extensions` for local path/tarball/file specs.
### Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[163-179]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[311-455]
### What to change
- Add a precheck that scans `config.package_extensions` entries for local file specs using `is_local_file_spec()`.
- Consider `included.optional_dependencies` when evaluating optionalDependencies-style extensions (if represented in pacquet’s `PackageExtension` type).
- If a local file spec is found in package extensions, return `Decision::Skipped` with a clear reason (e.g. "package_extensions include a local file dependency and its contents may have changed").

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


7. Git .tgz shorthand false-positive 🐞 Bug ➹ Performance
Description
isLocalFileSpec (and pacquet’s is_local_file_spec) treats any colon-free spec ending in
.tgz/.tar.gz/.tar as a local file dependency, so valid git shorthands like
user/repo#release.tgz are misclassified and will always skip the optimistic repeat-install fast
path. This doesn’t break correctness of installs, but it unnecessarily forces full installs
(performance regression) for that dependency shape.
Code

deps/status/src/checkDepsStatus.ts[R753-758]

+function isLocalFileSpec (spec: string): boolean {
+  if (spec.startsWith('file:')) return true
+  if (LOCAL_PATH_PREFIX.test(spec)) return true
+  if (spec.includes(':')) return false
+  return LOCAL_TARBALL_EXTENSION.test(spec)
+}
Evidence
The new heuristic returns true for colon-free specs that end with tarball extensions, which will
match git shorthands that include a #committish ending in .tgz. The git resolver accepts
user/repo-style shorthands via HostedGit.fromUrl, so such specs are valid git dependencies and
should not force the local-file-deps bail purely due to the committish suffix.

deps/status/src/checkDepsStatus.ts[732-758]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[397-431]
resolving/git-resolver/src/parseBareSpecifier.ts[34-58]

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

## Issue description
`isLocalFileSpec()` (TypeScript) and `is_local_file_spec()` (Rust) classify any spec without a colon that ends in `.tgz`/`.tar.gz`/`.tar` as a local file dependency. This unintentionally matches hosted-git shorthands with a `#committish` suffix that happens to end with one of these extensions (e.g. `user/repo#release.tgz`), disabling the optimistic repeat-install fast path even though the dependency is git-resolved.
### Issue Context
Git shorthand parsing is handled by the git resolver via `HostedGit.fromUrl(bareSpecifier)`, which accepts `user/repo#<committish>`.
### Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[753-758]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[414-431]
### Suggested fix
Refine the tarball-suffix heuristic to exclude git-shorthand forms. Practical options:
- If `spec.includes('#')`, treat it as **not** a local tarball unless it is explicitly `file:` or path-prefixed (./, ../, ~/, absolute, drive path).
- Or, add a small git-shorthand detector for `user/repo#...` (without requiring a protocol) and exclude those from the tarball-suffix match.
Add a unit test covering `user/repo#release.tgz` staying on the fast path (no local-file bail).

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


View more (7)
8. Unnecessary catalog lookup 🐞 Bug ➹ Performance
Description
findLocalFileDep() calls resolveFromCatalog() for every dependency spec (even non-"catalog:" ones),
adding avoidable per-dependency work on the optimisticRepeatInstall fast path. This can slow the
common "Already up to date" case on projects with large dependency lists.
Code

deps/status/src/checkDepsStatus.ts[R703-705]

+        if (isLocalFileSpec(spec)) return depName
+        const catalogResult = resolveFromCatalog(catalogs ?? {}, { alias: depName, bareSpecifier: spec })
+        if (catalogResult.type === 'found' && isLocalFileSpec(catalogResult.resolution.specifier)) return depName
Evidence
The new scan calls resolveFromCatalog() even when a dependency spec is not a catalog: reference.
The catalog protocol parser shows that catalog: is the only prefix that can trigger a lookup; the
Rust port already short-circuits on that prefix to avoid unnecessary allocations/work, indicating
the intended optimization pattern.

deps/status/src/checkDepsStatus.ts[693-706]
catalogs/protocol-parser/src/parseCatalogProtocol.ts[7-17]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[359-368]

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

## Issue description
`findLocalFileDep()` unconditionally calls `resolveFromCatalog()` for every dependency spec that is not already classified as a local-file spec. Only `catalog:` specs can resolve via catalogs, so this adds unnecessary work to the optimistic repeat-install fast path.
### Issue Context
This code runs when `installDeps()` uses the optimisticRepeatInstall short-circuit and now always enables `treatLocalFileDepsAsOutdated`. In the common case (no local file deps), the scan iterates all deps; avoiding needless catalog resolution calls keeps this path as cheap as possible.
### Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[693-706]
### Suggested change
Add a cheap guard before calling `resolveFromCatalog`, e.g.:
- `if (spec.startsWith('catalog:')) { ... resolveFromCatalog ... }`
This matches the Rust port’s explicit short-circuit and avoids extra work for the vast majority of dependency specs.

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


9. Fast-path allocates per dep 🐞 Bug ➹ Performance
Description
pacquet’s optimistic repeat-install check now allocates new Strings per dependency when probing
catalog resolution and also allocates a lowercased copy of most non-protocol specs to check tarball
suffixes. This adds avoidable CPU/memory overhead to the repeat-install hot path on large
workspaces/manifests.
Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R355-362]

+fn catalog_resolves_to_local_file(catalogs: &Catalogs, alias: &str, spec: &str) -> bool {
+    match resolve_from_catalog(
+        catalogs,
+        &WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() },
+    ) {
+        CatalogResolutionResult::Found(found) => is_local_file_spec(&found.resolution.specifier),
+        _ => false,
+    }
Evidence
The pacquet repeat-install check calls catalog_resolves_to_local_file() for each dependency and
that function constructs owned Strings (to_string()) for WantedDependency, and
is_local_file_spec() allocates with to_ascii_lowercase() for specs without : (common for
semver ranges). The catalog resolver itself returns Unused for non-catalog: specs, so the call
can be gated before allocating.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[322-363]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[404-420]
pacquet/crates/catalogs-resolver/src/lib.rs[118-125]

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_optimistic_repeat_install` now performs avoidable heap allocations while scanning manifests for local-file deps:
- `catalog_resolves_to_local_file()` allocates `alias.to_string()` and `spec.to_string()` even when the spec is not `catalog:`.
- `is_local_file_spec()` allocates via `to_ascii_lowercase()` for most semver specs (no `:` and no path prefix), even though it only needs a case-insensitive suffix check.
This work runs on every optimistic repeat-install attempt, so on large projects it can measurably erode the benefit of the fast path.
### Issue Context
`resolve_from_catalog()` already returns `Unused` for non-`catalog:` specifiers, so `catalog_resolves_to_local_file()` can cheaply short-circuit before constructing an owned `WantedDependency`.
### Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[332-363]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[404-420]
- pacquet/crates/catalogs-resolver/src/lib.rs[118-125]
### Suggested approach
1. In `catalog_resolves_to_local_file()`, return `false` immediately unless `spec.starts_with("catalog:")` to avoid per-dep `String` allocations.
2. In `is_local_file_spec()`, replace `to_ascii_lowercase()` with an ASCII-case-insensitive suffix check on bytes (e.g., check the last N bytes for `.tgz`, `.tar.gz`, `.tar` using `eq_ignore_ascii_case`) to avoid allocations.

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


10. Misleading override skip reason 🐞 Bug ◔ Observability
Description
In pacquet’s optimistic repeat-install check, has_local_file_override() returns true on any
pnpm.overrides parse error, but the caller’s skip reason claims an override maps to a local file
dependency. This can mislead troubleshooting when the real cause is an invalid override selector or
misconfigured catalog rather than a local override.
Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R357-362]

+fn has_local_file_override(config: &Config, catalogs: &Catalogs) -> bool {
+    match crate::install::parse_config_overrides(config, catalogs) {
+        Ok(Some(overrides)) => overrides.iter().any(|o| is_local_file_spec(&o.new_bare_specifier)),
+        Ok(None) => false,
+        Err(_) => true,
+    }
Evidence
The skip reason for overrides is emitted whenever has_local_file_override() returns true, but
has_local_file_override() returns true even when override parsing fails (Err(_) => true). The
parse error originates from parse_overrides_iter(...) failing (invalid selector / catalog issue),
which is unrelated to mapping to a local file dependency.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[158-170]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[345-363]
pacquet/crates/package-manager/src/install.rs[1226-1244]

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

## Issue description
`has_local_file_override()` treats *any* override parsing failure as `true`, which causes the optimistic repeat-install fast path to be skipped under the reason "an override maps to a local file dependency...". When the failure is actually an invalid override selector / catalog misconfiguration, this reason is inaccurate and makes debugging harder.
### Issue Context
- `has_local_file_override()` currently collapses `Err(_)` into `true`.
- The caller emits a fixed "local file override" skip reason whenever the function returns `true`.
- `parse_config_overrides()` errors indicate override parsing/validation failure, not necessarily a local file override.
### Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[158-170]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[357-363]
### Suggested fix
- Change `has_local_file_override` to return a tri-state (e.g. `enum OverrideCheck { None, LocalFile, ParseError }` or `Result<bool, ()>`).
- In `check_optimistic_repeat_install`, emit a distinct static skip reason for parse errors (e.g. `"pnpm.overrides cannot be parsed"`, matching the existing message used in the modified-manifests content check), and keep the current message only for the true “local file override present” case.
- (Optional) Add a unit test covering invalid overrides triggering the parse-error reason path.

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


11. Excluded deps disable fast path 🐞 Bug ➹ Performance
Description
When treatLocalFileDepsAsOutdated is enabled, checkDepsStatus() bails if it finds a local file spec
in devDependencies/optionalDependencies even when those groups are excluded for the current install,
unnecessarily forcing a full install. This reduces optimisticRepeatInstall’s effectiveness for
installs like --no-optional/--no-dev where the local file deps wouldn’t be installed anyway.
Code

deps/status/src/checkDepsStatus.ts[R668-677]

+function findLocalFileDep (manifests: ProjectManifest[]): string | undefined {
+  for (const manifest of manifests) {
+    for (const depField of DEPENDENCIES_FIELDS) {
+      const deps = manifest[depField]
+      if (deps == null) continue
+      for (const [depName, spec] of Object.entries(deps)) {
+        // A malformed manifest may carry a non-string spec; skip it rather
+        // than throw — checkDepsStatus() must never crash.
+        if (typeof spec === 'string' && isLocalFileSpec(spec)) return depName
+      }
Evidence
The bailout uses findLocalFileDep(), which iterates DEPENDENCIES_FIELDS (optional/dev/prod) without
checking which groups are included. However, the install command derives an include/includeDirect
object from flags like opts.optional !== false, and dependency selection uses that include object
to omit excluded sections—meaning local file specs in excluded groups should not require refetching
and therefore shouldn’t disable the fast path.

deps/status/src/checkDepsStatus.ts[155-179]
deps/status/src/checkDepsStatus.ts[662-681]
installing/commands/src/install.ts[392-407]
pkg-manifest/utils/src/filterDependenciesByType.ts[3-11]

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

## Issue description
`checkDepsStatus()`'s `treatLocalFileDepsAsOutdated` bailout scans all dependency groups (`optionalDependencies`, `dependencies`, `devDependencies`) unconditionally. If an install excludes a group (e.g. `--no-optional`, `--no-dev`), local file specs in the excluded group still force `upToDate: false`, unnecessarily disabling the optimistic repeat-install fast path.
## Issue Context
Install-time dependency selection is controlled via include/includeDirect flags derived from CLI options like `--no-optional`, and these flags determine which dependency sections are actually installed.
## Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[155-179]
- deps/status/src/checkDepsStatus.ts[662-681]
### Suggested approach
- Extend `CheckDepsStatusOptions` to optionally accept `includeDirect?: IncludedDependencies` (or derive from existing `dev`/`optional`/`production` settings if that’s the canonical source).
- Update `findLocalFileDep()` to only scan dependency fields that are included for this install (e.g., skip `optionalDependencies` when `includeDirect.optionalDependencies` is false; skip `devDependencies` when `includeDirect.devDependencies` is false).
- Add/adjust tests for `treatLocalFileDepsAsOutdated` ensuring `--no-optional` / excluded groups do not trigger the bailout when local file specs exist only in excluded sections.

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


12. Pacquet bail ignores include 🐞 Bug ➹ Performance
Description
pacquet’s optimistic repeat-install bailout scans dependencies/devDependencies/optionalDependencies
unconditionally, so a local file spec in an excluded group still forces Decision::Skipped and a full
install. This can unnecessarily disable the fast path in configurations like “no optional
dependencies”.
Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R292-300]

+fn has_local_file_dep(project_manifests: &[(PathBuf, &PackageManifest)]) -> bool {
+    const FIELDS: [&str; 3] = ["dependencies", "devDependencies", "optionalDependencies"];
+    project_manifests.iter().any(|(_, manifest)| {
+        FIELDS.iter().any(|field| {
+            manifest.value().get(*field).and_then(|value| value.as_object()).is_some_and(|deps| {
+                deps.values().any(|spec| spec.as_str().is_some_and(is_local_file_spec))
+            })
+        })
+    })
Evidence
has_local_file_dep() currently scans all three dependency sections unconditionally. But pacquet’s
lockfile filtering explicitly drops dev/optional sections when the corresponding
IncludedDependencies flags are false, demonstrating that excluded groups are not part of the install
output and shouldn’t necessarily invalidate the repeat-install fast path.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[285-301]
pacquet/crates/package-manager/src/current_lockfile.rs[112-136]

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

## Issue description
`has_local_file_dep()` checks for local file specs in `dependencies`, `devDependencies`, and `optionalDependencies` regardless of which dependency groups the install actually includes. If `included.optional_dependencies` or `included.dev_dependencies` is false, local file specs in those excluded groups should not force the fast path to bail.
## Issue Context
The repeat-install check already receives `included: IncludedDependencies`, and other pacquet code paths use `IncludedDependencies` to drop excluded dependency sections.
## Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[156-169]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[285-301]
### Suggested approach
- Change `has_local_file_dep(project_manifests)` to accept `included: IncludedDependencies`.
- Only scan `devDependencies` if `included.dev_dependencies` is true; only scan `optionalDependencies` if `included.optional_dependencies` is true; only scan `dependencies` if `included.dependencies` is true.
- Add a test case to `optimistic_repeat_install/tests.rs` showing a local file spec in `optionalDependencies` does **not** bail when `IncludedDependencies { optional_dependencies: false, ... }`.

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


13. Pacquet catalog locals missed 🐞 Bug ≡ Correctness
Description
Pacquet’s optimistic repeat-install local-file bail checks only the raw manifest/override spec
strings with is_local_file_spec()/isLocalFileSpec(), so catalog:... entries never trigger the
bail even when the catalog ultimately resolves to a bare local path or tarball. As a result, the
fast path can incorrectly short-circuit repeat installs and report up-to-date while local-catalog
dependency contents have changed and remain unfetched.
Code

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[R309-346]

+fn has_local_file_override(config: &Config) -> bool {
+    config
+        .overrides
+        .as_ref()
+        .is_some_and(|overrides| overrides.values().any(|spec| is_local_file_spec(spec)))
+}
+
+/// Whether the specifier resolves to a local directory or tarball whose
+/// contents can change without any manifest or lockfile mtime moving:
+/// the `file:` protocol, path-prefixed specs (`./`, `../`, `~/`,
+/// absolute POSIX paths, and Windows drive paths including
+/// drive-relative ones like `c:dir`), and bare tarball file names.
+/// Port of upstream's `isLocalFileSpec` in
+/// `deps/status/src/checkDepsStatus.ts`.
+///
+/// Deliberately narrower than the local resolver's bare-path matching:
+/// a bare path like `user/repo` is statically indistinguishable from a
+/// git shorthand at this layer, and matching it would disable the
+/// repeat-install fast path for every project with git dependencies.
+/// Such specs (and anything else carrying a protocol or URL) stay on
+/// the fast path.
+fn is_local_file_spec(spec: &str) -> bool {
+    if spec.starts_with("file:") {
+        return true;
+    }
+    if spec.starts_with(['.', '/', '\\'])
+        || spec.starts_with("~/")
+        || spec.starts_with(r"~\")
+        || is_windows_drive_path(spec)
+    {
+        return true;
+    }
+    if spec.contains(':') {
+        return false;
+    }
+    let spec = spec.to_ascii_lowercase();
+    spec.ends_with(".tgz") || spec.ends_with(".tar.gz") || spec.ends_with(".tar")
+}
Evidence
The cited logic determines “local” purely from the unexpanded dependency/override specifier string
and explicitly treats any value containing : as non-local, which excludes catalog: and prevents
it from triggering the intended bail. At the same time, catalog resolution is allowed to yield
arbitrary bare specifiers (including local paths or tarballs), and later stages already
resolve/install via catalog resolution (and pacquet already performs catalog expansion for overrides
elsewhere), so leaving catalog: unexpanded at the bail decision point means catalog-indirected
local dependencies can bypass the local-file outdated check.

pacquet/crates/package-manager/src/optimistic_repeat_install.rs[156-169]
pacquet/crates/package-manager/src/optimistic_repeat_install.rs[303-346]
pacquet/crates/catalogs-resolver/src/lib.rs[147-180]
pacquet/crates/package-manager/src/install.rs[1226-1244]
deps/status/src/checkDepsStatus.ts[164-183]
deps/status/src/checkDepsStatus.ts[690-712]
catalogs/resolver/src/resolveFromCatalog.ts[88-126]
config/parse-overrides/src/index.ts[20-43]
installing/deps-resolver/src/resolveDependencyTree.ts[180-186]

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 optimistic repeat-install “local file dependency” bail is intended to disable the fast path when any dependency/override ultimately points to a mutable local directory or tarball, but it currently only inspects the *raw* specifier strings; because `is_local_file_spec()`/`isLocalFileSpec()` returns false for any spec containing `:`, `catalog:...` indirection is never dereferenced and can hide a local path/tarball, allowing the fast path to incorrectly proceed.
## Issue Context
- `is_local_file_spec()` / `isLocalFileSpec()` classifies specs as local only for `file:`, path-prefixed, or bare tarballs, and explicitly treats any `:`-containing spec (including `catalog:`) as non-local.
- Catalog resolution can legally return arbitrary specifiers (it only bans `workspace:`, `link:`, and `file:`), so a catalog entry can be a bare local path (e.g. `../lib`) or tarball (e.g. `vendor/pkg.tgz`) that later installs as a local dependency.
- Pacquet already resolves `catalog:` in overrides for other checks (e.g., override parsing/resolution used for lockfile comparisons), indicating `config.overrides` may contain raw `catalog:` values that should be expanded before applying the local-file bail.
## Fix Focus Areas
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[156-169]
- pacquet/crates/package-manager/src/optimistic_repeat_install.rs[303-346]
- deps/status/src/checkDepsStatus.ts[164-183]
- deps/status/src/checkDepsStatus.ts[690-712]

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


14. PnP skips local-file bail ✓ Resolved 🐞 Bug ≡ Correctness
Description
In _checkDepsStatus(), the nodeLinker === 'pnp' early return happens before the new
treatLocalFileDepsAsOutdated logic, so the optimistic repeat-install check can still return
upToDate: true under PnP even when local file deps/overrides are present. Since installDeps()
always sets treatLocalFileDepsAsOutdated: true on the optimistic path, this leaves the PR’s
intended “local file deps force real install” behavior ineffective for PnP installs.
Code

deps/status/src/checkDepsStatus.ts[R164-183]

+  if (opts.treatLocalFileDepsAsOutdated) {
+    const manifests = allProjects?.map(({ manifest }) => manifest) ??
+      (rootProjectManifest ? [rootProjectManifest] : [])
+    const localFileDep = findLocalFileDep(manifests)
+    if (localFileDep != null) {
+      return {
+        upToDate: false,
+        issue: `The dependency "${localFileDep}" is a local file dependency and its contents may have changed`,
+        workspaceState,
+      }
+    }
+    const localFileOverride = findLocalFileOverride(opts.overrides)
+    if (localFileOverride != null) {
+      return {
+        upToDate: false,
+        issue: `The override "${localFileOverride}" maps to a local file dependency and its contents may have changed`,
+        workspaceState,
+      }
+    }
+  }
Evidence
The PnP-specific early return occurs before the newly-added local-file-dependency/override
detection, and installDeps() always enables that detection on the optimistic repeat-install path;
workspace state is still persisted under node_modules, so repeat-install checks can run under PnP.

deps/status/src/checkDepsStatus.ts[146-183]
installing/commands/src/installDeps.ts[177-196]
workspace/state/src/filePath.ts[1-4]
workspace/state/src/updateWorkspaceState.ts[21-28]

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

## Issue description
`_checkDepsStatus()` returns early for `nodeLinker === 'pnp'` before running the new `treatLocalFileDepsAsOutdated` checks. This means the optimistic repeat-install shortcut can still report up-to-date under PnP even when local file dependencies/overrides exist.
## Issue Context
`installDeps()` sets `treatLocalFileDepsAsOutdated: true` specifically to prevent skipping installs when local file deps may have changed. Under PnP, the early return prevents this safeguard from running.
## Fix Focus Areas
- deps/status/src/checkDepsStatus.ts[155-183]
### Suggested fix
Adjust the `nodeLinker === 'pnp'` branch to respect the optimistic-install caller intent. For example:
- If `opts.treatLocalFileDepsAsOutdated` is `true`, do **not** return `upToDate: true`; instead return `upToDate: false` (or `undefined`) with an explanatory issue string, so `installDeps()` will run a real install.
- Keep the existing behavior (warn + `upToDate: true`) for non-install callers (i.e., when the flag is not set), preserving `verifyDepsBeforeRun`’s behavior.

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


Grey Divider

Qodo Logo

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Restores detection of changes inside local file: dependencies by treating projects with local file-style specs as outdated in TypeScript deps-status and Rust optimistic repeat-install; adds tests, updates docs/changeset, and extends cspell allowlist.

Changes

File-Dependency Detection and Fast-Path Prevention

Layer / File(s) Summary
Changeset documentation
.changeset/repeat-install-file-deps.md
Documents the behavioral fix: pnpm install now detects changes inside file: dependencies again by skipping the optimistic repeat-install fast path that only tracked manifest and lockfile modification times.
TypeScript file-dep detection in checkDepsStatus
deps/status/src/checkDepsStatus.ts, deps/status/test/checkDepsStatus.test.ts
CheckDepsStatusOptions adds treatLocalFileDepsAsOutdated?: boolean. findLocalFileDep and isLocalFileSpec scan manifests for local file-style specs; _checkDepsStatus returns out-of-date when the flag is set and a matching dep exists. Tests cover root/workspace manifests, path/tarball formats, and protocol exemptions.
TypeScript install flow integration
installing/commands/src/installDeps.ts
installDeps forwards treatLocalFileDepsAsOutdated: true to checkDepsStatus in the optimistic repeat-install path to force full installs for local file deps.
Rust file-dep guard in optimistic repeat install
pacquet/crates/package-manager/src/optimistic_repeat_install.rs, pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs
Module docs updated; check_optimistic_repeat_install returns Decision::Skipped if any manifest has a local file dep. Implements has_local_file_dep and spec heuristics (including Windows drive-path detection). Tests assert skip for local specs and allow link: and remote specs.
cspell allowlist updates
cspell.json
Adds mtimes and refetches to the spell allowlist.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant installDeps
  participant checkDepsStatus
  participant findLocalFileDep
  participant OptimisticRepeatInstall
  Client->>installDeps: run pnpm install
  installDeps->>checkDepsStatus: checkDepsStatus(..., treatLocalFileDepsAsOutdated: true)
  checkDepsStatus->>findLocalFileDep: scan manifests for local file deps
  findLocalFileDep-->>checkDepsStatus: found / not found
  checkDepsStatus-->>installDeps: return upToDate result
  installDeps->>OptimisticRepeatInstall: attempt optimistic fast-path
  OptimisticRepeatInstall->>findLocalFileDep: has_local_file_dep across manifests
  findLocalFileDep-->>OptimisticRepeatInstall: true / false
  OptimisticRepeatInstall-->>installDeps: Decision::Skipped or Decision::UpToDate
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • pnpm/pnpm#11945: Related changes to check_optimistic_repeat_install early-skip logic; overlaps on fast-path gating.
  • pnpm/pnpm#11943: Related optimistic repeat-install fast-path modifications; touches similar decision flow.
  • pnpm/pnpm#12315: Also modifies the pacquet optimistic repeat-install fast-path and overlaps with decision/skip conditions.

Suggested reviewers

  • zkochan

Poem

🐰 I sniffed the manifests and hopped through mtimes,

file: changed — no more stale installs tonight.
TS checks and Rust guards now watch the gate,
Local deps refetch, installs no longer wait. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately and specifically describes the main fix: detecting changes inside file: dependencies during repeat install across both pacquet and pnpm implementations.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

PR Summary by Qodo

Fix repeat install to invalidate optimistic fast path for file: dependencies
🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

Walkthroughs

Description
• Ensure optimistic repeat-install never short-circuits when a project declares a file: dependency.
• Add a caller-scoped flag so install avoids the fast path, while pnpm run behavior stays unchanged.
• Port the same guard to pacquet and cover the behavior with new TS and Rust tests.
Diagram
graph TD
A["installDeps (optimistic path)"] --> B["checkDepsStatus"] --> C{"treatLocalFileDepsAsOutdated?"}
C -->|"yes + has file:"| D["Report upToDate=false (force real install)"]
C -->|"no / no file:"| E["Mtime-based checks (manifest/lockfile)"]
F["pacquet: check_optimistic_repeat_install"] --> G{"has file: dep?"} --> H["Decision::Skipped (force full install)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fingerprint file: dependency contents
  • ➕ Could preserve the optimistic fast path even with file: deps when contents truly unchanged
  • ➕ More precise than a blanket bail-out
  • ➖ Hard to define correctly for directories vs tarballs (symlinks, ignores, pack rules, permissions)
  • ➖ Potentially expensive (hashing large trees) and still prone to edge cases
  • ➖ Requires new persistent metadata to avoid repeated full scans
2. Persist file: dep snapshot metadata in lockfile/workspace state
  • ➕ Enables correctness without always forcing a full install
  • ➕ Can be optimized with incremental metadata (e.g., per-path mtimes or content hashes)
  • ➖ Schema/format changes and migration complexity
  • ➖ Still needs robust rules for what constitutes 'content change' for file: sources
  • ➖ Adds ongoing maintenance and compatibility burden across implementations (pnpm + pacquet)
3. Disable optimistic repeat install automatically when file: deps exist
  • ➕ Simple behavioral rule aligned with the underlying limitation
  • ➕ Keeps the decision close to user config/feature gating
  • ➖ Still effectively a bail-out; differs mainly in where the logic lives
  • ➖ Could unintentionally affect other callers (e.g., verify-before-run) unless carefully scoped

Recommendation: The chosen approach (caller-scoped bail-out via treatLocalFileDepsAsOutdated on the install fast path, plus an unconditional bail in pacquet where the only caller is install) is the best trade-off: it restores v10-correctness for file: deps with minimal surface area and avoids regressing verify-deps-before-run. More precise solutions require durable fingerprinting metadata and substantially higher complexity/risk.

Grey Divider

File Changes

Bug fix (3)
checkDepsStatus.ts Add optional bail-out for file: dependencies in checkDepsStatus +47/-5

Add optional bail-out for file: dependencies in checkDepsStatus

• Introduces treatLocalFileDepsAsOutdated to force upToDate=false when any manifest declares a file: dependency. Adds findLocalFileDep helper scanning dependency fields (excluding link:) and returns an explanatory issue string.

deps/status/src/checkDepsStatus.ts


installDeps.ts Force install optimistic path to treat file: deps as outdated +1/-0

Force install optimistic path to treat file: deps as outdated

• Sets treatLocalFileDepsAsOutdated: true when installDeps calls checkDepsStatus on the optimistic repeat-install path, ensuring projects with file: deps always run the full install and refetch those inputs.

installing/commands/src/installDeps.ts


optimistic_repeat_install.rs Skip pacquet optimistic repeat install when any file: dependency exists +34/-1

Skip pacquet optimistic repeat install when any file: dependency exists

• Adds an unconditional file: dependency check (has_local_file_dep) that forces Decision::Skipped with a clear reason. Updates module documentation to explain parity with pnpm’s caller-scoped behavior and why link: is excluded.

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


Tests (2)
checkDepsStatus.test.ts Test file: dependency invalidation is opt-in and excludes link: +134/-0

Test file: dependency invalidation is opt-in and excludes link:

• Adds coverage ensuring checkDepsStatus returns upToDate=false when the new option is enabled and file: deps exist in root/workspace manifests. Also pins that behavior is unchanged when the option is unset and that link:/registry specs do not trigger invalidation.

deps/status/test/checkDepsStatus.test.ts


tests.rs Add pacquet tests for file: and link: dependency behavior +62/-0

Add pacquet tests for file: and link: dependency behavior

• Adds tests asserting that any file: dependency (including file: tarballs in devDependencies) prevents short-circuiting, while link: dependencies still allow the fast path to report up-to-date.

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


Documentation (1)
repeat-install-file-deps.md Add changeset describing restored file: repeat-install behavior +7/-0

Add changeset describing restored file: repeat-install behavior

• Adds a patch changeset for affected packages and documents the regression and the fix: file: dependencies now force a real install instead of reporting 'Already up to date'.

.changeset/repeat-install-file-deps.md


Grey Divider

Qodo Logo

@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.

🧹 Nitpick comments (1)
pacquet/crates/package-manager/src/optimistic_repeat_install.rs (1)

278-295: ⚡ Quick win

Prefer the PackageManifest::dependencies API over raw JSON access.

The implementation accesses the raw JSON via manifest.value().get(*field), but the PackageManifest::dependencies method (shown in context snippets) provides a type-safe iterator over dependency specs. Using it would simplify the code and make it more maintainable.

♻️ Proposed refactor
-fn has_local_file_dep(project_manifests: &[(PathBuf, &PackageManifest)]) -> bool {
-    const FIELDS: [&str; 3] = ["dependencies", "devDependencies", "optionalDependencies"];
-    project_manifests.iter().any(|(_, manifest)| {
-        FIELDS.iter().any(|field| {
-            manifest.value().get(*field).and_then(|value| value.as_object()).is_some_and(|deps| {
-                deps.values()
-                    .any(|spec| spec.as_str().is_some_and(|spec| spec.starts_with("file:")))
-            })
-        })
-    })
-}
+fn has_local_file_dep(project_manifests: &[(PathBuf, &PackageManifest)]) -> bool {
+    use DependencyGroup::{Dev, Optional, Prod};
+    project_manifests.iter().any(|(_, manifest)| {
+        manifest
+            .dependencies([Prod, Dev, Optional])
+            .any(|(_, spec)| spec.starts_with("file:"))
+    })
+}
🤖 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/package-manager/src/optimistic_repeat_install.rs` around lines
278 - 295, The function has_local_file_dep currently inspects manifest JSON via
manifest.value().get(...) to find "file:" dependency specs; instead, use the
manifest's typed API by calling PackageManifest::dependencies (or the existing
dependencies() method on PackageManifest) to iterate dependency entries for
"dependencies", "devDependencies", and "optionalDependencies" and check each
spec string for starts_with("file:"); replace the raw JSON access in
has_local_file_dep with calls to PackageManifest::dependencies (or equivalent
methods) for each of the three fields (formerly FIELDS) so you get a type-safe
iterator over specs and then any(|spec| spec.starts_with("file:")).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pacquet/crates/package-manager/src/optimistic_repeat_install.rs`:
- Around line 278-295: The function has_local_file_dep currently inspects
manifest JSON via manifest.value().get(...) to find "file:" dependency specs;
instead, use the manifest's typed API by calling PackageManifest::dependencies
(or the existing dependencies() method on PackageManifest) to iterate dependency
entries for "dependencies", "devDependencies", and "optionalDependencies" and
check each spec string for starts_with("file:"); replace the raw JSON access in
has_local_file_dep with calls to PackageManifest::dependencies (or equivalent
methods) for each of the three fields (formerly FIELDS) so you get a type-safe
iterator over specs and then any(|spec| spec.starts_with("file:")).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3601fca4-9dd5-4a7d-8573-20337378eef2

📥 Commits

Reviewing files that changed from the base of the PR and between 52be454 and 5145284.

📒 Files selected for processing (6)
  • .changeset/repeat-install-file-deps.md
  • deps/status/src/checkDepsStatus.ts
  • deps/status/test/checkDepsStatus.test.ts
  • installing/commands/src/installDeps.ts
  • pacquet/crates/package-manager/src/optimistic_repeat_install.rs
  • pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Compile & Lint
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Dylint
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Doc
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Code Coverage
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use Standard Style with trailing commas, prefer functions over classes, declare functions after they are used (relying on hoisting), limit functions to no more than two or three arguments, and use a single options object for functions needing more parameters
Follow import order: standard libraries first, then external dependencies (sorted alphabetically), then relative imports
Do not write comments that restate what the code already says; rename variables, split helpers, or move checks to more obvious places instead
Do not repeat documentation at call sites that already lives on the callee; update the JSDoc once and let every call site benefit
Use JSDoc for the function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the function body
Do not record past implementation shape, refactor history, or removed code in comments; use git log and git blame for that information instead
Write comments only when the reason for code is non-obvious, a hidden invariant exists, a workaround for a known bug is needed, or an exception to surrounding pattern is deliberate

Files:

  • installing/commands/src/installDeps.ts
  • deps/status/test/checkDepsStatus.test.ts
  • deps/status/src/checkDepsStatus.ts
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/optimistic_repeat_install/tests.rs
  • pacquet/crates/package-manager/src/optimistic_repeat_install.rs
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not use instanceof Error for checking if a caught error is an Error object in Jest tests; use util.types.isNativeError() instead to work across realms

Files:

  • deps/status/test/checkDepsStatus.test.ts
🧠 Learnings (8)
📚 Learning: 2026-05-26T21:01:06.666Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.

Applied to files:

  • .changeset/repeat-install-file-deps.md
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

  • installing/commands/src/installDeps.ts
  • deps/status/test/checkDepsStatus.test.ts
  • deps/status/src/checkDepsStatus.ts
📚 Learning: 2026-06-05T13:47:26.046Z
Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/src/install/index.ts:2337-2343
Timestamp: 2026-06-05T13:47:26.046Z
Learning: In the pnpm/pnpm codebase, `PnpmError` automatically prefixes `err.code` with `ERR_PNPM_` when you pass a code that does not already start with `ERR_PNPM_` (it normalizes `this.code` via `code.startsWith('ERR_PNPM_') ? code : `ERR_PNPM_${code}``). Therefore, during code review you should NOT flag `new PnpmError(...)` call sites for passing a bare error code (e.g., `new PnpmError('FROZEN_STORE_INCOMPATIBLE_WITH_PNPR', ...)`); the resulting `err.code` will still be `ERR_PNPM_FROZEN_STORE_INCOMPATIBLE_WITH_PNPR`.

Applied to files:

  • installing/commands/src/installDeps.ts
  • deps/status/test/checkDepsStatus.test.ts
  • deps/status/src/checkDepsStatus.ts
📚 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/optimistic_repeat_install/tests.rs
  • pacquet/crates/package-manager/src/optimistic_repeat_install.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/optimistic_repeat_install/tests.rs
  • pacquet/crates/package-manager/src/optimistic_repeat_install.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/optimistic_repeat_install/tests.rs
  • pacquet/crates/package-manager/src/optimistic_repeat_install.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/optimistic_repeat_install/tests.rs
  • pacquet/crates/package-manager/src/optimistic_repeat_install.rs
📚 Learning: 2026-06-05T13:47:05.929Z
Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/test/install/frozenStore.ts:2-17
Timestamp: 2026-06-05T13:47:05.929Z
Learning: In the pnpm/pnpm repository, the shared Jest preset keeps `injectGlobals` at its default (`true`), so `test` and `expect` are available as Jest globals. Therefore, reviewers should not flag (or treat as TypeScript/compilation errors) missing `import { test, expect } from 'jest/globals'` when a test file uses `test`/`expect` without importing them. Importing from `jest/globals` may still be used for consistency with sibling files, but it is not required for execution in this repo unless a Jest preset is explicitly configured with `injectGlobals: false`.

Applied to files:

  • deps/status/test/checkDepsStatus.test.ts
🔇 Additional comments (12)
pacquet/crates/package-manager/src/optimistic_repeat_install.rs (2)

30-35: LGTM!


154-161: LGTM!

pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs (3)

136-156: LGTM!


158-176: LGTM!


178-196: LGTM!

.changeset/repeat-install-file-deps.md (1)

1-8: LGTM!

deps/status/src/checkDepsStatus.ts (4)

30-36: LGTM!


72-80: LGTM!


163-174: LGTM!


648-665: LGTM!

deps/status/test/checkDepsStatus.test.ts (1)

794-926: LGTM!

installing/commands/src/installDeps.ts (1)

185-185: LGTM!

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2e54668

Comment thread pacquet/crates/package-manager/src/optimistic_repeat_install.rs Outdated
@github-actions

github-actions Bot commented Jun 10, 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.248 ± 0.173 4.080 4.557 2.00 ± 0.13
pacquet@main 4.229 ± 0.205 4.052 4.567 1.99 ± 0.14
pnpr@HEAD 2.120 ± 0.104 1.966 2.308 1.00
pnpr@main 2.151 ± 0.167 1.957 2.362 1.01 ± 0.09
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 4.24839802978,
      "stddev": 0.17283668755338266,
      "median": 4.159619127079999,
      "user": 4.07218196,
      "system": 3.41633188,
      "min": 4.080380872579999,
      "max": 4.55660938258,
      "times": [
        4.55660938258,
        4.11262729758,
        4.36463093058,
        4.080380872579999,
        4.4326905625799995,
        4.39855000858,
        4.1946697435799996,
        4.124458758579999,
        4.09479423058,
        4.12456851058
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 4.22907311208,
      "stddev": 0.20480009132314458,
      "median": 4.109486112079999,
      "user": 4.02824436,
      "system": 3.4509968800000004,
      "min": 4.052138321579999,
      "max": 4.56729668958,
      "times": [
        4.56729668958,
        4.066483594579999,
        4.064191171579999,
        4.06710328558,
        4.052138321579999,
        4.51489778958,
        4.115554630579999,
        4.4009513365799995,
        4.33869670758,
        4.10341759358
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 2.11993962398,
      "stddev": 0.10440143307674862,
      "median": 2.1070098545800002,
      "user": 2.7171987599999996,
      "system": 2.9079628800000004,
      "min": 1.9661425065800002,
      "max": 2.30779488758,
      "times": [
        2.08901407758,
        2.02692231458,
        2.19340090558,
        2.1923879625800002,
        2.19989287158,
        2.10216946558,
        2.00982100458,
        2.11185024358,
        1.9661425065800002,
        2.30779488758
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 2.15084691578,
      "stddev": 0.1665393742604394,
      "median": 2.07072743358,
      "user": 2.7202663599999997,
      "system": 2.9254101799999996,
      "min": 1.9571139555800001,
      "max": 2.3618581985800002,
      "times": [
        1.9571139555800001,
        2.29284035058,
        2.0678828135800003,
        1.9987481535800002,
        2.3618581985800002,
        2.04161839758,
        2.07357205358,
        2.35568495958,
        2.34671178358,
        2.01243849158
      ]
    }
  ]
}

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

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 629.9 ± 13.5 616.3 652.1 1.00
pacquet@main 672.2 ± 108.4 613.4 975.9 1.07 ± 0.17
pnpr@HEAD 684.9 ± 24.5 657.7 745.9 1.09 ± 0.05
pnpr@main 682.5 ± 19.1 667.2 731.1 1.08 ± 0.04
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.62985401988,
      "stddev": 0.013477050904801752,
      "median": 0.62291489108,
      "user": 0.36068461999999996,
      "system": 1.3371802,
      "min": 0.61630897608,
      "max": 0.65210547508,
      "times": [
        0.64277279808,
        0.62378399808,
        0.61707430808,
        0.64198446608,
        0.65210547508,
        0.62204578408,
        0.64294811508,
        0.61630897608,
        0.61808535808,
        0.62143092008
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.67219933748,
      "stddev": 0.108428013517013,
      "median": 0.63680261758,
      "user": 0.36422292,
      "system": 1.3387181,
      "min": 0.61343862108,
      "max": 0.97589058508,
      "times": [
        0.67613601708,
        0.63945289108,
        0.65855621508,
        0.63262277708,
        0.61343862108,
        0.63415234408,
        0.61789664308,
        0.62272611708,
        0.97589058508,
        0.65112116408
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.68485909708,
      "stddev": 0.024542969282961208,
      "median": 0.68001474858,
      "user": 0.37237802000000003,
      "system": 1.3662254,
      "min": 0.65769678208,
      "max": 0.74591508008,
      "times": [
        0.69607657108,
        0.67858593208,
        0.65769678208,
        0.69236555908,
        0.67808415208,
        0.66509614708,
        0.66795762208,
        0.74591508008,
        0.68144356508,
        0.68536956008
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.6825293093800001,
      "stddev": 0.019099616304447342,
      "median": 0.67683604608,
      "user": 0.37627872,
      "system": 1.3576024,
      "min": 0.66715236408,
      "max": 0.73107059208,
      "times": [
        0.73107059208,
        0.66969084908,
        0.68252740408,
        0.66976998808,
        0.67029357508,
        0.68852479708,
        0.67448091408,
        0.67919117808,
        0.69259143208,
        0.66715236408
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 4.318 ± 0.035 4.241 4.357 1.94 ± 0.09
pacquet@main 4.256 ± 0.056 4.177 4.339 1.91 ± 0.09
pnpr@HEAD 2.246 ± 0.130 2.100 2.436 1.01 ± 0.08
pnpr@main 2.228 ± 0.106 2.107 2.403 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 4.31849694224,
      "stddev": 0.03485979183470616,
      "median": 4.32970531734,
      "user": 3.8757404799999997,
      "system": 3.3350613200000003,
      "min": 4.2408191223400005,
      "max": 4.35659604234,
      "times": [
        4.35659604234,
        4.30944295234,
        4.33489806534,
        4.32451256934,
        4.3392289143400005,
        4.31727386934,
        4.27909697034,
        4.33857524934,
        4.34452566734,
        4.2408191223400005
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 4.25591208134,
      "stddev": 0.056221424047973895,
      "median": 4.25059893034,
      "user": 3.8288838800000002,
      "system": 3.3262532199999995,
      "min": 4.17663428034,
      "max": 4.33896845934,
      "times": [
        4.29618748634,
        4.338013413340001,
        4.33896845934,
        4.24796703134,
        4.26560478934,
        4.21698971334,
        4.18538936434,
        4.24013544634,
        4.25323082934,
        4.17663428034
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 2.24593640784,
      "stddev": 0.12973064338309204,
      "median": 2.2102198998400002,
      "user": 2.54158008,
      "system": 2.82065562,
      "min": 2.09985389134,
      "max": 2.43593852034,
      "times": [
        2.43593852034,
        2.19302193734,
        2.11321272134,
        2.34103242734,
        2.36942358434,
        2.1685424863400002,
        2.4017604173400002,
        2.2274178623400003,
        2.09985389134,
        2.10916023034
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 2.22844000794,
      "stddev": 0.10566730065536296,
      "median": 2.22059643334,
      "user": 2.55908098,
      "system": 2.84046402,
      "min": 2.10745378734,
      "max": 2.40296418634,
      "times": [
        2.10745378734,
        2.11600897134,
        2.40296418634,
        2.22003013534,
        2.32256483034,
        2.10764795534,
        2.22116273134,
        2.1933704343400002,
        2.23080268434,
        2.36239436334
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 1.393 ± 0.018 1.355 1.418 2.09 ± 0.06
pacquet@main 1.423 ± 0.017 1.395 1.451 2.14 ± 0.06
pnpr@HEAD 0.665 ± 0.017 0.647 0.699 1.00
pnpr@main 0.675 ± 0.039 0.648 0.780 1.01 ± 0.06
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.39254553118,
      "stddev": 0.01806178951099464,
      "median": 1.39567033848,
      "user": 1.35857526,
      "system": 1.7476858399999997,
      "min": 1.35521763098,
      "max": 1.41771149198,
      "times": [
        1.41031894798,
        1.37954213498,
        1.38336850598,
        1.35521763098,
        1.39546836098,
        1.39587231598,
        1.40657816998,
        1.38304897198,
        1.41771149198,
        1.39832878098
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.42287144298,
      "stddev": 0.016991718442795383,
      "median": 1.41972463348,
      "user": 1.37859386,
      "system": 1.76046784,
      "min": 1.39502081298,
      "max": 1.45103911598,
      "times": [
        1.40990191798,
        1.42074313598,
        1.41057834398,
        1.39502081298,
        1.41870613098,
        1.41413658198,
        1.43103505298,
        1.44320882198,
        1.43434451498,
        1.45103911598
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.66548362068,
      "stddev": 0.016522534386570226,
      "median": 0.66444442598,
      "user": 0.3430514599999999,
      "system": 1.29786394,
      "min": 0.64741428598,
      "max": 0.69870516698,
      "times": [
        0.67033440498,
        0.64902329998,
        0.66990146298,
        0.69870516698,
        0.68301351298,
        0.67066950898,
        0.65898738898,
        0.64870017198,
        0.65808700298,
        0.64741428598
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.67470323978,
      "stddev": 0.038724914899723545,
      "median": 0.66274543898,
      "user": 0.34507086,
      "system": 1.30328014,
      "min": 0.64794339298,
      "max": 0.77952814798,
      "times": [
        0.64794339298,
        0.65106016498,
        0.68127084598,
        0.66752058498,
        0.6575497889799999,
        0.67667242698,
        0.77952814798,
        0.65797029298,
        0.65109323898,
        0.67642351298
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 3.163 ± 0.048 3.095 3.242 4.94 ± 0.10
pacquet@main 3.092 ± 0.033 3.044 3.151 4.83 ± 0.08
pnpr@HEAD 0.653 ± 0.012 0.624 0.665 1.02 ± 0.02
pnpr@main 0.640 ± 0.008 0.618 0.649 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 3.16326288004,
      "stddev": 0.04825819735675574,
      "median": 3.15244006164,
      "user": 1.8857446999999996,
      "system": 2.0366999600000004,
      "min": 3.09528827414,
      "max": 3.24244244014,
      "times": [
        3.21188451914,
        3.1399012381399998,
        3.18236404114,
        3.15146848914,
        3.21347943514,
        3.24244244014,
        3.10541984114,
        3.09528827414,
        3.1369688881399997,
        3.15341163414
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 3.09161499334,
      "stddev": 0.03347412813495982,
      "median": 3.09686769264,
      "user": 1.7911249000000002,
      "system": 2.0141028600000004,
      "min": 3.04426258014,
      "max": 3.15104483014,
      "times": [
        3.08458871814,
        3.10084083914,
        3.11503333314,
        3.0595701641399997,
        3.10948266214,
        3.09289454614,
        3.11061971114,
        3.04781254914,
        3.15104483014,
        3.04426258014
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.65346179344,
      "stddev": 0.011533503286689215,
      "median": 0.6533737526400001,
      "user": 0.31809659999999995,
      "system": 1.2919521599999997,
      "min": 0.6243287381400001,
      "max": 0.6653434571400001,
      "times": [
        0.65182893214,
        0.6521116371400001,
        0.66127171914,
        0.6243287381400001,
        0.6504886321400001,
        0.6653434571400001,
        0.6516889171400001,
        0.6609610361400001,
        0.6619589971400001,
        0.6546358681400001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.63974598944,
      "stddev": 0.008476394837847187,
      "median": 0.6423619016400002,
      "user": 0.32275970000000004,
      "system": 1.27750166,
      "min": 0.6183932511400001,
      "max": 0.6487278891400001,
      "times": [
        0.6415233561400001,
        0.6183932511400001,
        0.6447301471400001,
        0.6392750731400001,
        0.64551365514,
        0.63719580614,
        0.6487278891400001,
        0.6432004471400001,
        0.64331853914,
        0.6355817301400001
      ]
    }
  ]
}

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12317
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
4,318.50 ms
(+3.49%)Baseline: 4,172.95 ms
5,007.54 ms
(86.24%)
isolated-linker.fresh-install.cold-cache.hot-store📈 view plot
🚷 view threshold
3,163.26 ms
(+5.69%)Baseline: 2,992.85 ms
3,591.42 ms
(88.08%)
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
🚷 view threshold
1,392.55 ms
(+5.85%)Baseline: 1,315.54 ms
1,578.65 ms
(88.21%)
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
🚷 view threshold
4,248.40 ms
(+6.97%)Baseline: 3,971.44 ms
4,765.72 ms
(89.14%)
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
🚷 view threshold
629.85 ms
(+1.37%)Baseline: 621.36 ms
745.63 ms
(84.47%)
🐰 View full continuous benchmarking report in Bencher

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12317
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
2,245.94 ms
isolated-linker.fresh-install.cold-cache.hot-store📈 view plot
⚠️ NO THRESHOLD
653.46 ms
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
⚠️ NO THRESHOLD
665.48 ms
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
⚠️ NO THRESHOLD
2,119.94 ms
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
⚠️ NO THRESHOLD
684.86 ms
🐰 View full continuous benchmarking report in Bencher

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2b3196c

@codecov-commenter

codecov-commenter commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.06542% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.07%. Comparing base (c112b61) to head (a531a26).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...s/package-manager/src/optimistic_repeat_install.rs 99.06% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12317      +/-   ##
==========================================
+ Coverage   88.03%   88.07%   +0.03%     
==========================================
  Files         309      309              
  Lines       41595    41702     +107     
==========================================
+ Hits        36618    36727     +109     
+ Misses       4977     4975       -2     

☔ 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.

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit fb9c546

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 7a3b64c

Comment thread deps/status/src/checkDepsStatus.ts
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 8251a00

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 471f5b0

Comment thread deps/status/src/checkDepsStatus.ts Outdated
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 81652c9

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit fda788d

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d983e4b

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f705dcf

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 8fcfff7

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit e3b880f

@truffle-dev truffle-dev force-pushed the fix/repeat-install-file-deps branch from e3b880f to 8c800b6 Compare June 11, 2026 20:27
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 8c800b6

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 89b5d13

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

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit c3dcdca

truffle-dev and others added 13 commits June 16, 2026 19:15
…uet + pnpm)

An override that maps a dependency to a file: specifier (or a bare local
path/tarball) redirects every matching dependency in the graph to that
directory, so its contents can change without any tracked mtime moving,
the same blind spot as a direct local file dependency
(pnpm#11795). The fast path now scans the
resolved override values with the same local-file predicate on both the
TypeScript and pacquet sides.
… pnpm)

The local resolver's isFilespec (resolving/local-resolver/src/parseBareSpecifier.ts)
treats any [a-z]: prefix as a local path, with no separator required after the
colon, so a drive-relative spec like C:dir resolves locally. The repeat-install
predicate required a separator, leaving such specs on the fast path. Relaxing is
unambiguous: no registry or pnpm protocol is a single letter.
The catalog-registry-range test reaches catalogs_cache_matches now that
the workspace state records catalogs (pnpm#12382). Stamp the state
with the test's catalogs so the cache comparison passes and the spec
deref is what gets exercised, not the catalogs-outdated bail.
…Projects omits it

When allProjects was provided, rootProjectManifest was ignored, so a
root-only local file dependency (for example under
includeWorkspaceRoot: false) could slip past the
treatLocalFileDepsAsOutdated bail-out.
…nstall

Installs a file: directory dependency, edits a file inside it, reruns
install with optimisticRepeatInstall enabled, and asserts node_modules
refreshes (pnpm#11795).
Avoid per-dependency allocations: catalog_resolves_to_local_file
short-circuits before building an owned WantedDependency for
non-catalog: specs, and is_local_file_spec checks tarball suffixes
case-insensitively on bytes instead of allocating a lowercased copy.

Use a distinct skip reason when pnpm.overrides cannot be parsed instead
of misattributing the failure to a local file override.
@zkochan zkochan force-pushed the fix/repeat-install-file-deps branch from 2b1e94b to 4b4d436 Compare June 16, 2026 17:21
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

Qodo Logo

… dep check

findLocalFileDep only needs resolveFromCatalog for catalog: specs;
guarding the call keeps the optimistic fast path allocation-free for
the common case, matching the pacquet port.
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit c0e4d9c

Comment thread deps/status/src/checkDepsStatus.ts
… fast path (pacquet + pnpm)

isLocalFileSpec / is_local_file_spec treated any colon-free spec ending
in .tgz/.tar.gz/.tar as a local tarball, so a hosted-git shorthand like
user/repo#release.tgz was misclassified and always forced a full
install. Exclude specs carrying a # committish from the bare-tarball
match.
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4ea0486

Comment thread deps/status/src/checkDepsStatus.ts
Comment thread pacquet/crates/package-manager/src/optimistic_repeat_install.rs
…install (pacquet + pnpm)

packageExtensions are merged into matching packages' manifests by the
read-package hook during install, so a file:/local-path/tarball spec
added there has the same content-change blind spot as a direct local
file dependency without appearing in any project manifest. Scan
dependencies and optionalDependencies (peers aren't fetched), respecting
the install's include flags.
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit a531a26

@zkochan zkochan enabled auto-merge (squash) June 16, 2026 18:54
@zkochan zkochan disabled auto-merge June 16, 2026 20:31
@zkochan zkochan merged commit 6d35338 into pnpm:main Jun 16, 2026
31 of 33 checks passed
truffle-dev added a commit to truffle-dev/truffleagent-site that referenced this pull request Jun 16, 2026
Already live on prod via the prior deploy; committing so main matches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pnpm install no longer detects changes in file: directory dependencies (regression from v10)

3 participants