fix(automation): sync staged output before publication

Sync staged output after writing all bytes and before replacing its destination, closing the handle before rename. Return sync failures while keeping the old destination and ordinary save state available for retry.

Fixes #1804

crates/automation/README.md

@@ -25,7 +25,7 @@ mcp::McpServer::new(a).serve(stdin, stdout)?             // newline-delimited JS
 - Tools that change a document return its summary (`doc`, `pages`, `dirty`, `undo`, `redo`, …).
 - `comment_image_preview` returns JSON and a PNG: `layer: "background"` (the default) gives the page without the selected image signature/initials; `layer: "image"` gives its embedded image with alpha. The metadata supplies its displayed rectangle, document rotation, `image_rotation` (how far clockwise the image is shown turned: 0 for one placed by PdfCraft, which keeps it upright as displayed) and annotation opacity. Each result can be saved with the CLI's `--out`. Cache the background and transform the image for live movement/resizing; `comment_edit` commits the final geometry in one undo step. Previewing never changes the document.
 - For proportional image signature resizing, use the embedded image layer's width/height ratio rather than a previously stretched annotation rectangle. The GUI preserves this ratio at corners with the opposite corner anchored; edge midpoint handles change only their own axis. `comment_edit` accepts the resulting rectangle.
-- `doc_close` refuses to drop unsaved changes unless `discard_changes: true`. `doc_save` writes atomically, incrementally in place, and in full for a new path.
+- `doc_close` refuses to drop unsaved changes unless `discard_changes: true`. `doc_save` writes atomically, incrementally in place, and in full for a new path. Atomic output writes call `sync_all` on the staging file before replacing the destination. A write or sync error leaves the destination intact, removes staging and leaves the document dirty for retry. A filesystem that does not support syncing returns an error; there is no unsynced fallback. This flushes file contents, but does not guarantee persistence of the parent-directory rename across power loss.
 - With a root set, every read and write path must resolve inside it (symlinks and `..` included). Relative paths resolve inside the root, and `..` is resolved by name before the check. Every path outside the root gets the same refusal (`<path> is outside the allowed directory <root>`), whether or not it exists, so a confined agent can't probe the rest of the disk; on Windows another network share or device path (`\\host\share`, `\\?\UNC\…`, `\\.\…`) is refused without being contacted. "Not found" and other filesystem errors are reported only for paths inside the root. Tools that write several files into a folder name them after the document or input file, with separators, colons and control characters replaced by `_`, so a name can't lead them out of the folder. The root itself must be a folder.
 
 ## Text search and cache limits

crates/automation/src/lib.rs

