Fix Gemfile rewrites duplicating gem declarations (#482, #548)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #482
Fixes #548

Root cause

Both Gemfile rewriters decide between "rewrite the declaration" and "append a block" from a regex probe of the root Gemfile text alone:

• Hosted (patch/redirect/mod.rs, rewrite_gem): gem_line_re.captures(gf) takes only the first literal match. A gem declared twice (for example in two group blocks, Hosted gem redirect rewrites only the first of a gem's declarations, so a gem listed in two group blocks makes every bundle install fail with "You cannot specify the same gem twice" #548) gets one line rewritten to = x.y.z while the other stays >= 0, so Bundler refuses the Gemfile.
• Hosted + vendored (declared_re / plan_gemfile_edit): when the root text shows no declaration, the gem is treated as transitive and a block is appended. The rewriter never checks whether Bundler sees the gem as a direct dependency through eval_gemfile or a loop (Hosted gem redirect appends a second declaration when the gem is declared through eval_gemfile or a loop, so every bundle install fails with "You cannot specify the same gem twice" #482). It ends up declared twice, and every bundle install exits 4.

Fix

• formats/gem: the shared lock parser now records every DEPENDENCIES entry (GemfileLock::direct), exposed as lock_lists_direct_dependency.
• Hosted: a declaration count runs first, over gem calls whose first argument is the gem (another gem's require: "x" or a comment quoting the name does not count). More than one declaration gives redirect_gem_declared_more_than_once, with nothing written and nothing attested (vendored mode already refused this shape). Before appending, a gem the lock lists as direct gives redirect_gem_declaration_not_visible instead.
• Vendored: an Append plan for a gem the lock lists as direct is refused as gemfile_declaration_not_editable (refuse_append_of_direct_dependency).

Trade-off: a gemspec development dependency also appears under DEPENDENCIES, so patching one of those through an appended block is now refused rather than relying on Bundler's "gemspec dependency overridden" warning path. This fails closed, and the warning text names the gemspec case.

Test evidence

Red→green: with the fix neutered (lock_lists_direct_dependency forced to false, duplicate-declaration guard disabled), all three new unit tests fail; with the fix they pass.

Issue	Regression test(s)	Without fix	With fix
#548 (gem declared twice)	patch::redirect::tests::gemfile_gem_declared_twice_fails_closed; Bundler capstone Driver::ScanVexDuplicateDeclaration in e2e_redirect_gem_build	FAILED	ok
#548 (Bugbot follow-up: another gem call quoting the name is not a duplicate)	patch::redirect::tests::gemfile_name_quoted_by_another_gem_call_still_rewrites	FAILED	ok
#482 (eval_gemfile / loop direct dep)	patch::redirect::tests::gemfile_direct_dependency_declared_out_of_sight_is_not_appended, vendor::gem::tests::direct_dependency_declared_out_of_sight_refuses_instead_of_appending; Bundler capstones in e2e_redirect_gem_build and e2e_vendor_gem_build	FAILED	ok

Commands run locally (Ruby 3.3.6, Bundler 4.0.17):

• cargo clippy --workspace --all-features -- -D warnings: clean
• rustfmt --check on every file this PR touches: hunks clean (main has pre-existing unformatted code elsewhere, and CI does not gate on fmt)
• cargo test --workspace --all-features --no-fail-fast: 208 test binaries ok. 12 failures, all write-permission-failure tests (*_write_failure_*, *unremovable*, relax_loop_must_not_traverse_symlinked_root) that cannot fail as root in this sandbox. None touch gem code.
• cargo test -p socket-patch-cli --all-features --test e2e_redirect_gem_build -- --ignored: 10 passed
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_gem_build -- --ignored: 6 passed (re-run on e13a087: both gem build suites green again)
• e2e_gem needs the live patch API, which this sandbox cannot reach; left to CI

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lkn7GeDBpBAZrZ1sFQuc1y

---

Note

Medium Risk
Changes hosted redirect and vendored Gemfile planning for gems; behavior is fail-closed with new warnings/errors, but incorrect lock parsing or declaration counting could block legitimate patches.

Overview
Fixes #482 and #548 by making gem hosted redirect and vendored Gemfile edits fail closed instead of leaving Bundler with duplicate or conflicting declarations.

The shared lock parser now tracks every name under DEPENDENCIES (GemfileLock::direct / lock_lists_direct_dependency). Hosted redirect counts real gem declarations whose first argument is the target gem; more than one yields redirect_gem_declared_more_than_once with no Gemfile/lock writes. If the rewriter cannot see a declaration but the lock lists the gem as direct (eval_gemfile, loops, etc.), it skips with redirect_gem_declaration_not_visible instead of appending a source block. Vendored mode refuses an Append plan in that situation as gemfile_declaration_not_editable via refuse_append_of_direct_dependency.

Unit tests cover duplicate declarations, false-positive guards (require: / comments), and out-of-sight direct deps; e2e capstones exercise two-group Gemfiles, eval_gemfile, and vendor refusal on the same layout.

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

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at e13a087d9048b26a9fed6c051f2e5fa8963b50d3.

• CI: 487/487 check runs on the head are green or skipped, with 0 failing or pending.
• Bugbot: the Cursor Bugbot check passed on e13a087. Its one finding ("Loose probe blocks valid gem rewrites", raised on e13a087's parent) was confirmed and fixed in e13a087, and the thread is resolved. No review threads are open.
• Mergeable: yes. The branch is 10 commits behind main with no conflicts, and only human approval is outstanding.
• Reviewer focus: the stricter fail-closed refusals (redirect_gem_declared_more_than_once, redirect_gem_declaration_not_visible, gemfile_declaration_not_editable). They now refuse some Gemfiles that the old code would have half-edited.

Slack announcement not sent (no Slack send tool available this run).

---

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

[Mikola Lysenko (mikolalysenko)]

Reviewed e13a087d9048b26a9fed6c051f2e5fa8963b50d3 and the updated merge fdbf3af49bd853ce241df56cfce3e69943fb9911. Code review is clear. The merge resolves the test conflict with current main while retaining duplicate-declaration, eval_gemfile, and BUNDLE_GEMFILE configuration-precedence coverage. No additional correctness regression found in the declaration counting, direct-dependency lock check, or hosted/vendor refusal paths.

Validation: 261 gem unit tests passed on the original head. On the merged code, 11 real-Bundler hosted tests and the vendor eval_gemfile regression passed, using Ruby 3.4.10/Bundler 4.0.15 with missing-toolchain skips disabled. Full version/platform matrix not rerun. CI on the updated SHA is now green; ready to merge from code review.

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at fdbf3af49bd853ce241df56cfce3e69943fb9911 (re-applied after the maintainer's main merge fdbf3af).

• CI: 481/487 check runs green on this head (6 skipped by path/matrix filters), 0 failing. Mergeable; 10 commits behind main with patch/redirect/mod.rs touched on both sides — I merged current main locally: it merges cleanly, socket-patch-core builds, and the 729 redirect/gem unit tests pass (one unrelated permission test fails only because the sandbox runs as root).
• Bugbot: reviewed fdbf3af49b, no issues found; all review threads resolved.
• Reviewer focus: fail-closed trade-off — patching a gemspec development dependency via an appended block is now refused (see Root cause / Fix).

---

Generated by Claude Code