Fix human scan skipping re-apply of recorded patches (#732)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #732

Summary

A human-output (no --json) socket-patch scan --mode agent or scan --sync now re-applies a patch already recorded in .socket/manifest.json after a reinstall put the pristine bytes back. It no longer prints [skip] … (already recorded), exits 0 and leaves the package vulnerable. The --json path has re-applied since #456; the two output formats now write the same thing.

Root cause

#456 fixed #454 in the shared fetch loop (get.rs: to_apply = downloaded + batch.already_recorded). But the human scan path (scan/mod.rs) partitioned manifest-recorded selections out of selected before that loop, and returned finish_human(0) when nothing new remained. So the recorded selections never reached the nested apply. When a run also had a new patch, only the new one was applied.

Change

• scan/mod.rs: on a wet agent run, the recorded selections are passed to download_and_apply_patches_with together with the new ones. The fetch loop skips their records (already in manifest) and counts them in already_recorded, and the nested apply re-applies them. That apply is idempotent, so an in-sync re-run changes nothing on disk. They are listed as [re-apply] <purl> (already recorded: <uuid>). The early "nothing selected" return now only fires when there is nothing to re-apply, and the "Patches to apply:" header is printed only when there are new patches.
• --dry-run and report-only scans still change nothing and keep the [skip] line. A dry run now says "a run without --dry-run re-applies them" instead of pointing at socket-patch apply.
• render.rs: already_recorded_line takes a reapply flag; adds an ALL_ALREADY_RECORDED_DRY_RUN message.
• Vendored and hosted modes are untouched: vendored never reads the manifest, and hosted returns before this code.
• Wrappers (npm/, pypi/, gem/) only dispatch to the binary, so no parallel change is needed.

Tests (red → green)

crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs:

• scan_human_reapplies_an_already_recorded_patch_after_reinstall: the Human-output scan --mode agent / --sync still never re-applies an already-recorded patch after a reinstall (#454 fixed only the --json path) #732 repro for both --mode agent and --sync. Scan patches the package, the bytes are restored to pristine, and the re-run must re-patch.
• scan_human_reapplies_recorded_patch_alongside_a_new_one: the mixed case from the issue. A recorded-but-reverted patch plus a newly published one; both must land.
• scan_human_dry_run_previews_reapply_of_a_recorded_patch: dry-run downloads nothing and leaves the file and manifest bytes unchanged, with the corrected message.
• This replaces scan_human_does_not_offer_an_already_recorded_patch, which pinned the buggy behaviour. Its premise ("would only be downloaded to be skipped") stopped being true once Fix agent scan/get skipping apply for recorded patches (#454) #456 landed.
• render.rs unit test covers both [skip] and [re-apply] lines.

All three new tests failed on main 045d7ec: the file was still before\n after the re-run, and the dry-run printed the old "run socket-patch apply" message. They pass with the fix.

Issue	Regression test
#732 human scan --mode agent after reinstall	scan_human_reapplies_an_already_recorded_patch_after_reinstall (agent arm)
#732 human scan --sync after reinstall	same test (--sync arm)
#732 recorded patch dropped when a new one is also selected	scan_human_reapplies_recorded_patch_alongside_a_new_one

Local verification

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features --no-fail-fast: 219 suites, 9726 passed. The 12 failures are all chmod/unwritable-path failure-injection tests (vendor state write, redirect write failure, repair lock, core copy_tree/vlt_heal/poetry/requirements wire failures) that can't fail a write when run as root in this sandbox. Re-running each one as an unprivileged user (setpriv --reuid=65534) passes. None of them touch the scan code changed here.
• e2e_scan -- --ignored needs the live patch API, which is blocked from this sandbox; CI runs it.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes when scan mutates node_modules after reinstall by routing recorded patches through download/apply; behavior is scoped to agent/sync human paths and is covered by new integration tests.

Overview
Fixes #732: human scan --mode agent / scan --sync no longer stops at [skip] … (already recorded) and leaves packages vulnerable after a reinstall restores pristine files. Wet agent runs now pass manifest-recorded selections into download_and_apply_patches_with together with new picks (matching the --json path since #454), so the nested apply re-applies them idempotently.

Human output distinguishes [re-apply] on wet runs vs [skip] on previews; dry-run messaging says a run without --dry-run re-applies, while report-only --prune --dry-run still points at socket-patch apply. Early exit and the "Patches to apply:" header only apply when there is nothing new and nothing to re-apply.

Tests replace the old "do not offer already recorded" expectation with reinstall, mixed new+recorded, dry-run, and report-only cases; render.rs covers both tag variants.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] CI status on 77ce09e, with two failures that don't come from this diff:

• PDM patch compatibility / native (ubuntu-latest, 1.15.5): one backtest cell, dev hosted, failed. On the previous head 8c9081c a different cell failed instead (2.22.4 pep582 hosted). This PR only changes the human-output agent branch of scan. Hosted scans return before it (scan/mod.rs if hosted { return boxed_run_redirect_selected(..) }), and the backtest always invokes the CLI with --json (scripts/backtest-pdm.py:745), so neither failing cell can reach the changed code. The same workflow passed on other recent PRs (Fix gem source-option guard missing git sources (#652) #731, Fix vendor --check passing unwired vendored entries (#725) #730, Fix UTF-16 requirements.txt silently skipped (#721) #724), and run 500 also needed a second attempt. I have re-run the failed jobs once. The artifact host is blocked from my sandbox, so I couldn't read the per-case logs. If the re-run fails again, I'll treat it as real.
• CI / e2e (windows-latest, e2e_redirect_vlt_build, 1.2.0, …): the runner failed before any step ran, timing out on launch.actions.githubusercontent.com:443 during "Getting action download info" (infra). I'll re-run it once the CI run finishes.

No fix to port: neither failure comes from this diff.

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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

[Mikola Lysenko (mikolalysenko)]

[agent] On e0cc8eb, PDM patch compatibility / native (macos-latest, 0.12.3) failed one backtest cell: extras vendored (rescanAfterRelockApplies,rescanReusesWheel). The other 30 cells passed. This doesn't come from this PR:

• Vendored mode never reaches the changed code. The agent-mode partition in scan/mod.rs returns early for vendor.
• The backtest runs the CLI with --json (scripts/backtest-pdm.py:745). This PR only changes the human-output path.
• e0cc8eb only removes formatting churn and doesn't touch Rust logic.

This is the third different PDM cell to fail on this PR (2.22.4 pep582 hosted, 1.15.5 dev hosted, now 0.12.3 extras vendored). Each passed on the other heads, so the backtest looks flaky in vendored and hosted mode independent of this change. No fix exists to port. I've re-run the failed job once.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Ready for review at e0cc8eb.

• CI: 476/476 completed checks green (6 skipped by path filters).
• Bugbot: reviewed e0cc8eb, no new issues. The one earlier thread (scan/mod.rs) is resolved.
• Changed this run: the fix commit had run cargo fmt across the whole workspace, which reformatted 127 files the fix doesn't touch. I confirmed those files were formatting-only (re-running cargo fmt --all on main reproduces them exactly) and restored them to main. The diff is now 3 files: scan/mod.rs, scan/render.rs and tests/covgap_commands_scan_mod.rs.
• Worth a look: human scan --mode agent / --sync now re-applies already-recorded patches ([re-apply]) instead of printing [skip] and exiting. --dry-run and report-only runs still change nothing.
• Note: cargo clippy --all-targets -D warnings fails on tests/prebuilt_common/mod.rs and tests/e2e_vendor_pypi_build.rs. That failure is already on main and not from this PR.

---

Generated by Claude Code