[Mikola Lysenko (mikolalysenko)]

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

Fixes #1626

Summary

On a Gemfile.lock with no CHECKSUMS section, a hosted re-scan that picks up a superseding patch (same gem and version, new uuid) moves only the Gemfile's source "<patch registry>" do … end block to the new uuid. The lock keeps pinning the old patch until the next bundle install. If rollback or remove ran in that window, the lock was restored but the new block stayed. The command exited 0, frozen installs then failed, and the next unfrozen bundle install installed the patched gem again.

After this change, rollback and remove restore both files. If only the newer patch's pin is being restored, it is refused with a clear message instead of leaving the pair half-restored.

Root cause

patch/redirect/upstream/gem.rs: restore_one → find_block only matched a Gemfile block whose patch-registry uuid equals the uuid of the pin being restored. The lock's pin (old uuid) never matched the block the superseding scan wrote (new uuid).

Fix

• superseded_block: when the lock wires the pin but the manifest has no block for its uuid, undo the one rewriter-shaped Socket block for the same gem name and version under another hosted uuid. The shapes that stay refused are several such blocks, or that uuid named outside the block.
• If that superseding uuid is also a pin in the same run, it is marked handled and does not get a second edit. Pins the lock really wires (checked with wiring, not a substring match) are processed first, so pin order doesn't change the outcome.
• The other side: a pin whose only wiring is the Gemfile block, while the lock still pins the gem to an earlier Socket patch, is refused with a message naming that earlier patch. Undoing the block alone would leave the lock on that patch.
• CLI_CONTRACT.md's gem unwind entry now describes this case.

Test evidence

Issue	Regression test	Without fix	With fix
#1626 (rollback + remove)	unit gem::tests::restore_undoes_a_superseding_patch_block (old pin alone; both pins in both orders; new pin alone is refused; two superseding blocks are refused)	FAIL (Gemfile left unchanged: left: None)	pass
#1626 (Bugbot: merged bundler < 2.2 section)	unit gem::tests::earlier_lock_pin_needs_a_section_of_its_own	FAIL (merged section read as an earlier pin)	pass
#1626 (rollback + remove, real bundler)	e2e gem_hosted_unwind_after_unconverged_supersede_undoes_the_new_block in e2e_redirect_gem_build: the issue's repro, then a frozen bundle install that must accept the restored pair	FAIL (rollback: the Gemfile is still wired to a Socket patch)	pass

To get the red runs, I disabled the new superseded_block branch locally and ran the same tests.

Local runs (Linux, Ruby 3.3.6, Bundler 4.0.18):

• cargo fmt --all -- --check: ok
• cargo clippy --workspace --all-features -- -D warnings: ok
• SOCKET_PATCH_BUNDLER_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_redirect_gem_build --test e2e_vendor_gem_build -- --ignored: 31 + 8 passed, 0 failed
• cargo test --workspace --all-features --no-fail-fast: 14,671 passed, 10 failed. All 10 are environment-only in this sandbox, which runs as root and has no access to the real patch API: the read-only-dir and unremovable-file simulations (copy_tree, vlt_heal, pypi_poetry, pypi_requirements, in_process_redirect write-failure tests, repair cleanup tests), and e2e_bun_lockb, which tries to reach patches-api.socket.dev and gets a proxy tunnel error. None of them touch gem code. CI runs them as non-root.
• No wrapper changes needed: npm/, pypi/ and gem/ only dispatch to the binary.

/code-review high: findings not fixed

• "The superseding block is removed even though its uuid isn't a pin." Removing it is the fix. That uuid is never a discovered pin: discovery reads the lock, which is why the issue's second rollback fails with manifest_not_found. Its wiring has to go together with the lock pin it replaced.
• "Two regex passes per pin." Only on the rare path where the lock wires the pin but has no matching block, over a short Gemfile. Not worth changing the shape of the code.
• "Fix it on the scan side so the window never exists." On a lock with no CHECKSUMS, the superseding scan can't converge the lock without a bundle install. That is the documented redirect_gem_no_checksums_section behaviour, and changing it is a separate design decision. This PR makes the unwind correct in that window.

https://claude.ai/code/session_01HwsQHc975WcR4onkKf9gvf

---

Note

Medium Risk
Changes hosted gem rollback/remove behavior in a narrow but user-visible window (unconverged supersede); logic is well-tested but touches lock/manifest pairing and refusal paths.

Overview
Fixes #1626: after a superseding hosted re-scan on a CHECKSUMS-less lock, the Gemfile can point at a new patch UUID while Gemfile.lock still pins the old one. rollback and remove used to restore only the lock and leave the new source "<patch registry>" do block, so the pair looked “restored” but frozen bundle install failed and the next unfrozen install re-patched.

Gem upstream restore now treats that mismatch explicitly. When the lock wires the old pin but the manifest block names another hosted UUID for the same gem/version, superseded_block removes that superseding block as part of restoring the lock pin; pins the lock actually wires are processed first, and the superseding UUID is marked handled so it isn’t edited twice. Restoring only the newer pin is refused with guidance when the lock still pins an earlier Socket patch (earlier_lock_pin, ignoring merged Bundler < 2.2 sections). CLI_CONTRACT.md documents the case; unit and Bundler e2e tests cover rollback/remove and frozen install after unwind.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at 423f4866.

