From 1c8321774530ba4c04142648630d0443b13195b7 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 21 Jul 2026 20:34:03 +0200 Subject: [PATCH] Delete captures from disk at any depth, behind a confirmation Deletion (d) previously reached only top-level folders of editable stores. /captures now exposes its whole tree: nested folders delete recursively, single tracks delete their metadata file plus the downloaded audio next to it (never audio outside the instance root). Tracks advertise this through the new LibraryNode.tracks_deletable flag. Because these deletes destroy slow-to-redo downloads, the TUI asks delete ? [y/N] first; cheap deletables (search terms, bookmarks, saved queues) stay unconfirmed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --- README.md | 4 +- architecture/capture-deletion.md | 98 +++++++++ cbd-tui/src/app/bindings.rs | 8 +- cbd-tui/src/app/library.rs | 15 +- cbd-tui/src/app/mod.rs | 172 +++++++++++++++- cbd-tui/src/lib.rs | 8 +- crabidy-core/crabidy/v1/crabidy.proto | 8 +- crabidy-core/src/lib.rs | 2 + crabidy-server/src/provider.rs | 8 +- fsdy/README.md | 6 + fsdy/src/lib.rs | 276 ++++++++++++++++++++++++-- plan/summary.md | 32 +++ tidaldy/src/lib.rs | 8 + ytdy/src/lib.rs | 5 + 14 files changed, 612 insertions(+), 38 deletions(-) create mode 100644 architecture/capture-deletion.md diff --git a/README.md b/README.md index 171bbc8..2d99eb5 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,9 @@ renames, `d` deletes. re-capturing the same name resumes and completes it; tracks whose source cannot be captured are recorded as *skipped* (red in the UI, skipped by playback). Download captures can take long; progress is - shown in the library pane. + shown in the library pane. Inside `/captures`, `d` deletes any + folder or single track *from disk* (audio included) after a `y/N` + confirmation. Press `?` for the full binding table. diff --git a/architecture/capture-deletion.md b/architecture/capture-deletion.md new file mode 100644 index 0000000..a61eaab --- /dev/null +++ b/architecture/capture-deletion.md @@ -0,0 +1,98 @@ +# Capture deletion + +Deleting under `/captures` reclaims disk: downloaded audio is the one +library content that is expensive to recreate (slow, throttled downloads +— architecture/youtube-rustypipe.md), so stale captures must be +removable from the TUI, and removal must actually delete the files. + +## Context + +Before this feature, deletion (`d`) was limited to *top-level* folders +of editable fsdy instances (architecture/bookmarks.md D4): whole +captures could be deleted (and were removed from disk via +`remove_dir_all`), but nothing below — no single album, no single +track. Deletes were deliberately unconfirmed because every deletable +node was cheap to recreate. + +## Decisions + +### D1 — deletable tree as an fsdy instance option + +`fsdy::Client::with_deletable_tree()` makes every folder below the +instance root deletable (recursively, any depth) and every track file +deletable. Only the `/captures` instance sets it: + +- `/queues` and `/bookmarks` keep the top-level-only contract; their + nested structure mirrors a snapshot and partial edits are better done + by re-saving. +- Nested folders become deletable but **not renamable** — renames would + break the incremental-capture merge by name + (architecture/incremental-captures.md), deletes cannot: a re-capture + of the same name simply re-downloads what is missing. +- The instance root itself and reserved top-level names stay + undeletable even on a deletable tree. + +### D2 — tracks advertise deletability through their node + +Wire truth, not client guessing: the TUI must not hardcode which tracks +are deletable. But a per-`Track` flag would touch every Track literal +in every provider for a capability only fsdy uses. Instead +`LibraryNode.tracks_deletable` says "this node's listed tracks may be +deleted", exactly like the existing `is_downloadable` inheritance +("tracks inherit their node's blessing", architecture/captures.md D4). +Child folders keep using the existing per-child `is_deletable`. + +### D3 — a deleted track takes its audio with it, inside the root only + +Deleting a track file removes the `.cbd-track.toml` **and** the audio +its `[playable] file` points to — that is the point of the feature. +Safety boundary: the audio path (relative values resolved against the +track file's directory) is canonicalized and must live inside the +canonicalized instance root; anything else is kept and logged. So a +hand-written track file referencing `~/Music/song.flac` from inside the +captures folder can never delete foreign data, and `..`/symlink tricks +resolve before the check. A track file that no longer parses is deleted +blind (its audio cannot be located; the listing skipped it anyway). +Deletes stay idempotent per the proto contract. + +### D4 — confirmation in the client, scoped to /captures + +`d` on anything under `/captures` opens a one-line modal prompt +(`delete <title>? [y/N]`, red) instead of sending; only `y`/`Y` +confirms, any other key cancels. Every other deletable (search terms, +bookmarks, saved queues) stays a single unconfirmed keypress +(architecture/node-editing.md D4) — they are cheap to recreate, and a +blanket confirmation would train reflexive `y`. The scoping is a path +check in the TUI (like the `/captures` never-cache rule in +`cbd-tui/src/rpc.rs`): the server does not know which deletes a client +should consider expensive. + +## Flow + +```d2 +direction: right +tui: cbd-tui { + d: "d on /captures/…" + confirm: "delete …? [y/N]" + d -> confirm +} +server: crabidy-server { + provider_loop: provider loop +} +fsdy: fsdy /captures instance { + folder: "folder: remove_dir_all" + track: "track: toml + contained audio" +} +tui.confirm -> server.provider_loop: y → DeleteLibraryNode +server.provider_loop -> fsdy.folder +server.provider_loop -> fsdy.track +fsdy.folder -> tui: refreshed parent listing +``` + +## Risks / notes + +- The confirmation prompt occupies the same line as the text-input + overlay; both are strictly modal and never open together. +- Deleting the folder of a *running* capture is possible; the capture + walk recreates directories as it goes and re-downloads on the next + run, so the race wastes bandwidth but corrupts nothing. diff --git a/cbd-tui/src/app/bindings.rs b/cbd-tui/src/app/bindings.rs index 796eabd..7682460 100644 --- a/cbd-tui/src/app/bindings.rs +++ b/cbd-tui/src/app/bindings.rs @@ -65,8 +65,10 @@ pub enum Action { /// term, whose rename re-runs the search). LibraryEditNode, /// Delete the selected item. No-op unless the selection `is_deletable`. - /// Deliberately unconfirmed: today's deletable nodes (search terms) are - /// free to recreate (architecture/node-editing.md, D4). + /// Cheap deletables (search terms, bookmarks, saved queues) delete + /// unconfirmed (architecture/node-editing.md, D4); captures hold + /// downloaded audio and open a y/N confirmation instead + /// (architecture/capture-deletion.md). LibraryDeleteNode, /// Open the input overlay (prefilled with the selection's title) to /// capture the selected queueable subtree as a bookmark under @@ -296,7 +298,7 @@ pub const BINDINGS: &[Binding] = &[ mods: KeyModifiers::NONE, code: KeyCode::Char('d'), action: Action::LibraryDeleteNode, - description: "Delete selected node", + description: "Delete selection (captures ask y/N, and delete files)", }, Binding { scope: Scope::Library, diff --git a/cbd-tui/src/app/library.rs b/cbd-tui/src/app/library.rs index 3b35525..43f6eee 100644 --- a/cbd-tui/src/app/library.rs +++ b/cbd-tui/src/app/library.rs @@ -60,11 +60,12 @@ impl Library { item.is_editable .then(|| (item.path.clone(), item.title.clone())) } - /// Path of the selected item, if it may be deleted (`d`). `None` when - /// nothing is selected or the item is not deletable. - pub fn selected_deletable(&self) -> Option<String> { + /// Path and title of the selected item, if it may be deleted (`d`). + /// `None` when nothing is selected or the item is not deletable. + pub fn selected_deletable(&self) -> Option<(String, String)> { let item = self.list.get(self.list_state.selected()?)?; - item.is_deletable.then(|| item.path.clone()) + item.is_deletable + .then(|| (item.path.clone(), item.title.clone())) } /// Path and title of the bare selection, if it is queueable — what `w` /// captures as a bookmark. Marks are deliberately ignored: one capture @@ -194,9 +195,9 @@ impl Library { is_queable: true, is_creatable: false, is_editable: false, - is_deletable: false, - // Tracks carry no wire flag: they inherit their node's - // blessing (architecture/captures.md D4). + // Tracks carry no wire flags of their own: they inherit + // their node's blessing (architecture/captures.md D4). + is_deletable: node.tracks_deletable, is_downloadable: node.is_downloadable, is_skipped: t.is_skipped, }) diff --git a/cbd-tui/src/app/mod.rs b/cbd-tui/src/app/mod.rs index 9c6911c..5bd91fb 100644 --- a/cbd-tui/src/app/mod.rs +++ b/cbd-tui/src/app/mod.rs @@ -242,6 +242,24 @@ pub struct InputState { pub buffer: String, } +/// A delete waiting for its `y` — opened instead of sending when the +/// selection is a capture (`d` there destroys downloaded data on disk, +/// architecture/capture-deletion.md). Modal like [`InputState`]: while +/// `Some`, keys go to [`App::handle_confirm_key`] only. +pub struct ConfirmDelete { + pub path: String, + /// Display title of the doomed item, shown in the prompt. + pub title: String, +} + +/// Whether deleting `path` needs a confirmation: deletes under +/// `/captures` remove downloaded audio from disk — expensive to redo — +/// while every other deletable node (search terms, bookmarks, saved +/// queues) is cheap to recreate and stays one keypress. +fn delete_needs_confirmation(path: &str) -> bool { + path == "/captures" || path.starts_with("/captures/") +} + pub struct App { pub focus: UiFocus, /// Whether the help modal is open. While true, `bindings::lookup` only @@ -250,6 +268,9 @@ pub struct App { /// `Some` while the input overlay is open; takes precedence over the /// bindings table (checked first in the event loop). pub input: Option<InputState>, + /// `Some` while a capture delete awaits confirmation; modal like + /// `input` and checked before it in the event loop. + pub confirm: Option<ConfirmDelete>, /// Progress of running (and recently finished) captures, rendered as /// status lines at the bottom of the library pane. pub captures: CaptureBoard, @@ -268,6 +289,7 @@ impl App { focus: UiFocus::Library, show_help: false, input: None, + confirm: None, captures: CaptureBoard::default(), library, now_playing, @@ -276,6 +298,21 @@ impl App { } } + /// Handles one key while a delete confirmation is open + /// (`confirm.is_some()`). Only `y`/`Y` confirms and sends the delete; + /// every other key cancels — the safe answer is any answer. + pub fn handle_confirm_key(&mut self, key: crossterm::event::KeyEvent) { + use crossterm::event::KeyCode; + let Some(confirm) = self.confirm.take() else { + return; + }; + if matches!(key.code, KeyCode::Char('y') | KeyCode::Char('Y')) { + let _ = self + .tx + .send(MessageFromUi::DeleteNode { path: confirm.path }); + } + } + /// Handles one key while the input overlay is open (`input.is_some()`). /// /// `Esc` cancels, `Enter` submits a non-empty trimmed buffer as the @@ -399,10 +436,16 @@ impl App { } } Action::LibraryDeleteNode => { - // Unconfirmed by design: today's deletable nodes are search - // terms, free to recreate (architecture/node-editing.md, D4). - if let Some(path) = self.library.selected_deletable() { - let _ = self.tx.send(MessageFromUi::DeleteNode { path }); + // Cheap deletables (search terms, bookmarks, saved queues) + // stay unconfirmed by design (architecture/node-editing.md, + // D4); captures hold downloaded audio and get a y/N prompt + // first (architecture/capture-deletion.md). + if let Some((path, title)) = self.library.selected_deletable() { + if delete_needs_confirmation(&path) { + self.confirm = Some(ConfirmDelete { path, title }); + } else { + let _ = self.tx.send(MessageFromUi::DeleteNode { path }); + } } } Action::LibraryCaptureNode => { @@ -521,6 +564,22 @@ impl App { } } + // The capture-delete confirmation: same one-line slot as the text + // input (the two are never open together), red — it is the one + // destructive prompt in the UI. + if let Some(confirm) = &self.confirm { + let area = main[0]; + if area.height >= 3 && area.width >= 4 { + let line = Rect::new(area.x + 1, area.y + area.height - 2, area.width - 2, 1); + f.render_widget(Clear, line); + f.render_widget( + Paragraph::new(format!("delete {}? [y/N]", confirm.title)) + .style(Style::default().fg(COLOR_RED)), + line, + ); + } + } + // Capture progress lines: stacked up from the bottom of the library // pane, above the input overlay when that is open. Failures render // red. At most three — more concurrent captures than that keep @@ -528,7 +587,11 @@ impl App { let capture_lines = self.captures.lines(); if !capture_lines.is_empty() { let area = main[0]; - let bottom_offset = if self.input.is_some() { 3 } else { 2 }; + let bottom_offset = if self.input.is_some() || self.confirm.is_some() { + 3 + } else { + 2 + }; for (i, (text, is_error)) in capture_lines.iter().take(3).enumerate() { let offset = bottom_offset + i as u16; if area.height <= offset + 1 || area.width < 4 { @@ -635,6 +698,7 @@ mod tests { is_queable: false, is_creatable: true, is_downloadable: false, + tracks_deletable: false, } } @@ -885,6 +949,104 @@ mod tests { } other => panic!("expected DeleteNode, got {:?}", other.is_ok()), } + assert!(app.confirm.is_none(), "cheap deletes are unconfirmed"); + } + + /// A /captures/mix listing: one deletable track (tracks come first in + /// the pane) and one deletable album folder. + fn captures_listing() -> LibraryNode { + use crabidy_core::proto::crabidy::{LibraryNodeChild, Track}; + LibraryNode { + path: "/captures/mix".to_string(), + title: "mix".to_string(), + children: vec![LibraryNodeChild { + is_deletable: true, + ..LibraryNodeChild::new( + "/captures/mix/album".to_string(), + "album".to_string(), + true, + ) + }], + parent: Some("/captures".to_string()), + tracks: vec![Track { + path: "/captures/mix/0001%20song.cbd-track.toml".to_string(), + artist: "artist".to_string(), + title: "song".to_string(), + duration: None, + album: None, + is_skipped: false, + }], + is_queable: true, + is_creatable: false, + is_downloadable: false, + tracks_deletable: true, + } + } + + #[test] + fn capture_deletes_open_a_confirmation_instead_of_sending() { + let (mut app, rx) = app(); + app.library.update(captures_listing()); + // The first item is the capture's track, deletable via the node's + // tracks_deletable flag. + let _ = app.dispatch(Action::LibraryDeleteNode); + assert!(rx.try_recv().is_err(), "nothing sent before the y"); + let confirm = app.confirm.as_ref().expect("confirmation open"); + assert_eq!(confirm.path, "/captures/mix/0001%20song.cbd-track.toml"); + assert_eq!(confirm.title, "artist - song"); + app.handle_confirm_key(key(crossterm::event::KeyCode::Char('y'))); + assert!(app.confirm.is_none()); + match rx.try_recv() { + Ok(MessageFromUi::DeleteNode { path }) => { + assert_eq!(path, "/captures/mix/0001%20song.cbd-track.toml"); + } + other => panic!("expected DeleteNode, got {:?}", other.is_ok()), + } + + // Folders below the top level are deletable (and confirmed) too. + let mut app_state = app; + app_state.library.update(captures_listing()); + app_state.library.last(); + let _ = app_state.dispatch(Action::LibraryDeleteNode); + let confirm = app_state.confirm.as_ref().expect("confirmation open"); + assert_eq!(confirm.path, "/captures/mix/album"); + } + + #[test] + fn capture_delete_confirmations_cancel_on_anything_but_y() { + use crossterm::event::KeyCode; + let (mut app, rx) = app(); + for cancel in [KeyCode::Esc, KeyCode::Char('n'), KeyCode::Enter] { + app.library.update(captures_listing()); + let _ = app.dispatch(Action::LibraryDeleteNode); + assert!(app.confirm.is_some(), "confirmation open"); + app.handle_confirm_key(key(cancel)); + assert!(app.confirm.is_none(), "{cancel:?} closes"); + assert!(rx.try_recv().is_err(), "{cancel:?} must not delete"); + } + } + + #[test] + fn capture_delete_confirmation_renders_its_prompt() { + let (mut app, _rx) = app(); + app.library.update(captures_listing()); + let _ = app.dispatch(Action::LibraryDeleteNode); + + let backend = ratatui::backend::TestBackend::new(80, 24); + let mut terminal = ratatui::Terminal::new(backend).expect("test terminal"); + terminal.draw(|f| app.render(f)).expect("draw app"); + let buf = terminal.backend().buffer(); + let mut text = String::new(); + for y in 0..buf.area.height { + for x in 0..buf.area.width { + text.push_str(buf[(x, y)].symbol()); + } + text.push('\n'); + } + assert!( + text.contains("delete artist - song? [y/N]"), + "prompt line visible" + ); } #[test] diff --git a/cbd-tui/src/lib.rs b/cbd-tui/src/lib.rs index 89792b7..59082a1 100644 --- a/cbd-tui/src/lib.rs +++ b/cbd-tui/src/lib.rs @@ -277,10 +277,12 @@ fn run_ui(tx: Sender<MessageFromUi>, rx: Receiver<MessageToUi>) { if event::poll(timeout).unwrap() { if let Event::Key(key) = event::read().unwrap() { if key.kind == KeyEventKind::Press { - // The input overlay is strictly modal: while it is open, - // keys edit the buffer and the bindings table (including + // The overlays are strictly modal: while one is open, + // keys answer it and the bindings table (including // quit) is unreachable. - if app.input.is_some() { + if app.confirm.is_some() { + app.handle_confirm_key(key); + } else if app.input.is_some() { app.handle_input_key(key); } else if let Some(action) = bindings::lookup(app.focus, app.show_help, key) { if app.dispatch(action) == DispatchResult::Quit { diff --git a/crabidy-core/crabidy/v1/crabidy.proto b/crabidy-core/crabidy/v1/crabidy.proto index 9c49515..02f1706 100644 --- a/crabidy-core/crabidy/v1/crabidy.proto +++ b/crabidy-core/crabidy/v1/crabidy.proto @@ -17,7 +17,10 @@ service CrabidyService { // changes with the title. Renaming onto an existing sibling title merges // with it. Returns the renamed node at its new path. rpc RenameLibraryNode(RenameLibraryNodeRequest) returns (RenameLibraryNodeResponse); - // Deletes a node whose listing entry sets is_deletable. Idempotent: + // Deletes a node whose listing entry sets is_deletable, or a track + // whose parent node sets tracks_deletable. On filesystem-backed stores + // this removes the data from disk: a folder is deleted recursively, a + // track loses its metadata file and its local audio. Idempotent: // deleting an already-gone node succeeds. Returns the refreshed parent. rpc DeleteLibraryNode(DeleteLibraryNodeRequest) returns (DeleteLibraryNodeResponse); // Captures the queueable subtree at `path` as the bookmark `name`: a @@ -307,4 +310,7 @@ message LibraryNode { // This node allows download captures; its listed tracks inherit the // flag (CaptureLibraryNode with download). bool is_downloadable = 8; + // This node's listed tracks may be deleted (see DeleteLibraryNode) — + // like is_downloadable, tracks inherit the node's flag. + bool tracks_deletable = 9; } diff --git a/crabidy-core/src/lib.rs b/crabidy-core/src/lib.rs index 9438c5d..f6521cd 100644 --- a/crabidy-core/src/lib.rs +++ b/crabidy-core/src/lib.rs @@ -210,6 +210,7 @@ impl LibraryNode { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } } @@ -366,6 +367,7 @@ mod tests { is_queable, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } diff --git a/crabidy-server/src/provider.rs b/crabidy-server/src/provider.rs index 82afb69..9b53dad 100644 --- a/crabidy-server/src/provider.rs +++ b/crabidy-server/src/provider.rs @@ -360,7 +360,13 @@ impl ProviderClient for ProviderOrchestrator { }; let captures_client = capture_store.as_ref().and_then(|store| { match fsdy::Client::new(CAPTURES_PROVIDER_ROOT, store.dir().to_path_buf()) { - Ok(client) => Some(Arc::new(client.with_editable_top_level(&[]))), + // The whole tree is deletable: stale downloads are + // reclaimed through the library, folders recursively and + // tracks together with their audio. Clients confirm these + // deletes (architecture/capture-deletion.md). + Ok(client) => Some(Arc::new( + client.with_editable_top_level(&[]).with_deletable_tree(), + )), Err(err) => { warn!("captures library disabled: {err}"); None diff --git a/fsdy/README.md b/fsdy/README.md index cb4483c..68ce464 100644 --- a/fsdy/README.md +++ b/fsdy/README.md @@ -59,3 +59,9 @@ manager — they are just folders of the track files described above. Their top-level folders can be renamed (`e`) and deleted (`d`) from the TUI; `/queues/current` is the continuously persisted play queue and is protected. + +`/captures` additionally lets `d` delete *anything* in its tree — a +whole capture, a nested album folder, or a single track (which takes +its downloaded audio file with it). Because that removes data from +disk that was slow to download, the TUI asks for confirmation first +(`delete <name>? [y/N]`); deletes elsewhere stay unconfirmed. diff --git a/fsdy/src/lib.rs b/fsdy/src/lib.rs index 0694594..3e43daa 100644 --- a/fsdy/src/lib.rs +++ b/fsdy/src/lib.rs @@ -375,6 +375,12 @@ pub struct Client { /// downloadable providers; off for `/fs` and `/captures`. Tracks /// whose source cannot be captured are skipped by the capture walk. downloadable_nodes: bool, + /// Whether the whole tree is deletable through the library: every + /// folder below the instance root and every track file (which loses + /// its local audio along with its metadata). On for `/captures` + /// only — deletes there destroy downloaded data, so clients confirm + /// them (architecture/capture-deletion.md). + deletable_tree: bool, } impl Client { @@ -405,6 +411,7 @@ impl Client { editable_top_level: false, reserved: Vec::new(), downloadable_nodes: false, + deletable_tree: false, }) } @@ -430,6 +437,18 @@ impl Client { self } + /// Makes the whole tree deletable through the library + /// ([`ProviderClient::delete_lib_node`]): every folder below the + /// instance root (recursively, any depth) and every track file. A + /// deleted track loses its metadata file *and* its local audio, as + /// long as the audio resolves inside the instance root. Used by + /// `/captures`, where stale downloads are reclaimed through the + /// library (architecture/capture-deletion.md). + pub fn with_deletable_tree(mut self) -> Self { + self.deletable_tree = true; + self + } + /// The provider root this instance owns (e.g. `/fs`), as configured /// through [`Self::new`]. pub fn provider_root(&self) -> &str { @@ -538,7 +557,8 @@ impl Client { } // Only an editable instance's direct root children are modifiable // (architecture/bookmarks.md D4); reserved names (the auto-persisted - // `current` queue) keep their immutability. + // `current` queue) keep their immutability. A deletable tree + // additionally makes every folder deletable (but not renamable). let editable_here = self.editable_top_level && lib_path == self.provider_root; let children = dir_names .into_iter() @@ -548,7 +568,7 @@ impl Client { let editable = editable_here && !self.reserved.contains(&name); let mut child = LibraryNodeChild::new(child_lib, name, true); child.is_editable = editable; - child.is_deletable = editable; + child.is_deletable = editable || self.deletable_tree; child.is_downloadable = self.downloadable_nodes; child }) @@ -571,6 +591,7 @@ impl Client { is_queable: true, is_creatable: false, is_downloadable: self.downloadable_nodes, + tracks_deletable: self.deletable_tree, }) } @@ -599,6 +620,78 @@ impl Client { Ok((self.disk_path(path)?, name)) } + /// Whether `path` addresses a reserved direct child of the instance + /// root (e.g. the auto-persisted `current` queue) — immutable even on + /// a deletable tree. + fn is_reserved_top_level(&self, path: &str) -> bool { + path.strip_prefix(self.provider_root.as_str()) + .and_then(|rest| rest.strip_prefix('/')) + .is_some_and(|rest| { + !rest.contains('/') && self.reserved.contains(&crabidy_core::decode_segment(rest)) + }) + } + + /// Deletes one track file and, when its playable is a local audio + /// file that resolves inside the instance root, that audio file too + /// (captures store audio next to the metadata; audio elsewhere on + /// disk — an `/fs`-style absolute reference — is never touched). + /// Idempotent: an already-gone track file is a success; a track file + /// that no longer parses still gets removed (its audio cannot be + /// located then, which the log notes). + async fn delete_track_file(&self, path: &str) -> Result<(), ProviderError> { + let disk = self.disk_path(path)?; + let audio = match self.read_track_file(&disk).await { + Ok(file) => match file.playable() { + Ok(Playable::File(target)) => { + let absolute = if target.is_absolute() { + target + } else { + disk.parent().map(|dir| dir.join(&target)).unwrap_or(target) + }; + Some(absolute) + } + _ => None, + }, + // read_track_file warned already; a missing file stays + // idempotent below, an unparseable one is deleted blind. + Err(_) => None, + }; + match tokio::fs::remove_file(&disk).await { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + return Ok(()); + } + Err(err) => { + warn!(path, "cannot delete track file: {err}"); + return Err(ProviderError::InternalError); + } + } + let Some(audio) = audio else { + return Ok(()); + }; + // Containment check on the *resolved* audio path: canonicalize + // both sides so `..` segments or symlinks in the track file + // cannot direct the delete outside the instance root. + let Ok(root) = tokio::fs::canonicalize(&self.root).await else { + return Ok(()); + }; + match tokio::fs::canonicalize(&audio).await { + Ok(canonical) if canonical.starts_with(&root) => { + if let Err(err) = tokio::fs::remove_file(&canonical).await { + if err.kind() != std::io::ErrorKind::NotFound { + warn!(path, "track file deleted, but not its audio: {err}"); + } + } + } + Ok(_) => { + warn!(path, "keeping audio outside the instance root"); + } + // Already gone (or unreadable): nothing left to delete. + Err(_) => {} + } + Ok(()) + } + /// Reads and parses one serialized track file. Failures are warned /// here (with the file's path, never its contents) so every caller /// can simply skip. @@ -725,6 +818,7 @@ impl ProviderClient for Client { is_queable: true, is_creatable: false, is_downloadable: self.downloadable_nodes, + tracks_deletable: self.deletable_tree, } } @@ -789,24 +883,45 @@ impl ProviderClient for Client { self.get_lib_node(&new_path).await } - /// Deletes a modifiable top-level folder (editable instances only); - /// everything else is [`ProviderError::NotSupported`]. Idempotent: an - /// already-gone folder is a success. Returns the refreshed root - /// listing. + /// Deletes a modifiable top-level folder (editable instances), or — + /// on a deletable tree ([`Self::with_deletable_tree`]) — any nested + /// folder recursively or a single track file together with its local + /// audio. Everything else is [`ProviderError::NotSupported`]. + /// Idempotent: an already-gone target is a success. Returns the + /// refreshed parent listing. async fn delete_lib_node(&self, path: &str) -> Result<LibraryNode, ProviderError> { - let (disk, _name) = self.editable_child_dir(path)?; - match tokio::fs::remove_dir_all(&disk).await { - Ok(()) => {} - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - // Idempotent by contract (crabidy.proto). - } - Err(err) => { - warn!(path, "cannot delete folder: {err}"); - return Err(ProviderError::InternalError); + if self.deletable_tree && self.is_track_path(path) { + self.delete_track_file(path).await?; + } else { + let disk = match self.editable_child_dir(path) { + Ok((disk, _name)) => disk, + // Below the top level only a deletable tree may delete — + // never the instance root itself, and reserved names keep + // their immutability even there. + Err(ProviderError::NotSupported) + if self.deletable_tree + && path != self.provider_root + && !self.is_reserved_top_level(path) => + { + self.disk_path(path)? + } + Err(err) => return Err(err), + }; + match tokio::fs::remove_dir_all(&disk).await { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + // Idempotent by contract (crabidy.proto). + } + Err(err) => { + warn!(path, "cannot delete folder: {err}"); + return Err(ProviderError::InternalError); + } } } - let root = self.provider_root.clone(); - self.get_lib_node(&root).await + let parent = crabidy_core::parent_path(path) + .map(String::from) + .unwrap_or_else(|| self.provider_root.clone()); + self.get_lib_node(&parent).await } // `resolve_tracks_into` deliberately keeps the default pre-order walk: @@ -1509,6 +1624,133 @@ mod tests { .expect_err(undeletable); assert_eq!(err, ProviderError::NotSupported, "{undeletable}"); } + // Neither are track files: only deletable trees delete tracks. + fs::create_dir(dir.path().join("hiking")).expect("mkdir"); + write_track( + &dir.path().join("hiking"), + "0001 a.cbd-track.toml", + &url_track("a"), + ); + let err = client + .delete_lib_node("/queues/hiking/0001%20a.cbd-track.toml") + .await + .expect_err("track"); + assert_eq!(err, ProviderError::NotSupported); + } + + /// A captures-shaped instance: editable top level plus a deletable + /// tree, with one nested album holding a downloaded track (audio next + /// to its toml) and a URL track. + async fn deletable_tree_client() -> (Client, TempDir) { + let dir = TempDir::new().expect("tempdir"); + let album = dir.path().join("mix/album"); + fs::create_dir_all(&album).expect("mkdir"); + write_track( + &album, + "0001 song.cbd-track.toml", + "title = \"song\"\n[playable]\nfile = \"0001 song.m4a\"\n", + ); + fs::write(album.join("0001 song.m4a"), b"audio").expect("write audio"); + write_track(&album, "0002 radio.cbd-track.toml", &url_track("radio")); + let client = Client::new("/captures", dir.path().to_path_buf()) + .expect("instance") + .with_editable_top_level(&[]) + .with_deletable_tree(); + (client, dir) + } + + #[tokio::test] + async fn deletable_trees_flag_every_folder_and_track() { + let (client, _dir) = deletable_tree_client().await; + let root = client.get_lib_node("/captures").await.expect("root"); + assert!(root.tracks_deletable); + let top = &root.children[0]; + assert!( + top.is_editable && top.is_deletable, + "top level: rename + delete" + ); + let nested = client + .get_lib_node("/captures/mix") + .await + .expect("nested listing"); + assert!(nested.tracks_deletable); + let album = &nested.children[0]; + assert!(album.is_deletable, "nested folders are deletable"); + assert!(!album.is_editable, "but not renamable"); + + // Editable-only instances (queues, bookmarks) keep tracks and + // nested folders immutable. + let (queues, _dir2) = editable_client().await; + let root = queues.get_lib_node("/queues").await.expect("root"); + assert!(!root.tracks_deletable); + } + + #[tokio::test] + async fn deletable_trees_delete_nested_folders_recursively() { + let (client, dir) = deletable_tree_client().await; + let parent = client + .delete_lib_node("/captures/mix/album") + .await + .expect("delete"); + assert_eq!(parent.path, "/captures/mix", "refreshed parent listing"); + assert!(parent.children.is_empty()); + assert!(!dir.path().join("mix/album").exists(), "gone from disk"); + // Idempotent, like top-level deletes. + client + .delete_lib_node("/captures/mix/album") + .await + .expect("idempotent delete"); + // The instance root itself stays undeletable. + let err = client.delete_lib_node("/captures").await.expect_err("root"); + assert_eq!(err, ProviderError::NotSupported); + } + + #[tokio::test] + async fn deleting_a_track_removes_its_file_and_local_audio() { + let (client, dir) = deletable_tree_client().await; + let album = dir.path().join("mix/album"); + let parent = client + .delete_lib_node("/captures/mix/album/0001%20song.cbd-track.toml") + .await + .expect("delete track"); + assert_eq!(parent.path, "/captures/mix/album"); + assert_eq!(parent.tracks.len(), 1, "only the radio track remains"); + assert!(!album.join("0001 song.cbd-track.toml").exists()); + assert!(!album.join("0001 song.m4a").exists(), "audio deleted too"); + // Idempotent: the track is already gone. + client + .delete_lib_node("/captures/mix/album/0001%20song.cbd-track.toml") + .await + .expect("idempotent delete"); + // URL tracks have no local audio; only the toml goes. + client + .delete_lib_node("/captures/mix/album/0002%20radio.cbd-track.toml") + .await + .expect("delete url track"); + assert!(!album.join("0002 radio.cbd-track.toml").exists()); + } + + #[tokio::test] + async fn deleting_a_track_never_touches_audio_outside_the_root() { + let (client, dir) = deletable_tree_client().await; + let outside = TempDir::new().expect("outside dir"); + let audio = outside.path().join("keep.flac"); + fs::write(&audio, b"audio").expect("write audio"); + let album = dir.path().join("mix/album"); + write_track( + &album, + "0003 external.cbd-track.toml", + &format!( + "title = \"external\"\n[playable]\nfile = {:?}\n", + audio.to_str().unwrap() + ), + ); + client + .delete_lib_node("/captures/mix/album/0003%20external.cbd-track.toml") + .await + .expect("delete track"); + assert!(!album.join("0003 external.cbd-track.toml").exists()); + assert!(audio.exists(), "external audio is not ours to delete"); } // ---- shared naming ---------------------------------------------------- diff --git a/plan/summary.md b/plan/summary.md index 8e03187..67a3847 100644 --- a/plan/summary.md +++ b/plan/summary.md @@ -572,3 +572,35 @@ Built per `plan/help-modal.md`: `app/bindings.rs` (declarative - **`main.rs`** passes `tx` to `App::new` without the now-unneeded clone; the `KeyCode`/`KeyModifiers`/`UiFocus`/`StatefulList` imports moved out with the old match. + +## capture-deletion (2026-07-21) + +Deletes under `/captures` now work at any depth and remove data from +disk, behind a TUI confirmation (`architecture/capture-deletion.md`; +direct implementation, no separate plan file — the change is four +bounded seams): + +- **Proto**: `LibraryNode.tracks_deletable` (field 9) — node-level + "listed tracks may be deleted", mirroring the `is_downloadable` + inheritance so no `Track` literal anywhere had to change. + `DeleteLibraryNode` doc extended to tracks and recursive folders. +- **fsdy**: `with_deletable_tree()` (only the `/captures` instance sets + it): nested folders delete recursively; track deletes remove the toml + plus its `[playable] file` audio **iff** the canonicalized audio path + stays inside the canonicalized instance root (`..`/symlink-proof); + reserved names and the instance root remain undeletable; everything + idempotent. Deletes now return the actual parent listing (was: root — + identical for the previously-only-possible top-level case). +- **crabidy-server**: captures fsdy instance gains the flag; the + delete RPC path was already generic. +- **cbd-tui**: tracks inherit `is_deletable` from `tracks_deletable`; + `d` under `/captures` opens a modal red `delete <title>? [y/N]` line + (only `y`/`Y` sends, any other key cancels) — other deletables stay + unconfirmed by design; `selected_deletable()` now returns + `(path, title)`. + +Tests: 4 new fsdy tests (flags, recursive delete, track+audio delete, +outside-root audio kept) and 3 new TUI tests (confirm-then-send, +cancel-on-anything-else, prompt render) plus a guard in the existing +queues delete test that track deletion stays `NotSupported` there. +188 workspace tests green; clippy `-D warnings` and fmt clean. diff --git a/tidaldy/src/lib.rs b/tidaldy/src/lib.rs index d4b4dca..b296ed9 100644 --- a/tidaldy/src/lib.rs +++ b/tidaldy/src/lib.rs @@ -129,6 +129,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } @@ -157,6 +158,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, }; let user_id = user_id.ok_or(crabidy_core::ProviderError::UnknownUser)?; for playlist in self @@ -189,6 +191,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: true, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } TidalPath::Artists => { @@ -201,6 +204,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, }; let user_id = user_id.ok_or(crabidy_core::ProviderError::UnknownUser)?; for artist in self.get_users_artists(&user_id).await? { @@ -236,6 +240,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: true, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } TidalPath::Album { album, .. } => { @@ -255,6 +260,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: true, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } TidalPath::Search => crabidy_core::proto::crabidy::LibraryNode { @@ -282,6 +288,7 @@ impl crabidy_core::ProviderClient for Client { is_queable: false, is_creatable: true, is_downloadable: false, + tracks_deletable: false, }, TidalPath::SearchTerm(encoded) => { let term = crabidy_core::decode_segment(encoded); @@ -915,6 +922,7 @@ impl Client { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, }) } diff --git a/ytdy/src/lib.rs b/ytdy/src/lib.rs index bd23242..530428d 100644 --- a/ytdy/src/lib.rs +++ b/ytdy/src/lib.rs @@ -191,6 +191,7 @@ impl Client { is_queable: true, is_creatable: false, is_downloadable: false, + tracks_deletable: false, }) } @@ -224,6 +225,7 @@ impl Client { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, }) } @@ -252,6 +254,7 @@ impl Client { is_queable: true, is_creatable: false, is_downloadable: false, + tracks_deletable: false, }) } @@ -428,6 +431,7 @@ impl ProviderClient for Client { is_queable: false, is_creatable: false, is_downloadable: false, + tracks_deletable: false, } } @@ -462,6 +466,7 @@ impl ProviderClient for Client { is_queable: false, is_creatable: true, is_downloadable: false, + tracks_deletable: false, }, YtPath::SearchTerm(encoded) => { let term = crabidy_core::decode_segment(encoded);