Fix vendored NuGet revert on CRLF checkouts (#537)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #537

Summary

vendor --revert, remove and rollback of a vendored NuGet package now fully revert both nuget.config and packages.lock.json on a core.autocrlf checkout. They also never leave the project half-reverted.

Root cause

1. Vendor records the LF text it wrote to nuget.config. After an autocrlf checkout the file is CRLF, and the revert's whole-file fast path (w.new == live) and its fragment excision (LF-anchored <add …/>\n and <packageSource …>\n) both missed. The config was treated as drift and left wired.
2. The revert walks the wiring in reverse order, so the lock pin had already gone back to the upstream contentHash, under a config that still maps the id exclusively to the vendored feed. Every restore then failed NU1403, while vendor --revert exited 0.

Fix

• revert_config_record compares with utils::line_endings::eol_eq. When the checkout changed the line endings, the original is restored spelled in the live file's endings (respell + terminator). The excision looks for our fragments in the file's own terminator.
• revert_nuget_opts first previews the config restore (read-only). If the config will be drift-kept and still names the vendored feed dir, the lock pin is kept too (vendor_lock_entry_drifted: "… still routes … so its packages.lock.json pin is kept"). The package stays consistently vendored, and the existing drift-keep keeps the artifact. The lock still reverts first otherwise, so a lock I/O failure still leaves the config wired for a retry (fifo_lockfile_fails_fast_in_revert unchanged).
• CLI_CONTRACT.md: the nuget clause of the vendor --revert drift rules.

Per-issue tests (vendor::nuget_feed::tests)

• revert_on_an_autocrlf_checkout_restores_config_and_lock: pre-existing config. After CRLF conversion of both files, revert restores both in CRLF with no warnings, and the feed is removed.
• revert_on_an_autocrlf_checkout_deletes_a_created_config
• revert_excises_our_crlf_fragments_beside_a_sibling
• drift_kept_config_keeps_the_lock_pin: the half-revert guard.

Red→green: before the fix, the first three fail with vendor_lock_entry_drifted / config left wired, and the last shows the lock reverted under a wired config.

Commands run

• cargo test -p socket-patch-core --lib nuget_feed (96) and --lib nuget: green (the only failures are 2 crawlers::nuget_crawler tests that pick up this machine's real ~/.nuget/packages, also failing on main locally)
• cargo test -p socket-patch-cli --all-features --test e2e_nuget (21) and --test e2e_nuget_dotnet_build -- --ignored (2, local SDK 8.0.129): green
• cargo clippy --workspace --all-features -- -D warnings: clean. cargo fmt --check: clean for touched files.

Includes #1288's commits (approved, about to merge).

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes NuGet vendor revert and nuget.config mutation paths that affect restore correctness; behavior is guarded by extensive new tests but mistakes could break revert or lock/config consistency.

Overview
NuGet vendor revert now treats nuget.config as unchanged when only line endings differ (core.autocrlf / CRLF checkouts), restoring the original config in the checkout’s EOL and matching fragment excision to the live file’s terminators instead of falsely marking drift (#537).

When revert would drift-keep a config that still routes a package id to the vendored feed, packages.lock.json pins are kept too (vendor_lock_entry_drifted), avoiding a half-revert that reverted the lock under a still-wired feed (NU1403).

Config wiring/editing drops comment-blanking and string scans in favor of the shared formats::nuget::parse_config reader (with refusal on malformed/repeated sections), plus insert_children / insert_before_close and xml_attribute for round-tripping source keys. In-sync detection uses the same parser. CLI_CONTRACT.md documents the NuGet revert drift rules for v5.0.

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

[Mikola Lysenko (mikolalysenko)]

BugBot review

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

Comment @cursor review or bugbot run to trigger another review on this PR

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

[cursor]

Revert misses mixed-EOL fragments

Medium Severity

Fragment revert now searches for terminator spellings of the source <add> and mapping block, but build_config_edit still inserts those elements with \n. An existing CRLF nuget.config therefore becomes mixed, so a later sibling vendor or edit takes the fragment path, the CRLF-majority search misses the LF fragments, and --revert drift-keeps the package.

Additional Locations (2)

• crates/socket-patch-core/src/vendor/nuget_feed.rs#L968-L972
• crates/socket-patch-core/src/vendor/nuget_feed.rs#L1232-L1237

 

^{Triggered by learned rule: CRLF-aware regexes when matching user workspace file content}

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

[Mikola Lysenko (mikolalysenko)]

[agent] Fixed in 5eebde2: the excision tries the LF spelling vendor writes first, then the file's own terminator. New test revert_excises_lf_fragments_from_a_mixed_crlf_config (CRLF config with a mapping, vendored, CRLF sibling added, revert clean with no drift warning).

[Tanmay Singla (Tanmay182003)]

[agent] The forward pass (build_config_edit) always inserts LF fragments, even into a CRLF nuget.config, but the revert now looks for them spelled in terminator(&live), which is the majority ending for a mixed file. Scenario: Windows autocrlf tree with an existing CRLF nuget.config; vendor package A, then the file changes before any git round-trip (a second vendored package or a user edit adds CRLF lines), which sends A's revert down the fragment path. The original CRLF lines outnumber our LF ones, so our <add> / <packageSource> aren't found → false drift: config stays wired, lock pin and artifact kept, vendor_lock_entry_drifted + vendor_artifact_kept. I reproduced it with an 11-line CRLF config that already has a mapping section, vendoring once, adding a CRLF sibling <add key="corp">, then reverting. On fb711df^ it reverts cleanly (byte-correct CRLF file, lock reverted, artifact removed); on fb711df it's left wired with warnings. Nothing is corrupted, but it's a regression in the setting this PR targets. Trying the LF spelling (what the forward pass writes) first and falling back to terminator(&live), in both the source_add excision here and excise_source_mapping (~L1234-1240), plus a mixed-file sibling test, would fix it.

[Mikola Lysenko (mikolalysenko)]

[agent] Fixed in 5eebde2: the excision tries the LF spelling vendor writes first, then the file's own terminator. New test revert_excises_lf_fragments_from_a_mixed_crlf_config (CRLF config with a mapping, vendored, CRLF sibling added, revert clean with no drift warning).