• CI: 35/35 green, 19 skipped; mergeable, no conflicts.
• Bugbot: clean on 423f486. Its one earlier finding on b3caa54 (a merged bundler < 2.2 section read as an earlier pin) was fixed and resolved.
• Reviewer focus: superseded_block in upstream/gem.rs, which undoes another hosted uuid's Gemfile block only when there is exactly one rewriter-shaped block for the same gem and version.

Slack announcement not sent: this run has no Slack send tool.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

Final reviewer: pushed da3c353, a whitespace-only fix (a misindented &ctx, argument in the restore_one round-trip test that rustfmt skips). The rest of the review found nothing blocking: the gem unit tests pass locally (20/20), and the logic in superseded_block, earlier_lock_pin and the pin ordering in restore checks out. I removed Ready for review so the burn-down re-labels it once CI is green, and I will post the review brief on the next run.

---

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

[Mikola Lysenko (mikolalysenko)]

Ready for review at da3c3539.

• CI: 35/35 green, 19 skipped; mergeable, no conflicts.
• Bugbot: clean on da3c353. Its one review thread (gem.rs:1563) is resolved.
• Since the last ready label: the final reviewer pushed a whitespace-only test fix (da3c353), so the label was removed and is now re-applied.
• Slack: not announced in this run because no Slack send tool was available.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. A superseding hosted re-scan on a lock with no CHECKSUMS moves only the Gemfile's source … do block to the new patch uuid. The lock keeps pinning the old patch until bundle install runs. Before this PR, rolling back or removing the old pin in that state left the new block behind. Now, restoring the old pin also removes the superseding block. Restoring the superseding pin on its own, while the lock still pins the earlier patch, is refused with a hint to restore the earlier pin or run bundle install first.

Risk: low. The change is limited to the gem restore path and only fires in a narrow state: the lock wires A, and the Gemfile has only a rewriter-shaped Socket block for B with the same name and version. Ambiguous cases, such as several candidate blocks, are refused rather than guessed. Blocks a user wrote themselves are never matched.

Look here

• gem.rs#L503 superseded_block: decides when B's block is deleted, and refuses on more than one match.
• gem.rs#L1275-L1291:``` restore orders lock-wired pins first. The `undone` set stops a superseded pin being restored twice.
• gem.rs#L1438 restore_one: the superseding block is used only when the pin is wired and has no block of its own. This is also where restoring the superseding pin alone is refused.
• gem.rs#L1550 earlier_lock_pin: counts only a single-remote GEM section, so a merged pre-2.2 section can't refuse another gem.
• gem.rs#L1335:`````` the undone pin is skipped in the stale-cache warning pass. Its archive is the same .gem file, so no warning is lost.

Verified

• Read the full diff, the description and CLI_CONTRACT.md.
• cargo test -p socket-patch-core gem: 361 passed.
• Ran a mutation check on both new paths: disabling the superseding-block lookup, and separately disabling the earlier_lock_pin refusal. Each makes restore_undoes_a_superseding_patch_block fail, so the test does guard the fix.
• clippy -p socket-patch-core --lib -D warnings: clean. rustfmt --check: clean.
• The regex accepts CRLF.
• No debug leftovers. CHANGELOG.md is untouched.
• CI: 35/35 green (19 skipped by design). Bugbot: success.
• Not run locally: the #[ignore]d e2e, which needs ruby and bundler.

Changes I made: none.

Open questions: none blocking. Nits:

• CLI_CONTRACT.md says the block is undone "on a lock with no CHECKSUMS section", but the code doesn't check for CHECKSUMS. It undoes the block whenever the lock wires the old uuid and the Gemfile has no block for it. That's harmless, but the docs describe a narrower case than the code.
• lock_wires re-parses the lock on every sort comparison. That's trivial at real sizes.
• There's no test for the CRLF or "uuid outside the source block" error paths.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Dequeued with CI_FAILURE, but the failure isn't in this PR's code. In the merge-group CI run 39820682471 (bef72f2), every job passed or was skipped except two:

• yarn-berry 4.18.0 (ubuntu-latest) was cancelled during Set up job after about 7 minutes in the queue, so no step ran.
• ci-ok then reported failure because of that cancel.

No test or build step failed. The same head (da3c353) is 35/35 green on the PR, and the diff doesn't touch yarn. Nothing needs fixing here: the PR is still approved, mergeable and clean, and it just needs to be re-queued.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[final reviewer] Re-added to the merge queue (first re-add). The 17:24 UTC eviction came from a yarn-berry 4.18.0 job cancelled during Set up job in merge-group run 39820682471, not from this diff. Head da3c353 is green, clean and approved.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Evicted a second time (19:31 UTC) for the same reason as before, which isn't this diff. In merge-group CI run 39838789738 (d10ae08), lint-ecosystems was cancelled during Set up job about 7 minutes after it started. No step ran, and ci-ok failed only because of that cancel. Every other job passed. The 17:24 eviction had the same cause (yarn-berry, Set up job).

This head also passed full queue CI twice in between, as the PR was rebuilt behind other entries: runs 39826164711 and 39837255979 both succeeded. A job that dies at runner setup points to Actions runner provisioning, not the code. Nothing to fix here. The PR is still approved and green, and it needs re-queueing again. If it keeps happening, the runner setup cancels are worth raising separately.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[final reviewer] Re-added to the merge queue at 19:49 UTC after the 19:31 eviction (lint-ecosystems cancelled during Set up job, runner provisioning, not this diff). Head da3c353 is approved, green and clean. If it is evicted again for a runner-setup cancel, that's a third infra eviction today and points at the runner pool (#1660); CI janitor and owner, please look before another re-add.

---

Generated by Claude Code