From ebd1d78194082efba477983ebb24569a41d7dd41 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 12 Oct 2026 11:08:17 -0400 Subject: [PATCH 1/5] Release workflow: pass the Azure signing secrets so Windows builds are signed (#534) Since the release workflow moved to storytold/craft-common-actions, it has passed only the Apple and WINDOWS_CERTIFICATE secrets. The signing job reads values from the release environment, but only for names the caller lists, so the AZURE_* names that Trusted Signing uses came through empty and Windows builds would ship unsigned. Same fix as storytold/craft-launcher#174. Co-authored-by: Claude Opus 5.5 --- .github/workflows/release.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 37c71fc6..57eb9bb5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,3 +35,9 @@ jobs: APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} WINDOWS_CERTIFICATE: ${{ secrets.WINDOWS_CERTIFICATE }} WINDOWS_CERTIFICATE_PASSWORD: ${{ secrets.WINDOWS_CERTIFICATE_PASSWORD }} + AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + AZURE_SIGNING_ENDPOINT: ${{ secrets.AZURE_SIGNING_ENDPOINT }} + AZURE_SIGNING_ACCOUNT: ${{ secrets.AZURE_SIGNING_ACCOUNT }} + AZURE_CERT_PROFILE: ${{ secrets.AZURE_CERT_PROFILE }} From 3624f488192a8eeade11bd27609c9e5056214e75 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 12 Oct 2026 11:11:06 -0400 Subject: [PATCH 2/5] Release 0.7.1: rebuild with the fixed shared release workflow The previous release build failed in craft-common-actions, not in this repo: riscv64 packaging got --formats deb,rpm,tar (fixed in craft-common-actions#3), Linux jobs lacked the ALSA headers (craft-common-actions#5), and Windows signing needed the AZURE_* secrets listed in the caller. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 40 ++++++++++++++++++++-------------------- Cargo.toml | 2 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e01165c..5e4dc897 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -887,7 +887,7 @@ dependencies = [ [[package]] name = "designcraft" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-engine", "designcraft-ui-egui", @@ -904,7 +904,7 @@ dependencies = [ [[package]] name = "designcraft-cli" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -920,7 +920,7 @@ dependencies = [ [[package]] name = "designcraft-color" -version = "0.7.0" +version = "0.7.1" dependencies = [ "kurbo", "moxcms", @@ -932,7 +932,7 @@ dependencies = [ [[package]] name = "designcraft-compose" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-doc", @@ -949,7 +949,7 @@ dependencies = [ [[package]] name = "designcraft-doc" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-geom", @@ -961,7 +961,7 @@ dependencies = [ [[package]] name = "designcraft-engine" -version = "0.7.0" +version = "0.7.1" dependencies = [ "caseless", "craft-print", @@ -995,7 +995,7 @@ dependencies = [ [[package]] name = "designcraft-epub" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-doc", @@ -1004,7 +1004,7 @@ dependencies = [ [[package]] name = "designcraft-fonts" -version = "0.7.0" +version = "0.7.1" dependencies = [ "harfrust 0.13.3", "kurbo", @@ -1016,7 +1016,7 @@ dependencies = [ [[package]] name = "designcraft-format" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-doc", "serde", @@ -1027,7 +1027,7 @@ dependencies = [ [[package]] name = "designcraft-geom" -version = "0.7.0" +version = "0.7.1" dependencies = [ "flo_curves", "kurbo", @@ -1040,7 +1040,7 @@ dependencies = [ [[package]] name = "designcraft-idml" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1053,7 +1053,7 @@ dependencies = [ [[package]] name = "designcraft-images" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-fonts", @@ -1064,7 +1064,7 @@ dependencies = [ [[package]] name = "designcraft-mcp" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-engine", "designcraft-geom", @@ -1077,7 +1077,7 @@ dependencies = [ [[package]] name = "designcraft-pdf" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1099,7 +1099,7 @@ dependencies = [ [[package]] name = "designcraft-render" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1117,7 +1117,7 @@ dependencies = [ [[package]] name = "designcraft-textimport" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-doc", "quick-xml 0.38.4", @@ -1126,7 +1126,7 @@ dependencies = [ [[package]] name = "designcraft-tools" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1138,7 +1138,7 @@ dependencies = [ [[package]] name = "designcraft-ui-egui" -version = "0.7.0" +version = "0.7.1" dependencies = [ "craft-ui", "designcraft-color", @@ -1165,7 +1165,7 @@ dependencies = [ [[package]] name = "designcraft-web" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-engine", "designcraft-format", @@ -5685,7 +5685,7 @@ checksum = "9440ea3e5aeabb0ac63af70daf835274065238cdd0cec83418f417eae38bacee" [[package]] name = "xtask" -version = "0.7.0" +version = "0.7.1" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 82d13fe8..c2126ade 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/designcraft", "apps/designcraft-cli", "xtask"] [workspace.package] -version = "0.7.0" +version = "0.7.1" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" From 5cdcac4aceb87ebe0a1f2e0e3cdffd2b4e9b3fb3 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:19:36 -0400 Subject: [PATCH 3/5] fix(save): replace files atomically Replace native engine saves and engine/MCP exports atomically, preserving private permissions during staging and cleaning up failed writes. Add regressions for long filenames and caught writer unwinding. Fixes #436. Co-authored-by: aiongg Co-authored-by: skylar taylor-barrick --- ROADMAP.md | 3 +- crates/engine/src/atomic.rs | 359 +++++++++++++++++++++++++++ crates/engine/src/cmd/book.rs | 6 +- crates/engine/src/cmd/export.rs | 10 +- crates/engine/src/cmd/file.rs | 37 ++- crates/engine/src/cmd/interchange.rs | 2 +- crates/engine/src/cmd/library.rs | 2 +- crates/engine/src/cmd/style.rs | 6 +- crates/engine/src/cmd/xml.rs | 2 +- crates/engine/src/lib.rs | 2 + crates/mcp/src/headless.rs | 4 +- crates/mcp/src/tools.rs | 2 +- docs/target-app-parity.md | 1 + 13 files changed, 417 insertions(+), 19 deletions(-) create mode 100644 crates/engine/src/atomic.rs diff --git a/ROADMAP.md b/ROADMAP.md index d33353ca..6045a000 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-11 · **Change:** minor (shared print spooler; Windows CI passes) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (atomic engine saves and exports) · **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** · 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. - **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. - **2026-10-11** · `transform.move {copy}`, Edit › Duplicate and Step and Repeat return the copies' ids, so a script can go on with them. No readiness percentage change. - **2026-10-11** · `object.fill` and `object.stroke` refuse a swatch name the document doesn't have (it painted nothing) and suggest the name meant (`Paper` → `[Paper]`). No readiness percentage change. diff --git a/crates/engine/src/atomic.rs b/crates/engine/src/atomic.rs new file mode 100644 index 00000000..7eed2f02 --- /dev/null +++ b/crates/engine/src/atomic.rs @@ -0,0 +1,359 @@ +//! Crash-safe file replacement: how documents, exports and other user files reach the disk. +//! +//! [`write_atomic`] never writes the destination in place. It writes a uniquely named temporary +//! file in the same folder, flushes it (`sync_all`), renames it over the destination and then +//! flushes the folder (Unix) so the rename survives a power loss. When any step before the rename +//! fails, the temporary file is removed and the destination keeps its old bytes. +//! +//! - A replaced file keeps its permissions (Unix mode). +//! - A symbolic link keeps pointing at its file: the file it names is replaced. +//! - Windows: antivirus scanners and the search indexer briefly hold files open, which makes the +//! replacing rename fail with `PermissionDenied`; the rename is retried with a short backoff. +//! - A failed rename (cross-device, unwritable folder) is returned as an error; there is no +//! fallback to writing in place. +//! - Flushing the parent folder after replacement is best effort. A successful save confirms +//! that the new file is in place; it cannot guarantee survival of a subsequent power loss if +//! the file system rejects that folder flush. +//! +//! The web has no file system, so this module is not built for `wasm32`. + +use std::fs::File; +#[cfg(unix)] +use std::fs::OpenOptions; +use std::io::{self, Write as _}; +use std::path::{Path, PathBuf}; + +/// How many times a replacing rename is tried before its error is returned. +const RENAME_TRIES: u32 = if cfg!(windows) { 5 } else { 1 }; +/// The pause before the first rename retry; it doubles up to [`RENAME_MAX_DELAY_MS`]. +const RENAME_FIRST_DELAY_MS: u64 = 10; +const RENAME_MAX_DELAY_MS: u64 = 200; +/// Symbolic links followed before giving up (a loop, or an absurd chain). +const MAX_LINKS: usize = 40; + +/// Atomically replace (or create) `path` with `bytes`. See the module docs. +pub fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> { + write_atomic_with(path, |f| f.write_all(bytes)) +} + +/// [`write_atomic`] for a command's `path` parameter: the error names the path. +pub(crate) fn write_file(path: &str, bytes: &[u8]) -> crate::Result<()> { + write_atomic(Path::new(path), bytes).map_err(|e| crate::EngineError::Other(format!("{path}: {e}"))) +} + +/// Atomically replace (or create) `path` with what `fill` writes into the temporary file. When +/// `fill`, the flush or the rename fails, the temporary file is removed, `path` is untouched and +/// the error is returned. +pub fn write_atomic_with(path: &Path, fill: impl FnOnce(&mut File) -> io::Result<()>) -> io::Result<()> { + let target = resolve_links(path)?; + target.file_name().ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, format!("{}: not a file path", path.display())))?; + let dir = match target.parent() { + Some(d) if !d.as_os_str().is_empty() => d.to_path_buf(), + _ => PathBuf::from("."), + }; + let existing = std::fs::metadata(&target).ok(); + if existing.is_some() { + check_writable(&target)?; + } + let mut tmp = create_temp(&dir, existing.as_ref())?; + if let Some(existing) = existing { + // Restore the exact mode before any document bytes are written, and flush it along with + // the content. The caller's umask may have restricted the initial temporary-file mode. + keep_permissions(tmp.as_file(), &existing)?; + } + fill(tmp.as_file_mut())?; + tmp.as_file().sync_all()?; + // Closing the file before renaming also allows replacement on Windows. TempPath removes the + // staging file on every error; NamedTempFile does the same if the writer unwinds. + let tmp = tmp.into_temp_path(); + retry_rename(RENAME_TRIES, || std::fs::rename(&tmp, &target))?; + // The new bytes are in place. A failed folder flush can't be undone and must not report the + // save as failed. + let _ = sync_dir(&dir); + Ok(()) +} + +/// Create a fresh temporary file beside the target (the same folder, so the rename stays on one +/// volume). A short randomized name supports long destination names and skips existing entries. +fn create_temp(dir: &Path, existing: Option<&std::fs::Metadata>) -> io::Result { + let mut builder = tempfile::Builder::new(); + builder.prefix(".designcraft-save-").suffix(".tmp"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + // Private replacements are private from creation. New files retain fs::write's normal + // 0666 mode restricted by the caller's umask. + let mode = existing.map_or(0o666, |metadata| metadata.permissions().mode()); + builder.permissions(std::fs::Permissions::from_mode(mode)); + } + #[cfg(not(unix))] + let _ = existing; + builder.tempfile_in(dir) +} + +/// The file a chain of symbolic links ends at (`path` itself when it is not a link). A dangling +/// link resolves to the missing file it names, which is then created. +fn resolve_links(path: &Path) -> io::Result { + let mut p = path.to_path_buf(); + for _ in 0..MAX_LINKS { + match std::fs::symlink_metadata(&p) { + Ok(m) if m.file_type().is_symlink() => { + let to = std::fs::read_link(&p)?; + p = match p.parent() { + Some(dir) if to.is_relative() => dir.join(to), + _ => to, + }; + } + _ => return Ok(p), + } + } + Err(io::Error::new(io::ErrorKind::InvalidInput, format!("{}: too many levels of symbolic links", path.display()))) +} + +/// A rename replaces a file the user can't write (Unix only needs the folder to be writable); +/// refuse, as writing it in place would. Opening for writing without truncating changes nothing. +#[cfg(unix)] +fn check_writable(target: &Path) -> io::Result<()> { + OpenOptions::new().write(true).open(target).map(drop) +} + +/// Windows refuses to rename over a read-only file by itself. +#[cfg(not(unix))] +fn check_writable(_: &Path) -> io::Result<()> { + Ok(()) +} + +/// The replacement keeps the replaced file's mode (Unix; Windows files keep only a read-only flag, +/// and a read-only file can't be replaced). +#[cfg(unix)] +fn keep_permissions(tmp: &File, replaced: &std::fs::Metadata) -> io::Result<()> { + tmp.set_permissions(replaced.permissions()) +} + +#[cfg(not(unix))] +fn keep_permissions(_: &File, _: &std::fs::Metadata) -> io::Result<()> { + Ok(()) +} + +/// Run `rename` up to `tries` times, pausing with a doubling delay while it fails with +/// `PermissionDenied` (a scanner holding the file on Windows). Other errors return at once. +fn retry_rename(tries: u32, mut rename: impl FnMut() -> io::Result<()>) -> io::Result<()> { + let mut delay = RENAME_FIRST_DELAY_MS; + let mut left = tries.max(1); + loop { + left = left.saturating_sub(1); + match rename() { + Err(e) if e.kind() == io::ErrorKind::PermissionDenied && left > 0 => { + std::thread::sleep(std::time::Duration::from_millis(delay)); + delay = delay.saturating_mul(2).min(RENAME_MAX_DELAY_MS); + } + r => return r, + } + } +} + +/// Flush a folder's entries (Unix). Rust's portable file API cannot open a directory on Windows; +/// this does not establish power-loss durability for its directory entries. +#[cfg(unix)] +fn sync_dir(dir: &Path) -> io::Result<()> { + File::open(dir)?.sync_all() +} + +#[cfg(not(unix))] +fn sync_dir(_: &Path) -> io::Result<()> { + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn dir(name: &str) -> PathBuf { + let d = std::env::temp_dir().join(format!("dc-atomic-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d + } + + /// The names in `d`, sorted. + fn names(d: &Path) -> Vec { + let mut v: Vec = std::fs::read_dir(d).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect(); + v.sort(); + v + } + + #[test] + fn replaces_and_creates_without_leftovers() { + let d = dir("replace"); + let path = d.join("doc.designcraft"); + write_atomic(&path, b"first").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"first"); + write_atomic(&path, b"second, longer").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"second, longer"); + write_atomic(&path, b"3").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"3"); + assert_eq!(names(&d), ["doc.designcraft"], "no temporary file is left"); + let _ = std::fs::remove_dir_all(&d); + } + + #[test] + fn a_failed_write_leaves_the_original_and_no_temporary_file() { + let d = dir("fail"); + let path = d.join("doc.designcraft"); + std::fs::write(&path, b"original").unwrap(); + let r = write_atomic_with(&path, |f| { + f.write_all(b"half a docu")?; + Err(io::Error::other("disk full")) + }); + assert_eq!(r.unwrap_err().to_string(), "disk full"); + assert_eq!(std::fs::read(&path).unwrap(), b"original"); + assert_eq!(names(&d), ["doc.designcraft"], "the temporary file is removed"); + // A folder that doesn't exist: an error, nothing written. + assert!(write_atomic(&d.join("missing").join("x.designcraft"), b"x").is_err()); + assert_eq!(names(&d), ["doc.designcraft"]); + // A folder in the way: an error, the folder stays and nothing is left beside it. + std::fs::create_dir(d.join("folder")).unwrap(); + assert!(write_atomic(&d.join("folder"), b"x").is_err()); + assert_eq!(names(&d), ["doc.designcraft", "folder"]); + let _ = std::fs::remove_dir_all(&d); + } + + #[test] + fn a_long_destination_name_does_not_lengthen_the_temporary_name() { + let d = dir("long-name"); + let name = format!("{}.designcraft", "a".repeat(235)); + let path = d.join(&name); + std::fs::write(&path, b"old").unwrap(); + write_atomic(&path, b"new").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"new"); + assert_eq!(names(&d), [name]); + let _ = std::fs::remove_dir_all(&d); + } + + #[cfg(unix)] + #[test] + fn private_permissions_are_preserved_while_the_file_is_written() { + use std::os::unix::fs::PermissionsExt as _; + let d = dir("private-staging"); + let path = d.join("private.designcraft"); + std::fs::write(&path, b"old").unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + write_atomic_with(&path, |file| { + assert_eq!(file.metadata()?.permissions().mode() & 0o777, 0o600); + file.write_all(b"private document")?; + assert_eq!(file.metadata()?.permissions().mode() & 0o777, 0o600); + Ok(()) + }) + .unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"private document"); + assert_eq!(std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o600); + assert_eq!(names(&d), ["private.designcraft"]); + let _ = std::fs::remove_dir_all(&d); + } + + #[test] + fn an_unwinding_writer_removes_its_partial_temporary_file() { + let d = dir("unwinding-writer"); + let path = d.join("private.designcraft"); + std::fs::write(&path, b"old").unwrap(); + let result = std::panic::catch_unwind(|| { + write_atomic_with(&path, |file| { + file.write_all(b"partial private document")?; + panic!("synthetic writer panic"); + }) + }); + assert!(result.is_err()); + assert_eq!(std::fs::read(&path).unwrap(), b"old"); + assert_eq!(names(&d), ["private.designcraft"]); + let _ = std::fs::remove_dir_all(&d); + } + + #[test] + fn the_rename_is_retried_only_while_access_is_denied() { + let mut calls = 0; + let r = retry_rename(3, || { + calls += 1; + if calls < 3 { Err(io::Error::from(io::ErrorKind::PermissionDenied)) } else { Ok(()) } + }); + assert!(r.is_ok() && calls == 3); + calls = 0; + let r = retry_rename(3, || { + calls += 1; + Err(io::Error::from(io::ErrorKind::PermissionDenied)) + }); + assert_eq!(r.map_err(|e| e.kind()), Err(io::ErrorKind::PermissionDenied)); + assert_eq!(calls, 3, "gives up after the last try"); + calls = 0; + let r = retry_rename(3, || { + calls += 1; + Err(io::Error::from(io::ErrorKind::CrossesDevices)) + }); + assert!(r.is_err() && calls == 1, "other errors are not retried"); + } + + #[cfg(unix)] + #[test] + fn permissions_are_kept_and_unwritable_files_are_not_replaced() { + use std::os::unix::fs::PermissionsExt as _; + let d = dir("perms"); + let path = d.join("private.designcraft"); + std::fs::write(&path, b"old").unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + write_atomic(&path, b"new").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"new"); + assert_eq!(std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o600); + // A read-only file stays as it is (root may write it anyway). + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o400)).unwrap(); + if OpenOptions::new().write(true).open(&path).is_err() { + assert!(write_atomic(&path, b"newer").is_err()); + assert_eq!(std::fs::read(&path).unwrap(), b"new"); + assert_eq!(names(&d), ["private.designcraft"]); + } + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + let _ = std::fs::remove_dir_all(&d); + } + + /// A folder the user can't write: the save fails and the original stays. + #[cfg(unix)] + #[test] + fn an_unwritable_folder_fails_without_touching_the_file() { + use std::os::unix::fs::PermissionsExt as _; + let d = dir("ro-dir"); + let path = d.join("doc.designcraft"); + std::fs::write(&path, b"original").unwrap(); + std::fs::set_permissions(&d, std::fs::Permissions::from_mode(0o555)).unwrap(); + // Root writes anyway; the check only means something for an ordinary user. + let probe = d.join("probe"); + let unwritable = std::fs::write(&probe, b"").is_err(); + if unwritable { + assert!(write_atomic(&path, b"new").is_err()); + assert_eq!(std::fs::read(&path).unwrap(), b"original"); + } + std::fs::set_permissions(&d, std::fs::Permissions::from_mode(0o755)).unwrap(); + let _ = std::fs::remove_file(&probe); + assert_eq!(names(&d), ["doc.designcraft"]); + let _ = std::fs::remove_dir_all(&d); + } + + #[cfg(unix)] + #[test] + fn a_symbolic_link_keeps_pointing_at_the_replaced_file() { + let d = dir("link"); + std::fs::create_dir(d.join("real")).unwrap(); + let target = d.join("real").join("doc.designcraft"); + std::fs::write(&target, b"old").unwrap(); + let link = d.join("link.designcraft"); + std::os::unix::fs::symlink("real/doc.designcraft", &link).unwrap(); + write_atomic(&link, b"new").unwrap(); + assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink()); + assert_eq!(std::fs::read(&target).unwrap(), b"new"); + assert_eq!(names(&d), ["link.designcraft", "real"]); + assert_eq!(names(&d.join("real")), ["doc.designcraft"]); + // A loop of links is an error, not a hang. + std::os::unix::fs::symlink("b", d.join("a")).unwrap(); + std::os::unix::fs::symlink("a", d.join("b")).unwrap(); + assert!(write_atomic(&d.join("a"), b"x").is_err()); + let _ = std::fs::remove_dir_all(&d); + } +} diff --git a/crates/engine/src/cmd/book.rs b/crates/engine/src/cmd/book.rs index 9631dac2..1204029c 100644 --- a/crates/engine/src/cmd/book.rs +++ b/crates/engine/src/cmd/book.rs @@ -28,7 +28,7 @@ fn no_book() -> EngineError { #[cfg(not(target_arch = "wasm32"))] fn save(b: &Book) -> Result<()> { let bytes = serde_json::to_vec_pretty(b).map_err(|e| EngineError::Other(e.to_string()))?; - std::fs::write(&b.path, bytes).map_err(|e| EngineError::Other(format!("{}: {e}", b.path))) + crate::atomic::write_file(&b.path, &bytes) } #[cfg(target_arch = "wasm32")] @@ -52,7 +52,7 @@ fn load(path: &str) -> Result { fn store(path: &str, d: &designcraft_doc::Document) -> Result<()> { #[cfg(not(target_arch = "wasm32"))] - return std::fs::write(path, super::file::to_bytes(d)).map_err(|e| EngineError::Other(format!("{path}: {e}"))); + return crate::atomic::write_file(path, &super::file::to_bytes(d)); #[cfg(target_arch = "wasm32")] { let _ = (path, d); @@ -210,7 +210,7 @@ pub fn specs() -> Vec { match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len(), "pages": pages})) } None => Ok(json!({"base64": super::file::base64_encode(&bytes), "bytes": bytes.len(), "pages": pages})), diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index f4786385..8e0f04d6 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -44,7 +44,7 @@ fn print_booklet(s: &mut Session, p: &Value) -> Result { match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, &r.bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, &r.bytes)?; Ok(json!({"path": path, "bytes": r.bytes.len(), "sheets": r.pages, "warnings": r.warnings})) } None => Ok(json!({"base64": super::file::base64_encode(&r.bytes), "bytes": r.bytes.len(), "sheets": r.pages, "warnings": r.warnings})), @@ -59,7 +59,7 @@ fn export_html(s: &mut Session, p: &Value) -> Result { match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, text.as_bytes())?; Ok(json!({"path": path, "bytes": text.len()})) } None => Ok(json!({"bytes": text.len(), "text": text})), @@ -91,7 +91,7 @@ fn export_text(s: &mut Session, p: &Value) -> Result { match path { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, text.as_bytes())?; Ok(json!({"path": path, "bytes": text.len()})) } None => Ok(json!({"bytes": text.len(), "text": text})), @@ -160,7 +160,7 @@ fn export_epub(s: &mut Session, p: &Value) -> Result { match p.get("path").and_then(Value::as_str) { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, &bytes).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, &bytes)?; Ok(serde_json::json!({"path": path, "bytes": bytes.len()})) } None => Ok(serde_json::json!({"base64": super::file::base64_encode(&bytes), "bytes": bytes.len()})), @@ -598,7 +598,7 @@ fn export_pdf(s: &mut Session, p: &Value) -> Result { #[cfg(not(target_arch = "wasm32"))] fn write_file(path: &str, bytes: &[u8]) -> Result<()> { - std::fs::write(path, bytes).map_err(|e| EngineError::Other(format!("{path}: {e}"))) + crate::atomic::write_file(path, bytes) } #[cfg(target_arch = "wasm32")] fn write_file(path: &str, _bytes: &[u8]) -> Result<()> { diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index ed126eee..b5887bb5 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -27,7 +27,7 @@ pub fn specs() -> Vec { let path = str_param(p, "path").ok_or_else(|| bad("file.saveACopy", "missing `path`"))?.to_string(); let bytes = to_bytes(&s.doc()?.doc); #[cfg(not(target_arch = "wasm32"))] - std::fs::write(&path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(&path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len()})) }), cmd!(noundo "file.revert", "Revert", ["File"], None, "{} — back to the last saved version (not undoable)", can_revert, file_revert), @@ -247,7 +247,7 @@ fn file_save(s: &mut Session, p: &Value) -> Result { } let bytes = to_bytes(&st.doc); #[cfg(not(target_arch = "wasm32"))] - std::fs::write(&path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(&path, &bytes)?; st.path = Some(path.clone()); st.saved_revision = st.revision; st.saved_doc = st.doc.clone(); @@ -518,7 +518,7 @@ fn snippet_export(s: &mut Session, p: &Value) -> Result { match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len(), "items": d.spreads.first().map(|s| s.items.len()).unwrap_or(0)})) } None => Ok(json!({"base64": base64_encode(&bytes), "bytes": bytes.len()})), @@ -735,6 +735,37 @@ mod layout_place_tests { } } +#[cfg(test)] +mod save_tests { + use serde_json::json; + + use crate::Session; + + /// Saving over an existing document replaces it whole and leaves nothing beside it. + #[test] + fn save_replaces_the_existing_file_without_leftovers() { + let dir = std::env::temp_dir().join(format!("dc-save-atomic-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("doc.designcraft"); + std::fs::write(&path, vec![b'x'; 1 << 20]).unwrap(); + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": "Saved whole", "caret": false})).unwrap(); + s.execute("file.save", &json!({"path": path.to_string_lossy()})).unwrap(); + s.execute("file.saveACopy", &json!({"path": path.to_string_lossy()})).unwrap(); + let saved = super::from_bytes(&std::fs::read(&path).unwrap()).unwrap(); + assert!(saved.stories.values().any(|st| st.text == "Saved whole")); + let names: Vec<_> = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().file_name()).collect(); + assert_eq!(names, ["doc.designcraft"], "no temporary file is left"); + // A folder that doesn't exist: an error naming the path. + let missing = dir.join("missing").join("doc.designcraft"); + let e = s.execute("file.save", &json!({"path": missing.to_string_lossy()})).unwrap_err(); + assert!(e.to_string().contains("missing"), "{e}"); + let _ = std::fs::remove_dir_all(&dir); + } +} + #[cfg(test)] mod document_fonts_tests { use std::path::{Path, PathBuf}; diff --git a/crates/engine/src/cmd/interchange.rs b/crates/engine/src/cmd/interchange.rs index a0af09f5..96498e86 100644 --- a/crates/engine/src/cmd/interchange.rs +++ b/crates/engine/src/cmd/interchange.rs @@ -25,7 +25,7 @@ fn export_idml(s: &mut Session, p: &Value) -> Result { match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, &bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len()})) } None => Ok(json!({"base64": base64_encode(&bytes), "bytes": bytes.len()})), diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index f0e77af5..00855dd0 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -29,7 +29,7 @@ impl Library { #[cfg(not(target_arch = "wasm32"))] if let Some(p) = &self.path { let bytes = serde_json::to_vec_pretty(self).map_err(|e| crate::EngineError::Other(e.to_string()))?; - std::fs::write(p, bytes).map_err(|e| crate::EngineError::Other(format!("{p}: {e}")))?; + crate::atomic::write_file(p, &bytes)?; } Ok(()) } diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index 8a520b68..4bf69ec4 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -844,7 +844,11 @@ fn save_swatches(s: &mut Session, p: &Value) -> Result { let bytes = designcraft_color::ase::write(&chosen); match str_param(p, "path") { Some(path) => { - std::fs::write(path, &bytes).map_err(|e| bad("swatch.save", format!("{path}: {e}")))?; + #[cfg(not(target_arch = "wasm32"))] + let written = crate::atomic::write_atomic(std::path::Path::new(path), &bytes); + #[cfg(target_arch = "wasm32")] + let written = std::fs::write(path, &bytes); + written.map_err(|e| bad("swatch.save", format!("{path}: {e}")))?; Ok(json!({"path": path, "bytes": bytes.len()})) } None => Ok(json!({"base64": super::file::base64_encode(&bytes)})), diff --git a/crates/engine/src/cmd/xml.rs b/crates/engine/src/cmd/xml.rs index 32252a9d..6dab205f 100644 --- a/crates/engine/src/cmd/xml.rs +++ b/crates/engine/src/cmd/xml.rs @@ -305,7 +305,7 @@ pub fn specs() -> Vec { match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] - std::fs::write(path, text.as_bytes()).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))?; + crate::atomic::write_file(path, text.as_bytes())?; Ok(json!({"path": path, "bytes": text.len()})) } None => Ok(json!({"text": text, "bytes": text.len()})), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 771c89b1..16471930 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -7,6 +7,8 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![forbid(unsafe_code)] +#[cfg(not(target_arch = "wasm32"))] +pub mod atomic; pub mod cmd; pub mod dtd; pub mod guard; diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index aea3e876..eea81ddd 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -201,7 +201,7 @@ impl Headless { let png = img.to_png(); match s(p, "path") { Some(path) => { - std::fs::write(path, &png).map_err(|e| format!("write {path}: {e}"))?; + designcraft_engine::atomic::write_atomic(std::path::Path::new(path), &png).map_err(|e| format!("write {path}: {e}"))?; Ok(json!({"path": path, "width": img.width, "height": img.height})) } None => Ok(json!({"width": img.width, "height": img.height, "pngBase64": designcraft_engine::cmd::base64_encode(&png)})), @@ -221,7 +221,7 @@ impl Headless { .ok_or_else(|| format!("no page {page}"))?; let lower = path.to_ascii_lowercase(); let bytes = if lower.ends_with(".jpg") || lower.ends_with(".jpeg") { img.to_jpeg(90) } else { img.to_png() }; - std::fs::write(path, &bytes).map_err(|e| format!("write {path}: {e}"))?; + designcraft_engine::atomic::write_atomic(std::path::Path::new(path), &bytes).map_err(|e| format!("write {path}: {e}"))?; Ok(json!({"path": path, "width": img.width, "height": img.height})) } } diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 802f615d..c51bac8e 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -479,7 +479,7 @@ fn render_page(b: &mut dyn Backend, a: &Args) -> Result { let path = a.get("path").and_then(Value::as_str); if let Some(path) = path { let png = designcraft_engine::cmd::base64_decode(&b64); - std::fs::write(path, png).map_err(|e| format!("write {path}: {e}"))?; + designcraft_engine::atomic::write_atomic(std::path::Path::new(path), &png).map_err(|e| format!("write {path}: {e}"))?; } Ok(ToolResult::image(b64, &json!({"width": r.get("width"), "height": r.get("height"), "path": path}))) } diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index e9c27a9f..6c033d70 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -288,6 +288,7 @@ It is evidence of breadth: each item exists. It is not a claim of InDesign-level - Missing fonts: highlighted pink on screen (never in output), listed by Preflight; Type › Find/Replace Font (fonts in the document, missing first; replace in text, cells, footnotes and styles); `font.list` / `font.replace`. - 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. - Crash recovery: unsaved documents are written to the recovery folder every 30 s and reopened (unsaved, remembering their files) after a crash; File › Revert and Save a Copy…; `file.recovery.*`, `file.revert`, `file.saveACopy`. - 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. - Edit › Paste Into (copied objects become a frame's content, clipped by it; rendered, PDF and IDML both ways — verified in InDesign 2026); Polygon Settings (sides, star inset; double-click the Polygon tool). From 328b4d5a52f095a49b946a1e9189a65ba08dde79 Mon Sep 17 00:00:00 2001 From: Noah Scholz <48360809+noah1400@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:26:34 +0200 Subject: [PATCH 4/5] fix(recovery): isolate recovery data between app instances Keep each app's recovery data in an owned run folder. Validate current folder and lock ownership and disable recovery when isolation fails instead of using the shared root. Fixes #496. Co-authored-by: noah1400 <48360809+noah1400@users.noreply.github.com> Co-authored-by: skylar taylor-barrick --- ROADMAP.md | 3 +- apps/designcraft/src/profile.rs | 39 +- crates/engine/src/recovery.rs | 641 +++++++++++++++++++++++++++++++- docs/target-app-parity.md | 2 +- 4 files changed, 662 insertions(+), 23 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 6045a000..15c9af8d 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 (atomic engine saves and exports) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (isolated recovery for each running app) · **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** · 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. - **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. - **2026-10-11** · `transform.move {copy}`, Edit › Duplicate and Step and Repeat return the copies' ids, so a script can go on with them. No readiness percentage change. diff --git a/apps/designcraft/src/profile.rs b/apps/designcraft/src/profile.rs index c3da12b0..3e90879e 100644 --- a/apps/designcraft/src/profile.rs +++ b/apps/designcraft/src/profile.rs @@ -22,7 +22,19 @@ fn config_dir_with(os: &str, env: impl Fn(&str) -> Option) -> Option

Option { - recovery_dir_with(std::env::var_os("DESIGNCRAFT_CONFIG_DIR"), designcraft_engine::recovery::default_dir) + recovery_dir_with(std::env::var_os("DESIGNCRAFT_CONFIG_DIR"), designcraft_engine::recovery::default_dir).and_then(own_run) +} + +/// This run's folder in the recovery folder `root`. A second running DesignCraft has its own, so +/// neither writes over the other's documents nor takes them for a crash's. +fn own_run(root: PathBuf) -> Option { + match designcraft_engine::recovery::run_dir(&root) { + Ok(dir) => Some(dir), + Err(e) => { + log::warn!("recovery is unavailable because this run cannot be isolated: {e}"); + None + } + } } fn recovery_dir_with(root: Option, default: impl FnOnce() -> Option) -> Option { @@ -68,4 +80,29 @@ mod tests { assert_eq!(config_dir_with("linux", |key| if key == "HOME" { environment(key) } else { None }), Some("/user/.config/designcraft".into())); assert_eq!(recovery_dir_with(None, || Some("/existing/recovery".into())), Some("/existing/recovery".into())); } + + #[test] + fn a_run_has_its_own_recovery_folder_when_one_can_be_made() { + let dir = std::env::temp_dir().join(format!("designcraft-profile-run-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let root = dir.join("Recovery"); + assert_eq!(own_run(root.clone()).as_deref().and_then(|dir| dir.parent()), Some(root.as_path())); + // Failure must not fall back to the shared root. + let blocked = dir.join("blocked"); + std::fs::write(&blocked, "a file where the folder should be").unwrap(); + assert_eq!(own_run(blocked.join("Recovery")), None, "isolation failure must not select the shared root"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn an_exhausted_run_namespace_never_falls_back_to_shared_recovery() { + let root = std::env::temp_dir().join(format!("designcraft-profile-exhausted-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + for n in 0..64 { + std::fs::create_dir(root.join(format!("run-{}-{n}", std::process::id()))).unwrap(); + } + assert_eq!(own_run(root.clone()), None, "a shared root would restore inter-process UID collisions"); + let _ = std::fs::remove_dir_all(&root); + } } diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index a038c2d2..2ab67ce4 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -6,6 +6,11 @@ //! A recovery entry is `.designcraft` (the document) plus `.json` (`{"path", "title", //! "saved"}`: the original file, the title, and when it was written). //! +//! Document uids start at 1 in every process, so each running app keeps its entries in a folder +//! of its own in the recovery folder (`run_dir`) and holds the lock in it for as long as it runs. +//! An app that starts moves into its own folder what apps that have ended left in theirs, and +//! leaves the folder of one still running alone. +//! //! Each file is replaced atomically, but the pair is not a transaction. The document is published //! and made durable before its metadata so a crash can leave metadata missing or stale, never a //! listed metadata record without a complete primary document. Listing and opening recovery data @@ -31,6 +36,295 @@ pub fn default_dir() -> Option { std::env::var_os("XDG_DATA_HOME").map(PathBuf::from).or_else(|| home.map(|h| h.join(".local/share"))).map(|d| d.join("designcraft/recovery")) } +/// This process's folder in a recovery folder, with the lock that says it is still running. +#[cfg(not(target_arch = "wasm32"))] +struct Run { + root: PathBuf, + dir: PathBuf, + lock: std::fs::File, + #[cfg(unix)] + directory: std::fs::File, +} + +#[cfg(not(target_arch = "wasm32"))] +static RUNS: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); + +/// The lock in a run's folder. +#[cfg(not(target_arch = "wasm32"))] +const LOCK: &str = "lock"; + +/// This run's own folder in the recovery folder `root`, made on the first call. Document uids +/// start at 1 in every process, so two running apps writing to one folder wrote over each +/// other's entries. The folder's lock is held until the process ends: an app that starts leaves +/// the folder of one that is still running alone. +#[cfg(not(target_arch = "wasm32"))] +pub fn run_dir(root: &Path) -> Result { + let mut runs = RUNS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(run) = runs.iter().find(|r| r.root == root) { + return Ok(run.dir.clone()); + } + secure_recovery_dir(root)?; + for n in 0..64 { + let dir = root.join(format!("run-{}-{n}", std::process::id())); + // Only a folder this run makes is its own. One that is there is another app's, or was + // left by a run that ended under this process id. + #[cfg(unix)] + let made = { + use std::os::unix::fs::DirBuilderExt; + + let mut builder = std::fs::DirBuilder::new(); + builder.mode(0o700).create(&dir) + }; + #[cfg(not(unix))] + let made = std::fs::create_dir(&dir); + match made { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(e) => return Err(EngineError::Other(format!("{}: {e}", dir.display()))), + } + let locked = validate_recovery_dir(&dir).and_then(|()| { + let lock = new_lock(&dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + match lock.try_lock() { + Ok(()) => { + validate_lock(&dir, &lock)?; + Ok(Some(lock)) + } + Err(std::fs::TryLockError::WouldBlock) => Ok(None), + Err(std::fs::TryLockError::Error(e)) => Err(EngineError::Other(format!("{}: {e}", dir.display()))), + } + }); + match locked { + // An app that is starting can take a folder this new for one a run left, and remove + // it before the lock is held: then the lock is gone, or that app holds it. + Ok(Some(lock)) => { + #[cfg(unix)] + let directory = std::fs::File::open(&dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + runs.push(Run { + root: root.to_path_buf(), + dir: dir.clone(), + lock, + #[cfg(unix)] + directory, + }); + return Ok(dir); + } + Ok(_) => {} + Err(e) => { + // Never unlink a lock that another starting run may now own. + let _ = std::fs::remove_dir(&dir); + return Err(e); + } + } + } + Err(EngineError::Other(format!("{}: no free folder for this run", root.display()))) +} + +#[cfg(not(target_arch = "wasm32"))] +fn new_lock(dir: &Path) -> std::io::Result { + let mut new = std::fs::OpenOptions::new(); + new.read(true).write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + new.mode(0o600); + } + let mut file = new.open(dir.join(LOCK))?; + // A marker distinguishes the held handle from a replacement at the same pathname on + // platforms without portable file-identity metadata. Empty older-version locks remain valid. + static NEXT_LOCK: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_err(std::io::Error::other)?.as_nanos(); + let marker = format!("{}:{}:{now}", std::process::id(), NEXT_LOCK.fetch_add(1, std::sync::atomic::Ordering::Relaxed)); + file.write_all(marker.as_bytes())?; + file.sync_all()?; + Ok(file) +} + +/// Check the current lock name without accepting a symlink or a lock held through an unlinked +/// handle. An advisory lock on an old handle does not protect a replacement at the same name. +#[cfg(not(target_arch = "wasm32"))] +fn validate_lock(dir: &Path, lock: &std::fs::File) -> Result<()> { + let path = dir.join(LOCK); + let fail = || EngineError::Other(format!("{}: this run no longer owns its recovery lock", path.display())); + let before = std::fs::symlink_metadata(&path).map_err(|_| fail())?; + if is_link_like(&before) || !before.file_type().is_file() { + return Err(fail()); + } + let named = std::fs::File::open(&path).map_err(|_| fail())?; + let held = lock.metadata().map_err(|_| fail())?; + let current = named.metadata().map_err(|_| fail())?; + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if held.dev() != before.dev() || held.ino() != before.ino() || held.dev() != current.dev() || held.ino() != current.ino() { + return Err(fail()); + } + } + #[cfg(not(unix))] + let _ = (held, current); + if lock_marker(lock).map_err(|_| fail())? != lock_marker(&named).map_err(|_| fail())? { + return Err(fail()); + } + let after = std::fs::symlink_metadata(&path).map_err(|_| fail())?; + if is_link_like(&after) || !after.file_type().is_file() { + return Err(fail()); + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if held.dev() != after.dev() || held.ino() != after.ino() { + return Err(fail()); + } + } + Ok(()) +} + +#[cfg(not(target_arch = "wasm32"))] +fn lock_marker(file: &std::fs::File) -> std::io::Result> { + use std::io::{Read, Seek}; + let mut file = file.try_clone()?; + file.seek(std::io::SeekFrom::Start(0))?; + let mut bytes = Vec::new(); + file.take(129).read_to_end(&mut bytes)?; + if bytes.len() > 128 { + return Err(std::io::Error::other("invalid recovery lock marker")); + } + Ok(bytes) +} + +/// A run's folder that was removed while it runs is made again by the next save: so is its lock. +#[cfg(not(target_arch = "wasm32"))] +fn relock(dir: &Path, created: bool) -> Result<()> { + let mut runs = RUNS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let Some(run) = runs.iter_mut().find(|r| r.dir == dir) else { return Ok(()) }; + validate_recovery_dir(&run.root)?; + #[cfg(unix)] + let directory = std::fs::symlink_metadata(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + // Reclaim a newly recreated folder only when this operation actually created it. Existing + // replacements, including ones without a lock, may contain somebody else's documents. + let same_directory = { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let held = run.directory.metadata().map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + held.dev() == directory.dev() && held.ino() == directory.ino() + } + #[cfg(not(unix))] + { + // Without stable directory identity, a valid held lock is the proof of ownership. + // A missing lock cannot authorize an existing folder, even on coarse-timestamp disks. + validate_lock(dir, &run.lock).is_ok() + } + }; + if !created && !same_directory { + return Err(EngineError::Other(format!("{}: this run no longer owns its recovery folder", dir.display()))); + } + match std::fs::symlink_metadata(dir.join(LOCK)) { + Ok(_) => return validate_lock(dir, &run.lock), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(EngineError::Other(format!("{}: {e}", dir.display()))), + } + let lock = new_lock(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + lock.try_lock().map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + validate_lock(dir, &lock)?; + #[cfg(unix)] + let replacement_directory = std::fs::File::open(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let held = replacement_directory.metadata().map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + if held.dev() != directory.dev() || held.ino() != directory.ino() { + return Err(EngineError::Other(format!("{}: recovery folder changed while restoring its lock", dir.display()))); + } + } + validate_lock(dir, &lock)?; + run.lock = lock; + #[cfg(unix)] + { + run.directory = replacement_directory; + } + Ok(()) +} + +/// Move into `own`, this run's folder, what is left to recover beside it: the entries of runs +/// that have ended, and those written straight into the recovery folder before runs had folders. +#[cfg(not(target_arch = "wasm32"))] +fn take_over(own: &Path) { + let root = { + let runs = RUNS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let Some(run) = runs.iter().find(|r| r.dir == own) else { return }; + run.root.clone() + }; + move_entries(&root, own); + let Ok(rd) = std::fs::read_dir(&root) else { return }; + for e in rd.flatten() { + let dir = e.path(); + if dir == own || !e.file_name().to_string_lossy().starts_with("run-") || validate_recovery_dir(&dir).is_err() { + continue; + } + // Opened for writing: over NFS an exclusive lock on a file opened for reading is refused. + let Ok(metadata) = std::fs::symlink_metadata(dir.join(LOCK)) else { continue }; + if is_link_like(&metadata) || !metadata.file_type().is_file() { + continue; + } + let Ok(lock) = std::fs::OpenOptions::new().read(true).write(true).open(dir.join(LOCK)) else { continue }; + match lock.try_lock() { + // Free: the run has ended. Holding it keeps another starting app out. + Ok(()) => {} + Err(std::fs::TryLockError::WouldBlock) => continue, + Err(std::fs::TryLockError::Error(e)) => { + log::warn!("recovery data in {} is left there: {e}", dir.display()); + continue; + } + } + if let Err(e) = validate_lock(&dir, &lock) { + log::warn!("recovery data in {} is left there: {e}", dir.display()); + continue; + } + move_entries(&dir, own); + remove_emptied(&dir); + } +} + +/// Remove the folder of a run that ended, with what the app left in it beside documents. A +/// document still in it, or a file the app did not write, keeps the folder as it is. +#[cfg(not(target_arch = "wasm32"))] +fn remove_emptied(dir: &Path) { + let Ok(rd) = std::fs::read_dir(dir) else { return }; + let mut left = Vec::new(); + for e in rd { + let Ok(e) = e else { return }; + let path = e.path(); + let name = e.file_name(); + let name = name.to_string_lossy(); + let numbered = path.file_stem().and_then(|s| s.to_str()).is_some_and(|s| s.parse::().is_ok()); + // Metadata whose document has gone, and a file a crash left half written. + if (numbered && name.ends_with(".json")) || (name.starts_with(".designcraft-recovery-") && name.ends_with(".tmp")) { + left.push(path); + } else if name != LOCK { + return; + } + } + for path in left { + let _ = std::fs::remove_file(path); + } + // The lock last: until then the folder is still that of a run that ended. + let _ = std::fs::remove_file(dir.join(LOCK)); + let _ = std::fs::remove_dir(dir); +} + +#[cfg(not(target_arch = "wasm32"))] +fn move_entries(from: &Path, own: &Path) { + for (uid, _) in list(from) { + let (doc, meta) = files(from, uid); + // Under a uid no document of this run has or gets. + let (own_doc, own_meta) = files(own, crate::NEXT_UID.fetch_add(1, std::sync::atomic::Ordering::Relaxed)); + // The document first: its metadata is optional. + if std::fs::rename(&doc, &own_doc).is_ok() { + let _ = std::fs::rename(&meta, &own_meta); + } + } +} + fn files(dir: &Path, uid: u64) -> (PathBuf, PathBuf) { (dir.join(format!("{uid}.designcraft")), dir.join(format!("{uid}.json"))) } @@ -38,7 +332,7 @@ fn files(dir: &Path, uid: u64) -> (PathBuf, PathBuf) { /// Remove a document's recovery entry (it was saved or closed). pub fn discard(dir: &Path, uid: u64) { #[cfg(not(target_arch = "wasm32"))] - if validate_recovery_dir(dir).is_err() { + if validate_recovery_dir(dir).is_err() || relock(dir, false).is_err() { return; } let (d, m) = files(dir, uid); @@ -49,7 +343,11 @@ pub fn discard(dir: &Path, uid: u64) { /// Write recovery entries for the dirty documents (and drop those of clean ones). Returns how /// many were written. pub fn save(s: &Session, dir: &Path) -> Result { - secure_recovery_dir(dir)?; + let created = secure_recovery_dir(dir)?; + #[cfg(not(target_arch = "wasm32"))] + relock(dir, created)?; + #[cfg(target_arch = "wasm32")] + let _ = created; let mut n = 0; for d in &s.docs { if !d.is_dirty() { @@ -68,35 +366,45 @@ pub fn save(s: &Session, dir: &Path) -> Result { } #[cfg(not(target_arch = "wasm32"))] -fn secure_recovery_dir(dir: &Path) -> Result<()> { +fn secure_recovery_dir(dir: &Path) -> Result { match std::fs::symlink_metadata(dir) { - Ok(_) => return validate_recovery_dir(dir), + Ok(_) => return validate_recovery_dir(dir).map(|()| false), Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} Err(e) => return Err(EngineError::Other(format!("{}: {e}", dir.display()))), } - // Set the mode while creating directories so existing caller-owned directories are never - // chmodded. DirBuilder applies this mode only to components it creates; the process umask may - // make it more restrictive. + // Create parent directories privately without changing existing caller-owned permissions. + // Create the leaf separately so a creation race can never be mistaken for our own folder. #[cfg(unix)] let create_result = { use std::os::unix::fs::DirBuilderExt; - - let mut builder = std::fs::DirBuilder::new(); - builder.recursive(true).mode(0o700).create(dir) + if let Some(parent) = dir.parent().filter(|p| !p.as_os_str().is_empty()) { + std::fs::DirBuilder::new() + .recursive(true) + .mode(0o700) + .create(parent) + .map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + } + std::fs::DirBuilder::new().mode(0o700).create(dir) }; #[cfg(not(unix))] - let create_result = std::fs::create_dir_all(dir); - - create_result.map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; - - // Re-check after creation. This also fails closed if another process won a creation race with - // a symlink or, on Windows, any reparse-point leaf. - validate_recovery_dir(dir) + let create_result = { + if let Some(parent) = dir.parent().filter(|p| !p.as_os_str().is_empty()) { + std::fs::create_dir_all(parent).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + } + std::fs::create_dir(dir) + }; + let created = match create_result { + Ok(()) => true, + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => false, + Err(e) => return Err(EngineError::Other(format!("{}: {e}", dir.display()))), + }; + validate_recovery_dir(dir)?; + Ok(created) } #[cfg(target_arch = "wasm32")] -fn secure_recovery_dir(_dir: &Path) -> Result<()> { +fn secure_recovery_dir(_dir: &Path) -> Result { Err(EngineError::Other("recovery files are unavailable on the web".into())) } @@ -185,7 +493,7 @@ fn write_recovery_file(_path: &Path, _bytes: &[u8]) -> Result<()> { pub fn list(dir: &Path) -> Vec<(u64, Value)> { let mut out = Vec::new(); #[cfg(not(target_arch = "wasm32"))] - if validate_recovery_dir(dir).is_err() { + if validate_recovery_dir(dir).is_err() || relock(dir, false).is_err() { return out; } let Ok(rd) = std::fs::read_dir(dir) else { return out }; @@ -211,8 +519,14 @@ pub fn list(dir: &Path) -> Vec<(u64, Value)> { /// writes its own). Returns the opened documents' indices. pub fn open(s: &mut Session, dir: &Path) -> Result> { #[cfg(not(target_arch = "wasm32"))] - validate_recovery_dir(dir)?; + { + validate_recovery_dir(dir)?; + relock(dir, false)?; + } let mut opened = Vec::new(); + // For this run's own folder, first bring in what runs that ended left in theirs. + #[cfg(not(target_arch = "wasm32"))] + take_over(dir); for (uid, meta) in list(dir) { #[cfg(not(target_arch = "wasm32"))] validate_recovery_dir(dir)?; @@ -350,6 +664,293 @@ mod tests { assert!(open(&mut Session::new(), &link).is_err()); assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink()); } + + #[test] + fn a_run_has_a_locked_folder_of_its_own() { + let root = tempfile::tempdir().unwrap(); + let dir = root.path().join("recovery"); + let own = run_dir(&dir).unwrap(); + assert_eq!(own.parent(), Some(dir.as_path())); + assert_eq!(run_dir(&dir).unwrap(), own, "one for the whole run"); + let lock = std::fs::File::open(own.join(LOCK)).unwrap(); + assert!(matches!(lock.try_lock(), Err(std::fs::TryLockError::WouldBlock)), "held while the run lives"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(std::fs::metadata(&own).unwrap().permissions().mode() & 0o777, 0o700); + assert_eq!(std::fs::metadata(own.join(LOCK)).unwrap().permissions().mode() & 0o777, 0o600); + } + } + + /// The recovery folder was cleared while the app runs: the next save makes the run's folder + /// again, and its lock with it. + #[cfg(unix)] + #[test] + fn a_removed_run_folder_is_made_again_with_its_lock() { + let root = tempfile::tempdir().unwrap(); + let own = run_dir(root.path()).unwrap(); + std::fs::remove_dir_all(&own).unwrap(); + assert_eq!(save(&dirty_session(), &own).unwrap(), 1); + let lock = std::fs::File::open(own.join(LOCK)).unwrap(); + assert!(matches!(lock.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + } + + /// Replacing the name of a live run's lock must not authorize writes under the old handle. + #[cfg(unix)] + #[test] + fn save_rejects_a_run_whose_lock_was_replaced() { + let root = tempfile::tempdir().unwrap(); + let own = run_dir(root.path()).unwrap(); + std::fs::remove_file(own.join(LOCK)).unwrap(); + let replacement = new_lock(&own).unwrap(); + replacement.try_lock().unwrap(); + assert!(save(&dirty_session(), &own).is_err()); + assert!(list(&own).is_empty(), "another owner's folder must not receive our recovery data"); + } + + /// A blocked attempt to recreate a run's lock must stop the save before it writes documents. + #[cfg(unix)] + #[test] + fn save_rejects_a_run_whose_lock_cannot_be_recreated() { + let root = tempfile::tempdir().unwrap(); + let own = run_dir(root.path()).unwrap(); + std::fs::remove_file(own.join(LOCK)).unwrap(); + std::fs::create_dir(own.join(LOCK)).unwrap(); + assert!(save(&dirty_session(), &own).is_err()); + assert!(list(&own).is_empty()); + } + + #[cfg(unix)] + #[test] + fn discarding_a_document_does_not_touch_a_replaced_runs_entries() { + let root = tempfile::tempdir().unwrap(); + let own = run_dir(root.path()).unwrap(); + std::fs::remove_file(own.join(LOCK)).unwrap(); + let replacement = new_lock(&own).unwrap(); + replacement.try_lock().unwrap(); + let (doc, meta) = files(&own, 1); + std::fs::write(&doc, b"the replacement owner's document").unwrap(); + std::fs::write(&meta, b"{}").unwrap(); + discard(&own, 1); + assert_eq!(std::fs::read(doc).unwrap(), b"the replacement owner's document"); + assert!(meta.exists()); + } + + #[cfg(unix)] + #[test] + fn saving_a_run_rejects_a_symlinked_lock() { + let root = tempfile::tempdir().unwrap(); + let own = run_dir(root.path()).unwrap(); + std::fs::remove_file(own.join(LOCK)).unwrap(); + let unrelated = root.path().join("unrelated"); + std::fs::write(&unrelated, b"unrelated file").unwrap(); + std::os::unix::fs::symlink(&unrelated, own.join(LOCK)).unwrap(); + assert!(save(&dirty_session(), &own).is_err()); + assert_eq!(std::fs::read(unrelated).unwrap(), b"unrelated file"); + } + + #[cfg(unix)] + #[test] + fn a_replaced_run_directory_is_not_claimed_or_overwritten() { + let root = tempfile::tempdir().unwrap(); + let own = run_dir(root.path()).unwrap(); + let session = dirty_session(); + let uid = session.docs[0].uid; + std::fs::remove_dir_all(&own).unwrap(); + std::fs::create_dir(&own).unwrap(); + let (doc, _) = files(&own, uid); + std::fs::write(&doc, b"another owner's document").unwrap(); + assert!(save(&session, &own).is_err()); + assert_eq!(std::fs::read(doc).unwrap(), b"another owner's document"); + assert!(!own.join(LOCK).exists()); + } + + #[cfg(unix)] + #[test] + fn a_run_gets_no_folder_in_a_symlinked_recovery_folder() { + use std::os::unix::fs::symlink; + + let root = tempfile::tempdir().unwrap(); + let destination = root.path().join("destination"); + let link = root.path().join("recovery"); + std::fs::create_dir(&destination).unwrap(); + symlink(&destination, &link).unwrap(); + + assert!(run_dir(&link).is_err()); + assert!(std::fs::read_dir(&destination).unwrap().next().is_none()); + } + + fn unsaved(text: &str) -> Session { + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + session.execute("frame.create", &json!({"rect": [10, 10, 100, 100], "content": "text", "text": text})).unwrap(); + session + } + + fn texts(s: &Session) -> Vec { + let mut texts: Vec = s.documents().iter().flat_map(|d| d.doc.stories.values().map(|st| st.text.clone())).collect(); + texts.sort(); + texts + } + + /// What a run that has ended leaves: its folder with its lock, free now, and the entry of + /// one unsaved document, numbered `uid` as that process numbered it. + fn run_left(root: &Path, name: &str, text: &str, uid: u64) -> PathBuf { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join(LOCK), b"").unwrap(); + let mut s = unsaved(text); + s.docs[0].uid = uid; + assert_eq!(save(&s, &dir).unwrap(), 1); + dir + } + + /// Every process numbers its first document 1: two running apps both wrote `1.designcraft` + /// into the one recovery folder, and only the document written last was recovered. + #[test] + fn documents_of_two_apps_that_ended_are_both_recovered() { + let root = tempfile::tempdir().unwrap(); + let first = run_left(root.path(), "run-7-0", "first app", 1); + let second = run_left(root.path(), "run-8-0", "second app", 1); + let own = run_dir(root.path()).unwrap(); + let mut s = Session::new(); + assert_eq!(open(&mut s, &own).unwrap().len(), 2); + assert_eq!(texts(&s), ["first app", "second app"]); + assert!(s.documents().iter().all(DocState::is_dirty)); + assert!(!first.exists() && !second.exists(), "nothing is left of the runs that ended"); + } + + /// An app that started reopened the unsaved documents of one still running, and removed + /// its entries. + #[test] + fn a_running_apps_recovery_data_is_left_alone() { + let root = tempfile::tempdir().unwrap(); + let running = run_left(root.path(), "run-7-0", "still open", 1); + let lock = std::fs::File::open(running.join(LOCK)).unwrap(); + lock.try_lock().unwrap(); + let own = run_dir(root.path()).unwrap(); + let mut s = Session::new(); + assert!(open(&mut s, &own).unwrap().is_empty()); + assert_eq!(list(&running).len(), 1); + // Once it has ended, its document is recovered. + drop(lock); + assert_eq!(open(&mut s, &own).unwrap().len(), 1); + assert_eq!(texts(&s), ["still open"]); + assert!(!running.exists()); + } + + /// A process id comes round again: the folder a run left under it is not this run's. + #[test] + fn a_folder_left_under_this_process_id_is_not_reused() { + let root = tempfile::tempdir().unwrap(); + let left = run_left(root.path(), &format!("run-{}-0", std::process::id()), "left behind", 1); + let own = run_dir(root.path()).unwrap(); + assert_ne!(own, left); + let mut s = Session::new(); + assert_eq!(open(&mut s, &own).unwrap().len(), 1); + assert_eq!(texts(&s), ["left behind"]); + } + + /// Entries written straight into the recovery folder, as before runs had folders. + #[test] + fn recovery_data_of_an_earlier_version_is_recovered() { + let root = tempfile::tempdir().unwrap(); + assert_eq!(save(&unsaved("earlier version"), root.path()).unwrap(), 1); + let own = run_dir(root.path()).unwrap(); + let mut s = Session::new(); + assert_eq!(open(&mut s, &own).unwrap().len(), 1); + assert_eq!(texts(&s), ["earlier version"]); + assert!(list(root.path()).is_empty()); + } + + /// An entry that can't be opened stays on disk, in this run's folder now. + #[test] + fn an_unreadable_entry_of_a_run_that_ended_is_kept() { + let root = tempfile::tempdir().unwrap(); + let left = run_left(root.path(), "run-7-0", "readable", 1); + std::fs::write(left.join("2.designcraft"), b"not a document").unwrap(); + let own = run_dir(root.path()).unwrap(); + let mut s = Session::new(); + assert_eq!(open(&mut s, &own).unwrap().len(), 1); + assert!(list(&own).iter().any(|e| std::fs::read(files(&own, e.0).0).unwrap() == b"not a document")); + assert!(!left.exists()); + } + + /// Only a folder named as a run's, with a lock that is free, is a run that ended. + #[test] + fn what_is_no_run_that_ended_is_passed_over() { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("run-1-0"), b"a file").unwrap(); + // An app that is starting has made its folder and not yet its lock. + let starting = root.path().join("run-2-0"); + std::fs::create_dir(&starting).unwrap(); + let other = run_left(root.path(), "backup", "not a run", 1); + #[cfg(unix)] + let (_elsewhere, linked) = { + let elsewhere = tempfile::tempdir().unwrap(); + let linked = run_left(elsewhere.path(), "run-3-0", "behind a link", 1); + std::os::unix::fs::symlink(&linked, root.path().join("run-3-0")).unwrap(); + (elsewhere, linked) + }; + let own = run_dir(root.path()).unwrap(); + let mut s = Session::new(); + assert!(open(&mut s, &own).unwrap().is_empty()); + assert!(starting.exists()); + assert_eq!(list(&other).len(), 1); + #[cfg(unix)] + assert!(list(&linked).len() == 1 && root.path().join("run-3-0").is_symlink()); + } + + /// The metadata goes with its document: a document taken over remembers its file. + #[test] + fn a_document_taken_over_remembers_its_file() { + let root = tempfile::tempdir().unwrap(); + let file = root.path().join("layout.designcraft"); + let recovery = root.path().join("recovery"); + let left = recovery.join("run-7-0"); + std::fs::create_dir_all(&left).unwrap(); + std::fs::write(left.join(LOCK), b"").unwrap(); + let mut a = unsaved("saved once"); + a.execute("file.saveAs", &json!({"path": file.to_string_lossy()})).unwrap(); + a.execute("frame.create", &json!({"rect": [10, 10, 100, 100]})).unwrap(); + assert_eq!(save(&a, &left).unwrap(), 1); + let own = run_dir(&recovery).unwrap(); + let mut s = Session::new(); + assert_eq!(open(&mut s, &own).unwrap().len(), 1); + assert_eq!(s.doc().unwrap().path.as_deref(), Some(file.to_string_lossy().as_ref())); + } + + /// The folder of a run that ended goes with what the app left in it. A file the app did not + /// write keeps the folder, and is not deleted. + #[test] + fn only_what_the_app_left_goes_with_the_folder_of_a_run_that_ended() { + let root = tempfile::tempdir().unwrap(); + let emptied = run_left(root.path(), "run-7-0", "moved", 1); + std::fs::write(emptied.join("9.json"), b"{}").unwrap(); + std::fs::write(emptied.join(".designcraft-recovery-abc.tmp"), b"half written").unwrap(); + let kept = run_left(root.path(), "run-8-0", "moved too", 1); + std::fs::write(kept.join("copy.designcraft"), b"a copy someone put here").unwrap(); + let own = run_dir(root.path()).unwrap(); + let mut s = Session::new(); + assert_eq!(open(&mut s, &own).unwrap().len(), 2); + assert!(!emptied.exists()); + assert_eq!(std::fs::read(kept.join("copy.designcraft")).unwrap(), b"a copy someone put here"); + assert!(kept.join(LOCK).exists(), "still the folder of a run that ended"); + } + + /// A folder that is there is another app's or was left behind, lock or no lock: a run's own + /// is one it made itself. + #[test] + fn a_folder_that_is_there_is_not_made_this_runs() { + let root = tempfile::tempdir().unwrap(); + let left = root.path().join(format!("run-{}-0", std::process::id())); + std::fs::create_dir(&left).unwrap(); + std::fs::write(left.join("1.designcraft"), b"left with no lock").unwrap(); + let own = run_dir(root.path()).unwrap(); + assert_ne!(own, left); + assert!(!left.join(LOCK).exists()); + assert_eq!(std::fs::read(left.join("1.designcraft")).unwrap(), b"left with no lock"); + } } #[cfg(test)] diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 6c033d70..97fb5d01 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -289,7 +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. -- Crash recovery: unsaved documents are written to the recovery folder every 30 s and reopened (unsaved, remembering their files) after a crash; File › Revert and Save a Copy…; `file.recovery.*`, `file.revert`, `file.saveACopy`. +- 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`. - 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. - Edit › Paste Into (copied objects become a frame's content, clipped by it; rendered, PDF and IDML both ways — verified in InDesign 2026); Polygon Settings (sides, star inset; double-click the Polygon tool). - Color panel (fill/stroke proxy, CMYK/RGB/Lab sliders with channel ramps, tint slider for swatches, spectrum ramp, Add to Swatches) and Color Picker (double-click the proxy); mixed colours are unnamed (not listed in Swatches until added); `object.color`, `swatch.addToSwatches`, `swatch.addUnnamed`. From d020b4e85d89ddf120933107a24b23b33036c8a0 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:31:24 -0400 Subject: [PATCH 5/5] fix(ui): preserve documents when quitting is cancelled Prompt for unsaved documents on window close and application/menu quit. Defer discarding documents until all approvals are current, retaining every tab when a later decision cancels quitting. Refs #544. Cancelable macOS Dock Quit remains separate. Co-authored-by: aiongg Co-authored-by: skylar taylor-barrick --- ROADMAP.md | 3 +- apps/designcraft/src/native_menu.rs | 5 +- crates/ui-egui/src/control.rs | 2 + crates/ui-egui/src/dialogs.rs | 3 + crates/ui-egui/src/i18n.rs | 1 + crates/ui-egui/src/i18n/ar.rs | 1 + crates/ui-egui/src/i18n/it.rs | 1 + crates/ui-egui/src/i18n/ja.rs | 1 + crates/ui-egui/src/i18n/pt_br.rs | 1 + crates/ui-egui/src/i18n/uk.rs | 1 + crates/ui-egui/src/lib.rs | 475 ++++++++++++++++++++++++++++ crates/ui-egui/src/menus.rs | 5 + docs/control-protocol.md | 3 +- docs/target-app-parity.md | 1 + docs/ui-parity.md | 2 +- 15 files changed, 501 insertions(+), 4 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 15c9af8d..4b7163c7 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 (isolated recovery for each running app) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (window and menu quit confirmation) · **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** · 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. - **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. diff --git a/apps/designcraft/src/native_menu.rs b/apps/designcraft/src/native_menu.rs index 8bf75b15..f51db197 100644 --- a/apps/designcraft/src/native_menu.rs +++ b/apps/designcraft/src/native_menu.rs @@ -40,6 +40,8 @@ impl NativeMenu { // Our own About splash (with the community links) instead of the stock panel. let about = MenuItem::with_id("dc-about", "About DesignCraft", true, None); let discord = MenuItem::with_id("dc-discord", "Join the ArtCraft Discord…", true, None); + // Not the predefined Quit: it terminates the app at once, past the unsaved-changes prompt. + let quit = MenuItem::with_id("dc-quit", designcraft_ui_egui::i18n::tr(&app.ui.language, "Quit DesignCraft"), true, accel("Cmd+Q")); let _ = app_menu.append_items(&[ &about, &discord, @@ -50,10 +52,11 @@ impl NativeMenu { &PredefinedMenuItem::hide_others(None), &PredefinedMenuItem::show_all(None), &PredefinedMenuItem::separator(), - &PredefinedMenuItem::quit(None), + &quit, ]); items.insert("dc-about".into(), ("help.about".into(), serde_json::Value::Null, Handle::Plain(about))); items.insert("dc-discord".into(), ("help.discord".into(), serde_json::Value::Null, Handle::Plain(discord))); + items.insert("dc-quit".into(), ("app.quit".into(), serde_json::Value::Null, Handle::Plain(quit))); let _ = menu.append(&app_menu); for (title, entries) in menus::menu_tree() { let sub = Submenu::new(designcraft_ui_egui::i18n::tr(&app.ui.language, title), true); diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index f0d34a7b..12df640d 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -353,6 +353,8 @@ pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) -> "app.save" => wrap(app.run("file.save", json!({"path": s("path")}))), "app.export" => wrap(app.run("app.exportPng", p.clone())), "app.quit" => { + // Automation must not wait on a prompt: quit without asking about unsaved changes. + app.quit_without_asking = true; ctx.send_viewport_cmd(egui::ViewportCommand::Close); ok(Value::Null) } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 2ab7a3ba..14d2552e 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2078,6 +2078,9 @@ pub fn confirm(app: &mut DesignApp) -> Result { "closeDocument" => { // The document may have moved in the tab row (or gone) while the dialog was open. let uid = d.fields.get("uid").and_then(Value::as_u64); + if d.b("quitting") { + return app.confirm_quit_document(uid.ok_or("Missing document in quit confirmation")?, d.b("discard")); + } let index = |app: &DesignApp| app.session.documents().iter().position(|doc| Some(doc.uid) == uid); let Some(i) = index(app) else { return Ok(Value::Null) }; if !d.b("discard") { diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index e5e8a8d0..5976d7d0 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -699,6 +699,7 @@ const TABLE: &[(&str, [&str; 5])] = &[ ), ("for", ["for", "pour", "por", "に対して", "行"]), ("About DesignCraft", ["About DesignCraft", "About DesignCraft", "About DesignCraft", "About DesignCraft", "关于 DesignCraft"]), + ("Quit DesignCraft", ["DesignCraft beenden", "Quitter DesignCraft", "Salir de DesignCraft", "DesignCraft を終了", "退出 DesignCraft"]), ("Accept All Changes", ["Accept All Changes", "Accept All Changes", "Accept All Changes", "Accept All Changes", "接受所有更改"]), ("Activate Document", ["Activate Document", "Activate Document", "Activate Document", "Activate Document", "激活文档"]), ("Add Anchor Point", ["Add Anchor Point", "Add Anchor Point", "Add Anchor Point", "Add Anchor Point", "添加锚点"]), diff --git a/crates/ui-egui/src/i18n/ar.rs b/crates/ui-egui/src/i18n/ar.rs index 10e393cd..c3373486 100644 --- a/crates/ui-egui/src/i18n/ar.rs +++ b/crates/ui-egui/src/i18n/ar.rs @@ -149,6 +149,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Bookmarks", "إشارات مرجعية"), ("Show All Menu Items", "إظهار كل عناصر القائمة"), ("About DesignCraft", "حول DesignCraft"), + ("Quit DesignCraft", "إنهاء DesignCraft"), ("Welcome to DesignCraft", "مرحباً بك في DesignCraft"), ("Page layout for print and screen — fast, open and scriptable.", "تخطيط الصفحات للطباعة والشاشة — سريع ومفتوح وقابل للبرمجة."), (" New file ", " ملف جديد "), diff --git a/crates/ui-egui/src/i18n/it.rs b/crates/ui-egui/src/i18n/it.rs index c91afd6b..34766cf2 100644 --- a/crates/ui-egui/src/i18n/it.rs +++ b/crates/ui-egui/src/i18n/it.rs @@ -396,6 +396,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Spreads of up to 10 pages", "Doppie pagine con al massimo 10 pagine"), ("for", "per"), ("About DesignCraft", "Informazioni su DesignCraft"), + ("Quit DesignCraft", "Esci da DesignCraft"), ("Accept All Changes", "Accetta tutte le modifiche"), ("Activate Document", "Attiva documento"), ("Add Anchor Point", "Aggiungi punto di ancoraggio"), diff --git a/crates/ui-egui/src/i18n/ja.rs b/crates/ui-egui/src/i18n/ja.rs index be6e2c4d..de57db5e 100644 --- a/crates/ui-egui/src/i18n/ja.rs +++ b/crates/ui-egui/src/i18n/ja.rs @@ -398,6 +398,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Spreads of up to 10 pages", "最大10ページのスプレッド"), ("for", "に対して"), ("About DesignCraft", "DesignCraft について"), + ("Quit DesignCraft", "DesignCraft を終了"), ("Accept All Changes", "すべての変更を採用"), ("Activate Document", "ドキュメントをアクティブ化"), ("Add Anchor Point", "アンカーポイントを追加"), diff --git a/crates/ui-egui/src/i18n/pt_br.rs b/crates/ui-egui/src/i18n/pt_br.rs index d25717dc..a4d06bbe 100644 --- a/crates/ui-egui/src/i18n/pt_br.rs +++ b/crates/ui-egui/src/i18n/pt_br.rs @@ -149,6 +149,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Bookmarks", "Indicadores"), ("Show All Menu Items", "Mostrar todos os itens de menu"), ("About DesignCraft", "Sobre o DesignCraft"), + ("Quit DesignCraft", "Encerrar o DesignCraft"), ("Welcome to DesignCraft", "Bem-vindo ao DesignCraft"), ("Page layout for print and screen — fast, open and scriptable.", "Layout de página para impressão e tela — rápido, aberto e scriptável."), (" New file ", " Novo arquivo "), diff --git a/crates/ui-egui/src/i18n/uk.rs b/crates/ui-egui/src/i18n/uk.rs index 9a9fb89b..9b70204a 100644 --- a/crates/ui-egui/src/i18n/uk.rs +++ b/crates/ui-egui/src/i18n/uk.rs @@ -399,6 +399,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Spreads of up to 10 pages", "Розвороти до 10 сторінок"), ("for", "для"), ("About DesignCraft", "Про DesignCraft"), + ("Quit DesignCraft", "Завершити DesignCraft"), ("Accept All Changes", "Прийняти всі зміни"), ("Activate Document", "Активувати документ"), ("Add Anchor Point", "Додати опорну точку"), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3d94bdb0..e0fa1dff 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -510,6 +510,18 @@ pub struct DesignApp { last_time: f64, /// When recovery data was last written (seconds, egui time). pub last_recovery: f64, + /// Quit was chosen: close the window on the next frame, as its close button does. + quit_requested: bool, + /// Close the window without asking about unsaved changes (the control channel's `app.quit`). + pub(crate) quit_without_asking: bool, + /// No document is discarded until every unsaved document has an up-to-date approval. + quitting: Option, +} + +struct QuitState { + original_active: Option, + accepted: HashMap, + pending: Option, } impl DesignApp { @@ -547,6 +559,9 @@ impl DesignApp { native_shortcuts: Default::default(), last_time: 0.0, last_recovery: 0.0, + quit_requested: false, + quit_without_asking: false, + quitting: None, } } @@ -802,6 +817,146 @@ impl DesignApp { let _ = self.open_dropped(&p); } } + self.handle_quit(ctx); + } + + /// Quit: close the window the way its close button does, so unsaved documents ask first. + pub fn request_quit(&mut self) { + self.quit_requested = true; + } + + /// Ask about unsaved documents without closing any of them yet. A later Cancel must also + /// keep documents whose earlier answer was Don't Save, including their history and recovery. + fn handle_quit(&mut self, ctx: &egui::Context) { + if std::mem::take(&mut self.quit_requested) { + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + } + if ctx.input(|i| i.viewport().close_requested()) { + if self.quit_without_asking { + self.quitting = None; + return; + } + if self.quitting.is_none() && !self.session.documents().iter().any(|d| d.is_dirty()) { + return; + } + let approved = + self.quitting.as_ref().is_some_and(|q| q.pending.is_none()) && self.ui.dialog.is_none() && self.next_unapproved_document().is_none(); + if approved { + // Commit only when the backend receives the final close, after checking for + // edits that arrived since the last answer. Closing also discards recovery data. + if let Err(e) = self.finish_confirmed_quit() { + self.status(e); + self.cancel_quit(); + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + } + return; + } + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + if self.quitting.is_none() { + self.quitting = Some(QuitState { original_active: self.session.active().map(|d| d.uid), accepted: HashMap::new(), pending: None }); + } + } + let Some(q) = self.quitting.as_ref() else { return }; + if let Some(uid) = q.pending { + let asking = self.ui.dialog.as_ref().is_some_and(|d| { + d.id == "closeDocument" + && d.fields.get("quitting").and_then(Value::as_bool) == Some(true) + && d.fields.get("uid").and_then(Value::as_u64) == Some(uid) + }); + if !asking { + self.cancel_quit(); + } + return; + } + if self.ui.dialog.is_some() { + self.cancel_quit(); + return; + } + self.ask_next_unsaved(ctx); + } + + /// Finalize a complete, still-current quit plan at an orderly host exit. The host can + /// terminate before egui receives its queued Close; an unfinished/stale plan leaves every + /// document live so the host can preserve its recovery data instead. + pub fn finish_confirmed_quit(&mut self) -> Result<(), String> { + let approved = + self.quitting.as_ref().is_some_and(|q| q.pending.is_none()) && self.ui.dialog.is_none() && self.next_unapproved_document().is_none(); + if !approved { + return Ok(()); + } + while !self.session.documents().is_empty() { + self.run("file.close", json!({"index": 0}))?; + } + self.quitting = None; + Ok(()) + } + + fn next_unapproved_document(&self) -> Option<(usize, u64)> { + let q = self.quitting.as_ref()?; + self.session + .documents() + .iter() + .enumerate() + .find(|(_, d)| d.is_dirty() && q.accepted.get(&d.uid) != Some(&d.revision)) + .map(|(i, d)| (i, d.uid)) + } + + fn cancel_quit(&mut self) { + let Some(q) = self.quitting.take() else { return }; + if let Some(i) = q.original_active.and_then(|uid| self.session.documents().iter().position(|d| d.uid == uid)) { + let _ = self.run("file.activate", json!({"index": i})); + } + } + + /// Save/Don't Save during Quit approves this UID's current revision, without destroying it. + pub(crate) fn confirm_quit_document(&mut self, uid: u64, discard: bool) -> Result { + if self.quitting.as_ref().and_then(|q| q.pending) != Some(uid) { + return Err("Quit confirmation is no longer active".into()); + } + let Some(i) = self.session.documents().iter().position(|d| d.uid == uid) else { + if let Some(q) = self.quitting.as_mut() { + q.pending = None; + } + return Ok(Value::Null); + }; + if !discard { + let saved = self.run("file.activate", json!({"index": i})).and_then(|_| self.run("app.save", json!({}))); + if let Err(e) = saved { + self.cancel_quit(); + return Err(e); + } + if self.session.documents().iter().find(|d| d.uid == uid).is_none_or(|d| d.is_dirty()) { + // No file chosen (or an asynchronous host save has not completed): stop quitting. + self.cancel_quit(); + return Ok(Value::Null); + } + } + if let Some(q) = self.quitting.as_mut() { + if let Some(d) = self.session.documents().iter().find(|d| d.uid == uid) { + q.accepted.insert(uid, d.revision); + } + q.pending = None; + } + Ok(Value::Null) + } + + /// Show the next unapproved document, or request a final close that revalidates all approvals. + fn ask_next_unsaved(&mut self, ctx: &egui::Context) { + let Some((i, uid)) = self.next_unapproved_document() else { + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + return; + }; + if self.run("file.activate", json!({"index": i})).is_err() { + self.cancel_quit(); + return; + } + menus::close_document(self, Some(i)); + if let Some(d) = self.ui.dialog.as_mut() { + d.fields.insert("quitting".into(), json!(true)); + } + if let Some(q) = self.quitting.as_mut() { + q.pending = Some(uid); + } } /// A file dropped on the window: documents open, anything else is placed. @@ -1173,6 +1328,326 @@ mod tests { assert!(app.ui.dialog.is_none()); } + /// One frame of the quit handling, with the window asked to close when `close`; returns the + /// commands sent to the window. + fn quit_frame(app: &mut DesignApp, ctx: &egui::Context, close: bool) -> Vec { + let mut raw = egui::RawInput::default(); + if close { + raw.viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + } + let mut out = ctx.run_ui(raw, |ui| app.handle_quit(ui.ctx())); + out.textures_delta.clear(); + out.viewport_output.get(&egui::ViewportId::ROOT).map(|v| v.commands.clone()).unwrap_or_default() + } + + fn dialog_uid(app: &DesignApp) -> Option { + app.ui.dialog.as_ref().filter(|d| d.id == "closeDocument").and_then(|d| d.fields.get("uid")?.as_u64()) + } + + /// Quitting closed the window at once, losing unsaved changes (#544). + #[test] + fn quitting_with_unsaved_changes_asks_and_cancel_keeps_everything() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + let cmds = quit_frame(&mut app, &ctx, true); + assert!(cmds.contains(&egui::ViewportCommand::CancelClose), "{cmds:?}"); + assert_eq!(dialog_uid(&app), Some(app.session.documents()[0].uid)); + // Cancel: the window and the document stay as they were. + app.ui.dialog = None; + for _ in 0..2 { + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + } + assert_eq!(app.session.documents().len(), 1); + assert!(app.session.documents()[0].is_dirty()); + // A Save whose file picker is cancelled stops quitting too. + quit_frame(&mut app, &ctx, true); + dialogs::confirm(&mut app).unwrap(); + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert!(app.session.documents()[0].is_dirty()); + } + + #[test] + fn quitting_asks_about_each_unsaved_document_then_closes() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + app.run("file.new", json!({})).unwrap(); + app.run("frame.create", json!({"rect": [36, 36, 136, 136]})).unwrap(); + app.run("file.new", json!({})).unwrap(); + let uids: Vec = app.session.documents().iter().map(|d| d.uid).collect(); + assert!(quit_frame(&mut app, &ctx, true).contains(&egui::ViewportCommand::CancelClose)); + for (n, uid) in uids.iter().take(2).enumerate() { + assert_eq!(dialog_uid(&app), Some(*uid), "question {n}"); + assert_eq!(app.session.active().map(|d| d.uid), Some(*uid), "the document asked about is shown"); + app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true)); + dialogs::confirm(&mut app).unwrap(); + let cmds = quit_frame(&mut app, &ctx, false); + assert_eq!(cmds.contains(&egui::ViewportCommand::Close), n == 1, "{cmds:?}"); + } + // All documents stay open until the backend's final close commits the decision. + assert_eq!(app.session.documents().len(), 3); + assert!(quit_frame(&mut app, &ctx, true).is_empty()); + assert!(app.session.documents().is_empty()); + } + + /// A later Cancel must not destroy documents already approved for discard. + #[test] + fn quitting_later_cancel_retains_every_document_and_active_tab() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + app.run("file.new", json!({})).unwrap(); + app.run("frame.create", json!({"rect": [20, 20, 120, 120]})).unwrap(); + let uids: Vec = app.session.documents().iter().map(|d| d.uid).collect(); + let active = app.session.active().unwrap().uid; + let revisions: Vec = app.session.documents().iter().map(|d| d.revision).collect(); + let docs: Vec<_> = app.session.documents().iter().map(|d| d.doc.clone()).collect(); + quit_frame(&mut app, &ctx, true); + app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true)); + dialogs::confirm(&mut app).unwrap(); + quit_frame(&mut app, &ctx, false); + assert_eq!(dialog_uid(&app), Some(uids[1])); + app.ui.dialog = None; + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(app.session.documents().iter().map(|d| d.uid).collect::>(), uids); + assert_eq!(app.session.active().unwrap().uid, active); + assert_eq!(app.session.documents().iter().map(|d| d.revision).collect::>(), revisions); + for (d, original) in app.session.documents().iter().zip(docs) { + assert!(std::sync::Arc::ptr_eq(&d.doc, &original)); + assert!(d.is_dirty()); + assert!(!d.history.undo.is_empty()); + } + } + + fn second_unsaved_document(app: &mut DesignApp) { + app.run("file.new", json!({})).unwrap(); + app.run("frame.create", json!({"rect": [20, 20, 120, 120]})).unwrap(); + } + + fn discard_quit_dialog(app: &mut DesignApp) { + app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true)); + dialogs::confirm(app).unwrap(); + } + + #[test] + fn quitting_first_cancel_restores_original_clean_active_document() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + app.run("file.new", json!({})).unwrap(); + let original_active = app.session.active().unwrap().uid; + quit_frame(&mut app, &ctx, true); + assert_ne!(app.session.active().unwrap().uid, original_active); + app.ui.dialog = None; + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(app.session.active().unwrap().uid, original_active); + assert_eq!(app.session.documents().len(), 2); + } + + #[test] + fn quitting_save_as_cancel_after_discard_keeps_all_documents() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + second_unsaved_document(&mut app); + let active = app.session.active().unwrap().uid; + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + quit_frame(&mut app, &ctx, false); + dialogs::confirm(&mut app).unwrap(); // No picker: Save As was cancelled. + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(app.session.documents().len(), 2); + assert!(app.session.documents().iter().all(|d| d.is_dirty())); + assert_eq!(app.session.active().unwrap().uid, active); + } + + #[test] + fn quitting_save_failure_after_discard_keeps_documents_and_can_retry() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + second_unsaved_document(&mut app); + let path = std::env::temp_dir() + .join(format!("designcraft-quit-missing-{}-{}", std::process::id(), app.session.active().unwrap().uid)) + .join("missing") + .join("document.designcraft"); + assert!(!path.parent().unwrap().exists()); + let picked = path.to_string_lossy().to_string(); + app.services.pick_save = Some(Box::new(move |_| Some(picked.clone()))); + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + quit_frame(&mut app, &ctx, false); + assert!(dialogs::confirm(&mut app).is_err()); + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(app.session.documents().len(), 2); + assert!(app.session.documents().iter().all(|d| d.is_dirty())); + // Starting over must ask about the previously discarded first document again. + quit_frame(&mut app, &ctx, true); + assert_eq!(dialog_uid(&app), Some(app.session.documents()[0].uid)); + } + + #[test] + fn quitting_save_then_later_cancel_keeps_saved_document_open() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + second_unsaved_document(&mut app); + let first = app.session.documents()[0].uid; + let path = std::env::temp_dir().join(format!("designcraft-quit-save-{}-{first}.designcraft", std::process::id())); + let picked = path.to_string_lossy().to_string(); + app.services.pick_save = Some(Box::new(move |_| Some(picked.clone()))); + quit_frame(&mut app, &ctx, true); + dialogs::confirm(&mut app).unwrap(); + quit_frame(&mut app, &ctx, false); + assert_eq!(app.session.documents().len(), 2); + assert!(!app.session.documents()[0].is_dirty()); + assert!(path.exists()); + app.ui.dialog = None; + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(app.session.documents()[0].uid, first); + assert!(app.session.documents()[1].is_dirty()); + std::fs::remove_file(path).unwrap(); + } + + #[test] + fn quitting_rechecks_discard_approval_after_a_later_edit() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + second_unsaved_document(&mut app); + let first = app.session.documents()[0].uid; + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + quit_frame(&mut app, &ctx, false); + app.run("file.activate", json!({"index": 0})).unwrap(); + app.run("frame.create", json!({"rect": [200, 200, 300, 300]})).unwrap(); + discard_quit_dialog(&mut app); // Approval is bound to the second UID, despite the switch. + assert!(!quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(dialog_uid(&app), Some(first), "new edit needs a fresh decision"); + assert_eq!(app.session.documents().len(), 2); + } + + #[test] + fn quitting_final_close_rechecks_new_unsaved_document() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + assert!(quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + second_unsaved_document(&mut app); // Arrives after Close was queued, before its event. + let new_uid = app.session.active().unwrap().uid; + assert!(quit_frame(&mut app, &ctx, true).contains(&egui::ViewportCommand::CancelClose)); + assert_eq!(dialog_uid(&app), Some(new_uid)); + assert_eq!(app.session.documents().len(), 2); + } + + #[test] + fn quitting_pending_save_uses_dialog_uid_after_tab_switch() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + second_unsaved_document(&mut app); + let first = app.session.documents()[0].uid; + let path = std::env::temp_dir().join(format!("designcraft-quit-uid-{}-{first}.designcraft", std::process::id())); + let picked = path.to_string_lossy().to_string(); + app.services.pick_save = Some(Box::new(move |_| Some(picked.clone()))); + quit_frame(&mut app, &ctx, true); + app.run("file.activate", json!({"index": 1})).unwrap(); + dialogs::confirm(&mut app).unwrap(); + assert_eq!(app.session.documents()[0].path.as_deref(), path.to_str()); + assert!(!app.session.documents()[0].is_dirty()); + assert!(app.session.documents()[1].is_dirty()); + quit_frame(&mut app, &ctx, false); + assert_eq!(dialog_uid(&app), Some(app.session.documents()[1].uid)); + std::fs::remove_file(path).unwrap(); + } + + #[test] + fn quitting_discard_preserves_recovery_until_final_commit() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + let uid = app.session.active().unwrap().uid; + let dir = std::env::temp_dir().join(format!("designcraft-quit-recovery-{}-{uid}", std::process::id())); + app.session.recovery_dir = Some(dir.clone()); + app.run("file.recovery.save", json!({})).unwrap(); + let entry = dir.join(format!("{uid}.designcraft")); + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + assert!(quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert!(entry.exists(), "a queued close is not a committed close"); + assert!(quit_frame(&mut app, &ctx, true).is_empty()); + assert!(!entry.exists(), "accepted Don't Save must not be recovered on restart"); + assert!(app.session.documents().is_empty()); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn quitting_native_exit_commits_completed_approvals_before_close_event() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + assert!(quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert_eq!(app.session.documents().len(), 1); + app.finish_confirmed_quit().unwrap(); + assert!(app.session.documents().is_empty()); + } + + #[test] + fn quitting_native_exit_retains_incomplete_stale_and_new_document_plans() { + for case in ["incomplete", "stale", "new document"] { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + if case == "incomplete" { + second_unsaved_document(&mut app); + } + quit_frame(&mut app, &ctx, true); + discard_quit_dialog(&mut app); + quit_frame(&mut app, &ctx, false); + if case == "stale" { + app.run("frame.create", json!({"rect": [200, 200, 300, 300]})).unwrap(); + } else if case == "new document" { + second_unsaved_document(&mut app); + } + let active = app.session.active().unwrap().uid; + let before: Vec<_> = app + .session + .documents() + .iter() + .map(|d| (d.uid, d.revision, d.doc.clone(), d.selection.items.clone(), d.history.undo.len(), d.history.redo.len())) + .collect(); + app.finish_confirmed_quit().unwrap(); + assert_eq!(app.session.documents().len(), before.len(), "{case}"); + assert_eq!(app.session.active().unwrap().uid, active, "{case}"); + for (d, (uid, revision, doc, selection, undo, redo)) in app.session.documents().iter().zip(before) { + assert_eq!( + (d.uid, d.revision, &d.selection.items, d.history.undo.len(), d.history.redo.len()), + (uid, revision, &selection, undo, redo), + "{case}" + ); + assert!(std::sync::Arc::ptr_eq(&d.doc, &doc), "{case}"); + } + } + } + + #[test] + fn quitting_without_unsaved_changes_closes_at_once() { + let ctx = egui::Context::default(); + let mut app = DesignApp::new(designcraft_engine::Session::new(), Services::default()); + app.run("file.new", json!({})).unwrap(); + // Quit (⌘Q) closes the window like its close button. + app.run("app.quit", json!({})).unwrap(); + assert!(quit_frame(&mut app, &ctx, false).contains(&egui::ViewportCommand::Close)); + assert!(quit_frame(&mut app, &ctx, true).is_empty()); + assert!(app.ui.dialog.is_none()); + } + + /// Scripts must not hang on a question: the control channel's quit never asks. + #[test] + fn control_channel_quit_does_not_ask() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + let (req, _rx) = control::ControlRequest::new("app.quit", json!({})); + let mut out = ctx.run_ui(egui::RawInput::default(), |ui| { + control::handle(&mut app, ui.ctx(), &req); + }); + out.textures_delta.clear(); + assert!(quit_frame(&mut app, &ctx, true).is_empty()); + assert!(app.ui.dialog.is_none()); + } + /// The Control panel's two-row groups were centred as if they were one row tall, so the second /// row hung below the panel and was cut off. #[test] diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index d123ec4e..cb142239 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -11,6 +11,7 @@ pub const UI_COMMANDS: &[(&str, &str, Option<&str>, &str)] = &[ ("app.placeDialog", "Place…", Some("Cmd+D"), "{}"), ("app.saveDialog", "Save As…", Some("Cmd+Shift+S"), "{}"), ("app.save", "Save", Some("Cmd+S"), "{}"), + ("app.quit", "Quit DesignCraft", Some("Cmd+Q"), "{} — close the window; each document with unsaved changes asks first"), ("app.saveCopyDialog", "Save a Copy…", Some("Cmd+Alt+S"), "{} — choose where to write a copy (the document stays as it is)"), ("app.loadSwatches", "Load Swatches…", None, "{} — pick a swatch exchange (.ase) file"), ("app.packageDialog", "Package…", Some("Cmd+Alt+Shift+P"), "{} — choose the package folder to create (file.package)"), @@ -769,6 +770,10 @@ pub fn run_ui(app: &mut DesignApp, id: &str, p: &Value) -> Option { + app.request_quit(); + Ok(Value::Null) + } "app.saveCopyDialog" => { let name = app.session.active().map(|d| format!("{} copy.designcraft", d.doc.title)).unwrap_or_default(); if let Some(path) = app.services.pick_save.as_mut().and_then(|f| f(&name)) { diff --git a/docs/control-protocol.md b/docs/control-protocol.md index d97c2bad..9e514561 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -46,7 +46,8 @@ so only enable it while you use it. Transport: `apps/designcraft/src/control_ser | `ui.resize`, `ui.focus` | | Window control | | `ui.screenshot` | `{path?}` | PNG of the whole window | | `ui.render` | `{path?, page?, scale?, bleed?}` | Render a page headlessly (PNG; base64 if no path) | -| `app.open` / `app.save` / `app.export` / `app.quit` | | Files | +| `app.open` / `app.save` / `app.export` | | Files | +| `app.quit` | | Quit without asking about unsaved changes (the `app.quit` command, like ⌘Q, asks) | Data merge runs through `engine.execute` like any command: `data.source.select`, `data.fields`, `data.placeholder.add` / `.remove`, `data.options`, `data.preview` / `data.preview.stop`, `data.merge`. `data.merge` creates and activates a new diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 97fb5d01..5705ee73 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -290,6 +290,7 @@ It is evidence of breadth: each item exists. It is not a claim of InDesign-level - 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. - 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. - Edit › Paste Into (copied objects become a frame's content, clipped by it; rendered, PDF and IDML both ways — verified in InDesign 2026); Polygon Settings (sides, star inset; double-click the Polygon tool). - Color panel (fill/stroke proxy, CMYK/RGB/Lab sliders with channel ramps, tint slider for swatches, spectrum ramp, Add to Swatches) and Color Picker (double-click the proxy); mixed colours are unnamed (not listed in Swatches until added); `object.color`, `swatch.addToSwatches`, `swatch.addUnnamed`. diff --git a/docs/ui-parity.md b/docs/ui-parity.md index ee6db423..8805defc 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -50,7 +50,7 @@ basics of editing fail for too many users on Windows and Linux. | Workspaces | Essentials, Advanced, Book, Digital Publishing, Interactive for PDF, Printing and Proofing, Typography, Touch | ✓ named workspaces + custom | ~ | tab bar overlaps menus in Book/Advanced (#182) | | Story Editor | ✓ | ✓ | ~ | style-name column, depth ruler missing | | Undo granularity (a drag = one step) | ✓ | ✓ (Begin/Preview/Commit) | ✓ | | -| Closing with unsaved changes asks | ✓ | ✓ (#146) | ✓ | | +| Closing with unsaved changes asks | ✓ | ✓ File › Close and window/menu quit (#146, #545) | ✓ | macOS Dock Quit still bypasses the prompt (#544) | | UI scaling, System appearance | ✓ | scaling ✓ (#281); follows macOS light/dark ✗ (#237) | ~ | | | Window controls, full screen | ✓ | ~ | ~ | misaligned on macOS (#333) |