Repository navigation
Fix merged gem locks with several hosted gems (#1749) - #1750
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Conversation
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
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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.
|
Burn-down agent: labeled Ready for review at head
Generated by Claude Code |
Final review briefWhat it does. Bundler 1.17–2.1 lock every rubygems remote into one merged 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 Look here
Verified
Open questions: none that block merging. Bundler 1.17.3 is covered only by the next scheduled Auto-merge is armed, so approving sends it straight to the merge queue. Generated by Claude Code |
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
GEMsection. When two gems are hosted, that section lists two Socket patch-registry remotes. Gem lockfile discovery gave up on such a section:vexfound no patch refs and exited 2 (patched_ref_unattributable, thenmanifest_not_found)scan --mode hostedskipped both gems asredirect_unattributable, and still exited 0Root cause
hosted_wiringincrates/socket-patch-core/src/vex/discover/gem.rsattributed 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 beforerefresh_merged_gem_lock_remote, which already handles several remotes, could run.Change
hosted_wiringsreturns one wiring per Socket remote of a section. In a merged section each remote gets the gems source-pinned to it:!in DEPENDENCIES, a Gemfilesource "<that remote>" doblock, and no block for another source. That last rule also keeps a gem declared under two Socket remotes from being attributed to either.ref_unattributableon its own, and no longer hides the section's other Socket remotes.ref_invalid, as in a single-remote section. Before, it was misreported as an unpinned gem.ref_unattributablediagnostic instead of silently yielding nothing.CLI_CONTRACT.mdgem row already describes the cross-check without a remote count, so it is unchanged.Tests (red → green)
vex::discover::gem::tests::merged_section_attributes_each_socket_remotevex::discover::gem::tests::merged_section_unpinned_socket_remote_is_diagnosed_alonegem_hosted_two_gems_rescan_supersede_and_attest(real bundler)bundle install→ one merged section on 2.1; an in-sync re-scan changes nothing and skips nothing; manifest-lessvexattests both; a superseding patch for one gem lands in the Gemfile and the lock; a cold checkout installs frozenredirect_unattributable×2, the issue's symptommerged_section_without_a_full_source_pin_is_unattributabledrops 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:
vexfinds no patch refs and every re-scan skips both gems asredirect_unattributable(exit 0), so superseding patches never land #1749 re-scan → e2e re-scan step and unitmerged_section_attributes_each_socket_remotevexfinds no patch refs and every re-scan skips both gems asredirect_unattributable(exit 0), so superseding patches never land #1749vex→ e2e vex step (both purls attested(redirected))vexfinds no patch refs and every re-scan skips both gems asredirect_unattributable(exit 0), so superseding patches never land #1749 supersede → e2e generation-3 stepCommands run locally
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleane2e_redirect_gem_buildwith--ignoredandSOCKET_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_buildwith--ignored: 9/9 on 2.1.4 and on 4.0.18cargo test -p socket-patch-core --all-features --no-fail-fast: 7457 passed, 4 failed. The 4 arecopy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_…andpypi_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 passedcargo test --workspace --all-featuresbuild 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./code-review high
Fixed: the non-index remote misdiagnosis and the double
is_patch_registry_indexcall; 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