From 10b51021b2e261f51ad9d22a572f1d265956c482 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 18:17:05 +0000 Subject: [PATCH 1/5] Plans refuse to trash your own folders Plan validation checks each trash against the inventory's only-yours folders (Documents, Desktop, Pictures, Movies, Music, Mail, Messages under home, and any Photos library). A path that is one, sits inside one, or holds one gets an error naming the folder, so the plan can't be approved. Before, only the human approval stopped a model proposing ~/Documents. PLACES moves to a shared constant so the Cleanup window and plan checks use one list. validate() keeps its signature; validate_with() takes the home folder for tests. Pavol chose "Refuse such plans in validation" on 2026-11-03. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017mKbacqC4bPDSYsaK9dVdu --- CHANGELOG.md | 5 ++++ README.md | 5 +++- docs/SIGNATURE.md | 1 + src/cleanup.rs | 65 ++++++++++++++++++++++++++++++++++------ src/plan.rs | 50 ++++++++++++++++++++++++++++--- src/server/mcp.rs | 2 +- tests/inventory_tests.rs | 33 ++++++++++++++++++++ tests/server_tests.rs | 27 +++++++++++++++++ 8 files changed, 173 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e2ba03..36523e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,11 @@ Release notes for the GitHub Release: [`docs/release-notes/v0.7.0.md`](docs/rele folders, and loose files, hidden space and the rest as unnamed sums; no file names or lists) in `~/Library/Application Support/mac-dir-stat/history.json`, six per folder and twenty folders at most. It never leaves the Mac. +- **Plans can't trash your own folders.** A plan proposed over the web page, REST or MCP that + would trash Documents, Desktop, Pictures, Movies, Music, Mail, Messages, a Photos library, + anything inside one, or a folder that holds one is refused with an error naming the folder, and + can't be approved. Before, only your approval stopped it. + ### Changed - **New look: the survey register.** Book typography on paper: Source Serif 4 (bundled, SIL Open Font License) for all text with a real italic for notes, ink on paper, hairline rules, and one diff --git a/README.md b/README.md index e53f941..c687c75 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,10 @@ file is, so the token stays out of log files. The page works on a phone in light disks and free space, the largest folders and files, cleanup groups, and a find box (`node_modules`, `.venv`, `*.dmg`, …). Tick items, choose **Move to Trash** or **Move to **, and create a **plan**. The plan lists every item with its size, destination, errors and warnings. Nothing runs -until you press **Approve**. +until you press **Approve**. A plan that would trash one of your own folders (Documents, Desktop, +Pictures, Movies, Music, Mail, Messages, a Photos library), anything inside one, or a folder that +holds one is refused: the error names the folder, and the plan can't be approved. This is the same +list as the Cleanup window's *Only you have these* column. It listens on `127.0.0.1` only, so a phone cannot reach it until you pass `--bind`. Over Tailscale, bind to the Mac's Tailscale IP (`--bind 100.x.y.z`; `tailscale ip -4` shows it) and diff --git a/docs/SIGNATURE.md b/docs/SIGNATURE.md index 2e6102f..0282ba3 100644 --- a/docs/SIGNATURE.md +++ b/docs/SIGNATURE.md @@ -145,6 +145,7 @@ scale bar, names set in the parcel. | 2026-10-30 | 3. The disk over time | 3B small multiples of past scans | Pavol | Store a small summary per scan (totals and top folders, no file lists), show at most 6, and keep scans on the Mac: nothing leaves the machine. | | 2026-10-30 | Visual signature | Survey register (serif, paper, oxblood accent) | Pavol | Replaces the violet, dark-first theme. Keep a dark variant that follows the system setting. | | 2026-10-30 | All products | A move approved for one product may be reused by another | Pavol | "Dont just blindly copy the pattern though." | +| 2026-11-03 | Safety: plan checks | Refuse such plans in validation | Pavol | Chosen on the decision page (card mac-dir-stat.o0, "Should the MCP server refuse cleanup plans that include one of your own folders?", 11:52 UTC), no note; relayed by the orchestrating session. Plans that would trash an only-yours folder, or anything in it, are refused naming the folder. | --- diff --git a/src/cleanup.rs b/src/cleanup.rs index 2072f51..90de5e7 100644 --- a/src/cleanup.rs +++ b/src/cleanup.rs @@ -54,6 +54,62 @@ pub fn is_rebuildable(path: &Path) -> bool { path.ancestors().any(|a| classify(a).is_some()) } +/// Where personal data lives, relative to the home folder, and its label. +/// The right-hand column of the inventory, and what plans may not trash. +pub const PLACES: &[(&str, &str)] = &[ + ("Documents", "Documents"), + ("Desktop", "Desktop"), + ("Pictures", "Pictures"), + ("Movies", "Movies"), + ("Music", "Music"), + ("Library/Mail", "Mail"), + ("Library/Messages", "Messages"), +]; + +/// Why trashing `path` would touch something only the user has, or `None`. +/// Covers the [`PLACES`] under `home`, anything inside them, a folder that +/// holds one of them, and any Photos library. Compared case-insensitively, +/// as APFS does. +pub fn only_yours_reason(path: &Path, home: Option<&Path>) -> Option { + let lower = |p: &Path| -> Vec { + p.components() + .map(|c| c.as_os_str().to_string_lossy().to_lowercase()) + .collect() + }; + let shown = |p: &Path| match home.and_then(|h| p.strip_prefix(h).ok()) { + Some(rel) if rel.as_os_str().is_empty() => "~".to_string(), + Some(rel) => format!("~/{}", rel.display()), + None => p.display().to_string(), + }; + let never = "MacDirStat never offers your own files for deletion; remove this from the \ + plan, and trash it yourself in Finder if you mean to."; + let p = lower(path); + if p.iter().any(|c| c.ends_with(".photoslibrary")) { + return Some(format!("{} is in a Photos library, one of your own. {never}", shown(path))); + } + let home = home?; + for &(rel, label) in PLACES { + let place = home.join(rel); + let q = lower(&place); + if p.starts_with(&q) { + let what = if p.len() == q.len() { + format!("{} is your own {label} folder.", shown(path)) + } else { + format!("{} is inside {}, your own {label} folder.", shown(path), shown(&place)) + }; + return Some(format!("{what} {never}")); + } + if q.starts_with(&p) { + return Some(format!( + "{} holds {}, your own {label} folder. {never}", + shown(path), + shown(&place) + )); + } + } + None +} + /// A folder the app never offers for deletion: it holds things only the /// user has. Shown so they know where the space is. #[derive(Debug, Clone)] @@ -74,15 +130,6 @@ pub fn find_only_yours( home: Option<&Path>, rebuildable: &[CleanupCandidate], ) -> Vec { - const PLACES: &[(&str, &str)] = &[ - ("Documents", "Documents"), - ("Desktop", "Desktop"), - ("Pictures", "Pictures"), - ("Movies", "Movies"), - ("Music", "Music"), - ("Library/Mail", "Mail"), - ("Library/Messages", "Messages"), - ]; let Some(home) = home else { return Vec::new() }; let mut out: Vec = Vec::new(); for &(rel, label) in PLACES { diff --git a/src/plan.rs b/src/plan.rs index 5135c32..ade41fc 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -140,6 +140,13 @@ pub fn parse_ops(v: &Value) -> Result, String> { /// Checks one op against the filesystem (sizes, rules, free space). The /// returned op carries the normalized path: that is what is shown and run. pub fn validate(op: &Op) -> OpState { + validate_with(op, crate::relocate::home().as_deref()) +} + +/// [`validate`] for a given home folder. A trash that touches one of the +/// user's own folders (`cleanup::only_yours_reason`) is an error, so the +/// plan can't be approved. +pub fn validate_with(op: &Op, home: Option<&std::path::Path>) -> OpState { let mut op = op.clone(); let normalized = crate::relocate::normalize(&op.path); if let Ok(p) = &normalized { @@ -155,11 +162,15 @@ pub fn validate(op: &Op) -> OpState { }; match &op.action { Action::Trash => match normalized { - Err(e) => st.errors.push(e), + // Even a path that can't be resolved is refused by name if it + // points into one of the user's own folders. + Err(e) => st + .errors + .push(crate::cleanup::only_yours_reason(&op.path, home).unwrap_or(e)), Ok(p) => { - if let Some(r) = - crate::relocate::protected_reason(&p, crate::relocate::home().as_deref()) - { + if let Some(r) = crate::relocate::protected_reason(&p, home) { + st.errors.push(r); + } else if let Some(r) = crate::cleanup::only_yours_reason(&p, home) { st.errors.push(r); } else if std::fs::symlink_metadata(&p).is_err() { st.errors.push(format!("{} does not exist", p.display())); @@ -438,4 +449,35 @@ mod tests { assert!(s.get(keep).is_some(), "pending plans are never pruned"); assert_eq!(s.list().last().unwrap().id, 251); } + + /// A model proposing over MCP that ~/Documents, or anything in it, goes + /// to the Trash: the plan is refused, naming the folder. + #[test] + fn validate_refuses_your_own_folders_and_their_contents() { + let home = std::env::temp_dir().join(format!("mds-plan-home-{}", std::process::id())); + let sub = home.join("Documents/taxes"); + std::fs::create_dir_all(&sub).unwrap(); + std::fs::write(sub.join("2025.pdf"), b"x").unwrap(); + std::fs::create_dir_all(home.join("code/node_modules")).unwrap(); + let home = home.canonicalize().unwrap(); + let check = |p: PathBuf| validate_with(&Op { path: p, action: Action::Trash }, Some(&home)); + + for p in [home.join("Documents"), home.join("Documents/taxes")] { + let st = check(p.clone()); + assert_eq!(st.errors.len(), 1, "{}: {:?}", p.display(), st.errors); + assert!(st.errors[0].contains("~/Documents"), "{}", st.errors[0]); + assert!(st.errors[0].contains("your own"), "{}", st.errors[0]); + assert_eq!(st.size, 0, "nothing measured for a refused path"); + } + let mut s = PlanStore::default(); + let ops = vec![ + check(home.join("code/node_modules")), + check(home.join("Documents/taxes/2025.pdf")), + ]; + assert!(ops[0].errors.is_empty(), "{:?}", ops[0].errors); + let id = s.create(ops, "", "mcp").id; + assert!(!s.get(id).unwrap().valid(), "one refused op refuses the plan"); + assert!(s.approve(id).is_err()); + std::fs::remove_dir_all(&home).unwrap(); + } } diff --git a/src/server/mcp.rs b/src/server/mcp.rs index f6a75f9..9cdab50 100644 --- a/src/server/mcp.rs +++ b/src/server/mcp.rs @@ -33,7 +33,7 @@ pub fn tools() -> Value { {"name": "find", "description": "Find items whose name matches a glob (* and ?), e.g. node_modules, .venv, target, *.dmg. Matches are not searched inside. Returns sizes, modified times and a total.", "inputSchema": {"type": "object", "required": ["pattern"], "properties": {"pattern": {"type": "string"}, "limit": {"type": "integer"}}}}, {"name": "cleanup_suggestions", "description": "Known regenerable data (caches, build output, simulators...) grouped by category with totals.", "inputSchema": none}, - {"name": "propose_plan", "description": "Propose trash/move operations. Returns the plan with per-op validation errors and warnings. It does NOT run anything: the user reviews and approves it in MacDirStat.", + {"name": "propose_plan", "description": "Propose trash/move operations. Returns the plan with per-op validation errors and warnings. It does NOT run anything: the user reviews and approves it in MacDirStat. Trashing the user's own folders (Documents, Desktop, Pictures, Movies, Music, Mail, Messages, Photos libraries) or anything in them is refused.", "inputSchema": {"type": "object", "required": ["ops"], "properties": {"ops": {"type": "array", "items": op}, "note": {"type": "string", "description": "Why, in one or two sentences, shown to the user"}}}}, {"name": "list_plans", "description": "All plans with their status (pending, approved, done, failed, rejected).", "inputSchema": none}, {"name": "get_plan", "description": "One plan with per-op results.", diff --git a/tests/inventory_tests.rs b/tests/inventory_tests.rs index 81c1fbe..18fe894 100644 --- a/tests/inventory_tests.rs +++ b/tests/inventory_tests.rs @@ -215,3 +215,36 @@ fn click(ctx: &egui::Context, app: &mut App, frame: &mut eframe::Frame, pos: egu let _ = ctx.run(input, |ctx| eframe::App::update(app, ctx, frame)); } } + +// Plans proposed over the server or MCP: an only-yours folder is refused. + +#[test] +fn a_plan_may_not_trash_your_own_folders_or_anything_in_them() { + use mac_dir_stat::cleanup::only_yours_reason; + let home = Some(Path::new("/Users/p")); + let docs = only_yours_reason(Path::new("/Users/p/Documents"), home).expect("Documents"); + assert!(docs.contains("~/Documents") && docs.contains("your own"), "{docs}"); + let sub = only_yours_reason(Path::new("/Users/p/Documents/tax/2025.pdf"), home) + .expect("inside Documents"); + assert!(sub.contains("~/Documents/tax/2025.pdf") && sub.contains("Documents"), "{sub}"); + // APFS is case-insensitive: ~/documents is the same folder. + assert!(only_yours_reason(Path::new("/Users/p/documents/x"), home).is_some()); + assert!(only_yours_reason(Path::new("/Users/p/Library/Mail/V10"), home).is_some()); + assert!( + only_yours_reason(Path::new("/Volumes/Ext/Old.photoslibrary/originals"), home).is_some(), + "a Photos library anywhere" + ); + // A folder that holds one of them is refused too. + let lib = only_yours_reason(Path::new("/Users/p/Library"), home).expect("holds Mail"); + assert!(lib.contains("Mail"), "{lib}"); + // Rebuildable things, and look-alike names, are not affected. + for ok in [ + "/Users/p/code/app/node_modules", + "/Users/p/Library/Caches", + "/Users/p/DocumentsOld", + "/Users/q/Documents", + ] { + assert_eq!(only_yours_reason(Path::new(ok), home), None, "{ok}"); + } + assert_eq!(only_yours_reason(Path::new("/Users/p/Documents"), None), None); +} diff --git a/tests/server_tests.rs b/tests/server_tests.rs index 36ecf76..5af4ba4 100644 --- a/tests/server_tests.rs +++ b/tests/server_tests.rs @@ -554,3 +554,30 @@ fn a_poisoned_lock_does_not_take_the_server_down() { ); std::fs::remove_dir_all(root).unwrap(); } + +/// Journey: a model connected over MCP proposes trashing ~/Documents and a +/// folder inside it. The plan comes back refused, naming the folder, and +/// Pavol's approve can't run it. Nothing on disk is touched. +#[test] +fn mcp_plan_touching_your_own_folders_is_refused() { + let root = fixture("onlyyours"); + let runs = Arc::new(AtomicUsize::new(0)); + let h = start(&root, runs.clone()); + let home = mac_dir_stat::relocate::home().expect("a home folder"); + for path in [home.join("Documents"), home.join("Documents/Taxes 2025")] { + let (_, r) = mcp( + &h, + json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": "propose_plan", "arguments": {"note": "free space", "ops": [{"action": "trash", "path": path}]}}}), + ); + let plan: Value = + serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap(); + assert_eq!(plan["valid"], false, "{plan}"); + let msg = plan["ops"][0]["errors"][0].as_str().unwrap_or(""); + assert!(msg.contains("~/Documents") && msg.contains("your own"), "{msg}"); + let id = plan["id"].as_u64().unwrap(); + let (code, _) = call(&h, "POST", &format!("/api/v1/plans/{id}/approve"), None); + assert_eq!(code, 409, "an invalid plan can't be approved"); + } + std::thread::sleep(std::time::Duration::from_millis(100)); + assert_eq!(runs.load(Ordering::SeqCst), 0); +} From 20eed3d79f52501503ab78dde1d47ab9c4335b43 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 18:18:16 +0000 Subject: [PATCH 2/5] Plan check: a rebuildable folder inside your own (node_modules under Documents) may still go Matches the inventory, which lists it on the rebuildable side. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017mKbacqC4bPDSYsaK9dVdu --- src/cleanup.rs | 8 +++++++- tests/inventory_tests.rs | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cleanup.rs b/src/cleanup.rs index 90de5e7..65dae73 100644 --- a/src/cleanup.rs +++ b/src/cleanup.rs @@ -69,7 +69,8 @@ pub const PLACES: &[(&str, &str)] = &[ /// Why trashing `path` would touch something only the user has, or `None`. /// Covers the [`PLACES`] under `home`, anything inside them, a folder that /// holds one of them, and any Photos library. Compared case-insensitively, -/// as APFS does. +/// as APFS does. Something a cleanup rule can rebuild, inside one of them, +/// is not refused (it is in the inventory's left column). pub fn only_yours_reason(path: &Path, home: Option<&Path>) -> Option { let lower = |p: &Path| -> Vec { p.components() @@ -91,6 +92,11 @@ pub fn only_yours_reason(path: &Path, home: Option<&Path>) -> Option { for &(rel, label) in PLACES { let place = home.join(rel); let q = lower(&place); + // A rebuildable folder inside one (a project's node_modules under + // Documents) is on the other side of the inventory, and may go. + if p.len() > q.len() && p.starts_with(&q) && is_rebuildable(path) { + continue; + } if p.starts_with(&q) { let what = if p.len() == q.len() { format!("{} is your own {label} folder.", shown(path)) diff --git a/tests/inventory_tests.rs b/tests/inventory_tests.rs index 18fe894..2a9582f 100644 --- a/tests/inventory_tests.rs +++ b/tests/inventory_tests.rs @@ -237,8 +237,11 @@ fn a_plan_may_not_trash_your_own_folders_or_anything_in_them() { // A folder that holds one of them is refused too. let lib = only_yours_reason(Path::new("/Users/p/Library"), home).expect("holds Mail"); assert!(lib.contains("Mail"), "{lib}"); - // Rebuildable things, and look-alike names, are not affected. + // Rebuildable things, and look-alike names, are not affected; nor is a + // rebuildable folder inside Documents (the inventory's left column). for ok in [ + "/Users/p/Documents/thesis/node_modules", + "/Users/p/Documents/thesis/node_modules/x/y.js", "/Users/p/code/app/node_modules", "/Users/p/Library/Caches", "/Users/p/DocumentsOld", From 8367da4a9a3b372049ee5c2ab7a3189a4237003d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 18:23:49 +0000 Subject: [PATCH 3/5] Plan check: review fixes - Links: check the path as asked and as resolved, and each folder at its real location, so ~/Movies moved to a disk and left as a link stays protected. - A folder that holds a Photos library is refused (bounded search, three levels, no links followed). - The build-output exemption covers project build rules only; caches, logs, .gradle and .m2 inside your folders stay refused. - Docs say what the code does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017mKbacqC4bPDSYsaK9dVdu --- CHANGELOG.md | 5 ++- README.md | 6 ++- docs/SIGNATURE.md | 2 +- src/cleanup.rs | 96 ++++++++++++++++++++++++++++++++++++----------- src/plan.rs | 50 +++++++++++++++++++++++- src/server/mcp.rs | 2 +- 6 files changed, 132 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36523e1..1f2af62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,8 +31,9 @@ Release notes for the GitHub Release: [`docs/release-notes/v0.7.0.md`](docs/rele - **Plans can't trash your own folders.** A plan proposed over the web page, REST or MCP that would trash Documents, Desktop, Pictures, Movies, Music, Mail, Messages, a Photos library, - anything inside one, or a folder that holds one is refused with an error naming the folder, and - can't be approved. Before, only your approval stopped it. + anything inside one (also through a link to another disk), or a folder that holds one is refused + with an error naming the folder, and can't be approved. Before, only your approval stopped it. + A project's build output inside them (`node_modules`, `target/`, `.next/`, …) may still go. ### Changed - **New look: the survey register.** Book typography on paper: Source Serif 4 (bundled, SIL Open diff --git a/README.md b/README.md index c687c75..04dda77 100644 --- a/README.md +++ b/README.md @@ -127,8 +127,10 @@ disks and free space, the largest folders and files, cleanup groups, and a find `.venv`, `*.dmg`, …). Tick items, choose **Move to Trash** or **Move to **, and create a **plan**. The plan lists every item with its size, destination, errors and warnings. Nothing runs until you press **Approve**. A plan that would trash one of your own folders (Documents, Desktop, -Pictures, Movies, Music, Mail, Messages, a Photos library), anything inside one, or a folder that -holds one is refused: the error names the folder, and the plan can't be approved. This is the same +Pictures, Movies, Music, Mail, Messages, a Photos library), anything inside one (also when the +folder was moved to another disk and left as a link), or a folder that holds one is refused: the +error names the folder, and the plan can't be approved. A project's build output inside one, such +as `node_modules` under Documents, may still go. This is the same list as the Cleanup window's *Only you have these* column. It listens on `127.0.0.1` only, so a phone cannot reach it until you pass `--bind`. Over diff --git a/docs/SIGNATURE.md b/docs/SIGNATURE.md index 0282ba3..570a11d 100644 --- a/docs/SIGNATURE.md +++ b/docs/SIGNATURE.md @@ -145,7 +145,7 @@ scale bar, names set in the parcel. | 2026-10-30 | 3. The disk over time | 3B small multiples of past scans | Pavol | Store a small summary per scan (totals and top folders, no file lists), show at most 6, and keep scans on the Mac: nothing leaves the machine. | | 2026-10-30 | Visual signature | Survey register (serif, paper, oxblood accent) | Pavol | Replaces the violet, dark-first theme. Keep a dark variant that follows the system setting. | | 2026-10-30 | All products | A move approved for one product may be reused by another | Pavol | "Dont just blindly copy the pattern though." | -| 2026-11-03 | Safety: plan checks | Refuse such plans in validation | Pavol | Chosen on the decision page (card mac-dir-stat.o0, "Should the MCP server refuse cleanup plans that include one of your own folders?", 11:52 UTC), no note; relayed by the orchestrating session. Plans that would trash an only-yours folder, or anything in it, are refused naming the folder. | +| 2026-11-03 | Safety: plan checks | Refuse such plans in validation | Pavol | Chosen on the decision page (card mac-dir-stat.o0, "Should the MCP server refuse cleanup plans that include one of your own folders?", 11:52 UTC), no note; relayed by the orchestrating session. Plans that would trash an only-yours folder, anything in it (also through a link) or a folder holding one are refused, naming the folder. Reversible call on top (not Pavol's): a project's build output inside them, such as `node_modules` under Documents, stays allowed, as the inventory lists it on the rebuildable side. | --- diff --git a/src/cleanup.rs b/src/cleanup.rs index 65dae73..9295c9a 100644 --- a/src/cleanup.rs +++ b/src/cleanup.rs @@ -82,35 +82,87 @@ pub fn only_yours_reason(path: &Path, home: Option<&Path>) -> Option { Some(rel) => format!("~/{}", rel.display()), None => p.display().to_string(), }; - let never = "MacDirStat never offers your own files for deletion; remove this from the \ - plan, and trash it yourself in Finder if you mean to."; let p = lower(path); if p.iter().any(|c| c.ends_with(".photoslibrary")) { - return Some(format!("{} is in a Photos library, one of your own. {never}", shown(path))); + return Some(format!("{} is in a Photos library, one of your own. {NEVER}", shown(path))); } let home = home?; for &(rel, label) in PLACES { let place = home.join(rel); - let q = lower(&place); - // A rebuildable folder inside one (a project's node_modules under - // Documents) is on the other side of the inventory, and may go. - if p.len() > q.len() && p.starts_with(&q) && is_rebuildable(path) { - continue; - } - if p.starts_with(&q) { - let what = if p.len() == q.len() { - format!("{} is your own {label} folder.", shown(path)) - } else { - format!("{} is inside {}, your own {label} folder.", shown(path), shown(&place)) - }; - return Some(format!("{what} {never}")); + // Where the folder really lives too: ~/Movies may be a link to an + // external disk (Move to disk leaves one), and plans see the target. + let real = place.canonicalize().ok().filter(|r| *r != place); + for q in std::iter::once(&place).chain(real.as_ref()) { + let q = lower(q); + // A project's build output inside one (node_modules under + // Documents) is on the inventory's rebuildable side, and may go. + if p.len() > q.len() && p.starts_with(&q) && is_build_output(path) { + continue; + } + if p.starts_with(&q) { + let what = if p.len() == q.len() { + format!("{} is your own {label} folder.", shown(path)) + } else { + format!("{} is inside {}, your own {label} folder.", shown(path), shown(&place)) + }; + return Some(format!("{what} {NEVER}")); + } + if q.starts_with(&p) { + return Some(format!( + "{} holds {}, your own {label} folder. {NEVER}", + shown(path), + shown(&place) + )); + } } - if q.starts_with(&p) { - return Some(format!( - "{} holds {}, your own {label} folder. {never}", - shown(path), - shown(&place) - )); + } + None +} + +const NEVER: &str = "MacDirStat never offers your own files for deletion; remove this from the \ + plan, and trash it yourself in Finder if you mean to."; + +/// Inside a project's build output: the rules a build recreates exactly. +/// Caches, logs, `.gradle` and `.m2` (which can hold credentials) are left +/// out, so those stay refused inside your own folders. +fn is_build_output(path: &Path) -> bool { + const BUILD: &[&str] = &[ + "node_modules", + "Rust target/", + "Python __pycache__", + ".pytest_cache", + ".next/", + ".turbo/", + "DerivedData (loose)", + ]; + path.ancestors() + .any(|a| matches!(classify(a), Some((cat, _)) if BUILD.contains(&cat))) +} + +/// A Photos library within `dir`, looked for a few levels down (Photos +/// libraries are packages, usually one or two folders deep). Bounded so a +/// huge folder can't stall validation; symlinks are not followed. +pub fn photos_library_within(dir: &Path) -> Option { + let mut budget = 20_000usize; + let mut stack = vec![(dir.to_path_buf(), 0u8)]; + while let Some((d, depth)) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&d) else { continue }; + for e in entries.flatten() { + if budget == 0 { + return None; + } + budget -= 1; + let Ok(ft) = e.file_type() else { continue }; + if !ft.is_dir() { + continue; + } + let p = e.path(); + if e.file_name().to_string_lossy().to_lowercase().ends_with(".photoslibrary") { + return Some(p); + } + if depth < 3 { + stack.push((p, depth + 1)); + } } } None diff --git a/src/plan.rs b/src/plan.rs index ade41fc..bcfe0f8 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -148,6 +148,7 @@ pub fn validate(op: &Op) -> OpState { /// plan can't be approved. pub fn validate_with(op: &Op, home: Option<&std::path::Path>) -> OpState { let mut op = op.clone(); + let raw = op.path.clone(); let normalized = crate::relocate::normalize(&op.path); if let Ok(p) = &normalized { op.path = p.clone(); @@ -170,8 +171,19 @@ pub fn validate_with(op: &Op, home: Option<&std::path::Path>) -> OpState { Ok(p) => { if let Some(r) = crate::relocate::protected_reason(&p, home) { st.errors.push(r); - } else if let Some(r) = crate::cleanup::only_yours_reason(&p, home) { + } else if let Some(r) = crate::cleanup::only_yours_reason(&p, home) + .or_else(|| crate::cleanup::only_yours_reason(&raw, home)) + { + // Both the path as asked and as resolved: a link in the + // middle must not carry a path out of your folders. st.errors.push(r); + } else if let Some(lib) = crate::cleanup::photos_library_within(&p) { + st.errors.push(format!( + "{} holds a Photos library ({}), one of your own. MacDirStat never \ + offers your own files for deletion; remove this from the plan.", + p.display(), + lib.display() + )); } else if std::fs::symlink_metadata(&p).is_err() { st.errors.push(format!("{} does not exist", p.display())); } else { @@ -480,4 +492,40 @@ mod tests { assert!(s.approve(id).is_err()); std::fs::remove_dir_all(&home).unwrap(); } + + /// ~/Movies moved to another disk with a link left behind: its contents + /// are still refused, by the linked path and by the real one. A folder + /// holding a Photos library is refused too. + #[test] + fn validate_follows_links_and_finds_photos_libraries() { + let base = std::env::temp_dir().join(format!("mds-plan-link-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let home = base.join("home"); + let ext = base.join("ext/Movies"); + std::fs::create_dir_all(ext.join("Holiday")).unwrap(); + std::fs::create_dir_all(&home).unwrap(); + std::os::unix::fs::symlink(&ext, home.join("Movies")).unwrap(); + std::fs::create_dir_all(base.join("archive/2019/Old.photoslibrary/originals")).unwrap(); + std::fs::create_dir_all(home.join("Documents/app/node_modules/x")).unwrap(); + std::fs::create_dir_all(home.join("Documents/app/.cache")).unwrap(); + let home = home.canonicalize().unwrap(); + let base = base.canonicalize().unwrap(); + let check = |p: PathBuf| validate_with(&Op { path: p, action: Action::Trash }, Some(&home)); + + for p in [home.join("Movies/Holiday"), base.join("ext/Movies/Holiday")] { + let st = check(p.clone()); + assert!( + st.errors.iter().any(|e| e.contains("Movies")), + "{}: {:?}", + p.display(), + st.errors + ); + } + let st = check(base.join("archive")); + assert!(st.errors.iter().any(|e| e.contains("Photos library")), "{:?}", st.errors); + // Build output inside Documents may go; a cache there may not. + assert!(check(home.join("Documents/app/node_modules")).errors.is_empty()); + assert!(!check(home.join("Documents/app/.cache")).errors.is_empty()); + std::fs::remove_dir_all(&base).unwrap(); + } } diff --git a/src/server/mcp.rs b/src/server/mcp.rs index 9cdab50..dae054d 100644 --- a/src/server/mcp.rs +++ b/src/server/mcp.rs @@ -33,7 +33,7 @@ pub fn tools() -> Value { {"name": "find", "description": "Find items whose name matches a glob (* and ?), e.g. node_modules, .venv, target, *.dmg. Matches are not searched inside. Returns sizes, modified times and a total.", "inputSchema": {"type": "object", "required": ["pattern"], "properties": {"pattern": {"type": "string"}, "limit": {"type": "integer"}}}}, {"name": "cleanup_suggestions", "description": "Known regenerable data (caches, build output, simulators...) grouped by category with totals.", "inputSchema": none}, - {"name": "propose_plan", "description": "Propose trash/move operations. Returns the plan with per-op validation errors and warnings. It does NOT run anything: the user reviews and approves it in MacDirStat. Trashing the user's own folders (Documents, Desktop, Pictures, Movies, Music, Mail, Messages, Photos libraries) or anything in them is refused.", + {"name": "propose_plan", "description": "Propose trash/move operations. Returns the plan with per-op validation errors and warnings. It does NOT run anything: the user reviews and approves it in MacDirStat. Trashing the user's own folders (Documents, Desktop, Pictures, Movies, Music, Mail, Messages, Photos libraries), anything in them, or a folder holding one is refused; a project's build output inside them (node_modules, target, .next) is allowed.", "inputSchema": {"type": "object", "required": ["ops"], "properties": {"ops": {"type": "array", "items": op}, "note": {"type": "string", "description": "Why, in one or two sentences, shown to the user"}}}}, {"name": "list_plans", "description": "All plans with their status (pending, approved, done, failed, rejected).", "inputSchema": none}, {"name": "get_plan", "description": "One plan with per-op results.", From d2efb021724a1d029d42f0784f4c2886734dfbca Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 18:27:52 +0000 Subject: [PATCH 4/5] Plan check: Photos search counts folders only, breadth first Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017mKbacqC4bPDSYsaK9dVdu --- src/cleanup.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/cleanup.rs b/src/cleanup.rs index 9295c9a..e467d32 100644 --- a/src/cleanup.rs +++ b/src/cleanup.rs @@ -143,25 +143,27 @@ fn is_build_output(path: &Path) -> bool { /// libraries are packages, usually one or two folders deep). Bounded so a /// huge folder can't stall validation; symlinks are not followed. pub fn photos_library_within(dir: &Path) -> Option { + // Breadth first, counting folders only: a folder full of files can't + // use up the budget before a sibling library is seen. let mut budget = 20_000usize; - let mut stack = vec![(dir.to_path_buf(), 0u8)]; - while let Some((d, depth)) = stack.pop() { + let mut queue = std::collections::VecDeque::from([(dir.to_path_buf(), 0u8)]); + while let Some((d, depth)) = queue.pop_front() { let Ok(entries) = std::fs::read_dir(&d) else { continue }; for e in entries.flatten() { - if budget == 0 { - return None; - } - budget -= 1; let Ok(ft) = e.file_type() else { continue }; if !ft.is_dir() { continue; } + if budget == 0 { + return None; + } + budget -= 1; let p = e.path(); if e.file_name().to_string_lossy().to_lowercase().ends_with(".photoslibrary") { return Some(p); } if depth < 3 { - stack.push((p, depth + 1)); + queue.push_back((p, depth + 1)); } } } From 070cc81a279be80b11d83af2d9eeed13cb03b172 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 18:28:14 +0000 Subject: [PATCH 5/5] Review file: hand-back for the plan check (#20) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017mKbacqC4bPDSYsaK9dVdu --- docs/review/2026-10-30.json | 139 +++++++++++++++++++++++++++++++----- 1 file changed, 121 insertions(+), 18 deletions(-) diff --git a/docs/review/2026-10-30.json b/docs/review/2026-10-30.json index a79434b..aa43227 100644 --- a/docs/review/2026-10-30.json +++ b/docs/review/2026-10-30.json @@ -35,91 +35,139 @@ "decisions": [ { "question": "How to run tests in a Linux container for a macOS-only crate?", - "options": ["cfg shims for the 4 macOS-only calls", "skip macOS-only tests off a Mac", "don't test locally"], + "options": [ + "cfg shims for the 4 macOS-only calls", + "skip macOS-only tests off a Mac", + "don't test locally" + ], "default_taken": "cfg shims; no tests skipped", "why": "Lets 100+ tests run here without touching macOS behaviour; the 6 that need macOS still run in CI." }, { "question": "Screenshots without a Mac or GPU?", - "options": ["CPU rasteriser over egui's meshes (examples/screenshots.rs)", "update the mock-ups instead"], + "options": [ + "CPU rasteriser over egui's meshes (examples/screenshots.rs)", + "update the mock-ups instead" + ], "default_taken": "CPU rasteriser; the real App::update over a made-up home folder", "why": "Shows what was built, not a sketch. Font hinting differs a little from macOS; check on a Mac before release." }, { "question": "Which Source Serif 4 cut?", - "options": ["SmText (small-text optical size)", "Text", "Display"], + "options": [ + "SmText (small-text optical size)", + "Text", + "Display" + ], "default_taken": "SmText Regular + Italic, bundled (SIL OFL)", "why": "The UI is set at 10–14 pt, where SmText reads best." }, { "question": "Tabular figures for sizes?", - "options": ["accept the face's default figures, right-aligned", "bundle a tabular-figures font for numbers"], + "options": [ + "accept the face's default figures, right-aligned", + "bundle a tabular-figures font for numbers" + ], "default_taken": "default figures, right-aligned", "why": "egui can't switch OpenType features; columns still align on their right edge." }, { "question": "Colour of destructive buttons?", - "options": ["the oxblood accent", "a separate red"], + "options": [ + "the oxblood accent", + "a separate red" + ], "default_taken": "oxblood", "why": "Move to Trash is the one action of its window; SIGNATURE.md allows one accent per window." }, { "question": "In-app light/dark toggle?", - "options": ["follow the system only", "add a toggle"], + "options": [ + "follow the system only", + "add a toggle" + ], "default_taken": "follow the system only", "why": "Pavol asked for a dark variant that follows the system setting." }, { "question": "How to combine 1A and 1B?", - "options": ["1B's line under each left-column row", "a separate 1B table"], + "options": [ + "1B's line under each left-column row", + "a separate 1B table" + ], "default_taken": "one line under each row: 'To get it back: …. Last changed ….'", "why": "The proposal's recommendation; keeps the two-column look." }, { "question": "Time estimates in minutes (mock-up: '~10 min')?", - "options": ["plain words per rule", "minute estimates"], + "options": [ + "plain words per rule", + "minute estimates" + ], "default_taken": "plain words ('about a minute each' only for npm)", "why": "No claim the app can't back (Article VIII)." }, { "question": "Label for trashed bytes no rule covers in the closing account?", - "options": ["'of which no tool can rebuild'", "'of which only you had' (mock-up)"], + "options": [ + "'of which no tool can rebuild'", + "'of which only you had' (mock-up)" + ], "default_taken": "'of which no tool can rebuild'", "why": "An app bundle trashed from the map isn't something only you had; the mock-up wording would misstate it." }, { "question": "When does the closing account show?", - "options": ["after trashes made while Cleanup is open, until Close", "after every trash"], + "options": [ + "after trashes made while Cleanup is open, until Close", + "after every trash" + ], "default_taken": "while Cleanup is open; closing the window clears it", "why": "No pop-up after a single trash from the map." }, { "question": "Ledger row layout?", - "options": ["one line with a bar column", "bar under each name (mock-up)"], + "options": [ + "one line with a bar column", + "bar under each name (mock-up)" + ], "default_taken": "one line; bar scaled to the largest row of its level", "why": "Keeps the tree compact and still compares by length." }, { "question": "Where does the folder list start when the map is zoomed?", - "options": ["at the zoomed folder", "always at the scan root"], + "options": [ + "at the zoomed folder", + "always at the scan root" + ], "default_taken": "at the zoomed folder; Locate outside it zooms back out", "why": "List and map show the same level, as 2A describes." }, { "question": "What does a history summary keep?", - "options": ["8 largest top-level folders by name; loose files, hidden and the rest as unnamed sums", "more folders", "deeper levels"], + "options": [ + "8 largest top-level folders by name; loose files, hidden and the rest as unnamed sums", + "more folders", + "deeper levels" + ], "default_taken": "8 named top-level folders + unnamed sums; 6 per folder, 20 folders", "why": "Pavol: totals and top folders, no file lists. File names are never kept." }, { "question": "Do partial refreshes (⇧⌘R) record history?", - "options": ["full scans only", "every refresh"], + "options": [ + "full scans only", + "every refresh" + ], "default_taken": "full scans only", "why": "A partial refresh isn't a new look at the whole folder." }, { "question": "Version", - "options": ["bump Cargo.toml to 0.7.0 now", "leave at 0.6.0 until release"], + "options": [ + "bump Cargo.toml to 0.7.0 now", + "leave at 0.6.0 until release" + ], "default_taken": "bumped to 0.7.0; CHANGELOG heading says 'not yet released'", "why": "The tag must equal Cargo.toml's version; with the bump done, releasing is one tag push when Pavol decides." } @@ -127,17 +175,28 @@ "proposals": [ { "name": "Use the inventory in the server's plan checks, so a plan proposed over MCP can't include an only-yours folder", - "options": ["refuse such plans in validation", "warn in the plan", "leave the server as it is"], + "options": [ + "refuse such plans in validation", + "warn in the plan", + "leave the server as it is" + ], "recommendation": "Refuse in validation. Today an AI can propose trashing ~/Documents and only Pavol's approval stops it; the inventory already knows these folders." }, { "name": "Bring the landing site (site/) and its screenshot into the survey register", - "options": ["update with the 0.7.0 release", "update now", "keep the old look"], + "options": [ + "update with the 0.7.0 release", + "update now", + "keep the old look" + ], "recommendation": "Update with the release: it's outward-facing, so it waits for Pavol's sign-off." }, { "name": "A one-line 'since last scan' note (3A) drawn from the stored history", - "options": ["add it under the toolbar after a rescan", "leave history to the History window"], + "options": [ + "add it under the toolbar after a rescan", + "leave history to the History window" + ], "recommendation": "Wait for return-scan data: 3A wasn't picked, and History already shows growth." } ], @@ -162,5 +221,49 @@ "docs/screenshots/main-light.png", "docs/screenshots/main-dark.png", "docs/screenshots/inventory-dark.png" + ], + "followups": [ + { + "date": "2026-11-03", + "title": "Plans refuse to trash your own folders (proposal 1)", + "pr": "https://github.com/Chartres/mac-dir-stat/pull/20", + "merged": true, + "decided_by": "Pavol, 2026-11-03 11:52 UTC on decision card mac-dir-stat.o0: \"Refuse such plans in validation\" (relayed by the orchestrating session; recorded in SIGNATURE.md's decision log)", + "what": "Plan validation refuses any trash op that is, is inside, or holds Documents, Desktop, Pictures, Movies, Music, Mail or Messages under home, or a Photos library, naming the folder; the plan can't be approved. Checked by the path as asked and as resolved, and at each folder's real location if it is a link.", + "checked": "Tests first (unit, inventory, and an MCP journey proposing ~/Documents and a subfolder, refused, approve 409). macOS CI green on head. Independent review found two blockers (symlink bypass; folder holding a Photos library), both fixed and confirmed closed by a second review.", + "decisions": [ + { + "question": "Moves to an external disk of an own folder?", + "options": [ + "allowed (copy verified first)", + "refused too" + ], + "default_taken": "allowed", + "why": "Offloading Movies or Pictures is what Move to disk is for; only trash is cleanup." + }, + { + "question": "Build output inside an own folder (node_modules under ~/Documents/project)?", + "options": [ + "allowed for project build rules only", + "refused" + ], + "default_taken": "allowed for node_modules, target/, __pycache__, .pytest_cache, .next, .turbo, loose DerivedData; caches, logs, .gradle, .m2 stay refused", + "why": "The inventory lists them as rebuildable; refusing would contradict the Cleanup suggestions." + }, + { + "question": "Refuse at propose (4xx) or store an invalid plan?", + "options": [ + "invalid plan with the reason", + "HTTP error" + ], + "default_taken": "invalid plan", + "why": "Same as protected system paths today; the model sees why." + } + ], + "open": [ + "A subfolder moved to disk with a link left behind (e.g. ~/Documents/Proj -> ext/Proj) is protected only through the link, not by its real path. Fixing it means remembering what the app moved; not done.", + "The Photos-library search is bounded (20k folders, 4 levels); a larger tree is allowed rather than refused. Folding it into relocate::measure would remove the bound." + ] + } ] }