Skip to content

Fix vendored gems on CRLF Gemfile.lock (#1808) - #1811

Merged
Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-vendored-gem-crlf-lock
Oct 29, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 6 commits into
mainfrom
agent/fix-vendored-gem-crlf-lock

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1808

Summary

Vendored gem mode failed with apply_failed ("Gemfile.lock has no GEM section") on any Gemfile.lock with CRLF line endings. Git for Windows core.autocrlf and eol=crlf produce such locks, and Bundler keeps CRLF on purpose. Because the hosted→vendored eject dry run never reaches that edit, vendor --dry-run and scan --mode vendored --dry-run previewed eject_planned (exit 0) while the real run failed and rolled back (exit 1).

After this PR a CRLF lock vendors, re-runs (in-sync hot path) and --reverts like an LF one. It's edited in LF form and written back in CRLF, so the result stays byte-exact. Dry and wet runs agree again.

Root cause

vendor/gem.rs does its lock surgery (edit_lock, its section and entry helpers, and the revert splices) by splitting on \n and comparing lines exactly. On a CRLF lock every line keeps a trailing \r, so GEM and specs: never match. Hosted mode already handles CRLF through utils::line_endings. The vendored path didn't.

Fix

  • lock_lf_form (new, shared): a CRLF lock is folded to LF and its LineEndings is remembered. gem_prelude reads the lock through it, so every forward check sees LF text: the already-wired hot path, lock_checksum_in_sync and edit_lock. The edit is written back with LineEndings::restore.
  • RevertFiles reads the lock through the same helper and writes it back in its own ending. Ledger records stay bare LF lines, the same as for an LF lock, so revert is byte-exact.
  • A lock that mixes CRLF and LF has no single ending to restore. It fails closed before any other check with "mixes CRLF and LF line endings" (the same apply_failed class as the other edit_lock shape failures) and writes nothing. The hosted restore already refuses such a lock the same way.
  • docs/ecosystems.md: one line on CRLF locks in the gem vendored cell.

Tests (red → green)

Each new test fails on main with the issue's exact error and passes with the fix.

Test Covers Without fix With fix
vendor::gem::tests::crlf_lock_vendors_in_crlf_and_reverts_byte_exact (unit) direct and transitive CRLF lock: dry, wet, hot-path re-run, byte-exact revert FAIL "has no GEM section" pass
vendor::gem::tests::crlf_checksums_lock_vendors_and_reverts_byte_exact (unit) bundler ≥ 2.6 CHECKSUMS lock in CRLF FAIL pass
vendor::gem::tests::mixed_line_ending_lock_fails_closed (unit) mixed lock fails closed dry and wet, writes nothing FAIL (wrong error) pass
e2e_vendor_gem_build::gem_vendor_crlf_lock_fresh_checkout_and_revert (real bundler) issue symptom 1: plain vendor dry and wet, frozen fresh-checkout install keeps the CRLF lock byte-identical, idempotent re-run, byte-exact revert FAIL "has no GEM section" pass
e2e_redirect_gem_build::gem_hosted_pin_with_a_crlf_lock_ejects_cleanly (real bundler) issue symptom 2: vendor eject (CRLF lock only; CRLF pair) and scan --mode vendored takeover, dry and wet both exit 0, lock stays CRLF FAIL at wet (exit 1, apply_failed + rollback) pass

Per-issue checklist:

Commands run locally (Linux, ruby 3.3.6, bundler 4.0.18):

  • cargo fmt --all -- --check: ok
  • cargo clippy --workspace --all-features -- -D warnings: ok
  • cargo test -p socket-patch-cli --all-features --test e2e_vendor_gem_build --test e2e_redirect_gem_build -- --ignored: all pass on bundler 4.0.18 and 4.0.21 (4.0.21 refuses a frozen install of any CRLF lock, including a pristine one socket-patch never touched, so on that bundler the CRLF test checks a plain install keeps the vendored lock byte-identical instead)
  • cargo test --workspace --all-features --no-fail-fast: all pass except 11 tests that also can't pass in this sandbox and don't touch gem code. 9 are write-failure tests that rely on read-only modes the container's root user ignores (in_process_redirect ×3, repair ×2, core lib copy_tree/vlt_heal/pypi_poetry/pypi_requirements). 2 are e2e_bun_lockb tests that need the live patches-api.socket.dev, which the sandbox proxy blocks. CI is the authoritative run.
  • Wrappers (npm/, pypi/, gem/) only dispatch to the binary, so they need no change.

/code-review high: findings not changed here

  • A CRLF Gemfile gets its spliced line in LF. Since fixed on main by Fix vendored Gemfile line endings on CRLF (#1821) #1822, which this branch now merges. The CRLF eject e2e also asserts the Gemfile keeps CRLF.
  • A mixed lock reports apply_failed instead of a new dedicated code. That matches every other edit_lock shape failure. A new code needs CLI_CONTRACT table changes.
  • The already-wired hot path reads a mixed lock raw. That's exactly what main does, so there's no behavior change for mixed locks beyond the fail-closed edit.
  • Fixed from the review: the mixed check now runs first, a no-op normalization in the takeover preflight is gone, the forward and revert paths share one helper, and the e2e doc is accurate.

Follow-up (not in this PR)

The eject/takeover dry-run preview (takeover_dry_refusal) still ignores a failed Done from the backend. Only Refused outcomes are previewed. With CRLF fixed, no gem lock shape I could build reaches it: the hosted restore refuses mixed locks first. A generic preview of backend failures would touch every ecosystem, so it's left for a separate change.


Note

Medium Risk
Changes Gemfile.lock read/write and revert logic for all vendored gem flows; behavior is well-tested but incorrect EOL handling could corrupt locks or break Bundler installs.

Overview
Fixes #1808: vendored RubyGems mode used to fail on CRLF Gemfile.lock files (common on Git for Windows autocrlf checkouts) with apply_failed / "Gemfile.lock has no GEM section", because lock surgery split on \n and matched section headers that still had trailing \r. Hosted mode already normalized line endings; vendored mode did not, which also made dry-run eject/takeover preview exit 0 while wet runs failed and rolled back.

The vendored gem path now folds CRLF locks to LF for editing, remembers the original style via lock_lf_form, and writes back with LineEndings::restore on vendor and revert. Mixed CRLF/LF locks fail closed before any write. Docs note CRLF support in the gem vendored matrix cell.

Coverage adds unit tests (CRLF vendor/revert, CHECKSUMS locks, mixed failure) and ignored e2e tests with real Bundler for plain vendor, hosted→vendored eject, and scan --mode vendored on CRLF locks (including Gemfile CRLF per #1821).

Reviewed by Cursor Bugbot for commit 8486320. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A Gemfile.lock checked out with Windows line endings (Git for Windows
autocrlf, or eol=crlf) made vendored gem mode fail with "Gemfile.lock
has no GEM section", and the hosted-to-vendored eject dry run said the
eject would succeed while the real run failed and rolled back.

The lock is now edited and reverted in LF form and written back with
its own CRLF endings, so the vendored pair edit, the in-sync re-run and
--revert all work byte for byte. A lock that mixes CRLF and LF fails
closed with a clear message. The eject and takeover dry runs now
report a backend failure over the restored files, so they agree with
the wet run.

Fixes #1808

Assisted-by: Claude Code:claude-opus-5-5
Drop the generic eject dry-run preview change: the hosted restore
already refuses a lock that mixes CRLF and LF before the vendored edit
runs, so there was no reachable case to test it with. The CRLF eject
test shows the dry and wet runs now agree.

Note in docs/ecosystems.md that a CRLF Gemfile.lock is vendored and
reverted in its own line ending and a mixed one fails closed.

Assisted-by: Claude Code:claude-opus-5-5
A Gemfile.lock that mixes CRLF and LF now fails before any Gemfile
check, so a user fixing a Gemfile declaration is not sent round twice.
The forward edit and the revert read the lock through one helper, so
they always agree on which locks are folded to LF.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 05:25
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Bundler 4.0.21 refuses a frozen install of any CRLF Gemfile.lock, even
one socket-patch never edited, so the new CRLF vendor test failed on
that CI leg. The test now checks the pristine CRLF lock first and only
asserts a frozen install when bundler accepts the pristine one. It
still checks that bundler takes the vendored lock unchanged.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review by the burn-down agent.

  • Head: e38787013a4dcfaac43cf290634ae53a21db5221
  • CI: 50/50 check runs passing on this head (31 success, 19 skipped); no main-wide failures.
  • Bugbot: reviewed e387870, no new issues; no unresolved review threads.
  • Mergeable, no conflicts; CHANGELOG.md untouched.
  • Reviewer focus: CRLF handling in crates/socket-patch-core/src/vendor/gem.rs (mixed-ending locks are refused up front) and the frozen-leg comparison in the CRLF e2e.

Slack announcement not sent this run (Slack posting unavailable); the next run should retry.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. Vendored gem mode failed with "Gemfile.lock has no GEM section" on a CRLF Gemfile.lock (Git for Windows autocrlf), and the eject dry run previewed success while the real run failed (#1808). Now the lock is read in LF form, edited, and written back in CRLF. Revert does the same, so the lock comes back byte-identical. A lock that mixes CRLF and LF fails closed before any write.

Risk: low. An LF lock passes through lock_lf_form unchanged, so nothing changes for existing users. The ledger keeps bare LF lines, as before.

Look here

Verified. Read the full diff. Checked that LineEndings::restore is the identity for LF/None/Mixed, and that the takeover preflight's lock readers (lock_lists_direct_dependency, bundler_merges_dev_dependencies) already accept CRLF, so dry and wet runs agree. The unit tests cover CRLF direct/transitive/CHECKSUMS locks, hot-path re-runs and a byte-exact revert, and real-bundler e2e covers both issue symptoms. CI 50/50 on e387870 (31 success, 19 skipped), Cursor Bugbot green, mergeable, no open review threads, CHANGELOG.md untouched.

Changes I made. None.

Open questions. None blocking. The PR notes follow-ups: a CRLF Gemfile gets its spliced line in LF (pre-existing, #815), and the takeover dry run doesn't preview backend Done failures.

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


Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 29, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026
Resolve the gem.rs import conflict with #1822, which writes Gemfile
edits in the Gemfile's own line ending. The CRLF eject test now also
checks that a CRLF Gemfile keeps CRLF on every line.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Re-enqueued once (final reviewer) after the 13:40 merge-queue eviction: main has since been merged into the branch, head 8486320 is approved, ci-ok/clippy green, mergeable, no open threads. Approvals stay valid across later pushes per the repo owner's rule.


Generated by Claude Code

Merged via the queue into main with commit 0ba3bf1 Oct 29, 2026
50 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-vendored-gem-crlf-lock branch October 29, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants