From a7d47dbc1b739a1aed597768340368dd05830271 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 14 Oct 2026 15:47:24 -0400 Subject: [PATCH] Fix up #488: background opens enter the session through file.adopt The progress window's result was added with add_presentation_file, straight into the session: not journaled and outside the command panic guard that file.open runs under. The host now hands the loaded file to Session::stage_loaded and runs the new internal command file.adopt {path, token}, which shares file.open's tail (repaired-copy handling, damaged-media report) and so is journaled and guarded. Without the staged copy (a replayed journal, a stale token) file.adopt reads path like file.open. Reading still happens on the background thread. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 42 +++++++++++++++++++++++++++-------- crates/engine/src/lib.rs | 22 ++++++++++++++++++ crates/engine/src/tests.rs | 32 ++++++++++++++++++++++++++ crates/ui-egui/src/opening.rs | 14 ++++++++---- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index aa0b4719..3a24f70a 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -19,6 +19,15 @@ pub fn specs() -> Vec { ), cmd!(query "file.templates", "Starter Templates", [], None, "{} → [{name, summary, theme, slides}]", always, templates), cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), "{path}", always, open), + cmd!( + noundo "file.adopt", + "Open", + [], + None, + "{path, token?} (a presentation the host read in the background and staged with Session::stage_loaded; without one, reads path like file.open)", + always, + adopt + ), cmd!(noundo "file.openBytes", "Open Data", [], None, "{name, data: base64}", always, open_bytes), cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?, format?: deckcraft|pptx}", has_doc, save), cmd!(noundo "file.saveAs", "Save As…", ["File"], Some("Cmd+Shift+S"), "{path, format?}", has_doc, save_as), @@ -163,8 +172,8 @@ pub fn add_opened(s: &mut Session, name: &str, path: Option, p: Presenta } /// A presentation read from disk, not yet added to a session: [`read_presentation_file`] needs no -/// session, so a host can run it on a background thread and add the result with -/// [`add_presentation_file`]. +/// session, so a host can run it on a background thread, hand the result to +/// [`Session::stage_loaded`] and add it with `file.adopt`. #[derive(Debug)] pub struct LoadedFile { pub path: String, @@ -184,22 +193,37 @@ pub fn read_presentation_file(path: &str) -> Result { Ok(LoadedFile { path: path.to_string(), doc, native, damaged }) } -/// Add a presentation read by [`read_presentation_file`] as the active document. A damaged file -/// opens as an untitled repaired copy, so Save can't overwrite the original with the empty media. -pub fn add_presentation_file(s: &mut Session, f: LoadedFile) -> Result<(usize, Vec)> { +/// Add a loaded presentation as the active document: the shared end of `file.open` and +/// `file.adopt`. A damaged file opens as an untitled repaired copy, so Save can't overwrite the +/// original with the empty media. +fn adopt_loaded(s: &mut Session, f: LoadedFile) -> Result { let LoadedFile { path, doc, native, damaged } = f; let keep_path = native && damaged.is_empty(); let i = add_opened(s, &path, keep_path.then(|| path.clone()), doc); report_damage(s, &damaged)?; - Ok((i, damaged)) + let st = s.doc()?; + Ok(json!({"document": i, "slides": st.doc.slides.len(), "missingFonts": st.missing_fonts, "damagedMedia": damaged})) } #[cfg(not(target_arch = "wasm32"))] fn open(s: &mut Session, p: &Value) -> Result { let path = str_param(p, "path").ok_or_else(|| bad("file.open", "missing `path`"))?; - let (i, damaged) = add_presentation_file(s, read_presentation_file(path)?)?; - let st = s.doc()?; - Ok(json!({"document": i, "slides": st.doc.slides.len(), "missingFonts": st.missing_fonts, "damagedMedia": damaged})) + adopt_loaded(s, read_presentation_file(path)?) +} + +/// `file.adopt {path, token?}`: add the presentation a host read in the background and staged +/// with [`Session::stage_loaded`]. Without that staged file (a replayed journal, a stale token) +/// it reads `path` like `file.open`, so the journal entry stays replayable. +fn adopt(s: &mut Session, p: &Value) -> Result { + let path = str_param(p, "path").ok_or_else(|| bad("file.adopt", "missing `path`"))?; + let staged = p.get("token").and_then(Value::as_u64).and_then(|t| s.take_loaded(t)).filter(|f| f.path == path); + match staged { + Some(f) => adopt_loaded(s, f), + #[cfg(not(target_arch = "wasm32"))] + None => adopt_loaded(s, read_presentation_file(path)?), + #[cfg(target_arch = "wasm32")] + None => Err(bad("file.adopt", format!("{path} was not loaded"))), + } } #[cfg(target_arch = "wasm32")] fn open(_s: &mut Session, _p: &Value) -> Result { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 60eebaf8..29fae722 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -470,6 +470,9 @@ pub struct Session { pub mac_keys: bool, /// Command panics caught by the guard (the document was rolled back); soak tests require 0. pub panics_caught: u64, + /// Presentations a host read off the UI thread, waiting for `file.adopt` (by token). + loaded: Vec<(u64, cmd::file::LoadedFile)>, + next_loaded: u64, } impl Default for Session { @@ -499,8 +502,27 @@ impl Session { gif_paused: Default::default(), mac_keys: cfg!(target_os = "macos"), panics_caught: 0, + loaded: vec![], + next_loaded: 0, } } + /// Hand over a presentation read on another thread ([`cmd::file::read_presentation_file`]); + /// `file.adopt` with the returned token adds it as a document, through the command path like + /// `file.open` (journaled, panic-guarded). Only the latest few wait; older ones are dropped. + pub fn stage_loaded(&mut self, f: cmd::file::LoadedFile) -> u64 { + const KEEP: usize = 4; + self.next_loaded = self.next_loaded.wrapping_add(1); + if self.loaded.len() >= KEEP { + self.loaded.remove(0); + } + self.loaded.push((self.next_loaded, f)); + self.next_loaded + } + /// The staged presentation for `token`, removed from the session. + pub(crate) fn take_loaded(&mut self, token: u64) -> Option { + let i = self.loaded.iter().position(|(t, _)| *t == token)?; + Some(self.loaded.remove(i).1) + } /// A session with one new presentation open. pub fn with_new() -> Self { let mut s = Self::new(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index a4359ee8..e55127e0 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2739,3 +2739,35 @@ fn three_d_reset_and_flat_shapes_drop_the_xml() { s3.apply(&json!({"depth": 3})); assert!(s3.to_xml().unwrap().contains("")); } + +/// A presentation read off the UI thread enters the session through `file.adopt`: journaled like +/// `file.open`, and replayable from the journal without the staged copy. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn background_loads_are_adopted_through_a_command() { + let mut src = Session::with_new(); + src.execute("slide.new", &json!({})).unwrap(); + let path = std::env::temp_dir().join(format!("deckcraft-adopt-{}.deckcraft", std::process::id())); + let path = path.to_string_lossy().to_string(); + src.execute("file.saveAs", &json!({"path": path})).unwrap(); + + let mut s = Session::new(); + let loaded = cmd::file::read_presentation_file(&path).unwrap(); + let token = s.stage_loaded(loaded); + let r = s.execute("file.adopt", &json!({"path": path, "token": token})).unwrap(); + assert_eq!(r["slides"], 2, "{r}"); + assert_eq!(s.documents().len(), 1); + assert_eq!(s.doc().unwrap().path.as_deref(), Some(path.as_str())); + let (id, params) = s.journal.last().cloned().unwrap(); + assert_eq!(id, "file.adopt"); + // The token was used up; replaying the entry reads the file again. + let mut replay = Session::new(); + replay.execute(&id, ¶ms).unwrap(); + assert_eq!(replay.doc().unwrap().doc.slides.len(), 2); + // A token staged for another path is not taken for this one. + let other = s.stage_loaded(cmd::file::LoadedFile { path: "/elsewhere.pptx".into(), doc: Default::default(), native: true, damaged: vec![] }); + s.execute("file.adopt", &json!({"path": path, "token": other})).unwrap(); + assert_eq!(s.doc().unwrap().doc.slides.len(), 2); + assert!(s.execute("file.adopt", &json!({})).is_err()); + std::fs::remove_file(&path).ok(); +} diff --git a/crates/ui-egui/src/opening.rs b/crates/ui-egui/src/opening.rs index d48734ec..debeb03c 100644 --- a/crates/ui-egui/src/opening.rs +++ b/crates/ui-egui/src/opening.rs @@ -71,10 +71,14 @@ impl SlideApp { }; let Some(o) = self.opening.take() else { return }; match r { - Ok(f) => match deckcraft_engine::cmd::file::add_presentation_file(&mut self.session, f) { - Ok(_) => self.note_recent(&o.path), - Err(e) => self.set_status(e.to_string()), - }, + Ok(f) => { + // Into the session through the command path, like file.open: journaled and guarded. + let token = self.session.stage_loaded(f); + match self.session.execute("file.adopt", &serde_json::json!({"path": o.path, "token": token})) { + Ok(_) => self.note_recent(&o.path), + Err(e) => self.set_status(e.to_string()), + } + } Err(e) => self.set_status(e), } } @@ -144,6 +148,8 @@ mod tests { assert_eq!(app.session.documents().len(), 1); assert_eq!(app.session.active().unwrap().doc.slides.len(), 3); assert_eq!(app.ui.recent.first(), Some(&path)); + // It went in through the command path (journaled like file.open). + assert_eq!(app.session.journal.last().map(|(id, _)| id.as_str()), Some("file.adopt")); std::fs::remove_file(&path).ok(); }