Fix scan human/JSON exit and prune forks (#1062)

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

@@ -554,6 +554,9 @@ pub(super) async fn run_redirect(
     stage: &mut super::rollout::Stage,
     // Scan's pre-redirect lockfile discovery (see `rollout::Gate::prior`).
     prior: Option<super::rollout::Prior<'_>>,
+    // `--prune` / `--sync`, gated by the policy exactly as the human arm
+    // gates it (`patches.enabled: false` writes nothing, the GC included).
+    prune: bool,
 ) -> i32 {
     // Same discovery/selection as agent and vendored mode.
     let discovered = match discover_selected(
@@ -606,7 +609,7 @@ pub(super) async fn run_redirect(
     run_redirect_selected(
         &args.common,
         &args.vex,
-        args.prune || args.sync,
+        prune,
         api_client,
         &pairs,
         scan_result,

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

@@ -489,7 +489,6 @@ async fn discover_selected(
     // Some queries failed, some succeeded: a `--json` run has no stderr
     // warning (`warn` is human-only), so each failed package becomes a
     // run-level `warnings[]` entry — never a silent drop from the envelope.
-    let fetched = all_search_results.len();
     let offers = select_accessible(all_search_results, can_access_paid_patches, policy);
     if let Some(result) = json_warnings {
         for (purl, e) in &failures {
@@ -503,18 +502,15 @@ async fn discover_selected(
     }
     Ok(Discovered {
         offers,
-        fetched,
         failed: failures,
     })
 }
 
-/// [`discover_selected`]'s result: the offers, how many records came back
-/// (before the tier filter), and each failed detail query as `(purl,
-/// error)` (a failure for a package with no recorded patch makes a capped
-/// run's data incomplete).
+/// [`discover_selected`]'s result: the offers and each failed detail
+/// query as `(purl, error)` (a failure for a package with no recorded
+/// patch makes a capped run's data incomplete).
 struct Discovered {
     offers: rollout::Offers,
-    fetched: usize,
     failed: Vec<(String, String)>,
 }
 
@@ -672,8 +668,9 @@ fn open_paragraph(opened: &mut bool) {
 /// arm treats an empty merged set as a fetch failure). The two output
 /// knobs are human-only: `show_progress` shows the status-line counter on
 /// stderr, `warn` prints a warning per failed package once the loop is
-/// done — only when some query succeeded (when every one failed, the
-/// caller's error line carries the cause instead, so nothing repeats).
+/// done — only when some query succeeded, even with no records (when
+/// every one failed, the caller's error line carries the cause instead,
+/// so nothing repeats).
 async fn fetch_patch_details(
     api_client: &socket_patch_core::api::client::ApiClient,
     packages: &[BatchPackagePatches],
@@ -715,7 +712,8 @@ async fn fetch_patch_details(
         }
     }
     status.finish();
-    if warn && !results.is_empty() {
+    // Not when every query failed: the caller's error line names it.
+    if warn && failures.len() < packages.len() {
         for (purl, e) in &failures {
             eprintln!("Warning: could not fetch details for {purl}: {e}");
         }
@@ -2544,6 +2542,7 @@ async fn run_scan(
                 batch_error_count > 0,
                 &mut stage,
                 prior_discovery,
+                prune,
             )
             .await;
         }
@@ -2843,10 +2842,11 @@ async fn run_scan(
 
     // The by-package records every arm selects from, fetched before the
     // table so its `[UPDATE]` markers are the same UPGRADE rows the
-    // selection acts on (§5.1). Discovery said these packages HAVE
-    // patches, so an empty merged set is a fetch failure.
-    // A failed discovery still prints the table first; its exit code is
-    // returned below it.
+    // selection acts on (§5.1). Only `discover_selected`'s own `Err`
+    // (every query failed) is a fetch failure: queries that succeed with
+    // no records leave nothing to select, in every arm and in `--json`
+    // alike (#1062). A failed discovery still prints the table first; its
+    // exit code is returned below it.
     let mut discovery_failure: Option<i32> = None;
     let rows: Vec<rollout::Row> = if downloadable_count == 0 {
         Vec::new()
@@ -2864,13 +2864,6 @@ async fn run_scan(
         )
         .await
         {
-            // The agent / vendored / report-only arms need records to show:
-            // an empty merged set is a fetch failure there.
-            Ok(discovered) if !hosted && discovered.fetched == 0 => {
-                eprintln!("{}", render::fetch_details_failed(&discovered.failed));
-                discovery_failure = Some(1);
-                Vec::new()
-            }
             Ok(discovered) => {
                 let rows = classified_rows(
                     &mut stage,

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

@@ -2774,3 +2774,199 @@ async fn scan_agent_json_mismatch_overwrite_reaches_the_apply_block() {
         b"after\n"
     );
 }
+
+// ---------------------------------------------------------------------------
+// #1062: human / JSON parity when every detail query succeeds but is empty
+// ---------------------------------------------------------------------------
+
+/// Mount a by-package response that succeeds with NO patch records (the
+/// batch said the package has one; the detail query found none to offer).
+async fn mount_by_package_empty(mock: &MockServer, purl: &str) {
+    Mock::given(method("GET"))
+        .and(path(format!(
+            "/v0/orgs/{ORG_SLUG}/patches/by-package/{}",
+            encode_purl(purl)
+        )))
+        .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
+            "patches": [],
+            "canAccessPaidPatches": false,
+        })))
+        .mount(mock)
+        .await;
+}
+
+/// #1062: when every detail query succeeds but returns no records, scan
+/// has nothing to select. That is a successful run with no applicable
+/// patches in every mode, so the human arm must exit like the `--json`
+/// arm (0), not report a fetch failure and exit 1.
+#[tokio::test]
+async fn scan_empty_detail_results_exit_alike_in_human_and_json() {
+    let purl = "pkg:npm/[email protected]";
+    // `--prune` alone is report-only (no mode); `--dry-run` keeps every
+    // mode read-only.
+    let modes: [&[&str]; 4] = [
+        &["--mode", "agent"],
+        &["--mode", "vendored"],
+        &["--mode", "hosted"],
+        &["--prune"],
+    ];
+    for mode in modes {
+        for dry in [false, true] {
+            let mock = MockServer::start().await;
+            mount_batch_one(&mock, purl, UUID, "free", &[], false).await;
+            mount_by_package_empty(&mock, purl).await;
+
+            let mut codes = Vec::new();
+            for json in [false, true] {
+                let tmp = tempfile::tempdir().unwrap();
+                write_root_package_json(tmp.path());
+                write_npm_package(tmp.path(), "minimist", "1.2.2", b"x\n");
+                let mut extra: Vec<&str> = mode.to_vec();
+                if dry {
+                    extra.push("--dry-run");
+                }
+                if json {
+                    extra.push("--json");
+                }
+                let (code, stdout, stderr) = run_scan_human(tmp.path(), &mock.uri(), &extra);
+                assert!(
+                    !stderr.contains("could not fetch patch details"),
+                    "{extra:?}: no query failed, so no fetch failure; stderr={stderr}"
+                );
+                if json {
+                    let v: serde_json::Value =
+                        serde_json::from_str(stdout.trim()).expect("valid JSON");
+                    assert_ne!(v["status"], "error", "{extra:?}: {v}");
+                }
+                codes.push((code, stdout, stderr));
+            }
+            let (human, json) = (&codes[0], &codes[1]);
+            assert_eq!(
+                human.0, json.0,
+                "{mode:?} dry={dry}: human and --json exit alike\n\
+                 human stdout={}\nhuman stderr={}\njson stdout={}\njson stderr={}",
+                human.1, human.2, json.1, json.2
+            );
+            assert_eq!(human.0, 0, "{mode:?} dry={dry}: nothing to do is a success");
+        }
+    }
+}
+
+/// #1062: a human `--dry-run --prune` previews the GC (once) where the
+/// `--json` arm previews it (`gc` block).
+#[tokio::test]
+async fn scan_dry_run_prune_previews_gc_in_human_and_json() {
+    let purl = "pkg:npm/[email protected]";
+    let stale = "pkg:npm/[email protected]";
+    // Agent and report-only (no mode; `--prune` below). Vendored mode's
+    // human GC on early exits is #1127.
+    for mode in [&["--mode", "agent"][..], &[][..]] {
+        let mock = MockServer::start().await;
+        mount_batch_one(&mock, purl, UUID, "free", &[], false).await;
+        mount_by_package(&mock, purl, UUID, serde_json::json!({})).await;
+        for json in [false, true] {
+            let tmp = tempfile::tempdir().unwrap();
+            write_root_package_json(tmp.path());
+            write_npm_package(tmp.path(), "minimist", "1.2.2", b"x\n");
+            // A recorded patch for a package that is not installed: the
+            // GC would prune its entry.
+            seed_manifest(tmp.path(), &[(stale, OLD_UUID)]);
+            let manifest = tmp.path().join(".socket/manifest.json");
+            let before = std::fs::read(&manifest).unwrap();
+            let mut extra: Vec<&str> = mode.to_vec();
+            extra.extend(["--prune", "--dry-run", "--yes"]);
+            if json {
+                extra.push("--json");
+            }
+            let (code, stdout, stderr) = run_scan_human(tmp.path(), &mock.uri(), &extra);
+            assert_eq!(code, 0, "{extra:?}: stdout={stdout}; stderr={stderr}");
+            if json {
+                let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON");
+                assert!(
+                    v["gc"]["prunableManifestEntries"]
+                        .as_array()
+                        .is_some_and(|a| a.iter().any(|p| p == stale)),
+                    "{extra:?}: {v}"
+                );
+            } else {
+                assert_eq!(
+                    stdout
+                        .matches("[dry-run] GC would prune 1 manifest entry")
+                        .count(),
+                    1,
+                    "{extra:?}: the human dry run previews the GC once; stdout={stdout}"
+                );
+            }
+            assert_eq!(
+                std::fs::read(&manifest).unwrap(),
+                before,
+                "{extra:?}: a dry run writes nothing"
+            );
+        }
+    }
+}
+
+/// #1062: when some detail queries fail and the rest succeed with no
+/// records, the human run still warns per failed package (the `--json`
+/// arm adds a `patch_details_failed` warning each) and exits like it.
+#[tokio::test]
+async fn scan_partial_failure_with_empty_results_still_warns() {
+    let empty = "pkg:npm/[email protected]";
+    let bad = "pkg:npm/[email protected]";
+    let mock = MockServer::start().await;
+    let patch = |purl: &str| {
+        serde_json::json!({
+            "uuid": UUID, "purl": purl, "tier": "free", "cveIds": [],
+            "ghsaIds": [], "severity": "high", "title": "t"
+        })
+    };
+    Mock::given(method("POST"))
+        .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/batch")))
+        .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
+            "packages": [
+                {"purl": empty, "patches": [patch(empty)]},
+                {"purl": bad, "patches": [patch(bad)]},
+            ],
+            "canAccessPaidPatches": false,
+        })))
+        .mount(&mock)
+        .await;
+    mount_by_package_empty(&mock, empty).await;
+    Mock::given(method("GET"))
+        .and(path(format!(
+            "/v0/orgs/{ORG_SLUG}/patches/by-package/{}",
+            encode_purl(bad)
+        )))
+        .respond_with(ResponseTemplate::new(500))
+        .mount(&mock)
+        .await;
+
+    let mut codes = Vec::new();
+    for json in [false, true] {
+        let tmp = tempfile::tempdir().unwrap();
+        write_root_package_json(tmp.path());
+        write_npm_package(tmp.path(), "minimist", "1.2.2", b"x\n");
+        write_npm_package(tmp.path(), "lodash", "4.17.20", b"x\n");
+        let mut extra = vec!["--mode", "agent", "--dry-run"];
+        if json {
+            extra.push("--json");
+        }
+        let (code, stdout, stderr) = run_scan_human(tmp.path(), &mock.uri(), &extra);
+        if json {
+            let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON");
+            assert!(
+                v["warnings"]
+                    .as_array()
+                    .is_some_and(|w| w.iter().any(|w| w["code"] == "patch_details_failed")),
+                "{v}"
+            );
+        } else {
+            assert!(
+                stderr.contains(&format!("Warning: could not fetch details for {bad}")),
+                "the failed package is named; stderr={stderr}"
+            );
+        }
+        codes.push(code);
+    }
+    assert_eq!(codes[0], codes[1], "human and --json exit alike");
+}

crates/socket-patch-cli/tests/in_process_scan.rs

@@ -930,10 +930,13 @@ async fn scan_non_json_with_patches_prints_table() {
 
     let code = run_scrubbed(args).await;
     // Non-JSON path: discovery → batch query → render table → fetch
-    // per-package details. We only mount the batch mock, so detail-fetch
-    // 404s and scan exits 1 ("Error: could not fetch patch details"). That exit is
-    // deterministic given these mocks.
-    assert_eq!(code, 1, "missing detail mock → detail fetch fails → exit 1");
+    // per-package details. We only mount the batch mock, so the detail
+    // query 404s, which the client reads as "no records": nothing to
+    // select, a clean exit 0, exactly like the `--json` arm (#1062).
+    assert_eq!(
+        code, 0,
+        "an empty detail result is no patch to apply, not a failure"
+    );
     // Prove the table-rendering path actually ran against real discovered
     // data: the batch endpoint was queried with the package, and the path
     // proceeded to the per-package detail fetch (i.e. it had a row to print).

crates/socket-patch-core/src/patch/redirect/upstream/mod.rs

@@ -917,7 +917,10 @@ mod tests {
     fn bun_lock_remedies_name_the_forced_reinstall() {
         for file in ["bun.lockb", "bun.lock", "packages/app/bun.lockb"] {
             let remedy = checkout_remedy(&[file.to_string()]);
-            assert!(remedy.contains(&format!("`git checkout -- {file}`")), "{remedy}");
+            assert!(
+                remedy.contains(&format!("`git checkout -- {file}`")),
+                "{remedy}"
+            );
             assert!(remedy.ends_with(
                 ", then run `bun install --force` (a plain `bun install` keeps the patched copy)"
             ), "{remedy}");