From 104444edcbd4cd8020904a48f81e2da1b06c2cb6 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 5 Oct 2026 02:01:57 -0400 Subject: [PATCH] Persistence: a failed save fails the command (change kept in memory, queued, retried) A command whose journal append fails now returns EngineError::NotSaved ("saved in memory but not written to disk: ; LightCraft will retry") instead of OK. The change stays applied and undoable, its ops stay queued and are written by the next save (any command, or the frame loop with a 2 s backoff). Queries and commands that change nothing are unaffected; a failed compaction is reported in lastError, not as a failed command (and a failed synchronous snapshot now backs off like a background one). Session::unsaved(), library.info unsavedOps/unsavedError, ui.inspect unsaved; the desktop UI toasts the failure and shows a warning on the top bar's cloud icon until a save succeeds. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + crates/catalog/src/journal.rs | 10 +- crates/engine/Cargo.toml | 1 + crates/engine/src/cmd/library.rs | 3 + crates/engine/src/lib.rs | 21 +++- crates/engine/src/library.rs | 64 ++++++++++-- crates/engine/src/tests_persist.rs | 147 ++++++++++++++++++++++++++++ crates/ui-egui/src/control.rs | 1 + crates/ui-egui/src/lib.rs | 23 +++++ crates/ui-egui/src/panels/topbar.rs | 27 ++++- crates/ui-egui/src/state.rs | 5 + crates/ui-egui/src/tests_unsaved.rs | 81 +++++++++++++++ docs/control-protocol.md | 12 +++ docs/mcp.md | 4 + docs/parity.md | 2 +- docs/whats-new.md | 3 + 16 files changed, 389 insertions(+), 16 deletions(-) create mode 100644 crates/engine/src/tests_persist.rs create mode 100644 crates/ui-egui/src/tests_unsaved.rs diff --git a/Cargo.lock b/Cargo.lock index a8fef8e0..2955df6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2188,6 +2188,7 @@ dependencies = [ "serde", "serde_json", "thiserror 2.0.21", + "web-time", ] [[package]] diff --git a/crates/catalog/src/journal.rs b/crates/catalog/src/journal.rs index c146b8b4..adc8348d 100644 --- a/crates/catalog/src/journal.rs +++ b/crates/catalog/src/journal.rs @@ -348,7 +348,15 @@ impl Journal { log::warn!("catalog: background snapshot failed ({e}); writing one now"); } let t0 = web_time::Instant::now(); - let mut timing = write_snapshot(self.store.as_mut(), self.seq, catalog).map_err(io)?; + let mut timing = match write_snapshot(self.store.as_mut(), self.seq, catalog) { + Ok(t) => t, + Err(e) => { + // nothing lost (the log is whole); don't retry on every append + self.stats.failed_snapshots += 1; + self.retry_at_records = self.log_records + self.policy.max_records; + return Err(io(e)); + } + }; let t2 = web_time::Instant::now(); // A crash here leaves old records in the log; they are skipped by seq on load. self.store.write_atomic(LOG, b"").map_err(io)?; diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 27083e4f..5c8e3e46 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -17,6 +17,7 @@ lightcraft-scenes = { workspace = true } lightcraft-pipeline = { workspace = true } lightcraft-gpu = { workspace = true } lightcraft-catalog = { workspace = true } +web-time = { workspace = true } lightcraft-preview = { workspace = true } lightcraft-codecs = { workspace = true } lightcraft-raw = { workspace = true } diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 8d1d5497..aafff826 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -867,6 +867,9 @@ pub fn specs() -> Vec { "logRecords": j.log_records(), "logBytes": j.log_bytes(), "lastError": lib.last_error, + // changes applied in memory whose write failed (retried by every save) + "unsavedOps": s.unsaved().map_or(0, |u| u.0), + "unsavedError": s.unsaved().map(|u| u.1), "persistence": j.stats(), "cache": cache, "load": { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index d32dfe27..37a5a363 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -52,6 +52,10 @@ pub enum EngineError { BadParams { cmd: String, msg: String }, #[error("{0}")] Catalog(#[from] lightcraft_catalog::CatalogError), + /// The command's change is applied (in memory, undoable) but its journal records could not + /// be written. They stay queued and are written by the next successful save. + #[error("saved in memory but not written to disk: {0}; LightCraft will retry")] + NotSaved(String), #[error("{0}")] Other(String), } @@ -266,9 +270,18 @@ impl Session { self.auto_write_sidecars(&self.pending_log[log_start..]); } if self.depth == 0 && self.library.is_some() { - // Make the command durable before reporting success (on failure the ops stay pending, - // are retried after the next command, and `library.info` reports the error). - let _ = self.persist(); + // Make the command durable before reporting success. When the command's own records + // can't be appended, it fails with `NotSaved`: the change stays applied in memory and + // queued, and the next save (any later command, or the frame loop) retries it. Other + // persistence trouble (a failed compaction, an older queued change still unwritten by + // a command that changed nothing) doesn't fail the command; `library.info` reports it. + let produced = self.pending_log.len() > log_start; + if let Err(e @ EngineError::NotSaved(_)) = self.persist() + && produced + && r.is_ok() + { + return Err(e); + } } r } @@ -611,6 +624,8 @@ mod tests_merge; #[cfg(test)] mod tests_organize; #[cfg(test)] +mod tests_persist; +#[cfg(test)] mod tests_prefs; #[cfg(test)] mod tests_spots; diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index 24660ea5..54400764 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -14,7 +14,10 @@ //! via [`Session::open_library_in`] (the browser build keeps them in OPFS or IndexedDB). //! //! Every top-level [`Session::execute`] persists the ops it produced (fsynced) before returning, -//! so a crash loses at most the command in flight. The log is compacted into a snapshot when it +//! so a crash loses at most the command in flight. When that write fails the command returns +//! [`EngineError::NotSaved`]: its change stays applied in memory and queued, and every later save +//! retries the queue ([`Session::unsaved`] reports it meanwhile), so nothing is lost once the +//! disk is writable again. The log is compacted into a snapshot when it //! grows (see [`lightcraft_catalog::SnapshotPolicy`]; written by a worker thread on native, see //! [`lightcraft_catalog::journal`]) and on [`Session::close_library`]. @@ -52,6 +55,11 @@ pub struct Library { pub report: LoadReport, /// Last persistence error (shown by the UI; ops stay pending and are retried). pub last_error: Option, + /// Why the last append of queued ops failed; `None` once a save succeeds. While set, changes + /// exist only in memory ([`Session::unsaved`]). + pub unsaved_error: Option, + /// When the frame loop may retry a failed append ([`Session::persist_if_dirty`] backs off). + retry_at: Option, presets_written: String, view_written: Vec, } @@ -155,6 +163,9 @@ fn poll_compaction(lib: &mut Library) { } } +/// How long the frame loop waits before retrying a failed append (commands retry at once). +const RETRY_BACKOFF: std::time::Duration = std::time::Duration::from_secs(2); + fn read_json(store: &mut dyn Store, name: &str) -> Option { store.read(name).ok().flatten().and_then(|b| serde_json::from_slice(&b).ok()) } @@ -248,28 +259,54 @@ impl Session { self.media.attach_disk_cache(&dir.join("thumbs"), self.cache_bytes()); } let view_written = self.view_json(); - let library = self.library.insert(Library { dir, on_disk, journal, files, report, last_error: None, presets_written, view_written }); + let library = self.library.insert(Library { + dir, + on_disk, + journal, + files, + report, + last_error: None, + unsaved_error: None, + retry_at: None, + presets_written, + view_written, + }); Ok(&library.report) } /// Write pending ops to the log (fsynced), compact when due, and save changed presets. - /// Called after every top-level command; cheap when nothing changed. + /// Called after every top-level command; cheap when nothing changed. Fails (with + /// [`EngineError::NotSaved`]) only when the queued ops couldn't be written; they stay queued. pub fn persist(&mut self) -> Result<()> { let Some(lib) = self.library.as_mut() else { return Ok(()) }; if !self.pending_log.is_empty() { if let Err(e) = lib.journal.append(&self.pending_log) { - lib.last_error = Some(e.to_string()); - log::error!("library: {e}"); - return Err(e.into()); + // the ops stay queued (and applied in memory): the next persist retries them + let reason = e.to_string(); + log::error!("library: {} change(s) not written to disk: {reason}", self.pending_log.len()); + lib.last_error = Some(reason.clone()); + lib.unsaved_error = Some(reason.clone()); + lib.retry_at = Some(web_time::Instant::now() + RETRY_BACKOFF); + return Err(EngineError::NotSaved(reason)); } + if lib.unsaved_error.take().is_some() { + log::info!("library: queued changes written to disk"); + } + lib.retry_at = None; self.pending_log.clear(); lib.last_error = None; } poll_compaction(lib); // Never snapshot mid-interaction: the catalog then holds an uncommitted preview value. // The snapshot is written by a worker thread (natively); appends go on meanwhile. - if lib.journal.wants_snapshot() && self.interaction.is_none() { - lib.journal.snapshot_in_background(&self.catalog)?; + // A failed compaction loses nothing (the log is kept whole and compacted later): it is + // reported (`library.info` → `lastError`), not returned. + if lib.journal.wants_snapshot() + && self.interaction.is_none() + && let Err(e) = lib.journal.snapshot_in_background(&self.catalog) + { + log::error!("library: compaction: {e}"); + lib.last_error = Some(format!("compaction: {e}")); } let presets = presets_json(self); let Some(lib) = self.library.as_mut() else { return Ok(()) }; @@ -286,13 +323,22 @@ impl Session { /// Persist if commands left ops pending (cheap; frontends call it once per frame for state /// changed outside [`Session::execute`]). pub fn persist_if_dirty(&mut self) { - if self.library.is_some() && !self.pending_log.is_empty() { + let backing_off = self.library.as_ref().and_then(|l| l.retry_at).is_some_and(|t| web_time::Instant::now() < t); + if self.library.is_some() && !self.pending_log.is_empty() && !backing_off { let _ = self.persist(); } else if let Some(lib) = self.library.as_mut() { poll_compaction(lib); } } + /// Changes applied in memory but not yet written to disk because saving failed: the number of + /// queued ops and why (`None` when everything is saved, or no library is open). Retried by + /// every command and by [`Session::persist_if_dirty`]. + pub fn unsaved(&self) -> Option<(usize, &str)> { + let lib = self.library.as_ref()?; + lib.unsaved_error.as_deref().filter(|_| !self.pending_log.is_empty()).map(|e| (self.pending_log.len(), e)) + } + /// Flush everything and write a snapshot (on quit). Ends an open interaction first. pub fn close_library(&mut self) -> Result<()> { if self.library.is_none() { diff --git a/crates/engine/src/tests_persist.rs b/crates/engine/src/tests_persist.rs new file mode 100644 index 00000000..90f76cad --- /dev/null +++ b/crates/engine/src/tests_persist.rs @@ -0,0 +1,147 @@ +//! A failed save is an error, not a silent success: a command whose journal append fails returns +//! [`EngineError::NotSaved`], its change stays applied and queued, and the next save writes it. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; + +use lightcraft_catalog::{Flag, MemStore, SnapshotPolicy, Store}; +use serde_json::json; + +use crate::library::LibraryStores; +use crate::{EngineError, Session}; + +/// A store whose writes fail while `fail` is set (a full or unplugged disk). +#[derive(Clone, Default)] +struct Flaky { + files: MemStore, + fail: Arc, + /// Only snapshots fail (appends work). + snapshots_only: bool, +} + +impl Flaky { + fn failing(&self) -> bool { + self.fail.load(Ordering::SeqCst) + } + fn set_failing(&self, on: bool) { + self.fail.store(on, Ordering::SeqCst); + } + /// The files as a crash at this moment would leave them. + fn crash_copy(&self) -> Flaky { + let m = MemStore::new(); + *m.files.lock().unwrap() = self.files.files.lock().unwrap().clone(); + Flaky { files: m, ..Default::default() } + } +} + +impl Store for Flaky { + fn read(&mut self, name: &str) -> std::io::Result>> { + self.files.read(name) + } + fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + if self.failing() { + return Err(std::io::Error::other("disk full")); + } + self.files.write_atomic(name, data) + } + fn append(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + if self.failing() && !self.snapshots_only { + return Err(std::io::Error::other("disk full")); + } + self.files.append(name, data) + } + fn truncate(&mut self, name: &str, len: u64) -> std::io::Result<()> { + self.files.truncate(name, len) + } + fn describe(&self) -> String { + "flaky".into() + } +} + +fn open(store: &Flaky, seed: bool) -> Session { + let mut s = Session::new(); + let stores = LibraryStores { dir: "flaky".into(), catalog: Box::new(store.clone()), files: Box::new(MemStore::new()), on_disk: false }; + s.open_library_in(stores, seed).unwrap(); + s +} + +#[test] +fn failed_append_fails_the_command_and_is_retried() { + let store = Flaky::default(); + let mut s = open(&store, true); + let id = s.selection.active.unwrap(); + s.execute("photo.rate", &json!({"rating": 1})).unwrap(); + store.set_failing(true); + + let e = s.execute("photo.rate", &json!({"rating": 4})).unwrap_err(); + assert!(matches!(e, EngineError::NotSaved(_)), "{e:?}"); + let msg = e.to_string(); + assert!( + msg.starts_with("saved in memory but not written to disk: ") && msg.contains("disk full") && msg.ends_with("LightCraft will retry"), + "{msg}" + ); + // applied in memory, undoable, queued + assert_eq!(s.catalog.photo(id).unwrap().rating, 4); + assert!(!s.undo.is_empty()); + assert_eq!(s.unsaved().map(|u| u.0), Some(1)); + // queries and commands that change nothing are unaffected (they retry the queue, but report + // their own result) + let info = s.execute("library.info", &json!({})).unwrap(); + assert_eq!(info["unsavedOps"], 1, "{info}"); + assert!(info["unsavedError"].as_str().unwrap().contains("disk full"), "{info}"); + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + // a second change fails too; both stay queued, in order + let e = s.execute("photo.flag", &json!({"flag": "pick"})).unwrap_err(); + assert!(matches!(e, EngineError::NotSaved(_))); + assert_eq!(s.unsaved().map(|u| u.0), Some(2)); + // the frame loop backs off instead of retrying every frame + s.persist_if_dirty(); + assert_eq!(s.unsaved().map(|u| u.0), Some(2)); + // a crash now loses only the unsaved changes; the earlier ones are on disk + assert_eq!(open(&store.crash_copy(), true).catalog.photo(id).unwrap().rating, 1); + + // the disk comes back: the next save writes the whole queue + store.set_failing(false); + s.execute("library.info", &json!({})).unwrap(); + assert_eq!(s.unsaved(), None); + assert!(s.library.as_ref().unwrap().last_error.is_none()); + let expect = s.catalog.to_snapshot(); + drop(s); // no close: like a crash + let s2 = open(&store, true); + assert_eq!(s2.catalog.to_snapshot(), expect); + let p = s2.catalog.photo(id).unwrap(); + assert_eq!((p.rating, p.flag), (4, Flag::Pick)); +} + +/// Undo is a command like any other: a failed write of its op is reported, and retried. +#[test] +fn failed_append_of_undo_is_reported_and_retried() { + let store = Flaky::default(); + let mut s = open(&store, true); + let id = s.selection.active.unwrap(); + let before = s.catalog.photo(id).unwrap().rating; + s.execute("photo.rate", &json!({"rating": 3})).unwrap(); + store.set_failing(true); + assert!(matches!(s.execute("edit.undo", &json!({})), Err(EngineError::NotSaved(_)))); + assert_eq!(s.catalog.photo(id).unwrap().rating, before); + store.set_failing(false); + s.persist().unwrap(); + drop(s); + assert_eq!(open(&store, true).catalog.photo(id).unwrap().rating, before); +} + +/// A failed compaction is not a failed command: the command's own records were appended. +#[test] +fn failed_snapshot_does_not_fail_the_command() { + let store = Flaky { snapshots_only: true, ..Default::default() }; + let mut s = open(&store, false); + s.library.as_mut().unwrap().journal_mut().policy = SnapshotPolicy { max_records: 1, max_bytes: u64::MAX }; + store.set_failing(true); + s.execute("album.create", &json!({"name": "A"})).unwrap(); + s.execute("album.create", &json!({"name": "B"})).unwrap(); + assert_eq!(s.unsaved(), None); + assert!(s.library.as_ref().unwrap().last_error.is_some(), "the compaction failure is reported"); + drop(s); + store.set_failing(false); + assert_eq!(open(&store, false).catalog.albums().count(), 2); +} diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index ca90d323..02e14aa8 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -82,6 +82,7 @@ pub fn inspect(app: &LightcraftApp, ctx: &egui::Context) -> Value { "loupe": app.loupe_shown.map(|(p, src)| json!({"photo": p.0, "source": src, "pending": app.renderer.is_pending(crate::render::Slot::Main)})), "hoverPreview": app.hover_preview.as_ref().map(|h| h.label.clone()), "status": app.ui.status, + "unsaved": app.session.unsaved().map(|(n, e)| json!({"ops": n, "error": e})), "scan": app.scan.as_ref().map(crate::import::ScanTask::status), "export": {"running": app.export.as_ref().map(crate::export_task::ExportTask::status), "last": app.last_export_result}, "memory": memory(app), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 13c15a6e..8624f0a0 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -33,6 +33,8 @@ mod tests_masking; mod tests_panels; #[cfg(test)] mod tests_scroll; +#[cfg(test)] +mod tests_unsaved; use std::sync::mpsc::{Receiver, Sender}; @@ -281,6 +283,26 @@ impl LightcraftApp { } } + /// Announce when saving the library starts failing (changes then live only in memory and are + /// retried) and when it works again; the top bar's cloud icon shows the state meanwhile. + fn save_status(&mut self, ctx: &egui::Context) { + let unsaved = self.session.unsaved().map(|(n, e)| (n, e.to_string())); + match (unsaved, self.ui.unsaved_seen) { + (Some((n, e)), false) => { + self.ui.unsaved_seen = true; + let t = ctx.input(|i| i.time); + let what = if n == 1 { "1 change".to_string() } else { format!("{n} changes") }; + self.ui.toast = Some((format!("{what} saved in memory but not written to disk: {e} — LightCraft will retry"), t + 6.0)); + } + (None, true) => { + self.ui.unsaved_seen = false; + self.toast(ctx, "Library saved"); + } + (Some(_), true) => ctx.request_repaint_after(std::time::Duration::from_secs(2)), // retry + (None, false) => {} + } + } + pub fn toast(&mut self, ctx: &egui::Context, text: impl Into) { let t = ctx.input(|i| i.time); self.ui.toast = Some((text.into(), t + 1.4)); @@ -448,6 +470,7 @@ impl LightcraftApp { import::poll_scan(self, ctx); import::tick(self, ctx); self.preview_build_status(ctx); + self.save_status(ctx); self.slideshow_tick(ctx); // back from an external editor: pick up the files it saved let focused = ctx.input(|i| i.focused); diff --git a/crates/ui-egui/src/panels/topbar.rs b/crates/ui-egui/src/panels/topbar.rs index dc0b0638..8142b9ec 100644 --- a/crates/ui-egui/src/panels/topbar.rs +++ b/crates/ui-egui/src/panels/topbar.rs @@ -111,9 +111,18 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } // right icons let mut x = full.right() - 18.0; + // saving is failing: the cloud icon turns into a warning until a save succeeds + let unsaved = app.session.unsaved().map(|(n, e)| { + format!( + "{n} change{} saved in memory but not written to disk: {e}\nLightCraft retries automatically; quitting now would lose {}.", + if n == 1 { "" } else { "s" }, + if n == 1 { "it" } else { "them" } + ) + }); + let cloud_tip = unsaved.as_deref().unwrap_or("Local library — no cloud account needed"); for (id, icon, tip, cmd) in [ ("discord", Icon::Chat, "Join the ArtCraft community on Discord", "app.discord"), - ("cloud", Icon::Cloud, "Local library — no cloud account needed", ""), + ("cloud", Icon::Cloud, cloud_tip, ""), ("help", Icon::Help, "Keyboard shortcuts", "app.shortcuts"), ("share", Icon::Share, "Export", "dialog.export"), ("bell", Icon::Bell, "Activity", "panel.activity"), @@ -121,7 +130,21 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let r = Rect::from_center_size(pos2(x, full.center().y), vec2(28.0, 28.0)); let resp = ui.interact(r, egui::Id::new(("top", id)), Sense::click()).on_hover_text(tip); register(ui.ctx(), format!("icon:{id}"), r); - paint(ui.painter(), r.shrink(5.0), icon, if resp.hovered() { t.text } else { t.icon }); + let warn = id == "cloud" && unsaved.is_some(); + let colour = if warn { + t.caution + } else if resp.hovered() { + t.text + } else { + t.icon + }; + paint(ui.painter(), r.shrink(5.0), icon, colour); + if warn { + let c = r.right_top() + vec2(-5.0, 6.0); + ui.painter().circle_filled(c, 6.0, t.reject); + ui.painter().text(c, Align2::CENTER_CENTER, "!", t.semibold(9.5), t.canvas); + register(ui.ctx(), "indicator:unsaved", r); + } if resp.clicked() && !cmd.is_empty() { let _ = app.run(cmd, json!({})); } diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index ad759a06..59c8087e 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -203,6 +203,10 @@ pub struct UiState { /// The Build Previews run last announced (its identity, finished?). #[serde(skip)] pub preview_build_seen: Option<(usize, bool)>, + /// Saving the library is failing (announced with a toast; the top bar shows a warning until + /// a later save succeeds). + #[serde(skip)] + pub unsaved_seen: bool, pub view: ViewMode, pub left_panel: bool, pub right: RightPanel, @@ -505,6 +509,7 @@ impl Default for UiState { fn default() -> Self { UiState { preview_build_seen: None, + unsaved_seen: false, luminance_map_restore: None, view: ViewMode::Detail, left_panel: false, diff --git a/crates/ui-egui/src/tests_unsaved.rs b/crates/ui-egui/src/tests_unsaved.rs new file mode 100644 index 00000000..68638e1d --- /dev/null +++ b/crates/ui-egui/src/tests_unsaved.rs @@ -0,0 +1,81 @@ +//! A failed save is visible: the command reports the error, a toast says the change is only in +//! memory, and the top bar shows a warning until a later save succeeds. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::Duration; + +use lightcraft_catalog::{MemStore, Store}; +use lightcraft_engine::library::LibraryStores; +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +/// Appends fail while `fail` is set. +#[derive(Clone, Default)] +struct Flaky { + files: MemStore, + fail: Arc, +} + +impl Store for Flaky { + fn read(&mut self, name: &str) -> std::io::Result>> { + self.files.read(name) + } + fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.files.write_atomic(name, data) + } + fn append(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + if self.fail.load(Ordering::SeqCst) { + return Err(std::io::Error::other("disk full")); + } + self.files.append(name, data) + } + fn truncate(&mut self, name: &str, len: u64) -> std::io::Result<()> { + self.files.truncate(name, len) + } + fn describe(&self) -> String { + "flaky".into() + } +} + +#[test] +fn failed_save_is_reported_and_shown_until_a_save_succeeds() { + let store = Flaky::default(); + let mut s = lightcraft_engine::Session::new(); + let stores = LibraryStores { dir: "flaky".into(), catalog: Box::new(store.clone()), files: Box::new(MemStore::new()), on_disk: false }; + s.open_library_in(stores, false).unwrap(); + let app = LightcraftApp::new(s, Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + h.step(); + let has_indicator = |h: &Headless| h.app.widgets.iter().any(|(w, _)| w == "indicator:unsaved"); + assert!(!has_indicator(&h)); + + store.fail.store(true, Ordering::SeqCst); + let r = h.request("engine.execute", json!({"command": "album.create", "params": {"name": "Kept"}}), T); + assert_eq!(r["ok"], false, "{r}"); + assert!( + r["error"].as_str().unwrap().contains("saved in memory but not written to disk: ") && r["error"].as_str().unwrap().contains("disk full"), + "{r}" + ); + assert_eq!(h.app.session.catalog.albums().count(), 1, "the change is applied"); + h.step(); + h.step(); + let toast = h.app.ui.toast.clone().map(|t| t.0).unwrap_or_default(); + assert!(toast.contains("not written to disk"), "{toast}"); + assert!(has_indicator(&h)); + let inspect = h.request("ui.inspect", json!({}), T); + assert_eq!(inspect["result"]["unsaved"]["ops"], 1, "{inspect}"); + + // the disk comes back: the next command writes the queue, the warning goes away + store.fail.store(false, Ordering::SeqCst); + let r = h.request("engine.execute", json!({"command": "library.info"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); + assert!(!has_indicator(&h)); + assert_eq!(h.app.ui.toast.clone().map(|t| t.0).as_deref(), Some("Library saved")); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index f9e32f2a..9623a757 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -37,6 +37,18 @@ mode ([mcp.md](mcp.md)) is a thin layer over this channel. Implementation: | `ui.render` | `{id?, size?, path?}` | Render a photo (PNG to `path`), `{width, height}` | | `app.quit` | — | Close the app | +### When the library can't be saved + +With a persistent library, every command that changes something is written to the catalog journal (fsynced) before +it replies. If that write fails (disk full, volume gone, permissions), the command replies `ok: false` with +`"saved in memory but not written to disk: ; LightCraft will retry"`. The change itself **is** applied (and +undoable) and stays queued: the next command, and the app's frame loop every couple of seconds, retry the write, so +nothing is lost once the disk is writable again — unless the app quits first. Meanwhile `ui.inspect` → `unsaved` +is `{ops, error}` (else `null`), `library.info` reports `unsavedOps` / `unsavedError`, and the top bar's cloud icon +shows a warning (widget `indicator:unsaved`). Queries and commands that change nothing still succeed. A failed +compaction (snapshot) is not a failed command — the log is kept whole — and only shows in `library.info` → +`lastError`. + ## Headless rendering (no window, no GPU) The egui UI can be rasterized on the CPU (`crates/ui-egui/src/softpaint.rs`, driven by diff --git a/docs/mcp.md b/docs/mcp.md index 9a239c46..9b4c65f1 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -141,6 +141,10 @@ results with `isError: true` so the model can read and correct them; malformed J standard error codes (-32700 parse, -32600 invalid request, -32601 method not found, -32602 invalid params, -32002 resource not found). +With `--library`, a command whose change can't be written to disk (disk full, …) is an error too: +`saved in memory but not written to disk: ; LightCraft will retry` — the change is applied in the session and +written by the next successful save (see [control-protocol.md](control-protocol.md#when-the-library-cant-be-saved)). + ## One-shot commands: `lightcraft-cli run` For agents that prefer a shell over an MCP session: run any chain of commands in one process and read one JSON diff --git a/docs/parity.md b/docs/parity.md index 16f5fa09..fecb3ce6 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -472,7 +472,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | | +| LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | every change fsynced before the command returns; a failed write fails the command ("saved in memory but not written to disk"), keeps the change queued and retries; top-bar warning until saved | | LR-BEHAV-UNDO | Global undo | P0 | ✅ | `cmd:edit.undo`, `crates/engine/src/tests.rs` (`rating_flag_undo_redo`) | covers ratings, albums, deletes, edits | | LR-BEHAV-MULTISELECT | Multi-selection | P0 | ✅ | `cmd:library.select` (`replace`/`add`/`toggle`/`range`), `cmd:library.selectAll` | | | LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | | diff --git a/docs/whats-new.md b/docs/whats-new.md index cfdd963f..19b28629 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -21,6 +21,9 @@ - Export file names use the Rename Photos tokens ({title}, {seq:2}, {date:%Y-%m-%d}…), plus new {num}, {folder}, {lens}, {iso}, {rating}, {creator}. - Copyright status, rights usage terms and copyright info URL in Info, metadata presets and exports. - Auto-Tag from Tracklog: GPS locations for your photos from a GPX track log, matched by capture time. +- A change that can't be saved to disk (full or unplugged drive) is no longer silent: the command reports + "saved in memory but not written to disk", the top bar shows a warning, and LightCraft keeps retrying until the + save goes through. ### Editing - Auto Sync: edits apply to every selected photo. Auto B&W mix. Automatic versions.