Sweep update, user_patches, vendor_orphans, vlt_heal

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

@@ -90,19 +90,6 @@ async fn fail(args: &UpdateArgs, code: &str, message: &str) -> i32 {
     1
 }
 
-/// A stable machine name for `channel` (telemetry), unlike the
-/// human-facing [`channel_label`] (`cargo install`, `the RubyGems launcher`).
-fn channel_id(channel: InstallChannel) -> &'static str {
-    match channel {
-        InstallChannel::Standalone => "standalone",
-        InstallChannel::Npm => "npm",
-        InstallChannel::Pypi => "pypi",
-        InstallChannel::Cargo => "cargo",
-        InstallChannel::LauncherCache => "launcher_cache",
-        InstallChannel::Homebrew => "homebrew",
-    }
-}
-
 /// The no-op message when there is nothing to install: a pin already
 /// satisfied, the latest release already running, or a build newer than
 /// the latest release (`latest` never downgrades).
@@ -167,33 +154,15 @@ fn installed_message(
     }
 }
 
-/// The status line shown while the release downloads and installs.
-fn download_status(target: &semver::Version, asset: &str) -> String {
-    format!("Downloading socket-patch {target} ({asset})...")
-}
-
 /// Record a non-fatal advisory: stderr for humans, `warnings[]` on the
-/// envelope for machines. `--json` suppresses the stderr line (stdout is
-/// the machine channel and stderr must stay clean), so a warning that only
-/// ever went to stderr would vanish entirely for JSON consumers — the
-/// managed-install override in particular is the "your package manager
-/// will silently revert this" signal, and a silent override is the bug
-/// class the channel suite exists to catch. Same stderr-or-envelope
-/// split `vendor`/`remove` use for their run-level advisories (the
-/// rendered stderr line keeps update's own `Warning: <detail>` wording).
+/// envelope for machines, which never see stderr under `--json`.
 fn note_warning(warnings: &mut Vec<RunWarning>, quiet: bool, code: &str, detail: String) {
     if !quiet {
         eprintln!("Warning: {}", crate::ui::sentence_case(&detail));
     }
-    warnings.push(RunWarning {
-        code: code.to_string(),
-        detail,
-    });
+    warnings.push(RunWarning::new(code, detail));
 }
 
