GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix gem rollback leaving superseding Gemfile block (#1626) by mikolalysenko · Pull Request #1664 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #1626

fe87eaf

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 22, 2026

Hosted gem rollback / remove after an unconverged superseding re-scan restore Gemfile.lock but leave the new patch's source … do block in the Gemfile, report success, and the next bundle install re-patches #1626

Closed

Claude (claude) added 2 commits

October 22, 2026 08:46


          Undo a superseding patch's Gemfile block on unwind

ed663e4

On a Gemfile.lock without a CHECKSUMS section, a hosted re-scan that
picks up a newer patch for the same gem moves only the Gemfile's
source block to the new patch; the lock keeps the old one until the
next bundle install. A rollback or remove in that window restored
the lock but left the new block, reported success, and the next
bundle install put the patched gem back.

The restore now also undoes a Socket source block for the same gem
name and version under another patch uuid when the lock pins the
gem, so the Gemfile and lock come back consistent. Ambiguous shapes
(several such blocks, or the uuid named elsewhere) are still refused.

Fixes #1626

Assisted-by: Claude Code:claude-opus-5-5


          Test gem unwinds after an unconverged supersede

387f83d

Adds the end-to-end repro from the issue: hosted scan, converge,
superseding re-scan that leaves the Gemfile on the new patch and the
lock on the old one, then rollback and remove. Both must leave no
Socket wiring in either file, and a frozen bundle install must accept
the restored pair. Documents the case in the gem unwind contract.

Refs #1626

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 22, 2026

Swarm health manager log #1382

Open


          Refuse the superseding gem pin restored alone

b3caa54

When the Gemfile names the newer patch but the lock still pins the
older one, restoring only the newer patch's pin undid its block and
left the lock on the older patch, so the pair stayed inconsistent.
That pin is now refused with a message naming the older patch, while
the older patch's pin restores both files. Pins the lock really wires
(not ones whose uuid just appears in it) go first, so either order
of the two pins gives the same result.

Refs #1626

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 22, 2026 09:30

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 22, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/patch/redirect/upstream/gem.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Only own lock sections name an earlier gem patch

423f486

On a bundler < 2.2 lock, one merged GEM section lists the specs of
every remote together, so another gem's Socket remote there says
nothing about which patch serves this gem. The earlier-patch check
read it as one and refused a valid restore of this gem. It now looks
only at sections with a single remote.

Refs #1626

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 22, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 22, 2026

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Fix misindented restore_one argument in a gem test

da3c353

rustfmt skips this test function (its long string literals defeat it),
so the extra indentation the #1626 change introduced on `&ctx,` was never
corrected by `cargo fmt --check`. Align it with the other arguments.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 22, 2026

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

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[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 22, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 22, 2026

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) enabled auto-merge

October 22, 2026 13:15

Tanmay Singla (Tanmay182003) approved these changes

Oct 22, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Oct 22, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks

Oct 22, 2026

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

[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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 22, 2026

Depot runners lost mid-job evicted 2 merge-queue entries in 27h #1660

Open

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Oct 22, 2026

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

[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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 22, 2026

CI perf (settings): 90-min merge-queue check timeout let one stuck check freeze main for 77 min #1670

Open

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 22, 2026

Fix npm 12 pinned allowScripts lost on scan (#1595) #1657

Merged

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks

Oct 22, 2026

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

[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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Oct 22, 2026

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Copy link

Copy Markdown

Collaborator Author

[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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Merged via the queue into main with commit e2b8914

Oct 22, 2026

54 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-gem-restore-superseded-block branch

October 22, 2026 20:02

This was referenced Oct 22, 2026

CI performance dashboard #1182

Open

Nightly: gem unwind-after-unconverged-supersede e2e fails on bundler 1.17.3/2.1.4 #1685

Closed

Build: fix nightly gem unwind test on bundler < 2.2 (#1685) #1687

Closed

Fix Windows nightly test failures from path forms #1686

Merged

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Fix

Test evidence

/code-review high: findings not fixed

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Final review brief

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.