From b9048a5b083176399ebb7d6bc9c9461e749bcc5a Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Mon, 12 Oct 2026 11:53:46 -0600 Subject: [PATCH] fix(save): preserve live state when persistence fails Preserve document paths, revisions and collection contents when persistence fails. Rebase Save a Copy paths without changing the open document. Follow-up to #485. PR #613. --- ROADMAP.md | 3 +- crates/engine/src/cmd/book.rs | 15 ++-- crates/engine/src/cmd/file.rs | 23 ++--- crates/engine/src/cmd/library.rs | 9 +- .../tests/collection_state_atomicity.rs | 87 +++++++++++++++++++ crates/engine/tests/save_state_atomicity.rs | 68 +++++++++++++++ docs/target-app-parity.md | 1 + 7 files changed, 186 insertions(+), 20 deletions(-) create mode 100644 crates/engine/tests/collection_state_atomicity.rs create mode 100644 crates/engine/tests/save_state_atomicity.rs diff --git a/ROADMAP.md b/ROADMAP.md index 4b7163c..8260a78 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: pre-alpha** · next: alpha, ~0% and ~15–25 h away; beta ~28 points and ~420–750 h beyond that (the core-workflow gate, not the percentage: ready for real work is already ~47%) -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (window and menu quit confirmation) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (failed writes preserve live save and collection state) · **Target:** Adobe InDesign 2026 DesignCraft aims at full Adobe InDesign parity, and to be better: faster, open (documented JSON format + IDML), scriptable by agents (MCP), and available on the web. @@ -129,6 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. +- **2026-10-12** · Save As, library edits and book collection edits persist prospective values before changing live state; failed writes preserve the document and existing collections. Save a Copy rebases data-source paths for its destination without changing the open document. No readiness percentage change. - **2026-10-12** · Window close and application/menu Quit ask about unsaved documents; later Cancel keeps every tab open, including earlier Don't Save choices. Approvals are rechecked after edits. Cancelable macOS Dock Quit remains open (#544). No readiness percentage change. - **2026-10-12** · Each running app keeps recovery data in its own locked folder; current folder and lock ownership are checked before writes and cleanup. Failed isolation disables recovery with a warning instead of sharing document IDs. No readiness percentage change. - **2026-10-12** · Native engine saves and exports replace files atomically; failed writes retain the original, private staging preserves Unix permissions, and valid long filenames work. Desktop Services and CLI raster exports remain separate follow-ups. No readiness percentage change. diff --git a/crates/engine/src/cmd/book.rs b/crates/engine/src/cmd/book.rs index 1204029..ad0d44b 100644 --- a/crates/engine/src/cmd/book.rs +++ b/crates/engine/src/cmd/book.rs @@ -108,15 +108,16 @@ pub fn specs() -> Vec { cmd!(noundo "book.add", "Add Document", [], None, "{path, at?: index}", has_book, |s, p| { let path = str_param(p, "path").ok_or_else(|| bad("book.add", "`path` required"))?.to_string(); load(&path)?; - let b = s.book.as_mut().ok_or_else(no_book)?; + let mut b = s.book.as_ref().ok_or_else(no_book)?.clone(); let at = p.get("at").and_then(Value::as_u64).map_or(b.documents.len(), |i| (i as usize).min(b.documents.len())); b.documents.insert(at, path); - save(b)?; + save(&b)?; + s.book = Some(b); ok() }), cmd!(noundo "book.remove", "Remove Document", [], None, "{index}", has_book, |s, p| { let i = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad("book.remove", "`index` required"))? as usize; - let b = s.book.as_mut().ok_or_else(no_book)?; + let mut b = s.book.as_ref().ok_or_else(no_book)?.clone(); if i >= b.documents.len() { return Err(bad("book.remove", format!("no document {i}"))); } @@ -124,17 +125,19 @@ pub fn specs() -> Vec { if b.style_source >= b.documents.len() { b.style_source = 0; } - save(b)?; + save(&b)?; + s.book = Some(b); ok() }), cmd!(noundo "book.styleSource", "Style Source", [], None, "{index}", has_book, |s, p| { let i = p.get("index").and_then(Value::as_u64).unwrap_or(0) as usize; - let b = s.book.as_mut().ok_or_else(no_book)?; + let mut b = s.book.as_ref().ok_or_else(no_book)?.clone(); if i >= b.documents.len() { return Err(bad("book.styleSource", format!("no document {i}"))); } b.style_source = i; - save(b)?; + save(&b)?; + s.book = Some(b); ok() }), cmd!(query "book.list", "Book", [], None, "{} → {path, styleSource, documents: [{path, pages, firstPage}]}", has_book, |s, _| { diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index b5887bb..2537d1e 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -25,7 +25,9 @@ pub fn specs() -> Vec { cmd!(noundo "file.saveAs", "Save As…", ["File"], Some("Cmd+Shift+S"), "{path}", has_doc, file_save), cmd!(noundo "file.saveACopy", "Save a Copy…", ["File"], None, "{path} — writes the document without changing which file it is or its unsaved state", has_doc, |s, p| { let path = str_param(p, "path").ok_or_else(|| bad("file.saveACopy", "missing `path`"))?.to_string(); - let bytes = to_bytes(&s.doc()?.doc); + let mut copy = (*s.doc()?.doc).clone(); + super::datamerge::refresh_relative_paths(&mut copy, std::path::Path::new(&path)); + let bytes = to_bytes(©); #[cfg(not(target_arch = "wasm32"))] crate::atomic::write_file(&path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len()})) @@ -237,17 +239,18 @@ fn file_save(s: &mut Session, p: &Value) -> Result { .map(str::to_string) .or_else(|| st.path.clone()) .ok_or_else(|| bad("file.save", "missing `path` (document has never been saved)"))?; - { - let mut d = (*st.doc).clone(); - super::datamerge::refresh_relative_paths(&mut d, std::path::Path::new(&path)); - if d.data_merge != st.doc.data_merge { - st.doc = Arc::new(d); - st.revision = st.revision.saturating_add(1); - } - } - let bytes = to_bytes(&st.doc); + // Prepare paths for the destination without changing the live document until persistence succeeds. + let mut saved = (*st.doc).clone(); + super::datamerge::refresh_relative_paths(&mut saved, std::path::Path::new(&path)); + let paths_changed = saved.data_merge != st.doc.data_merge; + let saved = if paths_changed { Arc::new(saved) } else { st.doc.clone() }; + let bytes = to_bytes(&saved); #[cfg(not(target_arch = "wasm32"))] crate::atomic::write_file(&path, &bytes)?; + if paths_changed { + st.doc = saved; + st.revision = st.revision.saturating_add(1); + } st.path = Some(path.clone()); st.saved_revision = st.revision; st.saved_doc = st.doc.clone(); diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 00855dd..384b4f8 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -86,10 +86,12 @@ pub fn specs() -> Vec { let n = st.selection.items.len(); (str_param(p, "name").map(str::to_string).unwrap_or_else(|| if n > 1 { format!("{n} objects") } else { kind.clone() }), str_param(p, "description").unwrap_or("").to_string()) }; - let lib = s.library.as_mut().ok_or_else(no_library)?; + let mut lib = s.library.as_ref().ok_or_else(no_library)?.clone(); + let index = lib.items.len(); lib.items.push(LibraryItem { name, description, snippet: super::file::base64_encode(&snip) }); lib.save()?; - Ok(json!({"index": lib.items.len() - 1})) + s.library = Some(lib); + Ok(json!({"index": index})) }), cmd!( "library.place", @@ -107,13 +109,14 @@ pub fn specs() -> Vec { } ), cmd!(noundo "library.remove", "Delete Item", [], None, "{index}", has_library, |s, p| { - let lib = s.library.as_mut().ok_or_else(no_library)?; + let mut lib = s.library.as_ref().ok_or_else(no_library)?.clone(); let i = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad("library.remove", "`index` required"))? as usize; if i >= lib.items.len() { return Err(bad("library.remove", format!("no item {i}"))); } lib.items.remove(i); lib.save()?; + s.library = Some(lib); ok() }), cmd!(noundo "conveyor.collect", "Collect", [], None, "{ids? (default: the selection)} — each object onto the Content Collector conveyor → {count}", has_selection, |s, p| { diff --git a/crates/engine/tests/collection_state_atomicity.rs b/crates/engine/tests/collection_state_atomicity.rs new file mode 100644 index 0000000..b73eefa --- /dev/null +++ b/crates/engine/tests/collection_state_atomicity.rs @@ -0,0 +1,87 @@ +#![cfg(not(target_arch = "wasm32"))] + +use designcraft_engine::Session; +use serde_json::json; + +fn library_failure(command: &str) { + let temp = tempfile::tempdir().unwrap(); + let folder = temp.path().join("library"); + std::fs::create_dir(&folder).unwrap(); + let file = folder.join("items.dclib"); + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("frame.create", &json!({"rect": [0, 0, 50, 50]})).unwrap(); + s.execute("library.new", &json!({"path": file})).unwrap(); + s.execute("library.add", &json!({"name": "Keep"})).unwrap(); + let before = serde_json::to_value(&s.library).unwrap(); + let path = s.library.as_ref().unwrap().path.clone(); + let journal = s.journal.clone(); + let original_bytes = std::fs::read(&file).unwrap(); + let kept = temp.path().join("kept-library"); + std::fs::rename(&folder, &kept).unwrap(); + + assert!(s.execute(command, &json!({"name": "New", "index": 0})).is_err()); + + assert_eq!(before, serde_json::to_value(&s.library).unwrap(), "a failed library write must retain its original items"); + assert_eq!(path, s.library.as_ref().unwrap().path); + assert_eq!(journal, s.journal); + assert_eq!(original_bytes, std::fs::read(kept.join("items.dclib")).unwrap()); +} + +#[test] +fn failed_library_add_keeps_existing_items() { + library_failure("library.add"); +} + +#[test] +fn failed_library_remove_keeps_existing_items() { + library_failure("library.remove"); +} + +fn book_failure(command: &str) { + let temp = tempfile::tempdir().unwrap(); + let mut s = Session::new(); + let mut chapters = Vec::new(); + for i in 0..3 { + s.execute("file.new", &json!({})).unwrap(); + let path = temp.path().join(format!("chapter-{i}.designcraft")); + s.execute("file.saveAs", &json!({"path": path})).unwrap(); + chapters.push(path); + } + let folder = temp.path().join("book"); + std::fs::create_dir(&folder).unwrap(); + let file = folder.join("chapters.dcbook"); + s.execute("book.new", &json!({"path": file})).unwrap(); + s.execute("book.add", &json!({"path": chapters[0]})).unwrap(); + s.execute("book.add", &json!({"path": chapters[1]})).unwrap(); + s.execute("book.styleSource", &json!({"index": 1})).unwrap(); + let before = serde_json::to_value(&s.book).unwrap(); + let path = s.book.as_ref().unwrap().path.clone(); + let journal = s.journal.clone(); + let original_bytes = std::fs::read(&file).unwrap(); + let kept = temp.path().join("kept-book"); + std::fs::rename(&folder, &kept).unwrap(); + let params = if command == "book.add" { json!({"path": chapters[2]}) } else { json!({"index": 0}) }; + + assert!(s.execute(command, ¶ms).is_err()); + + assert_eq!(before, serde_json::to_value(&s.book).unwrap(), "a failed book write must retain its original chapter list and style source"); + assert_eq!(path, s.book.as_ref().unwrap().path); + assert_eq!(journal, s.journal); + assert_eq!(original_bytes, std::fs::read(kept.join("chapters.dcbook")).unwrap()); +} + +#[test] +fn failed_book_add_keeps_existing_chapters() { + book_failure("book.add"); +} + +#[test] +fn failed_book_remove_keeps_existing_chapters() { + book_failure("book.remove"); +} + +#[test] +fn failed_book_style_source_keeps_existing_source() { + book_failure("book.styleSource"); +} diff --git a/crates/engine/tests/save_state_atomicity.rs b/crates/engine/tests/save_state_atomicity.rs new file mode 100644 index 0000000..326d83b --- /dev/null +++ b/crates/engine/tests/save_state_atomicity.rs @@ -0,0 +1,68 @@ +#![cfg(not(target_arch = "wasm32"))] + +use std::path::Path; +use std::sync::Arc; + +use designcraft_engine::Session; +use serde_json::json; + +fn linked_document(root: &Path) -> (Session, std::path::PathBuf) { + let source = root.join("source/contacts.csv"); + std::fs::create_dir_all(source.parent().unwrap()).unwrap(); + std::fs::write(&source, "Name\nAda\n").unwrap(); + let original = root.join("original/template.designcraft"); + std::fs::create_dir_all(original.parent().unwrap()).unwrap(); + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + session.execute("data.source.select", &json!({"path": source})).unwrap(); + session.execute("file.saveAs", &json!({"path": original})).unwrap(); + (session, original) +} + +#[test] +fn failed_save_as_preserves_document_saved_state_history_and_original_file() { + let temp = tempfile::tempdir().unwrap(); + let (mut session, original) = linked_document(temp.path()); + session.execute("frame.create", &json!({"rect": [0, 0, 50, 50]})).unwrap(); + session.execute("edit.undo", &json!({})).unwrap(); + let before = session.doc().unwrap().clone(); + let journal = session.journal.clone(); + let original_bytes = std::fs::read(&original).unwrap(); + let destination = temp.path().join("missing/elsewhere/template.designcraft"); + + assert!(session.execute("file.saveAs", &json!({"path": destination})).is_err()); + + let after = session.doc().unwrap(); + assert!(Arc::ptr_eq(&before.doc, &after.doc), "failed Save As must not change data-source paths"); + assert!(Arc::ptr_eq(&before.saved_doc, &after.saved_doc)); + assert_eq!(before.path, after.path); + assert_eq!((before.revision, before.saved_revision, before.is_dirty()), (after.revision, after.saved_revision, after.is_dirty())); + assert_eq!(before.doc.data_merge, after.doc.data_merge); + assert_eq!(before.history.undo.len(), after.history.undo.len()); + assert_eq!(before.history.redo.len(), after.history.redo.len()); + assert_eq!(serde_json::to_value(&before.selection).unwrap(), serde_json::to_value(&after.selection).unwrap()); + assert_eq!(journal, session.journal); + assert_eq!(original_bytes, std::fs::read(original).unwrap()); + assert!(!destination.exists()); +} + +#[test] +fn save_a_copy_rebases_its_data_source_without_changing_the_open_document() { + let temp = tempfile::tempdir().unwrap(); + let (mut session, original) = linked_document(temp.path()); + let before = session.doc().unwrap().clone(); + let original_bytes = std::fs::read(&original).unwrap(); + let destination = temp.path().join("copies/nested/template.designcraft"); + std::fs::create_dir_all(destination.parent().unwrap()).unwrap(); + + session.execute("file.saveACopy", &json!({"path": destination})).unwrap(); + + let copy = designcraft_format::load(&std::fs::read(destination).unwrap()).unwrap(); + assert_eq!(copy.data_merge.sources[0].relative_path.as_deref(), Some("../../source/contacts.csv")); + let after = session.doc().unwrap(); + assert!(Arc::ptr_eq(&before.doc, &after.doc)); + assert!(Arc::ptr_eq(&before.saved_doc, &after.saved_doc)); + assert_eq!(before.path, after.path); + assert_eq!((before.revision, before.saved_revision, before.is_dirty()), (after.revision, after.saved_revision, after.is_dirty())); + assert_eq!(original_bytes, std::fs::read(original).unwrap()); +} diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 5705ee7..bbf1796 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -289,6 +289,7 @@ It is evidence of breadth: each item exists. It is not a claim of InDesign-level - Missing glyphs, as in InDesign: a character the applied font lacks is drawn as that font's missing-glyph box (its .notdef; a drawn box when the font's is empty), on screen and in PDF (as a path, so PDF/A and PDF/UA exports take it), and Preflight lists the characters by font. Preferences › Composition › Draw Missing Glyphs from Fallback Fonts (`document.preferences {glyphFallback}`, stored with the document) draws them from other fonts instead; it is off in new documents and IDML imports, on in documents saved before it existed. A missing font's substitute always uses fallback fonts. - Damage-region repaint: after an edit the canvas re-renders only the frames whose composed text (or the items that) changed and patches its texture — pixel-identical to a full render; typing in a frame repaints in under a millisecond instead of a full-viewport render (22 ms measured on the sample). - Native engine saves, Save a Copy, snippets, books, libraries, swatches and engine/MCP exports use same-folder atomic replacement. Failed writes before replacement retain the original; existing Unix permissions apply before staging document bytes. Desktop Services writes and CLI raster exports still need this routing. Parent-folder flushing is best effort, so this is not a power-loss durability guarantee. +- Save As prepares destination-relative data-source paths before writing and installs them only after a successful save. Failed Save As retains the document, saved state and history; Save a Copy rebases only the saved copy. Library and book collection edits likewise update live values only after persistence succeeds. - Crash recovery: unsaved documents are written every 30 s into a locked folder owned by that running app and reopened (unsaved, remembering their files) after a crash. Startup takes entries from ended runs and leaves live runs alone; writes and cleanup validate current folder and lock ownership. Failed isolation disables recovery with a warning. Concurrent older versions still use unlocked root entries; mixed-version isolation remains open. File › Revert and Save a Copy…; `file.recovery.*`, `file.revert`, `file.saveACopy`. - Window close and application/menu Quit ask about every unsaved document. Cancel or a cancelled/failed save keeps all tabs open; Don't Save approvals discard nothing until the complete plan is current. Later edits and new documents are checked before closing. Control-channel quit remains immediate; a cancelable macOS Dock Quit prompt is still missing (#544). - Graphics backend chosen before the window exists (DirectX 12 on Windows, Vulkan on Linux, Metal on macOS, OpenGL as the fallback; `WGPU_BACKEND` overrides) with a start-up fallback: a backend whose start never showed a frame is skipped at the next start and the status bar says so (`gpu.json`). Found on an AMD hybrid-graphics laptop whose Vulkan driver faulted at the first present.