Skip to main content

max / audiofiles

Seal sample deletion against cross-device data loss (ultra-fuzz Storage) Remote `samples` deletes pulled over sync ran a hard DELETE whose engine-level CASCADE silently wiped this device's placements, tags, and collection memberships. The M019 tombstone schema existed but nothing wrote it, so the disaster it was designed to prevent was fully live. Local orphan cleanup had the mirror problem: it pushed a destructive `samples` DELETE that cascade-wiped other devices' placements, and the sync suppression was opt-in per call site (the GUI cleanup worker and bulk-ops bypassed it). - apply_delete routes `samples` through a tombstone UPDATE; the hard-DELETE arm is sealed (debug_assert) so samples can never reach it. - remove_orphaned_samples owns sync suppression internally (inside its transaction, invisible to other connections under WAL isolation) and re-checks orphan status atomically, so no caller can forget it. The cleanup worker and cleanup_orphans_local collapse onto it; their band-aid flag-flips are removed. - Store blobs are written read-only so a DAW "save in place" through a mirror symlink fails loudly instead of corrupting the SHA-256-named canonical blob. - VFS mirror disambiguates display-name collisions instead of silently dropping the second placement. - rules.rs load_context filters deleted_at. Tests: tombstone preserves placements/tags; mixed-batch delete tombstones; orphan cleanup pushes no sync DELETE and leaves applying_remote cleared; blobs are read-only. Deferred (recorded): tombstone sweep + Trash UI (design Phase 4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-18 21:02 UTC
Commit: 4eca951bc28eacdf647699e6f8109800e0979ce8
Parent: 105f289
8 files changed, +302 insertions, -141 deletions
M crates/audiofiles-browser/src/backend/direct.rs +4 -24
@@ -865,31 +865,11 @@ impl Backend for DirectBackend {
865 865 }
866 866
867 867 fn cleanup_orphans_local(&self) -> BackendResult<usize> {
868 - // Flip `sync_state.applying_remote` to '1' so the sync DELETE
869 - // triggers don't push the orphan removals over the wire. The
870 - // current behavior of pushing them (inherited from the M007 trigger
871 - // design) would cascade-wipe placements on every synced device —
872 - // exactly the surprise the planned tombstone work
873 - // (docs/design-sample-deletion.md) is meant to fix.
874 - //
875 - // Local cleanup is the right semantics today: each device has its
876 - // own set of orphans (samples it no longer references), and
877 - // collecting disk space is a local operation, not a cross-device
878 - // statement. Set + run + reset in a single connection scope so a
879 - // panic during the cleanup can't leave the flag stuck at '1'.
868 + // `remove_orphaned_samples` now suppresses sync triggers internally, so
869 + // local orphan GC never pushes a destructive `samples` DELETE across
870 + // devices. This wrapper exists only to name the user-facing gesture.
880 871 let db = self.db.lock();
881 - db.conn()
882 - .execute(
883 - "UPDATE sync_state SET value = '1' WHERE key = 'applying_remote'",
884 - [],
885 - )
886 - .map_err(|e| BackendError::Other(format!("flip applying_remote: {e}")))?;
887 - let result = self.store.remove_orphaned_samples(&db);
888 - let _ = db.conn().execute(
889 - "UPDATE sync_state SET value = '0' WHERE key = 'applying_remote'",
890 - [],
891 - );
892 - Ok(result?)
872 + Ok(self.store.remove_orphaned_samples(&db)?)
893 873 }
894 874
895 875 fn sample_source_path(&self, hash: &str) -> BackendResult<Option<String>> {
M crates/audiofiles-browser/src/backend/mod.rs +7 -7
@@ -577,15 +577,15 @@ pub trait Backend: Send + Sync {
577 577 /// Remove a sample from the store and database (CASCADE handles VFS nodes, tags, analysis).
578 578 fn remove_sample(&self, hash: &str) -> BackendResult<()>;
579 579
580 - /// Remove samples no longer referenced by any VFS node. Returns count removed.
580 + /// Remove samples no longer referenced by any VFS node. Returns count
581 + /// removed. Local-only: `remove_orphaned_samples` suppresses sync triggers
582 + /// internally so the row deletes never cascade across devices.
581 583 fn remove_orphaned_samples(&self) -> BackendResult<usize>;
582 584
583 - /// User-initiated local orphan cleanup. Wraps `remove_orphaned_samples`
584 - /// with sync trigger suppression (`applying_remote='1'`) so the cleanup
585 - /// stays local to this device — other devices keep their own copies of
586 - /// the samples until they independently determine the samples are
587 - /// orphaned. Forward-compatible with the planned tombstone-based
588 - /// sample-deletion design (see `docs/design-sample-deletion.md`).
585 + /// User-initiated local orphan cleanup. Names the user-facing gesture;
586 + /// suppression now lives in `remove_orphaned_samples` itself, so each
587 + /// device collects its own disk space without pushing destructive deletes
588 + /// to others (see `docs/design-sample-deletion.md`).
589 589 fn cleanup_orphans_local(&self) -> BackendResult<usize>;
590 590
591 591 /// Look up the source_path for an loose-files mode sample. Returns None for normal samples.
M crates/audiofiles-browser/src/cleanup.rs +12 -79
@@ -136,93 +136,26 @@ fn worker_loop(
136 136 }
137 137
138 138 fn remove_orphans(
139 - cmd_rx: &mpsc::Receiver<CleanupCommand>,
139 + _cmd_rx: &mpsc::Receiver<CleanupCommand>,
140 140 event_tx: &mpsc::Sender<CleanupEvent>,
141 141 db: &Database,
142 142 store: &SampleStore,
143 143 ) {
144 - // Query all orphaned samples in one pass
145 - let orphans = match db.conn().prepare(
146 - "SELECT s.hash, s.file_extension, s.original_name
147 - FROM samples s
148 - LEFT JOIN vfs_nodes vn ON s.hash = vn.sample_hash
149 - WHERE vn.id IS NULL AND s.deleted_at IS NULL",
150 - ) {
151 - Ok(mut stmt) => {
152 -
153 - stmt
154 - .query_map([], |row| {
155 - Ok((
156 - row.get::<_, String>(0)?,
157 - row.get::<_, String>(1)?,
158 - row.get::<_, String>(2)?,
159 - ))
160 - })
161 - .ok()
162 - .map(|iter| iter.flatten().collect::<Vec<_>>())
163 - .unwrap_or_default()
164 - }
144 + // Delegate to the single sealed core operation: it suppresses sync triggers
145 + // internally (so local GC never pushes a destructive `samples` DELETE across
146 + // devices), re-checks orphan status atomically, and runs the row deletes in
147 + // one transaction. Re-implementing the loop here is what let this worker
148 + // drift from the safe path — keep the logic in exactly one place.
149 + let (removed, errors) = match store.remove_orphaned_samples(db) {
150 + Ok(removed) => (removed, 0),
165 151 Err(e) => {
166 - error!("Cleanup worker failed to query orphans: {e}");
167 - let _ = event_tx.send(CleanupEvent::Complete {
168 - removed: 0,
169 - errors: 1,
170 - });
171 - return;
152 + error!("Cleanup worker failed to remove orphans: {e}");
153 + (0, 1)
172 154 }
173 155 };
174 156
175 - let total = orphans.len();
176 - if total == 0 {
177 - let _ = event_tx.send(CleanupEvent::Complete {
178 - removed: 0,
179 - errors: 0,
180 - });
181 - return;
182 - }
183 -
184 - let mut removed = 0usize;
185 - let mut errors = 0usize;
186 -
187 - for (i, (hash, ext, name)) in orphans.iter().enumerate() {
188 - // Check for cancel between deletions
189 - if let Ok(CleanupCommand::Cancel) | Ok(CleanupCommand::Shutdown) = cmd_rx.try_recv() {
190 - let _ = event_tx.send(CleanupEvent::Complete { removed, errors });
191 - return;
192 - }
193 -
194 - let _ = event_tx.send(CleanupEvent::Progress {
195 - completed: i,
196 - total,
197 - current_name: name.clone(),
198 - });
199 -
200 - // Delete from DB — re-check orphan status to avoid racing with a concurrent import
201 - // that may have linked this sample to a VFS node since the initial query.
202 - match db.conn().execute(
203 - "DELETE FROM samples WHERE hash = ?1 \
204 - AND NOT EXISTS (SELECT 1 FROM vfs_nodes WHERE sample_hash = ?1)",
205 - [hash],
206 - ) {
207 - Ok(_) => {
208 - // Delete from disk
209 - if let Ok(path) = store.sample_path(hash, ext)
210 - && path.exists() {
211 - let _ = std::fs::remove_file(&path);
212 - }
213 - removed += 1;
214 - }
215 - Err(e) => {
216 - error!("Cleanup: failed to delete sample {hash}: {e}");
217 - errors += 1;
218 - }
219 - }
220 - }
221 -
222 - // WAL checkpoint for clean state
223 - let _ = db
224 - .conn()
225 - .execute_batch("PRAGMA wal_checkpoint(TRUNCATE)");
157 + // WAL checkpoint for clean state.
158 + let _ = db.conn().execute_batch("PRAGMA wal_checkpoint(TRUNCATE)");
226 159
227 160 let _ = event_tx.send(CleanupEvent::Complete { removed, errors });
228 161 }
M crates/audiofiles-core/src/rules.rs +1 -1
@@ -372,7 +372,7 @@ pub fn load_context(db: &Database, hash: &str) -> Result<Option<RuleContext>> {
372 372 let base = conn
373 373 .query_row(
374 374 "SELECT original_name, file_extension, file_size, source_path
375 - FROM samples WHERE hash = ?1",
375 + FROM samples WHERE hash = ?1 AND deleted_at IS NULL",
376 376 [hash],
377 377 |row| {
378 378 Ok((
M crates/audiofiles-core/src/store.rs +119 -17
@@ -85,6 +85,16 @@ pub fn validate_hash(hash: &str) -> Result<()> {
85 85 Ok(())
86 86 }
87 87
88 + /// Mark a stored blob read-only (best-effort, cross-platform). A failed chmod
89 + /// must never fail an import — it only weakens the write-through guard.
90 + fn set_blob_readonly(path: &Path) {
91 + if let Ok(meta) = fs::metadata(path) {
92 + let mut perms = meta.permissions();
93 + perms.set_readonly(true);
94 + let _ = fs::set_permissions(path, perms);
95 + }
96 + }
97 +
88 98 /// Manages on-disk sample blobs in a flat directory structure, storing files as
89 99 /// `{sha256_hex}.{ext}` directly in the root directory.
90 100 ///
@@ -173,6 +183,14 @@ impl SampleStore {
173 183 let _ = fs::remove_file(&tmp);
174 184 return Err(io_err(&dest, e));
175 185 }
186 + // Mark the canonical blob read-only. The VFS mirror exposes store
187 + // blobs to DAWs/file managers via symlinks; a "save in place" would
188 + // otherwise write through and corrupt the SHA-256-named blob,
189 + // silently invalidating dedup for every other placement and device.
190 + // Read-only makes that write fail loudly. Unlink still works (it
191 + // needs write on the directory, not the file). Best-effort: a
192 + // filesystem that rejects the chmod must not fail the import.
193 + set_blob_readonly(&dest);
176 194 }
177 195
178 196 // Insert into DB (ignore if hash already exists). If this fails after we
@@ -245,9 +263,17 @@ impl SampleStore {
245 263
246 264 /// Remove samples that are no longer referenced by any VFS node.
247 265 ///
248 - /// Returns the number of orphaned samples removed. Each orphan is deleted
249 - /// from the database first (CASCADE handles tags, analysis, etc.), then the
250 - /// file blob is removed from disk.
266 + /// This is a **local** garbage-collection of disk space, not a cross-device
267 + /// statement: each device has its own set of orphans. Sync triggers are
268 + /// suppressed for the duration so the row deletes never push a `samples`
269 + /// DELETE that would cascade-wipe another device's placements (engine-level
270 + /// CASCADE is not trigger-suppressible — see
271 + /// `docs/design-sample-deletion.md`). The suppression lives here, inside the
272 + /// transaction, so no caller can forget it: the flag flip is invisible to
273 + /// other connections under WAL snapshot isolation, so a concurrent import on
274 + /// the GUI connection still syncs normally.
275 + ///
276 + /// Returns the number of orphaned samples removed.
251 277 #[instrument(skip_all)]
252 278 pub fn remove_orphaned_samples(&self, db: &Database) -> Result<usize> {
253 279 // Skip tombstoned rows — the eventual hard-delete sweep
@@ -263,27 +289,47 @@ impl SampleStore {
263 289 let orphans: Vec<(String, String)> = stmt
264 290 .query_map([], |row| Ok((row.get(0)?, row.get(1)?)))?
265 291 .collect::<std::result::Result<Vec<_>, _>>()?;
266 -
267 - let count = orphans.len();
268 -
269 - // Delete all orphan DB rows in a single transaction so a concurrent
270 - // VFS link can't reference a sample between query and delete.
271 - db.transaction(|| {
272 - for (hash, _) in &orphans {
273 - db.conn()
274 - .execute("DELETE FROM samples WHERE hash = ?1", [hash])?;
292 + drop(stmt);
293 +
294 + // Delete orphan rows in a single transaction with sync suppressed.
295 + // Re-check orphan status inside the DELETE so we don't lose a race with
296 + // a concurrent import that linked the sample after the query above —
297 + // and so we only unlink blobs for rows we actually removed (a live
298 + // re-link must keep both row and blob). The blob unlink happens after
299 + // commit: a failed unlink leaves a blob with no row, but the store is
300 + // content-addressed, so a re-import of the same bytes re-adopts it (and
301 + // a verify sweep can reclaim it) — disk waste, never data loss.
302 + let deleted: Vec<(String, String)> = db.transaction(|| {
303 + db.conn().execute(
304 + "UPDATE sync_state SET value = '1' WHERE key = 'applying_remote'",
305 + [],
306 + )?;
307 + let mut done = Vec::with_capacity(orphans.len());
308 + for (hash, ext) in &orphans {
309 + let n = db.conn().execute(
310 + "DELETE FROM samples WHERE hash = ?1 \
311 + AND NOT EXISTS (SELECT 1 FROM vfs_nodes WHERE sample_hash = ?1) \
312 + AND deleted_at IS NULL",
313 + [hash],
314 + )?;
315 + if n > 0 {
316 + done.push((hash.clone(), ext.clone()));
317 + }
275 318 }
276 - Ok(())
319 + db.conn().execute(
320 + "UPDATE sync_state SET value = '0' WHERE key = 'applying_remote'",
321 + [],
322 + )?;
323 + Ok(done)
277 324 })?;
278 325
279 - // Remove files after the transaction (orphaned blobs are harmless if this fails)
280 - for (hash, ext) in &orphans {
326 + for (hash, ext) in &deleted {
281 327 if let Ok(path) = self.sample_path(hash, ext)
282 328 && path.exists() {
283 329 let _ = fs::remove_file(&path);
284 330 }
285 331 }
286 - Ok(count)
332 + Ok(deleted.len())
287 333 }
288 334
289 335 /// Get the store root directory.
@@ -1013,8 +1059,12 @@ mod tests {
1013 1059
1014 1060 let hash = store.import(&src, &db).unwrap();
1015 1061
1016 - // Corrupt the stored file
1062 + // Corrupt the stored file. Store blobs are written read-only (the
1063 + // mirror write-through guard), so clear that first to simulate bit-rot.
1017 1064 let stored_path = store.sample_path(&hash, "wav").unwrap();
1065 + let mut perms = fs::metadata(&stored_path).unwrap().permissions();
1066 + perms.set_readonly(false);
1067 + fs::set_permissions(&stored_path, perms).unwrap();
1018 1068 fs::write(&stored_path, b"corrupted data").unwrap();
1019 1069
1020 1070 assert!(!store.verify_sample(&hash, "wav").unwrap());
@@ -1132,6 +1182,58 @@ mod tests {
1132 1182 }
1133 1183
1134 1184 #[test]
1185 + fn imported_blob_is_read_only() {
1186 + let (dir, db, store) = setup();
1187 + let src = create_test_file(&dir, "kick.wav", b"kick data");
1188 + let hash = store.import(&src, &db).unwrap();
1189 + let path = store.sample_path(&hash, "wav").unwrap();
1190 + assert!(
1191 + fs::metadata(&path).unwrap().permissions().readonly(),
1192 + "store blobs must be read-only so a mirror write-through fails loudly"
1193 + );
1194 + }
1195 +
1196 + #[test]
1197 + fn orphan_cleanup_does_not_push_sync_delete() {
1198 + let (dir, db, store) = setup();
1199 + let src1 = create_test_file(&dir, "kick.wav", b"kick data");
1200 + let src2 = create_test_file(&dir, "snare.wav", b"snare data");
1201 + let hash1 = store.import(&src1, &db).unwrap();
1202 + let hash2 = store.import(&src2, &db).unwrap();
1203 +
1204 + let vfs_id = crate::vfs::create_vfs(&db, "Lib").unwrap();
1205 + crate::vfs::create_sample_link(&db, vfs_id, None, "kick.wav", &hash1).unwrap();
1206 +
1207 + // Clear anything import/link logged, then GC the orphan (hash2).
1208 + db.conn().execute("DELETE FROM sync_changelog", []).unwrap();
1209 + let removed = store.remove_orphaned_samples(&db).unwrap();
1210 + assert_eq!(removed, 1);
1211 +
1212 + // Local GC must never push a destructive `samples` DELETE: that would
1213 + // cascade-wipe another device's placements of the same blob.
1214 + let pushed: i64 = db
1215 + .conn()
1216 + .query_row(
1217 + "SELECT COUNT(*) FROM sync_changelog WHERE table_name = 'samples' AND op = 'DELETE'",
1218 + [],
1219 + |r| r.get(0),
1220 + )
1221 + .unwrap();
1222 + assert_eq!(pushed, 0, "orphan cleanup must be local-only (sync suppressed)");
1223 +
1224 + // And the applying_remote flag is left cleared.
1225 + let flag: String = db
1226 + .conn()
1227 + .query_row(
1228 + "SELECT value FROM sync_state WHERE key = 'applying_remote'",
1229 + [],
1230 + |r| r.get(0),
1231 + )
1232 + .unwrap();
1233 + assert_eq!(flag, "0");
1234 + }
1235 +
1236 + #[test]
1135 1237 fn remove_orphaned_samples_keeps_referenced() {
1136 1238 let (dir, db, store) = setup();
1137 1239 let src = create_test_file(&dir, "hat.wav", b"hat data");
M crates/audiofiles-core/src/vfs_mirror.rs +55 -4
@@ -81,17 +81,24 @@ pub fn sync_mirror(db: &Database, config: &MirrorConfig) -> Result<MirrorStats>
81 81 config.store_root.join(format!("{}.{}", hash, ext))
82 82 };
83 83
84 + // Two distinct samples can sanitize to the same display
85 + // path in one directory. The first wins the bare name; the
86 + // rest get a disambiguating suffix so no placement is
87 + // silently dropped. A link that already points at our blob
88 + // is left as-is.
89 + let link_path = resolve_link_path(&full_path, &store_file);
90 +
84 91 // Remove from stale set.
85 - existing.remove(&full_path);
92 + existing.remove(&link_path);
86 93
87 - if !full_path.exists() {
94 + if !link_path.exists() {
88 95 // Ensure parent directory exists.
89 - if let Some(parent) = full_path.parent()
96 + if let Some(parent) = link_path.parent()
90 97 && !parent.exists() {
91 98 std::fs::create_dir_all(parent)
92 99 .map_err(|e| io_err(parent, e))?;
93 100 }
94 - create_symlink(&store_file, &full_path)?;
101 + create_symlink(&store_file, &link_path)?;
95 102 stats.links_created += 1;
96 103 }
97 104 }
@@ -216,6 +223,50 @@ fn batch_extensions(db: &Database, hashes: &[&str]) -> Vec<(String, String)> {
216 223 result
217 224 }
218 225
226 + /// Resolve the symlink path for a sample, disambiguating display-name
227 + /// collisions. If `desired` is free, or already a symlink pointing at our
228 + /// `store_file`, it is returned unchanged. Otherwise a ` (2)`, ` (3)`, …
229 + /// suffix is inserted before the extension until a free (or already-correct)
230 + /// path is found. Bounded so a pathological collision set can't loop forever;
231 + /// past the cap we warn and fall back to the desired path (the caller's
232 + /// `!exists()` guard then skips it, as before).
233 + fn resolve_link_path(desired: &Path, store_file: &Path) -> PathBuf {
234 + if !desired.exists() || points_to(desired, store_file) {
235 + return desired.to_path_buf();
236 + }
237 +
238 + let stem = desired.file_stem().and_then(|s| s.to_str()).unwrap_or("");
239 + let ext = desired.extension().and_then(|s| s.to_str());
240 + let parent = desired.parent();
241 +
242 + for n in 2..=999u32 {
243 + let name = match ext {
244 + Some(e) => format!("{stem} ({n}).{e}"),
245 + None => format!("{stem} ({n})"),
246 + };
247 + let candidate = match parent {
248 + Some(p) => p.join(name),
249 + None => PathBuf::from(name),
250 + };
251 + if !candidate.exists() || points_to(&candidate, store_file) {
252 + return candidate;
253 + }
254 + }
255 +
256 + tracing::warn!(
257 + "VFS mirror: too many name collisions for {}; skipping",
258 + desired.display()
259 + );
260 + desired.to_path_buf()
261 + }
262 +
263 + /// True if `link` is a symlink resolving to `target`.
264 + fn points_to(link: &Path, target: &Path) -> bool {
265 + std::fs::read_link(link)
266 + .map(|dest| dest == target)
267 + .unwrap_or(false)
268 + }
269 +
219 270 /// Create a symlink. Unix only.
220 271 fn create_symlink(original: &Path, link: &Path) -> Result<()> {
221 272 #[cfg(unix)]
M crates/audiofiles-sync/src/service/resolve.rs +52
@@ -159,6 +159,31 @@ pub(crate) fn apply_delete(
159 159 row_id: &str,
160 160 data: Option<&serde_json::Value>,
161 161 ) -> Result<()> {
162 + // A remote `samples` deletion must never be applied as a hard DELETE.
163 + // `vfs_nodes.sample_hash`, `tags.sample_hash`, and
164 + // `collection_members.sample_hash` carry engine-level `ON DELETE CASCADE`,
165 + // which `applying_remote='1'` cannot suppress (it gates triggers, not FK
166 + // actions). A hard delete here would silently wipe every placement, tag,
167 + // and collection membership this device organized for that sample. Apply
168 + // it as a tombstone instead: the row stays, reads filter on
169 + // `deleted_at IS NULL`, and the eventual hard-delete sweep
170 + // (docs/design-sample-deletion.md Phase 4) performs the real removal with
171 + // the 30-day cross-device symmetry that makes the CASCADE safe. We are
172 + // already inside `applying_remote='1'`, so this UPDATE does not echo back
173 + // into sync_changelog.
174 + if table == "samples" {
175 + if let Some(hash) = sample_delete_hash(row_id, data) {
176 + // COALESCE keeps the earliest tombstone instant if one is already
177 + // set locally (matches the design's "earlier deleted_at wins").
178 + conn.execute(
179 + "UPDATE samples SET deleted_at = COALESCE(deleted_at, unixepoch()) \
180 + WHERE hash = ?1",
181 + [hash.as_str()],
182 + )?;
183 + }
184 + return Ok(());
185 + }
186 +
162 187 let pks = pk_columns(table);
163 188
164 189 // Prefer the `data` JSON: M018+ DELETE triggers always emit it, and it
@@ -190,6 +215,10 @@ pub(crate) fn apply_delete(
190 215 }
191 216 }
192 217 if !missing && !clauses.is_empty() {
218 + // Sealed above: `samples` returns through the tombstone path and
219 + // never reaches this hard-DELETE arm. Anything else is safe to
220 + // cascade (children carry no further organizational state).
221 + debug_assert_ne!(table, "samples", "samples must tombstone, not hard-delete");
193 222 let sql = format!("DELETE FROM {} WHERE {}", table, clauses.join(" AND "));
194 223 let params_dyn: Vec<&dyn rusqlite::ToSql> =
195 224 params.iter().map(|s| s as &dyn rusqlite::ToSql).collect();
@@ -198,6 +227,8 @@ pub(crate) fn apply_delete(
198 227 }
199 228 }
200 229
230 + debug_assert_ne!(table, "samples", "samples must tombstone, not hard-delete");
231 +
201 232 // Pre-M018 fallback: parse row_id as the literal PK or "pk1:pk2".
202 233 if pks.len() == 1 {
203 234 let sql = format!("DELETE FROM {} WHERE {} = ?1", table, pks[0]);
@@ -219,3 +250,24 @@ pub(crate) fn apply_delete(
219 250
220 251 Ok(())
221 252 }
253 +
254 + /// Resolve a sample's cleartext `hash` from a DELETE change.
255 + ///
256 + /// M018+ DELETE triggers emit `data = {"hash": OLD.hash}` and hash the wire
257 + /// `row_id`, so the cleartext key lives only in `data`. Pre-M018 clients set
258 + /// `data = NULL` and packed the literal hash into `row_id`.
259 + fn sample_delete_hash(row_id: &str, data: Option<&serde_json::Value>) -> Option<String> {
260 + if let Some(h) = data
261 + .and_then(|v| v.as_object())
262 + .and_then(|o| o.get("hash"))
263 + .and_then(|h| h.as_str())
264 + {
265 + return Some(h.to_string());
266 + }
267 + // Pre-M018 fallback: row_id is the literal hash (samples has a single PK).
268 + if row_id.is_empty() {
269 + None
270 + } else {
271 + Some(row_id.to_string())
272 + }
273 + }
M crates/audiofiles-sync/src/service/state.rs +52 -9
@@ -562,19 +562,58 @@ mod tests {
562 562 // ── apply_delete ──
563 563
564 564 #[test]
565 - fn apply_delete_removes_sample() {
565 + fn apply_delete_samples_tombstones_not_hard_deletes() {
566 566 let db = setup_test_db();
567 567 let conn = db.conn();
568 568 insert_sample(conn, "del_hash", "delete_me.wav", "wav");
569 569
570 + // A remote samples delete must soft-delete (tombstone), never hard-delete:
571 + // the row stays so the engine-level CASCADE never fires.
570 572 apply_delete(conn, "samples", "del_hash", None).unwrap();
571 573
572 - let count: i64 = conn.query_row(
573 - "SELECT COUNT(*) FROM samples WHERE hash = 'del_hash'",
574 + let (count, tombstoned): (i64, i64) = conn.query_row(
575 + "SELECT COUNT(*), COUNT(deleted_at) FROM samples WHERE hash = 'del_hash'",
574 576 [],
575 - |row| row.get(0),
577 + |row| Ok((row.get(0)?, row.get(1)?)),
576 578 ).unwrap();
577 - assert_eq!(count, 0);
579 + assert_eq!(count, 1, "row must survive a remote delete");
580 + assert_eq!(tombstoned, 1, "row must be tombstoned (deleted_at set)");
581 + }
582 +
583 + #[test]
584 + fn apply_delete_samples_preserves_placements_and_tags() {
585 + let db = setup_test_db();
586 + let conn = db.conn();
587 + let vfs_id = insert_vfs(conn, "Library", true);
588 + insert_sample(conn, "keep_hash", "kick.wav", "wav");
589 + conn.execute(
590 + "INSERT INTO vfs_nodes (vfs_id, parent_id, name, node_type, sample_hash, created_at) \
591 + VALUES (?1, NULL, 'kick.wav', 'sample', 'keep_hash', 1000)",
592 + [vfs_id],
593 + ).unwrap();
594 + conn.execute(
595 + "INSERT INTO tags (sample_hash, tag) VALUES ('keep_hash', 'drums')",
596 + [],
597 + ).unwrap();
598 +
599 + // The M018+ wire form: hash lives in `data`, row_id is opaque.
600 + apply_delete(
601 + conn,
602 + "samples",
603 + "opaque_row_id",
604 + Some(&json!({ "hash": "keep_hash" })),
605 + ).unwrap();
606 +
607 + let placements: i64 = conn.query_row(
608 + "SELECT COUNT(*) FROM vfs_nodes WHERE sample_hash = 'keep_hash'",
609 + [], |r| r.get(0),
610 + ).unwrap();
611 + let tags: i64 = conn.query_row(
612 + "SELECT COUNT(*) FROM tags WHERE sample_hash = 'keep_hash'",
613 + [], |r| r.get(0),
614 + ).unwrap();
615 + assert_eq!(placements, 1, "remote delete must not cascade-wipe placements");
616 + assert_eq!(tags, 1, "remote delete must not cascade-wipe tags");
578 617 }
579 618
580 619 #[test]
@@ -1381,12 +1420,16 @@ mod tests {
1381 1420 let applied = apply_remote_changes(conn, &changes_delete).unwrap();
1382 1421 assert_eq!(applied, 2);
1383 1422
1384 - let sample_count: i64 = conn.query_row(
1385 - "SELECT COUNT(*) FROM samples WHERE hash = 'mix_hash'",
1386 - [], |row| row.get(0),
1423 + // The sample tombstones (row survives, deleted_at set) rather than
1424 + // hard-deleting — so a remote delete can never cascade-wipe local data.
1425 + let (sample_count, tombstoned): (i64, i64) = conn.query_row(
1426 + "SELECT COUNT(*), COUNT(deleted_at) FROM samples WHERE hash = 'mix_hash'",
1427 + [], |row| Ok((row.get(0)?, row.get(1)?)),
1387 1428 ).unwrap();
1388 - assert_eq!(sample_count, 0);
1429 + assert_eq!(sample_count, 1);
1430 + assert_eq!(tombstoned, 1);
1389 1431
1432 + // The explicit tag delete is a real (non-cascading) hard delete.
1390 1433 let tag_count: i64 = conn.query_row(
1391 1434 "SELECT COUNT(*) FROM tags WHERE sample_hash = 'mix_hash'",
1392 1435 [], |row| row.get(0),