sweep/2026-10-04: 50 fixes, 15 refactors (36→43 tests)

.gitignore

@@ -2,3 +2,5 @@
 /dist
 .superpowers/
 .DS_Store
+.omc/
+.wrangler/

src/cleanup.rs

@@ -72,11 +72,6 @@ pub fn classify(path: &Path) -> Option<(&'static str, &'static str)> {
             "Docker Desktop Data",
             "Docker images, volumes, build cache. Prefer `docker system prune -a` for finer-grained cleanup.",
         ),
-        (
-            "Library/Containers/com.apple.iCloud.iCloudDrive",
-            "iCloud Drive evictable cache",
-            "Local cache of iCloud-stored files. Files reload from iCloud on next access.",
-        ),
         (
             ".cargo/registry",
             "Cargo registry cache",
@@ -89,7 +84,9 @@ pub fn classify(path: &Path) -> Option<(&'static str, &'static str)> {
         ),
     ];
     for &(suffix, label, desc) in SUFFIX_RULES {
-        if s.ends_with(suffix) {
+        // Require a path-component boundary: either the path IS the suffix or
+        // it ends with /<suffix>, preventing "MyLibrary/Caches" matching "Library/Caches".
+        if s == suffix || s.ends_with(&format!("/{suffix}")) {
             return Some((label, desc));
         }
     }
@@ -111,17 +108,17 @@ pub fn classify(path: &Path) -> Option<(&'static str, &'static str)> {
             )),
             ".gradle" => Some((
                 "Gradle caches",
-                "Gradle redownloads dependencies on next build.",
+                "Gradle redownloads dependencies on next build. Warning: gradle.properties (which may hold credentials) will also be deleted.",
             )),
             ".m2" => Some((
                 "Maven local repo",
-                "Maven local artifacts cache; rebuilt on next dependency resolve.",
+                "Maven cache — settings.xml (credentials) will also be deleted.",
             )),
             "DerivedData" => Some((
                 "DerivedData (loose)",
                 "Xcode build artifacts found outside the standard location.",
             )),
-            "target" => Some((
+            "target" if path.parent().map(|p| p.join("Cargo.toml").exists()).unwrap_or(false) => Some((
                 "Rust target/",
                 "`cargo build` recreates these. Often the largest single dir on a Rust dev machine.",
             )),
@@ -137,10 +134,6 @@ pub fn classify(path: &Path) -> Option<(&'static str, &'static str)> {
                 ".cache directories",
                 "App- and tool-specific caches. Generally regenerable.",
             )),
-            ".Trash" | ".Trashes" => Some((
-                "Trash",
-                "Files in the macOS Trash. Empty when you're sure you don't need them.",
-            )),
             _ => None,
         };
     }
@@ -150,13 +143,121 @@ pub fn classify(path: &Path) -> Option<(&'static str, &'static str)> {
 
 /// Walks the tree once and returns all directories matching cleanup rules,
 /// sorted by size descending. Doesn't recurse into matched directories.
+///
+/// The scan root itself is never classified — only its descendants are
+/// examined. This prevents the "Trash" button from targeting the entire
+/// scanned directory when the root path matches a cleanup pattern (e.g.
+/// the user scanned ~/Library/Caches directly).
 pub fn find_candidates(tree: &FileTree, root: NodeId) -> Vec<CleanupCandidate> {
     let mut candidates = Vec::new();
-    walk(tree, root, &mut candidates);
+    for &child in tree.node(root).children() {
+        walk(tree, child, &mut candidates);
+    }
     candidates.sort_by(|a, b| b.size.cmp(&a.size));
     candidates
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::fs;
+
+    #[test]
+    fn cargo_target_with_cargo_toml_is_classified() {
+        let dir = std::env::temp_dir().join(format!("mac_dir_stat_test_cargo_{}", std::process::id()));
+        fs::create_dir_all(&dir).unwrap();
+        fs::write(dir.join("Cargo.toml"), "[package]").unwrap();
+        let target = dir.join("target");
+        let (cat, _) = classify(&target).expect("should classify");
+        assert_eq!(cat, "Rust target/");
+        fs::remove_dir_all(&dir).ok();
+    }
+
+    #[test]
+    fn target_without_cargo_toml_is_not_classified() {
+        let dir = std::env::temp_dir().join(format!("mac_dir_stat_test_notcargo_{}", std::process::id()));
+        fs::create_dir_all(&dir).unwrap();
+        // No Cargo.toml in parent
+        let target = dir.join("target");
+        assert!(classify(&target).is_none(), "should not classify without Cargo.toml");
+        fs::remove_dir_all(&dir).ok();
+    }
+
+    #[test]
+    fn library_caches_is_classified() {
+        let path = std::path::Path::new("/Users/someone/Library/Caches");
+        let (cat, _) = classify(path).expect("should classify");
+        assert_eq!(cat, "Application Caches");
+    }
+
+    #[test]
+    fn gradle_description_warns_about_credentials() {
+        let path = std::path::Path::new("/Users/someone/.gradle");
+        let (_, desc) = classify(path).expect("should classify");
+        assert!(desc.contains("credentials") || desc.contains("gradle.properties"),
+            "description should warn about credentials");
+    }
+
+    #[test]
+    fn find_candidates_1mib_threshold() {
+        use std::time::SystemTime;
+        let mut tree = FileTree::new();
+        let root = tree.root();
+        // Build: root → "Users" → "pavol" → "Library" → "Caches"
+        let users = tree.add_dir(root, b"Users", false, SystemTime::now(), 1);
+        let pavol = tree.add_dir(users, b"pavol", false, SystemTime::now(), 2);
+        let lib   = tree.add_dir(pavol, b"Library", false, SystemTime::now(), 3);
+        let caches = tree.add_dir(lib, b"Caches", false, SystemTime::now(), 4);
+
+        // Under 1 MiB → should not surface.
+        tree.add_file(caches, b"tiny.dat", 512 * 1024, None, SystemTime::now(), 5);
+        tree.compute_sizes();
+        assert!(find_candidates(&tree, root).is_empty(), "below 1 MiB threshold");
+
+        // Over 1 MiB → should surface exactly once.
+        tree.add_file(caches, b"big.dat", 1024 * 1024 + 1, None, SystemTime::now(), 5);
+        tree.compute_sizes();
+        let candidates = find_candidates(&tree, root);
+        assert_eq!(candidates.len(), 1);
+        assert_eq!(candidates[0].category, "Application Caches");
+    }
+
+    #[test]
+    fn find_candidates_no_recursion_into_match() {
+        use std::time::SystemTime;
+        let mut tree = FileTree::new();
+        let root = tree.root();
+        // root → "Users" → "pavol" → "Library" → "Caches" → "node_modules"
+        let users   = tree.add_dir(root, b"Users", false, SystemTime::now(), 1);
+        let pavol   = tree.add_dir(users, b"pavol", false, SystemTime::now(), 2);
+        let lib     = tree.add_dir(pavol, b"Library", false, SystemTime::now(), 3);
+        let caches  = tree.add_dir(lib, b"Caches", false, SystemTime::now(), 4);
+        let nm      = tree.add_dir(caches, b"node_modules", false, SystemTime::now(), 5);
+        tree.add_file(nm, b"pkg.js", 5 * 1024 * 1024, None, SystemTime::now(), 6);
+        tree.compute_sizes();
+        // Caches matches first; node_modules inside must not produce a second candidate.
+        let candidates = find_candidates(&tree, root);
+        assert_eq!(candidates.len(), 1, "no recursion into matched dir");
+        assert_eq!(candidates[0].category, "Application Caches");
+    }
+
+    #[test]
+    fn find_candidates_skips_dead_node() {
+        use std::time::SystemTime;
+        let mut tree = FileTree::new();
+        let root = tree.root();
+        let users   = tree.add_dir(root, b"Users", false, SystemTime::now(), 1);
+        let pavol   = tree.add_dir(users, b"pavol", false, SystemTime::now(), 2);
+        let lib     = tree.add_dir(pavol, b"Library", false, SystemTime::now(), 3);
+        let caches  = tree.add_dir(lib, b"Caches", false, SystemTime::now(), 4);
+        tree.add_file(caches, b"big.dat", 10 * 1024 * 1024, None, SystemTime::now(), 5);
+        tree.compute_sizes();
+        tree.remove_node(caches);
+        let candidates = find_candidates(&tree, root);
+        assert!(candidates.is_empty(), "dead node must not surface");
+    }
+}
+
 fn walk(tree: &FileTree, id: NodeId, out: &mut Vec<CleanupCandidate>) {
     if !tree.is_alive(id) {
         return;

src/flywheel.rs

@@ -17,7 +17,7 @@
 //! Opt-out: `MACDIRSTAT_NO_TELEMETRY=1` in the environment, or the user toggle
 //! persisted in `telemetry.txt`. With no config baked, the SDK ships dark.
 
-use std::path::PathBuf;
+use crate::state::state_dir;
 use std::process::{Command, Stdio};
 use std::sync::OnceLock;
 use std::time::{SystemTime, UNIX_EPOCH};
@@ -43,10 +43,6 @@ pub struct Flywheel {
     session_id: String,
 }
 
-fn state_dir() -> Option<PathBuf> {
-    std::env::var_os("HOME")
-        .map(|h| PathBuf::from(h).join("Library/Application Support/mac-dir-stat"))
-}
 
 /// Resolve `(url, publishable_key)` from runtime env first, then a compile-time bake.
 /// Returns `None` when neither is present — the ships-dark default.
@@ -72,11 +68,12 @@ pub fn telemetry_opted_out() -> bool {
     ) {
         return true;
     }
+    // Absent file = opted out; only "enabled" written by explicit opt-in counts.
     state_dir()
         .map(|d| d.join("telemetry.txt"))
         .and_then(|p| std::fs::read_to_string(p).ok())
-        .map(|s| s.trim() == "disabled")
-        .unwrap_or(false)
+        .map(|s| s.trim() != "enabled")
+        .unwrap_or(true)
 }
 
 /// Persist the user's telemetry choice so it survives restarts.
@@ -85,10 +82,12 @@ pub fn set_telemetry_opt_out(opted_out: bool) {
     if std::fs::create_dir_all(&dir).is_err() {
         return;
     }
-    let _ = std::fs::write(
+    if let Err(e) = std::fs::write(
         dir.join("telemetry.txt"),
         if opted_out { "disabled" } else { "enabled" },
-    );
+    ) {
+        eprintln!("mac-dir-stat: failed to persist telemetry choice: {e}");
+    }
 }
 
 /// 16 random bytes from `/dev/urandom`, or a time-seeded fallback.
@@ -278,7 +277,7 @@ impl Flywheel {
     /// so a slow or unreachable network can't stall a UI frame.
     fn post(&self, table: &str, body: &str) {
         let endpoint = format!("{}/rest/v1/{}", self.url, table);
-        let _ = Command::new("curl")
+        let result = Command::new("curl")
             .args([
                 "-s",
                 "-m",
@@ -301,6 +300,10 @@ impl Flywheel {
             .stdout(Stdio::null())
             .stderr(Stdio::null())
             .spawn();
+        // Reap child on completion so it does not sit as a zombie.
+        if let Ok(mut child) = result {
+            std::thread::spawn(move || { let _ = child.wait(); });
+        }
     }
 }
 
@@ -385,6 +388,17 @@ mod tests {
         assert!(!fw.is_enabled());
     }
 
+    #[test]
+    fn no_telemetry_env_var_opts_out() {
+        // Serialize env mutation so parallel test workers don't race.
+        static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
+        let _guard = ENV_LOCK.lock().unwrap();
+        std::env::set_var("MACDIRSTAT_NO_TELEMETRY", "1");
+        let result = telemetry_opted_out();
+        std::env::remove_var("MACDIRSTAT_NO_TELEMETRY");
+        assert!(result, "MACDIRSTAT_NO_TELEMETRY=1 should opt out");
+    }
+
     #[test]
     fn taxonomy_matches_ts_client() {
         assert!(TAXONOMY.contains(&"conversion"));

src/platform/fda.rs

@@ -6,19 +6,18 @@
 //! again. Without FDA the scanner still works; it just can't see TCC-protected
 //! locations, which it already prunes to avoid a wall of permission prompts.
 
+use crate::state::state_dir;
 use std::path::PathBuf;
 use std::process::Command;
 
-fn state_dir() -> Option<PathBuf> {
-    std::env::var_os("HOME")
-        .map(|h| PathBuf::from(h).join("Library/Application Support/mac-dir-stat"))
-}
-
 /// Open System Settings → Privacy & Security → Full Disk Access.
 pub fn open_settings() {
-    let _ = Command::new("open")
+    if let Ok(mut child) = Command::new("open")
         .arg("x-apple.systempreferences:com.apple.preference.security?Privacy_AllFiles")
-        .spawn();
+        .spawn()
+    {
+        std::thread::spawn(move || { let _ = child.wait(); });
+    }
 }
 
 /// Best-effort check for whether the app already has Full Disk Access: try to

src/platform/finder.rs

@@ -1,23 +1,28 @@
 use std::path::Path;
 use std::process::Command;
 
+/// Spawn `cmd` and wait for it on a background thread so it doesn't linger as a zombie.
+fn spawn_reaped(cmd: &mut Command) {
+    if let Ok(mut child) = cmd.spawn() {
+        std::thread::spawn(move || {
+            let _ = child.wait();
+        });
+    }
+}
+
 /// Reveal a path in Finder (selects it in its containing folder).
 pub fn reveal_in_finder(path: &Path) {
-    let _ = Command::new("open").arg("-R").arg(path).spawn();
+    spawn_reaped(Command::new("open").arg("-R").arg(path));
 }
 
 /// Open a file in its default app, or a folder in Finder.
 pub fn open_path(path: &Path) {
-    let _ = Command::new("open").arg(path).spawn();
+    spawn_reaped(Command::new("open").arg(path));
 }
 
 /// Open a Terminal window at `dir` (or a file's containing folder).
 pub fn open_terminal_at(dir: &Path) {
-    let _ = Command::new("open")
-        .arg("-a")
-        .arg("Terminal")
-        .arg(dir)
-        .spawn();
+    spawn_reaped(Command::new("open").arg("-a").arg("Terminal").arg(dir));
 }
 
 /// Open the Finder "Get Info" window for a path (WinDirStat's Properties).
@@ -32,5 +37,5 @@ pub fn get_info(path: &Path) {
          end tell",
         escaped
     );
-    let _ = Command::new("osascript").arg("-e").arg(script).spawn();
+    spawn_reaped(Command::new("osascript").arg("-e").arg(script));
 }

src/platform/trash.rs

@@ -5,19 +5,19 @@ pub fn move_to_trash(path: &Path) -> Result<(), String> {
     trash::delete(path).map_err(|e| format!("Failed to move to trash: {}", e))
 }
 
-/// Empties the user's Trash via Finder. We've already shown our own
-/// confirmation, so we use `without warning` to skip Finder's.
+/// Empties the user's Trash by spawning `osascript` as a background process.
+/// Returns immediately after spawn — success only means the process started,
+/// not that the trash was emptied.  Finder performs the actual work.
 pub fn empty_trash() -> Result<(), String> {
-    let status = Command::new("osascript")
+    Command::new("osascript")
         .args([
             "-e",
             "tell application \"Finder\" to empty trash without warning",
         ])
-        .status()
-        .map_err(|e| format!("osascript failed to start: {}", e))?;
-    if status.success() {
-        Ok(())
-    } else {
-        Err(format!("osascript exited with status {}", status))
-    }
+        .stdin(std::process::Stdio::null())
+        .stdout(std::process::Stdio::null())
+        .stderr(std::process::Stdio::null())
+        .spawn()
+        .map(|mut child| { std::thread::spawn(move || { let _ = child.wait(); }); })
+        .map_err(|e| format!("osascript failed to start: {}", e))
 }

src/scanner/mod.rs

@@ -18,7 +18,22 @@ pub enum ScanProgress {
 }
 
 pub fn scan(root: PathBuf, tx: Sender<ScanProgress>) {
+    scan_impl(root, tx, true);
+}
+
+/// Like `scan` but omits synthetic Free Space / Hidden nodes — used for
+/// partial subtree refreshes where the target is not the tree root.
+pub fn scan_subtree(root: PathBuf, tx: Sender<ScanProgress>) {
+    scan_impl(root, tx, false);
+}
+
+fn scan_impl(root: PathBuf, tx: Sender<ScanProgress>, inject_synthetics: bool) {
     std::thread::spawn(move || {
-        walk::walk_directory(root, tx);
+        let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            walk::walk_directory(root, tx.clone(), inject_synthetics);
+        }));
+        if result.is_err() {
+            let _ = tx.send(ScanProgress::Error("Scanner panicked".into()));
+        }
     });
 }