Fix gem rollback pinning redirected transitive gems (#457)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #457

Summary

Hosted rollback / remove no longer turn a redirected transitive gem into a top-level exact pin. On a converged (CHECKSUMS) lock, the Gemfile and lock now come back byte for byte, so bundle update <gem> can move off the vulnerable version again.

Root cause

When hosted mode redirects a transitive gem, rewrite_gem (patch/redirect/mod.rs) appends a source "<patch registry>" do … end block. If the Gemfile already ends in \n, it writes no blank separator. The upstream restore (patch/redirect/upstream/gem.rs, provably_appended) only treats the block as the rewriter's append (Decl::Transitive) when a blank line comes right before it, because an in-place rewrite swallows the blank lines before the declaration. The rewriter's own append never had that blank line, so the restore fell back to Decl::Direct. The result was a new gem "<name>", "<version>" line and a <name> (= <version>) DEPENDENCIES entry.

Fix

• rewrite_gem: the transitive append always writes exactly one blank line before the block, using the Gemfile's own line ending (two line breaks if the file had no final one). The append is now provably an append.
• restore_manifest: Decl::Transitive removes that blank separator together with the block, so the restore is byte-identical.
• Docs: CLI_CONTRACT.md "Hosted unwind coverage" (gem bullet) and the gem.rs module docs.
• Limitation: a block written before this change (no blank line before it) still can't be told apart from an in-place rewrite of a last-line declaration. It still comes back as the exact pin, which is documented and covered by a test.
• Follow-up outside this repo: the depscan TS twin (registry-rewrite gem.ts) should write the same blank separator.

Tests (red → green)

Issue	Test	Without fix	With fix
#457	upstream::gem::tests::rewriter_transitive_append_restores_byte_identically: real rewriter output (LF, trailing blank, CRLF, no final newline, two appends undone in either order)	FAIL: "the rewriter's append must be provably an append"	pass
#457	upstream::gem::tests::transitive_redirect_round_trips_through_restore: full restore_one over the rewriter's CHECKSUMS lock + Gemfile (upstream sha seeded through a test-only client hook)	FAIL: restored Gemfile gained gem "rails", "7.0.0"	pass
#457	upstream_restore_golden::gem_transitive_append_round_trips_unless_unprovable: replaces gem_transitive_without_proof_stays_declared, which pinned the old behavior; the legacy no-blank shape is still asserted to stay as the exact pin	FAIL	pass

Local runs on 3aaf1dc:

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --all-features --lib upstream::gem: 10/10 pass.
• cargo test -p socket-patch-core --all-features --test upstream_restore_golden gem: 5/5 pass.
• cargo test --workspace --all-features --no-fail-fast: everything gem- and redirect-related passes. 15 tests fail only in this sandbox because it runs as root, which bypasses the read-only / chmod fixtures: *_write_failure_*, *unremovable*, *cleanup_failure*, vlt_heal an_unremovable_hidden_lock…, copy_tree relax_loop_must_not_traverse_symlinked_root, and pypi_poetry / pypi_requirements wire-failure rollback. None of them touches gem code, and CI runs them as a non-root user.
• cargo fmt --all -- --check: not run as a gate. Main itself isn't rustfmt-clean under the pinned 1.93.1 toolchain (about 125 files churn), CI has no fmt job, and this PR keeps its formatting changes to the lines it edits.

No wrapper change is needed: npm/, pypi/ and gem/ only dispatch to the binary.

Priority note: this is a p1 single-issue cluster. I picked it over the older #328 because #457 silently freezes a vulnerable version after rollback, while #328 fails closed.

---

Note

Medium Risk
Changes gem hosted rewrite and upstream-restore behavior for Gemfile.lock pairs; incorrect handling previously froze vulnerable versions after rollback, while new redirects gain a mandatory blank line before appended blocks.

Overview
Fixes #457: hosted gem rollback / remove no longer leave a redirected transitive gem as a new top-level gem "name", "version" line (and matching lock DEPENDENCIES pin) when the manifest should return unchanged.

The hosted rewriter now always inserts one blank line before an appended source "<patch>" do … end block (respecting LF vs CRLF), so upstream restore can treat it as provably appended. restore_manifest drops that separator together with the block for Decl::Transitive, yielding a byte-identical Gemfile/lock round-trip on converged CHECKSUMS locks. Legacy appends written without that blank line still cannot be distinguished from a last-line in-place rewrite and continue to restore as an exact pin—documented in CLI_CONTRACT.md and covered in tests.

Adds seed_rubygems_sha256 on the test upstream client, expands gem unit/golden tests (including full rewriter → restore_one round-trip), and renames the golden case to assert both the new round-trip and the legacy ambiguous behavior.

^{Reviewed by Cursor Bugbot for commit 3aaf1dc. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] On 3aaf1dc, native (macos-latest, 1.5.1) and native (macos-latest, 2.3.4) in "Poetry patch compatibility" failed one case: crlf hosted → rescanIdempotent. I don't think this PR causes it:

• The diff only touches the gem rewriter, the gem upstream restore, and tests and docs. No Poetry code path changes.
• The same workflow passed on 89dfc9e 15 minutes earlier. That commit has identical non-test code (3aaf1dc only changes tests/upstream_restore_golden.rs).
• It's green on main and on the other open agent PRs.
• These legs talk to the live patch service, so a rescan can see a different answer.

I've re-run the failed jobs once. If it fails again I'll treat it as real and dig into the rescan diff in the uploaded poetry-results-macos-latest-* captures.

---

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

[Mikola Lysenko (mikolalysenko)]

[agent] Ready for review on 3aaf1dce.

• CI: 97/97 check runs green (3 skipped by path filters). The earlier macOS Poetry crlf hosted → rescanIdempotent failure went green on rerun. This diff doesn't touch any Poetry code.
• Bugbot reviewed 3aaf1dce and found no issues.
• For the reviewer: the gem rewriter's transitive append/restore path and the tests/upstream_restore_golden.rs fixtures.

Slack announcement not sent this run: the Slack send tool isn't available in the agent session. The next run will retry.

---

Generated by Claude Code