Skip to content

fix(install): ignore stale pnpm patch entries - #1022

Merged
jdx merged 1 commit into
mainfrom
codex/fix-stale-patch-lockfile
Jul 13, 2026
Merged

jdx merged 1 commit into
mainfrom
codex/fix-stale-patch-lockfile

Conversation

@jdx

@jdx jdx commented Jul 12, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • replace stale pnpm lockfile patch metadata with current manifest/workspace declarations after a fresh resolution
  • avoid reading deleted patch paths or pnpm 11 hash-only values as files
  • add end-to-end coverage for pnpm 10 object-form and pnpm 11 hash-only stale entries

Root cause

Fresh resolution produced an empty patch map, then overlay_metadata_from restored every patch entry from the prior lockfile. Materialization merged those stale entries with current declarations and eagerly read both keys, even when the stale package version was no longer in the resolved graph.

Fixes the bug reported in Discussion #1019.

Validation

  • mise run test:bats test/patch.bats
  • cargo fmt --check
  • cargo clippy -p aube --all-targets -- -D warnings

This PR was generated by Codex.


Note

Medium Risk
Touches install graph state for pnpm patch metadata on the re-resolve path; behavior change is intentional but could affect edge cases where users relied on lockfile-only patch entries without manifest declarations.

Overview
After a non-frozen install re-resolves because the lockfile no longer matches the manifest, overlay_metadata_from could copy patchedDependencies from the old pnpm-lock.yaml, so removed or wrong-version patches were still applied and written back.

For projects that already use a pnpm lockfile, the fresh-resolve path now replaces graph.patched_dependencies with read_patched_dependencies from the current package.json / workspace yaml. Materialization and the rewritten lockfile follow today’s patch declarations only, and pnpm 11 hash-only scalars are not treated as patch file paths.

Bats coverage was added for stale lockfile entries (object path form and hash-only form) while the workspace still declares a patch for the resolved version.

Reviewed by Cursor Bugbot for commit 775089f. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@jdx, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: b5637f56-3bd6-4592-9593-4c0a0e9c1f49

📥 Commits

Reviewing files that changed from the base of the PR and between 06601ee and 775089f.

📒 Files selected for processing (2)
  • crates/aube/src/commands/install/mod.rs
  • test/patch.bats

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.

@jdx
jdx marked this pull request as ready for review July 12, 2026 00:24
@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where stale patchedDependencies entries from a pnpm lockfile were restored via overlay_metadata_from during a fresh re-resolve, causing deleted or wrong-version patches to be re-applied and written back. The fix replaces the overlaid patch map with a fresh read of current manifest/workspace declarations immediately after the overlay call, scoped to the pnpm lockfile path.

  • install/mod.rs: A single conditional after both overlay_metadata_from branches calls crate::patches::read_patched_dependencies(&cwd) to reset graph.patched_dependencies to the authoritative manifest state whenever a pnpm lockfile was the source. This also prevents pnpm 11 hash-only scalar values from being misread as file paths.
  • test/patch.bats: Two new end-to-end bats tests cover the pnpm 10 object-form (hash + path) and pnpm 11 hash-only stale lockfile entries, verifying that install succeeds, the patch is applied from the current manifest declaration, and the rewritten lockfile no longer references the stale version.

Confidence Score: 4/5

Safe to merge; the change is narrowly scoped to the pnpm re-resolve path and is backed by two new end-to-end tests exercising both stale-entry shapes.

The logic is straightforward: one conditional that replaces at most one field on the graph object, only on the non-frozen pnpm re-resolve path. The read_patched_dependencies function is already well-exercised by existing unit tests, and the new bats tests cover the exact failure mode being fixed. The one edge worth noting is that the override fires even when the overlay_metadata_from branches were both skipped (e.g. lockfile parse failure), but in that case the graph's patch map would be empty and the manifest read is the correct thing to do anyway.

No files require special attention; both changed files are focused and self-contained.

Important Files Changed

Filename Overview
crates/aube/src/commands/install/mod.rs Adds a pnpm-scoped override after overlay_metadata_from to replace stale lockfile patchedDependencies with current manifest/workspace declarations; fix is well-placed and correctly scoped to the re-resolve path only
test/patch.bats Adds two bats end-to-end tests covering pnpm 10 object-form and pnpm 11 hash-only stale lockfile entries; tests validate install success, patch application, and lockfile rewrite

Reviews (1): Last reviewed commit: "fix(install): ignore stale pnpm patch en..." | Re-trigger Greptile

@jdx
jdx merged commit ed31f8c into main Jul 13, 2026
18 checks passed
@jdx
jdx deleted the codex/fix-stale-patch-lockfile branch July 13, 2026 18:40
@cursor cursor Bot mentioned this pull request Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant