diff --git a/architecture/captures.md b/architecture/captures.md index 0145233..6e27357 100644 --- a/architecture/captures.md +++ b/architecture/captures.md @@ -90,8 +90,14 @@ capture must not fill the disk. Downloads run sequentially (gentle on the provider, trivially bounded memory); the whole capture already runs on a spawned task, so the orchestrator keeps serving. -All-or-nothing is kept: any failed download aborts the capture and removes -the temp folder. A capture that lists 30 tracks *has* 30 playable files. +All-or-nothing is kept for real failures: any failed download (bad +status, transport error, timeout) aborts the capture and removes the temp +folder. The one softening: a track whose source **cannot be captured at +all** — its stream fails to resolve, or resolves to a non-http(s) target +(a local file playable) — is *skipped* with a warning instead of +aborting. Queue and bookmark captures mix providers (D4), and one local +`/fs` entry must not kill the downloadable rest; the skipped track simply +has no pair in the capture. ### D4 — Nodes opt in via `is_downloadable` @@ -100,10 +106,13 @@ New proto fields `LibraryNode.is_downloadable = 8` and centrally at the end of `get_lib_node`: a node is downloadable when it is **queueable or lists tracks** (the "or lists tracks" covers search-term result nodes, which are not queueable as a whole but whose track results -are downloadable); children mirror `is_queable`. Everything else (fs, -queues, bookmarks, captures, orchestrator roots) leaves the default -`false`; capturing a capture is pointless and links must not masquerade -as downloads. +are downloadable); children mirror `is_queable`. The `/queues` and +`/bookmarks` instances opt in wholesale +(`fsdy::Client::with_downloadable_nodes`): their entries are links into +downloadable providers, so `W` on a saved queue or bookmark downloads +its resolvable tracks and skips the rest (D3). `/fs` and `/captures` +stay `false` — capturing a capture is pointless, and local trees have +nothing to download. Tracks carry no flag: a listed track inherits its containing node's `is_downloadable` (TUI) — a Tidal album's tracks are downloadable because diff --git a/crabidy-server/src/capture.rs b/crabidy-server/src/capture.rs index 87a874a..60ceadb 100644 --- a/crabidy-server/src/capture.rs +++ b/crabidy-server/src/capture.rs @@ -11,6 +11,7 @@ use std::path::{Path, PathBuf}; use crabidy_core::proto::crabidy::Track; use crabidy_core::ProviderClient; +use tracing::warn; /// Connect timeout for download requests. pub const DOWNLOAD_CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); @@ -141,6 +142,13 @@ impl Downloader { /// The unbounded body of [`Self::download_track`]: resolve the stream /// URL, stream the response to disk against the byte budget, then /// write the toml. + /// + /// A track whose source cannot be captured — the stream fails to + /// resolve, or it resolves to something other than an http(s) URL + /// (e.g. a local file playable) — is **skipped** with a warning + /// instead of aborting the capture (architecture/captures.md D3): + /// queue and bookmark captures mix providers, and one local track + /// must not kill the rest. Actual download failures stay fatal. async fn fetch_track( &self, client: &C, @@ -153,13 +161,23 @@ impl Downloader { C: ProviderClient + Sync, { let track_path = track.path.as_str(); - let urls = client - .get_urls_for_track(track_path) - .await - .map_err(|err| CaptureError::BadSource(format!("{track_path}: {err}")))?; - let url = urls - .first() - .ok_or_else(|| CaptureError::BadSource(format!("{track_path}: no stream url")))?; + let urls = match client.get_urls_for_track(track_path).await { + Ok(urls) => urls, + Err(err) => { + warn!(path = track_path, "skipping uncapturable track: {err}"); + return Ok(()); + } + }; + let Some(url) = urls.first() else { + warn!(path = track_path, "skipping track without a stream url"); + return Ok(()); + }; + if !(url.starts_with("http://") || url.starts_with("https://")) { + // Local file playables (fs tracks, existing captures) resolve + // to paths, not URLs — nothing to download. + warn!(path = track_path, "skipping non-http playable"); + return Ok(()); + } let download_err = |err: reqwest::Error| { CaptureError::Download(format!("{track_path}: {}", err.without_url())) }; diff --git a/crabidy-server/src/capture_store.rs b/crabidy-server/src/capture_store.rs index 646a4de..646db8b 100644 --- a/crabidy-server/src/capture_store.rs +++ b/crabidy-server/src/capture_store.rs @@ -413,6 +413,46 @@ mod tests { assert_eq!(fs::read_dir(store.dir()).expect("store dir").count(), 0); } + #[tokio::test] + async fn download_capture_skips_uncapturable_tracks() { + // A queue-like source: a downloadable fsdy instance whose folder + // mixes an http track with a local-file track — like a persisted + // queue linking tidal and fs entries. + let url = serve("200 OK", "audio/flac", b"flacbytes".to_vec()).await; + let src = TempDir::new().expect("source tempdir"); + let mix = src.path().join("mix"); + fs::create_dir_all(&mix).expect("mkdir"); + fs::write( + mix.join("01 web.cbd-track.toml"), + format!("title = \"web\"\n[playable]\nurl = {url:?}\n"), + ) + .expect("write"); + fs::write(mix.join("audio.flac"), b"local").expect("write"); + fs::write( + mix.join("02 local.cbd-track.toml"), + "title = \"local\"\n[playable]\nfile = \"audio.flac\"\n", + ) + .expect("write"); + let source = fsdy::Client::new("/queues", src.path().to_path_buf()) + .expect("source instance") + .with_downloadable_nodes(); + + let (store, _dir) = store().await; + store + .capture(&source, "/queues/mix", "mixed") + .await + .expect("capture succeeds despite the local track"); + // The http track downloaded; the local-file track was skipped + // (no pair, no abort), keeping its listing position's prefix. + assert_eq!( + visible(&store.dir().join("mixed")), + vec![ + "0001 web.cbd-track.toml".to_string(), + "0001 web.flac".into() + ] + ); + } + #[tokio::test] async fn capture_validates_names_and_overwrites() { let url = serve("200 OK", "audio/flac", b"x".to_vec()).await; diff --git a/crabidy-server/src/provider.rs b/crabidy-server/src/provider.rs index 51b1f86..695bb90 100644 --- a/crabidy-server/src/provider.rs +++ b/crabidy-server/src/provider.rs @@ -274,7 +274,9 @@ impl ProviderClient for ProviderOrchestrator { 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.with_editable_top_level(&[CURRENT_QUEUE_NAME]), + client + .with_editable_top_level(&[CURRENT_QUEUE_NAME]) + .with_downloadable_nodes(), )), Err(err) => { warn!("queues library disabled: {err}"); @@ -304,7 +306,11 @@ impl ProviderClient for ProviderOrchestrator { }; 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(&[]))), + Ok(client) => Some(Arc::new( + client + .with_editable_top_level(&[]) + .with_downloadable_nodes(), + )), Err(err) => { warn!("bookmarks library disabled: {err}"); None diff --git a/fsdy/src/lib.rs b/fsdy/src/lib.rs index 4943886..6573f06 100644 --- a/fsdy/src/lib.rs +++ b/fsdy/src/lib.rs @@ -315,6 +315,12 @@ pub struct Client { /// Folder names exempt from top-level editing (e.g. the auto-persisted /// `current` queue) and rejected as rename targets. reserved: Vec, + /// Whether this instance's nodes advertise download captures + /// (`is_downloadable`, architecture/captures.md D4) — on for + /// `/queues` and `/bookmarks`, whose entries mostly link to + /// downloadable providers; off for `/fs` and `/captures`. Tracks + /// whose source cannot be captured are skipped by the capture walk. + downloadable_nodes: bool, } impl Client { @@ -344,6 +350,7 @@ impl Client { provider_root: provider_root.to_string(), editable_top_level: false, reserved: Vec::new(), + downloadable_nodes: false, }) } @@ -359,6 +366,16 @@ impl Client { self } + /// Marks every node of this instance downloadable (`is_downloadable` + /// — `W` captures the subtree, see architecture/captures.md D4). Used + /// by `/queues` and `/bookmarks`, whose entries are links into + /// downloadable providers; the capture walk skips tracks whose + /// source cannot be captured (local files, unresolvable links). + pub fn with_downloadable_nodes(mut self) -> Self { + self.downloadable_nodes = true; + self + } + /// The provider root this instance owns (e.g. `/fs`), as configured /// through [`Self::new`]. pub fn provider_root(&self) -> &str { @@ -478,6 +495,7 @@ impl Client { let mut child = LibraryNodeChild::new(child_lib, name, true); child.is_editable = editable; child.is_deletable = editable; + child.is_downloadable = self.downloadable_nodes; child }) .collect(); @@ -498,7 +516,7 @@ impl Client { tracks, is_queable: true, is_creatable: false, - is_downloadable: false, + is_downloadable: self.downloadable_nodes, }) } @@ -646,7 +664,7 @@ impl ProviderClient for Client { tracks: Vec::new(), is_queable: true, is_creatable: false, - is_downloadable: false, + is_downloadable: self.downloadable_nodes, } } @@ -1237,6 +1255,26 @@ mod tests { assert_eq!(reparsed.to_track("/queues/x.cbd-track.toml"), track); } + #[tokio::test] + async fn downloadable_instances_flag_every_node() { + let dir = TempDir::new().expect("tempdir"); + fs::create_dir_all(dir.path().join("mix")).expect("mkdir"); + // Off by default: /fs and /captures stay uncapturable. + let plain = Client::new("/fs", dir.path().to_path_buf()).expect("instance"); + let node = plain.get_lib_node("/fs").await.expect("list"); + assert!(!node.is_downloadable); + assert!(!node.children[0].is_downloadable); + // Opted in (/queues, /bookmarks): nodes and children advertise W. + let blessed = Client::new("/queues", dir.path().to_path_buf()) + .expect("instance") + .with_downloadable_nodes(); + let node = blessed.get_lib_node("/queues").await.expect("list"); + assert!(node.is_downloadable); + assert!(node.children[0].is_downloadable); + let nested = blessed.get_lib_node("/queues/mix").await.expect("list"); + assert!(nested.is_downloadable); + } + // ---- mutable top level (bookmarks / saved queues) --------------------- /// An editable instance over a root with two folders (one reserved) and diff --git a/plan/summary.md b/plan/summary.md index 975bae6..f97ee35 100644 --- a/plan/summary.md +++ b/plan/summary.md @@ -1,5 +1,20 @@ # Implementation summaries +## captures follow-up: W on queues and bookmarks (2026-07-21) + +Small fix on top of the captures feature: `/queues` and `/bookmarks` +nodes are now `W`-capturable. `fsdy::Client` gained +`with_downloadable_nodes()` (instance-wide `is_downloadable`, applied to +the queues and bookmarks mounts; `/fs` and `/captures` stay off), and +the download sink softens all-or-nothing for exactly one case: a track +whose source cannot be captured — stream resolution fails, or resolves +to a non-http(s) target like a local file playable — is skipped with a +warning instead of aborting, since queue/bookmark captures mix +providers. Real download failures (bad status, transport, timeout) stay +fatal. `architecture/captures.md` D3/D4 reconciled; new tests +`downloadable_instances_flag_every_node` (fsdy) and +`download_capture_skips_uncapturable_tracks` (capture store). + ## youtube-provider (2026-07-21) Built per `plan/youtube-provider.md`: a new workspace crate **`ytdy`**