diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 37c71fc65..57eb9bb53 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 }} diff --git a/Cargo.lock b/Cargo.lock index 7c1177b70..5a4157bf0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -899,7 +899,7 @@ dependencies = [ [[package]] name = "designcraft" -version = "0.7.0" +version = "0.7.1" dependencies = [ "arboard", "designcraft-engine", @@ -921,7 +921,7 @@ dependencies = [ [[package]] name = "designcraft-cli" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -937,7 +937,7 @@ dependencies = [ [[package]] name = "designcraft-color" -version = "0.7.0" +version = "0.7.1" dependencies = [ "kurbo", "moxcms", @@ -949,7 +949,7 @@ dependencies = [ [[package]] name = "designcraft-compose" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-doc", @@ -969,7 +969,7 @@ dependencies = [ [[package]] name = "designcraft-doc" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-geom", @@ -981,7 +981,7 @@ dependencies = [ [[package]] name = "designcraft-engine" -version = "0.7.0" +version = "0.7.1" dependencies = [ "caseless", "craft-print", @@ -1016,7 +1016,7 @@ dependencies = [ [[package]] name = "designcraft-epub" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-doc", @@ -1025,7 +1025,7 @@ dependencies = [ [[package]] name = "designcraft-fonts" -version = "0.7.0" +version = "0.7.1" dependencies = [ "core-text", "harfrust 0.13.3", @@ -1038,7 +1038,7 @@ dependencies = [ [[package]] name = "designcraft-format" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-doc", "serde", @@ -1049,7 +1049,7 @@ dependencies = [ [[package]] name = "designcraft-geom" -version = "0.7.0" +version = "0.7.1" dependencies = [ "flo_curves", "kurbo", @@ -1062,7 +1062,7 @@ dependencies = [ [[package]] name = "designcraft-idml" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1076,7 +1076,7 @@ dependencies = [ [[package]] name = "designcraft-images" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-fonts", @@ -1087,7 +1087,7 @@ dependencies = [ [[package]] name = "designcraft-mcp" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-engine", "designcraft-geom", @@ -1100,7 +1100,7 @@ dependencies = [ [[package]] name = "designcraft-pdf" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1122,7 +1122,7 @@ dependencies = [ [[package]] name = "designcraft-render" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1140,7 +1140,7 @@ dependencies = [ [[package]] name = "designcraft-textimport" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-doc", "quick-xml 0.38.4", @@ -1149,7 +1149,7 @@ dependencies = [ [[package]] name = "designcraft-tools" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-color", "designcraft-compose", @@ -1161,7 +1161,7 @@ dependencies = [ [[package]] name = "designcraft-ui-egui" -version = "0.7.0" +version = "0.7.1" dependencies = [ "craft-ui", "designcraft-color", @@ -1187,7 +1187,7 @@ dependencies = [ [[package]] name = "designcraft-web" -version = "0.7.0" +version = "0.7.1" dependencies = [ "designcraft-engine", "designcraft-format", @@ -5740,7 +5740,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 f15331e12..45fe8f626 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" diff --git a/ROADMAP.md b/ROADMAP.md index 5546b77cf..31342a1fd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: pre-alpha** · next: alpha, ~5–7 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-11 · **Last updated:** 2026-10-12 · **Change:** minor (text threads by hand; the alpha gate waits on Primary Text Frame page fixes; Japanese print, import and table controls localized; Live corners (#186); Optical kerning from outlines (#181)) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (text threads by hand; the alpha gate waits on Primary Text Frame page fixes; Japanese print, import and table controls localized; Live corners (#186); Optical kerning from outlines (#181); 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. @@ -152,6 +152,9 @@ Newest first. - **2026-10-12** · Clipping paths saved with images in IDML (Detect Edges, Alpha Channel, Photoshop Path, User-Modified Path, inverted or not) clip the image on screen and in PDF and survive IDML export; `object.clippingPathOptions` reports, inverts or removes one (#450). No readiness percentage change. - **2026-10-12** · Anchored objects with text wrap push text aside: other stories' text under them and the lines of their own story after the anchor's line (#557). No readiness percentage change. - **2026-10-12** · Hyphenation Zone applies to the Single-line Composer only, as in InDesign. With the default Paragraph Composer, ragged text hyphenates where InDesign does (aper-natem), giving the same line breaks. No readiness percentage change. +- **2026-10-12** · Window close and application/menu Quit ask about unsaved documents; later Cancel keeps every tab open, including earlier Don't Save choices. Approvals are rechecked after edits. Cancelable macOS Dock Quit remains open (#544). No readiness percentage change. +- **2026-10-12** · Each running app keeps recovery data in its own locked folder; current folder and lock ownership are checked before writes and cleanup. Failed isolation disables recovery with a warning instead of sharing document IDs. No readiness percentage change. +- **2026-10-12** · Native engine saves and exports replace files atomically; failed writes retain the original, private staging preserves Unix permissions, and valid long filenames work. Desktop Services and CLI raster exports remain separate follow-ups. No readiness percentage change. - **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-12** · Load Swatches refuses an `.ase` file with more than 100,000 colors, and `swatch.load` merges a file of 100,000 colors in under 0.1 s (it took 9.6 s of CPU in a test build). No readiness percentage change. - **2026-10-11 (printing):** `file.printers` and `file.print` use craft-libs/craft-print for CUPS and Windows. DesignCraft retains PDF export, page selection, spreads, marks and bleed; dry runs create no temporary PDF or print job. Windows selects matching or default driver paper and shrinks oversized sheets without enlarging smaller ones; queue duplex defaults are preserved. Headless tests exercise exported bytes, job settings, dry runs and refusals; shared native Windows CI passes, including paused-queue submission and cleanup. PostScript, production print controls and physical output validation remain open. No readiness percentage change. diff --git a/apps/designcraft/src/profile.rs b/apps/designcraft/src/profile.rs index d9c0fc75d..ed52597c9 100644 --- a/apps/designcraft/src/profile.rs +++ b/apps/designcraft/src/profile.rs @@ -85,16 +85,19 @@ fn executable_dir() -> Option { } pub fn recovery_dir() -> Option { - recovery_dir_with(root_with(|key| std::env::var_os(key), executable_dir().as_deref()), designcraft_engine::recovery::default_dir).map(own_run) + recovery_dir_with(root_with(|key| std::env::var_os(key), executable_dir().as_deref()), 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) -> PathBuf { - designcraft_engine::recovery::run_dir(&root).unwrap_or_else(|e| { - log::warn!("recovery data is shared with other running apps: {e}"); - root - }) +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 { @@ -248,11 +251,23 @@ mod tests { 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()).parent(), Some(root.as_path())); - // Nothing can be made under a file: the recovery folder itself, as before. + 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")), blocked.join("Recovery")); + 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/atomic.rs b/crates/engine/src/atomic.rs index ea29f4e92..7eed2f028 100644 --- a/crates/engine/src/atomic.rs +++ b/crates/engine/src/atomic.rs @@ -11,13 +11,17 @@ //! 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, OpenOptions}; +use std::fs::File; +#[cfg(unix)] +use std::fs::OpenOptions; use std::io::{self, Write as _}; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicU64, Ordering}; /// How many times a replacing rename is tried before its error is returned. const RENAME_TRIES: u32 = if cfg!(windows) { 5 } else { 1 }; @@ -42,11 +46,7 @@ pub(crate) fn write_file(path: &str, bytes: &[u8]) -> crate::Result<()> { /// 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)?; - let name = target - .file_name() - .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, format!("{}: not a file path", path.display())))? - .to_string_lossy() - .into_owned(); + 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("."), @@ -55,15 +55,18 @@ pub fn write_atomic_with(path: &Path, fill: impl FnOnce(&mut File) -> io::Result if existing.is_some() { check_writable(&target)?; } - let (tmp, mut file) = create_temp(&dir, &name)?; - let written = fill(&mut file).and_then(|()| file.sync_all()).and_then(|()| existing.map_or(Ok(()), |m| keep_permissions(&file, &m))); - drop(file); - let placed = written.and_then(|()| retry_rename(RENAME_TRIES, || std::fs::rename(&tmp, &target))); - if let Err(e) = placed { - // Best effort: the temporary file is all there is to clean up. - let _ = std::fs::remove_file(&tmp); - return Err(e); + 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); @@ -71,20 +74,22 @@ pub fn write_atomic_with(path: &Path, fill: impl FnOnce(&mut File) -> io::Result } /// Create a fresh temporary file beside the target (the same folder, so the rename stays on one -/// volume). Names are unique per process and call; a leftover of the same name is skipped. -fn create_temp(dir: &Path, name: &str) -> io::Result<(PathBuf, File)> { - static NEXT: AtomicU64 = AtomicU64::new(0); - let mut last = None; - for _ in 0..8 { - let n = NEXT.fetch_add(1, Ordering::Relaxed); - let tmp = dir.join(format!(".{name}.{}-{n}.tmp", std::process::id())); - match OpenOptions::new().write(true).create_new(true).open(&tmp) { - Ok(f) => return Ok((tmp, f)), - Err(e) if e.kind() == io::ErrorKind::AlreadyExists => last = Some(e), - Err(e) => return Err(e), - } +/// 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)); } - Err(last.unwrap_or_else(|| io::Error::from(io::ErrorKind::AlreadyExists))) + #[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 @@ -148,7 +153,8 @@ fn retry_rename(tries: u32, mut rename: impl FnMut() -> io::Result<()>) -> io::R } } -/// Flush a folder's entries (Unix); Windows can't open a folder as a file, and needs no flush. +/// 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() @@ -213,6 +219,56 @@ mod tests { 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; diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index ca1f2ac1f..ea885dcf8 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -42,6 +42,8 @@ struct Run { root: PathBuf, dir: PathBuf, lock: std::fs::File, + #[cfg(unix)] + directory: std::fs::File, } #[cfg(not(target_arch = "wasm32"))] @@ -83,7 +85,10 @@ pub fn run_dir(root: &Path) -> Result { 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(()) => Ok(Some(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()))), } @@ -91,13 +96,21 @@ pub fn run_dir(root: &Path) -> Result { 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)) if std::fs::symlink_metadata(dir.join(LOCK)).is_ok() => { - runs.push(Run { root: root.to_path_buf(), dir: dir.clone(), lock }); + 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) => { - let _ = std::fs::remove_file(dir.join(LOCK)); + // Never unlink a lock that another starting run may now own. let _ = std::fs::remove_dir(&dir); return Err(e); } @@ -109,26 +122,127 @@ pub fn run_dir(root: &Path) -> Result { #[cfg(not(target_arch = "wasm32"))] fn new_lock(dir: &Path) -> std::io::Result { let mut new = std::fs::OpenOptions::new(); - new.write(true).create_new(true); + new.read(true).write(true).create_new(true); #[cfg(unix)] { use std::os::unix::fs::OpenOptionsExt; new.mode(0o600); } - new.open(dir.join(LOCK)) + 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) { +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 }; - if std::fs::symlink_metadata(dir.join(LOCK)).is_err() - && let Ok(lock) = new_lock(dir) - && lock.try_lock().is_ok() - { - run.lock = lock; + 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 @@ -148,7 +262,11 @@ fn take_over(own: &Path) { continue; } // Opened for writing: over NFS an exclusive lock on a file opened for reading is refused. - let Ok(lock) = std::fs::OpenOptions::new().write(true).open(dir.join(LOCK)) else { continue }; + 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(()) => {} @@ -158,6 +276,10 @@ fn take_over(own: &Path) { 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); } @@ -210,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); @@ -221,9 +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); + relock(dir, created)?; + #[cfg(target_arch = "wasm32")] + let _ = created; let mut n = 0; for d in &s.docs { if !d.is_dirty() { @@ -247,35 +371,45 @@ fn write_entry(dir: &Path, d: &DocState) -> 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())) } @@ -364,7 +498,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 }; @@ -392,7 +526,10 @@ pub fn list(dir: &Path) -> Vec<(u64, Value)> { /// 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"))] @@ -581,6 +718,76 @@ mod tests { 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() { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index d14c22ab4..469cf9d50 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2524,6 +2524,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/lib.rs b/crates/ui-egui/src/lib.rs index bdfe2e0f5..3db39c4b1 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -572,8 +572,14 @@ pub struct DesignApp { quit_requested: bool, /// Close the window without asking about unsaved changes (the control channel's `app.quit`). pub(crate) quit_without_asking: bool, - /// Quitting waits for the user's answer about the unsaved document with this uid. - quitting: Option, + /// 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 { @@ -1064,48 +1070,138 @@ impl DesignApp { self.quit_requested = true; } - /// Closing the window (close button, Quit) with unsaved documents: keep it open and ask about - /// each of them in turn with the dialog File › Close uses. Save and Don't Save go on to the - /// next one and the window closes after the last; Cancel, or a cancelled Save As, stops. + /// 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.session.documents().iter().any(|d| d.is_dirty()) { + 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); - let asking = self.quitting.is_some() && self.ui.dialog.as_ref().is_some_and(|d| d.id == "closeDocument"); + 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.ask_next_unsaved(ctx); + self.cancel_quit(); } return; } - let Some(uid) = self.quitting else { return }; if self.ui.dialog.is_some() { - return; - } - // Still open: the user cancelled, or the save didn't happen. - if self.session.documents().iter().any(|d| d.uid == uid) { - self.quitting = None; + self.cancel_quit(); return; } self.ask_next_unsaved(ctx); } - /// Ask about the first document with unsaved changes (shown, so the user sees which one), - /// or close the window when none is left. + /// 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.session.documents().iter().enumerate().find(|(_, d)| d.is_dirty()).map(|(i, d)| (i, d.uid)) else { - self.quitting = None; + let Some((i, uid)) = self.next_unapproved_document() else { ctx.send_viewport_cmd(egui::ViewportCommand::Close); return; }; - let _ = self.run("file.activate", json!({"index": i})); + if self.run("file.activate", json!({"index": i})).is_err() { + self.cancel_quit(); + return; + } menus::close_document(self, Some(i)); - self.quitting = Some(uid); + 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. A file without an @@ -1653,8 +1749,243 @@ mod tests { let cmds = quit_frame(&mut app, &ctx, false); assert_eq!(cmds.contains(&egui::ViewportCommand::Close), n == 1, "{cmds:?}"); } - // Only the saved document is left: the close goes through. + // 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] diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 1b4582215..c6db07837 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -288,7 +288,9 @@ 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). -- Crash recovery: unsaved documents are written to the recovery folder every 30 s and reopened (unsaved, remembering their files) after a crash; each running app writes to a folder of its own there, and an app that starts takes over only what apps that have ended left; File › Revert and Save a Copy…; `file.recovery.*`, `file.revert`, `file.saveACopy`. +- 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. - Graphics adapter chosen by the app: the GPU that drives the primary display (Windows `EnumDisplayDevices`, Linux sysfs), then the power preference (`WGPU_POWER_PREF`, `WGPU_ADAPTER_NAME` override). An adapter that fails or hangs while the window starts is left out and the app starts again on the next one, then on the next backend, with software renderers last; wgpu's indirect-call check is off (it lost the Metal device on an Intel Mac, #334); why the app stopped is logged, also when the window never opened (#273). Ported from VectorCraft. - 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). diff --git a/docs/ui-parity.md b/docs/ui-parity.md index c30d6c74c..060bc6715 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -51,7 +51,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) |