From be2676080d5ee9c714f878ac82eec3c3036ddd29 Mon Sep 17 00:00:00 2001 From: Test User Date: Wed, 22 Jul 2026 22:35:48 +0200 Subject: [PATCH] Let a same-source W re-capture replace its save in place Capturing the same source under an existing name used to be refused as a conflict. A hidden .cbd-save.toml marker now records each save's origin, so re-capturing the same source replaces the folder in place (the shared store audio is never touched), while a different source under the same name still refuses. Updates the crabidy-store D5 design note and quality gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- architecture/crabidy-store.md | 34 +++++--- crabidy-server/src/crabidy_store.rs | 121 ++++++++++++++++++++++++++-- quality/crabidy-store.md | 6 +- 3 files changed, 141 insertions(+), 20 deletions(-) diff --git a/architecture/crabidy-store.md b/architecture/crabidy-store.md index 0b892a8..2c6bb6a 100644 --- a/architecture/crabidy-store.md +++ b/architecture/crabidy-store.md @@ -234,14 +234,27 @@ A **save** takes a *source* (a live-queue snapshot or a library-node path) and a - **`W`** on a library node or the queue → `w` **plus** run the D4 capture per track; tomls carry `playable.store`. Works on both library nodes and the queue. -**Conflict = refuse.** If `/crabidy/` already exists, do nothing and warn -`name "" already exists`; the user deletes the old folder and saves again. -This replaces the old overwrite (bookmark tmp-swap) and merge-by-name (capture -resume). The save is still built in a hidden `.tmp-` sibling and swapped in -atomically, so a crashed/failed run leaves **no** blocking partial folder and the -name stays free to retry — while any audio already committed to the store -persists and makes the retry fast via D4. Resumability thus moves from the folder -to the store; the folder is all-or-nothing. +**Conflict handling.** Each save records its origin in a hidden +`.cbd-save.toml` marker at the save root (`source = ""`, +`capture = true|false`). When `/crabidy/` already exists: + +- **`w` (link save)** always refuses — do nothing and warn `name "" + already exists`; the user deletes the old folder and saves again. +- **`W` (capture)** refuses **unless it is a re-capture of the same source**: + if the existing save's marker `source` equals the node being captured, the + save **replaces** it; a different source still refuses. So pressing `W` + again on the same item (or `W` on the queue again) refreshes the capture in + place, while `W` naming a *different* item after an existing save is still + protected. + +The save is built in a hidden `.tmp-` sibling and swapped in atomically; +a permitted replace removes the old folder and renames the temp over it. A +crashed/failed run leaves **no** blocking partial folder, and any audio already +committed to the store persists and makes a retry fast via D4 (resumability +lives in the store, not the folder). Replacing is cheap and safe because a +save folder holds only tomls — the shared store audio is never rewritten or +deleted (D7). A save that predates the marker (no `.cbd-save.toml`) is treated +as a different source and refuses; delete it once to re-establish it. `current` is exempt: the playback loop overwrites it on every queue change; the user cannot save over the reserved name `current`. @@ -286,8 +299,9 @@ for this provider (folder/toml removal remains). - `/queues`, `/bookmarks`, `/captures` disappear from the root; one `/crabidy` child appears (title `crabidy`). Inside it, `w`-saves (links) and `W`-saves (store-backed) coexist, distinguished by the captured marker. -- **Captured marker:** a captured row is prefixed with `|` as the *first - character of the row* (before the selection padding), e.g. `|Bohemian…`. +- **Captured marker:** a captured row carries a trailing `↓` at the *end of + the row* — a status marker after the action-key brackets (outside them), + e.g. `Bohemian… ↓`. - A **track** row is captured per `Track.is_captured` (D3): store-backed, or its `(provider, id)` is in the store index — visible even while browsing tidal. - A **node/child** row is captured iff all its tracks and child nodes are diff --git a/crabidy-server/src/crabidy_store.rs b/crabidy-server/src/crabidy_store.rs index 3e17d59..8dbce38 100644 --- a/crabidy-server/src/crabidy_store.rs +++ b/crabidy-server/src/crabidy_store.rs @@ -45,6 +45,11 @@ const SIDECAR_SUFFIX: &str = ".cbd-store.toml"; /// Dot-prefixed, so library listings never show it. const STATE_FILE_NAME: &str = ".queue-state.toml"; +/// Hidden per-save marker recording a save's origin, so a `W` re-capture of +/// the same source can replace it in place (a different source refuses). +/// Dot-prefixed, so library listings never show it. +const SAVE_MARKER: &str = ".cbd-save.toml"; + /// Whether a save writes bookmark links (`w`) or captures audio into the /// store (`W`). #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -112,6 +117,16 @@ pub struct StoreSidecar { /// name). A bare file name, no separators. pub type StoreName = String; +/// The `.cbd-save.toml` marker recording where a save came from, so a `W` +/// re-capture of the same source can replace it (D5). +#[derive(Debug, Clone, Deserialize, Serialize)] +struct SaveMarker { + /// The library path of the captured/saved node. + source: String, + /// Whether the save was a capture (`W`) rather than a link save (`w`). + capture: bool, +} + /// In-memory index over the store's sidecars, derived by scanning /// `*.cbd-store.toml` at open and updated on every write. No separate /// persisted index exists — this *is* the "grep the sidecars" search, memoized @@ -373,15 +388,45 @@ impl CrabidyStore { { let name = fsdy::validate_folder_name(name, &[CURRENT_NAME]).map_err(CaptureError::InvalidName)?; - if tokio::fs::try_exists(self.tree_root.join(name)).await? { - return Err(CaptureError::Conflict(name.to_string())); - } + // A genuine conflict rejects here; a same-source `W` re-capture is + // allowed (it will replace, D5). + self.resolve_collision(name, source_path, mode).await?; if mode == SaveMode::Capture { self.source_allows_download(client, source_path).await?; } Ok(()) } + /// Decides what a save to `name` does about an existing folder: `Ok(false)` + /// = create fresh (no collision), `Ok(true)` = replace an existing save + /// that is a same-source `W` re-capture, `Err(Conflict)` = a different + /// existing save (refuse). `w` link saves never replace (D5). + async fn resolve_collision( + &self, + name: &str, + source_path: &str, + mode: SaveMode, + ) -> Result { + if !tokio::fs::try_exists(self.tree_root.join(name)).await? { + return Ok(false); + } + if mode == SaveMode::Capture { + if let Some(marker) = self.read_save_marker(name).await { + if marker.source == source_path { + return Ok(true); + } + } + } + Err(CaptureError::Conflict(name.to_string())) + } + + /// Reads a save's `.cbd-save.toml` marker, if present and parseable. + async fn read_save_marker(&self, name: &str) -> Option { + let path = self.tree_root.join(name).join(SAVE_MARKER); + let text = tokio::fs::read_to_string(path).await.ok()?; + toml::from_str(&text).ok() + } + /// Whether the source root (or a track's parent node) advertises download /// captures (`is_downloadable`). Mirrors the old capture-store gate. async fn source_allows_download( @@ -429,6 +474,9 @@ impl CrabidyStore { { let name = fsdy::validate_folder_name(name, &[CURRENT_NAME]).map_err(CaptureError::InvalidName)?; + // Refuse a different existing save up front; a same-source `W` + // re-capture is permitted and replaces the old folder (D5). + let replace = self.resolve_collision(name, source_path, mode).await?; let tmp = self.tree_root.join(format!(".tmp-{name}")); if tokio::fs::try_exists(&tmp).await? { tokio::fs::remove_dir_all(&tmp).await?; @@ -443,10 +491,16 @@ impl CrabidyStore { } let target = self.tree_root.join(name); if tokio::fs::try_exists(&target).await? { - // A conflicting save appeared since validation; refuse rather than - // clobber it. - let _ = tokio::fs::remove_dir_all(&tmp).await; - return Err(CaptureError::Conflict(name.to_string())); + if replace { + // Same-source re-capture: drop the old tomls and swap the + // fresh capture in. The shared store audio is never touched. + tokio::fs::remove_dir_all(&target).await?; + } else { + // A conflicting save appeared since validation; refuse rather + // than clobber it. + let _ = tokio::fs::remove_dir_all(&tmp).await; + return Err(CaptureError::Conflict(name.to_string())); + } } tokio::fs::rename(&tmp, &target).await?; Ok(()) @@ -503,6 +557,14 @@ impl CrabidyStore { } } } + // Record the save's origin so a `W` re-capture of the same source can + // replace it in place (D5). Dot-prefixed, so listings never show it. + let marker = SaveMarker { + source: source_path.to_string(), + capture: mode == SaveMode::Capture, + }; + let text = toml::to_string_pretty(&marker).map_err(StoreError::TomlWrite)?; + tokio::fs::write(tmp.join(SAVE_MARKER), text).await?; Ok(()) } @@ -1361,6 +1423,51 @@ mod tests { assert!(matches!(again, Err(CaptureError::Conflict(_)))); } + #[tokio::test] + async fn re_capturing_the_same_source_replaces_but_another_refuses() { + let (store, dir) = open_store().await; + let src = write_bytes(dir.path(), "src.flac", b"AAAA").await; + let mut mock = MockProvider::default(); + let t = track("/tidal/album/1/100", "100", "Song"); + mock.tracks.insert(t.path.clone(), (t.clone(), src.clone())); + mock.nodes.insert( + "/tidal/album/1".to_string(), + node("/tidal/album/1", vec![t]), + ); + let progress = Progress::silent("x", true); + + store + .save(&mock, "/tidal/album/1", "s", SaveMode::Capture, &progress) + .await + .expect("first capture"); + let store_entries = entries(store.store_dir()).len(); + + // Re-capturing the SAME source under the same name replaces in place. + store + .save(&mock, "/tidal/album/1", "s", SaveMode::Capture, &progress) + .await + .expect("same-source re-capture replaces"); + assert_eq!( + entries(store.store_dir()).len(), + store_entries, + "the store is not duplicated by a replace" + ); + assert!(store.tree_dir().join("s").is_dir()); + + // A DIFFERENT source under the same name is refused. + let mut other = MockProvider::default(); + let t2 = track("/tidal/album/2/200", "200", "Other"); + other.tracks.insert(t2.path.clone(), (t2.clone(), src)); + other.nodes.insert( + "/tidal/album/2".to_string(), + node("/tidal/album/2", vec![t2]), + ); + let refused = store + .save(&other, "/tidal/album/2", "s", SaveMode::Capture, &progress) + .await; + assert!(matches!(refused, Err(CaptureError::Conflict(_)))); + } + #[tokio::test] async fn persist_current_and_load_round_trip() { let (store, _dir) = open_store().await; diff --git a/quality/crabidy-store.md b/quality/crabidy-store.md index a30ec74..7309730 100644 --- a/quality/crabidy-store.md +++ b/quality/crabidy-store.md @@ -82,9 +82,9 @@ inline and in `plan/summary.md`. - [x] The root library shows a single `crabidy` child; `queues`/`bookmarks`/ `captures` no longer appear. -- [x] Captured rows are prefixed with `|` as the first character of the row - (before selection padding), driven by `is_captured`; captured tracks are - marked even while browsing another provider (tidal/youtube). Node/child +- [x] Captured rows carry a trailing `↓` at the end of the row (after the + action-key brackets, outside them), driven by `is_captured`; captured tracks + are marked even while browsing another provider (tidal/youtube). Node/child marking is shallow: a node is marked captured when all its tracks are captured and it has no child nodes (flat saves); a nested save's top folder is not marked — full-recursion marking is future work.