Fix gem takeover un-hosting a grouped gem (#775)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #775

Summary

When a hosted gem is declared inside a group … do block, scan --mode vendored / get --mode vendored no longer un-host it and then refuse it. The takeover now asks the gem backend's Gemfile declaration gate before it restores the hosted pin:

• Wet run: failed gemfile_declaration_not_editable (exit 1 / partial_failure). The hosted Gemfile / Gemfile.lock are byte-untouched, so the gem stays hosted-patched.
• Dry run: the preview reports the gem as would_refuse with errorCode: gemfile_declaration_not_editable (exit 0, the same posture as the Bun/vlt would_refuse rows). It no longer says would_vendor.

socket-patch vendor (eject) already rolled back correctly and is unchanged.

Root cause

The hosted→vendored takeover (vendor_records_reusing in commands/vendor.rs) runs restore_upstream, which writes the upstream Gemfile + Gemfile.lock, before the gem vendored backend runs. The only gem gate before the restore was gem_manifest_refusal (a gems.rb twin or BUNDLE_GEMFILE). The declaration gate (plan_gemfile_edit / refuse_append_of_direct_dependency) only ran inside the backend, after the restore had been written. The dry-run preview is a ledger classification with no engine refusals, so it could not see the problem either.

Changes

• socket-patch-core/src/vendor/gem.rs: new gem_vendor_target_preflight, the gem twin of yarn_berry_vendor_target_preflight. It runs a dry-run restore_upstream of the pin and evaluates the declaration gate on the restored Gemfile / Gemfile.lock text. That way socket-patch's own hosted source … do block is never mistaken for the user's declaration. It never writes.
• socket-patch-cli/src/commands/vendor.rs: gem_takeover_refusal (manifest gate, then the declaration preflight) is used by the takeover loop before the restore. gem_takeover_preview_refusals does the same for the dry-run preview.
• scan/vendor_flow.rs, scan/mod.rs, get.rs: preview_vendor_json takes the resolved takeover refusals and renders them as would_refuse rows (JSON and the human [would-refuse] lines).
• get.rs (lock_text_refusals_for) and scan/vendor_flow.rs (preflight_refused_purls): the wet download phase and scan's rollout planning pass apply the same gem takeover refusal, so a refused gem is reported before its patch view is fetched and never takes a rollout slot (Bugbot finding on b8b7752).
• CLI_CONTRACT.md: new "Gem preflight before the takeover" paragraph under "Takeover reconciliation".

No wrapper changes are needed: npm/, pypi/ and gem/ only dispatch to the binary.

Test evidence

Issue	Test	Red without fix → green with fix
#775 (wet get/scan --mode vendored)	e2e_redirect_gem_build::gem_hosted_group_block_pin_survives_a_refused_vendored_takeover (real Bundler 4.0.17)	Red: vendor_takeover_reverted_redirect emitted, Gemfile/lock un-hosted. Green: failed gemfile_declaration_not_editable, files byte-identical.
#775 (dry run)	same test, dry_run=true legs for get and scan	Red: "action": "would_vendor". Green: would_refuse + errorCode.
preflight logic	vendor::gem::tests::takeover_preflight_* (3 tests: group block refused and nothing written, top-level hosted gem passes, a refused restore is left to the takeover)	new
preview rendering	scan::vendor_flow::preview_tests::preview_marks_a_refused_takeover_would_refuse	new

Red was shown by stubbing gem_vendor_target_preflight to return None. Both the dry legs and the wet leg failed exactly as reported in #775.

The e2e respells the mock upstream as https://rubygems.org/ in the hosted pair and serves it through SOCKET_RUBYGEMS_URL. The takeover restore only re-derives a CHECKSUMS sha256 for a rubygems.org remote, which is the shape of the report.

Commands run locally:

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-cli --all-features --test e2e_redirect_gem_build --test e2e_vendor_gem_build -- --ignored: 12 + 6 passed.
• cargo test --workspace --all-features: all pass except 16 write-failure tests across 4 binaries. Those depend on chmod making files unwritable, which does not hold when running as root (uid 0) in this sandbox. They do not touch the changed code, and CI runs them non-root.
• Formatting: main is not cargo fmt-clean under the pinned toolchain, and CI has no fmt check. Only the hunks this PR changes are rustfmt-formatted, so the diff stays reviewable.

Merge with main (42673c6)

The merge was clean and brings in #712 (gem VEX now honours an out-of-tree bundle path), #829 and #874. It also re-triggered the CodeQL Analyze jobs that the 2026-10-05 runner outage had cancelled. Re-validated on 42673c6:

• cargo clippy --workspace --all-features -- -D warnings: clean.
• core lib: 5249 passed. The 4 failures are the known chmod-based tests, which fail only because this sandbox runs as root.
• e2e_redirect_gem_stale_install 32/32 and e2e_vex_redirect 31/31 pass.
• e2e_redirect_gem_build --ignored 17/17 and e2e_vendor_gem_build --ignored 8/8 pass (real Bundler 4.0.18).
• CI: all 547 check runs on 42673c6 are green or skipped, including all four CodeQL Analyze jobs. Bugbot is clean, there are no unresolved review threads, and mergeable_state is clean.

Follow-ups (not in this PR)

• Gem hosted → vendored takeover un-hosts a gem declared inside a group block and then refuses to vendor it (gemfile_declaration_not_editable), so the project silently goes back to unpatched #775's side note: a top-level declaration whose service has no prebuilt artifact (vendor_prebuilt_required) is still un-hosted before the failure. That is the same "restore written before the vendor step can fail" class as npm lockfileVersion 1: scan/get --mode vendored un-host a hosted patch and then refuse to vendor it, so the project silently goes back to unpatched (vendor eject rolls back correctly) #659 / npm vendored refuses a registry package with vendor_workspace_member whenever a local file: directory (or workspace member) has the same name@version, and the hosted→vendored takeover then un-hosts it, leaving it unpatched #688. A general fix would snapshot the takeover's restore and roll it back on any backend failure, as vendor eject already does. That is a cross-ecosystem change and is better as its own PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_0145M3bGaAVYdPzNXXD75tqe

---

Note

Medium Risk
Changes hosted→vendored takeover ordering and vendored scan/get preflight for gems; mistakes could leave gems wrongly hosted or still un-host before refuse, but behavior is scoped to gem takeover paths with new tests.

Overview
Fixes #775: vendored takeover over a hosted gem inside a group block no longer restores upstream first and then fails, which left the gem unpatched in both modes.

Gem preflight before takeover restore (aligned with existing Bun/npm takeover gates): core adds gem_vendor_target_preflight, which dry-runs restore_upstream and runs the Gemfile declaration gate on the restored text so socket-patch’s hosted source … do wiring is not confused with the user’s declaration. The CLI wires this through gem_takeover_refusal / gem_takeover_preview_refusals in the vendor takeover loop, before any restore.

Wet runs report failed gemfile_declaration_not_editable with hosted Gemfile / Gemfile.lock unchanged. Dry runs show would_refuse instead of would_vendor. The same checks run in lock_text_refusals_for and scan rollout planning so refused gems are not downloaded or given rollout slots.

CLI_CONTRACT.md documents the gem preflight; e2e and unit tests cover group-block takeover (including real Bundler).

^{Reviewed by Cursor Bugbot for commit 9ca9497. 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)]

