Skip to main content

max / audiofiles

Content/Storage: live_samples view seals tombstone filter; atomic vfs ops Ultra-fuzz run-6 remediation, Content & Storage axis (B -> A). CHRONIC #2 (tombstone filter was opt-in per query and drifted): add a live_samples view (samples WHERE deleted_at IS NULL, migration 031) as the single source of "a sample the user can see". Redirect the leaking sites through it: tags find_by_tag/find_by_prefix/list_all_tags, collections list_collection_members + member counts, and vfs list_full_tree (the mirror's source, which now skips tombstoned/purged file nodes while keeping folders). Soft-delete is live, so these were surfacing deleted samples in tag/collection views and recreating dangling mirror symlinks. Removed the now-redundant inline deleted_at clauses in rules.rs. Note: the run-6 CRITICAL (applying_remote cross-connection changelog drop) was verified NOT reachable and is not fixed here. The flag is only ever '1' while a connection holds the SQLite write lock (setting it is itself a write under BEGIN IMMEDIATE), so no other connection can commit a write whose trigger would observe it. A 90-trigger rewrite would be churn for a non-bug. See report. Other findings: - vfs move_node/rename_node: wrap read-check-write in transaction_core so two concurrent moves can't both pass the cycle/conflict check and commit. - vfs_mirror sync_mirror: best-effort per-entry creation that never aborts the stale-removal pass (the early-? defect); surfaces the first error at the end. - fingerprint find_near_duplicates joins live_samples so orphaned fingerprints don't surface as phantom duplicates; load_fingerprint maps only NoRows to SampleNotFound. - rules set_rule_enabled now reconciles membership on toggle (mirrors delete_rule); update_rule uses a real UPDATE and errors RuleNotFound instead of INSERT OR REPLACE resurrecting a deleted rule; create_rule uses plain INSERT so an id collision errors loudly. - harvest groups by directory id not bare folder name (sibling "Kicks" folders stay distinct); rename dedup suffixes skip names already in the batch. 584 core tests pass (6 new tombstone/reconcile tests); clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-23 15:38 UTC
Commit: d07a34c360bb93be9699cb1ce12640b754f3d672
Parent: 72d2d3e
10 files changed, +373 insertions, -102 deletions
M crates/audiofiles-core/src/collections.rs +25 -3
@@ -83,9 +83,10 @@ pub fn update_collection_filter(
83 83 #[instrument(skip_all)]
84 84 pub fn list_collections(db: &Database) -> Result<Vec<Collection>> {
85 85 let mut stmt = db.conn().prepare(
86 - "SELECT c.id, c.name, c.description, c.created_at, COUNT(cm.sample_hash), c.filter_json
86 + "SELECT c.id, c.name, c.description, c.created_at, COUNT(ls.hash), c.filter_json
87 87 FROM collections c
88 88 LEFT JOIN collection_members cm ON cm.collection_id = c.id
89 + LEFT JOIN live_samples ls ON ls.hash = cm.sample_hash
89 90 GROUP BY c.id
90 91 ORDER BY c.name ASC",
91 92 )?;
@@ -163,7 +164,9 @@ pub fn remove_from_collection(db: &Database, collection_id: CollectionId, sample
163 164 #[instrument(skip_all)]
164 165 pub fn list_collection_members(db: &Database, collection_id: CollectionId) -> Result<Vec<String>> {
165 166 let mut stmt = db.conn().prepare(
166 - "SELECT sample_hash FROM collection_members WHERE collection_id = ?1 ORDER BY added_at ASC",
167 + "SELECT cm.sample_hash FROM collection_members cm \
168 + JOIN live_samples s ON s.hash = cm.sample_hash \
169 + WHERE cm.collection_id = ?1 ORDER BY cm.added_at ASC",
167 170 )?;
168 171 let rows = stmt.query_map([collection_id], |row| row.get::<_, String>(0))?;
169 172 Ok(rows.collect::<std::result::Result<Vec<_>, _>>()?)
@@ -173,10 +176,11 @@ pub fn list_collection_members(db: &Database, collection_id: CollectionId) -> Re
173 176 #[instrument(skip_all)]
174 177 pub fn get_sample_collections(db: &Database, sample_hash: &str) -> Result<Vec<Collection>> {
175 178 let mut stmt = db.conn().prepare(
176 - "SELECT c.id, c.name, c.description, c.created_at, COUNT(cm2.sample_hash), c.filter_json
179 + "SELECT c.id, c.name, c.description, c.created_at, COUNT(ls.hash), c.filter_json
177 180 FROM collections c
178 181 INNER JOIN collection_members cm ON cm.collection_id = c.id AND cm.sample_hash = ?1
179 182 LEFT JOIN collection_members cm2 ON cm2.collection_id = c.id
183 + LEFT JOIN live_samples ls ON ls.hash = cm2.sample_hash
180 184 GROUP BY c.id
181 185 ORDER BY c.name ASC",
182 186 )?;
@@ -228,6 +232,24 @@ mod tests {
228 232 }
229 233
230 234 #[test]
235 + fn tombstoned_members_drop_from_listing_and_count() {
236 + let db = setup();
237 + let id = create_collection(&db, "Drums", None).unwrap();
238 + add_to_collection(&db, id, "aaa").unwrap();
239 + add_to_collection(&db, id, "bbb").unwrap();
240 +
241 + assert_eq!(list_collection_members(&db, id).unwrap().len(), 2);
242 + assert_eq!(list_collections(&db).unwrap()[0].member_count, 2);
243 + assert_eq!(get_sample_collections(&db, "aaa").unwrap()[0].member_count, 2);
244 +
245 + // Soft-delete one member: listing, count, and per-sample count all drop it.
246 + crate::store::tombstone_sample(&db, "bbb").unwrap();
247 + assert_eq!(list_collection_members(&db, id).unwrap(), vec!["aaa"]);
248 + assert_eq!(list_collections(&db).unwrap()[0].member_count, 1);
249 + assert_eq!(get_sample_collections(&db, "aaa").unwrap()[0].member_count, 1);
250 + }
251 +
252 + #[test]
231 253 fn create_and_list() {
232 254 let db = setup();
233 255 let id = create_collection(&db, "Favorites", Some("My best samples")).unwrap();
M crates/audiofiles-core/src/db.rs +21 -6
@@ -1493,6 +1493,20 @@ CREATE TABLE IF NOT EXISTS hlc_ledger (
1493 1493 );
1494 1494 "#;
1495 1495
1496 + const MIGRATION_031: &str = r#"
1497 + -- Single source of truth for "a sample the user can see". Soft-delete
1498 + -- (deleted_at set, row retained for the tombstone-retention window) is live, so
1499 + -- any membership/listing/count query that reads the join tables (tags,
1500 + -- collection_members, vfs_nodes) must exclude tombstoned samples. That filter
1501 + -- was opt-in per query and drifted: search.rs and rules.rs filtered, but
1502 + -- tags/collections/list_full_tree did not, surfacing deleted samples in tag and
1503 + -- collection views and recreating dangling mirror symlinks. Defining the filter
1504 + -- once as a view means callers say `FROM live_samples` and the predicate lives
1505 + -- in exactly one place. Local-only derived object; no triggers, not synced.
1506 + CREATE VIEW IF NOT EXISTS live_samples AS
1507 + SELECT * FROM samples WHERE deleted_at IS NULL;
1508 + "#;
1509 +
1496 1510 /// Register `hash_row_id(salt, key) -> TEXT` as a deterministic SQLite
1497 1511 /// function on the given connection. Used by the M018 sync triggers so the
1498 1512 /// `sync_changelog.row_id` field never carries cleartext content (tag strings,
@@ -1628,6 +1642,7 @@ impl Database {
1628 1642 MIGRATION_028,
1629 1643 MIGRATION_029,
1630 1644 MIGRATION_030,
1645 + MIGRATION_031,
1631 1646 ];
1632 1647
1633 1648 for (i, sql) in MIGRATIONS.iter().enumerate() {
@@ -1868,7 +1883,7 @@ mod tests {
1868 1883 .conn()
1869 1884 .query_row("PRAGMA user_version", [], |row| row.get(0))
1870 1885 .unwrap();
1871 - assert_eq!(version, 30);
1886 + assert_eq!(version, 31);
1872 1887 }
1873 1888
1874 1889 #[test]
@@ -1879,7 +1894,7 @@ mod tests {
1879 1894 .conn()
1880 1895 .query_row("PRAGMA user_version", [], |row| row.get(0))
1881 1896 .unwrap();
1882 - assert_eq!(version, 30);
1897 + assert_eq!(version, 31);
1883 1898 }
1884 1899
1885 1900 /// Open a fresh file-backed DB, close, reopen. The second open re-enters
@@ -1898,7 +1913,7 @@ mod tests {
1898 1913 .conn()
1899 1914 .query_row("PRAGMA user_version", [], |row| row.get(0))
1900 1915 .unwrap();
1901 - assert_eq!(version, 30);
1916 + assert_eq!(version, 31);
1902 1917 }
1903 1918
1904 1919 /// Simulates the worst-case recovery path: a prior partial migration left
@@ -1942,7 +1957,7 @@ mod tests {
1942 1957 .conn()
1943 1958 .query_row("PRAGMA user_version", [], |row| row.get(0))
1944 1959 .unwrap();
1945 - assert_eq!(version, 30);
1960 + assert_eq!(version, 31);
1946 1961 }
1947 1962
1948 1963 /// M018 contract: the `sync_changelog.row_id` for sensitive tables must
@@ -2164,7 +2179,7 @@ mod tests {
2164 2179 let initial_version: i32 = conn
2165 2180 .query_row("PRAGMA user_version", [], |row| row.get(0))
2166 2181 .unwrap();
2167 - assert_eq!(initial_version, 30);
2182 + assert_eq!(initial_version, 31);
2168 2183
2169 2184 let batch = format!(
2170 2185 "BEGIN;\n{}\nPRAGMA user_version = 999;\nCOMMIT;",
@@ -2227,7 +2242,7 @@ mod tests {
2227 2242 .conn()
2228 2243 .query_row("PRAGMA user_version", [], |row| row.get(0))
2229 2244 .unwrap();
2230 - assert_eq!(version, 30);
2245 + assert_eq!(version, 31);
2231 2246 }
2232 2247
2233 2248 #[test]
M crates/audiofiles-core/src/error.rs +4
@@ -43,6 +43,10 @@ pub enum CoreError {
43 43 #[error("collection not found: {0}")]
44 44 CollectionNotFound(CollectionId),
45 45
46 + /// A tag rule with the given id does not exist.
47 + #[error("rule not found: {0}")]
48 + RuleNotFound(String),
49 +
46 50 /// A node with the same name already exists in the target directory.
47 51 #[error("name conflict: {0}")]
48 52 NameConflict(String),
M crates/audiofiles-core/src/fingerprint.rs +10 -2
@@ -94,7 +94,10 @@ pub fn load_fingerprint(db: &Database, hash: &str) -> Result<Fingerprint> {
94 94 })
95 95 },
96 96 )
97 - .map_err(|_| CoreError::SampleNotFound(hash.to_string()))
97 + .map_err(|e| match e {
98 + rusqlite::Error::QueryReturnedNoRows => CoreError::SampleNotFound(hash.to_string()),
99 + other => CoreError::Db(other),
100 + })
98 101 }
99 102
100 103 /// Find near-duplicates of the given sample by comparing peak envelopes (linear scan).
@@ -111,8 +114,13 @@ pub fn find_near_duplicates(
111 114 ) -> Result<Vec<DuplicateResult>> {
112 115 let reference = load_fingerprint(db, hash)?;
113 116
117 + // Join live_samples so a fingerprint left behind by a deleted or purged
118 + // sample (the fingerprints row is not FK-cascaded with the sample) can't
119 + // surface as a duplicate candidate pointing at a phantom hash.
114 120 let mut stmt = db.conn().prepare(
115 - "SELECT hash, envelope, sample_rate FROM fingerprints WHERE hash != ?1",
121 + "SELECT f.hash, f.envelope, f.sample_rate FROM fingerprints f \
122 + JOIN live_samples s ON s.hash = f.hash \
123 + WHERE f.hash != ?1",
116 124 )?;
117 125 let others: Vec<Fingerprint> = stmt
118 126 .query_map([hash], |row| {
M crates/audiofiles-core/src/harvest.rs +22 -13
@@ -53,34 +53,43 @@ pub fn normalize_to_tag(folder_name: &str) -> Option<String> {
53 53 #[instrument(skip_all)]
54 54 pub fn harvest_folder_labels(db: &Database) -> Result<Vec<FolderLabel>> {
55 55 let mut stmt = db.conn().prepare(
56 - "SELECT d.name, s.sample_hash
56 + "SELECT d.id, d.name, s.sample_hash
57 57 FROM vfs_nodes d
58 58 JOIN vfs_nodes s ON s.parent_id = d.id AND s.node_type = 'sample'
59 59 WHERE d.node_type = 'directory' AND s.sample_hash IS NOT NULL
60 - ORDER BY d.name, s.sample_hash",
60 + ORDER BY d.name, d.id, s.sample_hash",
61 61 )?;
62 62 let rows = stmt.query_map([], |row| {
63 - Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))
63 + Ok((
64 + row.get::<_, i64>(0)?,
65 + row.get::<_, String>(1)?,
66 + row.get::<_, String>(2)?,
67 + ))
64 68 })?;
65 69
66 - // Group hashes by folder name, preserving first-seen folder order.
67 - let mut order: Vec<String> = Vec::new();
68 - let mut by_folder: std::collections::HashMap<String, Vec<String>> = std::collections::HashMap::new();
70 + // Group hashes by directory id, not bare name: two sibling folders both
71 + // named e.g. "Kicks" (under 808/ and 909/) are distinct folders and must
72 + // each propose their own label, not merge into one union.
73 + let mut order: Vec<i64> = Vec::new();
74 + let mut by_folder: std::collections::HashMap<i64, (String, Vec<String>)> =
75 + std::collections::HashMap::new();
69 76 for r in rows {
70 - let (name, hash) = r?;
77 + let (id, name, hash) = r?;
71 78 by_folder
72 - .entry(name.clone())
79 + .entry(id)
73 80 .or_insert_with(|| {
74 - order.push(name.clone());
75 - Vec::new()
81 + order.push(id);
82 + (name, Vec::new())
76 83 })
84 + .1
77 85 .push(hash);
78 86 }
79 87
80 88 let mut labels = Vec::new();
81 - for folder_name in order {
82 - if let Some(suggested_tag) = normalize_to_tag(&folder_name) {
83 - let sample_hashes = by_folder.remove(&folder_name).unwrap_or_default();
89 + for id in order {
90 + if let Some((folder_name, sample_hashes)) = by_folder.remove(&id)
91 + && let Some(suggested_tag) = normalize_to_tag(&folder_name)
92 + {
84 93 labels.push(FolderLabel { folder_name, suggested_tag, sample_hashes });
85 94 }
86 95 }
M crates/audiofiles-core/src/rename.rs +15 -4
@@ -1,6 +1,6 @@
1 1 //! Rename pattern engine: parse token patterns and resolve them into filenames.
2 2
3 - use std::collections::HashMap;
3 + use std::collections::{HashMap, HashSet};
4 4
5 5 use crate::error::{CoreError, Result};
6 6
@@ -157,15 +157,26 @@ fn deduplicate(stems: Vec<String>) -> Vec<String> {
157 157 *counts.entry(stem.clone()).or_insert(0) += 1;
158 158 }
159 159
160 - // Second pass: assign suffixes where needed
160 + // Second pass: assign suffixes where needed. A generated suffix must not
161 + // collide with a stem that already exists anywhere in the batch — input
162 + // ["kick","kick","kick (2)"] must not produce two "kick (2)". `used` tracks
163 + // every name already present or emitted so suffixes skip past collisions.
164 + let mut used: HashSet<String> = stems.iter().cloned().collect();
161 165 let mut seen: HashMap<String, usize> = HashMap::new();
162 166 let mut result = Vec::with_capacity(stems.len());
163 167 for stem in stems {
164 168 if counts[&stem] <= 1 {
165 169 result.push(stem);
166 170 } else if let Some(n) = seen.get_mut(&stem) {
167 - *n += 1;
168 - result.push(format!("{stem} ({n})"));
171 + let mut next = *n + 1;
172 + let mut candidate = format!("{stem} ({next})");
173 + while used.contains(&candidate) {
174 + next += 1;
175 + candidate = format!("{stem} ({next})");
176 + }
177 + *n = next;
178 + used.insert(candidate.clone());
179 + result.push(candidate);
169 180 } else {
170 181 seen.insert(stem.clone(), 1);
171 182 result.push(stem);
M crates/audiofiles-core/src/rules.rs +114 -15
@@ -505,15 +505,23 @@ pub fn get_rule(db: &Database, id: &str) -> Result<Option<Rule>> {
505 505 Ok(db.conn().query_row(&sql, [id], parse_rule).ok())
506 506 }
507 507
508 - fn write_rule(db: &Database, rule: &Rule) -> Result<()> {
508 + /// Serialize a rule's JSON columns. Shared by insert and update.
509 + fn rule_json(rule: &Rule) -> Result<(String, String, String)> {
509 510 let match_mode = serde_json::to_string(&rule.match_mode)
510 511 .map_err(|e| CoreError::Serialization(e.to_string()))?;
511 512 let conditions = serde_json::to_string(&rule.conditions)
512 513 .map_err(|e| CoreError::Serialization(e.to_string()))?;
513 514 let actions = serde_json::to_string(&rule.actions)
514 515 .map_err(|e| CoreError::Serialization(e.to_string()))?;
516 + Ok((match_mode, conditions, actions))
517 + }
518 +
519 + /// Insert a brand-new rule. Plain INSERT (not INSERT OR REPLACE): a primary-key
520 + /// collision is a real error, not a silent clobber of an existing rule.
521 + fn insert_rule(db: &Database, rule: &Rule) -> Result<()> {
522 + let (match_mode, conditions, actions) = rule_json(rule)?;
515 523 db.conn().execute(
516 - "INSERT OR REPLACE INTO tag_rules
524 + "INSERT INTO tag_rules
517 525 (id, name, enabled, priority, match_mode, conditions, actions, created_at)
518 526 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)",
519 527 rusqlite::params![
@@ -556,7 +564,7 @@ pub fn create_rule(db: &Database, new: NewRule) -> Result<Rule> {
556 564 actions: new.actions,
557 565 created_at: now_secs(),
558 566 };
559 - write_rule(db, &rule)?;
567 + insert_rule(db, &rule)?;
560 568 Ok(rule)
561 569 }
562 570
@@ -568,16 +576,61 @@ pub fn update_rule(db: &Database, rule: &Rule) -> Result<()> {
568 576 validate_tag(tag)?;
569 577 }
570 578 }
571 - write_rule(db, rule)
579 + let (match_mode, conditions, actions) = rule_json(rule)?;
580 + // Real UPDATE (not INSERT OR REPLACE): updating a deleted/unknown id must
581 + // error, not silently resurrect the rule. created_at is preserved (not in
582 + // the SET list).
583 + let changed = db.conn().execute(
584 + "UPDATE tag_rules SET
585 + name = ?2, enabled = ?3, priority = ?4,
586 + match_mode = ?5, conditions = ?6, actions = ?7
587 + WHERE id = ?1",
588 + rusqlite::params![
589 + rule.id,
590 + rule.name,
591 + rule.enabled,
592 + rule.priority,
593 + match_mode,
594 + conditions,
595 + actions,
596 + ],
597 + )?;
598 + if changed == 0 {
599 + return Err(CoreError::RuleNotFound(rule.id.clone()));
600 + }
601 + Ok(())
572 602 }
573 603
574 - /// Toggle a rule's enabled flag.
604 + /// Toggle a rule's enabled flag and reconcile affected samples.
605 + ///
606 + /// Toggling membership without reconciling leaves stale `source = 'rule'` tags
607 + /// applied (when disabling) or missing (when enabling) until some unrelated
608 + /// `apply_*` runs. This mirrors [`delete_rule`]: the flag flip and the
609 + /// reconciliation run in one transaction so membership is never observed
610 + /// half-updated. Disabling only touches samples the rule had tagged; enabling
611 + /// re-evaluates the whole library, since the rule may now match samples it never
612 + /// tagged before.
575 613 #[instrument(skip_all)]
576 614 pub fn set_rule_enabled(db: &Database, id: &str, enabled: bool) -> Result<()> {
577 - db.conn().execute(
578 - "UPDATE tag_rules SET enabled = ?2 WHERE id = ?1",
579 - rusqlite::params![id, enabled],
580 - )?;
615 + let previously_tagged = samples_tagged_by_rule(db, id)?;
616 + db.transaction_core(|tx| {
617 + db.conn().execute(
618 + "UPDATE tag_rules SET enabled = ?2 WHERE id = ?1",
619 + rusqlite::params![id, enabled],
620 + )?;
621 + let rules = list_rules(db)?;
622 + let to_reconcile: Vec<String> = if enabled {
623 + let mut stmt = db.conn().prepare("SELECT hash FROM live_samples")?;
624 + stmt.query_map([], |row| row.get(0))?
625 + .collect::<std::result::Result<Vec<_>, _>>()?
626 + } else {
627 + previously_tagged
628 + };
629 + for hash in &to_reconcile {
630 + reconcile_sample(db, tx, hash, &rules)?;
631 + }
632 + Ok(())
633 + })?;
581 634 Ok(())
582 635 }
583 636
@@ -717,9 +770,7 @@ pub fn apply_rules_to_samples(db: &Database, hashes: &[String]) -> Result<usize>
717 770 pub fn apply_all_rules(db: &Database) -> Result<usize> {
718 771 let rules = list_rules(db)?;
719 772 let hashes: Vec<String> = {
720 - let mut stmt = db
721 - .conn()
722 - .prepare("SELECT hash FROM samples WHERE deleted_at IS NULL")?;
773 + let mut stmt = db.conn().prepare("SELECT hash FROM live_samples")?;
723 774 stmt.query_map([], |row| row.get(0))?
724 775 .collect::<std::result::Result<Vec<_>, _>>()?
725 776 };
@@ -738,9 +789,7 @@ pub fn apply_all_rules(db: &Database) -> Result<usize> {
738 789 #[instrument(skip_all)]
739 790 pub fn preview_rule_matches(db: &Database, rule: &Rule) -> Result<usize> {
740 791 let hashes: Vec<String> = {
741 - let mut stmt = db
742 - .conn()
743 - .prepare("SELECT hash FROM samples WHERE deleted_at IS NULL")?;
792 + let mut stmt = db.conn().prepare("SELECT hash FROM live_samples")?;
744 793 stmt.query_map([], |row| row.get(0))?
745 794 .collect::<std::result::Result<Vec<_>, _>>()?
746 795 };
@@ -932,6 +981,56 @@ mod tests {
932 981 }
933 982
934 983 #[test]
984 + fn toggling_enabled_reconciles_membership() {
985 + let db = db_with_sample("h6", "kick.wav");
986 + let rule = create_rule(
987 + &db,
988 + new_rule(
989 + "kicks",
990 + vec![cond(RuleField::Name, RuleOp::Contains, "kick")],
991 + vec![RuleAction::AddTag("instrument.drum.kick".into())],
992 + ),
993 + )
994 + .unwrap();
995 + apply_rules_to_sample(&db, "h6").unwrap();
996 + assert!(!crate::tags::get_sample_tags(&db, "h6").unwrap().is_empty());
997 +
998 + // Disabling must remove the rule-sourced tag immediately (no separate
999 + // apply_* call), not leave it stale.
1000 + set_rule_enabled(&db, &rule.id, false).unwrap();
1001 + assert!(crate::tags::get_sample_tags(&db, "h6").unwrap().is_empty());
1002 +
1003 + // Re-enabling must re-apply it across the library, again without an
1004 + // explicit apply_* call.
1005 + set_rule_enabled(&db, &rule.id, true).unwrap();
1006 + assert!(crate::tags::get_sample_tags(&db, "h6").unwrap().contains(
1007 + &"instrument.drum.kick".to_string()
1008 + ));
1009 + }
1010 +
1011 + #[test]
1012 + fn update_unknown_rule_errors_not_resurrects() {
1013 + let db = db_with_sample("h7", "kick.wav");
1014 + let rule = create_rule(
1015 + &db,
1016 + new_rule(
1017 + "kicks",
1018 + vec![cond(RuleField::Name, RuleOp::Contains, "kick")],
1019 + vec![RuleAction::AddTag("instrument.drum.kick".into())],
1020 + ),
1021 + )
1022 + .unwrap();
1023 + delete_rule(&db, &rule.id).unwrap();
1024 +
1025 + // Updating the now-deleted rule must error, not silently re-insert it.
1026 + assert!(matches!(
1027 + update_rule(&db, &rule),
1028 + Err(CoreError::RuleNotFound(_))
1029 + ));
1030 + assert!(get_rule(&db, &rule.id).unwrap().is_none());
1031 + }
1032 +
1033 + #[test]
935 1034 fn match_mode_any_vs_all() {
936 1035 let db = db_with_sample("h5", "snare hit.wav");
937 1036 let any = create_rule(
M crates/audiofiles-core/src/tags.rs +39 -7
@@ -65,9 +65,11 @@ pub fn get_sample_tags(db: &Database, hash: &str) -> Result<Vec<String>> {
65 65 /// Find sample hashes with an exact tag.
66 66 #[instrument(skip_all)]
67 67 pub fn find_by_tag(db: &Database, tag: &str) -> Result<Vec<String>> {
68 - let mut stmt = db
69 - .conn()
70 - .prepare("SELECT sample_hash FROM tags WHERE tag = ?1 ORDER BY sample_hash")?;
68 + let mut stmt = db.conn().prepare(
69 + "SELECT t.sample_hash FROM tags t \
70 + JOIN live_samples s ON s.hash = t.sample_hash \
71 + WHERE t.tag = ?1 ORDER BY t.sample_hash",
72 + )?;
71 73 let rows = stmt.query_map([tag], |row| row.get(0))?;
72 74 Ok(rows.collect::<std::result::Result<Vec<_>, _>>()?)
73 75 }
@@ -76,7 +78,9 @@ pub fn find_by_tag(db: &Database, tag: &str) -> Result<Vec<String>> {
76 78 #[instrument(skip_all)]
77 79 pub fn find_by_prefix(db: &Database, prefix: &str) -> Result<Vec<String>> {
78 80 let mut stmt = db.conn().prepare(
79 - "SELECT DISTINCT sample_hash FROM tags WHERE tag = ?1 OR tag LIKE ?2 ESCAPE '\\' ORDER BY sample_hash",
81 + "SELECT DISTINCT t.sample_hash FROM tags t \
82 + JOIN live_samples s ON s.hash = t.sample_hash \
83 + WHERE t.tag = ?1 OR t.tag LIKE ?2 ESCAPE '\\' ORDER BY t.sample_hash",
80 84 )?;
81 85 let rows = stmt.query_map(
82 86 rusqlite::params![prefix, tagtree::like_descendant_pattern(prefix)],
@@ -88,9 +92,11 @@ pub fn find_by_prefix(db: &Database, prefix: &str) -> Result<Vec<String>> {
88 92 /// List all distinct tags in the database, sorted.
89 93 #[instrument(skip_all)]
90 94 pub fn list_all_tags(db: &Database) -> Result<Vec<String>> {
91 - let mut stmt = db
92 - .conn()
93 - .prepare("SELECT DISTINCT tag FROM tags ORDER BY tag")?;
95 + let mut stmt = db.conn().prepare(
96 + "SELECT DISTINCT t.tag FROM tags t \
97 + JOIN live_samples s ON s.hash = t.sample_hash \
98 + ORDER BY t.tag",
99 + )?;
94 100 let rows = stmt.query_map([], |row| row.get(0))?;
95 101 Ok(rows.collect::<std::result::Result<Vec<_>, _>>()?)
96 102 }
@@ -191,12 +197,38 @@ pub fn list_children_tags(db: &Database, prefix: &str) -> Result<Vec<String>> {
191 197 #[cfg(test)]
192 198 mod tests {
193 199 use super::*;
200 + use crate::store::tombstone_sample;
194 201 use crate::test_helpers::insert_fake_sample;
195 202
196 203 fn setup() -> Database {
197 204 Database::open_in_memory().unwrap()
198 205 }
199 206
207 + #[test]
208 + fn discovery_queries_exclude_tombstoned_samples() {
209 + let db = setup();
210 + insert_fake_sample(&db, "live1");
211 + insert_fake_sample(&db, "dead1");
212 + add_tag(&db, "live1", "genre.house").unwrap();
213 + add_tag(&db, "dead1", "genre.house").unwrap();
214 +
215 + // Both visible before tombstoning.
216 + assert_eq!(find_by_tag(&db, "genre.house").unwrap().len(), 2);
217 + assert_eq!(find_by_prefix(&db, "genre").unwrap().len(), 2);
218 + assert!(list_all_tags(&db).unwrap().contains(&"genre.house".to_string()));
219 +
220 + // Soft-delete dead1: its tags must drop out of every discovery query.
221 + assert!(tombstone_sample(&db, "dead1").unwrap());
222 + assert_eq!(find_by_tag(&db, "genre.house").unwrap(), vec!["live1"]);
223 + assert_eq!(find_by_prefix(&db, "genre").unwrap(), vec!["live1"]);
224 + assert!(list_all_tags(&db).unwrap().contains(&"genre.house".to_string()));
225 +
226 + // Tombstone the last holder: the tag disappears from list_all_tags.
227 + assert!(tombstone_sample(&db, "live1").unwrap());
228 + assert!(find_by_tag(&db, "genre.house").unwrap().is_empty());
229 + assert!(list_all_tags(&db).unwrap().is_empty());
230 + }
231 +
200 232 // --- Validation tests ---
201 233
202 234 #[test]
M crates/audiofiles-core/src/vfs.rs +86 -42
@@ -340,16 +340,22 @@ pub fn get_node(db: &Database, id: NodeId) -> Result<VfsNode> {
340 340 #[instrument(skip_all)]
341 341 pub fn rename_node(db: &Database, id: NodeId, new_name: &str) -> Result<()> {
342 342 validate_node_name(new_name)?;
343 - let node = get_node(db, id)?;
344 - check_sibling_name_conflict(db, node.vfs_id, node.parent_id, new_name, id)?;
345 - let changed = db.conn().execute(
346 - "UPDATE vfs_nodes SET name = ?1 WHERE id = ?2",
347 - rusqlite::params![new_name, id],
348 - )?;
349 - if changed == 0 {
350 - return Err(CoreError::NodeNotFound(id));
351 - }
352 - Ok(())
343 + // Read-check-write under one write lock: the sibling-conflict check and the
344 + // UPDATE must be atomic, else two concurrent renames can both pass the check
345 + // and collide (the partial unique index catches root collisions, but not the
346 + // general sibling case).
347 + db.transaction_core(|_tx| {
348 + let node = get_node(db, id)?;
349 + check_sibling_name_conflict(db, node.vfs_id, node.parent_id, new_name, id)?;
350 + let changed = db.conn().execute(
351 + "UPDATE vfs_nodes SET name = ?1 WHERE id = ?2",
352 + rusqlite::params![new_name, id],
353 + )?;
354 + if changed == 0 {
355 + return Err(CoreError::NodeNotFound(id));
356 + }
357 + Ok(())
358 + })
353 359 }
354 360
355 361 /// Move a VFS node to a new parent directory (or root if `None`).
@@ -358,44 +364,50 @@ pub fn rename_node(db: &Database, id: NodeId, new_name: &str) -> Result<()> {
358 364 /// cross a VFS boundary, or conflict with an existing sibling name.
359 365 #[instrument(skip_all)]
360 366 pub fn move_node(db: &Database, id: NodeId, new_parent_id: Option<NodeId>) -> Result<()> {
361 - let node = get_node(db, id)?;
362 -
363 - // Reject cross-VFS moves.
364 - if let Some(parent) = new_parent_id {
365 - let parent_node = get_node(db, parent)?;
366 - if parent_node.vfs_id != node.vfs_id {
367 - return Err(CoreError::Internal(
368 - "cannot move a node to a different VFS".to_string(),
369 - ));
370 - }
371 - }
367 + // The cycle-walk, conflict check, and UPDATE run under one write lock so two
368 + // concurrent moves cannot each validate against the old tree and both commit
369 + // a cycle the schema has no constraint to reject (a later recursive CTE would
370 + // then loop to SQLite's recursion limit).
371 + db.transaction_core(|_tx| {
372 + let node = get_node(db, id)?;
372 373
373 - // Check for circular reference: walk from new_parent_id up to root.
374 - // If we encounter `id` along the way, the move would create a cycle.
375 - if let Some(parent) = new_parent_id {
376 - let mut current = Some(parent);
377 - while let Some(cur_id) = current {
378 - if cur_id == id {
374 + // Reject cross-VFS moves.
375 + if let Some(parent) = new_parent_id {
376 + let parent_node = get_node(db, parent)?;
377 + if parent_node.vfs_id != node.vfs_id {
379 378 return Err(CoreError::Internal(
380 - "move would create a circular parent reference".to_string(),
379 + "cannot move a node to a different VFS".to_string(),
381 380 ));
382 381 }
383 - let cur_node = get_node(db, cur_id)?;
384 - current = cur_node.parent_id;
385 382 }
386 - }
387 383
388 - // Check for name conflicts at the destination.
389 - check_sibling_name_conflict(db, node.vfs_id, new_parent_id, &node.name, id)?;
384 + // Check for circular reference: walk from new_parent_id up to root.
385 + // If we encounter `id` along the way, the move would create a cycle.
386 + if let Some(parent) = new_parent_id {
387 + let mut current = Some(parent);
388 + while let Some(cur_id) = current {
389 + if cur_id == id {
390 + return Err(CoreError::Internal(
391 + "move would create a circular parent reference".to_string(),
392 + ));
393 + }
394 + let cur_node = get_node(db, cur_id)?;
395 + current = cur_node.parent_id;
396 + }
397 + }
390 398
391 - let changed = db.conn().execute(
392 - "UPDATE vfs_nodes SET parent_id = ?1 WHERE id = ?2",
393 - rusqlite::params![new_parent_id, id],
394 - )?;
395 - if changed == 0 {
396 - return Err(CoreError::NodeNotFound(id));
397 - }
398 - Ok(())
399 + // Check for name conflicts at the destination.
400 + check_sibling_name_conflict(db, node.vfs_id, new_parent_id, &node.name, id)?;
401 +
402 + let changed = db.conn().execute(
403 + "UPDATE vfs_nodes SET parent_id = ?1 WHERE id = ?2",
404 + rusqlite::params![new_parent_id, id],
405 + )?;
406 + if changed == 0 {
407 + return Err(CoreError::NodeNotFound(id));
408 + }
409 + Ok(())
410 + })
399 411 }
400 412
401 413 /// Delete a VFS node. Child nodes are removed by `ON DELETE CASCADE`.
@@ -540,6 +552,12 @@ pub struct FullTreeNode {
540 552 #[instrument(skip_all)]
541 553 pub fn list_full_tree(db: &Database) -> Result<Vec<FullTreeNode>> {
542 554 let mut stmt = db.conn().prepare(
555 + // Folder nodes (sample_hash NULL) always recurse and appear. File nodes
556 + // appear only when their sample is live: a tombstoned sample (deleted_at
557 + // set) or a purged one (samples row gone, placement retained) is absent
558 + // from live_samples, so the outer filter drops the leaf and the mirror
559 + // never recreates a dangling symlink for it. The recursion is unfiltered
560 + // so a folder above a tombstoned file still builds its path.
543 561 "WITH RECURSIVE tree(id, path, node_type, sample_hash) AS (
544 562 SELECT n.id, v.name || '/' || n.name,
545 563 n.node_type, n.sample_hash
@@ -550,7 +568,11 @@ pub fn list_full_tree(db: &Database) -> Result<Vec<FullTreeNode>> {
550 568 n.node_type, n.sample_hash
551 569 FROM vfs_nodes n JOIN tree t ON n.parent_id = t.id
552 570 )
553 - SELECT path, node_type, sample_hash FROM tree ORDER BY path",
571 + SELECT t.path, t.node_type, t.sample_hash
572 + FROM tree t
573 + LEFT JOIN live_samples s ON s.hash = t.sample_hash
574 + WHERE t.sample_hash IS NULL OR s.hash IS NOT NULL
575 + ORDER BY t.path",
554 576 )?;
555 577 let rows = stmt.query_map([], |row| {
556 578 let nt: String = row.get(1)?;
@@ -678,6 +700,28 @@ mod tests {
678 700 }
679 701
680 702 #[test]
703 + fn full_tree_excludes_tombstoned_files_keeps_folders() {
704 + let db = setup();
705 + let vfs_id = create_vfs(&db, "Library").unwrap();
706 + let dir = create_directory(&db, vfs_id, None, "Drums").unwrap();
707 + insert_fake_sample(&db, "live");
708 + insert_fake_sample(&db, "dead");
709 + create_sample_link(&db, vfs_id, Some(dir), "live.wav", "live").unwrap();
710 + create_sample_link(&db, vfs_id, Some(dir), "dead.wav", "dead").unwrap();
711 +
712 + let before = list_full_tree(&db).unwrap();
713 + assert!(before.iter().any(|n| n.path.ends_with("dead.wav")));
714 +
715 + // Tombstone "dead": its leaf must vanish from the mirror's tree, but the
716 + // Drums folder (which still holds live.wav) must remain.
717 + crate::store::tombstone_sample(&db, "dead").unwrap();
718 + let after = list_full_tree(&db).unwrap();
719 + assert!(!after.iter().any(|n| n.path.ends_with("dead.wav")));
720 + assert!(after.iter().any(|n| n.path.ends_with("live.wav")));
721 + assert!(after.iter().any(|n| n.node_type == NodeType::Directory && n.path.ends_with("Drums")));
722 + }
723 +
724 + #[test]
681 725 fn rename_vfs_works() {
682 726 let db = setup();
683 727 let id = create_vfs(&db, "Old").unwrap();
M crates/audiofiles-core/src/vfs_mirror.rs +37 -10
@@ -54,6 +54,16 @@ pub fn sync_mirror(db: &Database, config: &MirrorConfig) -> Result<MirrorStats>
54 54 .collect();
55 55 let locations = batch_locations(db, &hashes);
56 56
57 + // The mirror is rebuilt incrementally, so a per-entry failure (disk full, a
58 + // permission error on one path) must NOT abort the whole sync — that would
59 + // skip the stale-removal pass and leave the mirror more inconsistent than
60 + // before (the original early-`?` defect). Each entry is best-effort: a
61 + // failure is logged, the first error is retained, and the loop continues so
62 + // every reachable entry is created and stale-removal always runs. The first
63 + // error is surfaced at the end, after the mirror has been made as consistent
64 + // as possible.
65 + let mut first_err: Option<crate::error::CoreError> = None;
66 +
57 67 for node in &tree {
58 68 let sanitized = sanitize_path(&node.path);
59 69 let full_path = config.mirror_root.join(&sanitized);
@@ -64,9 +74,13 @@ pub fn sync_mirror(db: &Database, config: &MirrorConfig) -> Result<MirrorStats>
64 74 existing.remove(&full_path);
65 75
66 76 if !full_path.exists() {
67 - std::fs::create_dir_all(&full_path)
68 - .map_err(|e| io_err(&full_path, e))?;
69 - stats.dirs_created += 1;
77 + match std::fs::create_dir_all(&full_path) {
78 + Ok(()) => stats.dirs_created += 1,
79 + Err(e) => {
80 + warn!(path = %full_path.display(), "mirror: dir create failed: {e}");
81 + first_err.get_or_insert_with(|| io_err(&full_path, e));
82 + }
83 + }
70 84 }
71 85 }
72 86 NodeType::Sample => {
@@ -98,19 +112,29 @@ pub fn sync_mirror(db: &Database, config: &MirrorConfig) -> Result<MirrorStats>
98 112 if !link_path.exists() {
99 113 // Ensure parent directory exists.
100 114 if let Some(parent) = link_path.parent()
101 - && !parent.exists() {
102 - std::fs::create_dir_all(parent)
103 - .map_err(|e| io_err(parent, e))?;
115 + && !parent.exists()
116 + && let Err(e) = std::fs::create_dir_all(parent)
117 + {
118 + warn!(path = %parent.display(), "mirror: parent create failed: {e}");
119 + first_err.get_or_insert_with(|| io_err(parent, e));
120 + continue;
121 + }
122 + match create_symlink(&target, &link_path) {
123 + Ok(()) => stats.links_created += 1,
124 + Err(e) => {
125 + warn!(path = %link_path.display(), "mirror: symlink failed: {e}");
126 + first_err.get_or_insert(e);
104 127 }
105 - create_symlink(&target, &link_path)?;
106 - stats.links_created += 1;
128 + }
107 129 }
108 130 }
109 131 }
110 132 }
111 133 }
112 134
113 - // Remove stale entries (files/dirs no longer in VFS).
135 + // Remove stale entries (files/dirs no longer in VFS). Runs unconditionally,
136 + // even after per-entry creation errors, so the mirror is never left with
137 + // orphaned entries because creation aborted early.
114 138 // Sort descending so children are removed before parents.
115 139 let mut stale: Vec<PathBuf> = existing.into_iter().collect();
116 140 stale.sort_by(|a, b| b.cmp(a));
@@ -136,7 +160,10 @@ pub fn sync_mirror(db: &Database, config: &MirrorConfig) -> Result<MirrorStats>
136 160 "Mirror sync complete"
137 161 );
138 162
139 - Ok(stats)
163 + match first_err {
164 + Some(e) => Err(e),
165 + None => Ok(stats),
166 + }
140 167 }
141 168
142 169 /// Remove the entire mirror directory tree.