[Mikola Lysenko (mikolalysenko)]

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

Sweep of crates/socket-patch-cli/src/commands/scan/gc.rs by the architecture refactor routine. It works through the file's notes in coverage/files.tsv (ledger) and the structure and tests parts of the living document. The file's callers, scan/mod.rs and scan/vendor_flow.rs, are changed by #1856 and #1884, so its pub(super) API stays the same. print_gc_vendored_line is not inlined into vendor_flow.rs for the same reason.

Changes

• Wrapper deleted: count, which only called ui::plural.
• Duplication removed:
  • The pre-lock ledger check and the GC lock acquire were copied in run_apply_gc and run_vendor_only_gc. Each is now written once, as has_ledger and lock.
  • print_human_gc builds its [dry-run] prefix once instead of using an if/else println!.
• Visibility: GcSummary's pruned, blobs, diffs and packages fields and total_bytes are private now, since nothing outside the file reads them.
• Comments: about 40 lines removed, including bare #998, #1063, #1278 and B20 references, comments that restated the code, a stale "split out so it's unit-testable" note, and 6 test banners. The contract test's doc said GC: skipped (<code>): <message>., but the code prints GC: skipped: <message>., so the doc now matches the code.
• Tests (17 remain, 0 lost coverage):
  • Five detect_prunable spelling tests re-tested core PurlKey, which purl_key.rs already covers. They are merged into detect_prunable_matches_by_purl_key. It checks that the encoded, mixed-case, PEP 503, padded and short composer, and qualified spellings of installed releases are kept; that a gone composer release prunes; and that every entry prunes on an empty scan. Red→green: comparing raw strings instead of PurlKey fails it.
  • uncrawled_ecosystems and maven_and_nuget are merged into one test, and the two vendored-exemption tests into detect_prunable_exempts_vendored_purls. Red→green: dropping the purl_keys_cover conjunct fails it.
  • Three baseline tests that the remaining tests subsume were deleted: empty manifest ×2, and all-present.
  • format_gc_vendored_lines_counts is folded into gc_vendored_lines_render_the_contract_vocabulary, which now asserts the full drift-keep line and the plural dir count exactly instead of using starts_with.
  • preview_apply_gc_reports_blobs_of_prunable_entry is folded into preview_and_apply_gc_agree_on_orphan_counts. That test now asserts the preview's exact counts (1 blob, 64 bytes), so it can no longer pass when both sides report 0.
  • run_apply_gc_deletes_nothing_when_manifest_corrupt and the corrupt half of the preview test duplicated apply_gc_reports_an_unloadable_manifest. That test now also asserts that the manifest bytes are unchanged and that zero diffs, packages and bytes are freed.

Size

git diff --stat: 1 file, +173 / −442. Production is 627 → 590 lines (−37), tests 1,213 → 981 (−232).

Behavior change

None. Output bytes, JSON, exit codes and error codes are unchanged.

Verification

• cargo clippy --workspace --all-features -- -D warnings is clean.
• cargo test -p socket-patch-cli --lib passes (927).
• cli_parse_scan, cli_scan_silent, e2e_scan and e2e_composer_version_identity pass.
• /code-review high ran. Its test-coverage and comment findings are fixed. These were not fixed:
  • The pre-lock ledger gate checks --cwd and not project_root(). This predates the PR (the C43 note) and the fix changes behavior, so it is left for an issue.
  • The [dry-run] GC: reverted … wording is past tense. It predates the PR and changing it changes output bytes.
  • lock returns Box<GcSummary>. The box satisfies clippy::result_large_err and keeps the skip summary in one place.

Also noted, not changed: the preview path drops the cargo_cache_patch_kept warnings that the wet path reports (let _ = keep_patched_cargo_copies(...)).

Risk

Low. The change is local to one file and covered by its own tests. No platform-specific code changed.