[agent] Ready for review at ec8f5f1.

• CI: 491/491 check runs finished (485 success, 6 skipped), 0 failing.
• Bugbot: reviewed ec8f5f1, no inline findings.
• Reviewer focus: gem_vendor_target_preflight in vendor/gem.rs (a dry-run restore_upstream and the declaration gate run before the takeover writes anything) and the new would_refuse preview rows.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] coverage is red on 5b14678 (the merge of main into this branch), but the failure comes from main, not this PR.

Failing tests (-p socket-patch-cli --lib):

• commands::vex_consumed::tests::hosted_expands_alias_only_copies (vex_consumed.rs:771)
• commands::vex_consumed::tests::hosted_reuses_expanded_npm_copies_and_merges_alias_variants (vex_consumed.rs:718)

Why it isn't this PR's: the same two tests fail on origin/main at 4646693 (reproduced locally), and main's own coverage check on 4646693 is red. This PR doesn't touch vex_consumed.rs or the npm discovery code.

Cause: the two merges conflict in meaning. #738 added these tests assuming find_manifest_package_copies_reusing misses aliased and nested-store copies, so that tracked_npm_hosted has to expand them. #605, merged afterwards, makes discovery find those copies directly. installed is no longer empty in the first test and already holds the alias and nested peers in the second.

