[Mikola Lysenko (mikolalysenko)]

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

Fixes #692

Summary

Vendored yarn classic rollback, remove and vendor --revert now undo a lock block that yarn re-keyed after vendoring. That happens when a workspace member adds left-pad@^1.3.0 next to ^1.1.0 and yarn dedupes it into the wired block (left-pad@^1.1.0, left-pad@^1.3.0:), or when it drops one range of a merged key. Before this change, those commands reported the block as gone, kept the lock wired to the patched tarball and exited 1 on every run, so the patch could not be undone.

Root cause

The vendor ledger records each wired block by its exact key line. revert_recorded_block looked the block up with b.key == key. A re-keyed block keeps our resolved/integrity, so it is still ours, but the exact lookup missed it and the revert treated it as removed (vendor_lock_entry_removed). The CLI then kept the artifact because the lock still mentioned the uuid.

Fix

• When the recorded key is gone, yarn classic now looks for the one live block whose key shares a pattern with the recorded key and whose resolved still points into this entry's uuid (rekeyed_block). It restores the recorded pre-vendor body under the block's live key line, so the ranges merged in since vendoring stay locked.
• A re-keyed block that was re-resolved elsewhere (e.g. yarn upgrade) is not followed. It is treated like a removed block, so the revert finishes once nothing resolves through the artifact, and the drift keep isn't stuck forever.
• Berry passes None and keeps its existing lookup. Its revert restores the recorded key line (vendoring rewrote its locator key), so the key-line swap applies only to re-keyed classic blocks.
• Re-vendor over a merged block was already in sync (already_vendored). With the fixed revert, the ledger it keeps now reverts.
• No wrapper changes are needed: npm/, pypi/ and gem/ only dispatch to the binary.

Tests (red → green)

Issue variant	Test	Without fix	With fix
#692 member merge, rollback (real yarn 1.22.22 workspace, from the issue repro)	e2e_vendor_yarn_classic_dev_flow::yarn_classic_rollback_after_member_add_merges_the_vendored_key	fails (rollback exit 1)	passes
#692 merged key, revert restores under merged key and drops the artifact; re-run is a quiet no-op	yarn_classic_lock::tests::issue_692_revert_restores_a_block_yarn_merged_after_vendoring	fails (vendor_lock_entry_removed + vendor_artifact_kept)	passes
key lost a range, CRLF lock	issue_692_revert_restores_a_block_whose_key_lost_a_range	fails	passes
re-vendor after the merge is in sync, then revert converges	issue_692_revendor_after_merge_then_revert_converges	fails	passes
merged and re-resolved elsewhere: left untouched, artifact released	issue_692_merged_block_resolved_elsewhere_is_left_alone	passes (guard, not red→green: it pins the ownership check so the new lookup never follows a block that isn't ours)	passes

Commands run locally (all green unless noted):

• cargo fmt --all -- --check
• cargo clippy --workspace --all-features -- -D warnings
• cargo test -p socket-patch-core --all-features --lib: all yarn tests (390) pass. Two pypi wire-failure tests fail locally only because the container runs as root, which defeats their chmod-based failure injection; CI runs them unprivileged.
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_yarn_classic_dev_flow (with SOCKET_PATCH_YARN_E2E_REQUIRED=1), --test e2e_vendor_yarn_classic_build, --test in_process_vendor: all pass.
• Full cargo test --workspace --all-features --no-fail-fast: the only other failures are environmental and in code this PR doesn't touch. e2e_bun_lockb can't reach patches-api.socket.dev through the sandbox proxy, and a vlt heal-failure test also depends on non-root permissions.

/code-review high

• Fixed: following a re-keyed block that isn't ours (it kept the artifact forever after a legitimate upgrade); a false drift when two wired blocks merge into one; and warnings naming a key that no longer exists.
• Not changed: making the member yarn add step of the e2e a soft skip. The suite's convention is that every assertion after the fixture install is hard, and --prefer-offline serves the add from the warmed cache.
• Not changed: folding the per-flavor arguments into a struct and moving the re-keyed lookup out of the shared routine. Those are style refactors outside this fix; #[allow(clippy::too_many_arguments)] is the repo's existing convention (80+ uses).
• Not changed: pre-splitting keys for rekeyed_block. It runs only for records whose exact key is gone, once per record.

https://claude.ai/code/session_012b8PuECQR7xpwuQ48Xm2Wd

---

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 cb1eec0. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review at head cb1eec0.

• CI: 88/88 checks on cb1eec0 passed or skipped. No main-wide failures.
• Bugbot: reviewed cb1eec0, no findings; no open review threads.
• Mergeable, no conflicts; CHANGELOG.md untouched.
• Worth a look: rekeyed_block in crates/socket-patch-core/src/vendor/yarn_classic_lock.rs (matches a re-keyed block only when its resolved still points at this entry's uuid).
• Slack announcement not sent this run (Slack send unavailable); no slack-announced marker, so a later run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Vendored yarn classic rollback, remove and vendor --revert can now undo a lock block that yarn re-keyed after vendoring (another range deduped into it, or one range dropped). Before this fix the revert looked the block up by its exact recorded key, reported it as gone, left the lock wired to the patched tarball and exited 1 on every run. Now, when the exact key is gone, it follows the single live block that shares a pattern with the recorded key and still resolves into this entry's vendor uuid, and restores the pre-vendor body under that block's live key line.

Risk: low. The new path runs only when the recorded key no longer exists, and only for classic (berry passes None). It matches only when exactly one block is still ours (uuid ownership gate), so it can't touch a third party's block, and a block re-resolved elsewhere is treated as removed, as before.

Look here:

• rekeyed_block:`` the matching rule (shared pattern + ours + exactly one match)
• fallback lookup and the live key line swap
• ownership gate on the followed block
• real-yarn e2e from the Vendored yarn classic rollback can't undo a block yarn has merged with another range (left-pad@^1.1.0, left-pad@^1.3.0:): it reports the block as gone, exits 1 forever and leaves the lock wired #692 repro`````

Verified:

• Read the full diff: 3 files, +361/−8. No scope creep or debug leftovers, CHANGELOG.md untouched, branch is up to date with main.
• cargo test -p socket-patch-core --lib yarn_classic: 112/112 pass.
• With the lookup fallback reverted, 3 of the 4 new issue_692_* unit tests fail. issue_692_merged_block_resolved_elsewhere_is_left_alone passes either way, so it is a guard for the ownership gate rather than a red→green test, despite what the PR table says.
• e2e_vendor_yarn_classic_dev_flow::yarn_classic_rollback_after_member_add_merges_the_vendored_key with real yarn 1.22 (SOCKET_PATCH_YARN_E2E_REQUIRED=1): passes.
• cargo clippy -p socket-patch-core -p socket-patch-cli --all-features -D warnings: clean.
• CRLF is covered by a dedicated test.
• CI: 88/88 on cb1eec0 passed or skipped. Bugbot clean, no review threads.

Changes I made: none.

Open questions: none.

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

---

Generated by Claude Code