Fix duplicate references request on hosted rescans (#1606, #1607)

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

@@ -619,6 +619,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<'_>>,
+    // The withdrawn-pin sweep's reference answers (see
+    // `rollout::Gate::references`).
+    references: Option<super::withdrawn::KnownReferences>,
     // `--prune` / `--sync`, gated by the policy exactly as the human arm
     // gates it (`patches.enabled: false` writes nothing, the GC included):
     // only feeds the `redirect_prune_ignored` warning.
@@ -685,7 +688,11 @@ pub(super) async fn run_redirect(
         &pairs,
         scan_result,
         npm_prior,
-        Some(super::rollout::Gate::new(stage, rows).with_prior(prior)),
+        Some(
+            super::rollout::Gate::new(stage, rows)
+                .with_prior(prior)
+                .with_references(references),
+        ),
         "scan",
     )
     .await
@@ -869,12 +876,33 @@ async fn run_redirect_selected_untracked(
 
     if !selected.is_empty() {
         let uuids: Vec<String> = selected.iter().map(|(_, uuid)| uuid.clone()).collect();
-        status.set(format!(
-            "Resolving hosted artifacts for {}...",
-            crate::ui::plural(uuids.len(), "patch", "patches")
-        ));
-        let fetched = api_client.fetch_registry_references(&uuids).await;
-        status.finish();
+        // A rescan selects the uuids its lockfiles pin, which the
+        // withdrawn-pin sweep just asked about: ask only about the uuids it
+        // has no settled answer for.
+        let known = rollout.as_mut().and_then(|gate| gate.references.take());
+        let to_fetch = match &known {
+            Some(known) => known.unknown(&uuids),
+            None => uuids.clone(),
+        };
+        let fetched = if to_fetch.is_empty() {
+            Ok(std::collections::HashMap::new())
+        } else {
+            status.set(format!(
+                "Resolving hosted artifacts for {}...",
+                crate::ui::plural(to_fetch.len(), "patch", "patches")
+            ));
+            let fetched = api_client.fetch_registry_references(&to_fetch).await;
+            status.finish();
+            fetched
+        };
+        // `build_candidates` looks up only the selected uuids, so the
+        // sweep's answers for pins outside the selection are inert.
+        let fetched = fetched.map(|mut fetched| {
+            if let Some(known) = known {
+                fetched.extend(known.into_answers());
+            }
+            fetched
+        });
         let references = match fetched {
             Ok(r) => r,
             // A capped run whose every row is NEW: the failure affects only

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

@@ -2644,12 +2644,16 @@ async fn run_scan(
     // longer serves, before the rewrite below re-pins any replacement.
     // Runs ahead of every "nothing found" exit: a package whose only patch
     // was withdrawn has no search results at all.
-    let withdrawn_sweep = if hosted && !args.common.is_global() && policy.writes_allowed() {
+    let mut withdrawn_sweep = if hosted && !args.common.is_global() && policy.writes_allowed() {
         let pins = withdrawn::pins_in_scope(&hosted_pin_list, &all_purls);
         Some(withdrawn::sweep(&args.common, &api_client, &pins).await)
     } else {
         None
     };
+    // The sweep's reference answers, reused by the hosted redirect.
+    let sweep_references = withdrawn_sweep
+        .as_mut()
+        .and_then(withdrawn::WithdrawnSweep::take_references);
 
     // Calculate patch counts
     let mut free_patches = 0usize;
@@ -2782,6 +2786,7 @@ async fn run_scan(
                 batch_error_count > 0,
                 &mut stage,
                 prior_discovery,
+                sweep_references,
                 prune,
             )
             .await;
@@ -3255,7 +3260,11 @@ async fn run_scan(
             &pairs,
             None,
             npm_crawl.as_ref(),
-            Some(rollout::Gate::new(&mut stage, rows).with_prior(prior_discovery)),
+            Some(
+                rollout::Gate::new(&mut stage, rows)
+                    .with_prior(prior_discovery)
+                    .with_references(sweep_references),
+            ),
             "scan",
         )
         .await;

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

@@ -47,6 +47,9 @@ pub(crate) struct Gate<'a> {
     /// (with the configured patch-server origins): the rewrite's
     /// attribution gate reuses it when nothing changed the project since.
     pub(crate) prior: Option<Prior<'a>>,
+    /// The withdrawn-pin sweep's reference answers: the redirect asks the
+    /// endpoint only about the selected uuids these do not cover.
+    pub(crate) references: Option<super::withdrawn::KnownReferences>,
 }
 
 /// Scan's discovery, made BEFORE the apply lock, with the paths it read.
@@ -76,6 +79,7 @@ impl<'a> Gate<'a> {
             stage,
             rows,
             prior: None,
+            references: None,
         }
     }
 
@@ -85,6 +89,16 @@ impl<'a> Gate<'a> {
         self
     }
 
+    /// This gate carrying the sweep's reference answers (see
+    /// [`Self::references`]).
+    pub(crate) fn with_references(
+        mut self,
+        references: Option<super::withdrawn::KnownReferences>,
+    ) -> Self {
+        self.references = references;
+        self
+    }
+
     /// `(purl, uuid)` of every NEW row, for O(1) [`Self::is_new`] checks.
     pub(crate) fn new_keys(&self) -> HashSet<(String, String)> {
         self.rows

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

@@ -23,11 +23,17 @@
 //! entitlement looks like. A `forbidden`, `pending_build` or `build_failed`
 //! answer keeps the pin too. If the reference lookup fails (the patch API
 //! is down, say), every pin is kept and the run warns.
+//!
+//! The settled answers are kept ([`WithdrawnSweep::take_references`]): on
+//! a rescan the redirect selects the same uuids the lockfiles pin, and asks
+//! the endpoint only about the ones the sweep has no answer for (#1606,
+//! #1607).
 
-use std::collections::BTreeSet;
+use std::collections::{BTreeSet, HashMap};
 use std::time::Duration;
 
 use socket_patch_core::api::client::ApiClient;
+use socket_patch_core::api::types::PackageVendorResult;
 use socket_patch_core::patch::redirect::upstream::HostedPin;
 use socket_patch_core::utils::purl_key::PurlKey;
 
@@ -60,9 +66,53 @@ pub(super) struct WithdrawnSweep {
     events: Vec<PatchEvent>,
     /// `(code, detail)` run-level warnings.
     warnings: Vec<(String, String)>,
+    /// The reference endpoint's answers, when the lookup succeeded.
+    references: Option<KnownReferences>,
+}
+
+/// Reference answers the sweep already has, for the redirect to reuse.
+/// Only settled answers are kept: a `pending_build` (or any status this
+/// build does not know) may have changed by the time the redirect runs, and
+/// a uuid the response left out (a 404 chunk answers nothing) is asked
+/// again rather than read as `not_found`.
+#[derive(Debug, Default)]
+pub(crate) struct KnownReferences(HashMap<String, PackageVendorResult>);
+
+impl KnownReferences {
+    /// The settled answers in `results` for the uuids in `asked` (an
+    /// answer the request did not ask for is not trusted).
+    fn settled(asked: &[String], mut results: HashMap<String, PackageVendorResult>) -> Self {
+        results.retain(|uuid, r| {
+            asked.contains(uuid)
+                && matches!(
+                    r.status.as_str(),
+                    "granted" | "reused" | "withdrawn" | "not_found" | "forbidden"
+                )
+        });
+        KnownReferences(results)
+    }
+
+    /// The uuids in `uuids` with no known answer.
+    pub(crate) fn unknown(&self, uuids: &[String]) -> Vec<String> {
+        uuids
+            .iter()
+            .filter(|uuid| !self.0.contains_key(*uuid))
+            .cloned()
+            .collect()
+    }
+
+    /// The known answers, keyed by uuid as the reference fetch returns them.
+    pub(crate) fn into_answers(self) -> HashMap<String, PackageVendorResult> {
+        self.0
+    }
 }
 
 impl WithdrawnSweep {
+    /// The reference answers the sweep fetched, for the redirect to reuse.
+    pub(super) fn take_references(&mut self) -> Option<KnownReferences> {
+        self.references.take()
+    }
+
     /// Fold the outcome into the scan envelope.
     pub(super) fn record_into(self, env: &mut Envelope) {
         for event in self.events {
@@ -140,6 +190,7 @@ pub(super) async fn sweep(
             is_gone(status).then(|| (pin.clone(), status.clone()))
         })
         .collect();
+    out.references = Some(KnownReferences::settled(&uuids, references));
     if gone.is_empty() {
         return out;
     }
@@ -271,4 +322,29 @@ mod tests {
         assert!(pins_in_scope(&pins, ["pkg:npm/b@2.0.1"]).is_empty());
         assert!(pins_in_scope(&pins, Vec::<String>::new()).is_empty());
     }
+
+    #[test]
+    fn only_settled_answers_are_known() {
+        let answer = |status: &str| -> PackageVendorResult {
+            serde_json::from_value(serde_json::json!({ "status": status })).unwrap()
+        };
+        // `left-out`: the response did not answer it (a 404 chunk, say).
+        let uuids = ["granted", "withdrawn", "building", "failed", "left-out"].map(String::from);
+        let known = KnownReferences::settled(
+            &uuids,
+            HashMap::from([
+                ("granted".to_string(), answer("granted")),
+                ("withdrawn".to_string(), answer("withdrawn")),
+                ("building".to_string(), answer("pending_build")),
+                ("failed".to_string(), answer("build_failed")),
+                // Not asked for: dropped.
+                ("unasked".to_string(), answer("granted")),
+            ]),
+        );
+        assert_eq!(known.unknown(&uuids), ["building", "failed", "left-out"]);
+        assert_eq!(known.unknown(&["unasked".to_string()]), ["unasked"]);
+        let mut answers: Vec<_> = known.into_answers().into_keys().collect();
+        answers.sort();
+        assert_eq!(answers, ["granted", "withdrawn"]);
+    }
 }

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

@@ -68,11 +68,19 @@ fn scan_args(cwd: &Path, api_url: String, dry_run: bool) -> ScanArgs {
 /// Run an in-process hosted scan with the npm registry pointed at
 /// `server`, scrubbing the env toggles `run` mirrors into the process.
 async fn scan(cwd: &Path, server: &MockServer, dry_run: bool) -> i32 {
+    scan_with(cwd, server, dry_run, true).await
+}
+
+/// [`scan`], with `--json` on or off (off runs `--silent`).
+async fn scan_with(cwd: &Path, server: &MockServer, dry_run: bool, json: bool) -> i32 {
     std::env::set_var(
         "SOCKET_NPM_REGISTRY",
         format!("{}/npm-registry", server.uri()),
     );
-    let code = scan_run(scan_args(cwd, server.uri(), dry_run)).await;
+    let mut args = scan_args(cwd, server.uri(), dry_run);
+    args.common.json = json;
+    args.common.silent = !json;
+    let code = scan_run(args).await;
     for var in ["SOCKET_NPM_REGISTRY", "SOCKET_DRY_RUN"] {
         std::env::remove_var(var);
     }
@@ -525,3 +533,117 @@ async fn the_envelope_reports_the_restored_pin() {
     assert_eq!(restored[0]["details"]["mode"], "hosted");
     assert_eq!(lock(tmp.path()), pristine);
 }
+
+/// The uuids each reference request asked about, in request order.
+async fn reference_uuids(server: &MockServer) -> Vec<Vec<String>> {
+    server
+        .received_requests()
+        .await
+        .unwrap_or_default()
+        .iter()
+        .filter(|r| r.url.path().ends_with("/patches/package"))
+        .map(|r| {
+            let body: Value = serde_json::from_slice(&r.body).unwrap();
+            let mut uuids: Vec<String> = body["uuids"]
+                .as_array()
+                .unwrap()
+                .iter()
+                .map(|u| u.as_str().unwrap().to_string())
+                .collect();
+            uuids.sort();
+            uuids
+        })
+        .collect()
+}
+
+/// A rescan of a pin whose patch is still offered asks the reference
+/// endpoint about its uuid once: the redirect reuses the sweep's answer
+/// instead of sending the same request again (#1606, #1607).
+async fn rescan_asks_once(json: bool) {
+    let tmp = tempfile::tempdir().unwrap();
+    let (_, wired) = wired_project(tmp.path()).await;
+
+    let server = MockServer::start().await;
+    mock_search(&server, &[UUID]).await;
+    mock_references(
+        &server,
+        serde_json::json!({ UUID: granted(HOSTED_URL, PATCHED_SHA512) }),
+    )
+    .await;
+    mock_view(&server, UUID).await;
+    assert_eq!(scan_with(tmp.path(), &server, false, json).await, 0);
+
+    assert_eq!(lock(tmp.path()), wired, "the rescan keeps the pin");
+    assert_eq!(
+        reference_uuids(&server).await,
+        vec![vec![UUID.to_string()]],
+        "one reference request for the pinned uuid, not one per phase"
+    );
+}
+
+#[tokio::test]
+#[serial]
+async fn a_json_rescan_asks_about_a_pinned_uuid_once() {
+    rescan_asks_once(true).await;
+}
+
+#[tokio::test]
+#[serial]
+async fn a_human_rescan_asks_about_a_pinned_uuid_once() {
+    rescan_asks_once(false).await;
+}
+
+/// A replacement patch the sweep never asked about is still looked up, and
+/// only it: the withdrawn uuid's answer is reused.
+#[tokio::test]
+#[serial]
+async fn the_redirect_asks_only_about_uuids_the_sweep_did_not() {
+    let tmp = tempfile::tempdir().unwrap();
+    wired_project(tmp.path()).await;
+
+    let server = MockServer::start().await;
+    mock_search(&server, &[UUID2]).await;
+    mock_references(
+        &server,
+        serde_json::json!({
+            UUID: { "status": "withdrawn" },
+            UUID2: granted(HOSTED_URL2, PATCHED_SHA512_2),
+        }),
+    )
+    .await;
+    mock_view(&server, UUID2).await;
+    mock_npm_registry(&server).await;
+    assert_eq!(scan(tmp.path(), &server, false).await, 0);
+
+    let wired = lock(tmp.path());
+    assert!(wired.contains(HOSTED_URL2), "re-pinned:\n{wired}");
+    assert_eq!(
+        reference_uuids(&server).await,
+        vec![vec![UUID.to_string()], vec![UUID2.to_string()]],
+    );
+}
+
+/// An answer that may change within the run (`pending_build`) is asked
+/// again by the redirect instead of reused.
+#[tokio::test]
+#[serial]
+async fn a_pending_build_answer_is_asked_again() {
+    let tmp = tempfile::tempdir().unwrap();
+    let (_, wired) = wired_project(tmp.path()).await;
+
+    let server = MockServer::start().await;
+    mock_search(&server, &[UUID]).await;
+    mock_references(
+        &server,
+        serde_json::json!({ UUID: { "status": "pending_build" } }),
+    )
+    .await;
+    mock_view(&server, UUID).await;
+    assert_eq!(scan(tmp.path(), &server, false).await, 0);
+
+    assert_eq!(lock(tmp.path()), wired, "a building patch keeps its pin");
+    assert_eq!(
+        reference_uuids(&server).await,
+        vec![vec![UUID.to_string()], vec![UUID.to_string()]],
+    );
+}