Fix: I found no open PR for this. A minimal patch would update the two tests to #605's discovery: drop the installed.is_empty() assert and expect the alias and nested-store copies in installed, with no extra expansion call. The other option is to narrow #605 if those tests are the intended contract. Which one is a call for whoever owns #605/#738, so I'm not putting it in this PR. I'll port the fix here once it exists.

I didn't re-run the job: the failure is deterministic on main, so a re-run would fail the same way.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] coverage failed on 90fadc4 (the latest merge of main) because of main, not this PR. I've ported the fix.

Failing test: socket-patch-core utils::digest::tests::production_digests_go_through_the_helpers

Why it isn't this PR's: the test fails the same way on origin/main at 9c43dfc (reproduced locally), and main's own coverage check is red. #865's new guard test flags three files that #646 added and that still compute digests inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. This PR touches none of them.

Ported fix: I cherry-picked #878 (659ac2c, "Route Gradle digests through utils::digest") as ecd761e. With it, the guard test passes and clippy is clean. The only failures left in the core lib tests are the four chmod-based write-failure tests, which fail only because this sandbox runs as root. The port becomes a no-op once #878 lands on main.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] The red checks on ecd761e (lock-diff, plus the cancelled clippy, test, coverage-merge, e2e-build and the compatibility matrices) come from a runner outage, not from code.

What failed: the cancelled jobs carry the annotation "The job was not acquired by Runner of type hosted even after multiple attempts", so no test code ran. lock-diff failed only because the vlt jobs it compares never produced any vlt-results-* artifacts ("locks from no OS"). None of the finished runs on this commit has a real job failure; every non-success is a runner-acquisition cancellation.

Done: I re-ran the cancelled jobs once in each finished run (npm, Gradle, pnpm, Go and Benchmarks). The PR #776 CodeQL run can't be retried (the API returned 403). The CI and vlt runs are still queued, so I can't re-run them yet.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] Ready for review at ecd761e. CI: all workflow checks are green, including the Gradle hosted Windows legs that were still running last hour. The only non-green items are CodeQL default-setup Analyze (actions) and Analyze (javascript-typescript), which the runner outage cancelled. They aren't required and the API won't re-run them. Bugbot reviewed ecd761e with no findings. No open review threads. The branch is 3 commits behind main but has no conflicts.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Merged main into the branch as 42673c6. The merge was clean and brings in #712, #829 and #874. Before this, the only non-green checks were the CodeQL default-setup Analyze jobs that the 2026-10-05 runner outage cancelled. The API won't re-run those (403), so this push, which was needed anyway to bring the branch up to date, re-triggers them.

Checks run locally on 42673c6:

• cargo clippy --workspace --all-features -- -D warnings: clean.
• core lib: 5249 passed, 4 failed. The 4 are the known chmod-based write-failure tests, which can't pass when run as root in this sandbox.
• e2e_redirect_gem_stale_install 32/32 and e2e_vex_redirect 31/31 pass.
• e2e_redirect_gem_build --ignored 17/17 and e2e_vendor_gem_build --ignored 8/8 pass (real Bundler 4.0.18).

Next: watching CI and Bugbot on 42673c6.

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot 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 9ca9497. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[agent] mill 1.1.10 failed on 9ca9497, but the failure is a CI artifact transfer problem, not this PR's code.

What failed: the job never reached a test. It downloads two artifacts from the sbt build job: a 1.7 GB Docker image and a binaries tarball. Only the tarball's download log shows it completing. docker load --input sbt-artifacts/socket-patch-test-sbt.tar then failed with unexpected EOF, so the image download was most likely cut off.

Why it isn't this PR's: the job died in its setup step, before any build or test of this branch ran. This PR doesn't touch the sbt/Mill workflow or its image build.

Re-run: the API refused to re-run it while the rest of the workflow run is still in progress ("This workflow is already running"). I'll re-run the failed job once the run finishes.

---

Generated by Claude Code