@@ -858,6 +858,10 @@ impl Automation {
     }
 
     fn doc_save(&mut self, a: &Args) -> Result<Value> {
+        self.doc_save_with(a, write_atomic)
+    }
+
+    fn doc_save_with(&mut self, a: &Args, write: impl FnOnce(&Path, &[u8]) -> Result<()>) -> Result<Value> {
         let doc = self.doc(a)?;
         let id = doc.id;
         let target = match a.opt_str("path")? {
@@ -875,7 +879,7 @@ impl Automation {
         // A save that adds or removes encryption, or must not keep the old revision (Sanitize,
         // redaction), is rewritten in full even when an incremental save was asked for.
         let incremental = !full && bytes.starts_with(&self.doc(a)?.saved_bytes());
-        write_atomic(&target, &bytes)?;
+        write(&target, &bytes)?;
         let path = target.to_string_lossy().into_owned();
         self.session.mark_saved(id, bytes.clone(), Some(path.clone())).map_err(failed)?;
         Ok(json!({ "path": path, "bytes": bytes.len(), "incremental": incremental, "document": summary(self.doc(a)?) }))
@@ -2306,13 +2310,24 @@ fn unused(dir: &Path, stem: &str) -> PathBuf {
     (2..10_000u32).map(|n| child(dir, &format!("{stem} ({n}).pdf"))).find(|p| !p.exists()).unwrap_or(first)
 }
 
-/// Write via a temporary file in the same directory, then rename over the target.
+/// Write and sync a temporary file in the same directory, then rename over the target.
+/// This flushes staged contents; it does not promise parent-directory durability after a
+/// power loss. Filesystems that refuse sync cause the save to fail before publication.
 fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
     write_atomic_with(path, bytes, staging_suffixes())
 }
 
 /// [`write_atomic`], trying the staging names that `suffixes` give.
 fn write_atomic_with(path: &Path, bytes: &[u8], suffixes: impl IntoIterator<Item = u64>) -> Result<()> {
+    write_atomic_synced(path, bytes, suffixes, std::fs::File::sync_all)
+}
+
+fn write_atomic_synced(
+    path: &Path,
+    bytes: &[u8],
+    suffixes: impl IntoIterator<Item = u64>,
+    sync: impl FnOnce(&std::fs::File) -> std::io::Result<()>,
+) -> Result<()> {
     use std::io::Write;
     // A folder can't be replaced, and staging beside it could land outside the root: "." names
     // the root itself, whose parent isn't ours to write in.
@@ -2327,7 +2342,9 @@ fn write_atomic_with(path: &Path, bytes: &[u8], suffixes: impl IntoIterator<Item
     // Closed at the end of the block, before the rename.
     let written = {
         let mut file = file;
-        file.write_all(bytes)
+        // Surface delayed writeback failures before replacing the destination or marking
+        // a document clean. The handle is closed before rename (required on Windows).
+        file.write_all(bytes).and_then(|()| sync(&file))
     };
     if let Err(e) = written {
         let _ = std::fs::remove_file(&tmp);
@@ -2685,6 +2702,79 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    #[test]
+    fn a_failed_staging_sync_keeps_the_destination_and_cleans_up() {
+        for exists in [false, true] {
+            let dir = staging_dir(if exists { "sync-existing" } else { "sync-new" });
+            let target = dir.join("out.pdf");
+            if exists {
+                std::fs::write(&target, "OLD").unwrap();
+            }
+            let result = write_atomic_synced(&target, b"NEW", [1], |_| {
+                assert_eq!(std::fs::read(staged(&dir, 1)).unwrap(), b"NEW", "sync follows writing");
+                if exists {
+                    assert_eq!(read(&target), "OLD", "sync precedes publication");
+                }
+                Err(std::io::Error::other("injected storage sync failure"))
+            });
+            assert!(result.is_err(), "sync failure must refuse publication");
+            assert!(result.unwrap_err().to_string().contains("injected storage sync failure"));
+            if exists {
+                assert_eq!(read(&target), "OLD");
+                assert_eq!(listing(&dir), ["out.pdf"]);
+            } else {
+                assert!(!target.exists());
+                assert!(listing(&dir).is_empty());
+            }
+            std::fs::remove_dir_all(dir).unwrap();
+        }
+    }
+
+    #[test]
+    fn a_failed_save_sync_keeps_the_document_dirty_and_can_be_retried() {
+        let dir = staging_dir("sync-dirty");
+        let target = dir.join("out.pdf");
+        let mut a = Automation::new().with_root(&dir).unwrap();
+        let id = search_document(&mut a, 2);
+        json(&mut a, "doc_save", json!({"doc": id.0, "path": "out.pdf"}));
+        let old = std::fs::read(&target).unwrap();
+        json(&mut a, "page_rotate", json!({"doc": id.0, "pages": [1], "degrees": 90}));
+        let args = json!({"doc": id.0});
+        let before = summary(a.doc(&Args(&args)).unwrap());
+        let result = a.doc_save_with(&Args(&args), |path, bytes| {
+            write_atomic_synced(path, bytes, [1], |_| Err(std::io::Error::other("injected storage sync failure")))
+        });
+        assert!(result.is_err());
+        let doc = a.doc(&Args(&args)).unwrap();
+        assert!(doc.dirty);
+        assert_eq!(summary(doc), before, "failed save must not reset history or saved state");
+        assert_eq!(std::fs::read(&target).unwrap(), old);
+        assert_eq!(listing(&dir), ["out.pdf"]);
+        json(&mut a, "doc_save", args.clone());
+        assert!(!a.doc(&Args(&args)).unwrap().dirty);
+        assert_ne!(std::fs::read(&target).unwrap(), old);
+        std::fs::remove_dir_all(dir).unwrap();
+    }
+
+    #[test]
+    fn successful_staging_sync_precedes_publication() {
+        let dir = staging_dir("sync-success");
+        let target = dir.join("out.pdf");
+        std::fs::write(&target, "OLD").unwrap();
+        let mut called = false;
+        write_atomic_synced(&target, b"NEW", [1], |file| {
+            called = true;
+            assert_eq!(read(&target), "OLD");
+            assert_eq!(std::fs::read(staged(&dir, 1)).unwrap(), b"NEW");
+            file.sync_all()
+        })
+        .unwrap();
+        assert!(called);
+        assert_eq!(read(&target), "NEW");
+        assert_eq!(listing(&dir), ["out.pdf"]);
+        std::fs::remove_dir_all(dir).unwrap();
+    }
+
     /// The names in a folder: what a test can see was left behind.
     fn listing(dir: &Path) -> Vec<String> {
         let mut names: Vec<_> = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();