GeezerVison

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

Fixes #988

Summary

Vendored gem vendor --revert (and remove / rollback) no longer
half-reverts when the user has commented the wired Gemfile line. Before,
the lock went back to rubygems.org while the Gemfile kept its
path: ".socket/vendor/gem/<uuid>/…" line, a pair bundle install --frozen refuses, and every later revert drift-kept again.

Root cause

Vendor's in-sync check treats the Gemfile as wired when it contains the
copy path, but revert_gemfile_record only accepted the recorded line
byte for byte. Records were also reverted and written one at a time
(lock first), so a drifted Gemfile record did not stop the lock from
being restored.

Fix (crates/socket-patch-core/src/vendor/gem.rs)

• Find the wired line by its code, not its comment. A line whose
  code equals what vendor wrote matches whatever its trailing comment
  says. The original line is put back, carrying the user's comment when
  they added or changed one. CRLF endings follow the original line.
  Lines whose code changed (for example , require: false added) are
  not rewritten, so a user edit is never silently dropped.
• All-or-nothing across Gemfile and Gemfile.lock. Records are planned
  against in-memory text (RevertFiles). When any known record drifts,
  nothing is written and the artifact is kept. If the Gemfile write fails
  after the lock was written, the lock's original bytes are put back.
• Convergence follows vendor's predicate. When the wired line is gone,
  a Gemfile that still contains the copy path, or that points the gem at
  another path:/source, counts as drift. A Gemfile that declares the
  gem from the registry again (a hand restore, even with a different
  constraint) has converged, so the lock follows it back. An edited
  managed block that still names the copy is now drift instead of "already
  removed". A lock with no CHECKSUMS section at all now counts as
  converged for the checksum record. Without that, the all-or-nothing rule
  would wedge that case forever.
• The drift warning now says that neither file was changed.
• formats::gem::gemfile::code_of is now pub(crate) (a quote-aware
  comment splitter).

Existing tests whose expectations changed

They pinned the old per-record partial revert, which is the behavior
#988 reports:

• test_checksums_revert_drift_warning and
  revert_missing_dep_pin_leaves_both_files_alone (renamed): a drifted
  lock record now leaves both files exactly as found.
• revert_checksums_section_gone_is_converged (renamed): no CHECKSUMS
  section counts as converged, so the artifact is removed.
• Two unit tests now build a RevertFiles instead of passing a path.

Test evidence

New tests failed on main before the fix (red run:
cargo test -p socket-patch-core --lib vendor::gem::tests::revert_ →
4 failed: revert_commented_gemfile_line_restores_both_files,
revert_edited_carried_comment_keeps_the_users_comment,
revert_commented_gemfile_line_keeps_crlf,
revert_drifted_gemfile_line_writes_neither_file). All pass after it.

Per-issue checklist (#988 acceptance criteria):

• Vendor, add a comment to the wired line, revert → both files back in
  registry form, comment kept, artifact removed:
  revert_commented_gemfile_line_restores_both_files (plus
  …edited_carried_comment… and …keeps_crlf).
• Truly drifted Gemfile line → Gemfile and lock unchanged, artifact
  kept, and the same on a second revert:
  revert_drifted_gemfile_line_writes_neither_file,
  revert_still_wired_but_edited_gemfile_writes_neither_file.
• test_revert_round_trip_*,
  test_revert_converged_files_are_silent_and_still_remove and the
  legacy-ledger fixtures stay green.

Commands run locally:

• cargo fmt --all -- --check: ok
• cargo clippy --workspace --all-features -- -D warnings: ok
• cargo test -p socket-patch-core --all-features: 6324 passed. The 4
  failures are chmod-based write-failure tests in copy_tree, vlt_heal,
  pypi_poetry and pypi_requirements, which can't trigger as root in this
  sandbox. None is in gem code, and CI runs them as non-root.
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_gem_build --test e2e_redirect_gem_build --test in_process_vendor -- --include-ignored
  (ruby 3.3.6, bundler 4.0.18): 41 + 49 + 132 passed.
• The full CLI workspace build ran out of sandbox disk, so CI covers the
  remaining suites.

/code-review high: findings not fixed

• An unknown wiring kind still lets the known records restore. This keeps
  the forward compatibility that
  revert_unrecognized_wiring_kind_warns_and_continues pins, and the
  artifact is still kept.
• A wired line moved into a group block or split across lines is drift
  (nothing written) rather than rewritten. Revert won't rewrite indented
  or multi-line declarations, the same rule the forward path refuses on.

Follow-ups (not in this PR)

• The forward vendor rewrite writes the wired line with a bare LF in a
  CRLF Gemfile (seen while writing the CRLF test). This is part of the
  line-ending tracking in Tracking: classify line endings in one place instead of five drifting rules #814.
• requirements.txt has the same forward/revert recognizer split (Vendored requirements.txt can't be reverted once the user touches the # socket-patch vendor: comment: vendor --revert drift-keeps forever (exit 0), and the suggested "re-vendor" remedy is a no-op or adds a duplicate line #977).

🤖 Generated with Claude Code

https://claude.ai/code/session_01QFoDP1VMMeczUdpPaToVKx

Generated by Claude Code