[jdx]

Summary

• preserve pnpm patch hashes when parsing v10 object-form and v11 scalar-form lockfiles
• derive pnpm-compatible SHA-256 hashes from current patch files, including CRLF normalization
• emit (patch_hash=...) identities on direct, transitive, snapshot, and peer-context references
• add unit and BATS coverage for the Discussion Non-frozen re-resolve rewrites the pnpm lockfile without pnpm-compatible patch metadata for a still-declared patch #1029 re-resolution scenario and native pnpm frozen-lockfile compatibility

Root cause

The shared lockfile graph collapsed pnpm patch metadata to a path, and the pnpm writer explicitly emitted that path without decorating importer versions or snapshot keys. PR #1022 made current manifest/workspace declarations authoritative after a fresh resolve, which kept patches applied but exposed this metadata loss whenever unrelated manifest drift rewrote pnpm-lock.yaml.

The pnpm boundary now preserves an existing hash or computes the same normalized patch-content SHA-256 as pnpm 11, then applies that identity consistently while writing pnpm-lock.yaml.

Resolves the behavior reported in Discussion #1029.

Validation

• cargo test
• cargo clippy --all-targets -- -D warnings
• cargo fmt --check
• mise run test:bats test/patch.bats
• supplied tmp-aube-issues/pnpm-patch-reresolve-drop/repro.sh with pnpm 11.7.0, including the native frozen reinstall

This PR was generated by Codex.

---

Note

Medium Risk
Touches lockfile parse/write and install-visible pnpm-lock.yaml identity strings; mistakes could break frozen installs or patch parity with pnpm 11, but scope is pnpm-native writes with new regression tests.

Overview
Fixes pnpm patched-dependency metadata being dropped when pnpm-lock.yaml is rewritten after a non-frozen install.

Read path: patchedDependencies no longer collapses to patch file paths only—v11 scalar hashes and v9/v10 { path, hash } objects are stored on the shared graph (hash preferred when present).

Write path: For native pnpm-lock.yaml, the writer either keeps existing 64-char hashes or SHA-256-hashes patch files (CRLF→LF, relative-path validation). Top-level patchedDependencies serializes as selector→hash (pnpm 11 style). Importer version: lines, snapshot keys, and dependency edges get (patch_hash=<sha256>) inserted before peer suffixes (e.g. 3.0.1(patch_hash=…)(react@19.0.0)), including transitive and alias targets.

Tests cover round-trip with object-form hashes, full decoration across direct/transitive/peer refs, lockfile ordering with real patch files, and a BATS non-frozen re-resolve scenario aligned with Discussion #1029.

