[Mikola Lysenko (mikolalysenko)]

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

Follow-up to #1762 (Refs #1761), from a review comment that arrived
after #1762 merged:
#1762 (comment)

Problem

#1762 made the gem vendored backend's dry run check the Gemfile/lock
edits the wet run would make. A dry hosted→vendored takeover in
vendor_records_reusing previews the backend twice: first over the
restored text (takeover_dry_refusal), then over the files on disk.
For a gem, the Gemfile on disk still wraps the declaration in the
hosted source "…/patch-registry/gem/…" do block, so the second
preview can now refuse it as "declaration is indented", even though the
wet run (which restores first) vendors it.

Fix

In that dry takeover branch, a gem stops after the restored-text
preview, the same way bun (bun.lock/bun.lockb) and sbt
(socket-patch.sbt) already do. The restored-text preview still raises
every real refusal, including the #1761 group-block one.

Note: no CLI path reached that branch with a hosted gem in my tests.
vendor ejects whenever a hosted pin is in scope (the eject previews
only over restored text), and the scan/get --mode vendored dry
runs classify against the ledger without calling a backend. The guard
is defensive: two lines, matching the bun/sbt handling.

Tests

• New e2e gem_hosted_pin_previews_and_runs_a_clean_eject
  (e2e_redirect_gem_build): a top-level hosted gem (converged lock)
  previews eject_planned (exit 0, nothing written), and the wet
  vendor then wires .socket/vendor/gem/…. This covers the
  "successful hosted→vendored gem dry run" case the review found
  untested. It also passes on the previous main; it is coverage, not a
  red→green test, since no CLI path reaches the branch.
• Local, on this branch: cargo fmt --all -- --check and
  cargo clippy --workspace --all-features -- -D warnings clean;
  e2e_redirect_gem_build + e2e_vendor_gem_build --ignored 36 + 9
  pass (Bundler 4.0.18, Ruby 3.3.6); in_process_vendor 147,
  in_process_vendor_bun_takeover 45, core vendor::gem 148 pass.

Code review (/code-review high)

Fixed in dc55a10: the guard skipped the disk preview for every gem, even when the restored-text preview never ran (uncaptured/binary reverted file), so nothing checked the gem. It now skips only when the restore rewrote a Gemfile/gems.rb and that text was previewable, and uses Ecosystem::from_purl.
Not fixed, on purpose:

• The skipped disk preview's Applied event/warnings are dropped for that gem: same trade-off bun and sbt already make here.
• Altitude: returning the full restored-overlay VendorOutcome from takeover_dry_refusal would remove all three ecosystem special cases. That is a larger refactor, out of scope for this follow-up.
• The e2e test does not reach the branch (no CLI path does, as noted above); it is coverage of the clean eject path.

---

Generated by Claude Code

---

Note

Medium Risk
Changes vendor takeover dry-run branching for gems; scoped like existing bun/sbt skips but can alter preview outcomes and warnings for hosted gem ejects.

Overview
Fixes a false dry-run refusal for hosted→vendored gem takeovers: after previewing the backend over restored Gemfile text, the CLI no longer runs a second preview against the on-disk Gemfile still wrapped in the hosted source … do block (which could trigger gemfile_declaration_not_editable even though the wet run restores first).

The preview gate is extracted into restored_text_previewable, and gems join the existing bun.lock / sbt skip path only when the restore rewrote Gemfile or gems.rb and that overlay was actually previewable.

Adds e2e gem_hosted_pin_previews_and_runs_a_clean_eject: converged hosted gem gets eject_planned on --dry-run (exit 0, no writes) and wires .socket/vendor/gem/… on the wet vendor.

^{Reviewed by Cursor Bugbot for commit dc55a10. Configure here.}

---

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

[Mikola Lysenko (mikolalysenko)]

[agent] dispatch-tests on dc55a10 failed before any step ran. The annotation says the self-hosted Depot runner (depot-t1f12b2hl8) "lost communication with the server", so this is runner loss, not the diff (which touches only vendor.rs and a gem e2e test, nothing the wrapper dispatch tests read). Once run 40726102548 finishes, it needs one gh run rerun 40726102548 --failed. The run was still in progress, so I could not re-run it yet. I am leaving the re-run to the burn-down agent, which holds this PR's heartbeat.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review at dc55a10 (burn-down agent). The previous heartbeat had gone stale, so I picked this up.

• Ran /code-review high. One finding fixed in dc55a10: the gem guard skipped the on-disk preview even when the restored-text preview never ran. It now skips only when the restore rewrote a Gemfile/gems.rb and that text was previewable. The findings left unfixed are listed in the description, each with its reason.
• Moved out of draft.
• CI: 82/82 check runs on dc55a10 green or skipped (32 success, 50 skipped); mergeable into main.
• Bugbot reviewed dc55a10: no issues found.
• Ran locally: e2e_redirect_gem_build + e2e_vendor_gem_build with ignored tests (57 + 44 pass), CLI lib tests (936 pass), and clippy on lib/bin.
• Slack announcement not sent: the Slack send tool was unavailable this run.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. A dry-run hosted→vendored takeover previews the vendor backend twice: over the restored text, then over the files on disk. Since #1762, the second preview for a gem could refuse the on-disk Gemfile because the hosted source … do block still wraps the declaration, even though the wet run (which restores first) vendors it fine. Now a gem skips the on-disk preview, as bun and sbt already do. It only skips when the restore rewrote a Gemfile/gems.rb and that restored text was actually previewed.

Risk: low. It changes a dry-run branch only (wet runs are untouched), and the restored-text preview still raises every real refusal. The author notes that no current CLI path reaches this branch with a hosted gem, so the guard is defensive.

Look here

• vendor.rs:3532: the new gem_block_reverted guard beside the bun/sbt skip
• vendor.rs:321: restored_text_previewable, moved out of takeover_dry_refusal with the same logic so both sites share it
• vendor.rs:3476: the restored-text preview that runs first and still refuses
• e2e_redirect_gem_build.rs:2957:`` the new dry and wet eject e2e test (it covers the clean eject path, but it passes without the fix, so it doesn't prove the fix)

Verified. Read the full diff and the call site. The skip happens only after takeover_dry_refusal returned no refusal over the restored text. Path::file_name matching works on both / and Windows paths. No debug leftovers, and CHANGELOG.md is untouched. CI 82/82 green on dc55a10 (clippy path-skipped). Cursor Bugbot passed. No unresolved threads.

Changes I made. None.

Open questions. None. The PR description lists the trade-offs the author left on purpose (the skipped disk preview's warnings are dropped, as for bun/sbt).

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code