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.

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
Takeover ordering changes affect hosted gem projects switching to vendored mode; incorrect preflight could leave pins hosted or still un-host in edge cases, but the change aligns with existing Bun/npm preflight patterns and is heavily tested.

Overview
Hosted→vendored takeover for gems now runs the same pre-checks as the gem vendored backend before restore_upstream un-hosts the pin. That covers the manifest gate (gemfile_not_loaded) and the Gemfile declaration gate on the restored Gemfile text via a dry-run restore (gem_vendor_target_preflight), so gems declared inside group blocks stay hosted when vendored mode cannot edit them.

Wet runs fail with gemfile_declaration_not_editable (or other gem codes) with Gemfile / Gemfile.lock unchanged. Dry-run scan / get --mode vendored adds would_refuse preview rows instead of would_vendor. The download phase and rollout planning apply the same refusals so refused gems are not fetched early.

CLI_CONTRACT.md documents the gem preflight alongside Bun/npm takeover rules. E2E covers the group-block fixture (ScanVexGroupBlock).

A small refactor routes Gradle/Maven JVM sha1/sha256 hashing through crate::utils::digest in gradle_cache, jvm_jar, and maven sidecars (behavior unchanged).

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

✅ 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 ecd761e. Configure here.}

[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