^{Reviewed by Cursor Bugbot for commit ca6abc9. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes
  • Improved pnpm patched-dependency handling across lockfile reads, writes, and re-resolution.
  • Preserved patch identities when installing with a non-frozen lockfile, preventing unnecessary patch changes.
  • Ensured patched dependency references remain consistent for direct, transitive, and peer dependencies.
• Tests
  • Added coverage confirming patched packages continue to work after re-resolution and frozen installs.

[coderabbitai]

📝 Walkthrough

Walkthrough

pnpm lockfile parsing now preserves patch hashes, while native pnpm writing computes normalized patch-content hashes and propagates them into patched dependency entries and dependency references. Tests cover round trips, peer-context references, and frozen installation behavior.

Changes

pnpm patch hash handling

Layer / File(s)	Summary
Patch value parsing contract crates/aube-lockfile/src/lib.rs, crates/aube-lockfile/src/pnpm/raw.rs, crates/aube-lockfile/src/pnpm/read.rs	Parsing preserves pnpm patch hashes when present and falls back to patch paths otherwise; related field documentation reflects the representation.
Native pnpm patch hash writing crates/aube-lockfile/src/pnpm/write.rs	Patch files are normalized and hashed, with hashes emitted in patchedDependencies and added to dependency and snapshot references.
Patch hash regression coverage crates/aube-lockfile/src/pnpm/tests.rs, test/patch.bats	Tests cover hash round trips, direct/transitive/peer-context references, and non-frozen followed by frozen installation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LockfileGraph
  participant PatchFile
  participant PnpmWriter
  participant PnpmLockfile
  LockfileGraph->>PnpmWriter: provide patched dependency paths
  PnpmWriter->>PatchFile: read and normalize patch content
  PatchFile-->>PnpmWriter: return SHA-256 patch hash
  PnpmWriter->>PnpmLockfile: emit hash and qualified references

Loading

Suggested reviewers: rubnogueira

Poem

A rabbit hops through hashes bright,
Patch paths turn to strings of light.
Direct and peer refs join the dance,
Frozen installs keep their stance.
“Hop!” says Bun, “the lock is right!”

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly matches the main change: preserving pnpm patch hashes during lockfile re-resolution.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR fixes a pnpm lockfile round-trip regression where patched package identities ((patch_hash=...) suffixes) were silently dropped during re-resolution. The graph layer now stores the patch-content SHA-256 hash (not just the patch file path) when parsing pnpm v9/v10 Object-form or v11 scalar-form lockfiles, and the writer resolves any remaining path-form values to hashes by reading and CRLF-normalizing patch files before emitting pnpm-lock.yaml.

• raw.rs / read.rs: into_path() → into_value() now returns the stored hash field from Object-form entries instead of always returning the path.
• write.rs: adds pnpm_patch_hashes() (validates & hashes patch files), with_patch_hash() / strip_patch_hash_suffix() helpers, and applies decorate_patch_hash to importer versions, snapshot keys, and transitive dep values when writing pnpm-lock.yaml.
• Tests: unit test pnpm_patch_hash_decorates_direct_transitive_and_peer_context_refs + BATS test covering the full re-resolve + frozen-install scenario.

Confidence Score: 5/5

Safe to merge. The change is scoped to pnpm lockfile read/write semantics and the new filesystem access is guarded by path traversal checks and skipped entirely when a stored hash is already present.

The path validation correctly blocks absolute paths, Windows drive letters, and parent-directory traversal. The hash detection uses a 64-char all-hex check matching both pnpm v9/v10 Object-form and v11 scalar-form bare-hex hashes (confirmed via web search). strip-then-reinsert in with_patch_hash handles re-resolution correctly, and the while-loop in strip_patch_hash_suffix removes multiple stale occurrences. Tests cover all three decoration sites and hash preservation without the patch file.

No files require special attention.

Important Files Changed

Filename	Overview
crates/aube-lockfile/src/pnpm/write.rs	Core of the fix: adds pnpm_patch_hashes() (path validation + SHA-256 computation), strip_patch_hash_suffix() loop, and with_patch_hash() insertion, then wires decorate_patch_hash into importer, snapshot, and transitive-dep write paths. Logic is correct for all covered cases; path traversal protection is adequate.
crates/aube-lockfile/src/pnpm/raw.rs	into_path() renamed to into_value(); Object-form now returns hash.unwrap_or(path). dead_code allow removed. Change is minimal and correct.
crates/aube-lockfile/src/pnpm/read.rs	One-line call-site update from into_path() to into_value(). Correct.
crates/aube-lockfile/src/pnpm/tests.rs	Updates existing tests to use a valid 64-char hex hash; adds pnpm_patch_hash_decorates_direct_transitive_and_peer_context_refs covering hash computation, CRLF normalization, all three decoration sites, and hash preservation without the patch file. One nit: the lodash patch file created in the setup is for the wrong package relative to the actual patched dep under test.
crates/aube-lockfile/src/lib.rs	Doc comment update only; accurately describes dual-purpose value field (hash or path).
test/patch.bats	Adds BATS integration test for the Discussion #1029 re-resolve scenario: starts from a v11 scalar-hash lockfile, runs non-frozen install, verifies patch is active, and checks that the output lockfile decorates all three reference sites. Hash is validated by length (64 chars) rather than a hard-coded value, which is the right approach.

_{Reviews (2): Last reviewed commit: "fix(lockfile): harden pnpm patch hash ha..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

crates/aube-lockfile/src/pnpm/write.rs (1)

184-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate "resolve target + apply patch hash" logic across three sites.

The target-package lookup (direct dep_path, falling back to peerless_dep_path) followed by with_patch_hash(&value, patch_hash_for(pkg)) is repeated in the importer loop, rewrite_local_deps, and the snapshot-key construction. Consider factoring the lookup+decorate step into one shared helper so future changes to the patch-hash rule only need updating once.

♻️ Sketch

+fn decorate_with_patch_hash<'a>(
+    value: &str,
+    target: Option<&'a crate::LockedPackage>,
+    patch_hash_for: impl Fn(&'a crate::LockedPackage) -> Option<&'a str>,
+) -> String {
+    target
+        .map(|pkg| with_patch_hash(value, patch_hash_for(pkg)))
+        .unwrap_or_else(|| value.to_string())
+}

Also applies to: 640-663, 674-690

🤖 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 `@crates/aube-lockfile/src/pnpm/write.rs` around lines 184 - 212, Factor the
repeated target-package lookup and patch-hash decoration into a shared helper in
the lockfile writer, covering the importer loop, rewrite_local_deps, and
snapshot-key construction. The helper should check the direct dep_path first,
fall back to peerless_dep_path, and apply with_patch_hash using patch_hash_for;
replace each duplicated implementation while preserving existing values and
native_pnpm_aliases conditions.

🤖 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 `@crates/aube-lockfile/src/pnpm/write.rs`:
- Around line 184-212: Factor the repeated target-package lookup and patch-hash
decoration into a shared helper in the lockfile writer, covering the importer
loop, rewrite_local_deps, and snapshot-key construction. The helper should check
the direct dep_path first, fall back to peerless_dep_path, and apply
with_patch_hash using patch_hash_for; replace each duplicated implementation
while preserving existing values and native_pnpm_aliases conditions.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a9a4c65-7f96-469b-984c-cccba82f0486

📥 Commits

Reviewing files that changed from the base of the PR and between aba456b and 5e86318.

📒 Files selected for processing (6)

• crates/aube-lockfile/src/lib.rs
• crates/aube-lockfile/src/pnpm/raw.rs
• crates/aube-lockfile/src/pnpm/read.rs
• crates/aube-lockfile/src/pnpm/tests.rs
• crates/aube-lockfile/src/pnpm/write.rs
• test/patch.bats

[jdx]

Addressed the remaining review and CI feedback in ca6abc9:

• removed every stale patch hash suffix defensively and added duplicate-suffix coverage
• centralized patch decoration across importer, transitive dependency, and snapshot paths
• removed the obsolete dead_code suppression
• added a hash-to-hash rewrite test that succeeds after the patch file is removed
• removed the undeclared pnpm executable dependency from the BATS case, which was the root cause on both Ubuntu and macOS shards

Local validation passes: cargo test, cargo clippy --all-targets -- -D warnings, and mise run test:bats test/patch.bats.

This comment was generated by Codex.