From 8f1341e9d14f87541d018c9db45ea2b7535f7c73 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Mon, 29 Jun 2026 22:40:43 -0700 Subject: [PATCH] =?UTF-8?q?feat(cli):=20uninstall=20finds=20every=20copy?= =?UTF-8?q?=20=E2=80=94=20PATH/standard=20scan=20+=20Windows=20live-drive?= =?UTF-8?q?=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make `uffs --uninstall` discover and remove UFFS wherever it actually lives, across both platforms, while never touching a shared bin dir it does not own. - PATH + standard-dir scan (which-style, stat-only, no walk): finds copies that are neither running nor the invoking exe, folded into the main plan. Reuses the real channel/scope classifier (WinGet delegated, root-owned dirs flagged for sudo). - Retired/optional binary names (uffs-daemon, uffs-mcp, uffs_tui, …) swept wherever they sit in a root. - PATH safety gate: only a dedicated uffs*-only directory is offered for PATH removal; a shared ~/bin / ~/.local/bin is left alone (we never added it). - Windows deep sweep: ask UFFS itself for stray copies on the live drives, version them, and remove under a SEPARATE confirmation (a copy placed by the user might be among them). - Windows drive-coverage: before the sweep, check which NTFS drives the daemon indexes and offer to start it / index the missing ones for a complete sweep. Runs under --dry-run too (start/index are non-destructive); only deletions are withheld. Off Windows there is no daemon sweep (UFFS indexes offline captures there, not the live filesystem), so the deep-sweep subsystem is #[cfg(windows)]. Co-Authored-By: Claude Opus 4.8 --- .../src/commands/uninstall/analyze.rs | 317 +++++++++++++++++- .../src/commands/uninstall/coverage.rs | 75 +++++ .../src/commands/uninstall/effects.rs | 11 +- crates/uffs-cli/src/commands/uninstall/mod.rs | 118 +++++-- .../uffs-cli/src/commands/uninstall/plan.rs | 105 +++++- .../uffs-cli/src/commands/uninstall/remove.rs | 11 + .../uffs-cli/src/commands/uninstall/render.rs | 27 +- .../uffs-cli/src/commands/uninstall/sweep.rs | 58 +++- crates/uffs-cli/src/commands/update/mod.rs | 2 +- 9 files changed, 678 insertions(+), 46 deletions(-) create mode 100644 crates/uffs-cli/src/commands/uninstall/coverage.rs diff --git a/crates/uffs-cli/src/commands/uninstall/analyze.rs b/crates/uffs-cli/src/commands/uninstall/analyze.rs index 935ca49bf..c8d725e59 100644 --- a/crates/uffs-cli/src/commands/uninstall/analyze.rs +++ b/crates/uffs-cli/src/commands/uninstall/analyze.rs @@ -8,10 +8,118 @@ //! Side effects are confined to reading the environment (`current_exe`, `PATH`, //! `current_dir`, `SystemRoot`); nothing here mutates the system. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use super::resolve_order::Candidate; -use crate::commands::update::model::DetectionReport; +use crate::commands::update::model::{BinaryInfo, Channel, DetectionReport, InstallRoot}; + +/// Binary stems UFFS used in the past (retired names) or for optional members +/// (the TUI/GUI that moved to the products repo). None are in the current +/// `KNOWN_BINARIES`, but they linger in an install root from an old build, so +/// uninstall sweeps any that exist (idempotent — absent ones are skipped). +pub(crate) const EXTRA_BINARY_STEMS: &[&str] = &[ + "uffs-tui", // optional member (moved to uffs-products) + "uffs-gui", // optional member (moved to uffs-products) + "uffs-daemon", // retired -> uffsd + "uffs-mcp", // retired -> uffsmcp + "uffs-mcp-http", // retired -> uffsmcp (HTTP gateway) + "uffs_tui", // ancient underscore naming + "uffs_gui", // ancient underscore naming + "uffs_mft", // ancient underscore naming +]; + +/// Add any [`EXTRA_BINARY_STEMS`] that actually exist in an unmanaged / +/// dev-build root to that root's binary list, so uninstall sweeps retired and +/// optional names alongside the current set. `WinGet` roots are left untouched +/// (managed externally). Read-only stat; mutates only the in-memory report. +pub(crate) fn augment_with_extra_binaries(report: &mut DetectionReport) { + for root in &mut report.roots { + if matches!(root.channel, Channel::WinGet) { + continue; + } + for stem in EXTRA_BINARY_STEMS { + if root.binaries.iter().any(|binary| binary.name == *stem) { + continue; + } + if root.dir.join(extra_exe_name(stem)).is_file() { + root.binaries.push(BinaryInfo { + name: (*stem).to_owned(), + version: None, + }); + } + } + } +} + +/// The on-disk file name for a stem (`uffs-tui` -> `uffs-tui.exe` on Windows). +fn extra_exe_name(stem: &str) -> String { + if cfg!(windows) { + format!("{stem}.exe") + } else { + stem.to_owned() + } +} + +/// Scan `PATH` and the standard binary directories for UFFS family binaries and +/// add any directory holding one as an install root, so copies that are neither +/// running nor the invoking exe are still found and removed. Stat-only +/// (`which`-style) — never a filesystem walk. Cross-platform: on Windows it +/// complements the live-drive deep sweep; off Windows (where UFFS cannot index +/// the live filesystem) it is the primary way we find off-anchor copies. +pub(crate) fn augment_with_path_locations(report: &mut DetectionReport) { + add_roots_for_dirs(report, &candidate_bin_dirs()); +} + +/// Add each directory in `dirs` that holds a family binary as a root, skipping +/// any already present (deduplicated by canonical path). The classification +/// (channel / scope) reuses the primary detection's logic, so a `WinGet` copy +/// is still delegated and a machine-scope copy is still flagged for elevation. +fn add_roots_for_dirs(report: &mut DetectionReport, dirs: &[PathBuf]) { + let mut seen: Vec = report.roots.iter().map(|root| root.dir.clone()).collect(); + for dir in dirs { + let key = std::fs::canonicalize(dir).unwrap_or_else(|_| dir.clone()); + if seen.iter().any(|existing| existing == &key) { + continue; + } + let binaries = crate::commands::update::binaries::enumerate(&key); + if binaries.is_empty() { + continue; + } + let (channel, scope) = crate::commands::update::channel::classify(&key); + report.roots.push(InstallRoot { + dir: key.clone(), + channel, + scope, + anchored_by: Vec::new(), + binaries, + }); + seen.push(key); + } +} + +/// `PATH` entries plus the standard binary directories, in scan order. Each is +/// stat-checked for family binaries; non-existent ones are simply skipped. +fn candidate_bin_dirs() -> Vec { + let mut dirs = path_entries(); + if let Some(home) = home_dir() { + dirs.push(home.join("bin")); + dirs.push(home.join(".local").join("bin")); + dirs.push(home.join(".cargo").join("bin")); + } + #[cfg(not(windows))] + dirs.extend([ + PathBuf::from("/usr/local/bin"), + PathBuf::from("/opt/homebrew/bin"), + ]); + dirs +} + +/// The user's home directory (`HOME`, or `USERPROFILE` on Windows). +fn home_dir() -> Option { + std::env::var_os("HOME") + .or_else(|| std::env::var_os("USERPROFILE")) + .map(PathBuf::from) +} /// Flatten the detection report's roots × binaries into resolution candidates /// (one per discovered binary copy). @@ -66,3 +174,208 @@ pub(crate) fn search_dirs() -> Vec { pub(crate) fn path_entries() -> Vec { std::env::var_os("PATH").map_or_else(Vec::new, |path| std::env::split_paths(&path).collect()) } + +/// The `PATH` directories that are actually *safe* to drop on uninstall: an +/// unmanaged / dev-build UFFS root that is **dedicated** to UFFS (contains only +/// `uffs*` files). Shared bin dirs (`~/bin`, `~/.local/bin`, …) are +/// deliberately never returned: UFFS installs into pre-existing +/// OS/shell-default locations and never adds a PATH entry of its own, so it +/// must not suggest removing one — that directory belongs to the user's whole +/// toolchain, not to us. +pub(crate) fn removable_path_dirs( + report: &DetectionReport, + path_entries: &[PathBuf], +) -> Vec { + report + .roots + .iter() + .filter(|root| !matches!(root.channel, Channel::WinGet) && !root.binaries.is_empty()) + .map(|root| root.dir.clone()) + .filter(|dir| { + path_entries.iter().any(|entry| { + entry + .as_os_str() + .to_string_lossy() + .eq_ignore_ascii_case(&dir.as_os_str().to_string_lossy()) + }) + }) + .filter(|dir| is_uffs_exclusive_dir(dir)) + .collect() +} + +/// Whether `dir` is a dedicated UFFS directory — every entry's file name starts +/// with `uffs` (case-insensitive). A directory holding any non-UFFS file (a +/// shared `~/bin`) is not exclusive, and an unreadable or empty one is treated +/// as not exclusive (we never claim a PATH entry we cannot prove is ours). +fn is_uffs_exclusive_dir(dir: &Path) -> bool { + let Ok(entries) = std::fs::read_dir(dir) else { + return false; + }; + let mut saw_entry = false; + for entry in entries.flatten() { + saw_entry = true; + if !entry + .file_name() + .to_string_lossy() + .to_ascii_lowercase() + .starts_with("uffs") + { + return false; + } + } + saw_entry +} + +#[cfg(test)] +mod tests { + use super::augment_with_extra_binaries; + use crate::commands::update::model::{Channel, DetectionReport, InstallRoot, Scope}; + + #[test] + fn extra_binaries_present_on_disk_are_added_absent_ones_are_not() { + let dir = std::env::temp_dir().join(format!( + "uffs-extra-bin-test-{}-{}", + std::process::id(), + "retired" + )); + std::fs::create_dir_all(&dir).unwrap(); + // A retired name that exists on disk (use the platform file name). + let present = if cfg!(windows) { + "uffs-daemon.exe" + } else { + "uffs-daemon" + }; + std::fs::write(dir.join(present), b"x").unwrap(); + + let mut report = DetectionReport { + roots: vec![InstallRoot { + dir: dir.clone(), + channel: Channel::Unmanaged, + scope: Scope::User, + anchored_by: Vec::new(), + binaries: Vec::new(), + }], + running: Vec::new(), + }; + augment_with_extra_binaries(&mut report); + + let names: Vec<&str> = report + .roots + .first() + .expect("a root") + .binaries + .iter() + .map(|binary| binary.name.as_str()) + .collect(); + assert!( + names.contains(&"uffs-daemon"), + "a retired name present on disk must be swept: {names:?}" + ); + assert!( + !names.contains(&"uffs-tui"), + "an absent retired name must NOT be added" + ); + + std::fs::remove_dir_all(&dir).expect("cleanup temp dir"); + } + + #[test] + fn shared_bin_dir_is_not_path_removable_but_a_dedicated_one_is() { + use crate::commands::update::model::BinaryInfo; + + let pid = std::process::id(); + // A shared bin dir: a uffs binary living next to a foreign tool. + let shared = std::env::temp_dir().join(format!("uffs-path-shared-{pid}")); + std::fs::create_dir_all(&shared).unwrap(); + std::fs::write(shared.join("uffs"), b"x").unwrap(); + std::fs::write(shared.join("git"), b"x").unwrap(); // a non-UFFS tool + // A dedicated dir: only uffs* files. + let dedicated = std::env::temp_dir().join(format!("uffs-path-dedicated-{pid}")); + std::fs::create_dir_all(&dedicated).unwrap(); + std::fs::write(dedicated.join("uffs"), b"x").unwrap(); + std::fs::write(dedicated.join("uffsd"), b"x").unwrap(); + + let bin = |dir: &std::path::Path| InstallRoot { + dir: dir.to_path_buf(), + channel: Channel::Unmanaged, + scope: Scope::User, + anchored_by: Vec::new(), + binaries: vec![BinaryInfo { + name: "uffs".to_owned(), + version: None, + }], + }; + let report = DetectionReport { + roots: vec![bin(&shared), bin(&dedicated)], + running: Vec::new(), + }; + let on_path = vec![shared.clone(), dedicated.clone()]; + let removable = super::removable_path_dirs(&report, &on_path); + + assert!( + !removable.contains(&shared), + "a shared bin dir (other tools present) must NOT be offered for PATH removal" + ); + assert!( + removable.contains(&dedicated), + "a dedicated uffs-only dir IS safe to offer for PATH removal" + ); + + std::fs::remove_dir_all(&shared).expect("cleanup shared"); + std::fs::remove_dir_all(&dedicated).expect("cleanup dedicated"); + } + + #[test] + fn path_scan_adds_a_dir_with_a_family_binary_once() { + let dir = std::env::temp_dir().join(format!("uffs-pathscan-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let name = if cfg!(windows) { "uffs.exe" } else { "uffs" }; + std::fs::write(dir.join(name), b"x").unwrap(); + + let mut report = DetectionReport { + roots: Vec::new(), + running: Vec::new(), + }; + // Pass the dir twice: it must still be added exactly once (deduped). + super::add_roots_for_dirs(&mut report, &[dir.clone(), dir.clone()]); + + let key = std::fs::canonicalize(&dir).unwrap(); + let matching: Vec<_> = report.roots.iter().filter(|root| root.dir == key).collect(); + assert_eq!( + matching.len(), + 1, + "the dir is added once: {:?}", + report.roots + ); + assert!( + matching + .first() + .expect("one matching root") + .binaries + .iter() + .any(|bin| bin.name == "uffs"), + "the discovered family binary is recorded" + ); + + std::fs::remove_dir_all(&dir).expect("cleanup"); + } + + #[test] + fn winget_roots_are_left_untouched() { + let mut report = DetectionReport { + roots: vec![InstallRoot { + dir: std::env::temp_dir(), + channel: Channel::WinGet, + scope: Scope::User, + anchored_by: Vec::new(), + binaries: Vec::new(), + }], + running: Vec::new(), + }; + augment_with_extra_binaries(&mut report); + assert!( + report.roots.first().expect("a root").binaries.is_empty(), + "winget roots must not be augmented" + ); + } +} diff --git a/crates/uffs-cli/src/commands/uninstall/coverage.rs b/crates/uffs-cli/src/commands/uninstall/coverage.rs new file mode 100644 index 000000000..6874aa4a9 --- /dev/null +++ b/crates/uffs-cli/src/commands/uninstall/coverage.rs @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2025-2026 SKY, LLC. + +//! Windows deep-sweep drive coverage for `uffs --uninstall`. +//! +//! Before the live cross-drive search, make sure the daemon is running and +//! indexes every NTFS drive, **offering** to start it and index the missing +//! drives so the sweep is actually complete. Windows-only: off Windows UFFS +//! indexes offline MFT captures, not the live filesystem, so there is no live +//! drive coverage to ensure. +//! +//! Best-effort throughout: any RPC failure leaves coverage as-is and the sweep +//! proceeds against whatever is currently indexed. + +#![cfg(windows)] + +use anyhow::Result; +use uffs_client::connect_sync::UffsClientSync; +use uffs_mft::platform::{DriveLetter, detect_ntfs_drives}; + +/// How long to wait for newly-requested drives to finish loading before the +/// sweep runs. A best-effort cap — a slow HDD index may still be in flight. +const INDEX_WAIT: core::time::Duration = core::time::Duration::from_secs(120); + +/// Ensure the daemon covers every NTFS drive before the deep sweep, offering to +/// start it and index the missing drives. `confirm` prompts the user (returns +/// their yes/no). Returns `Ok(())` whether or not coverage was completed — the +/// caller sweeps regardless. +/// +/// # Errors +/// +/// Propagates only a failure of the `confirm` callback itself; daemon/RPC +/// failures are swallowed (best-effort coverage). +pub(crate) fn ensure_drive_coverage(confirm: &mut dyn FnMut(&str) -> Result) -> Result<()> { + let all = detect_ntfs_drives(); + if all.is_empty() { + return Ok(()); + } + // `connect()` auto-starts the daemon if it is not already running. + let Ok(mut client) = UffsClientSync::connect() else { + // Could not reach or start a daemon: nothing to cover, sweep as-is. + return Ok(()); + }; + let indexed: Vec = client + .drives() + .map(|response| { + response + .drives + .into_iter() + .map(|drive| drive.letter) + .collect() + }) + .unwrap_or_default(); + let missing: Vec = all + .into_iter() + .filter(|drive| !indexed.contains(drive)) + .collect(); + if missing.is_empty() { + return Ok(()); + } + let list = missing + .iter() + .map(|drive| format!("{drive}:")) + .collect::>() + .join(", "); + let prompt = format!( + "\nThe deep sweep searches every indexed drive. Not yet indexed: {list}.\n\ + Index {list} now for a complete sweep? [y/N] " + ); + if confirm(&prompt)? && client.load_drive_letters(&missing, false).is_ok() { + // Give the freshly-requested drives a chance to load before we search. + let _ready = client.await_ready(INDEX_WAIT); + } + Ok(()) +} diff --git a/crates/uffs-cli/src/commands/uninstall/effects.rs b/crates/uffs-cli/src/commands/uninstall/effects.rs index 0b20ab4fe..d550f5ac3 100644 --- a/crates/uffs-cli/src/commands/uninstall/effects.rs +++ b/crates/uffs-cli/src/commands/uninstall/effects.rs @@ -49,6 +49,11 @@ impl Effects for SystemEffects { winget_uninstall(package_id, scope) } + #[cfg(windows)] + fn delete_file(&mut self, path: &Path) -> Result<()> { + remove_file_if_present(path).with_context(|| format!("removing {}", path.display())) + } + fn remove_dir(&mut self, path: &Path) -> Result<()> { remove_dir_if_present(path).with_context(|| format!("removing {}", path.display())) } @@ -80,14 +85,16 @@ fn remove_path_entry_impl(dir: &Path) -> Result<()> { /// Unix: the shell owns PATH (rc files), so editing it automatically is unsafe. /// Write a manual-cleanup hint to stderr instead (genuinely fallible, so no -/// `unnecessary_wraps`). +/// `unnecessary_wraps`). Only reached for a dir we vetted as UFFS-dedicated, so +/// removing its PATH line is safe — a shared bin dir never gets here. #[cfg(not(windows))] fn remove_path_entry_impl(dir: &Path) -> Result<()> { use std::io::Write as _; writeln!( std::io::stderr(), - " note: remove {} from your shell PATH manually (e.g. ~/.profile or ~/.zshrc)", + " note: {} was a UFFS-only directory; if you added it to your shell PATH \ + (~/.profile or ~/.zshrc), you can remove that line now", dir.display() ) .context("writing PATH cleanup hint") diff --git a/crates/uffs-cli/src/commands/uninstall/mod.rs b/crates/uffs-cli/src/commands/uninstall/mod.rs index fdbed7580..9f2772bc3 100644 --- a/crates/uffs-cli/src/commands/uninstall/mod.rs +++ b/crates/uffs-cli/src/commands/uninstall/mod.rs @@ -13,6 +13,8 @@ mod analyze; mod args; +#[cfg(windows)] +mod coverage; mod effects; mod inventory; mod journal; @@ -20,6 +22,9 @@ mod plan; mod remove; mod render; mod resolve_order; +/// Deep-sweep for stray copies on the live drives — Windows-only (off Windows +/// UFFS indexes offline captures, not the live filesystem). +#[cfg(windows)] mod sweep; mod verify; @@ -50,13 +55,22 @@ pub(crate) fn run_uninstall(args: &[String]) -> Result<()> { } // M1 analysis: reuse the self-update Phase-A detection for the binary - // resolution table, then inventory the non-binary artifacts. - let report = crate::commands::update::detect(); + // resolution table, then sweep in any retired/optional binary names that + // linger from old installs, then inventory the non-binary artifacts. + let mut report = crate::commands::update::detect(); + // Scan PATH + the standard bin dirs for copies that are neither running nor + // the invoking exe (which-style, stat-only — no filesystem walk), then sweep + // in any retired/optional binary names that linger from old installs. + analyze::augment_with_path_locations(&mut report); + analyze::augment_with_extra_binaries(&mut report); let candidates = analyze::build_candidates(&report); let resolved = resolve_order::group_and_resolve(&candidates, &analyze::search_dirs()); let inventory = inventory::collect(); - // M2: turn the analysis into an ordered removal plan (read-only). - let removal_plan = plan::build_plan(&report, &inventory, &parsed, &analyze::path_entries()); + // M2: turn the analysis into an ordered removal plan (read-only). Only PATH + // entries pointing at a *dedicated* UFFS dir are offered for removal — a + // shared bin dir (~/bin, ~/.local/bin) we never created is left alone. + let removable_path = analyze::removable_path_dirs(&report, &analyze::path_entries()); + let removal_plan = plan::build_plan(&report, &inventory, &parsed, &removable_path); if parsed.json { render::print_json(&resolved, &inventory, &removal_plan); @@ -67,15 +81,13 @@ pub(crate) fn run_uninstall(args: &[String]) -> Result<()> { render::print_inventory(&inventory); render::print_plan(&removal_plan); - // M7 deep sweep: while the daemon is still up, ask UFFS itself for stray - // family files elsewhere on the indexed drives. Read-only; reported only. - if !parsed.no_deep_sweep { - let known = plan_dirs(&removal_plan); - let mut search = sweep::DaemonSearch; - if let Ok(strays) = sweep::find_strays(&mut search, &known) { - render::print_strays(&strays); - } - } + // M7 deep sweep: ask UFFS itself for stray family files elsewhere on the + // live drives, version them, and build a separate plan removed only under + // its own confirmation (one may be a copy the user placed themselves). This + // is Windows-only — off Windows UFFS indexes offline captures, not the live + // filesystem, so PATH/standard-location copies (already folded into the main + // plan above) are all we can find. + let stray_plan = platform_stray_plan(&parsed, &removal_plan); if parsed.dry_run { print_dry_run_footer(); @@ -91,13 +103,16 @@ pub(crate) fn run_uninstall(args: &[String]) -> Result<()> { bail!("uninstall needs Administrator for the items listed above; re-run elevated"); } - if removal_plan.is_empty() { + // Nothing to remove at all: no install in the standard locations, and the + // deep sweep found no strays. + if removal_plan.is_empty() && stray_plan.is_empty() { return Ok(()); } // M4 consent (U-21): unless --yes, require explicit confirmation (default No) - // before any destructive effect. - if !parsed.assume_yes && !confirm_removal()? { + // before any destructive effect. Declining aborts the whole uninstall. + if !removal_plan.is_empty() && !parsed.assume_yes && !confirm("\nProceed with removal? [y/N] ")? + { print_aborted(); return Ok(()); } @@ -112,8 +127,29 @@ pub(crate) fn run_uninstall(args: &[String]) -> Result<()> { // M4 execute (U-40..42): run the ordered plan against the live effects sink, // best-effort. The outcome reports what was removed and what failed. let mut effects = effects::SystemEffects::new(); - let outcome = remove::execute(&removal_plan, &mut effects); - render::print_outcome(&outcome); + if !removal_plan.is_empty() { + let outcome = remove::execute(&removal_plan, &mut effects); + render::print_outcome(&outcome); + } + + // Strays found outside the standard locations get a SEPARATE confirmation + // (one may be a copy the user placed themselves), then are removed + // best-effort. `--yes` covers both prompts. Windows-only — see + // `platform_stray_plan`; off Windows `stray_plan` is always empty. + #[cfg(windows)] + if !stray_plan.is_empty() { + let approved = parsed.assume_yes + || confirm(&format!( + "\nAlso remove the {} file(s) found elsewhere (listed above)? [y/N] ", + stray_plan.item_count() + ))?; + if approved { + let stray_outcome = remove::execute(&stray_plan, &mut effects); + render::print_outcome(&stray_outcome); + } else { + render::print_strays_kept(); + } + } // M8 self-delete (U-80): the running uffs.exe (+ uffs-update.exe) cannot // delete themselves in place; schedule a deferred delete. If even scheduling @@ -169,18 +205,56 @@ fn plan_dirs(plan: &RemovalPlan) -> Vec { | PlanTarget::DelegateWinget { dir, .. } | PlanTarget::RemovePathEntry { dir } => Some(dir.clone()), PlanTarget::DeleteDir { path, .. } => Some(path.clone()), + #[cfg(windows)] + PlanTarget::DeleteFile { .. } => None, PlanTarget::StopProcess { .. } | PlanTarget::RemoveService { .. } => None, }) .collect() } -/// Prompt for confirmation before any removal. Default (empty / anything but -/// `y`/`yes`) is **No**. +/// Build the deep-sweep stray plan for the current platform. +/// +/// Windows: ensure the daemon covers every NTFS drive (offering to start it / +/// index the missing drives), then ask UFFS for stray copies outside the known +/// roots and present them for a separate confirmation. The coverage offer runs +/// under `--dry-run` too — starting the daemon and indexing drives are +/// non-destructive, and a dry run should preview the *complete* picture; only +/// the deletions themselves are withheld (the caller returns before executing). +#[cfg(windows)] +fn platform_stray_plan(parsed: &UninstallArgs, removal_plan: &RemovalPlan) -> RemovalPlan { + if parsed.no_deep_sweep { + return RemovalPlan::default(); + } + // Ensuring coverage may start the daemon / index drives — non-destructive, + // so it runs even under --dry-run to make the preview accurate. + if let Err(err) = coverage::ensure_drive_coverage(&mut |prompt| confirm(prompt)) { + render::print_journal_warning(&err); + } + let known = plan_dirs(removal_plan); + let mut search = sweep::DaemonSearch; + let strays = sweep::version_strays(sweep::find_strays(&mut search, &known).unwrap_or_default()); + render::print_strays(&strays); + plan::build_stray_plan(&strays) +} + +/// Build the deep-sweep stray plan for the current platform. +/// +/// Off Windows the daemon indexes offline captures, not the live filesystem, so +/// it cannot find local stray binaries; PATH/standard-location copies are +/// already folded into the main plan, leaving no separate stray phase. +#[cfg(not(windows))] +fn platform_stray_plan(_parsed: &UninstallArgs, _removal_plan: &RemovalPlan) -> RemovalPlan { + RemovalPlan::default() +} + +/// Prompt for a yes/no confirmation. Default (empty / anything but `y`/`yes`) +/// is **No**. `prompt` is written verbatim (caller includes any leading +/// newline). #[expect(clippy::print_stdout, reason = "interactive CLI prompt")] -fn confirm_removal() -> Result { +fn confirm(prompt: &str) -> Result { use std::io::Write as _; - print!("\nProceed with removal? [y/N] "); + print!("{prompt}"); std::io::stdout() .flush() .context("flushing the confirmation prompt")?; diff --git a/crates/uffs-cli/src/commands/uninstall/plan.rs b/crates/uffs-cli/src/commands/uninstall/plan.rs index 5fae0e4cf..4633358d0 100644 --- a/crates/uffs-cli/src/commands/uninstall/plan.rs +++ b/crates/uffs-cli/src/commands/uninstall/plan.rs @@ -17,6 +17,8 @@ use std::path::{Path, PathBuf}; use super::args::{UninstallArgs, UninstallScope}; use super::inventory::{ArtifactKind, BrokerServiceState, Inventory}; +#[cfg(windows)] +use super::sweep::StrayHit; use crate::commands::update::model::{Channel, DetectionReport, InstallRoot, Scope}; /// The `WinGet` package id UFFS publishes under. @@ -67,6 +69,16 @@ pub(crate) enum PlanTarget { /// The PATH entry to remove. dir: PathBuf, }, + /// Delete a single stray UFFS file the deep sweep found outside the known + /// roots. Confirmed separately from the main plan. Windows-only (the deep + /// sweep does not run off Windows). + #[cfg(windows)] + DeleteFile { + /// Absolute path of the stray file. + path: PathBuf, + /// Parsed `--version` if it is a probeable binary (display only). + version: Option, + }, } impl PlanTarget { @@ -79,6 +91,8 @@ impl PlanTarget { Self::DelegateWinget { .. } => "delegate-winget", Self::DeleteDir { .. } => "delete-dir", Self::RemovePathEntry { .. } => "remove-path-entry", + #[cfg(windows)] + Self::DeleteFile { .. } => "delete-file", } } @@ -101,6 +115,11 @@ impl PlanTarget { ), Self::DeleteDir { path, label } => format!("{label} ({})", path.display()), Self::RemovePathEntry { dir } => format!("PATH entry {}", dir.display()), + #[cfg(windows)] + Self::DeleteFile { path, version } => version.as_ref().map_or_else( + || path.display().to_string(), + |ver| format!("{} (v{ver})", path.display()), + ), } } } @@ -174,13 +193,16 @@ impl RemovalPlan { } } -/// Build the ordered removal plan from the analysis + flags. `path_entries` is -/// the live PATH (used to offer removal of entries that point at a UFFS root). +/// Build the ordered removal plan from the analysis + flags. +/// `removable_path_dirs` is the already-vetted set of PATH directories safe to +/// drop — dedicated UFFS roots only (see +/// [`super::analyze::removable_path_dirs`]). A shared bin dir is never in it, +/// so PATH cleanup never touches the user's general toolchain location. pub(crate) fn build_plan( report: &DetectionReport, inventory: &Inventory, args: &UninstallArgs, - path_entries: &[PathBuf], + removable_path_dirs: &[PathBuf], ) -> RemovalPlan { let mut groups: Vec = Vec::new(); @@ -240,18 +262,19 @@ pub(crate) fn build_plan( .collect(); push_group(&mut groups, "Data / cache / config", dirs, args.scope); - // 5. PATH entries that point at a removed unmanaged/dev root — provably UFFS - // (the exact dir we just deleted), so safe to drop. WinGet roots are managed - // by winget; never touched here. Skipped under --no-path. + // 5. PATH entries that point at a removed unmanaged/dev root that is + // *dedicated* to UFFS (only uffs* files) — provably ours, so safe to drop. A + // shared bin dir (~/bin, ~/.local/bin) is filtered out upstream and never + // appears here. WinGet roots are managed by winget. Skipped under --no-path. if !args.no_path { let path_items: Vec = report .roots .iter() .filter(|root| !root.binaries.is_empty() && !matches!(root.channel, Channel::WinGet)) .filter(|root| { - path_entries + removable_path_dirs .iter() - .any(|entry| paths_equal_ignore_case(entry, &root.dir)) + .any(|dir| paths_equal_ignore_case(dir, &root.dir)) }) .map(|root| { let machine = matches!(root.scope, Scope::Machine); @@ -275,6 +298,37 @@ pub(crate) fn build_plan( RemovalPlan { groups } } +/// Build a one-group plan for the stray files the deep sweep found outside the +/// known roots. Presented + confirmed **separately** from the main plan (a copy +/// the user placed themselves might be among them). Each item is a best-effort +/// single-file delete; none require elevation up front — a protected location +/// simply fails best-effort and is reported. Windows-only (the deep sweep does +/// not run off Windows). +#[cfg(windows)] +pub(crate) fn build_stray_plan(strays: &[StrayHit]) -> RemovalPlan { + if strays.is_empty() { + return RemovalPlan::default(); + } + let items: Vec = strays + .iter() + .map(|stray| PlanItem { + target: PlanTarget::DeleteFile { + path: stray.path.clone(), + version: stray.version.clone(), + }, + needs_elevation: false, + scope: ItemScope::Any, + bytes: 0, + }) + .collect(); + RemovalPlan { + groups: vec![PlanGroup { + title: "Found elsewhere (deep sweep)", + items, + }], + } +} + /// Case-insensitive path equality (Windows file systems + PATH entries vary in /// case; a redundant exact match is what we require before touching PATH). fn paths_equal_ignore_case(left: &Path, right: &Path) -> bool { @@ -370,6 +424,8 @@ const fn scope_admits(requested: UninstallScope, item: ItemScope) -> bool { mod tests { use std::path::PathBuf; + #[cfg(windows)] + use super::build_stray_plan; use super::{PlanTarget, RemovalPlan, build_plan}; use crate::commands::uninstall::args::{UninstallArgs, UninstallScope}; use crate::commands::uninstall::inventory::{ @@ -540,6 +596,35 @@ mod tests { ))); } + #[test] + #[cfg(windows)] + fn stray_plan_is_one_group_of_unprivileged_delete_file_items() { + use crate::commands::uninstall::sweep::StrayHit; + + assert!(build_stray_plan(&[]).is_empty(), "no strays -> empty plan"); + let strays = vec![ + StrayHit { + path: PathBuf::from("/home/me/Downloads/uffs"), + version: Some("0.5.0".to_owned()), + }, + StrayHit { + path: PathBuf::from("/tmp/x_compact.uffs"), + version: None, + }, + ]; + let plan = build_stray_plan(&strays); + assert_eq!(plan.item_count(), 2); + assert!( + plan.items() + .all(|item| matches!(item.target, PlanTarget::DeleteFile { .. })), + "every stray item is a DeleteFile" + ); + assert!( + !plan.requires_elevation(), + "strays never require up-front elevation (best-effort on failure)" + ); + } + #[test] fn path_entry_matching_a_removed_root_is_offered_and_respects_no_path() { let report = DetectionReport { @@ -547,7 +632,9 @@ mod tests { running: Vec::new(), }; let inv = inventory(BrokerServiceState::Absent, 1024); - // Case-insensitive match of a PATH entry to the removed root → offered. + // The 4th arg is the already-vetted removable-dir set; a case-insensitive + // match to the removed root → offered. (Exclusivity vetting is tested in + // analyze::removable_path_dirs; here we exercise build_plan's emission.) let on_path = [PathBuf::from(r"c:\users\me\bin")]; let offered = build_plan(&report, &inv, &UninstallArgs::default(), &on_path); assert!(has_target(&offered, |target| matches!( diff --git a/crates/uffs-cli/src/commands/uninstall/remove.rs b/crates/uffs-cli/src/commands/uninstall/remove.rs index e66adefd3..cebc1a02d 100644 --- a/crates/uffs-cli/src/commands/uninstall/remove.rs +++ b/crates/uffs-cli/src/commands/uninstall/remove.rs @@ -27,6 +27,10 @@ pub(crate) trait Effects { fn remove_service(&mut self, service: &str) -> Result<()>; /// Delete the named binary stems inside `dir` (absent ones are a no-op). fn delete_binaries(&mut self, dir: &Path, stems: &[String]) -> Result<()>; + /// Delete one stray file by absolute path (absent is a no-op). Used for the + /// Windows deep-sweep hits found outside the known roots. + #[cfg(windows)] + fn delete_file(&mut self, path: &Path) -> Result<()>; /// Hand a `WinGet`-managed root to `winget uninstall`. fn delegate_winget(&mut self, package_id: &str, scope: Scope) -> Result<()>; /// Recursively delete a directory (absent is a no-op). @@ -101,6 +105,8 @@ fn dispatch(target: &PlanTarget, effects: &mut dyn Effects) -> Result<()> { PlanTarget::StopProcess { component, pid } => effects.stop_process(component, *pid), PlanTarget::RemoveService { service } => effects.remove_service(service), PlanTarget::DeleteBinaries { dir, stems } => effects.delete_binaries(dir, stems), + #[cfg(windows)] + PlanTarget::DeleteFile { path, .. } => effects.delete_file(path), PlanTarget::DelegateWinget { package_id, scope, .. } => effects.delegate_winget(package_id, *scope), @@ -148,6 +154,11 @@ mod tests { .push(format!("delete_binaries:{}:{}", dir.display(), stems.len())); Ok(()) } + #[cfg(windows)] + fn delete_file(&mut self, path: &Path) -> Result<()> { + self.calls.push(format!("delete_file:{}", path.display())); + Ok(()) + } fn delegate_winget(&mut self, package_id: &str, _scope: Scope) -> Result<()> { self.calls.push(format!("delegate_winget:{package_id}")); Ok(()) diff --git a/crates/uffs-cli/src/commands/uninstall/render.rs b/crates/uffs-cli/src/commands/uninstall/render.rs index 00ae2f7f9..8a3e9d84a 100644 --- a/crates/uffs-cli/src/commands/uninstall/render.rs +++ b/crates/uffs-cli/src/commands/uninstall/render.rs @@ -11,6 +11,8 @@ use super::inventory::Inventory; use super::plan::RemovalPlan; use super::remove::{ItemStatus, RemovalOutcome}; use super::resolve_order::{ResolutionState, StemResolution}; +#[cfg(windows)] +use super::sweep::StrayHit; /// Print the discovered-binary resolution table: for each stem, every copy in /// OS search order, with the one a bare command runs flagged ACTIVE. @@ -115,22 +117,33 @@ pub(crate) fn print_elevation_refusal(plan: &RemovalPlan) { ); } -/// Print stray UFFS-named files the deep sweep found outside the known roots. -/// These are listed for review only, never auto-removed. +/// Print stray UFFS files the deep sweep found outside the known roots, with +/// versions. These are removed only under a separate second confirmation (a +/// copy the user placed themselves might be among them). Windows-only. +#[cfg(windows)] #[expect(clippy::print_stdout, reason = "CLI user-facing output")] -pub(crate) fn print_strays(strays: &[std::path::PathBuf]) { +pub(crate) fn print_strays(strays: &[StrayHit]) { if strays.is_empty() { return; } println!( - "\nStray UFFS-named files found elsewhere (NOT removed — review and delete\n\ - manually if they are unwanted; one may be a copy you placed yourself):" + "\nAlso found elsewhere (deep sweep), outside the standard install locations.\n\ + These are removed only if you confirm a separate prompt below (one may be a\n\ + copy you placed yourself):" ); - for path in strays { - println!(" {}", path.display()); + for stray in strays { + let version = stray.version.as_deref().unwrap_or("-"); + println!(" {version:<9} {}", stray.path.display()); } } +/// Note that the user declined to remove the deep-sweep strays. Windows-only. +#[cfg(windows)] +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +pub(crate) fn print_strays_kept() { + println!("Left the file(s) found elsewhere in place."); +} + /// Note that a prior uninstall was interrupted and this run completes it. #[expect(clippy::print_stdout, reason = "CLI user-facing output")] pub(crate) fn print_resumed_note() { diff --git a/crates/uffs-cli/src/commands/uninstall/sweep.rs b/crates/uffs-cli/src/commands/uninstall/sweep.rs index 3cbaee871..7282bca79 100644 --- a/crates/uffs-cli/src/commands/uninstall/sweep.rs +++ b/crates/uffs-cli/src/commands/uninstall/sweep.rs @@ -3,8 +3,9 @@ //! Deep sweep for `uffs --uninstall` (task U-70/U-71): use UFFS's own search to //! find stray family files anywhere on the indexed drives, beyond the known -//! install roots. Strays are **reported for review, never auto-removed** — a -//! `uffs.exe` under `Downloads` might be the user's own copy (design §8). +//! install roots. Strays are versioned and removed only under a **separate, +//! explicit second confirmation** (a `uffs.exe` under `Downloads` might be the +//! user's own copy, so they never ride the main plan's single yes — design §8). //! //! The dedup logic is pure + unit-tested against a fake [`Search`]; the live //! backend ([`DaemonSearch`]) is best-effort (no daemon ⇒ no hits, never a @@ -35,6 +36,41 @@ pub(crate) trait Search { fn find(&mut self, pattern: &str) -> Result>; } +/// A stray family file found outside the known install roots, with its parsed +/// `--version` for binaries (data files like caches carry `None`). +#[derive(Debug, Clone)] +pub(crate) struct StrayHit { + /// Absolute path of the stray file. + pub(crate) path: PathBuf, + /// Parsed `--version`, or `None` for a data file or an unreadable version. + pub(crate) version: Option, +} + +/// Attach a version to each stray: probe `--version` on the executable hits and +/// leave UFFS data files (`*_compact.uffs`, `*_usn.cursor`) unversioned. No +/// daemon needed — each binary is run directly (the same probe the standard +/// detection uses). +pub(crate) fn version_strays(paths: Vec) -> Vec { + paths + .into_iter() + .map(|path| { + let version = is_probeable_binary(&path) + .then(|| crate::commands::update::binaries::probe_version(&path)) + .flatten(); + StrayHit { path, version } + }) + .collect() +} + +/// Whether `path` names an executable we can run `--version` on, rather than a +/// UFFS data file (`*.uffs` cache / `*.cursor`) that has no version. +fn is_probeable_binary(path: &Path) -> bool { + !path + .extension() + .and_then(|ext| ext.to_str()) + .is_some_and(|ext| ext.eq_ignore_ascii_case("uffs") || ext.eq_ignore_ascii_case("cursor")) +} + /// Find stray family files across every pattern, dropping any hit already under /// a directory the plan handles. Sorted + de-duplicated. pub(crate) fn find_strays(search: &mut dyn Search, known_dirs: &[PathBuf]) -> Result> { @@ -119,7 +155,7 @@ mod tests { use anyhow::Result; - use super::{Search, extract_paths, find_strays}; + use super::{Search, extract_paths, find_strays, version_strays}; /// Returns the same hits for every pattern (the dedup must collapse them). struct FakeSearch(Vec); @@ -155,6 +191,22 @@ mod tests { assert_eq!(strays.len(), 1, "sibling dir must not be filtered"); } + #[test] + fn data_files_are_not_probed_for_a_version() { + // Cache/cursor data files have no version and must not be executed; a + // (nonexistent) binary path probes to None rather than panicking. + let strays = version_strays(vec![ + PathBuf::from("/x/drive_c_compact.uffs"), + PathBuf::from("/x/journal_usn.cursor"), + PathBuf::from("/x/definitely-not-here/uffs"), + ]); + assert_eq!(strays.len(), 3); + assert!( + strays.iter().all(|stray| stray.version.is_none()), + "data files (and an absent binary) carry no version" + ); + } + #[test] fn extracts_path_fields_recursively() { let value = serde_json::json!({ diff --git a/crates/uffs-cli/src/commands/update/mod.rs b/crates/uffs-cli/src/commands/update/mod.rs index ae9d51ac5..547f221d6 100644 --- a/crates/uffs-cli/src/commands/update/mod.rs +++ b/crates/uffs-cli/src/commands/update/mod.rs @@ -23,7 +23,7 @@ mod acquire; mod apply; pub(crate) mod binaries; -mod channel; +pub(crate) mod channel; mod doctor; pub(crate) mod model; pub(crate) mod procinfo;