Platform: one save-staging helper for automation and the desktop app (#356)

Follow-up to #319, from review. create_staging, staging_suffixes and
STAGING_ATTEMPTS were copied in pdfcraft-automation and pdfcraft-ui-egui,
which are both L7 and can't depend on each other. They now live once in
the new pdfcraft-platform crate (L7, std only), which layers.rs already
reserved with both sideways edges.

A pure move: the body is unchanged apart from a StagingName parameter
that keeps each caller's existing file name (.{name}.{suffix}.pdfcraft-tmp
for automation, .{name}.pdfcraft-{suffix}.tmp for the desktop save). The
doc comment on staging_suffixes now says the suffix is SipHash keyed from
the OS random source: enough that a name can't be planted in advance, but
not a CSPRNG.

New tests in the platform crate pin both name forms, the 128-byte stem
cut, the 16-name limit and refusal when no suffix is left. The callers'
planted-file, link, folder and failed-rename tests stay where they are.

Co-authored-by: Claude Opus 5.5 <[email protected]>
Co-authored-by: Brandon Thomas <[email protected]>

Cargo.lock

@@ -3488,6 +3488,7 @@ dependencies = [
  "image",
  "pdfcraft-engine",
  "pdfcraft-organize",
+ "pdfcraft-platform",
  "pdfcraft-render",
  "pdfcraft-xfa",
  "png 0.18.1",
@@ -3703,6 +3704,10 @@ dependencies = [
  "thiserror 2.0.21",
 ]
 
+[[package]]
+name = "pdfcraft-platform"
+version = "0.4.0"
+
 [[package]]
 name = "pdfcraft-preflight"
 version = "0.4.0"
@@ -3792,6 +3797,7 @@ dependencies = [
  "pdfcraft-engine",
  "pdfcraft-fonts",
  "pdfcraft-model",
+ "pdfcraft-platform",
  "pdfcraft-render",
  "pdfcraft-xfa",
  "png 0.18.1",

Cargo.toml

@@ -40,6 +40,7 @@ pdfcraft-forms = { path = "crates/forms", version = "0.4.0" }
 pdfcraft-xfa = { path = "crates/xfa", version = "0.4.0" }
 pdfcraft-render = { path = "crates/render", version = "0.4.0" }
 pdfcraft-engine = { path = "crates/engine", version = "0.4.0" }
+pdfcraft-platform = { path = "crates/platform", version = "0.4.0" }
 pdfcraft-ui-egui = { path = "crates/ui-egui", version = "0.4.0" }
 pdfcraft-automation = { path = "crates/automation", version = "0.4.0", default-features = false }
 

crates/automation/Cargo.toml

@@ -12,6 +12,7 @@ workspace = true
 pdfcraft-engine = { workspace = true }
 pdfcraft-render = { workspace = true }
 pdfcraft-organize = { workspace = true }
+pdfcraft-platform = { workspace = true }
 serde_json = { workspace = true }
 png = "0.18"
 base64 = { version = "0.22", optional = true }

crates/automation/src/lib.rs

@@ -29,6 +29,7 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use pdfcraft_engine::{DocId, Document, Edit, Session, commands};
+use pdfcraft_platform::staging::{StagingName, create_staging, staging_suffixes};
 use pdfcraft_render::{PageRenderer, PageText, RenderConfig, RenderRequest, RequestKind};
 use serde_json::{Value, json};
 
@@ -1750,7 +1751,8 @@ fn write_atomic_with(path: &Path, bytes: &[u8], suffixes: impl IntoIterator<Item
     let dir = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(Path::new("."));
     let name = path.file_name().ok_or_else(|| failed(format!("{}: not a file path", path.display())))?;
     std::fs::create_dir_all(dir).map_err(|e| failed(format!("{}: {e}", dir.display())))?;
-    let (tmp, file) = create_staging(dir, &name.to_string_lossy(), suffixes).map_err(|e| failed(format!("{}: {e}", dir.display())))?;
+    let (tmp, file) =
+        create_staging(dir, &name.to_string_lossy(), StagingName::SuffixThenTag, suffixes).map_err(|e| failed(format!("{}: {e}", dir.display())))?;
     // Closed at the end of the block, before the rename.
     let written = {
         let mut file = file;
@@ -1766,49 +1768,10 @@ fn write_atomic_with(path: &Path, bytes: &[u8], suffixes: impl IntoIterator<Item
     })
 }
 
-/// How many staging names [`create_staging`] tries. A random 64-bit name is only taken if someone
-/// put a file there on purpose, so running out means refusing, not trying harder.
-const STAGING_ATTEMPTS: usize = 16;
-
-/// Create a new, empty staging file in `dir` for the file `name`, one name per suffix. It is
-/// opened with `create_new`, which fails if anything already has the name (a file, a hard link,
-/// a symbolic link even when dangling, a folder), on Windows as everywhere else; such a name is
-/// skipped, never opened, so a file planted at the staging path can't receive or redirect the
-/// write.
-fn create_staging(dir: &Path, name: &str, suffixes: impl IntoIterator<Item = u64>) -> std::io::Result<(PathBuf, std::fs::File)> {
-    // At most 128 bytes of the target's name, cut between characters, so the staging name fits
-    // the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is.
-    let mut stem = String::new();
-    for c in name.chars() {
-        if stem.len() + c.len_utf8() > 128 {
-            break;
-        }
-        stem.push(c);
-    }
-    for suffix in suffixes.into_iter().take(STAGING_ATTEMPTS) {
-        let tmp = dir.join(format!(".{stem}.{suffix:016x}.pdfcraft-tmp"));
-        match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
-            Ok(file) => return Ok((tmp, file)),
-            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
-            // Windows reports a folder at the name as "access denied"; it is taken all the same.
-            Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied && tmp.symlink_metadata().is_ok() => {}
-            Err(e) => return Err(e),
-        }
-    }
-    Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken"))
-}
-
-/// Unpredictable staging-name suffixes, so a name can't be planted in advance. `RandomState` is
-/// keyed from the operating system's random source.
-fn staging_suffixes() -> impl Iterator<Item = u64> {
-    use std::hash::BuildHasher;
-    let state = std::hash::RandomState::new();
-    (0u64..).map(move |i| state.hash_one(i))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
+    use pdfcraft_platform::staging::STAGING_ATTEMPTS;
 
     #[test]
     fn dot_dot_resolves_by_name_and_never_climbs_above_the_start() {

crates/platform/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+name = "pdfcraft-platform"
+description = "Operating-system services shared by the PdfCraft desktop app and automation tools"
+version.workspace = true
+edition.workspace = true
+license.workspace = true
+
+[lints]
+workspace = true
+
+[dependencies]

crates/platform/README.md

@@ -0,0 +1,7 @@
+# pdfcraft-platform
+
+Layer L7 holds operating-system services that both the desktop app (`pdfcraft-ui-egui`) and the automation tools (`pdfcraft-automation`) need. Those two crates sit on the same layer and can't depend on each other, so a helper they share lives here instead of being copied into each (`xtask/src/layers.rs` allows both edges). It uses only the standard library and has no UI dependencies.
+
+`staging` creates the temporary file an atomic save writes before renaming it over the target. The file is opened with `create_new`, so anything already at the name (a file, a hard link, a symbolic link even when dangling, a folder) is skipped and never written through. On Windows a folder at the name fails with "access denied" rather than "already exists", and is skipped only when something is actually there. Names carry an unpredictable 64-bit suffix from `staging_suffixes` (the standard library's keyed hash, currently SipHash-1-3, keyed from the operating system's random source), which stops a name being planted in advance but is not a CSPRNG; fixed suffixes are only for tests. At most 16 names are tried, and then the save is refused. The target's name is cut to 128 bytes on a character boundary so the staging name fits every system's limit. Each caller keeps its own name form (`.{name}.{suffix}.pdfcraft-tmp` for automation, `.{name}.pdfcraft-{suffix}.tmp` for the desktop save). Writing, closing the file before the rename, renaming and removing the staging file on failure stay with the caller.
+
+Synthetic tests pin both name forms, the 128-byte cut, the attempt limit and refusal when no name is left. The planted-file, link, folder and failed-rename tests stay with each caller, where they exercise the whole save.

crates/platform/src/lib.rs

@@ -0,0 +1,6 @@
+//! pdfcraft-platform — L7 operating-system services shared by the desktop app (`ui-egui`) and the
+//! automation tools (`automation`), which sit on the same layer and can't depend on each other.
+
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
+
+pub mod staging;

crates/platform/src/staging.rs

@@ -0,0 +1,129 @@
+//! Staging files for atomic saves: a new, unpredictably named file beside the target, written and
+//! then renamed over it. Writing, closing, renaming and cleaning up stay with the caller.
+
+use std::path::{Path, PathBuf};
+
+/// How many staging names [`create_staging`] tries. A random 64-bit name is only taken if someone
+/// put a file there on purpose, so running out means refusing, not trying harder.
+pub const STAGING_ATTEMPTS: usize = 16;
+
+/// Where the suffix goes in a staging name. Each caller keeps the form it has always used.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum StagingName {
+    /// `.{name}.{suffix}.pdfcraft-tmp`, used by the automation tools.
+    SuffixThenTag,
+    /// `.{name}.pdfcraft-{suffix}.tmp`, used by the desktop app's save.
+    TagThenSuffix,
+}
+
+/// Create a new, empty staging file in `dir` for the file `name`, one name per suffix. It is
+/// opened with `create_new`, which fails if anything already has the name (a file, a hard link,
+/// a symbolic link even when dangling, a folder), on Windows as everywhere else; such a name is
+/// skipped, never opened, so a file planted at the staging path can't receive or redirect the
+/// write. Pass [`staging_suffixes`]: fixed suffixes are for tests, and a predictable name can be
+/// taken in advance to refuse every save.
+pub fn create_staging(
+    dir: &Path,
+    name: &str,
+    form: StagingName,
+    suffixes: impl IntoIterator<Item = u64>,
+) -> std::io::Result<(PathBuf, std::fs::File)> {
+    // At most 128 bytes of the target's name, cut between characters, so the staging name fits
+    // the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is.
+    let mut stem = String::new();
+    for c in name.chars() {
+        if stem.len() + c.len_utf8() > 128 {
+            break;
+        }
+        stem.push(c);
+    }
+    for suffix in suffixes.into_iter().take(STAGING_ATTEMPTS) {
+        let tmp = dir.join(match form {
+            StagingName::SuffixThenTag => format!(".{stem}.{suffix:016x}.pdfcraft-tmp"),
+            StagingName::TagThenSuffix => format!(".{stem}.pdfcraft-{suffix:016x}.tmp"),
+        });
+        match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
+            Ok(file) => return Ok((tmp, file)),
+            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
+            // Windows reports a folder at the name as "access denied"; it is taken all the same.
+            Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied && tmp.symlink_metadata().is_ok() => {}
+            Err(e) => return Err(e),
+        }
+    }
+    Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken"))
+}
+
+/// Unpredictable staging-name suffixes, so a name can't be planted in advance. Each is the
+/// standard library's keyed hash of a counter (`RandomState::hash_one`, currently SipHash-1-3),
+/// keyed from the operating system's random source. That is all a staging name needs, but it is
+/// not a CSPRNG: don't use these where a value must stay secret or unguessable after it has been
+/// seen, such as a token or a key.
+pub fn staging_suffixes() -> impl Iterator<Item = u64> {
+    use std::hash::BuildHasher;
+    let state = std::hash::RandomState::new();
+    (0u64..).map(move |i| state.hash_one(i))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A fresh, empty folder for one test.
+    fn test_dir(test: &str) -> PathBuf {
+        let dir = std::env::temp_dir().join(format!("pdfcraft-platform-staging-{}-{test}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        dir
+    }
+
+    fn file_name(p: &Path) -> String {
+        p.file_name().unwrap().to_string_lossy().into_owned()
+    }
+
+    /// The callers' tests plant files at these exact names; if a name changed, those tests would
+    /// plant where nothing writes and pass without testing anything.
+    #[test]
+    fn each_form_keeps_its_exact_name() {
+        let dir = test_dir("forms");
+        let (tools, _) = create_staging(&dir, "out.pdf", StagingName::SuffixThenTag, [0xab]).unwrap();
+        assert_eq!(file_name(&tools), ".out.pdf.00000000000000ab.pdfcraft-tmp");
+        let (desktop, _) = create_staging(&dir, "out.pdf", StagingName::TagThenSuffix, [0xab]).unwrap();
+        assert_eq!(file_name(&desktop), ".out.pdf.pdfcraft-00000000000000ab.tmp");
+        assert_eq!(std::fs::read(&tools).unwrap(), b"", "a new, empty file");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    #[test]
+    fn a_long_name_is_cut_to_128_bytes_between_characters() {
+        let dir = test_dir("long");
+        // One byte, then four-byte characters: the 128-byte limit falls inside the 32nd one.
+        let name = format!("a{}.pdf", "\u{1F600}".repeat(40));
+        let (tmp, _) = create_staging(&dir, &name, StagingName::SuffixThenTag, [1]).unwrap();
+        assert_eq!(file_name(&tmp), format!(".a{}.0000000000000001.pdfcraft-tmp", "\u{1F600}".repeat(31)));
+        // One-byte characters: exactly 128 of them are kept.
+        let (tmp, _) = create_staging(&dir, &"x".repeat(200), StagingName::SuffixThenTag, [2]).unwrap();
+        assert_eq!(file_name(&tmp), format!(".{}.0000000000000002.pdfcraft-tmp", "x".repeat(128)));
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    #[test]
+    fn running_out_of_suffixes_refuses_and_creates_nothing() {
+        let dir = test_dir("empty");
+        let e = create_staging(&dir, "out.pdf", StagingName::TagThenSuffix, []).unwrap_err();
+        assert_eq!(e.kind(), std::io::ErrorKind::AlreadyExists);
+        assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 0);
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    #[test]
+    fn only_the_first_sixteen_names_are_tried() {
+        let dir = test_dir("cap");
+        let name = |s: u64| dir.join(format!(".out.pdf.{s:016x}.pdfcraft-tmp"));
+        for s in 1..=STAGING_ATTEMPTS as u64 {
+            std::fs::write(name(s), "PLANTED").unwrap();
+        }
+        assert!(create_staging(&dir, "out.pdf", StagingName::SuffixThenTag, 1..).is_err());
+        assert!(!name(STAGING_ATTEMPTS as u64 + 1).exists(), "a 17th name was tried");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+}

crates/ui-egui/Cargo.toml

@@ -12,6 +12,7 @@ workspace = true
 pdfcraft-fonts = { workspace = true }
 pdfcraft-engine = { workspace = true }
 pdfcraft-render = { workspace = true }
+pdfcraft-platform = { workspace = true }
 egui = { workspace = true }
 egui_extras = { workspace = true }
 eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "persistence", "links"] }

crates/ui-egui/src/editing.rs

@@ -2,6 +2,7 @@
 //! "save changes?" prompt when closing a tab or quitting with unsaved edits.
 
 use pdfcraft_engine::Edit;
+use pdfcraft_platform::staging::{StagingName, create_staging, staging_suffixes};
 
 use crate::PdfCraftApp;
 
@@ -527,7 +528,7 @@ fn write_atomically_with(path: &str, bytes: &[u8], suffixes: impl IntoIterator<I
     let target = std::path::Path::new(path);
     let dir = target.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(std::path::Path::new("."));
     let name = target.file_name().and_then(|n| n.to_str()).unwrap_or("save");
-    let (tmp, f) = create_staging(dir, name, suffixes)?;
+    let (tmp, f) = create_staging(dir, name, StagingName::TagThenSuffix, suffixes)?;
     let result = (|| {
         // Closed at the end of the block, before the rename.
         {
@@ -543,50 +544,6 @@ fn write_atomically_with(path: &str, bytes: &[u8], suffixes: impl IntoIterator<I
     result
 }
 
-/// How many staging names [`create_staging`] tries. A random 64-bit name is only taken if someone
-/// put a file there on purpose, so running out means refusing, not trying harder.
-const STAGING_ATTEMPTS: usize = 16;
-
-/// Create a new, empty staging file in `dir` for the file `name`, one name per suffix. It is
-/// opened with `create_new`, which fails if anything already has the name (a file, a hard link,
-/// a symbolic link even when dangling, a folder), on Windows as everywhere else; such a name is
-/// skipped, never opened, so a file planted at the staging path can't receive or redirect the
-/// save.
-fn create_staging(
-    dir: &std::path::Path,
-    name: &str,
-    suffixes: impl IntoIterator<Item = u64>,
-) -> std::io::Result<(std::path::PathBuf, std::fs::File)> {
-    // At most 128 bytes of the target's name, cut between characters, so the staging name fits
-    // the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is.
-    let mut stem = String::new();
-    for c in name.chars() {
-        if stem.len() + c.len_utf8() > 128 {
-            break;
-        }
-        stem.push(c);
-    }
-    for suffix in suffixes.into_iter().take(STAGING_ATTEMPTS) {
-        let tmp = dir.join(format!(".{stem}.pdfcraft-{suffix:016x}.tmp"));
-        match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) {
-            Ok(file) => return Ok((tmp, file)),
-            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
-            // Windows reports a folder at the name as "access denied"; it is taken all the same.
-            Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied && tmp.symlink_metadata().is_ok() => {}
-            Err(e) => return Err(e),
-        }
-    }
-    Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken"))
-}
-
-/// Unpredictable staging-name suffixes, so a name can't be planted in advance. `RandomState` is
-/// keyed from the operating system's random source.
-fn staging_suffixes() -> impl Iterator<Item = u64> {
-    use std::hash::BuildHasher;
-    let state = std::hash::RandomState::new();
-    (0u64..).map(move |i| state.hash_one(i))
-}
-
 #[cfg(not(target_arch = "wasm32"))]
 fn short_name(path: &str) -> String {
     std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string())
@@ -668,7 +625,8 @@ fn bookmark_at<'a>(items: &'a [pdfcraft_render::OutlineItem], path: &[usize]) ->
 
 #[cfg(test)]
 mod tests {
-    use super::{STAGING_ATTEMPTS, staging_suffixes, write_atomically, write_atomically_with};
+    use super::{write_atomically, write_atomically_with};
+    use pdfcraft_platform::staging::{STAGING_ATTEMPTS, staging_suffixes};
     use std::path::{Path, PathBuf};
 
     /// A fresh, empty folder for one staging test.