Fix vendored dry-run counting in-sync patches (#1850)

crates/socket-patch-cli/src/commands/get.rs

@@ -2290,8 +2290,7 @@ async fn run_get_vendored(
             preview.record_into(&mut env);
             emit(&env);
         } else if !args.common.silent {
-            println!("{}", format_dry_run("download and vendor", selected.len()));
-            preview.print_refusals();
+            preview.print_human();
         }
         return 0;
     }

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

@@ -3412,21 +3412,14 @@ async fn run_scan(
         }
     }
 
-    // What the dry-run line offers.
-    let plan = if vendor {
-        render::Plan::Vendor(selected.len())
-    } else {
-        render::Plan::Apply(selected.len())
-    };
-
     // `--dry-run` is a non-mutating preview: stop here, having printed the
     // table and the per-patch plan above, before the download/apply and the
     // prune GC (which runs as a read-only preview instead).
     if args.common.dry_run {
         if !silent {
             // Vendored preview: the JSON arm's ledger classification,
-            // rendered as `[would-refuse]` lines so a preview never
-            // advertises vendoring the wet run would refuse.
+            // rendered as `[skip]` and `[would-refuse]` lines so a preview
+            // never advertises vendoring the wet run would skip or refuse.
             let preview = if vendor {
                 // `vendor` is never report-only, so the planning pass
                 // above always computed it.
@@ -3438,10 +3431,12 @@ async fn run_scan(
             } else {
                 None
             };
-            let refused = preview.as_ref().map_or(0, |p| p.refused_count());
-            println!("{}", render::dry_run_line(plan, refused));
-            if let Some(preview) = &preview {
-                preview.print_refusals();
+            match &preview {
+                Some(preview) => preview.print_human(),
+                None => println!(
+                    "{}",
+                    render::dry_run_line(render::Plan::Apply(selected.len()))
+                ),
             }
         }
         print_rollout_human(&stage, true, silent);

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

@@ -249,22 +249,46 @@ pub(super) fn unwired_vendored_detail(purls: &[String]) -> String {
 pub(super) enum Plan {
     /// Agent mode: download `n` patches and apply them in place.
     Apply(usize),
-    /// Vendored mode: download `n` patches and vendor them.
-    Vendor(usize),
+    /// Vendored mode: of `total` selected patches, `refused` would be
+    /// refused by the vendored preflight (listed as `[would-refuse]`
+    /// lines) and `in_sync` are held by the ledger at this uuid (listed as
+    /// `[skip] … (already vendored)` lines); the rest are downloaded and
+    /// vendored.
+    Vendor {
+        total: usize,
+        refused: usize,
+        in_sync: usize,
+    },
 }
 
-/// The `--dry-run` headline. `refused` counts the patches the vendored
-/// preflight would refuse (listed right after as `[would-refuse]` lines),
-/// so the headline never promises to vendor what the wet run refuses.
-pub(super) fn dry_run_line(plan: Plan, refused: usize) -> String {
+/// The `--dry-run` headline. A vendored plan leaves out what the wet run
+/// refuses or skips as already vendored, so the headline never promises
+/// to vendor either (#1850).
+pub(super) fn dry_run_line(plan: Plan) -> String {
     let what = match plan {
         Plan::Apply(n) => format!("download and apply {}", plural(n, "patch", "patches")),
-        Plan::Vendor(n) if refused > 0 => format!(
-            "download and vendor {} of {} ({refused} would be refused)",
-            n.saturating_sub(refused),
-            plural(n, "patch", "patches")
-        ),
-        Plan::Vendor(n) => format!("download and vendor {}", plural(n, "patch", "patches")),
+        Plan::Vendor {
+            total,
+            refused,
+            in_sync,
+        } if refused > 0 || in_sync > 0 => {
+            let mut why = Vec::new();
+            if in_sync > 0 {
+                why.push(format!("{in_sync} already vendored"));
+            }
+            if refused > 0 {
+                why.push(format!("{refused} would be refused"));
+            }
+            format!(
+                "download and vendor {} of {} ({})",
+                total.saturating_sub(refused + in_sync),
+                plural(total, "patch", "patches"),
+                why.join(", ")
+            )
+        }
+        Plan::Vendor { total, .. } => {
+            format!("download and vendor {}", plural(total, "patch", "patches"))
+        }
     };
     format!("[dry-run] Would {what}. No changes made.")
 }
@@ -748,26 +772,48 @@ mod tests {
 
     // ---- dry-run / hints -------------------------------------------
 
+    fn vendor(total: usize, refused: usize, in_sync: usize) -> Plan {
+        Plan::Vendor {
+            total,
+            refused,
+            in_sync,
+        }
+    }
+
     #[test]
     fn dry_run_line_counts_refusals() {
         assert_eq!(
-            dry_run_line(Plan::Apply(1), 0),
+            dry_run_line(Plan::Apply(1)),
             "[dry-run] Would download and apply 1 patch. No changes made."
         );
         assert_eq!(
-            dry_run_line(Plan::Vendor(2), 0),
+            dry_run_line(vendor(2, 0, 0)),
             "[dry-run] Would download and vendor 2 patches. No changes made."
         );
         assert_eq!(
-            dry_run_line(Plan::Vendor(2), 2),
+            dry_run_line(vendor(2, 2, 0)),
             "[dry-run] Would download and vendor 0 of 2 patches (2 would be refused). No changes made."
         );
         assert_eq!(
-            dry_run_line(Plan::Vendor(1), 1),
+            dry_run_line(vendor(1, 1, 0)),
             "[dry-run] Would download and vendor 0 of 1 patch (1 would be refused). No changes made."
         );
     }
 
+    #[test]
+    fn dry_run_line_counts_already_vendored() {
+        // #1850: an in-sync patch is not work the wet run would do.
+        assert_eq!(
+            dry_run_line(vendor(1, 0, 1)),
+            "[dry-run] Would download and vendor 0 of 1 patch (1 already vendored). No changes made."
+        );
+        assert_eq!(
+            dry_run_line(vendor(4, 1, 2)),
+            "[dry-run] Would download and vendor 1 of 4 patches (2 already vendored, \
+             1 would be refused). No changes made."
+        );
+    }
+
     #[test]
     fn report_only_hint_names_agent_mode() {
         let project = report_only_hint(&GlobalArgs::default());

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

@@ -173,12 +173,43 @@ impl VendorPreview {
         }
     }
 
-    /// Human rendering of the preview's refusals and symlink advisories:
-    /// the count line above it still says "would download and vendor", so
-    /// name what the wet run would refuse and why. Shared by `scan --mode
-    /// vendored --dry-run` and `get … --mode vendored --dry-run` so the two
-    /// cannot drift. Callers gate it on `--silent`.
-    pub(crate) fn print_refusals(&self) {
+    /// How many rows the ledger already holds at their uuid.
+    pub(crate) fn in_sync_count(&self) -> usize {
+        self.rows
+            .iter()
+            .filter(|r| matches!(r.verdict, PreviewVerdict::AlreadyVendored))
+            .count()
+    }
+
+    /// The human `--dry-run` headline: every row, less the ones the wet
+    /// run would refuse or skip as already vendored (#1850).
+    fn dry_run_line(&self) -> String {
+        super::render::dry_run_line(super::render::Plan::Vendor {
+            total: self.rows.len(),
+            refused: self.refused_count(),
+            in_sync: self.in_sync_count(),
+        })
+    }
+
+    /// Human rendering of the preview: the headline, then a `[skip] …
+    /// (already vendored)` line for each in-sync row (stderr, the wet
+    /// run's line and stream), a `[would-refuse]` line naming what the wet
+    /// run would refuse and why, and the symlink advisories. Shared by
+    /// `scan --mode vendored --dry-run` and `get … --mode vendored
+    /// --dry-run` so the two cannot drift. Callers gate it on `--silent`.
+    pub(crate) fn print_human(&self) {
+        println!("{}", self.dry_run_line());
+        for row in &self.rows {
+            if matches!(row.verdict, PreviewVerdict::AlreadyVendored) {
+                eprintln!(
+                    "{}",
+                    crate::commands::agent_download::format_record_skip(
+                        &row.purl,
+                        "already vendored"
+                    )
+                );
+            }
+        }
         for row in &self.rows {
             if let PreviewVerdict::WouldRefuse { code, detail } = &row.verdict {
                 println!("  [would-refuse] {} ({code}): {detail}", row.purl);
@@ -1480,6 +1511,50 @@ mod preview_tests {
         assert_eq!(preview.refused_count(), 1);
     }
 
+    /// #1850: the human dry-run headline, shared by `scan` and `get`,
+    /// leaves out what the JSON records as `already_vendored`, so it
+    /// never offers to vendor a patch the wet run skips as in sync.
+    #[tokio::test]
+    async fn preview_headline_leaves_out_already_vendored() {
+        let tmp = tempfile::tempdir().unwrap();
+        seed_entry(tmp.path(), NPM, UUID);
+        let in_sync =
+            preview_vendor_with(tmp.path(), &[sel(UUID, NPM)], &[], &HashMap::new()).await;
+        assert_eq!(in_sync.in_sync_count(), 1);
+        assert_eq!(
+            in_sync.dry_run_line(),
+            "[dry-run] Would download and vendor 0 of 1 patch (1 already vendored). \
+             No changes made."
+        );
+
+        let other = "pkg:npm/preview-new@1.0.0";
+        let mixed = preview_vendor_with(
+            tmp.path(),
+            &[sel(UUID, NPM), sel(UUID, other), sel(UUID, PYPI)],
+            &[],
+            &HashMap::from([(
+                PYPI.to_string(),
+                ("vendor_test_refusal", "refused".to_string()),
+            )]),
+        )
+        .await;
+        assert_eq!(
+            mixed.dry_run_line(),
+            "[dry-run] Would download and vendor 1 of 3 patches (1 already vendored, \
+             1 would be refused). No changes made."
+        );
+
+        // Nothing in sync: the plain headline.
+        let fresh = tempfile::tempdir().unwrap();
+        let plain =
+            preview_vendor_with(fresh.path(), &[sel(UUID, NPM)], &[], &HashMap::new()).await;
+        assert_eq!(plain.in_sync_count(), 0);
+        assert_eq!(
+            plain.dry_run_line(),
+            "[dry-run] Would download and vendor 1 patch. No changes made."
+        );
+    }
+
     const UUID: &str = "11111111-1111-4111-8111-111111111111";
     const OLD_UUID: &str = "00000000-0000-4000-8000-000000000000";
     const NPM: &str = "pkg:npm/preview-bun@1.0.0";

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

@@ -46,9 +46,6 @@
 
 #![cfg(feature = "docker-e2e")]
 
-#[allow(clippy::duplicate_mod)]
-#[path = "common/fs_tree.rs"]
-mod fs_tree;
 #[path = "common/cache_env.rs"]
 mod cache_env;
 #[path = "composer_e2e_common/mod.rs"]
@@ -57,6 +54,9 @@ mod composer_e2e_common;
 mod docker_common;
 #[path = "docker_vendor_common/mod.rs"]
 mod docker_vendor_common;
+#[allow(clippy::duplicate_mod)]
+#[path = "common/fs_tree.rs"]
+mod fs_tree;
 #[path = "vex_e2e_common/mod.rs"]
 mod vex_e2e_common;
 

crates/socket-patch-cli/tests/get/get_render.rs

@@ -105,6 +105,51 @@ async fn human_vendored_uuid_dry_run_prints_line() {
     assert!(!tmp.path().join(".socket").exists());
 }
 
+/// #1850: `get <uuid> --mode vendored --dry-run` on a project the ledger
+/// already holds at this uuid shares scan's preview headline: nothing to
+/// download and vendor, and the in-sync patch named as skipped.
+#[tokio::test]
+async fn human_vendored_uuid_dry_run_skips_already_vendored() {
+    let server = MockServer::start().await;
+    mount_real_view(&server, UUID, PURL).await;
+
+    let tmp = tempfile::tempdir().unwrap();
+    write_project(tmp.path());
+    let vendor_dir = tmp.path().join(".socket/vendor");
+    std::fs::create_dir_all(&vendor_dir).unwrap();
+    std::fs::write(
+        vendor_dir.join("state.json"),
+        serde_json::json!({
+            "version": 1,
+            "entries": { PURL: {
+                "ecosystem": "npm", "basePurl": PURL, "uuid": UUID,
+                "artifact": { "path": format!(".socket/vendor/npm/{UUID}/{NAME}-1.0.0.tgz") },
+                "wiring": [],
+            }}
+        })
+        .to_string(),
+    )
+    .unwrap();
+
+    let (code, stdout, stderr) = run_get_bin(
+        tmp.path(),
+        &server.uri(),
+        &[UUID, "--mode", "vendored", "--dry-run"],
+    );
+    assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}");
+    assert!(
+        stdout.contains(
+            "[dry-run] Would download and vendor 0 of 1 patch (1 already vendored). \
+             No changes made."
+        ),
+        "stdout={stdout}"
+    );
+    assert!(
+        stderr.contains(&format!("  [skip] {PURL} (already vendored)")),
+        "stderr={stderr}"
+    );
+}
+
 /// Human search-path vendored dry-run line (the pluralized count flavor).
 #[tokio::test]
 async fn human_vendored_search_dry_run_prints_count() {

crates/socket-patch-cli/tests/scan/scan_render.rs

@@ -674,6 +674,45 @@ async fn scan_human_vendored_dry_run_names_would_refuse_records() {
     );
 }
 
+/// #1850: on a project already vendored at this uuid, the human
+/// `--mode vendored --dry-run` says so instead of offering to download and
+/// vendor the patch: the headline leaves it out, as the JSON's
+/// `already_vendored` row and the wet run's "nothing to do" do, and the
+/// wet run's `[skip] … (already vendored)` line names it.
+#[tokio::test]
+async fn scan_human_vendored_dry_run_skips_already_vendored() {
+    let mock = MockServer::start().await;
+    mount_batch_one(&mock, MINIMIST, UUID, "free", &[], false).await;
+    mount_by_package(&mock, MINIMIST, UUID, serde_json::json!({})).await;
+    let tmp = minimist_project(b"x\n");
+    seed_vendored_entry(tmp.path(), MINIMIST);
+    let ledger = std::fs::read(tmp.path().join(".socket/vendor/state.json")).unwrap();
+
+    let (code, stdout, stderr) = run_scan_api(
+        tmp.path(),
+        &mock.uri(),
+        &["--mode", "vendored", "--dry-run"],
+    );
+    assert_eq!(code, 0, "stdout={stdout}; stderr={stderr}");
+    assert!(
+        stdout.contains(
+            "[dry-run] Would download and vendor 0 of 1 patch (1 already vendored). \
+             No changes made."
+        ),
+        "the headline must not count the in-sync patch as work; got {stdout:?}"
+    );
+    assert!(
+        stderr.contains(&format!("  [skip] {MINIMIST} (already vendored)")),
+        "the in-sync patch is named as skipped; stderr={stderr:?}"
+    );
+    assert!(!stdout.contains("[would-refuse]"), "{stdout:?}");
+    assert_eq!(
+        std::fs::read(tmp.path().join(".socket/vendor/state.json")).unwrap(),
+        ledger,
+        "a dry run writes nothing"
+    );
+}
+
 // ---------------------------------------------------------------------------
 // Report-only runs, agent-mode applies, GC and wiring warnings
 // ---------------------------------------------------------------------------