Fix vendored berry revert restoring stale lock (#759)

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #759

Summary

After a Yarn Berry package was vendored, vendor --revert, rollback and remove wrote the yarn.lock entry back exactly as it was recorded at vendor time. If the lock had changed since then, the revert still exited 0, but every later yarn install --immutable failed with YN0028. The restored entry is now rebuilt from the live lock.

Root cause

revert_yarn_berry_opts → revert_recorded_block splices in the recorded pre-vendor block byte for byte. The issue reports three ways the lock can change in between:

1. A yarn 4.0.x → 4.1+ upgrade. It respells every checksum: as 10c0/<hex>, but the restored entry kept the old bare hex.
2. A workspace member adds a descriptor (left-pad: 1.3.0 next to the root's ^1.3.0). The resolutions pin routes it to the vendored entry, so the restored key was missing it.
3. A member removes its descriptor. The restored key still carried it. This is the vendored twin of Hosted yarn berry rollback/remove put back a descriptor that yarn remove deleted from one workspace member, so every yarn install --immutable fails YN0028 #1435.

Fix

• Checksum: the restored entry, and any pruned entry put back, is written in the lock's current spelling, read before the restore. Nothing is respelled when the lock has no checksum to read the spelling from.
• Key: it is rebuilt from the package's registry descriptors that the lock still declares. Descriptors added since vendoring join the key only if the restored version satisfies them (the range check follows npm's rules). A new descriptor the version doesn't satisfy, or whose range can't be read (such as a dist-tag), gets a vendor_lock_descriptor_unlocked warning telling the user to run yarn install. It is documented in CLI_CONTRACT.md.
• When the key is left alone:
  • when the lock declares none of the descriptors, as hosted already does;
  • when a user resolutions selector routes the package (a pin vendoring took over, or any other selector): yarn keys that entry by the pin's range, which no dependency map declares.
• Second pass: a revert run again after --preserve-state (or after a failed package.json write) recognizes the rebuilt entry and converges silently.
• Mixed line endings: a lock with mixed endings skips the rebuild and keeps the byte-splice result, so its CRLF lines aren't flattened.
• Shared helpers: declared_descriptors and descriptor_declared move from the hosted restore (patch/redirect/upstream/npm.rs, Hosted yarn berry rollback/remove put back a descriptor that yarn remove deleted from one workspace member, so every yarn install --immutable fails YN0028 #1435) to formats::yarn::berry_prune, so hosted and vendored use the same logic. Hosted behavior is unchanged.
• Wrappers: npm/, pypi/ and gem/ only dispatch to the binary, so nothing changes there.

Per-issue checklist

#759 covers three symptoms, and each has its own regression test.

• Symptom 1, checksum respelled by a yarn upgrade
  • unit: issue_759_revert_follows_a_checksum_respelling_since_vendoring (both directions)
  • real yarn: issue_759_revert_after_a_yarn_upgrade_respells_checksums (vendor on 4.0.2, install on 4.18, revert, fresh --immutable)
• Symptom 2, descriptor added while vendored
  • unit: issue_759_revert_keys_a_descriptor_added_since_vendoring
  • real yarn: issue_759_revert_after_a_member_adds_a_descriptor
• Symptom 3, descriptor removed while vendored
  • unit: issue_759_revert_drops_a_descriptor_removed_since_vendoring
  • real yarn: issue_759_revert_after_a_member_removes_a_descriptor
• Edge cases
  • issue_759_revert_warns_about_a_descriptor_it_cannot_lock
  • issue_759_taken_over_user_pin_keeps_its_key
  • issue_759_second_revert_pass_converges_on_a_rebased_entry
  • issue_759_mixed_line_endings_are_not_flattened
  • npm_range_admits_reads_npm_semantics
  • lock_checksum_spelling_tells_none_from_prefixed

Test evidence

• Red → green, unit: the four unit regressions for the symptoms and the warning failed on main's code with the exact wrong locks (bare checksum, stale key in each direction, no warning), and pass with the fix.
• Red → green, real yarn 4.18.0: with main's core, all three e2e tests fail with the issue's YN0028 diffs:

  ➤ YN0028: │ -"left-pad@npm:^1.3.0":
  ➤ YN0028: │ +"left-pad@npm:1.3.0, left-pad@npm:^1.3.0":
  ➤ YN0028: │ -  checksum: 3fb59c76…
  ➤ YN0028: │ +  checksum: 10c0/3fb59c76…
  ➤ YN0028: │ -"left-pad@npm:1.3.0, left-pad@npm:^1.3.0":
  ➤ YN0028: │ +"left-pad@npm:^1.3.0":
  

  With the fix, all three pass.
• Local gate:
  • cargo fmt --all -- --check: clean.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test --workspace --all-features --no-fail-fast: everything passes except 13 tests that fail only because of this sandbox, none of them in yarn code. The sandbox runs as root, so the chmod-based write-failure tests in covgap_commands_vendor, in_process_redirect, repair and the core lib can't make a write fail. e2e_bun_lockb needs patches-api.socket.dev, which the sandbox can't reach.
• scripts/yarn-berry-vex-matrix.sh 4.18.0 4.0.2: all four berry suites pass on both releases. The yarn 2 legacy-refusal step can't fetch yarn 2 in this sandbox (it isn't on the npm registry), so it is left to CI.
• /code-review high: ran on the first push. All seven findings were confirmed and fixed in 2cccdba: user-pin keys, second-pass convergence, range coverage, mixed endings, the dead quote filter, the shared spelling helper, and no-checksum locks.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y2UkYZUR7BXEk3odB4mdSS

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 2cccdba. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 2cccdba8843414953b11a6d5685d5993b316ece9
• CI: 36/36 green (50 skipped by path filters); no main-wide failures
• Mergeable against main, no conflicts; does not touch CHANGELOG.md
• Bugbot: reviewed 2cccdba, no findings; no unresolved review threads
• Reviewer focus: vendor/yarn_berry_lock.rs rebuilding the restored lock key from live descriptors (the semver satisfies check and the vendor_lock_descriptor_unlocked warning path), plus the helpers moved from patch/redirect/upstream/npm.rs into formats::yarn::berry_prune.

Slack announcement not sent: no Slack send tool was available to this run.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: After a Yarn Berry package was vendored, vendor --revert / rollback / remove wrote the recorded pre-vendor yarn.lock entry back byte for byte. If the lock had changed meanwhile, the revert exited 0 but the next yarn install --immutable failed (YN0028). The restored entry now follows the live lock: its checksum takes the lock's current spelling (bare vs 10c0/), and its key is rebuilt from the registry descriptors the lock still declares. A newly added descriptor the restored version doesn't satisfy gets a new vendor_lock_descriptor_unlocked warning. Fixes #759.

Risk: low to medium. It touches the revert path only, and every uncertain case keeps the old behavior: a user resolutions selector, a lock that declares none of the descriptors, mixed line endings, or an unreadable range. The real risk is the hand-rolled npm-range-to-semver translation, which decides whether a descriptor joins the key.

Look here:

• yarn_berry_lock.rs:1639:`` rekeyed, which builds the new key (drops stale descriptors, adds satisfied new ones, warns otherwise)
• yarn_berry_lock.rs:1721 and :1769:``` npm_range_admits / `npm_comparator` (npm's bare `1.3.0` is exact and `1.3` is `~1.3`; hyphen ranges; `||`; a dist-tag gives `None` and so a warning)
• yarn_berry_lock.rs:1545:```` user_routed_packages, the packages whose recorded key is kept
• yarn_berry_lock.rs:1518:````` restored_since, so a second revert pass converges instead of reporting drift
• berry_prune.rs:34:`````` declared_descriptors moved verbatim from the hosted restore in `npm.rs`, so hosted behavior is unchanged

Verified:

• Read the full diff. The helper move is a pure relocation, and CHANGELOG.md is untouched.
• The three real-yarn e2e regressions (add descriptor, remove descriptor, 4.0.2 to 4.18 checksum respelling) are plain #[test]s (not ignored) and ran in CI. The description shows their red YN0028 output on main.
• Ran locally on 2cccdba: the issue_759*, npm_range_admits*, lock_checksum_spelling* and berry_descriptor_declared* core unit tests (10/10 pass).
• CI: 36/36 green (50 path-skipped). Cursor Bugbot passed on this head. No unresolved threads. Mergeable.

Changes I made: none.

Open questions (non-blocking): npm_range_admits returns None (and so a warning) if any || alternative is unreadable, even when another alternative admits the version. That is conservative: it only adds a warning that tells the user to run yarn install.

Auto-merge (squash) is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code