From 765915e126053523efd8d6b92623682135c41e36 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:07:39 -0400 Subject: [PATCH 1/8] Workspaces: deleting the current workspace saves after switching to Default (#696) --- crates/ui-egui/src/menus.rs | 3 ++- crates/ui-egui/tests/ui_workspace_focus.rs | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index b10b0e31..e19e0c6d 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1054,10 +1054,11 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu if app.ui.saved_workspaces.remove(&name).is_none() && !builtin { return Err(format!("no workspace `{name}`")); } - app.store_saved_workspaces(); + // Leave the deleted workspace first: a current workspace needs a definition. if app.ui.workspace == name { app.set_workspace("Default"); } + app.store_saved_workspaces(); return Ok(json!({"deleted": name})); } if let Some(new) = p.get("rename").and_then(Value::as_str).map(str::trim).filter(|n| !n.is_empty() && *n != name) { diff --git a/crates/ui-egui/tests/ui_workspace_focus.rs b/crates/ui-egui/tests/ui_workspace_focus.rs index f9dd3c47..fe1ab2d7 100644 --- a/crates/ui-egui/tests/ui_workspace_focus.rs +++ b/crates/ui-egui/tests/ui_workspace_focus.rs @@ -330,3 +330,18 @@ fn unowned_tab_keeps_the_timeline_mini_flowchart_shortcut() { output.textures_delta.clear(); assert!(app.ui.mini_flowchart.is_some(), "an unowned Tab still invokes the application binding"); } + +/// Deleting the current workspace writes the file straight away (it used to be rejected for +/// having no definition until the next frame retried). +#[test] +fn deleting_the_current_workspace_saves_immediately() { + let store = Arc::new(MemoryConfig::default()); + let mut app = EffectcraftApp::new(Session { config: Some(store.clone()), ..Default::default() }); + let ctx = egui::Context::default(); + effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name": "Gone"})).unwrap(); + effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.editWorkspaces", json!({"name": "Gone", "delete": true})).unwrap(); + assert_eq!(app.ui.workspace, "Default"); + assert!(effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].is_null()); + let text = store.read("workspaces.json").unwrap(); + assert!(!text.contains("Gone"), "the deleted workspace must be gone from the file: {text}"); +} From 73ea427015333ed5ba21afae0e2dc370619dea19 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:31:16 -0400 Subject: [PATCH 2/8] Commands: a frontend-only command's own failure is its result (#696) --- crates/ui-egui/src/menus.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index e19e0c6d..ae0834ca 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -752,7 +752,7 @@ fn clipboard_note(s: &effectcraft_engine::Session) -> String { } pub(crate) fn run_engine(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { - let r = app.session.execute(id, params).map_err(|e| e.to_string()); + let mut r = app.session.execute(id, params).map_err(|e| e.to_string()); if let Err(e) = &r { app.ui.status = e.clone(); } else if COPY_COMMANDS.contains(&id) && app.session.state.text_edit.is_none() { @@ -763,7 +763,12 @@ pub(crate) fn run_engine(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str match ev { effectcraft_engine::Event::Frontend { command, params } => { if let Err(e) = frontend(app, ctx, &command, params) { - app.ui.status = e; + app.ui.status = e.clone(); + // A frontend-only command's own failure is the command's result (#696: a + // workspace save that didn't reach disk reported success). + if command == id && r.is_ok() { + r = Err(e); + } } } other => app.session.events.push(other), From b9a9a0a4148f16add9783e6d536e6b301591486b Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:32:18 -0400 Subject: [PATCH 3/8] Workspaces: Save, Save As, Rename and Delete fail when the file isn't written (#696) --- crates/ui-egui/src/lib.rs | 13 +++++++-- crates/ui-egui/src/menus.rs | 8 ++--- crates/ui-egui/src/native_menu.rs | 1 + crates/ui-egui/src/workspace_state.rs | 34 ++++++++++++++++++++++ crates/ui-egui/tests/ui_panels.rs | 2 ++ crates/ui-egui/tests/ui_workspace_focus.rs | 6 +++- 6 files changed, 56 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 746d917b..df886759 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -428,9 +428,16 @@ impl EffectcraftApp { } /// Keep the saved workspaces for the next session (see [`EffectcraftApp::new`]): After - /// Effects' saved workspaces last until they are deleted (#320). - pub(crate) fn store_saved_workspaces(&mut self) { - self.save_workspace_state(); + /// Effects' saved workspaces last until they are deleted (#320). `Err` when they weren't + /// written (#696): the layouts stay usable and a later save retries, but a restart loses them. + pub(crate) fn store_saved_workspaces(&mut self) -> Result<(), String> { + let Err(error) = self.workspace_store.save_now(self.session.config.as_deref(), &self.ui) else { return Ok(()) }; + let message = format!("Workspaces are kept for this session only and won't be there after a restart: {error}"); + log::warn!("{message}"); + self.ui.status = message.clone(); + // The raw error, so the background retry doesn't report the same failure again. + self.workspace_store.error = Some(error); + Err(message) } /// Offer the frontend's commands (tools, timeline navigation…) for keyboard shortcuts. diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index ae0834ca..4b8d14a4 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1017,7 +1017,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu let name = app.ui.workspace.clone(); app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone()); app.ui.saved_floating.insert(name.clone(), app.ui.floating.clone()); - app.store_saved_workspaces(); + app.store_saved_workspaces()?; json!({"workspace": name}) } "window.saveWorkspaceAs" => { @@ -1034,7 +1034,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone()); app.ui.saved_floating.insert(name.clone(), app.ui.floating.clone()); app.ui.workspace = name.clone(); - app.store_saved_workspaces(); + app.store_saved_workspaces()?; json!({"workspace": name}) } "window.editWorkspaces" => { @@ -1063,7 +1063,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu if app.ui.workspace == name { app.set_workspace("Default"); } - app.store_saved_workspaces(); + app.store_saved_workspaces()?; return Ok(json!({"deleted": name})); } if let Some(new) = p.get("rename").and_then(Value::as_str).map(str::trim).filter(|n| !n.is_empty() && *n != name) { @@ -1078,7 +1078,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu if app.ui.workspace == name { app.ui.workspace = new.to_string(); } - app.store_saved_workspaces(); + app.store_saved_workspaces()?; return Ok(json!({"renamed": new})); } json!({"workspaces": app.workspace_names()}) diff --git a/crates/ui-egui/src/native_menu.rs b/crates/ui-egui/src/native_menu.rs index 5d0a3c53..c3266acf 100644 --- a/crates/ui-egui/src/native_menu.rs +++ b/crates/ui-egui/src/native_menu.rs @@ -429,6 +429,7 @@ mod tests { #[test] fn saved_workspaces_are_in_the_native_workspace_menu() { let mut app = app(); + app.session.config = Some(std::sync::Arc::new(effectcraft_engine::config::MemoryConfig::default())); let ctx = egui::Context::default(); crate::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap(); let entry = |app: &EffectcraftApp| build(app).items().into_iter().find(|i| i.label == "My Layout").cloned(); diff --git a/crates/ui-egui/src/workspace_state.rs b/crates/ui-egui/src/workspace_state.rs index b7233a80..c2ac9e37 100644 --- a/crates/ui-egui/src/workspace_state.rs +++ b/crates/ui-egui/src/workspace_state.rs @@ -263,6 +263,19 @@ impl WorkspaceStore { self.error = None; Ok(true) } + + /// An explicit save (Save Workspace, Save as New Workspace, Edit Workspaces): unlike + /// [`Self::save`], no settings folder or a file blocked at load is an error, not "nothing to do". + pub(crate) fn save_now(&mut self, config: Option<&dyn ConfigStore>, ui: &UiState) -> Result<(), String> { + let Some(config) = config else { + return Err("there is no settings folder to save them in".into()); + }; + if !self.writable { + let path = config.dir().map_or_else(|| FILE.to_string(), |dir| dir.join(FILE).display().to_string()); + return Err(format!("{path} couldn't be loaded at startup, so it is left untouched; fix or move it and restart")); + } + self.save(Some(config), ui).map(|_| ()) + } } #[cfg(test)] @@ -283,10 +296,31 @@ mod tests { ui.focused = PanelKind::Project; ui.dock = crate::dock::workspace("Minimal"); assert!(!store.save(Some(&config), &ui).unwrap()); + assert!(store.save_now(Some(&config), &ui).unwrap_err().contains("couldn't be loaded")); assert_eq!(config.read(FILE).as_deref(), Some(text.as_str())); } } + #[test] + fn explicit_save_without_a_settings_folder_is_an_error() { + let mut ui = UiState::default(); + let mut store = WorkspaceStore::load(None, &mut ui); + ui.focused = PanelKind::Project; + assert!(!store.save(None, &ui).unwrap()); + assert!(store.save_now(None, &ui).is_err()); + } + + #[test] + fn explicit_save_with_nothing_new_succeeds() { + let config = MemoryConfig::default(); + let mut ui = UiState::default(); + let mut store = WorkspaceStore::load(Some(&config), &mut ui); + ui.focused = PanelKind::Project; + store.save_now(Some(&config), &ui).unwrap(); + store.save_now(Some(&config), &ui).unwrap(); + assert!(config.read(FILE).is_some()); + } + #[test] fn invalid_tabs_split_and_deep_tree_are_rejected() { let mut snapshot = Snapshot::capture(&UiState::default()); diff --git a/crates/ui-egui/tests/ui_panels.rs b/crates/ui-egui/tests/ui_panels.rs index e3fee868..38df1ecb 100644 --- a/crates/ui-egui/tests/ui_panels.rs +++ b/crates/ui-egui/tests/ui_panels.rs @@ -172,6 +172,8 @@ fn saved_workspaces_persist_across_sessions() { #[test] fn saved_workspace_is_listed_in_the_workspace_menu() { let mut h = harness(); + // Saving needs a settings folder. + h.state_mut().session.config = Some(Arc::new(MemoryConfig::default())); let ctx = h.ctx.clone(); effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.workspace", json!({"name": "Minimal"})).unwrap(); effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap(); diff --git a/crates/ui-egui/tests/ui_workspace_focus.rs b/crates/ui-egui/tests/ui_workspace_focus.rs index fe1ab2d7..ecd3a16a 100644 --- a/crates/ui-egui/tests/ui_workspace_focus.rs +++ b/crates/ui-egui/tests/ui_workspace_focus.rs @@ -85,7 +85,11 @@ fn unreadable_native_workspace_is_preserved_after_later_layout_changes() { assert!(effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].is_string()); app.set_workspace("Minimal"); app.ui.focused = PanelKind::Project; - effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name":"Must not overwrite"})).unwrap(); + let error = effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name":"Must not overwrite"})).unwrap_err(); + assert!(error.contains("won't be there after a restart") && error.contains("workspaces.json"), "{error}"); + assert_eq!(app.ui.status, error, "the failure is in the status bar"); + assert_eq!(app.ui.workspace, "Must not overwrite", "the layout stays usable for this session"); + assert!(app.ui.saved_workspaces.contains_key("Must not overwrite")); eframe::App::on_exit(&mut app); assert_eq!(std::fs::read(&path).unwrap(), bytes); assert!(effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].is_string()); From 41b04bf414efd0a97603aaf2f350b8d175624d34 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:32:22 -0400 Subject: [PATCH 4/8] Workspaces: tests for a failed, a storage-less and an on-disk workspace save (#696) --- crates/ui-egui/tests/ui_workspace_focus.rs | 89 ++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/crates/ui-egui/tests/ui_workspace_focus.rs b/crates/ui-egui/tests/ui_workspace_focus.rs index ecd3a16a..8464c1fb 100644 --- a/crates/ui-egui/tests/ui_workspace_focus.rs +++ b/crates/ui-egui/tests/ui_workspace_focus.rs @@ -349,3 +349,92 @@ fn deleting_the_current_workspace_saves_immediately() { let text = store.read("workspaces.json").unwrap(); assert!(!text.contains("Gone"), "the deleted workspace must be gone from the file: {text}"); } + +/// A settings store whose writes fail while `full` is set (a full or read-only disk). +#[derive(Default)] +struct FullDisk { + inner: MemoryConfig, + full: std::sync::atomic::AtomicBool, +} + +impl ConfigStore for FullDisk { + fn read(&self, name: &str) -> Option { + self.inner.read(name) + } + fn write(&self, name: &str, data: &str) -> std::io::Result<()> { + if self.full.load(std::sync::atomic::Ordering::Relaxed) { + return Err(std::io::Error::other("synthetic full disk")); + } + self.inner.write(name, data) + } + fn remove(&self, name: &str) -> std::io::Result<()> { + self.inner.remove(name) + } +} + +/// Issue #696: Save as New Workspace said nothing when the file couldn't be written, so the +/// workspace was gone after a restart. The command fails, the layout stays usable, and saving +/// again once the disk is writable keeps it. +#[test] +fn failed_workspace_save_is_reported_and_retryable() { + let store = Arc::new(FullDisk::default()); + store.full.store(true, std::sync::atomic::Ordering::Relaxed); + let mut app = EffectcraftApp::new(Session { config: Some(store.clone()), ..Default::default() }); + let ctx = egui::Context::default(); + let invoke = |app: &mut EffectcraftApp, id: &str, params| effectcraft_ui_egui::menus::invoke(app, &ctx, id, params); + let error = invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "caption craft"})).unwrap_err(); + assert!(error.contains("synthetic full disk") && error.contains("won't be there after a restart"), "{error}"); + assert_eq!(app.ui.status, error); + let storage_error = effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].clone(); + assert!(storage_error.as_str().is_some_and(|e| e.contains("synthetic full disk")), "{storage_error}"); + assert_eq!(app.ui.workspace, "caption craft"); + assert_eq!(app.saved_workspace_names(), ["caption craft"]); + assert!(store.read("workspaces.json").is_none()); + // Rename and Delete report the same way. + assert!(invoke(&mut app, "window.editWorkspaces", json!({"name": "caption craft", "rename": "Captions"})).is_err()); + assert_eq!(app.saved_workspace_names(), ["Captions"]); + invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "Scratch"})).unwrap_err(); + assert!(invoke(&mut app, "window.editWorkspaces", json!({"name": "Scratch", "delete": true})).is_err()); + assert_eq!(app.saved_workspace_names(), ["Captions"]); + // Writable again: an explicit Save keeps it. + store.full.store(false, std::sync::atomic::Ordering::Relaxed); + invoke(&mut app, "window.workspace", json!({"name": "Captions"})).unwrap(); + invoke(&mut app, "window.saveWorkspace", json!({})).unwrap(); + assert!(effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].is_null()); + let restarted = EffectcraftApp::new(Session { config: Some(store), ..Default::default() }); + assert_eq!(restarted.saved_workspace_names(), ["Captions"]); +} + +/// Without a settings folder there is nowhere to keep a workspace: saving says so. +#[test] +fn workspace_save_without_a_settings_folder_is_reported() { + let mut app = EffectcraftApp::new(Session::default()); + let ctx = egui::Context::default(); + let error = effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name": "Mine"})).unwrap_err(); + assert!(error.contains("no settings folder"), "{error}"); + assert_eq!(app.saved_workspace_names(), ["Mine"]); +} + +/// Save as New Workspace, Rename and Delete reach the real settings folder straight away (no +/// exit needed) and come back from it. +#[test] +fn saved_workspaces_reach_the_settings_folder_and_reload() { + let dir = scratch_dir("disk"); + let launch = || EffectcraftApp::new(Session { config: Some(Arc::new(DirConfig::new(&dir))), ..Default::default() }); + let mut app = launch(); + let ctx = egui::Context::default(); + let invoke = |app: &mut EffectcraftApp, id: &str, params| effectcraft_ui_egui::menus::invoke(app, &ctx, id, params).unwrap(); + invoke(&mut app, "window.workspace", json!({"name": "Minimal"})); + invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "caption craft"})); + let saved = app.ui.dock.clone(); + assert!(std::fs::read_to_string(dir.join("workspaces.json")).unwrap().contains("caption craft")); + invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "Scratch"})); + invoke(&mut app, "window.editWorkspaces", json!({"name": "caption craft", "rename": "Captions"})); + invoke(&mut app, "window.editWorkspaces", json!({"name": "Scratch", "delete": true})); + drop(app); + let mut app = launch(); + assert_eq!(app.saved_workspace_names(), ["Captions"]); + invoke(&mut app, "window.workspace", json!({"name": "Captions"})); + assert_eq!(app.ui.dock, saved); + std::fs::remove_dir_all(dir).unwrap(); +} From dbaf559b581ebf93aa478a0c93486fbab9cdf52b Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:32:24 -0400 Subject: [PATCH 5/8] Docs: workspace commands report a save that didn't reach disk (#696) --- docs/control-protocol.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/control-protocol.md b/docs/control-protocol.md index f5487981..0a425eba 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -81,6 +81,10 @@ errors. Unreadable, invalid or newer files leave the default workspace available preserved for the entire session, including after later layout changes. Load and save errors appear in the status message and the nullable `workspaceStorageError` field; a failed write remains eligible for retry. +`window.saveWorkspace`, `window.saveWorkspaceAs` and `window.editWorkspaces` (rename, delete) +write the file immediately and return an error when it isn't written (a failed write, a file +preserved after a failed load, or no configuration store). The changed workspaces still apply +for the session, and a later save retries them. ## Input From ecf485e3759aa1ecfd4fc2748110c5f264fc548a Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 21:35:48 -0400 Subject: [PATCH 6/8] Docs: engine.execute reports a failed workspace save through workspaceStorageError (#696) --- docs/control-protocol.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 0a425eba..89d319f7 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -82,9 +82,11 @@ preserved for the entire session, including after later layout changes. Load and appear in the status message and the nullable `workspaceStorageError` field; a failed write remains eligible for retry. `window.saveWorkspace`, `window.saveWorkspaceAs` and `window.editWorkspaces` (rename, delete) -write the file immediately and return an error when it isn't written (a failed write, a file -preserved after a failed load, or no configuration store). The changed workspaces still apply -for the session, and a later save retries them. +write the file immediately. Through `ui.menu.invoke` they return an error when it isn't written +(a failed write, a file preserved after a failed load, or no configuration store); +`engine.execute` hands them to the UI and returns `{frontend}` first, so check +`workspaceStorageError` there. The changed workspaces still apply for the session, and a later +save retries them. ## Input From e643457fa34ab624be29c8c85feeb488e758a957 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 22:03:01 -0400 Subject: [PATCH 7/8] Commands: an earlier invocation's frontend failure isn't a later command's result (#696) --- crates/ui-egui/src/menus.rs | 9 ++++--- crates/ui-egui/tests/ui_workspace_focus.rs | 28 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 4b8d14a4..6036ac6e 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -752,6 +752,8 @@ fn clipboard_note(s: &effectcraft_engine::Session) -> String { } pub(crate) fn run_engine(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { + // Events already queued belong to earlier invocations (`engine.execute` defers them). + let queued = app.session.events.len(); let mut r = app.session.execute(id, params).map_err(|e| e.to_string()); if let Err(e) = &r { app.ui.status = e.clone(); @@ -759,14 +761,15 @@ pub(crate) fn run_engine(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str ctx.copy_text(clipboard_note(&app.session)); } let events = app.session.drain_events(); - for ev in events { + for (index, ev) in events.into_iter().enumerate() { match ev { effectcraft_engine::Event::Frontend { command, params } => { if let Err(e) = frontend(app, ctx, &command, params) { app.ui.status = e.clone(); // A frontend-only command's own failure is the command's result (#696: a - // workspace save that didn't reach disk reported success). - if command == id && r.is_ok() { + // workspace save that didn't reach disk reported success). An earlier + // invocation's failure only sets the status. + if index >= queued && command == id && r.is_ok() { r = Err(e); } } diff --git a/crates/ui-egui/tests/ui_workspace_focus.rs b/crates/ui-egui/tests/ui_workspace_focus.rs index 8464c1fb..e3c5a381 100644 --- a/crates/ui-egui/tests/ui_workspace_focus.rs +++ b/crates/ui-egui/tests/ui_workspace_focus.rs @@ -438,3 +438,31 @@ fn saved_workspaces_reach_the_settings_folder_and_reload() { assert_eq!(app.ui.dock, saved); std::fs::remove_dir_all(dir).unwrap(); } + +/// A frontend event left queued by an earlier invocation (`engine.execute` defers them) only +/// sets the status: its failure is not the result of a later command with the same id. +#[test] +fn an_earlier_queued_frontend_failure_is_not_a_later_commands_result() { + let store = Arc::new(MemoryConfig::default()); + let mut app = EffectcraftApp::new(Session { config: Some(store.clone()), ..Default::default() }); + let ctx = egui::Context::default(); + let invoke = |app: &mut EffectcraftApp, id: &str, params| effectcraft_ui_egui::menus::invoke(app, &ctx, id, params); + // An earlier failure, then a success. + app.session.execute("window.workspace", json!({"name": "missing-old"})).unwrap(); + invoke(&mut app, "window.workspace", json!({"name": "Minimal"})).unwrap(); + assert_eq!(app.ui.workspace, "Minimal"); + // An earlier failing delete, then a rename that reaches the store. + invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "Original"})).unwrap(); + app.session.execute("window.editWorkspaces", json!({"name": "missing-old", "delete": true})).unwrap(); + invoke(&mut app, "window.editWorkspaces", json!({"name": "Original", "rename": "Renamed"})).unwrap(); + assert!(store.read("workspaces.json").unwrap().contains("Renamed")); + // Both fail: the result is the current invocation's own failure. + app.session.execute("window.workspace", json!({"name": "missing-old"})).unwrap(); + let error = invoke(&mut app, "window.workspace", json!({"name": "missing-new"})).unwrap_err(); + assert!(error.contains("missing-new") && !error.contains("missing-old"), "{error}"); + // An earlier failure of another command. + app.session.execute("window.workspace", json!({"name": "missing-old"})).unwrap(); + invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "Fine"})).unwrap(); + // Its own failure with nothing queued is still the result. + assert!(invoke(&mut app, "window.workspace", json!({"name": "missing"})).is_err()); +} From 544d46a21821ea67b4ce1998fa3d61aa2c7a36ec Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Mon, 12 Oct 2026 22:03:01 -0400 Subject: [PATCH 8/8] Workspaces: a failed save says a restart may undo the changes (#696) --- crates/ui-egui/src/lib.rs | 5 ++-- crates/ui-egui/tests/ui_workspace_focus.rs | 29 ++++++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index df886759..a3906e50 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -429,10 +429,11 @@ impl EffectcraftApp { /// Keep the saved workspaces for the next session (see [`EffectcraftApp::new`]): After /// Effects' saved workspaces last until they are deleted (#320). `Err` when they weren't - /// written (#696): the layouts stay usable and a later save retries, but a restart loses them. + /// written (#696): the change applies for this session and a later save retries it, but until + /// then a restart brings back what is on disk. pub(crate) fn store_saved_workspaces(&mut self) -> Result<(), String> { let Err(error) = self.workspace_store.save_now(self.session.config.as_deref(), &self.ui) else { return Ok(()) }; - let message = format!("Workspaces are kept for this session only and won't be there after a restart: {error}"); + let message = format!("Workspace changes couldn't be saved and may be lost after a restart: {error}"); log::warn!("{message}"); self.ui.status = message.clone(); // The raw error, so the background retry doesn't report the same failure again. diff --git a/crates/ui-egui/tests/ui_workspace_focus.rs b/crates/ui-egui/tests/ui_workspace_focus.rs index e3c5a381..0d0ecd00 100644 --- a/crates/ui-egui/tests/ui_workspace_focus.rs +++ b/crates/ui-egui/tests/ui_workspace_focus.rs @@ -86,7 +86,7 @@ fn unreadable_native_workspace_is_preserved_after_later_layout_changes() { app.set_workspace("Minimal"); app.ui.focused = PanelKind::Project; let error = effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name":"Must not overwrite"})).unwrap_err(); - assert!(error.contains("won't be there after a restart") && error.contains("workspaces.json"), "{error}"); + assert!(error.contains("may be lost after a restart") && error.contains("workspaces.json"), "{error}"); assert_eq!(app.ui.status, error, "the failure is in the status bar"); assert_eq!(app.ui.workspace, "Must not overwrite", "the layout stays usable for this session"); assert!(app.ui.saved_workspaces.contains_key("Must not overwrite")); @@ -383,7 +383,7 @@ fn failed_workspace_save_is_reported_and_retryable() { let ctx = egui::Context::default(); let invoke = |app: &mut EffectcraftApp, id: &str, params| effectcraft_ui_egui::menus::invoke(app, &ctx, id, params); let error = invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "caption craft"})).unwrap_err(); - assert!(error.contains("synthetic full disk") && error.contains("won't be there after a restart"), "{error}"); + assert!(error.contains("synthetic full disk") && error.contains("may be lost after a restart"), "{error}"); assert_eq!(app.ui.status, error); let storage_error = effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].clone(); assert!(storage_error.as_str().is_some_and(|e| e.contains("synthetic full disk")), "{storage_error}"); @@ -466,3 +466,28 @@ fn an_earlier_queued_frontend_failure_is_not_a_later_commands_result() { // Its own failure with nothing queued is still the result. assert!(invoke(&mut app, "window.workspace", json!({"name": "missing"})).is_err()); } + +/// A failed Save Workspace or Delete of a workspace already on disk leaves the file as it was: +/// after a restart the earlier layout, and the workspace that wasn't deleted, come back. +#[test] +fn failed_save_or_delete_of_a_saved_workspace_keeps_the_file() { + let store = Arc::new(FullDisk::default()); + let mut app = EffectcraftApp::new(Session { config: Some(store.clone()), ..Default::default() }); + let ctx = egui::Context::default(); + let invoke = |app: &mut EffectcraftApp, id: &str, params| effectcraft_ui_egui::menus::invoke(app, &ctx, id, params); + invoke(&mut app, "window.workspace", json!({"name": "Minimal"})).unwrap(); + invoke(&mut app, "window.saveWorkspaceAs", json!({"name": "Kept"})).unwrap(); + let saved = app.ui.dock.clone(); + let before = store.read("workspaces.json").unwrap(); + store.full.store(true, std::sync::atomic::Ordering::Relaxed); + app.ui.dock.close(PanelKind::Timeline); + let error = invoke(&mut app, "window.saveWorkspace", json!({})).unwrap_err(); + assert!(error.contains("synthetic full disk") && error.contains("may be lost after a restart"), "{error}"); + assert!(invoke(&mut app, "window.editWorkspaces", json!({"name": "Kept", "delete": true})).is_err()); + assert!(app.saved_workspace_names().is_empty(), "the delete applies for this session"); + assert_eq!(store.read("workspaces.json").unwrap(), before); + let mut restarted = EffectcraftApp::new(Session { config: Some(store), ..Default::default() }); + assert_eq!(restarted.saved_workspace_names(), ["Kept"]); + restarted.set_workspace("Kept"); + assert_eq!(restarted.ui.dock, saved); +}