Fix vendored gem rewrite breaking positional args (#847)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #847

Summary

In vendored mode, vendoring a gem declared with a splat, constant or method-call version (gem "rack", *V, gem "rack", VERSION, gem "rack", ENV.fetch(…)) produced a Gemfile that no bundle command could parse, while vendor exited 0 and VEX attested not_affected. The rewrite now produces a Gemfile that installs frozen and loads the vendored copy.

Root cause

plan_gemfile_edit (vendor/gem.rs) rewrites a declaration as gem name, version, path: rel, <opts>. <opts> comes from gem_line_trailing_options, which keeps everything from the first non-quoted argument on. So a positional constraint expression landed after the path: keyword, and Ruby rejects a positional argument after a keyword one.

Fix

• New split_kept_tail splits the kept tail at its first keyword argument (key:, "key":, a => pair or **splat). Commas, => and # only count outside strings and brackets, and Const::Path is not a label.
• The leading positional arguments are version constraints that the exact pin supersedes, so they're dropped, exactly like quoted constraints already are. Keyword options (require: false, group:) and a trailing comment still follow path:. Revert restores the original line verbatim from the ledger.
• The fix drops them rather than moving them in front of path:. I tried keeping them first: the real Bundler then sees rack (= 3.2.7, ~> 3.1) against the lock's rack (= 3.2.7)! and refuses every frozen install.
• Hosted mode inserts no keyword, so it's unaffected and unchanged. The npm, PyPI and gem wrappers only dispatch to the binary, so they need no change.
• Known limit: Bundler treats a trailing positional Hash (gem "x", OPTS with a hash constant) as options. The line grammar can't tell that apart from a version constant, so it's dropped like any other positional, and revert still restores it.

Also contains a cherry-pick of #851 (00d3eaa, test-only). Main is red since #605 on two vex_consumed tests, and that cherry-pick is needed for test/coverage to pass here. It becomes a no-op once #851 merges.

Test evidence

Issue	Test	Without fix	With fix
#847 *V, ENV.fetch(…), CONST, require: false, mixed =>/**/Const::Path	vendor::gem::tests::test_rewrite_drops_positional_constraints (unit)	FAILED (path: "…", *RV)	ok
#847 tail grammar	vendor::gem::tests::split_kept_tail_grammar (unit)	n/a (new helper)	ok
#847 with real Bundler 4.0.17 (Ruby 3.3.6)	e2e_vendor_gem_build::gem_vendor_drops_positional_constraints	FAILED: There was an error parsing Gemfile: syntax error, unexpected *	ok: frozen install, rack loads from .socket/vendor/gem/, revert byte-identical

Commands run locally:

• cargo test -p socket-patch-core --all-features --lib -- vendor::gem: 127 passed.
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_gem_build -- --ignored: 8/8 passed.
• cargo clippy --workspace --all-features -- -D warnings: clean.
• rustfmt --check on the changed files: clean. A workspace-wide cargo fmt --check with this sandbox's rustfmt also flags files already on main, which this PR leaves alone.
• cargo test --workspace --all-features --no-fail-fast: everything passes except tests that need a read-only directory to reject writes. Those can't fail here because the sandbox runs as root (uid 0); they cover the npm/pypi/vlt write-failure paths, which this PR doesn't touch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MxWLuzeHjHPXgKnhQWVncJ

---

Note

Medium Risk
Changes Gemfile rewrite logic for all gem vendoring; mistakes could break valid declarations or drop needed options, though coverage is strong and revert preserves originals.

Overview
Fixes #847: vendored gem Gemfile rewrites no longer append splats, constants, or method-call version arguments after the inserted path: keyword (invalid Ruby). split_kept_tail in vendor/gem.rs splits the kept tail at the first real keyword argument and drops leading positional constraints (same as quoted pins), while require:, group:, => pairs, ** splats, and trailing comments still follow path:.

Adds unit coverage (test_rewrite_drops_positional_constraints, split_kept_tail_grammar) and an ignored host e2e (gem_vendor_drops_positional_constraints) that checks frozen bundle install, vendored load path, and byte-identical revert.

Test-only: two vex_consumed hosted-npm tests are updated for post-#605 resolver behavior (alias expansion exercised with an alias-free installed map; resolver output asserted separately).

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

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

[Mikola Lysenko (mikolalysenko)]

PDM patch compatibility / native (ubuntu-latest, 2.17.3) is red on 00d3eaa. One of its 44 cells fails: space-unicode / vendored, on rescanAfterRelockApplies and rescanReusesWheel, the vendored re-scan after pdm lock. The other 43 cells pass (37 PASS, plus the expected REF/UNS cells).

I don't think this failure comes from this PR:

• The workflow runs here only because the cherry-picked Fix vex alias tests broken by store-copy merge #851 test fix touches commands/vex_consumed.rs (the commands/vex*.rs path filter). That change is in #[cfg(test)] only.
• The PR's other change is vendor/gem.rs. Neither change reaches the PDM / PyPI vendored scan path.
• main hasn't run this workflow since 792e836. Several PyPI vendoring changes have merged since (e.g. Fix vendored requirements.txt re-vendor to a superseding patch (#765) #766, Fix uv vendored revert half-reverting after a relock (#806, #821) #822, Fix PyPI vendored→hosted takeover stranding unreachable pins (#699) #708), so this cell hasn't been exercised on current main.

I couldn't reproduce it locally: the sandbox can't reach patches-api.socket.dev, so the first scan errors out before the relock step. No fix exists yet. Next I'll compare against the same cell in #712's run, which also has current main merged in, and re-run this job once when the run finishes. If the cell fails again, I'll root-cause it as a main regression in its own PR rather than widen this one.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review.

• Head: 00d3eaa5ac3c15d99ffd92f356996ef1175ab9b8
• CI: 458/458 check runs complete, all success or skipped (none failed).
• Bugbot: reviewed 00d3eaa, no findings; no unresolved review threads.
• Mergeable with no conflicts. The branch is 2 commits behind main (Write CHANGELOG only when cutting a release #848 CHANGELOG policy, Fix rollback leaving directories apply created (#838) #846 rollback dirs); neither touches these files, and main has no required up-to-date rule.
• Reviewer note: also carries the Fix vex alias tests broken by store-copy merge #851 cherry-pick (test-only). Look at split_kept_tail in vendor/gem.rs, which decides what counts as a keyword argument.

---

Generated by Claude Code