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

* Start fix for #732

Assisted-by: Claude Code:claude-opus-5-5

* Re-apply recorded patches in human scan output

After a reinstall put pristine bytes back, `scan --mode agent` or
`scan --sync` without `--json` printed "[skip] ... (already
recorded)" and exited 0 while leaving the package unpatched. Only the
`--json` path re-applied (#454).

The human path now hands recorded selections to the same download
step, whose nested apply re-applies them; they are listed as
`[re-apply]`. `--dry-run` and report-only scans still change nothing
and say what a wet run would do.

Fixes #732

Assisted-by: Claude Code:claude-opus-5-5

* Simplify the all-recorded early-return check

Clippy's nonminimal_bool lint rejected the negated condition.

Assisted-by: Claude Code:claude-opus-5-5

* Keep apply advice for report-only dry runs

`scan --prune --dry-run` with every selection already recorded said a
run without --dry-run would re-apply them, but a report-only scan never
applies, so following that advice changed nothing. It now points at
`socket-patch apply` again; only an agent-mode dry run promises the
re-apply.

Refs #732

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EPsXDSe1x9i96swon5nxdE

* Drop workspace-wide rustfmt churn from the #732 fix

The fix commit ran cargo fmt over the whole workspace, reformatting
127 files the change does not touch. That noise hides the real diff
from reviewers and conflicts with every other open PR editing those
files. Restore them to main; the fix itself (scan/mod.rs, render.rs
and its tests) is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: mikolalysenko

crates/socket-patch-cli/src/commands/scan/mod.rs

@@ -35,17 +35,17 @@ use crate::ui::{self, plural, print_json, StatusLine};
 
 use super::get::{download_and_apply_patches_with, DownloadParams, DownloadRun};
 
-pub use self::socket_yml_args::{SocketYmlArgs, MIN_SEVERITY_ENV};
 use self::policy::{load_invocation_policy, InvocationPolicy, PolicyLoadError, ScanPolicy};
+pub use self::socket_yml_args::{SocketYmlArgs, MIN_SEVERITY_ENV};
 
 mod discovery;
 mod gc;
 pub(crate) mod hosted;
 pub(crate) mod policy;
-mod socket_yml_args;
 pub(crate) mod render;
 pub(crate) mod rollout;
 pub mod rollout_args;
+mod socket_yml_args;
 pub(crate) mod vendor_flow;
 
 use self::discovery::{
@@ -65,13 +65,13 @@ use self::gc::gc_json;
 pub(crate) use self::hosted::boxed_run_redirect_selected;
 use self::hosted::run_redirect;
 pub(crate) use self::hosted::{vlt_rollback_heal, vlt_takeover_heal};
-pub(crate) use self::vendor_flow::{
-    boxed_vendor_step, preview_vendor_json, print_dry_run_refusals, VendorStep,
-};
 use self::vendor_flow::{
     boxed_vendor_interactive_path, boxed_vendor_json_path, fold_vendored_skips_into_apply,
     partition_skipped_selected,
 };
+pub(crate) use self::vendor_flow::{
+    boxed_vendor_step, preview_vendor_json, print_dry_run_refusals, VendorStep,
+};
 
 /// Packages per batch request on the authenticated API when `--batch-size`
 /// is not given: the server's own per-request maximum
@@ -318,11 +318,7 @@ pub struct ScanArgs {
     /// `requests`), or a purl with or without its version
     /// (`pkg:npm/lodash`, `pkg:pypi/requests@2.31.0`). Repeat the flag or
     /// separate with commas
-    #[arg(
-        long = "package",
-        env = "SOCKET_SCAN_PACKAGES",
-        value_delimiter = ','
-    )]
+    #[arg(long = "package", env = "SOCKET_SCAN_PACKAGES", value_delimiter = ',')]
     pub packages: Vec<String>,
 
     /// On a successful scan, also generate an OpenVEX 0.2.0 document.
@@ -500,9 +496,10 @@ async fn discover_selected(
     telemetry.flush().await;
     let error_count = failures.len();
     if error_count > 0 && error_count == packages.len() {
-        let err = failures
-            .last()
-            .map_or_else(|| "all patch-detail queries failed".to_string(), |(_, e)| e.clone());
+        let err = failures.last().map_or_else(
+            || "all patch-detail queries failed".to_string(),
+            |(_, e)| e.clone(),
+        );
         let message = format!("all {error_count} patch-detail queries failed: {err}");
         if detail_error_line {
             eprintln!("{}", render::fetch_details_failed(&failures));
@@ -568,7 +565,11 @@ fn classified_rows(
     packages: &[BatchPackagePatches],
     result: Option<&mut serde_json::Value>,
 ) -> Vec<rollout::Row> {
-    let failed: Vec<String> = discovered.failed.iter().map(|(purl, _)| purl.clone()).collect();
+    let failed: Vec<String> = discovered
+        .failed
+        .iter()
+        .map(|(purl, _)| purl.clone())
+        .collect();
     stage.incomplete = rollout::lookup_incomplete(&recorded.index, &failed, batch_failed);
     let rows = rollout::classify(&discovered.offers, &recorded.index, &stage.project);
     if let Some(result) = result {
@@ -1317,7 +1318,8 @@ fn project_dirs(cwd: &Path, paths: &[String]) -> Result<Vec<(PathBuf, bool)>, St
         let joined = cwd.join(raw);
         if raw.contains(['*', '?', '[']) {
             let pattern = joined.to_string_lossy().into_owned();
-            let matches = glob::glob(&pattern).map_err(|e| format!("invalid path pattern `{raw}`: {e}"))?;
+            let matches =
+                glob::glob(&pattern).map_err(|e| format!("invalid path pattern `{raw}`: {e}"))?;
             let before = dirs.len();
             dirs.extend(
                 matches
@@ -1390,7 +1392,10 @@ async fn run_project_dirs(
     }
     // One budget per invocation (§5.2): the directories spend it in sorted
     // order, and a package admitted in one is admitted free in the next.
-    let configured = match args.rollout.resolve_from_env(invocation.policy.max_new_patches()) {
+    let configured = match args
+        .rollout
+        .resolve_from_env(invocation.policy.max_new_patches())
+    {
         Ok(max) => max,
         Err(message) => {
             eprintln!("Error: {message}");
@@ -1491,19 +1496,19 @@ async fn run_scan(
     // error.
     let configured_cap = match args.rollout.carry.as_ref() {
         Some(carry) => carry.lock().configured,
-        None => match args.rollout.resolve_from_env(invocation.policy.max_new_patches()) {
+        None => match args
+            .rollout
+            .resolve_from_env(invocation.policy.max_new_patches())
+        {
             Ok(max) => max,
             Err(message) => {
                 eprintln!("Error: {message}");
                 return 2;
             }
         },
     };
-    let mut stage = rollout::Stage::new(
-        configured_cap,
-        args.rollout.carry.clone(),
-        &args.common.cwd,
-    );
+    let mut stage =
+        rollout::Stage::new(configured_cap, args.rollout.carry.clone(), &args.common.cwd);
 
     // Strict airgap (CLI_CONTRACT.md `--offline`): scan's patch discovery
     // is remote data, so refuse before the crawl and before the API client
@@ -1704,8 +1709,11 @@ async fn run_scan(
         .filter(|pkg| args.common.purl_ecosystem_selected(&pkg.purl))
         .collect();
 
-    let package_specs: Vec<&String> =
-        args.packages.iter().filter(|s| !s.trim().is_empty()).collect();
+    let package_specs: Vec<&String> = args
+        .packages
+        .iter()
+        .filter(|s| !s.trim().is_empty())
+        .collect();
     let filtered_crawled: Vec<_> = if package_specs.is_empty() {
         filtered_crawled
     } else {
@@ -1860,13 +1868,12 @@ async fn run_scan(
                 // `redirectState` rides the empty-discovery envelope too
                 // (same rule as the ≥1-package path). `wiringLive` is empty
                 // by construction: this run covered zero packages.
-                let redirect_state = (!args.common.is_global()).then_some(
-                    crate::commands::hosted_state_from_pins(
+                let redirect_state =
+                    (!args.common.is_global()).then_some(crate::commands::hosted_state_from_pins(
                         &socket_patch_core::patch::redirect::upstream::HostedPin::all(
                             ctx.discovery().await,
                         ),
-                    ),
-                );
+                    ));
                 if let Some(state) = redirect_state_json(redirect_state.as_ref(), &[]) {
                     result["redirectState"] = state;
                 }
@@ -2222,7 +2229,8 @@ async fn run_scan(
         // A report-only run selects nothing, but a severity floor or
         // `enabled: false` still hides candidates; report them like the
         // human arm does (the detail fetch runs only then).
-        if !apply && !vendor && policy.reports_selection() && !all_packages_with_patches.is_empty() {
+        if !apply && !vendor && policy.reports_selection() && !all_packages_with_patches.is_empty()
+        {
             if let Err((code, message)) = discover_selected(
                 &api_client,
                 &all_packages_with_patches,
@@ -2515,12 +2523,7 @@ async fn run_scan(
                     &all_packages_with_patches,
                     None,
                 );
-                updates = offer_updates(
-                    &rows,
-                    &discovered,
-                    &recorded,
-                    &all_packages_with_patches,
-                );
+                updates = offer_updates(&rows, &discovered, &recorded, &all_packages_with_patches);
                 rows
             }
             // `discover_selected` already printed the failure to stderr.
@@ -2730,8 +2733,11 @@ async fn run_scan(
         plan_kept_rows(&mut stage, rows, selected)
     };
 
-    // Drop selections the manifest already records at the same uuid.
-    // Agent mode only: vendored mode never reads the manifest.
+    // Set aside selections the manifest already records at the same uuid.
+    // A wet agent run still hands them to the download step below, whose
+    // nested apply re-applies them after a reinstall (#454, #732); a
+    // preview only names them. Agent mode only: vendored mode never reads
+    // the manifest.
     let recorded = |p: &PatchSearchResult| {
         existing_manifest
             .as_ref()
@@ -2743,17 +2749,18 @@ async fn run_scan(
     } else {
         selected.into_iter().partition(|p| recorded(p))
     };
+    let reapply = !report_only && !args.common.dry_run;
     if !silent {
         for p in &already_recorded {
             open_paragraph(&mut skip_paragraph);
             println!(
                 "{}",
-                render::already_recorded_line(&normalize_purl(&p.purl), &p.uuid)
+                render::already_recorded_line(&normalize_purl(&p.purl), &p.uuid, reapply)
             );
         }
     }
 
-    if selected.is_empty() {
+    if selected.is_empty() && (!reapply || already_recorded.is_empty()) {
         if !silent {
             open_paragraph(&mut skip_paragraph);
             if !stage.deferred_keys().is_empty() {
@@ -2764,6 +2771,8 @@ async fn run_scan(
                 }
             } else if already_recorded.is_empty() {
                 println!("No patches selected.");
+            } else if args.common.dry_run && !report_only {
+                println!("{}", render::ALL_ALREADY_RECORDED_DRY_RUN);
             } else {
                 println!("{}", render::ALL_ALREADY_RECORDED);
             }
@@ -2773,7 +2782,7 @@ async fn run_scan(
     }
 
     // Display detailed summary of selected patches (skipped under --silent).
-    if !silent {
+    if !silent && !selected.is_empty() {
         if vendor {
             println!("\nPatches to vendor:\n");
         } else {
@@ -2927,9 +2936,16 @@ async fn run_scan(
         )
         .await
     } else {
-        let (code, _) =
-            download_and_apply_patches_with(&selected, &params, &download_run(&args, &api_client))
-                .await;
+        // The recorded selections ride along: the fetch loop skips their
+        // record and the nested apply re-applies them (a no-op on disk
+        // when the installed copy is still patched).
+        let to_download: Vec<_> = selected.iter().chain(&already_recorded).cloned().collect();
+        let (code, _) = download_and_apply_patches_with(
+            &to_download,
+            &params,
+            &download_run(&args, &api_client),
+        )
+        .await;
         code
     };
 
@@ -2982,14 +2998,20 @@ mod tests {
             dirs.iter()
                 .map(|(d, explicit)| {
                     (
-                        d.strip_prefix(tmp.path()).unwrap().to_string_lossy().replace('\\', "/"),
+                        d.strip_prefix(tmp.path())
+                            .unwrap()
+                            .to_string_lossy()
+                            .replace('\\', "/"),
                         *explicit,
                     )
                 })
                 .collect()
         };
-        let got = project_dirs(tmp.path(), &["apps/*".into(), "libs/core".into(), "apps/web".into()])
-            .unwrap();
+        let got = project_dirs(
+            tmp.path(),
+            &["apps/*".into(), "libs/core".into(), "apps/web".into()],
+        )
+        .unwrap();
         // Named literally = explicit (also when a glob matches it too).
         assert_eq!(
             rel(got),

crates/socket-patch-cli/src/commands/scan/render.rs

@@ -300,18 +300,27 @@ pub(super) fn not_installed_skip_line(purl: &str) -> String {
     )
 }
 
-/// `[skip]` line for a selection the manifest already records.
-pub(super) fn already_recorded_line(purl: &str, uuid: &str) -> String {
+/// Line for a selection the manifest already records: `[re-apply]` on a
+/// wet agent run (the nested apply re-applies it), `[skip]` in a preview.
+pub(super) fn already_recorded_line(purl: &str, uuid: &str, reapply: bool) -> String {
+    let tag = if reapply { "re-apply" } else { "skip" };
     format!(
-        "  [skip] {purl} (already recorded: {})",
+        "  [{tag}] {purl} (already recorded: {})",
         super::super::get::short_uuid(uuid)
     )
 }
 
-/// Printed when every selection is already recorded (agent mode).
+/// Printed when every selection is already recorded and nothing is applied
+/// (report-only scan).
 pub(super) const ALL_ALREADY_RECORDED: &str =
     "All selected patches are already recorded in the manifest; run `socket-patch apply` to re-apply them.";
 
+/// [`ALL_ALREADY_RECORDED`] for an agent-mode `--dry-run`. A report-only
+/// dry run keeps [`ALL_ALREADY_RECORDED`]: dropping `--dry-run` there still
+/// only reports.
+pub(super) const ALL_ALREADY_RECORDED_DRY_RUN: &str =
+    "All selected patches are already recorded in the manifest; a run without --dry-run re-applies them.";
+
 /// The terminal error when no package's patch details could be fetched.
 pub(super) fn fetch_details_failed(failed: &[(String, String)]) -> String {
     match failed {
@@ -746,7 +755,10 @@ mod tests {
 
     #[test]
     fn report_only_hint_names_agent_mode() {
-        assert_eq!(report_only_hint()[0], "To apply these patches in place, run:");
+        assert_eq!(
+            report_only_hint()[0],
+            "To apply these patches in place, run:"
+        );
         assert!(report_only_hint()[1].contains("--mode agent"));
     }
 
@@ -760,9 +772,13 @@ mod tests {
         assert!(l.contains("`socket-patch scan --mode vendored`"), "{l}");
         assert!(!l.contains("--vendor`"), "{l}");
         assert_eq!(
-            already_recorded_line("pkg:npm/x@1", "884e9f6d-aaaa"),
+            already_recorded_line("pkg:npm/x@1", "884e9f6d-aaaa", false),
             "  [skip] pkg:npm/x@1 (already recorded: 884e9f6d)"
         );
+        assert_eq!(
+            already_recorded_line("pkg:npm/x@1", "884e9f6d-aaaa", true),
+            "  [re-apply] pkg:npm/x@1 (already recorded: 884e9f6d)"
+        );
     }
 
     #[test]