A stray branch arch-refactor/sweep-scan-gc-policy (same SHA as this PR's first commit) could not be deleted from the sandbox, and can be removed.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeSPFg9U8dSnJ1cikwy

---

Note

Low Risk
Internal refactor of scan GC in one module with expanded unit tests; public pub(super) API and user-visible behavior are unchanged.

Overview
Refactors scan --prune GC in gc.rs without changing CLI output, JSON, or exit behavior.

Production: Duplicated pre-lock ledger checks and apply-lock acquisition in run_apply_gc and run_vendor_only_gc move into has_ledger and lock. The local count wrapper is removed in favor of ui::plural. GcSummary sweep counters and total_bytes are file-private. Comments are trimmed (issue refs, restatements). print_human_gc applies a single [dry-run] prefix for vendored lines.

Tests: Overlapping detect_prunable cases merge into fewer, stricter tests (PurlKey matching, uncrawled ecosystems, vendored exemptions). Redundant corrupt-manifest and preview-blob tests fold into apply_gc_reports_an_unloadable_manifest and preview_and_apply_gc_agree_on_orphan_counts. Human GC line tests gain exact drift-keep and plural assertions. Net ~270 lines removed from the file.

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

---

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 9afadf9. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 9afadf9b.

• CI: 32/32 run checks green (50 skipped), ci-ok green.
• Bugbot: reviewed 9afadf9b, no findings; no open review threads.
• Mergeable, no conflicts. Single-file refactor of scan/gc.rs; reviewers may want to check the merged detect_prunable tests keep the coverage the deleted ones had.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. A sweep of scan/gc.rs with no intended behavior change. The duplicated ledger-existence and lock-acquire blocks in run_apply_gc and run_vendor_only_gc become two helpers, has_ledger() and lock(). count() gives way to ui::plural, and GcSummary fields are narrowed to private. The 30 test functions merge into 17, mostly table-driven.

Risk: low. The non-test code is a mechanical refactor of a deleting command, and I checked the deletion paths specifically.

• Deletion: the existence check, the single lock for both halves (vendor GC first), and the prune and sweep sets are all the same.
• Dry-run: it still never takes the lock and its wording is unchanged.
• Error paths: lock failure still yields an all-default skipped summary with nothing mutated, and the manifest_invalid, manifest_unreadable, manifest_write_failed and cleanup_failed warnings are unchanged.

Look here

• gc.rs#L240-L249: existence check, then the single lock(), in run_apply_gc
• gc.rs#L289-L309: the has_ledger / lock helpers (timeout unwrap_or(0), as before)
• gc.rs#L363-L369: run_vendor_only_gc reusing them
• gc.rs#L656-L686 and #L783-L844: the merged PurlKey table test and the unloadable-manifest test (2 seeds × apply/preview), which absorb most of the removed tests

Verified

• Compared non-test semantics against main for deletion, dry-run and error paths: unchanged.
• cargo test -p socket-patch-cli --lib gc: 40 passed. cargo clippy --locked -p socket-patch-cli --all-features -- -D warnings (CI's flags): clean.
• Mapped all 30 of main's tests to head: everything is still covered except the three trivial cases listed below. CHANGELOG.md is untouched.
• CI on head: 32 passed, 50 skipped, 0 failed (ci-ok and clippy green).

Changes I made: none.

Open questions (non-blocking):

• Three trivial cases are no longer tested: detect_prunable on an empty manifest (2) and "all entries present → empty prune list". The table test always expects exactly one prune. Re-adding one empty/no-prune row would be a cheap follow-up.
• The sweep removed issue references (apply, apply --check and vendor report noManifest (exit 0) when .socket/manifest.json exists but can't be stat'd #998, scan reads a corrupt manifest as empty, so --prune skips GC and reports success #1063, Agent-mode cargo scan --sync / --prune drops the manifest entry of a crate the lock no longer resolves but leaves its shared registry-cache copy patched, so rollback can't restore it (regression from #1205) #1278) from guard comments, so the history behind those guards is now only in git blame.

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

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 9afadf9b5b36d5c0265ee1e85fb2bb3f764db403. Checked GC lock lifetime, pre-lock existence gates, prune/reference preservation and output equivalence. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.