[Mikola Lysenko (mikolalysenko)]

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

Fixes #1821

Summary

Vendoring a gem over a CRLF Gemfile wrote the rewired gem line (or the appended managed block) with bare LF endings, so the committed Gemfile ended up with mixed line endings. Now every line the vendored gem backend writes into the Gemfile, and every line it restores on revert, uses the file's own line endings.

Root cause

The Gemfile writer in crates/socket-patch-core/src/vendor/gem.rs always built lines with LF, whatever the file used:

• plan_gemfile_edit's Rewrite arm built new_line from scratch, so the \r of the line it replaced was lost. It survived only when a trailing option or comment happened to carry it.
• The Append arm's managed block was LF-only, and apply_gemfile_plan added an LF before it.
• RewireOurs (a re-vendor after a patch update) only recognised the managed fence when the fence lines had no \r. In a CRLF file it recorded the block as a plain Rewritten line.
• On revert, restore_gemfile_line restored the recorded original with the ending it had at vendor time. An Added block was only found byte for byte, so a Gemfile re-checked-out in the other style (core.autocrlf) reverted to mixed endings or was reported as drift. This is the related shape described at the end of Vendored gem mode writes the rewired gem line of a CRLF Gemfile with an LF ending, leaving the Gemfile with mixed line endings #1821.

Changes

• Rewrite: the CR is stripped from the declaration's tail before parsing and put back at the end of the new line.
• Append: the managed block, and the separator before it, use line_endings::terminator of the file.
• RewireOurs: the fence matches with or without CR, and the recorded block is the fence lines as they stand in the file.
• Revert: a restored line keeps the original's ending unless the wired line's ending changed since vendor wrote it, in which case it follows the file. This keeps ledgers written by older releases (LF wired line, CRLF original) reverting byte for byte. An Added block is looked up as written, then in the file's current style. A hand-restored original counts as converged whatever its ending.
• The existing test revert_commented_gemfile_line_keeps_crlf asserted the old LF wired line (its comment called it "a separate line-ending gap"). It now expects the CRLF line.

The npm, pypi and gem wrappers don't touch this code path, so they need no changes.

Tests (red → green)

Issue / shape	Test	On main	On this branch
#1821 rewired line in a CRLF Gemfile	vendor_keeps_a_crlf_gemfile_crlf (direct case)	FAIL	pass
#1821 Append managed block (transitive gem)	vendor_keeps_a_crlf_gemfile_crlf (transitive case)	FAIL	pass
#1821 CR position with trailing options/comment	plan_gemfile_edit_keeps_the_lines_crlf	FAIL	pass
#1821 re-vendor over a CRLF managed fence	revendor_crlf_managed_block_keeps_the_fence	FAIL	pass
#1821 revert after an LF↔CRLF re-checkout (direct and transitive)	revert_follows_a_recheckout_line_ending_change	FAIL	pass
Old-release ledger restores byte for byte	restore_gemfile_line_ending	n/a	pass
#1821 end to end with real bundler 4.0.18: vendor, frozen install, revert	e2e_vendor_gem_build::gem_vendor_keeps_a_crlf_gemfile_crlf	FAIL (...path: ".socket/vendor/gem/.../rack-3.2.7"\n)	pass

Commands run locally:

• cargo fmt --all -- --check: ok
• cargo clippy --workspace --all-features -- -D warnings: ok
• cargo test -p socket-patch-core --all-features --lib -- vendor::gem: 153 passed
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_gem_build --test e2e_redirect_gem_build -- --ignored: 36 + 10 passed
• cargo test --workspace --all-features --no-fail-fast: everything passed except 11 tests that can't pass in this sandbox and are unrelated to this diff:
  • 9 simulate unwritable or unremovable files with permission bits, which root (the sandbox user) bypasses: copy_tree, vlt_heal, pypi_poetry, pypi_requirements, repair and redirect write-failure tests.
  • 2 e2e_bun_lockb tests need patches-api.socket.dev, which the sandbox proxy blocks.

CI runs all of them.

/code-review high

The fixes for findings 1 (old-ledger CR on revert), 5 (convergence check ignored a CR change), 7 (double scan in the Added revert) and 8 (a stricter CRLF check in the tests) are in 6cb35bc. Not fixed, and why:

• RewireOurs keeps an LF wired line that an older release wrote. It only affects ledgers from before this fix, and revert now restores those files byte for byte.
• A managed fence as the last line with no final newline, and the separator added before an Append to a file with no final newline. Both behave the same on main and don't depend on line endings.
• Consolidating on line_endings::split_lines. That's the broader refactor tracked in Tracking: classify line endings in one place instead of five drifting rules #814 / Pick the line terminator for spliced lines through one line_endings::terminator #815. This PR keeps the fix local.

---

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

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review at 6cb35bc77efb64e5b7f7de8f34cdb5ea481104a0.

• CI: 31/31 non-skipped checks green on this head (50 skipped, Cursor Bugbot neutral), PR is mergeable.
• Bugbot: reviewed this head, no findings; no review threads.
• Reviewer focus: the revert rule in restore_gemfile_line (keeps the original's ending unless the wired line's ending changed since vendor, so older ledgers still revert byte for byte), and the updated revert_commented_gemfile_line_keeps_crlf expectation.
• Slack announcement could not be sent this run (the Slack send tool is not available); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Vendoring a gem into a CRLF Gemfile used to write the rewired gem line (and the appended managed block) with bare LF, leaving the file mixed. This makes every write follow the file's line endings, and makes revert find and restore its lines whichever ending the file has now (including after a core.autocrlf re-checkout).

Risk: low. Confined to the Gemfile writer/reverter in vendor/gem.rs; LF Gemfiles take the same path as before (terminator is \n). Old ledgers that recorded a bare-LF wired line beside a CRLF original still restore CRLF (covered by restore_gemfile_line_ending).

Look here

• gem.rs:1290: appended managed block uses the file's terminator
• gem.rs:1305: CR peeled off the declaration and put back at the end of the rewritten line
• gem.rs:1332: re-vendor matches the fence lines with either ending and records them as they stand
• gem.rs:2342-2363: revert tolerates a changed line ending (Rewritten and Added)
• gem.rs:2434: restore_gemfile_line picks the restored line's ending

Verified. Read the full diff and the revert paths around it (find_wired_gemfile_line fallback strips the CR via trim_end, so a re-checked-out LF file still matches). Tests would fail without the change (all_crlf / exact-block asserts); the bundler capstone gem_vendor_keeps_a_crlf_gemfile_crlf runs in the gem e2e job, which passed. CI: 31 passed, 50 skipped, 0 failed on 6cb35bc (ci-ok, clippy green). Bugbot: its run on this head was cancelled (neutral), no findings. CHANGELOG.md untouched. Local test run was not possible this time (build OOM in the runner); relied on CI.

Changes I made. None.

Open questions. None. Pre-existing, not changed here: a managed block whose closing fence is the file's last line with no trailing newline is recorded with a newline it doesn't have.

Auto-merge is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code