Skip to content

Fix merged gem locks with several hosted gems (#1749) - #1750

Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-gem-merged-multi-socket-remotes
Open

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-gem-merged-multi-socket-remotes

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1749

Problem

Bundler 1.17–2.1 lock every rubygems remote into one merged GEM section. When two gems are hosted, that section lists two Socket patch-registry remotes. Gem lockfile discovery gave up on such a section:

  • manifest-less vex found no patch refs and exited 2 (patched_ref_unattributable, then manifest_not_found)
  • an in-sync scan --mode hosted skipped both gems as redirect_unattributable, and still exited 0
  • a superseding patch for either gem was skipped the same way on every run, so the project stayed on the older patch

Root cause

hosted_wiring in crates/socket-patch-core/src/vex/discover/gem.rs attributed a merged section only when it held exactly one Socket remote (socket_count == 1), and only ever returned the first Socket remote's wiring (socket.into_iter().next()?). The scan's pre-write discovery check vetoed the rewrite before refresh_merged_gem_lock_remote, which already handles several remotes, could run.

Change

  • hosted_wirings returns one wiring per Socket remote of a section. In a merged section each remote gets the gems source-pinned to it: ! in DEPENDENCIES, a Gemfile source "<that remote>" do block, and no block for another source. That last rule also keeps a gem declared under two Socket remotes from being attributed to either.
  • A Socket remote with no pinned gem is diagnosed ref_unattributable on its own, and no longer hides the section's other Socket remotes.
  • A merged-section Socket remote that is not a patch-registry index is now ref_invalid, as in a single-remote section. Before, it was misreported as an unpinned gem.
  • A merged remote whose pinned gems the section does not lock now gets a ref_unattributable diagnostic instead of silently yielding nothing.
  • The module docs are updated. The CLI_CONTRACT.md gem row already describes the cross-check without a remote count, so it is unchanged.
  • The npm, pypi and gem wrappers only dispatch to the binary, so they needed no change.

Tests (red → green)

Test Covers Without the fix
vex::discover::gem::tests::merged_section_attributes_each_socket_remote the issue's exact lock shape: two Socket remotes, both gems pinned → two refs, each tied to its own remote fails: no refs
vex::discover::gem::tests::merged_section_unpinned_socket_remote_is_diagnosed_alone partial pinning; a gem in blocks for both remotes; a non-index remote; a pinned gem missing from the section fails
e2e gem_hosted_two_gems_rescan_supersede_and_attest (real bundler) two hosted gems → bundle install → one merged section on 2.1; an in-sync re-scan changes nothing and skips nothing; manifest-less vex attests both; a superseding patch for one gem lands in the Gemfile and the lock; a cold checkout installs frozen fails at the re-scan with redirect_unattributable ×2, the issue's symptom

merged_section_without_a_full_source_pin_is_unattributable drops its "two Socket remotes" case. That case asserted the old all-or-nothing behavior, which is the bug. The new tests above cover the case instead.

Per-issue checklist:

Commands run locally

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --all-features -- -D warnings: clean
  • e2e_redirect_gem_build with --ignored and SOCKET_PATCH_BUNDLER_E2E_REQUIRED=1: 34/34 on bundler 2.1.4 and on 4.0.18. The new test alone also passes on 2.5.22.
  • e2e_vendor_gem_build with --ignored: 9/9 on 2.1.4 and on 4.0.18
  • cargo test -p socket-patch-core --all-features --no-fail-fast: 7457 passed, 4 failed. The 4 are copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_… and pypi_requirements::wire_failure_…. They are chmod-based tests that can't fail when run as root in this sandbox, and this PR doesn't touch their code (same 4 as noted on Fix hosted cargo dotted-key and literal pins (#757) #1743).
  • cargo test -p socket-patch-cli --all-features --lib --bins: 935 passed
  • The full cargo test --workspace --all-features build ran out of the sandbox's disk allowance, so the CLI integration suites are left to CI. This PR changes only gem discovery and one gem e2e file.
  • Bundler 1.17.3 was not run locally: it needs Ruby ≤ 3.1, and this sandbox has 3.3. The CI leg covers it.

/code-review high

Fixed: the non-index remote misdiagnosis and the double is_patch_registry_index call; the silent no-ref merged wiring; per-uuid diagnostic assertions; the frozen install now runs on a cold checkout; the docs wrap and wording.
Not fixed: the generation-fixture setup duplicated from gem_hosted_superseding_patch_keeps_gem_sections_in_bundler_order. A shared helper would touch an unrelated test, and the copy is test-only.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Bundler 1.17-2.1 lock all rubygems remotes into one GEM section.
With two hosted gems that section lists two Socket remotes, and
lockfile discovery gave up on it: vex found no patches, and a
re-scan skipped both gems as redirect_unattributable (exit 0), so
newer patches never landed (#1749).

Discovery now reads each Socket remote of a merged section on its
own and attributes the gems source-pinned to it. A remote with no
pinned gem is still diagnosed, without hiding the others.

Assisted-by: Claude Code:claude-opus-5-5
A merged Bundler 2.1 lock with two hosted gems must re-scan clean,
attest both gems in vex, and take a superseding patch for one of
them. Without the discovery fix the re-scan skips both gems as
redirect_unattributable.

Assisted-by: Claude Code:claude-opus-5-5
A merged-section Socket remote that is not a patch-registry index
is now reported as an invalid ref, as it is in a single-remote
section, rather than as an unpinned gem. A remote whose pinned
gems the section does not lock now gets a diagnostic instead of
yielding nothing silently. The e2e frozen install now runs on a
cold checkout.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 27, 2026 22:15
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 7b6a9ea. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

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

  • CI: 82/82 checks green (32 success, 50 skipped) on 7b6a9ea; no main-wide failures.
  • Bugbot: reviewed 7b6a9ea, no findings; no open review threads.
  • Reviewer focus: crates/socket-patch-core/src/vex/discover/gem.rs now attributes every Socket remote in a merged GEM section, plus the new e2e in e2e_redirect_gem_build.rs.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. Bundler 1.17–2.1 lock every rubygems remote into one merged GEM section. With two hosted gems that section lists two Socket patch-registry remotes, and gem discovery gave up on the whole section, so vex exited 2 and the hosted re-scan skipped both gems as redirect_unattributable (#1749). Discovery now attributes each Socket remote on its own, to the gems source-pinned to it (! in DEPENDENCIES plus a Gemfile source … do block for that remote and no other).

Risk: low–medium. The change is confined to gem lockfile discovery and stays fail-closed: a gem declared in blocks for two Socket remotes is attributed to neither. Two diagnostics change on purpose. A non-index Socket remote in a merged section is now ref_invalid (it was misreported as unattributable), and a pinned gem missing from the section is now diagnosed instead of silently dropped. The medium part: CI's bundler 1.17.3/2.1.4 legs (e2e-extended) run only on schedule or dispatch, not on PRs or in the merge queue, so CI here exercised the new e2e only on bundler 4.0.21, which doesn't produce a merged section. I ran it on 2.1.4 myself (below).

Look here

  • crates/socket-patch-core/src/vex/discover/gem.rs:501-579: hosted_wirings, one wiring per Socket remote, and the per-remote pin rule
  • crates/socket-patch-core/src/vex/discover/gem.rs:424-474: section_refs, which adds the new "pinned but not locked" diagnostic
  • crates/socket-patch-core/src/vex/discover/gem.rs:1344-1480:`` the two new unit tests, which cover partial pins, a gem pinned to both remotes, a non-index remote and a missing spec
  • crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs:2321:``` the real-bundler e2e (re-scan, vex, supersede, frozen cold install)

Verified

  • Read the full diff (2 files, +472/−100). CHANGELOG.md is untouched. No debug leftovers.
  • cargo test -p socket-patch-core --all-features --lib vex::discover::gem: 27/27 passed.
  • gem_hosted_two_gems_rescan_supersede_and_attest passed on real bundler 2.1.4, the merged-section era, with SOCKET_PATCH_BUNDLER_E2E_VERSION=2.1.4 and …_REQUIRED=1, so the version is asserted.
  • CI on 7b6a9ea: 32 success, 50 path-skipped, 0 failed. Bugbot: no issues. No unresolved threads.

Open questions: none that block merging. Bundler 1.17.3 is covered only by the next scheduled e2e-extended run.

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


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

2 participants