-/// The status line while `--update` asks which release is the latest.
-const CHECKING_LATEST: &str = "Checking for the latest socket-patch release...";
-
 pub async fn run(args: UpdateArgs) -> i32 {
     apply_env_toggles(&args.common);
     let quiet = args.common.json || args.common.silent;
@@ -268,7 +237,7 @@ pub async fn run(args: UpdateArgs) -> i32 {
             // each with its own connect and read budget): say what is
             // happening instead of a blank terminal.
             let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent);
-            status.set(CHECKING_LATEST);
+            status.set("Checking for the latest socket-patch release...");
             let latest = fetch_latest_version(&endpoints, &timeouts).await;
             status.finish();
             match latest {
@@ -369,7 +338,9 @@ pub async fn run(args: UpdateArgs) -> i32 {
     //    download can take a while (300 s budget), so a terminal gets a
     //    status line instead of a silent pause after the prompt.
     let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent);
-    status.set(download_status(&target_version, &asset));
+    status.set(format!(
+        "Downloading socket-patch {target_version} ({asset})..."
+    ));
     let result = core_update::perform_update(UpdateRequest {
         target_triple: UPDATE_TARGET,
         version: &target_version,
@@ -400,7 +371,7 @@ pub async fn run(args: UpdateArgs) -> i32 {
     // Before the result prints, like `fail`'s failure event.
     track_cli_updated(
         &target_version.to_string(),
-        channel_id(channel),
+        channel.id(),
         pinned,
         args.force,
         &args.common.telemetry_auth(),
@@ -441,14 +412,6 @@ pub async fn run(args: UpdateArgs) -> i32 {
 mod tests {
     use super::*;
 
-    #[test]
-    fn checking_latest_status_line() {
-        assert_eq!(
-            CHECKING_LATEST,
-            "Checking for the latest socket-patch release..."
-        );
-    }
-
     fn v(s: &str) -> semver::Version {
         semver::Version::parse(s).unwrap()
     }
@@ -529,21 +492,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn download_status_and_error_capitalization() {
-        assert_eq!(
-            download_status(&v("9.9.9"), "socket-patch-x.tar.gz"),
-            "Downloading socket-patch 9.9.9 (socket-patch-x.tar.gz)..."
-        );
-        assert_eq!(
-            crate::ui::sentence_case("could not check for updates: x"),
-            "Could not check for updates: x"
-        );
-        assert_eq!(crate::ui::sentence_case(""), "");
-        assert_eq!(crate::ui::sentence_case("éclair"), "Éclair");
-        assert_eq!(crate::ui::sentence_case("Already"), "Already");
-    }
-
     #[test]
     fn help_describes_the_update_flag_not_the_internal_subcommand() {
         use clap::CommandFactory;

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

@@ -33,6 +33,16 @@ pub(crate) async fn inspect(common: &GlobalArgs) -> UserPatchIndex {
     }
 }
 
+/// The project-patch sources protecting `purl`, if its ecosystem is selected
+/// and it has any.
+fn protecting(common: &GlobalArgs, index: &UserPatchIndex, purl: &str) -> Option<Vec<String>> {
+    if !common.purl_ecosystem_selected(purl) {
+        return None;
+    }
+    let sources = index.conflicts(purl);
+    (!sources.is_empty()).then_some(sources)
+}
+
 pub(crate) fn detail(purl: &str, sources: &[String], allowed: bool) -> String {
     let sources = sources.join(", ");
     if allowed {
@@ -76,13 +86,9 @@ pub(crate) async fn partition(
     let mut conflicts = Vec::new();
     for purl in &purls {
         let purl = purl.as_str();
-        if !common.purl_ecosystem_selected(purl) {
+        let Some(sources) = protecting(common, &index, purl) else {
             continue;
-        }
-        let sources = index.conflicts(purl);
-        if sources.is_empty() {
-            continue;
-        }
+        };
         let message = detail(purl, &sources, common.allow_patch_conflicts);
         if common.allow_patch_conflicts {
             warnings.push((ALLOWED.into(), message));
@@ -106,11 +112,7 @@ pub(crate) async fn evidence(
     }
     let index = inspect(common).await;
     for &purl in purls {
-        if !common.purl_ecosystem_selected(purl) {
-            continue;
-        }
-        let sources = index.conflicts(purl);
-        if !sources.is_empty() {
+        if let Some(sources) = protecting(common, &index, purl) {
             found.insert(purl.to_string(), sources);
         }
     }
@@ -149,13 +151,9 @@ pub(crate) async fn skip_protected(
     }
     let index = inspect(common).await;
     patches.retain(|purl, record| {
-        if !common.purl_ecosystem_selected(purl) {
+        let Some(sources) = protecting(common, &index, purl) else {
             return true;
-        }
-        let sources = index.conflicts(purl);
-        if sources.is_empty() {
-            return true;
-        }
+        };
         let message = detail(purl, &sources, common.allow_patch_conflicts);
         if common.allow_patch_conflicts {
             warnings.push((ALLOWED.into(), message));

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

@@ -1,7 +1,7 @@
 //! The orphan sweep `vendor --revert`, `scan --prune`, `rollback` and
 //! `remove` share: uuid dirs under `.socket/vendor/<eco>/` that no ledger
 //! entry owns (a hand-edited state file, an interrupted run, the losing
-//! side of a git merge of two vendor runs, #1610).
+//! side of a git merge of two vendor runs).
 
 use std::collections::HashSet;
 use std::path::Path;
@@ -24,10 +24,9 @@ pub(crate) struct OrphanSweep {
     pub(crate) failed: Vec<(vendor::path::SweptVendorDir, String)>,
 }
 
-/// Uuid dirs under `.socket/vendor/<eco>/` with no owning `(eco, uuid)`
-/// ledger entry (a hand-edited state file, or artifacts left by an
-/// interrupted run). Unparseable dirs are never returned (and never
-/// deleted). Returns the orphans so callers can emit events / counts.
+/// Sweep the uuid dirs under `.socket/vendor/<eco>/` with no owning
+/// `(eco, uuid)` ledger entry. Unparseable dirs are never returned (and
+/// never deleted).
 ///
 /// A missing ledger entry does NOT prove missing wiring: lockfiles can
 /// still point into `.socket/vendor/` after a deleted state.json or a
@@ -44,7 +43,7 @@ pub(crate) async fn sweep_orphan_vendor_dirs(
 }
 
 /// [`sweep_orphan_vendor_dirs`] limited to the orphans `select` accepts:
-/// `rollback` and `remove` sweep only the ones in their scope (#1610).
+/// `rollback` and `remove` sweep only the ones in their scope.
 pub(crate) async fn sweep_orphan_vendor_dirs_where(
     cwd: &Path,
     state: &VendorState,
@@ -69,20 +68,19 @@ pub(crate) async fn sweep_orphan_vendor_dirs_where(
     if candidates.is_empty() {
         return out;
     }
-    let wired: HashSet<(String, String)> =
-        crate::commands::vendored_backend::repair::scan_vendor_references(cwd)
-            .await
-            .into_iter()
-            .map(|(eco, uuid, _path)| (eco, uuid))
-            .collect();
+    let references = crate::commands::vendored_backend::repair::scan_vendor_references(cwd).await;
+    let wired: HashSet<(&str, &str)> = references
+        .iter()
+        .map(|(eco, uuid, _path)| (eco.as_str(), uuid.as_str()))
+        .collect();
     // Each removal also prunes the `<eco>/` and `vendor/` levels it
     // emptied (never `.socket/` — the lock guard owns that level): the
     // sweep runs AFTER the per-entry reverts and their ledger saves, so it
     // is the last thing that can leave a fully reverted project with empty
     // `.socket/vendor/<eco>/` husks.
     let stop_dir = cwd.join(SOCKET_DIR);
     for unit in candidates {
-        if wired.contains(&(unit.eco.clone(), unit.uuid.clone())) {
+        if wired.contains(&(unit.eco.as_str(), unit.uuid.as_str())) {
             out.still_wired.push(unit);
             continue;
         }
@@ -183,7 +181,7 @@ mod tests {
     /// `rollback <target>` / `remove <target>` scope their orphan sweep
     /// with [`orphan_in_target`]: a package target matches by the leaves'
     /// purls, a uuid target by the dir's own uuid, even when no leaf
-    /// parses (#1610).
+    /// parses.
     #[test]
     fn orphan_in_target_matches_package_and_uuid_targets() {
         let uuid = "aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa";
@@ -216,8 +214,7 @@ mod tests {
 
     /// A kept, still-wired orphan points the user at a remedy that works
     /// in v5: `repair` no longer rebuilds ledger entries from lockfiles,
-    /// so the advisory names the ledger restore (and the merge case,
-    /// #991) or the lockfile restore instead.
+    /// so the advisory names the ledger restore (and the merge case).
     #[test]
     fn orphan_still_wired_names_a_working_remedy() {
         let unit = vendor::path::SweptVendorDir {
@@ -228,15 +225,15 @@ mod tests {
         };
         let warning = orphan_still_wired(&unit);
         assert_eq!(warning.code, "vendor_orphan_still_wired");
-        let detail = &warning.detail;
-        assert!(
-            detail.contains(".socket/vendor/npm/aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa"),
-            "{detail}"
+        // The advisory follows a revert that rewrote the lockfile, so it
+        // names no lockfile checkout: that would re-wire what it removed.
+        assert_eq!(
+            warning.detail,
+            "a project lockfile still points at \
+             .socket/vendor/npm/aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa, which no ledger \
+             entry owns; the artifacts were kept (restore the missing entries in \
+             .socket/vendor/state.json from version control (if a git merge kept one \
+             side of state.json, merge both sides' `entries`), then revert again)"
         );
-        assert!(!detail.contains("socket-patch repair"), "{detail}");
-        assert!(detail.contains("entries"), "{detail}");
-        // The advisory follows a revert that rewrote the lockfile:
-        // checking the lockfile out again would re-wire what it removed.
-        assert!(!detail.contains("git checkout"), "{detail}");
     }
 }

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

@@ -29,11 +29,11 @@ pub(crate) struct RewriteHeal {
     /// Set when the heal ran (it may invalidate store entries): the store's
     /// bundled copies after it, discovery's only read of the installed tree
     /// (`Discovery::vlt_bundled_copies`). `None` when nothing was healed.
-    pub(crate) healed_store: Option<std::collections::BTreeMap<String, String>>,
+    pub(crate) healed_store: Option<BTreeMap<String, String>>,
 }
 
 pub(crate) const REINSTALL_REQUIRED: &str = "redirect_vlt_reinstall_required";
-/// A bundled copy of a redirected package that no rewire reaches (#471).
+/// A bundled copy of a redirected package that no rewire reaches.
 pub(crate) const BUNDLED_INSTANCE_SKIPPED: &str = "redirect_vlt_bundled_instance_skipped";
 
 /// The uuids of `deps` whose purl a vlt vendored ledger entry claims: a
@@ -85,7 +85,7 @@ pub(crate) async fn artifact_preflight(
 /// The origins the vlt heal treats as Socket-owned in the final lock: the
 /// patch server plus `--api-url`. Unlike discovery's
 /// [`crate::commands::hosted_unwind::patch_server_origins`], it also counts the
-/// `--api-url` origin (as it has since vlt support landed, #269), so a
+/// `--api-url` origin, so a
 /// setup serving the hosted tarballs from the API origin is healed too.
 /// Discovery-facing code must use the `hosted_unwind` helper, not this one.
 fn vlt_heal_origins(common: &crate::args::GlobalArgs) -> Vec<String> {
@@ -98,10 +98,6 @@ fn vlt_heal_origins(common: &crate::args::GlobalArgs) -> Vec<String> {
         .collect()
 }
 
-fn cleanup_disabled(common: &crate::args::GlobalArgs) -> bool {
-    common.dry_run || common.no_vlt_install_cleanup
-}
-
 /// How a heal ended, for the advisory wording.
 #[derive(Debug, Default, PartialEq, Eq)]
 struct HealTally {
@@ -146,7 +142,7 @@ async fn heal_targets(
     if stale.is_empty() {
         return tally;
     }
-    if cleanup_disabled(common) {
+    if common.dry_run || common.no_vlt_install_cleanup {
         tally.stale_left = stale.len();
         tally
             .stale_uuids
@@ -392,7 +388,7 @@ pub(crate) async fn heal_after_rewrite(
             out.stale_purls.insert(purl.clone());
         }
     }
-    // A bundled copy of a patched package (#471) has no lock node, so the
+    // A bundled copy of a patched package has no lock node, so the
     // redirect never reaches it: say so, and keep its purl out of the
     // in-run attestation (lockfile discovery contests it the same way).
     if inputs.final_lock.is_some() {
@@ -423,12 +419,8 @@ pub(crate) async fn heal_after_rewrite(
             out.stale_purls.insert(purl.clone());
         }
     }
-    // A heal implies a final lock, so the block above recorded the store;
-    // never report a heal without it.
-    if healed && out.healed_store.is_none() {
-        out.healed_store =
-            Some(socket_patch_core::vendor::vlt_bundled::bundled_copies(&common.cwd).await);
-    }
+    // Heal targets come from the final lock, so the block above saw it.
+    debug_assert!(!healed || out.healed_store.is_some());
     out
 }
 

crates/socket-patch-core/src/update/channel.rs

@@ -524,6 +524,21 @@ fn is_prefix_global_install(path: &Path) -> bool {
     }
 }
 
+impl InstallChannel {
+    /// A stable machine name for telemetry, unlike the human-facing
+    /// [`channel_label`] (`cargo install`, `the RubyGems launcher`).
+    pub fn id(self) -> &'static str {
+        match self {
+            InstallChannel::Standalone => "standalone",
+            InstallChannel::Npm => "npm",
+            InstallChannel::Pypi => "pypi",
+            InstallChannel::Cargo => "cargo",
+            InstallChannel::LauncherCache => "launcher_cache",
+            InstallChannel::Homebrew => "homebrew",
+        }
+    }
+}
+
 /// Short human label for refusal messages ("managed by npm"); see
 /// [`channel_label_for`] for the npm-family manager of a given path.
 pub fn channel_label(channel: InstallChannel) -> &'static str {
@@ -841,6 +856,20 @@ mod tests {
         );
     }
 
+    #[test]
+    fn channel_ids_are_the_telemetry_names() {
+        for (channel, id) in [
+            (InstallChannel::Standalone, "standalone"),
+            (InstallChannel::Npm, "npm"),
+            (InstallChannel::Pypi, "pypi"),
+            (InstallChannel::Cargo, "cargo"),
+            (InstallChannel::LauncherCache, "launcher_cache"),
+            (InstallChannel::Homebrew, "homebrew"),
+        ] {
+            assert_eq!(channel.id(), id);
+        }
+    }
+
     #[test]
     fn channel_label_covers_every_channel() {
         // Exact strings: these feed the "--update refused: managed by <label>"