From 3598bb2cfecf36af3fa319e88e44cfcbe01beb66 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Fri, 1 May 2026 19:10:24 +0000 Subject: [PATCH] multi-library: operator kill switch via libraries.enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A small follow-up to Branches A/B/C. Adds a nullable-default-1 boolean column to the `libraries` table that controls whether the watcher considers the library at all. Useful for staging a new mount before committing to ingest, and as a maintenance kill switch when a library needs to be quiet without being unmounted. Migration (2026-05-01-100000_libraries_enabled_flag) ALTER TABLE libraries ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT 1. Existing rows stay enabled — no behavior change on upgrade. Watcher gate (main.rs) At the top of the per-library loop, if !lib.enabled { continue; } — runs BEFORE the availability probe. Disabled libraries don't enter the health map, don't get probed, don't get ingest, don't get any maintenance pass. The initial sweep before the loop's first sleep also skips disabled libraries. Orphan-GC consensus (library_maintenance.rs) all_libraries_online filters disabled libraries out of the consensus check — they're treated as out-of-scope, not as blockers. Otherwise flipping enabled=false would permanently halt orphan GC for the rest of the system, which is the opposite of the intended kill-switch semantics. Cross-library duplicates: safe by construction. Hash-keyed derived data (face_detections, tagged_photo with hash, photo_insights with hash) is anchored by ANY image_exif row carrying the hash. Disabling a library does NOT delete its image_exif rows, so a hash referenced by a disabled library's row stays anchored — derived data survives. collect_orphan_hashes deliberately doesn't filter image_exif by library.enabled for exactly this reason. No HTTP endpoint. Library mutation is rare-enough infra work that a SQL toggle is fine, and a public mutation endpoint without a role / permission story would be poorly-prioritized exposure for a single-user tool. Documented in CLAUDE.md. Tests: 226 pass (225 from Branch C + 1 new all_libraries_online_treats_disabled_as_out_of_scope, which proves that even an explicit Stale entry on a disabled library doesn't block the consensus). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 15 ++++ .../down.sql | 2 + .../up.sql | 14 ++++ src/database/mod.rs | 1 + src/database/models.rs | 7 ++ src/database/schema.rs | 1 + src/libraries.rs | 53 ++++++++------- src/library_maintenance.rs | 68 +++++++++++++++++-- src/main.rs | 18 +++++ src/state.rs | 2 + 10 files changed, 149 insertions(+), 32 deletions(-) create mode 100644 migrations/2026-05-01-100000_libraries_enabled_flag/down.sql create mode 100644 migrations/2026-05-01-100000_libraries_enabled_flag/up.sql diff --git a/CLAUDE.md b/CLAUDE.md index b6864cd..ae0642c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -195,6 +195,21 @@ photos that have a copy under lib N, but the derived data attached to those photos is the merged hash-keyed view. This is the answer to "show me archive photos with their original tags." +**Operator kill switch (`libraries.enabled`).** Setting `enabled=0` on a +library is a hard pause: the watcher skips it entirely — before the +probe, before ingest, before any maintenance pass — and the orphan-GC +all-online consensus check filters disabled libraries out (they don't +keep the GC window closed). Reads / serving are unaffected; nothing +prevents `/image?path=...` from resolving against a disabled library's +root if the file is on disk. The existing `image_exif` rows for a +disabled library are **not deleted** — they continue to anchor +hash-keyed derived data, so cross-library duplicates survive the +disable. Toggle via SQL; there is intentionally no HTTP endpoint for +library mutation (single-user tool, no role / permission story). +Typical workflows: stage a new mount with `enabled=0` then flip to `1`; +quiet a flaky NAS during maintenance without disturbing the rest of +the system. + **Library availability and safety.** Libraries can be on network shares or removable media; the file watcher must not interpret a temporary unavailability as a mass-deletion event. Every tick begins with a diff --git a/migrations/2026-05-01-100000_libraries_enabled_flag/down.sql b/migrations/2026-05-01-100000_libraries_enabled_flag/down.sql new file mode 100644 index 0000000..45e7a97 --- /dev/null +++ b/migrations/2026-05-01-100000_libraries_enabled_flag/down.sql @@ -0,0 +1,2 @@ +-- Requires SQLite 3.35+ for ALTER TABLE DROP COLUMN. +ALTER TABLE libraries DROP COLUMN enabled; diff --git a/migrations/2026-05-01-100000_libraries_enabled_flag/up.sql b/migrations/2026-05-01-100000_libraries_enabled_flag/up.sql new file mode 100644 index 0000000..9952259 --- /dev/null +++ b/migrations/2026-05-01-100000_libraries_enabled_flag/up.sql @@ -0,0 +1,14 @@ +-- Operator-controlled kill switch for a library. When `enabled = 0` the +-- watcher tick skips that library entirely — before the availability +-- probe, before ingest, before any maintenance pass — and the orphan-GC +-- all-online check treats it as out-of-scope rather than as a blocker. +-- +-- The intended workflow is staging a new mount: insert with enabled=0, +-- verify the row appears in /libraries with enabled=false, then UPDATE +-- to 1 to start ingest. Same toggle works as a maintenance kill switch +-- after the fact ("don't keep probing this NAS while I'm rebooting it"). +-- +-- Default 1 so every existing library stays running on upgrade — no +-- behavior change without an explicit flip. + +ALTER TABLE libraries ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT 1; diff --git a/src/database/mod.rs b/src/database/mod.rs index d591c64..3ea1cd2 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -1212,6 +1212,7 @@ mod exif_dao_tests { name: "archive", root_path: "/tmp/archive", created_at: 0, + enabled: true, }) .execute(&mut conn) .expect("seed second library"); diff --git a/src/database/models.rs b/src/database/models.rs index 78edf00..d88aa77 100644 --- a/src/database/models.rs +++ b/src/database/models.rs @@ -144,6 +144,12 @@ pub struct LibraryRow { pub name: String, pub root_path: String, pub created_at: i64, + /// Operator kill switch. `false` = the watcher skips this library + /// entirely (no probe, no ingest, no maintenance) and orphan-GC + /// treats it as out-of-scope for the all-online consensus rule. + /// Toggle via SQL today — there is intentionally no HTTP endpoint + /// for library mutation (see CLAUDE.md "Multi-library data model"). + pub enabled: bool, } #[derive(Insertable)] @@ -152,6 +158,7 @@ pub struct InsertLibrary<'a> { pub name: &'a str, pub root_path: &'a str, pub created_at: i64, + pub enabled: bool, } // --- Knowledge memory models --- diff --git a/src/database/schema.rs b/src/database/schema.rs index 5e933ca..4374218 100644 --- a/src/database/schema.rs +++ b/src/database/schema.rs @@ -130,6 +130,7 @@ diesel::table! { name -> Text, root_path -> Text, created_at -> BigInt, + enabled -> Bool, } } diff --git a/src/libraries.rs b/src/libraries.rs index 4e55300..5ec96a5 100644 --- a/src/libraries.rs +++ b/src/libraries.rs @@ -28,6 +28,13 @@ pub struct Library { pub id: i32, pub name: String, pub root_path: String, + /// Operator kill switch (mirrors `libraries.enabled`). When `false` + /// the watcher skips this library entirely — before the probe, + /// before ingest, before maintenance. Reads / serving still work + /// (a request whose path resolves to a disabled library's root + /// will succeed if the file is on disk; nothing prevents that + /// today and there's no obvious reason to). Toggle via SQL. + pub enabled: bool, } impl Library { @@ -57,6 +64,7 @@ impl From for Library { id: row.id, name: row.name, root_path: row.root_path, + enabled: row.enabled, } } } @@ -111,6 +119,7 @@ pub fn seed_or_patch_from_env(conn: &mut SqliteConnection, base_path: &str) { name: "main", root_path: base_path, created_at: now, + enabled: true, }) .execute(conn); match result { @@ -343,6 +352,7 @@ mod tests { id: 1, name: "main".into(), root_path: "/tmp/media".into(), + enabled: true, }; let rel = lib.strip_root(Path::new("/tmp/media/2024/photo.jpg")); assert_eq!(rel.as_deref(), Some("2024/photo.jpg")); @@ -356,6 +366,7 @@ mod tests { id: 1, name: "main".into(), root_path: "/tmp/media".into(), + enabled: true, }; let abs = lib.resolve("2024/photo.jpg"); assert_eq!(abs, PathBuf::from("/tmp/media/2024/photo.jpg")); @@ -373,11 +384,13 @@ mod tests { id: 1, name: "main".into(), root_path: "/tmp/main".into(), + enabled: true, }, Library { id: 7, name: "archive".into(), root_path: "/tmp/archive".into(), + enabled: true, }, ] } @@ -431,15 +444,20 @@ mod tests { assert!(err.contains("unknown library name")); } + fn probe_lib(id: i32, root: String) -> Library { + Library { + id, + name: "main".into(), + root_path: root, + enabled: true, + } + } + #[test] fn probe_online_for_existing_non_empty_dir() { let tmp = tempfile::tempdir().unwrap(); std::fs::write(tmp.path().join("photo.jpg"), b"hello").unwrap(); - let lib = Library { - id: 1, - name: "main".into(), - root_path: tmp.path().to_string_lossy().into(), - }; + let lib = probe_lib(1, tmp.path().to_string_lossy().into()); // had_data doesn't matter when the dir has entries. assert!(probe_online(&lib, true).is_online()); assert!(probe_online(&lib, false).is_online()); @@ -447,11 +465,7 @@ mod tests { #[test] fn probe_stale_when_root_missing() { - let lib = Library { - id: 1, - name: "main".into(), - root_path: "/nonexistent/definitely/not/here".into(), - }; + let lib = probe_lib(1, "/nonexistent/definitely/not/here".into()); assert!(matches!( probe_online(&lib, false), LibraryHealth::Stale { .. } @@ -463,11 +477,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let file = tmp.path().join("not-a-dir"); std::fs::write(&file, b"x").unwrap(); - let lib = Library { - id: 1, - name: "main".into(), - root_path: file.to_string_lossy().into(), - }; + let lib = probe_lib(1, file.to_string_lossy().into()); assert!(matches!( probe_online(&lib, false), LibraryHealth::Stale { .. } @@ -478,11 +488,7 @@ mod tests { fn probe_empty_dir_is_online_when_no_prior_data() { // Fresh mount: empty directory, no rows in image_exif. Accept it. let tmp = tempfile::tempdir().unwrap(); - let lib = Library { - id: 1, - name: "main".into(), - root_path: tmp.path().to_string_lossy().into(), - }; + let lib = probe_lib(1, tmp.path().to_string_lossy().into()); assert!(probe_online(&lib, false).is_online()); } @@ -491,11 +497,7 @@ mod tests { // The "share went offline" signal: directory exists but is empty, // and we know the library used to have content. Treat as Stale. let tmp = tempfile::tempdir().unwrap(); - let lib = Library { - id: 1, - name: "main".into(), - root_path: tmp.path().to_string_lossy().into(), - }; + let lib = probe_lib(1, tmp.path().to_string_lossy().into()); match probe_online(&lib, true) { LibraryHealth::Stale { reason, .. } => { assert!(reason.contains("empty"), "unexpected reason: {}", reason) @@ -514,6 +516,7 @@ mod tests { id: 42, name: "test".into(), root_path: tmp.path().to_string_lossy().into(), + enabled: true, }; let map = new_health_map(&[lib.clone()]); diff --git a/src/library_maintenance.rs b/src/library_maintenance.rs index 8fd6059..458ec95 100644 --- a/src/library_maintenance.rs +++ b/src/library_maintenance.rs @@ -444,15 +444,26 @@ pub fn run_orphan_gc( stats } -/// Helper for the watcher: are *all* libraries currently Online? +/// Helper for the watcher: are *all enabled* libraries currently Online? +/// +/// Disabled libraries are out-of-scope for the orphan-GC consensus +/// rule — they don't get probed, don't have a health entry, and a +/// system with one disabled library should still be able to GC +/// orphans for the remaining online libraries. Treating disabled as +/// "blocking" would mean flipping a library to `enabled=false` would +/// permanently halt GC, which is the opposite of the intended kill- +/// switch semantics ("turn this library off and let the rest of the +/// system run normally"). pub fn all_libraries_online(libs: &[Library], health: &LibraryHealthMap) -> bool { let guard = health.read().unwrap_or_else(|e| e.into_inner()); - libs.iter().all(|lib| { - guard - .get(&lib.id) - .map(|h| h.is_online()) - .unwrap_or(false) - }) + libs.iter() + .filter(|lib| lib.enabled) + .all(|lib| { + guard + .get(&lib.id) + .map(|h| h.is_online()) + .unwrap_or(false) + }) } #[derive(QueryableByName, Debug)] @@ -733,11 +744,13 @@ mod tests { id: 1, name: "a".into(), root_path: "/x".into(), + enabled: true, }, Library { id: 2, name: "b".into(), root_path: "/y".into(), + enabled: true, }, ]; let health = new_health_map(&libs); @@ -756,4 +769,45 @@ mod tests { } assert!(!all_libraries_online(&libs, &health)); } + + #[test] + fn all_libraries_online_treats_disabled_as_out_of_scope() { + use crate::libraries::{LibraryHealth, new_health_map}; + // lib 1 enabled+online, lib 2 disabled (would be treated as + // Online in the health map's optimistic seed but the map + // entry is irrelevant — disabled libs are filtered out + // before the health lookup). + let libs = vec![ + Library { + id: 1, + name: "a".into(), + root_path: "/x".into(), + enabled: true, + }, + Library { + id: 2, + name: "b".into(), + root_path: "/y".into(), + enabled: false, + }, + ]; + let health = new_health_map(&libs); + // Sanity: forcibly mark lib 2 stale to prove disabled wins + // over even an explicit Stale entry — the filter skips it + // before the health check happens. + { + let mut g = health.write().unwrap(); + g.insert( + 2, + LibraryHealth::Stale { + reason: "intentionally stale".into(), + since: 0, + }, + ); + } + assert!( + all_libraries_online(&libs, &health), + "disabled library should not block consensus" + ); + } } diff --git a/src/main.rs b/src/main.rs index 57a3761..0d38214 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1976,7 +1976,12 @@ fn watch_files( // appear online for up to WATCH_QUICK_INTERVAL_SECONDS (default // 60s) after boot. Same probe logic as the per-tick gate // below; no ingest runs here, just the health update + log. + // Disabled libraries skip the probe entirely — they should + // never enter the health map (treated as out-of-scope). for lib in &libs { + if !lib.enabled { + continue; + } let context = opentelemetry::Context::new(); let had_data = exif_dao .lock() @@ -1998,6 +2003,19 @@ fn watch_files( let is_full_scan = since_last_full.as_secs() >= full_interval_secs; for lib in &libs { + // Operator kill switch: a disabled library is invisible + // to the watcher entirely. No probe, no ingest, no + // maintenance, no health entry. Distinct from Stale — + // Stale is "we wanted to but couldn't"; Disabled is + // "we don't want to". Toggle via SQL. + if !lib.enabled { + debug!( + "watcher: skipping library '{}' (id={}) — enabled=false", + lib.name, lib.id + ); + continue; + } + // Availability probe: every tick checks that the // library's mount is reachable, is a directory, is // readable, and (if image_exif has rows for it) is diff --git a/src/state.rs b/src/state.rs index bd2668e..9dd1cca 100644 --- a/src/state.rs +++ b/src/state.rs @@ -355,6 +355,7 @@ impl AppState { id: crate::libraries::PRIMARY_LIBRARY_ID, name: "main".to_string(), root_path: base_path_str.clone(), + enabled: true, }; let insight_generator = InsightGenerator::new( ollama.clone(), @@ -391,6 +392,7 @@ impl AppState { id: crate::libraries::PRIMARY_LIBRARY_ID, name: "main".to_string(), root_path: base_path_str.clone(), + enabled: true, }]; AppState::new( Arc::new(StreamActor {}.start()),