GeezerVison is showing github.com — Fix nightly gem e2e: attribute nested Gemfile source blocks by mikolalysenko · Pull Request #1999 · SocketDev/socket-patch
Conversation
Fixes #1997.
Problem
The nightly CI schedule run on main is red on two e2e-extended gem legs, and has been since #1961 added the CRLF source-block arm:
- run 41585451732 (2026-11-03, head 3eb2ec1)
e2e-extended (ubuntu-latest, e2e_redirect_gem_build e2e_vendor_gem_build, 3.1, 2.1.4), job 125750207261e2e-extended (ubuntu-latest, e2e_redirect_gem_build e2e_vendor_gem_build, 3.1, 1.17.3), job 125750207277
Both fail only in gem_hosted_redirect_keeps_a_crlf_gemfile_crlf, in the crlf-gemfile-source-block arm. Rollback exits 1 with hosted_wiring_contested / patched_ref_unattributable: "GEM section lists 2 remotes (a merged multi-source lock), and no gem in it is source-pinned to Socket patch …". The failure is deterministic and I reproduced it locally on bundler 2.1.4. PR CI runs only the bundler 4.0.21 leg, so only the nightly hits it.
#1995 tried a test-only fix that skipped this arm on bundler < 2.2. It was closed unmerged, so this PR does the product fix, option 1 in #1997.
Root cause
The redirect leaves this Gemfile and lock (captured from the failing run on bundler 2.1.4):
source "http://127.0.0.1:39379/upstream"
source "http://127.0.0.1:39379/upstream" do
source "http://127.0.0.1:39379/patch-registry/gem/<token>/<uuid>/" do
gem "vuln-gem", "1.0.0"
end
end GEM
remote: http://127.0.0.1:39379/upstream/
remote: http://127.0.0.1:39379/patch-registry/gem/<token>/<uuid>/
specs: …
DEPENDENCIES
vuln-gem (= 1.0.0)!
Bundler <= 2.1 merges every rubygems source into one GEM section. Discovery then attributes a gem in that section only when DEPENDENCIES marks it ! and the Gemfile declares it inside a source … do block for that Socket remote. parse_source_block_decls in vex/discover/gem.rs didn't handle nested blocks: inside the user's outer block it skipped the inner source … do line and gave the gem to the outer upstream URL. So no gem matched the Socket remote, and rollback refused. Bundler itself scopes a gem to the innermost block (Dsl#with_source).
Fix
The reader now keeps the open source … do blocks on a stack. Each gem line belongs to the innermost block, and end closes the innermost one. I moved the opener parsing unchanged into source_block_opener.
Three callers use this grammar, and all of them now see a nested Socket block:
- the merged-section cross-check, which is this fix;
gemfile_source_block_pins, which ledger liveness uses;manifest_source_pins, which the rollout recorded view uses.
For the redirect's own output this is more correct: before, a nested Socket pin was invisible to all three. The safeguards are unchanged. The gem still needs the DEPENDENCIES !, a block naming that exact remote, and no other source declaring it.
Proof
- New unit test
merged_section_nested_source_block_is_attributed_innermost. A nested Socket block attributes the gem with LF and with CRLF endings. The inverted nesting, with another source innermost, is still patched_ref_unattributable.
source_block_grammar gains a nested case: outer, inner, then outer again after the inner end.- Real-bundler e2e, run locally with
--ignored:
| suite |
bundler 2.1.4 (before) |
bundler 2.1.4 (after) |
bundler 4.0.18 (after) |
| e2e_redirect_gem_build |
37 passed, 1 FAILED (crlf_gemfile_crlf) |
38 passed |
38 passed |
| e2e_vendor_gem_build |
11 passed |
11 passed |
11 passed |
cargo test -p socket-patch-core --lib: 7098 passed. Four tests fail the same way on unmodified main when run as root in this container (permission tests): relax_loop_must_not_traverse_symlinked_root, an_unremovable_hidden_lock_keeps_every_store_entry, and two wire_* rollback tests. They are unrelated to this change.cargo clippy -p socket-patch-core --all-targets reports nothing in the touched file.- I couldn't run bundler 1.17.3 locally because it won't start on Ruby 3.3. The 1.17.3 leg takes the same merged-section path as 2.1.4. The nightly e2e-extended run (or a
workflow_dispatch of CI) confirms it.
Where tests run
No test is removed or moved. The failing arm stays in the nightly e2e-extended legs on every bundler era.
https://claude.ai/code/session_011f9aeSZhUZfmB5eQt427Fy
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The nightly e2e-extended gem legs on bundler 1.17.3 and 2.1.4 fail in
gem_hosted_redirect_keeps_a_crlf_gemfile_crlf (#1997). The hosted
redirect wraps a gem the user declared in their own `source "..." do`
block in a Socket block nested inside it. Bundler <= 2.1 writes one
merged GEM section, so discovery attributes the gem only through the
Gemfile source block that pins it. The Gemfile reader ignored nested
blocks and gave the gem to the outer upstream block, so rollback
refused with hosted_wiring_contested / patched_ref_unattributable.
Track open `source ... do` blocks as a stack and scope each gem to the
innermost one, as bundler's Dsl#with_source does. The same reader
backs ledger liveness and manifest_source_pins, which now see the
nested Socket pin too.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011f9aeSZhUZfmB5eQt427Fy
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 9a62f63. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Labeled Ready for review by the burn-down agent.
- Head:
9a62f63713e155731fe85f255710e6ef7f1770c3
- CI: 31/31 required runs green (19 skipped by path filters), no failures
- Bugbot: reviewed 9a62f63, no findings
- Diff touches only
crates/socket-patch-core/src/vex/discover/gem.rs; no CHANGELOG change
- Reviewer note: the nightly
e2e-extended gem legs (bundler 1.17.3 / 2.1.4) only run on the schedule, so the fix for Nightly gem e2e red on bundler 1.17/2.1: CRLF source-block rollback unattributable #1997 is confirmed by the first nightly after merge.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Final review brief
What it does: The Gemfile source … do block reader now keeps a stack of open blocks, so a gem belongs to the innermost block (as bundler's Dsl#with_source does). The hosted redirect nests its Socket block inside a user's own source block, and on bundler ≤ 2.1 merged locks that gem was unattributable, so rollback refused. This is the deterministic nightly e2e-extended gem failure (#1997).
Risk: low. One file, a small parser change, with the opener parsing moved unchanged into a helper. Attribution still needs the DEPENDENCIES !, a block naming that exact Socket remote, and no other source declaring the gem.
Look here:
- gem.rs:242-287:
parse_source_block_decls stack (push on opener, pop on end, gem goes to open.last())
- gem.rs:290-305:
source_block_opener, moved from the old match arm
- gem.rs:314 / gem.rs:360: the other two callers (ledger liveness, rollout recorded view), which now also see nested Socket pins
- gem.rs:1278: new test for nested LF and CRLF, plus the inverted nesting that must stay unattributed
Verified:
- Read the full diff (1 file, +101/−48). CHANGELOG is untouched and there's no leftover debug code.
cargo test -p socket-patch-core --lib vex::discover::gem: 28/28 pass.- Mutation check: with nesting disabled (inner
source … do ignored while a block is open, as before), merged_section_nested_source_block_is_attributed_innermost and source_block_grammar both fail. So the tests exercise the fix.
- CI:
ci-ok and clippy green on 9a62f63. Bugbot reviewed 9a62f63 with no findings. No review threads.
Changes I made: none.
Open questions: none that block. One note, not new to this PR: a non-source block (group … do / if) inside a source block still closes the source block at its end, as it did before. The redirect only writes gem lines inside Socket blocks, so this can't turn a gem into a false Socket attribution in practice. The bundler 1.17.3 / 2.1.4 legs run only nightly, so the first nightly after merge is the end-to-end confirmation.
Auto-merge is armed: approving sends this straight to the merge queue.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Final review brief
What it does. The Gemfile source … do block reader in gem discovery now keeps a stack of open blocks, so a gem belongs to the innermost one, the same way bundler's Dsl#with_source scopes it. The hosted redirect nests a Socket block inside a user's own source block. On bundler <= 2.1 (merged GEM section) that gem was attributed to the outer upstream URL, so rollback refused with patched_ref_unattributable. That is the nightly e2e-extended gem failure in #1997.
Risk: low. One file and one function. The block reader only ever adds a cross-check to lock evidence and never creates a ref by itself. Non-nested Gemfiles parse exactly as before.
Look here
- gem.rs:242: the
open stack replaces Option<String> (L245), and end pops the innermost block (L274)
- gem.rs:290:
source_block_opener, the opener grammar extracted unchanged
- gem.rs:1278: the new end-to-end test covers nesting (LF and CRLF) and the inverted nesting, which must stay unattributed
- gem.rs:1650: the parser test now covers a gem declared after a nested block closes. It fails on main, where the first
end closed the outer block.
Verified
Changes I made. None.
Open questions (non-blocking). This one predates the PR. Non-source do blocks (group :test do, platforms … do) are still not tracked, so their end closes the enclosing source block. Before this PR that could only drop an attribution. With nesting it could misattribute in one contrived layout: a Socket block outside, another source block inside it, and a group … do before the gem. The redirect never writes that layout, but a follow-up could push non-source do openers too, inheriting the enclosing URL.
Auto-merge (squash) is armed, so approving sends this straight to the merge queue.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Nov 5, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Nov 6, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
ci-janitor
Opened by the CI janitor routine (flakes, redundant tests, CI perf)
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
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.
|