From 655e8054b845f5ec837cdec75840edceb401f620 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 21 Jul 2026 11:11:27 +0200 Subject: [PATCH] Capture library subtrees as bookmarks w on a queueable library selection snapshots the whole subtree into a third fsdy instance at /bookmarks: the orchestrator walks the source iteratively and mirrors it as order-prefixed folders of link track files (shared naming with queue persistence), tmp-and-swapped with size caps so a runaway tree cannot fill the disk. One additive rpc, CaptureLibraryNode(path, name), carries the flow; the TUI reuses the input overlay prefilled with the selection title. fsdy instances can now opt into an editable top level: root child folders carry is_editable/is_deletable and support no-merge rename and idempotent delete. /bookmarks mounts with it, and /queues too (reserving current), so saved queues are renamable and deletable through the existing e/d flows without TUI changes. Co-Authored-By: Claude Fable 5 --- architecture/bookmarks.md | 184 +++++++++++++ architecture/queue-persistence.md | 8 +- cbd-tui/src/app/bindings.rs | 15 +- cbd-tui/src/app/library.rs | 8 + cbd-tui/src/app/mod.rs | 86 ++++++ cbd-tui/src/main.rs | 7 + cbd-tui/src/rpc.rs | 23 +- crabidy-core/crabidy/v1/crabidy.proto | 14 + crabidy-server/src/bookmark_store.rs | 382 ++++++++++++++++++++++++++ crabidy-server/src/lib.rs | 1 + crabidy-server/src/main.rs | 9 + crabidy-server/src/provider.rs | 133 ++++++++- crabidy-server/src/queue_store.rs | 23 +- crabidy-server/src/rpc.rs | 71 ++++- fsdy/src/lib.rs | 324 ++++++++++++++++++++-- plan/bookmarks.md | 51 ++++ plan/summary.md | 55 ++++ quality/bookmarks.md | 73 +++++ 18 files changed, 1406 insertions(+), 61 deletions(-) create mode 100644 architecture/bookmarks.md create mode 100644 crabidy-server/src/bookmark_store.rs create mode 100644 plan/bookmarks.md create mode 100644 quality/bookmarks.md diff --git a/architecture/bookmarks.md b/architecture/bookmarks.md new file mode 100644 index 0000000..7cbbdc7 --- /dev/null +++ b/architecture/bookmarks.md @@ -0,0 +1,184 @@ +# Bookmarks: capturing library subtrees + +## Context and problem statement + +Queue persistence (architecture/queue-persistence.md) flattens the queue +into one folder of link files. The user now wants to **capture whole +subtrees**: pressing `w` on a queueable item in the *library* (an artist, +an album, a playlist folder) snapshots it into a local tree that +**preserves structure** — an artist becomes a folder of album folders, +each holding the playable track files. The store is one more local fs +provider ("bookmarks"); replaying is plain fs-provider behavior. On top, +the created top-level folders must be **renamable and deletable** from +the TUI — and the same should hold for saved queues. + +## Assumptions (confirmed against the code) + +- `fsdy::Client` is instance-mountable since queue persistence + (`Client::new(provider_root, disk_root)`); a third instance is cheap. +- The TUI already routes `e`/`d` through `is_editable`/`is_deletable` + flags on `LibraryNodeChild` into the existing + `RenameLibraryNode`/`DeleteLibraryNode` rpcs, and the orchestrator + already routes those to the `/queues` (and future `/bookmarks`) + instances. Making folders modifiable therefore needs **zero TUI + changes** — only fsdy must set flags and implement rename/delete. +- The capture walk can reuse `get_lib_node` through the + `ProviderOrchestrator` (any provider reachable), and + `TrackFile::from_track` + `track_file_name` from queue persistence for + the leaves. +- `w` is unbound in the TUI's `Library` scope; the input overlay handles + ask-for-a-name flows and supports prefilling (rename does). + +## Decisions + +### D1 — Third fsdy instance `/bookmarks`; capture is a server-side walk + +A `BookmarkStore` (sibling of `QueueStore`) owns +`/crabidy/bookmarks/`; the orchestrator mounts a read-only fsdy +instance over it at `/bookmarks` and is the only writer. Capture runs on +the **orchestrator side** (it must call `get_lib_node` across providers): +a new `ProviderCommand::CaptureLibraryNode { path, name }` is handled on +a spawned task (like `ResolveTracks` — a big artist walk must not block +the loop). The reply arrives when the write finished. + +Not chosen: capturing client-side in the TUI (would duplicate provider +access) or reusing the playback loop (captures are not queue state). + +### D2 — Structure fidelity: order-prefixed folders and files + +The walk mirrors the subtree iteratively (worklist, pre-order): + +- Each child **node** becomes a folder named `NNNN ` (same + zero-padded prefix and sanitizer as queue entries, no suffix) — the + case-insensitive listing sort then reproduces the provider's child + order, which is meaningful (album track order, discography order). +- Each **track** becomes `NNNN <title>.cbd-track.toml` via the existing + `TrackFile::from_track` (uniform link playable) — metadata is captured + at save time; drift is accepted like everywhere else. +- A node carrying both tracks and children (search terms) writes both. +- Capturing a *track* selection is allowed: a folder with one link file. +- Whole-bookmark writes are tmp-and-swap like queues; an existing + bookmark of the same name is overwritten. + +**Safety caps**: the walk aborts (typed error, temp dir removed) beyond +1 000 directories or 20 000 tracks — a runaway provider tree must not +fill the disk. Cycles are impossible under the cap (it bounds total +nodes, not depth). Since captured listings rewrite link tracks to their +targets, capturing a bookmark re-links to the *original* targets — no +link chains ever get written. + +### D3 — New rpc `CaptureLibraryNode(path, name)` + +Additive proto change (the only wire change). Name validation is shared +with queue saving (trimmed, no separators/NUL, no leading dot; no +reserved names in `/bookmarks`). Mapping: invalid name/source → +`invalid_argument`, capture disabled (no config dir) → +`failed_precondition`, walk/write failures → `internal`. The response is +empty — the TUI stays where it is (unlike `%`-create, capturing is not a +navigation; the bookmark appears under `/bookmarks` on the next visit). + +### D4 — Mutable top-level folders as an fsdy instance option + +`fsdy::Client` gains a builder option +`with_editable_top_level(reserved_names)`: + +- The instance-root listing marks child *folders* `is_editable` and + `is_deletable`, except reserved names. +- `rename_lib_node`: only direct children of the instance root; new + title validated like a store name; renaming onto an existing sibling + is `InvalidInput` (folders never merge); returns the renamed node (the + TUI navigates into it, as with search terms). +- `delete_lib_node`: only direct children of the root; `remove_dir_all`; + idempotent (already gone → success); returns the refreshed root + listing. +- Deeper levels stay immutable — the user request covers the *created* + folders; restructuring inside a capture is file-manager work. + +Applied to `/bookmarks` (no reserved names) **and `/queues`** (reserved: +`current`, which auto-persist owns — it can be neither renamed nor +deleted, and nothing can be renamed onto it). `/fs` keeps the immutable +default. Not chosen: implementing rename/delete in the stores — the +providers already own path→disk mapping and the rpc routing exists. + +### D5 — TUI: `w` in the library scope + +`Action::LibraryCaptureNode` bound to `w` in `Scope::Library` ("Save +selection as bookmark"): opens the input overlay **prefilled with the +selected item's title**, gated on the bare selection being queueable +(marks are ignored — one capture per invocation). Submit sends +`MessageFromUi::CaptureNode { path, name }` → the new rpc. Rename (`e`) +and delete (`d`) of bookmark/queue folders ride the existing flows via +the D4 flags. + +### D6 — Out of scope (explicitly) + +- Capturing multiple marked items at once; capture progress display. +- Rename/delete below the top level; moving bookmarks between folders. +- Refreshing a bookmark from its source (re-capture under the same name + overwrites — that *is* the refresh). +- A creatable `/bookmarks` root (`%`) — bookmarks are created from the + source tree. + +## Structure + +```d2 +direction: right + +server: crabidy-server { + orch: ProviderOrchestrator { + cap: "capture walk (spawned):\nget_lib_node -> mirror tree" + } + bstore: BookmarkStore { + w: "validate name, caps,\ntmp-and-swap" + } +} + +tidal: tidaldy +fs: "fsdy /fs" +qfs: "fsdy /queues\n(editable top level,\nreserved: current)" +bfs: "fsdy /bookmarks\n(editable top level)" + +disk: "config/crabidy/bookmarks" { + shape: cylinder + tree: "<name>/NNNN <album>/NNNN <track>.cbd-track.toml" +} + +server.orch.cap -> tidal: "walk source subtree" +server.orch.cap -> server.bstore: "write mirrored tree" +server.bstore -> disk +bfs -> disk: "list + parse (read only)" +server.orch -> bfs: "/bookmarks/... (browse, queue, e/d)" +server.orch -> qfs: "e/d on saved queues" +``` + +## Key flow: capture an artist, rename it, replay an album + +```d2 +shape: sequence_diagram +tui: TUI +orch: Orchestrator +tidal: tidaldy +store: BookmarkStore + +tui -> orch: "CaptureLibraryNode(/tidal/artists/42, faves)" +orch -> tidal: "get_lib_node (artist, albums, ...)" +orch -> store: "write faves/0001 Album/0001 Song.cbd-track.toml ..." +store -> tui: OK +tui -> orch: "RenameLibraryNode(/bookmarks/faves, road faves)" +orch -> tui: "renamed node (TUI navigates in)" +tui -> orch: "ReplaceQueue([/bookmarks/road%20faves/0001%20Album])" +orch -> tui: "resolve walk streams the album's tracks" +``` + +## Risks and open questions + +- **Capture duration**: a large artist means many provider fetches; the + TUI's poll loop awaits the rpc like other slow calls (accepted, + consistent with search-term creation). The orchestrator loop itself + stays free (spawned task). +- **Rename/delete racing a re-capture** of the same name: last writer + wins on the swap; accepted for a single-user local server. +- **Prefix width** (9999 entries per folder) shared with queues; + accepted. +- Open (future): re-capture/refresh command; capturing marked sets; + editable nesting. diff --git a/architecture/queue-persistence.md b/architecture/queue-persistence.md index a4017b7..1ba9f75 100644 --- a/architecture/queue-persistence.md +++ b/architecture/queue-persistence.md @@ -142,9 +142,11 @@ library visit — no push update needed. ### D8 — Out of scope (explicitly) -- Renaming/deleting saved queues from the TUI (`fsdy` keeps - create/rename/delete `NotSupported`); file tools work today, marking - `/queues` children deletable is future work. +- ~~Renaming/deleting saved queues from the TUI~~ — delivered by the + bookmarks feature (architecture/bookmarks.md D4): `/queues` mounts with + an editable top level (reserved: `current`), so saved queues are + renamable (`e`) and deletable (`d`). Creating nodes stays + `NotSupported`. - Making the queues directory configurable; it is derived from the config dir. - Persisting the playback *position within the track*, autoplay on diff --git a/cbd-tui/src/app/bindings.rs b/cbd-tui/src/app/bindings.rs index 2c54282..2970aba 100644 --- a/cbd-tui/src/app/bindings.rs +++ b/cbd-tui/src/app/bindings.rs @@ -68,6 +68,10 @@ pub enum Action { /// Deliberately unconfirmed: today's deletable nodes (search terms) are /// free to recreate (architecture/node-editing.md, D4). LibraryDeleteNode, + /// Open the input overlay (prefilled with the selection's title) to + /// capture the selected queueable subtree as a bookmark under + /// `/bookmarks`. No-op unless the bare selection `is_queable`. + LibraryCaptureNode, // Queue pane QueueInsertHere, QueueFirst, @@ -255,6 +259,13 @@ pub const BINDINGS: &[Binding] = &[ action: Action::LibraryToggleMark, description: "Mark/unmark selection", }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('w'), + action: Action::LibraryCaptureNode, + description: "Save selection as bookmark", + }, Binding { scope: Scope::Library, mods: KeyModifiers::NONE, @@ -535,7 +546,7 @@ mod tests { } #[test] - fn save_binds_in_the_queue_scope_only() { + fn w_saves_the_queue_or_captures_the_selection_per_pane() { assert_eq!( lookup( UiFocus::Queue, @@ -550,7 +561,7 @@ mod tests { false, key(KeyCode::Char('w'), KeyModifiers::NONE) ), - None + Some(Action::LibraryCaptureNode) ); } diff --git a/cbd-tui/src/app/library.rs b/cbd-tui/src/app/library.rs index b187e7c..4303352 100644 --- a/cbd-tui/src/app/library.rs +++ b/cbd-tui/src/app/library.rs @@ -66,6 +66,14 @@ impl Library { let item = self.list.get(self.list_state.selected()?)?; item.is_deletable.then(|| item.path.clone()) } + /// Path and title of the bare selection, if it is queueable — what `w` + /// captures as a bookmark. Marks are deliberately ignored: one capture + /// per invocation (architecture/bookmarks.md D5). + pub fn selected_queueable(&self) -> Option<(String, String)> { + let item = self.list.get(self.list_state.selected()?)?; + item.is_queable + .then(|| (item.path.clone(), item.title.clone())) + } pub fn get_selected(&self) -> Option<Vec<String>> { if self.list.iter().any(|i| i.marked) { return Some( diff --git a/cbd-tui/src/app/mod.rs b/cbd-tui/src/app/mod.rs index 1802ad7..b47d24e 100644 --- a/cbd-tui/src/app/mod.rs +++ b/cbd-tui/src/app/mod.rs @@ -89,6 +89,12 @@ pub enum MessageFromUi { /// Save the current queue under a name (server-side snapshot; appears /// as `/queues/<name>` in the library on the next visit). SaveQueue(String), + /// Capture the queueable subtree at `path` as the bookmark `name` + /// (structure-preserving snapshot under `/bookmarks/<name>`). + CaptureNode { + path: String, + name: String, + }, AppendTracks(Vec<String>), QueueTracks(Vec<String>), InsertTracks(Vec<String>, usize), @@ -128,6 +134,9 @@ pub enum InputPurpose { Rename { path: String }, /// `w`: save the current queue under the entered name. SaveQueue, + /// `w` in the library: capture the queueable subtree at `path` as a + /// bookmark. The buffer starts prefilled with the selection's title. + Capture { path: String }, } /// State of the one-line text input overlay (node creation and rename). @@ -204,6 +213,12 @@ impl App { InputPurpose::SaveQueue => { let _ = self.tx.send(MessageFromUi::SaveQueue(title)); } + InputPurpose::Capture { path } => { + let _ = self.tx.send(MessageFromUi::CaptureNode { + path: path.clone(), + name: title, + }); + } } } self.input = None; @@ -292,6 +307,16 @@ impl App { let _ = self.tx.send(MessageFromUi::DeleteNode { path }); } } + Action::LibraryCaptureNode => { + // Prefilled with the selection's title; silently ignored on + // non-queueable selections like the other gated openers. + if let Some((path, title)) = self.library.selected_queueable() { + self.input = Some(InputState { + purpose: InputPurpose::Capture { path }, + buffer: title, + }); + } + } Action::QueueInsertHere => { if let Some(selected) = self.queue.selected() { self.library.queue_insert(selected); @@ -364,6 +389,7 @@ impl App { InputPurpose::Create { .. } => "new node", InputPurpose::Rename { .. } => "rename", InputPurpose::SaveQueue => "save queue", + InputPurpose::Capture { .. } => "bookmark", }; let line = Rect::new(area.x + 1, area.y + area.height - 2, area.width - 2, 1); f.render_widget(Clear, line); @@ -736,6 +762,66 @@ mod tests { assert!(text.contains("rename: abba"), "rename overlay label"); } + /// An artists listing with one queueable child. + fn queueable_listing() -> LibraryNode { + use crabidy_core::proto::crabidy::LibraryNodeChild; + LibraryNode { + children: vec![LibraryNodeChild::new( + "/tidal/artists/1".to_string(), + "artist".to_string(), + true, + )], + ..creatable_node("/tidal/artists") + } + } + + #[test] + fn capture_opens_prefilled_only_on_queueable_selections() { + let (mut app, _rx) = app(); + // Nothing loaded: 'w' must be a no-op. + assert_eq!( + app.dispatch(Action::LibraryCaptureNode), + DispatchResult::Continue + ); + assert!(app.input.is_none()); + + // A non-queueable selection must not open the overlay. + app.library.update(search_listing(false)); + let _ = app.dispatch(Action::LibraryCaptureNode); + assert!(app.input.is_none()); + + app.library.update(queueable_listing()); + let _ = app.dispatch(Action::LibraryCaptureNode); + let input = app.input.as_ref().expect("capture overlay open"); + assert!( + matches!(&input.purpose, InputPurpose::Capture { path } if path == "/tidal/artists/1") + ); + assert_eq!(input.buffer, "artist", "prefilled with the title"); + } + + #[test] + fn capture_submits_the_trimmed_name_and_esc_cancels() { + let (mut app, rx) = app(); + app.library.update(queueable_listing()); + + let _ = app.dispatch(Action::LibraryCaptureNode); + type_str(&mut app, " favs "); + app.handle_input_key(key(crossterm::event::KeyCode::Enter)); + assert!(app.input.is_none()); + match rx.try_recv() { + Ok(MessageFromUi::CaptureNode { path, name }) => { + assert_eq!(path, "/tidal/artists/1"); + assert_eq!(name, "artist favs"); + } + other => panic!("expected CaptureNode, got {:?}", other.is_ok()), + } + + let _ = app.dispatch(Action::LibraryCaptureNode); + app.handle_input_key(key(crossterm::event::KeyCode::Esc)); + assert!(app.input.is_none()); + assert!(rx.try_recv().is_err(), "Esc must not capture"); + } + /// A one-track queue update, as the server would broadcast it. fn one_track_queue() -> crabidy_core::proto::crabidy::Queue { use crabidy_core::proto::crabidy::{Queue as ProtoQueue, Track}; diff --git a/cbd-tui/src/main.rs b/cbd-tui/src/main.rs index 30d2ad7..4b0614f 100644 --- a/cbd-tui/src/main.rs +++ b/cbd-tui/src/main.rs @@ -203,6 +203,13 @@ async fn poll( error!(name, "failed to save queue: {err}"); } } + MessageFromUi::CaptureNode { path, name } => { + // A rejected capture (bad name, over-cap subtree) must + // not tear down the poll loop either. + if let Err(err) = rpc_client.capture_library_node(path.clone(), name.clone()).await { + error!(path, name, "failed to capture subtree: {err}"); + } + } } } Some(resp) = rpc_client.update_stream.next() => { diff --git a/cbd-tui/src/rpc.rs b/cbd-tui/src/rpc.rs index 1d50929..4900923 100644 --- a/cbd-tui/src/rpc.rs +++ b/cbd-tui/src/rpc.rs @@ -1,10 +1,11 @@ use crabidy_core::proto::crabidy::{ - crabidy_service_client::CrabidyServiceClient, AppendRequest, ChangeVolumeRequest, - ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, GetLibraryNodeRequest, - GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, InitResponse, InsertRequest, - LibraryNode, NextRequest, PrevRequest, QueueRequest, RemoveRequest, RenameLibraryNodeRequest, - ReplaceRequest, RestartTrackRequest, SaveQueueRequest, SetCurrentRequest, ToggleMuteRequest, - TogglePlayRequest, ToggleRepeatRequest, ToggleShuffleRequest, + crabidy_service_client::CrabidyServiceClient, AppendRequest, CaptureLibraryNodeRequest, + ChangeVolumeRequest, ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, + GetLibraryNodeRequest, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, + InitResponse, InsertRequest, LibraryNode, NextRequest, PrevRequest, QueueRequest, + RemoveRequest, RenameLibraryNodeRequest, ReplaceRequest, RestartTrackRequest, SaveQueueRequest, + SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest, + ToggleShuffleRequest, }; use std::{collections::HashMap, error::Error, fmt, time::Duration}; @@ -220,6 +221,16 @@ impl RpcClient { Ok(()) } + pub async fn capture_library_node( + &mut self, + path: String, + name: String, + ) -> Result<(), Box<dyn Error>> { + let capture_request = Request::new(CaptureLibraryNodeRequest { path, name }); + self.client.capture_library_node(capture_request).await?; + Ok(()) + } + pub async fn replace_queue(&mut self, paths: Vec<String>) -> Result<(), Box<dyn Error>> { let replace_request = Request::new(ReplaceRequest { paths }); self.client.replace(replace_request).await?; diff --git a/crabidy-core/crabidy/v1/crabidy.proto b/crabidy-core/crabidy/v1/crabidy.proto index 9e161d2..3117cb8 100644 --- a/crabidy-core/crabidy/v1/crabidy.proto +++ b/crabidy-core/crabidy/v1/crabidy.proto @@ -20,6 +20,12 @@ service CrabidyService { // Deletes a node whose listing entry sets is_deletable. 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 + // structure-preserving snapshot under /bookmarks/<name> (folders per + // child node, link track files per track). Overwrites an existing + // bookmark of the same name. The client stays where it is — the bookmark + // shows up under /bookmarks on the next visit. + rpc CaptureLibraryNode(CaptureLibraryNodeRequest) returns (CaptureLibraryNodeResponse); // Queue rpc Queue(QueueRequest) returns (QueueResponse); @@ -96,6 +102,14 @@ message DeleteLibraryNodeResponse { LibraryNode parent = 1; } +message CaptureLibraryNodeRequest { + // Path of the queueable node (or track) to capture. + string path = 1; + // Bookmark name; becomes the top-level folder under /bookmarks. + string name = 2; +} +message CaptureLibraryNodeResponse {} + // Queue message QueueRequest { repeated string paths = 1; diff --git a/crabidy-server/src/bookmark_store.rs b/crabidy-server/src/bookmark_store.rs new file mode 100644 index 0000000..2b69e3a --- /dev/null +++ b/crabidy-server/src/bookmark_store.rs @@ -0,0 +1,382 @@ +//! Captured library subtrees ("bookmarks") on disk +//! (see `architecture/bookmarks.md`). +//! +//! Every bookmark is a folder under `<config>/crabidy/bookmarks/` that +//! mirrors the captured subtree: one order-prefixed folder per child node, +//! one order-prefixed `*.cbd-track.toml` **link** file per track. The same +//! directory is mounted read-only into the library as `/bookmarks` by an +//! `fsdy` instance (with editable top-level folders) — this module is the +//! only writer. + +use std::path::{Path, PathBuf}; + +use crabidy_core::ProviderClient; + +/// The library mount point of the bookmarks directory. +pub const BOOKMARKS_PROVIDER_ROOT: &str = "/bookmarks"; + +/// The walk aborts beyond this many directories — a runaway provider tree +/// must not fill the disk. +pub const MAX_CAPTURE_DIRS: usize = 1_000; + +/// The walk aborts beyond this many track files. +pub const MAX_CAPTURE_TRACKS: usize = 20_000; + +/// The bookmarks directory: `bookmarks/` inside the crabidy config +/// directory. `None` when the platform has no config directory. +pub fn bookmarks_dir() -> Option<PathBuf> { + dirs::config_dir().map(|d| d.join("crabidy").join("bookmarks")) +} + +/// Errors from validating or writing a capture. +/// +/// At the RPC boundary: `InvalidName`/`BadSource` → `invalid_argument`, +/// `TooLarge` → `failed_precondition`, the rest → `internal`. Messages +/// carry names and paths, never file contents. +#[derive(Debug, thiserror::Error)] +pub enum CaptureError { + #[error("invalid bookmark name: {0}")] + InvalidName(&'static str), + #[error("bookmarks are disabled")] + Disabled, + #[error("the source path cannot be captured: {0}")] + BadSource(String), + #[error("the subtree is too large to capture ({0})")] + TooLarge(&'static str), + #[error("cannot write bookmark: {0}")] + Io(#[from] std::io::Error), + #[error(transparent)] + TrackFile(#[from] fsdy::TrackFileError), +} + +/// Writes captured subtrees. All I/O is `tokio::fs`; the whole bookmark is +/// built as a hidden temp sibling and swapped into place, so a crash never +/// leaves a half-written bookmark next to intact ones. +#[derive(Debug)] +pub struct BookmarkStore { + dir: PathBuf, +} + +impl BookmarkStore { + /// Opens the store at `dir`, creating the directory (and parents) if + /// missing. + pub async fn open(dir: PathBuf) -> Result<Self, std::io::Error> { + tokio::fs::create_dir_all(&dir).await?; + Ok(Self { dir }) + } + + /// The store directory (what the `/bookmarks` provider instance + /// mounts). + pub fn dir(&self) -> &Path { + &self.dir + } + + /// Captures the subtree at `source_path` as the bookmark `name`, + /// overwriting an existing bookmark of that name. + /// + /// Walks `client` (the orchestrator, so any provider is reachable) + /// iteratively in pre-order: every child node becomes an + /// order-prefixed folder, every track an order-prefixed link file + /// ([`fsdy::TrackFile::from_track`]). A `source_path` that is itself a + /// track captures as a folder with one file. Aborts with + /// [`CaptureError::TooLarge`] beyond [`MAX_CAPTURE_DIRS`] / + /// [`MAX_CAPTURE_TRACKS`]; an unreadable source is + /// [`CaptureError::BadSource`]. Never panics on provider contents. + pub async fn capture<C>( + &self, + client: &C, + source_path: &str, + name: &str, + ) -> Result<(), CaptureError> + where + C: ProviderClient + Sync, + { + self.capture_with_caps( + client, + source_path, + name, + MAX_CAPTURE_DIRS, + MAX_CAPTURE_TRACKS, + ) + .await + } + + /// [`Self::capture`] with explicit caps — the seam the cap tests use. + async fn capture_with_caps<C>( + &self, + client: &C, + source_path: &str, + name: &str, + max_dirs: usize, + max_tracks: usize, + ) -> Result<(), CaptureError> + where + C: ProviderClient + Sync, + { + let name = fsdy::validate_folder_name(name, &[]).map_err(CaptureError::InvalidName)?; + let tmp = self.dir.join(format!(".tmp-{name}")); + let written = self + .write_capture(client, source_path, &tmp, max_dirs, max_tracks) + .await; + if let Err(err) = written { + // Every failure path removes the temp folder: nothing + // half-written survives, not even hidden. + let _ = tokio::fs::remove_dir_all(&tmp).await; + return Err(err); + } + let target = self.dir.join(name); + if tokio::fs::try_exists(&target).await? { + tokio::fs::remove_dir_all(&target).await?; + } + tokio::fs::rename(&tmp, &target).await?; + Ok(()) + } + + /// Builds the mirrored tree inside `tmp`. All-or-nothing: any provider + /// or write failure aborts the whole capture (the caller removes + /// `tmp`) — a bookmark that *looks* complete must *be* complete. + async fn write_capture<C>( + &self, + client: &C, + source_path: &str, + tmp: &Path, + max_dirs: usize, + max_tracks: usize, + ) -> Result<(), CaptureError> + where + C: ProviderClient + Sync, + { + // A leftover temp folder from a crashed or racing capture is stale. + if tokio::fs::try_exists(tmp).await? { + tokio::fs::remove_dir_all(tmp).await?; + } + tokio::fs::create_dir_all(tmp).await?; + + // A track source captures as a folder with one file. + if client.is_track_path(source_path) { + let track = client + .get_metadata_for_track(source_path) + .await + .map_err(|err| CaptureError::BadSource(format!("{source_path}: {err}")))?; + let text = fsdy::TrackFile::from_track(&track).to_toml()?; + let file = tmp.join(fsdy::track_file_name(0, &track.title)); + tokio::fs::write(file, text).await?; + return Ok(()); + } + + let mut dirs = 1usize; + let mut tracks = 0usize; + // Iterative pre-order: a deep provider tree must not overflow the + // stack. Each entry pairs a library path with its mirror folder. + let mut worklist: Vec<(String, PathBuf)> = + vec![(source_path.to_string(), tmp.to_path_buf())]; + while let Some((lib_path, dir)) = worklist.pop() { + let node = client + .get_lib_node(&lib_path) + .await + .map_err(|err| CaptureError::BadSource(format!("{lib_path}: {err}")))?; + for (index, track) in node.tracks.iter().enumerate() { + tracks += 1; + if tracks > max_tracks { + return Err(CaptureError::TooLarge("too many tracks")); + } + let text = fsdy::TrackFile::from_track(track).to_toml()?; + let file = dir.join(fsdy::track_file_name(index, &track.title)); + tokio::fs::write(file, text).await?; + } + for (index, child) in node.children.iter().enumerate() { + dirs += 1; + if dirs > max_dirs { + return Err(CaptureError::TooLarge("too many directories")); + } + let child_dir = dir.join(fsdy::dir_name(index, &child.title)); + tokio::fs::create_dir(&child_dir).await?; + worklist.push((child.path.clone(), child_dir)); + } + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use std::path::Path; + use tempfile::TempDir; + + /// A real fsdy instance as the capture source: an artist with two + /// albums holding url tracks, plus one link track pointing at Tidal. + async fn source() -> (fsdy::Client, TempDir) { + let dir = TempDir::new().expect("source tempdir"); + let al1 = dir.path().join("artist/Album One"); + let al2 = dir.path().join("artist/Album Two"); + fs::create_dir_all(&al1).expect("mkdir"); + fs::create_dir_all(&al2).expect("mkdir"); + let url = + |t: &str| format!("title = {t:?}\n[playable]\nurl = \"https://example.org/s.mp3\"\n"); + fs::write(al1.join("01 one.cbd-track.toml"), url("one")).expect("write"); + fs::write(al1.join("02 two.cbd-track.toml"), url("two")).expect("write"); + fs::write( + al2.join("01 linked.cbd-track.toml"), + "title = \"linked\"\n[playable]\nlink = \"/tidal/artists/1/2\"\n", + ) + .expect("write"); + let client = fsdy::Client::new("/fs", dir.path().to_path_buf()).expect("source instance"); + (client, dir) + } + + async fn store() -> (BookmarkStore, TempDir) { + let dir = TempDir::new().expect("store tempdir"); + let store = BookmarkStore::open(dir.path().join("bookmarks")) + .await + .expect("open creates the directory"); + (store, dir) + } + + fn visible(dir: &Path) -> Vec<String> { + let mut names: Vec<String> = fs::read_dir(dir) + .expect("dir") + .map(|e| e.expect("entry").file_name().to_string_lossy().into_owned()) + .filter(|n| !n.starts_with('.')) + .collect(); + names.sort_by_key(|n| n.to_lowercase()); + names + } + + #[tokio::test] + async fn capture_mirrors_a_subtree_with_order_prefixes() { + let (client, _src) = source().await; + let (store, _dir) = store().await; + store + .capture(&client, "/fs/artist", "faves") + .await + .expect("capture"); + + let root = store.dir().join("faves"); + assert_eq!(visible(&root), vec!["0001 Album One", "0002 Album Two"]); + let album1 = visible(&root.join("0001 Album One")); + assert_eq!( + album1, + vec![ + "0001 one.cbd-track.toml".to_string(), + "0002 two.cbd-track.toml".into() + ] + ); + // Entries are link files; the url track links back to its /fs path, + // the link track re-links to its original target (no chains). + let one = fs::read_to_string(root.join("0001 Album One/0001 one.cbd-track.toml")) + .expect("read entry"); + let one = fsdy::TrackFile::parse(&one).expect("entry parses"); + assert_eq!( + one.to_track("/bookmarks/irrelevant").path, + "/fs/artist/Album%20One/01%20one.cbd-track.toml" + ); + let linked = fs::read_to_string(root.join("0002 Album Two/0001 linked.cbd-track.toml")) + .expect("read entry"); + let linked = fsdy::TrackFile::parse(&linked).expect("entry parses"); + assert_eq!( + linked.to_track("/bookmarks/irrelevant").path, + "/tidal/artists/1/2" + ); + } + + #[tokio::test] + async fn captured_trees_replay_through_a_bookmarks_instance() { + let (client, _src) = source().await; + let (store, _dir) = store().await; + store + .capture(&client, "/fs/artist", "faves") + .await + .expect("capture"); + + let bookmarks = fsdy::Client::new(BOOKMARKS_PROVIDER_ROOT, store.dir().to_path_buf()) + .expect("bookmarks instance"); + let (chunk_tx, chunk_rx) = flume::bounded(8); + bookmarks + .resolve_tracks_into("/bookmarks/faves", chunk_tx) + .await + .expect("resolve"); + let titles: Vec<String> = chunk_rx.into_iter().flatten().map(|t| t.title).collect(); + // Pre-order over the mirrored structure == source listing order. + assert_eq!( + titles, + vec!["one".to_string(), "two".into(), "linked".into()] + ); + } + + #[tokio::test] + async fn capturing_a_single_track_writes_one_file() { + let (client, _src) = source().await; + let (store, _dir) = store().await; + store + .capture( + &client, + "/fs/artist/Album%20One/01%20one.cbd-track.toml", + "just one", + ) + .await + .expect("capture track"); + let entries = visible(&store.dir().join("just one")); + assert_eq!(entries, vec!["0001 one.cbd-track.toml"]); + } + + #[tokio::test] + async fn capture_validates_names_and_overwrites() { + let (client, _src) = source().await; + let (store, _dir) = store().await; + for bad in ["", " ", "a/b", ".hidden"] { + assert!( + matches!( + store.capture(&client, "/fs/artist", bad).await, + Err(CaptureError::InvalidName(_)) + ), + "name {bad:?} must be rejected" + ); + } + store + .capture(&client, "/fs/artist", "faves") + .await + .expect("first capture"); + store + .capture(&client, "/fs/artist/Album%20Two", "faves") + .await + .expect("overwrite"); + // The overwrite fully replaces the older, larger capture. + assert_eq!( + visible(&store.dir().join("faves")), + vec!["0001 linked.cbd-track.toml"] + ); + } + + #[tokio::test] + async fn capture_rejects_an_unreadable_source() { + let (client, _src) = source().await; + let (store, _dir) = store().await; + assert!(matches!( + store.capture(&client, "/fs/nope", "x").await, + Err(CaptureError::BadSource(_)) + )); + } + + #[tokio::test] + async fn capture_aborts_over_the_caps_and_cleans_up() { + let (client, _src) = source().await; + let (store, _dir) = store().await; + // The tree has 3 directories (artist + 2 albums); a 2-dir cap trips. + let err = store + .capture_with_caps(&client, "/fs/artist", "big", 2, MAX_CAPTURE_TRACKS) + .await + .expect_err("over the dir cap"); + assert!(matches!(err, CaptureError::TooLarge(_))); + // ... same for the track cap. + let err = store + .capture_with_caps(&client, "/fs/artist", "big", MAX_CAPTURE_DIRS, 1) + .await + .expect_err("over the track cap"); + assert!(matches!(err, CaptureError::TooLarge(_))); + // Nothing half-written survives, not even hidden temp folders. + let leftovers = fs::read_dir(store.dir()).expect("store dir").count(); + assert_eq!(leftovers, 0); + } +} diff --git a/crabidy-server/src/lib.rs b/crabidy-server/src/lib.rs index 8b51b64..911c0fe 100644 --- a/crabidy-server/src/lib.rs +++ b/crabidy-server/src/lib.rs @@ -1,3 +1,4 @@ +pub mod bookmark_store; pub mod queue_store; use crabidy_core::proto::crabidy::{Queue, Track}; diff --git a/crabidy-server/src/main.rs b/crabidy-server/src/main.rs index 43fe112..19ee676 100644 --- a/crabidy-server/src/main.rs +++ b/crabidy-server/src/main.rs @@ -204,6 +204,14 @@ pub enum ProviderCommand { path: String, result_tx: flume::Sender<Result<LibraryNode, ProviderError>>, }, + /// Captures the queueable subtree at `path` as the bookmark `name` + /// (see `architecture/bookmarks.md` D1–D3). Handled on a spawned task — + /// a large walk must not block the orchestrator loop. + CaptureLibraryNode { + path: String, + name: String, + result_tx: flume::Sender<Result<(), crabidy_server::bookmark_store::CaptureError>>, + }, } impl ProviderCommand { @@ -215,6 +223,7 @@ impl ProviderCommand { Self::CreateLibraryNode { .. } => "create_library_node", Self::RenameLibraryNode { .. } => "rename_library_node", Self::DeleteLibraryNode { .. } => "delete_library_node", + Self::CaptureLibraryNode { .. } => "capture_library_node", } } } diff --git a/crabidy-server/src/provider.rs b/crabidy-server/src/provider.rs index 85d2030..c17e8f7 100644 --- a/crabidy-server/src/provider.rs +++ b/crabidy-server/src/provider.rs @@ -4,7 +4,8 @@ use crabidy_core::{ proto::crabidy::{LibraryNode, LibraryNodeChild, Track}, ProviderClient, ProviderError, }; -use crabidy_server::queue_store::QUEUES_PROVIDER_ROOT; +use crabidy_server::bookmark_store::{BookmarkStore, BOOKMARKS_PROVIDER_ROOT}; +use crabidy_server::queue_store::{CURRENT_QUEUE_NAME, QUEUES_PROVIDER_ROOT}; use std::{fs, path::PathBuf, sync::Arc}; use tracing::{debug, debug_span, error, instrument, warn, Instrument}; @@ -20,6 +21,13 @@ pub struct ProviderOrchestrator { /// `/queues` (architecture/queue-persistence.md D1). `None` without a /// config directory — the server then runs without `/queues`. queues_client: Option<Arc<fsdy::Client>>, + /// Third `fsdy` instance over the bookmarks folder, mounted at + /// `/bookmarks` (architecture/bookmarks.md D1). `None` without a + /// config directory. + bookmarks_client: Option<Arc<fsdy::Client>>, + /// The capture writer; `None` disables `CaptureLibraryNode` (and the + /// `/bookmarks` mount goes with it). + bookmark_store: Option<Arc<BookmarkStore>>, } /// Whether a path belongs to the filesystem provider. @@ -33,6 +41,11 @@ fn queues_owns(path: &str) -> bool { path == QUEUES_PROVIDER_ROOT || path.starts_with("/queues/") } +/// Whether a path belongs to the bookmarks provider instance. +fn bookmarks_owns(path: &str) -> bool { + path == BOOKMARKS_PROVIDER_ROOT || path.starts_with("/bookmarks/") +} + impl ProviderOrchestrator { /// The fs client, or `MalformedPath` (with a warning) when the /// provider is disabled — a `/fs` path then has no owner. @@ -51,6 +64,15 @@ impl ProviderOrchestrator { ProviderError::MalformedPath }) } + + /// The bookmarks client, or `MalformedPath` (with a warning) when the + /// instance is disabled — a `/bookmarks` path then has no owner. + fn bookmarks_provider(&self) -> Result<&fsdy::Client, ProviderError> { + self.bookmarks_client.as_deref().ok_or_else(|| { + warn!("bookmarks library is disabled"); + ProviderError::MalformedPath + }) + } pub fn run(self) { tokio::spawn(async move { // Behind an Arc so long-running resolves can be spawned onto @@ -123,6 +145,31 @@ impl ProviderOrchestrator { error!("failed to send delete_library_node result: {err}"); } } + ProviderCommand::CaptureLibraryNode { + path, + name, + result_tx, + } => { + // Spawned: capturing a large artist walks many provider + // nodes and must not block this loop (the walk itself + // calls back into `get_lib_node` via `this`). + let this = Arc::clone(&self); + tokio::spawn( + async move { + let result = match &this.bookmark_store { + Some(store) => store.capture(&*this, &path, &name).await, + None => Err(crabidy_server::bookmark_store::CaptureError::Disabled), + }; + if let Err(err) = &result { + warn!(path, name, "cannot capture subtree: {err}"); + } + if let Err(err) = result_tx.send_async(result).await { + error!("failed to send capture_library_node result: {err}"); + } + } + .in_current_span(), + ); + } } } } @@ -174,10 +221,13 @@ impl ProviderClient for ProviderOrchestrator { }; // The queues instance mounts the folder the playback side persists // into; a folder that does not exist yet lists as empty-on-arrival - // (created by QueueStore::open in main). + // (created by QueueStore::open in main). Saved queues are renamable + // and deletable; the auto-persisted `current` stays untouchable. let queues_client = match crabidy_server::queue_store::queues_dir() { Some(dir) => match fsdy::Client::new(QUEUES_PROVIDER_ROOT, dir) { - Ok(client) => Some(Arc::new(client)), + Ok(client) => Some(Arc::new( + client.with_editable_top_level(&[CURRENT_QUEUE_NAME]), + )), Err(err) => { warn!("queues library disabled: {err}"); None @@ -188,6 +238,31 @@ impl ProviderClient for ProviderOrchestrator { None } }; + // Bookmarks: the orchestrator owns the store (it is the capture + // writer) and mounts the same folder read-only. Non-fatal like the + // other local providers. + let bookmark_store = match crabidy_server::bookmark_store::bookmarks_dir() { + Some(dir) => match BookmarkStore::open(dir).await { + Ok(store) => Some(Arc::new(store)), + Err(err) => { + warn!("bookmarks disabled: {err}"); + None + } + }, + None => { + warn!("bookmarks disabled: no config directory"); + None + } + }; + let bookmarks_client = bookmark_store.as_ref().and_then(|store| { + match fsdy::Client::new(BOOKMARKS_PROVIDER_ROOT, store.dir().to_path_buf()) { + Ok(client) => Some(Arc::new(client.with_editable_top_level(&[]))), + Err(err) => { + warn!("bookmarks library disabled: {err}"); + None + } + } + }); let (provider_tx, provider_rx) = flume::bounded(100); Ok(Self { provider_rx, @@ -195,6 +270,8 @@ impl ProviderClient for ProviderOrchestrator { tidal_client, fs_client, queues_client, + bookmarks_client, + bookmark_store, }) } @@ -219,6 +296,12 @@ impl ProviderClient for ProviderOrchestrator { .as_ref() .is_some_and(|queues| queues.is_track_path(path)); } + if bookmarks_owns(path) { + return self + .bookmarks_client + .as_ref() + .is_some_and(|bookmarks| bookmarks.is_track_path(path)); + } false } @@ -233,6 +316,12 @@ impl ProviderClient for ProviderOrchestrator { if queues_owns(track_path) { return self.queues_provider()?.get_urls_for_track(track_path).await; } + if bookmarks_owns(track_path) { + return self + .bookmarks_provider()? + .get_urls_for_track(track_path) + .await; + } warn!(path = track_path, "no provider owns this track path"); Err(ProviderError::MalformedPath) } @@ -251,6 +340,12 @@ impl ProviderClient for ProviderOrchestrator { .get_metadata_for_track(track_path) .await; } + if bookmarks_owns(track_path) { + return self + .bookmarks_provider()? + .get_metadata_for_track(track_path) + .await; + } warn!(path = track_path, "no provider owns this track path"); Err(ProviderError::MalformedPath) } @@ -270,6 +365,14 @@ impl ProviderClient for ProviderOrchestrator { LibraryNodeChild::new(QUEUES_PROVIDER_ROOT.to_owned(), "queues".to_owned(), false); root_node.children.push(child); } + if self.bookmarks_client.is_some() { + let child = LibraryNodeChild::new( + BOOKMARKS_PROVIDER_ROOT.to_owned(), + "bookmarks".to_owned(), + false, + ); + root_node.children.push(child); + } root_node } @@ -288,6 +391,9 @@ impl ProviderClient for ProviderOrchestrator { if queues_owns(path) { return self.queues_provider()?.get_lib_node(path).await; } + if bookmarks_owns(path) { + return self.bookmarks_provider()?.get_lib_node(path).await; + } warn!(path, "no provider owns this path"); Err(ProviderError::MalformedPath) } @@ -315,6 +421,12 @@ impl ProviderClient for ProviderOrchestrator { .create_lib_node(parent_path, title) .await; } + if bookmarks_owns(parent_path) { + return self + .bookmarks_provider()? + .create_lib_node(parent_path, title) + .await; + } warn!(parent_path, "no provider supports creating nodes here"); Err(ProviderError::NotSupported) } @@ -339,6 +451,12 @@ impl ProviderClient for ProviderOrchestrator { .rename_lib_node(path, new_title) .await; } + if bookmarks_owns(path) { + return self + .bookmarks_provider()? + .rename_lib_node(path, new_title) + .await; + } warn!(path, "no provider supports renaming this node"); Err(ProviderError::NotSupported) } @@ -366,6 +484,12 @@ impl ProviderClient for ProviderOrchestrator { .resolve_tracks_into(path, chunk_tx) .await; } + if bookmarks_owns(path) { + return self + .bookmarks_provider()? + .resolve_tracks_into(path, chunk_tx) + .await; + } warn!(path, "no provider owns this path"); Err(ProviderError::MalformedPath) } @@ -383,6 +507,9 @@ impl ProviderClient for ProviderOrchestrator { if queues_owns(path) { return self.queues_provider()?.delete_lib_node(path).await; } + if bookmarks_owns(path) { + return self.bookmarks_provider()?.delete_lib_node(path).await; + } warn!(path, "no provider supports deleting this node"); Err(ProviderError::NotSupported) } diff --git a/crabidy-server/src/queue_store.rs b/crabidy-server/src/queue_store.rs index e24e302..6d344a3 100644 --- a/crabidy-server/src/queue_store.rs +++ b/crabidy-server/src/queue_store.rs @@ -102,26 +102,9 @@ impl QueueStore { /// with a dot (hidden folders are invisible to listings), and the /// reserved [`CURRENT_QUEUE_NAME`]. pub fn validate_name(name: &str) -> Result<&str, SaveQueueError> { - let name = name.trim(); - if name.is_empty() { - return Err(SaveQueueError::InvalidName("must not be empty")); - } - if name.contains(['/', '\\', '\0']) { - return Err(SaveQueueError::InvalidName( - "must not contain a path separator or NUL", - )); - } - if name.starts_with('.') { - return Err(SaveQueueError::InvalidName( - "must not start with a dot (hidden folders are invisible)", - )); - } - if name == CURRENT_QUEUE_NAME { - return Err(SaveQueueError::InvalidName( - "is reserved for the automatically maintained queue", - )); - } - Ok(name) + // The shared fs-provider naming rules, with the auto-persisted + // queue's folder reserved. + fsdy::validate_folder_name(name, &[CURRENT_QUEUE_NAME]).map_err(SaveQueueError::InvalidName) } /// Saves `snapshot` as the named queue, overwriting an existing one. diff --git a/crabidy-server/src/rpc.rs b/crabidy-server/src/rpc.rs index 545299f..e08a13d 100644 --- a/crabidy-server/src/rpc.rs +++ b/crabidy-server/src/rpc.rs @@ -1,19 +1,21 @@ use crate::{PlaybackCommand, PlaybackMessage, ProviderCommand, ProviderMessage}; use crabidy_core::proto::crabidy::{ crabidy_service_server::CrabidyService, get_update_stream_response::Update as StreamUpdate, - AppendRequest, AppendResponse, ChangeVolumeRequest, ChangeVolumeResponse, ClearQueueRequest, - ClearQueueResponse, CreateLibraryNodeRequest, CreateLibraryNodeResponse, - DeleteLibraryNodeRequest, DeleteLibraryNodeResponse, GetLibraryNodeRequest, - GetLibraryNodeResponse, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, - InitResponse, InsertRequest, InsertResponse, NextRequest, NextResponse, PrevRequest, - PrevResponse, QueueRequest, QueueResponse, RemoveRequest, RemoveResponse, - RenameLibraryNodeRequest, RenameLibraryNodeResponse, ReplaceRequest, ReplaceResponse, - RestartTrackRequest, RestartTrackResponse, SaveQueueRequest, SaveQueueResponse, - SetCurrentRequest, SetCurrentResponse, StopRequest, StopResponse, ToggleMuteRequest, - ToggleMuteResponse, TogglePlayRequest, TogglePlayResponse, ToggleRepeatRequest, - ToggleRepeatResponse, ToggleShuffleRequest, ToggleShuffleResponse, + AppendRequest, AppendResponse, CaptureLibraryNodeRequest, CaptureLibraryNodeResponse, + ChangeVolumeRequest, ChangeVolumeResponse, ClearQueueRequest, ClearQueueResponse, + CreateLibraryNodeRequest, CreateLibraryNodeResponse, DeleteLibraryNodeRequest, + DeleteLibraryNodeResponse, GetLibraryNodeRequest, GetLibraryNodeResponse, + GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, InitResponse, InsertRequest, + InsertResponse, NextRequest, NextResponse, PrevRequest, PrevResponse, QueueRequest, + QueueResponse, RemoveRequest, RemoveResponse, RenameLibraryNodeRequest, + RenameLibraryNodeResponse, ReplaceRequest, ReplaceResponse, RestartTrackRequest, + RestartTrackResponse, SaveQueueRequest, SaveQueueResponse, SetCurrentRequest, + SetCurrentResponse, StopRequest, StopResponse, ToggleMuteRequest, ToggleMuteResponse, + TogglePlayRequest, TogglePlayResponse, ToggleRepeatRequest, ToggleRepeatResponse, + ToggleShuffleRequest, ToggleShuffleResponse, }; use crabidy_core::ProviderError; +use crabidy_server::bookmark_store::CaptureError; use crabidy_server::queue_store::SaveQueueError; use std::pin::Pin; use tokio_stream::StreamExt; @@ -374,6 +376,53 @@ impl CrabidyService for RpcService { Ok(Response::new(Box::pin(output_stream))) } + /// Captures a queueable subtree as a bookmark via the provider loop + /// (structure-preserving snapshot under `/bookmarks/<name>`). + /// + /// Error mapping is part of the contract: invalid name or + /// uncapturable source → `invalid_argument`; an over-cap subtree or + /// disabled bookmarks → `failed_precondition`; walk/write failures → + /// `internal`. + #[instrument(skip(self, request), fields(path, name))] + async fn capture_library_node( + &self, + request: Request<CaptureLibraryNodeRequest>, + ) -> Result<Response<CaptureLibraryNodeResponse>, Status> { + let CaptureLibraryNodeRequest { path, name } = request.into_inner(); + tracing::Span::current().record("path", path.as_str()); + tracing::Span::current().record("name", name.as_str()); + debug!("received capture_library_node request"); + let (result_tx, result_rx) = flume::bounded(1); + self.provider_tx + .send_async(ProviderMessage::new(ProviderCommand::CaptureLibraryNode { + path, + name, + result_tx, + })) + .await + .map_err(|err| { + error!("provider channel closed: {err}"); + Status::internal("provider unavailable") + })?; + let result = result_rx.recv_async().await.map_err(|err| { + error!("no reply from provider: {err}"); + Status::internal("provider did not reply") + })?; + match result { + Ok(()) => Ok(Response::new(CaptureLibraryNodeResponse {})), + Err(err @ (CaptureError::InvalidName(_) | CaptureError::BadSource(_))) => { + Err(Status::invalid_argument(err.to_string())) + } + Err(err @ (CaptureError::TooLarge(_) | CaptureError::Disabled)) => { + Err(Status::failed_precondition(err.to_string())) + } + Err(err) => { + error!("capture_library_node failed: {err}"); + Err(Status::internal("cannot capture the subtree")) + } + } + } + /// Saves the current queue under a name (persisted queues, visible as /// `/queues/<name>` in the library). /// diff --git a/fsdy/src/lib.rs b/fsdy/src/lib.rs index 0dc7f38..345d1a9 100644 --- a/fsdy/src/lib.rs +++ b/fsdy/src/lib.rs @@ -210,16 +210,15 @@ impl TrackFile { } } -/// File name for the serialized queue entry at zero-based `index`: -/// a zero-padded four-digit one-based order prefix, a sanitized `title`, -/// and [`TRACK_FILE_SUFFIX`] — e.g. `0001 Bohemian Rhapsody.cbd-track.toml`. +/// The shared name builder behind [`track_file_name`] and [`dir_name`]: a +/// zero-padded four-digit one-based order prefix plus a sanitized `title`. /// -/// The prefix makes the case-insensitive listing sort reproduce queue order -/// (wrong beyond 9999 tracks — accepted, see +/// The prefix makes the case-insensitive listing sort reproduce the +/// source's order (wrong beyond 9999 entries — accepted, see /// `architecture/queue-persistence.md`). Sanitizing replaces path -/// separators and NUL, strips leading dots (hidden files are skipped by +/// separators and NUL, strips leading dots (hidden entries are skipped by /// listings), and falls back to `track` for an empty result. Never panics. -pub fn track_file_name(index: usize, title: &str) -> String { +fn ordered_name(index: usize, title: &str) -> String { let sanitized: String = title .chars() .map(|c| { @@ -236,7 +235,45 @@ pub fn track_file_name(index: usize, title: &str) -> String { } else { sanitized }; - format!("{:04} {name}{TRACK_FILE_SUFFIX}", index + 1) + format!("{:04} {name}", index + 1) +} + +/// File name for the serialized queue entry at zero-based `index`: +/// [`ordered_name`] plus [`TRACK_FILE_SUFFIX`] — e.g. +/// `0001 Bohemian Rhapsody.cbd-track.toml`. +pub fn track_file_name(index: usize, title: &str) -> String { + format!("{}{TRACK_FILE_SUFFIX}", ordered_name(index, title)) +} + +/// Folder name for the captured child node at zero-based `index`: the same +/// prefix and sanitizer as [`track_file_name`], without the suffix — e.g. +/// `0001 News of the World`. +pub fn dir_name(index: usize, title: &str) -> String { + ordered_name(index, title) +} + +/// Validates a user-supplied folder name for an fs-provider tree (bookmark +/// or saved-queue names, rename targets), returning the trimmed name. +/// +/// Rejected (with a human-readable reason): empty after trimming, +/// containing `/`, `\` or NUL, starting with a dot (hidden folders are +/// invisible to listings), or matching one of `reserved` (e.g. the +/// auto-persisted `current` queue). +pub fn validate_folder_name<'a>(name: &'a str, reserved: &[&str]) -> Result<&'a str, &'static str> { + let name = name.trim(); + if name.is_empty() { + return Err("must not be empty"); + } + if name.contains(['/', '\\', '\0']) { + return Err("must not contain a path separator or NUL"); + } + if name.starts_with('.') { + return Err("must not start with a dot (hidden folders are invisible)"); + } + if reserved.contains(&name) { + return Err("is a reserved name"); + } + Ok(name) } /// The filesystem provider. @@ -255,6 +292,13 @@ pub struct Client { root: PathBuf, /// First library path segment this instance owns, e.g. `/fs`. provider_root: String, + /// Whether direct children of the instance root may be renamed and + /// deleted through the library (bookmarks, saved queues). Off by + /// default — `/fs` trees are managed with file tools. + editable_top_level: bool, + /// Folder names exempt from top-level editing (e.g. the auto-persisted + /// `current` queue) and rejected as rename targets. + reserved: Vec<String>, } impl Client { @@ -282,9 +326,23 @@ impl Client { Ok(Self { root: disk_root, provider_root: provider_root.to_string(), + editable_top_level: false, + reserved: Vec::new(), }) } + /// Enables renaming and deleting the instance root's direct child + /// folders through the library (`is_editable`/`is_deletable` on the + /// root listing, [`ProviderClient::rename_lib_node`] / + /// [`ProviderClient::delete_lib_node`]). Names in `reserved` keep + /// their immutability and are rejected as rename targets. Deeper + /// levels always stay immutable (architecture/bookmarks.md D4). + pub fn with_editable_top_level(mut self, reserved: &[&str]) -> Self { + self.editable_top_level = true; + self.reserved = reserved.iter().map(|s| s.to_string()).collect(); + self + } + /// The provider root this instance owns (e.g. `/fs`), as configured /// through [`Self::new`]. pub fn provider_root(&self) -> &str { @@ -391,12 +449,20 @@ impl Client { tracks.push(file.to_track(&child_lib)); } } + // Only an editable instance's direct root children are modifiable + // (architecture/bookmarks.md D4); reserved names (the auto-persisted + // `current` queue) keep their immutability. + let editable_here = self.editable_top_level && lib_path == self.provider_root; let children = dir_names .into_iter() .map(|name| { let child_lib = crabidy_core::join_path(lib_path, &crabidy_core::encode_segment(&name)); - LibraryNodeChild::new(child_lib, name, true) + 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 }) .collect(); @@ -419,6 +485,31 @@ impl Client { }) } + /// The on-disk folder and decoded name of `path`, if it is a + /// modifiable top-level child of this instance: the editable option is + /// on, the path addresses a direct child folder of the instance root, + /// and the name is not reserved. Everything else is + /// [`ProviderError::NotSupported`] — exactly matching the + /// `is_editable`/`is_deletable` flags the listing advertised. + fn editable_child_dir(&self, path: &str) -> Result<(PathBuf, String), ProviderError> { + if !self.editable_top_level || self.is_track_path(path) { + return Err(ProviderError::NotSupported); + } + let rest = path + .strip_prefix(self.provider_root.as_str()) + .and_then(|rest| rest.strip_prefix('/')) + .ok_or(ProviderError::NotSupported)?; + if rest.is_empty() || rest.contains('/') { + return Err(ProviderError::NotSupported); + } + let name = crabidy_core::decode_segment(rest); + if self.reserved.contains(&name) { + return Err(ProviderError::NotSupported); + } + // The single traversal-validation site still guards the segment. + Ok((self.disk_path(path)?, name)) + } + /// 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. @@ -559,18 +650,67 @@ impl ProviderClient for Client { Err(ProviderError::NotSupported) } - /// Not supported — track files are renamed with normal file tools. + /// Renames a modifiable top-level folder (editable instances only, see + /// [`Self::with_editable_top_level`]); everything else is + /// [`ProviderError::NotSupported`]. + /// + /// The new title is validated like every store name (reserved names + /// rejected as targets); renaming onto an existing sibling is + /// [`ProviderError::InvalidInput`] — folders never merge. Renaming to + /// the current name is a no-op. Returns the renamed node at its new + /// path. async fn rename_lib_node( &self, - _path: &str, - _new_title: &str, + path: &str, + new_title: &str, ) -> Result<LibraryNode, ProviderError> { - Err(ProviderError::NotSupported) + let (disk, name) = self.editable_child_dir(path)?; + let reserved: Vec<&str> = self.reserved.iter().map(String::as_str).collect(); + let new_name = validate_folder_name(new_title, &reserved).map_err(|reason| { + warn!(path, new_title, "rejecting rename: name {reason}"); + ProviderError::InvalidInput + })?; + let new_path = + crabidy_core::join_path(&self.provider_root, &crabidy_core::encode_segment(new_name)); + if new_name == name { + return self.get_lib_node(&new_path).await; + } + // `validate_folder_name` rejected separators, NUL, and dot + // prefixes, so the target is a plain sibling name. + let target = self.root.join(new_name); + let taken = tokio::fs::try_exists(&target).await.map_err(|err| { + warn!(path, "cannot check rename target: {err}"); + ProviderError::InternalError + })?; + if taken { + warn!(path, new_name, "rejecting rename onto an existing folder"); + return Err(ProviderError::InvalidInput); + } + tokio::fs::rename(&disk, &target).await.map_err(|err| { + warn!(path, "cannot rename folder: {err}"); + ProviderError::InternalError + })?; + self.get_lib_node(&new_path).await } - /// Not supported — track files are deleted with normal file tools. - async fn delete_lib_node(&self, _path: &str) -> Result<LibraryNode, ProviderError> { - Err(ProviderError::NotSupported) + /// 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. + 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); + } + } + let root = self.provider_root.clone(); + self.get_lib_node(&root).await } // `resolve_tracks_into` deliberately keeps the default pre-order walk: @@ -1034,6 +1174,158 @@ mod tests { assert_eq!(reparsed.to_track("/queues/x.cbd-track.toml"), track); } + // ---- mutable top level (bookmarks / saved queues) --------------------- + + /// An editable instance over a root with two folders (one reserved) and + /// a nested subfolder. + async fn editable_client() -> (Client, TempDir) { + let dir = TempDir::new().expect("tempdir"); + fs::create_dir_all(dir.path().join("current")).expect("mkdir"); + fs::create_dir_all(dir.path().join("road trip/album")).expect("mkdir"); + let client = Client::new("/queues", dir.path().to_path_buf()) + .expect("instance") + .with_editable_top_level(&["current"]); + (client, dir) + } + + #[tokio::test] + async fn editable_instances_flag_only_unreserved_top_level_folders() { + let (client, _dir) = editable_client().await; + let root = client.get_lib_node("/queues").await.expect("root"); + let current = &root.children[0]; + let saved = &root.children[1]; + assert_eq!(current.title, "current"); + assert!(!current.is_editable && !current.is_deletable, "reserved"); + assert_eq!(saved.title, "road trip"); + assert!(saved.is_editable && saved.is_deletable); + + // Deeper levels stay immutable. + let nested = client + .get_lib_node("/queues/road%20trip") + .await + .expect("nested listing"); + assert!(nested + .children + .iter() + .all(|c| !c.is_editable && !c.is_deletable)); + + // Instances without the option (e.g. /fs) never set the flags. + let (immutable, _dir2) = client_with_root().await; + fs::create_dir(_dir2.path().join("music")).expect("mkdir"); + let root = immutable.get_lib_node("/fs").await.expect("fs root"); + assert!(root + .children + .iter() + .all(|c| !c.is_editable && !c.is_deletable)); + } + + #[tokio::test] + async fn rename_moves_a_top_level_folder() { + let (client, dir) = editable_client().await; + let node = client + .rename_lib_node("/queues/road%20trip", "hiking") + .await + .expect("rename"); + assert_eq!(node.path, "/queues/hiking"); + assert_eq!(node.title, "hiking"); + assert!(dir.path().join("hiking/album").is_dir(), "content moved"); + assert!(!dir.path().join("road trip").exists()); + } + + #[tokio::test] + async fn rename_rejects_reserved_invalid_and_colliding_targets() { + let (client, _dir) = editable_client().await; + // The reserved folder is not editable. + let err = client + .rename_lib_node("/queues/current", "x") + .await + .expect_err("reserved"); + assert_eq!(err, ProviderError::NotSupported); + // Nested folders are not editable. + let err = client + .rename_lib_node("/queues/road%20trip/album", "x") + .await + .expect_err("nested"); + assert_eq!(err, ProviderError::NotSupported); + // Bad new titles are invalid input: empty, separators, hidden, + // reserved target. + for bad in ["", " ", "a/b", ".hidden", "current"] { + let err = client + .rename_lib_node("/queues/road%20trip", bad) + .await + .expect_err(bad); + assert_eq!(err, ProviderError::InvalidInput, "{bad:?}"); + } + // Renaming onto an existing sibling never merges. + fs::create_dir(_dir.path().join("taken")).expect("mkdir"); + let err = client + .rename_lib_node("/queues/road%20trip", "taken") + .await + .expect_err("collision"); + assert_eq!(err, ProviderError::InvalidInput); + // Immutable instances keep the old contract. + let (immutable, _dir2) = client_with_root().await; + fs::create_dir(_dir2.path().join("music")).expect("mkdir"); + let err = immutable + .rename_lib_node("/fs/music", "x") + .await + .expect_err("immutable instance"); + assert_eq!(err, ProviderError::NotSupported); + } + + #[tokio::test] + async fn delete_removes_top_level_folders_idempotently() { + let (client, dir) = editable_client().await; + let parent = client + .delete_lib_node("/queues/road%20trip") + .await + .expect("delete"); + assert_eq!(parent.path, "/queues"); + assert!(!dir.path().join("road trip").exists()); + assert_eq!(parent.children.len(), 1, "only `current` remains"); + // Idempotent: deleting an already-gone folder succeeds. + client + .delete_lib_node("/queues/road%20trip") + .await + .expect("idempotent delete"); + // Reserved and nested folders are not deletable. + for undeletable in ["/queues/current", "/queues/current/nested"] { + let err = client + .delete_lib_node(undeletable) + .await + .expect_err(undeletable); + assert_eq!(err, ProviderError::NotSupported, "{undeletable}"); + } + } + + // ---- shared naming ---------------------------------------------------- + + #[test] + fn dir_names_share_the_track_file_sanitizer() { + assert_eq!(dir_name(0, "News of the World"), "0001 News of the World"); + assert_eq!(dir_name(10, "x"), "0011 x"); + let tricky = dir_name(1, "../a/b\\c\0"); + assert!(!tricky.contains(['/', '\\', '\0']), "{tricky}"); + assert!(!tricky.starts_with('.'), "{tricky}"); + assert_eq!(dir_name(2, ""), "0003 track"); + } + + #[test] + fn folder_name_validation_trims_and_rejects() { + assert_eq!( + validate_folder_name(" hiking ", &["current"]), + Ok("hiking") + ); + for bad in ["", " ", "a/b", "a\\b", "a\0b", ".hidden", "current"] { + assert!( + validate_folder_name(bad, &["current"]).is_err(), + "{bad:?} must be rejected" + ); + } + // Reservation applies to the trimmed form. + assert!(validate_folder_name(" current ", &["current"]).is_err()); + } + #[test] fn track_file_names_sort_in_queue_order_and_stay_plain() { // Zero-padded one-based prefix, suffix appended. diff --git a/plan/bookmarks.md b/plan/bookmarks.md new file mode 100644 index 0000000..977737b --- /dev/null +++ b/plan/bookmarks.md @@ -0,0 +1,51 @@ +# Plan: bookmarks + +Ordered tasks; each names its verification (tests in `fsdy/src/lib.rs` / +`crabidy-server/src/bookmark_store.rs` and/or gates in +`quality/bookmarks.md`). Stubs, the proto rpc, and orchestrator wiring +exist; the new fsdy/bookmark-store tests fail on `todo!()` at plan time. + +- [x] **T1 — fsdy shared naming.** `dir_name` (prefix + shared sanitizer, + no suffix; refactor `track_file_name` onto one helper) and + `validate_folder_name` (trim, reject separators/NUL/hidden/reserved). + Verifies: `dir_names_share_the_track_file_sanitizer`, + `folder_name_validation_trims_and_rejects`; gate "no second naming + scheme". +- [x] **T2 — fsdy mutable top level.** Flags on the editable-instance root + listing (skip reserved); `rename_lib_node`/`delete_lib_node` + implementations gated on option + direct child + not reserved + (`NotSupported` otherwise); rename validates via + `validate_folder_name`, refuses collisions (`InvalidInput`), returns + the renamed node; delete is idempotent and returns the root listing. + Verifies: `editable_instances_flag_only_unreserved_top_level_folders`, + `rename_moves_a_top_level_folder`, + `rename_rejects_reserved_invalid_and_colliding_targets`, + `delete_removes_top_level_folders_idempotently`; gates "Mutable top + level". +- [x] **T3 — queue_store reuse.** `QueueStore::validate_name` delegates to + `fsdy::validate_folder_name` with `current` reserved (behavior + unchanged — existing tests must stay green). +- [x] **T4 — BookmarkStore capture.** `open`, `capture_with_caps` + (iterative pre-order walk over `get_lib_node` / + `get_metadata_for_track`, mirrored dirs + link files, caps, temp + cleanup on every failure, tmp-and-swap). Verifies: all + `bookmark_store::tests`; gates "Capture". +- [x] **T5 — TUI capture flow.** `Action::LibraryCaptureNode` (`w`, + `Scope::Library`), `Library::selected_queueable()` (bare selection, + queueable, path + title), `InputPurpose::Capture { path }` prefilled + with the title, `MessageFromUi::CaptureNode { path, name }`, + orchestrator arm → new `rpc::capture_library_node` (log failures, keep + polling). TUI tests: binding lookup, overlay gating + prefill, submit + message. Verifies: new `cbd-tui` tests; gates "TUI". +- [x] **T6 — full verification.** Whole workspace suite green; + clippy/fmt/taplo/markdownlint clean; walk every gate in + `quality/bookmarks.md` and tick it; no `todo!()` left. +- [x] **T7 — live smoke test.** Capture a real Tidal artist subtree into a + temp store through the provider layer, browse it through a + `/bookmarks` instance, rename it, resolve an album, and fetch a stream + URL for one captured link — remove any temporary probe afterwards. + Verifies: end-to-end D1/D2/D4 behavior outside unit scope. +- [x] **T8 — docs.** `plan/summary.md` section incl. deviations; reconcile + `architecture/queue-persistence.md` D8 (saved queues now + renamable/deletable) and `architecture/bookmarks.md` if the + implementation diverged. diff --git a/plan/summary.md b/plan/summary.md index 1e9ae6e..8c7bfd6 100644 --- a/plan/summary.md +++ b/plan/summary.md @@ -1,5 +1,60 @@ # Implementation summaries +## bookmarks (2026-07-21) + +Built per `plan/bookmarks.md`: `w` on a queueable library selection now +captures the whole subtree as a **bookmark** — a structure-preserving +snapshot under `<config>/crabidy/bookmarks/<name>/`, mounted read-only at +`/bookmarks` by a third `fsdy` instance. The capture runs on the +orchestrator (a spawned task walking `get_lib_node` iteratively across +any provider): every child node becomes an order-prefixed folder +(`fsdy::dir_name`, sharing the queue entries' sanitizer), every track an +order-prefixed link file, so the case-insensitive listing reproduces the +source order and replaying is plain fs-provider behavior. Caps (1 000 +dirs / 20 000 tracks) abort cleanly with the temp folder removed; writes +are tmp-and-swap; re-capturing a name overwrites it. The wire gained one +additive rpc, `CaptureLibraryNode(path, name)` (invalid name/source → +`invalid_argument`, over-cap/disabled → `failed_precondition`). The TUI +opens the existing input overlay prefilled with the selection's title +(`bookmark`), gated on a queueable bare selection. + +On top, `fsdy::Client` gained `with_editable_top_level(reserved)`: +editable instances mark their root's child folders +`is_editable`/`is_deletable` and implement rename (no-merge, validated +titles, returns the renamed node) and delete (idempotent, returns the +refreshed root). Applied to `/bookmarks` (nothing reserved) **and +`/queues`** (reserved: `current`) — saved queues are now renamable and +deletable through the existing `e`/`d` flows with zero TUI changes. +`/fs` stays immutable. All 130 workspace tests green (6 new in `fsdy`, 7 +in `bookmark_store`, 2 TUI); every gate in `quality/bookmarks.md` +checked. A temporary live probe (removed after passing) captured a +19-track album from the live Tidal API, browsed it with editable flags, +renamed it, resolved it in order, and fetched a stream URL for a +captured link. + +The whole feature ran autonomously per standing instruction; decisions +are recorded in `architecture/bookmarks.md` (options + rationale). + +### Deviations from plan / architecture (bookmarks) + +- **Capture is all-or-nothing**: any provider or write failure mid-walk + aborts the whole capture (temp folder removed) instead of skipping the + failing subtree with a warning — a bookmark that *looks* complete must + *be* complete. The architecture only specified the unreadable-*root* + case; this extends it to every node. +- **Rename to the current name is a no-op success** (returns the node), + not a collision error — the target "exists" only because it is the + source. +- **Rename targets don't pass `disk_path`**: the new folder name is + validated by `validate_folder_name` (no separators, NUL, or leading + dots), which makes it a plain sibling name by construction; the + traversal gate still covers every client-supplied *path*. +- **`track_file_name` was refactored onto a shared `ordered_name`** + helper rather than duplicated for `dir_name` (planned as "shared + sanitizer", realized as one function). +- **Environment note**: builds/tests again ran with a session-local + `CARGO_TARGET_DIR`; no repo change. + ## queue-persistence (2026-07-21) Built per `plan/queue-persistence.md`: queues now survive server restarts, diff --git a/quality/bookmarks.md b/quality/bookmarks.md new file mode 100644 index 0000000..0f3d301 --- /dev/null +++ b/quality/bookmarks.md @@ -0,0 +1,73 @@ +# Quality gates: bookmarks + +Criteria the implementation must satisfy beyond the automatic tests +(`fsdy/src/lib.rs`, `crabidy-server/src/bookmark_store.rs`, plus the TUI +tests added during implementation). Each gate is pass/fail by reading the +code. + +## Capture + +- [x] The walk is iterative (worklist), never recursive — a deep provider + tree cannot overflow the stack. +- [x] The caps bound *total* directories and tracks and abort with a typed + error; the temp folder is removed on every failure path (cap, walk + error, write error). +- [x] Captures write through the same primitives as queues + (`TrackFile::from_track`, `track_file_name`, and `dir_name` sharing one + sanitizer) — no second serialization or naming scheme. +- [x] Whole-bookmark writes are tmp-and-swap; overwriting an existing + bookmark never leaves a mix of old and new entries. +- [x] A source path that is a track captures as a folder with one file; an + unreadable source is `BadSource`, not a panic or an empty bookmark. +- [x] The capture runs on a spawned task; the orchestrator loop keeps + serving commands during a large walk. +- [x] No file contents in logs (paths, names, and counts only). + +## Mutable top level (fsdy) + +- [x] The `is_editable`/`is_deletable` flags appear **only** on direct + child folders of an editable instance's root, never on reserved names, + nested nodes, tracks, or immutable instances. +- [x] `rename_lib_node`/`delete_lib_node` enforce the same gate they + advertise: option on + direct child + not reserved; everything else is + `NotSupported` (matching the flags a client saw). +- [x] Rename validates the new title with the shared + `validate_folder_name` (reserved names rejected as targets) and never + merges onto an existing sibling (`InvalidInput`). +- [x] Delete is idempotent (already gone → success) and returns the + refreshed root listing; rename returns the renamed node at its new + path. +- [x] The `/queues` instance reserves `current`; `/bookmarks` reserves + nothing; `/fs` stays fully immutable. +- [x] Path traversal validation still happens only in `disk_path`; rename + and delete go through it for every path they touch. + +## RPC and orchestrator + +- [x] `capture_library_node` maps errors: invalid name/source → + `invalid_argument`, over-cap or disabled → `failed_precondition`, + walk/write failures → `internal`. +- [x] The orchestrator routes `/bookmarks` in every `ProviderClient` + method (same completeness as `/queues`), and `get_lib_root` lists the + `bookmarks` child only when the instance exists. +- [x] Bookmarks init is non-fatal: no config dir or an unopenable store + disables capture and the `/bookmarks` mount, never the server. + +## TUI + +- [x] `w` is bound in `Scope::Library` (queue's `w` untouched), has a help + description, and passes the bindings-table invariant tests unchanged. +- [x] The capture overlay opens only for a queueable bare selection, + prefilled with the selection's title; marks are ignored. +- [x] `MessageFromUi::CaptureNode` reaches the new rpc; a failed capture + is logged and never tears down the poll loop. +- [x] Renaming/deleting bookmark and saved-queue folders works through the + existing `e`/`d` flows with no TUI code changes. + +## Hygiene + +- [x] New public items are documented; docs state error/edge behavior. +- [x] `clippy -D warnings`, `fmt`, `taplo`, `markdownlint` clean on the + whole workspace; all tests green. +- [x] `architecture/queue-persistence.md` D8 reconciled (rename/delete of + saved queues is no longer out of scope).