Recovery: unique uids, per-instance ownership, keep snapshots until replaced (#390)

Document uids started at 1 in every process, so two instances sharing
the recovery folder overwrote and deleted each other's entries, and
file.recovery.open deleted each entry as soon as it was loaded, leaving
nothing if the app died before the next checkpoint.

- Uids are a random per-process base plus a counter (below 2^53).
- Each instance holds a lock on owner-<id>.lock in the folder; entries
  record their owner, and recovery open skips a running owner's entries
  and removes stale lock files.
- Opening an entry no longer deletes it: the recovered presentation keeps
  the entry's uid and takes ownership, so the snapshot stays until the
  next recovery save replaces it or a save/close removes it.
- Entry files and metadata are both written via temp file + rename.

Co-authored-by: Cursor <cursoragent@cursor.com>

crates/engine/src/lib.rs

@@ -175,7 +175,14 @@ pub struct DocState {
     pub composition: Option<Composition>,
 }
 
-static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
+static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
+
+/// A document id unique across processes, not just within this one: recovery entries are named
+/// by it in a folder every running instance shares. Ids are this process's random base (see
+/// [`recovery::instance_id`]) plus a counter.
+fn next_uid() -> u64 {
+    recovery::instance_id().wrapping_add(NEXT_UID.fetch_add(1, std::sync::atomic::Ordering::Relaxed)) & recovery::UID_MASK
+}
 
 impl DocState {
     pub fn new(doc: Presentation, path: Option<String>, title: String) -> Self {
@@ -188,7 +195,7 @@ impl DocState {
             path,
             revision: 1,
             interaction: None,
-            uid: NEXT_UID.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
+            uid: next_uid(),
             title,
             missing_fonts: Vec::new(),
             composition: None,

crates/engine/src/recovery.rs

@@ -5,9 +5,18 @@
 //! they were saved.
 //!
 //! An entry is `<uid>.deckcraft` (the presentation) plus `<uid>.json` (`{"path", "title",
-//! "saved"}`: the original file, the title and when it was written, Unix seconds).
+//! "saved", "owner"}`: the original file, the title, when it was written (Unix seconds) and the
+//! instance that owns it).
+//!
+//! Several instances share the folder. Document uids are unique across processes, each instance
+//! holds a lock on `owner-<instance>.lock` while it runs, and `file.recovery.open` leaves alone
+//! the entries of an instance whose lock is still held. Opening an entry doesn't delete it: the
+//! recovered presentation keeps the entry's uid, so the snapshot stays on disk until the next
+//! recovery save replaces it or a save of the presentation removes it.
 
+use std::fs::File;
 use std::path::{Path, PathBuf};
+use std::sync::{Mutex, OnceLock};
 
 use serde_json::{Value, json};
 
@@ -25,6 +34,56 @@ pub fn default_dir() -> Option<PathBuf> {
     std::env::var_os("XDG_DATA_HOME").map(PathBuf::from).or_else(|| home.map(|h| h.join(".local/share"))).map(|d| d.join("deckcraft/recovery"))
 }
 
+/// Uids and instance ids stay below 2^53 so they survive JSON numbers read as `f64`.
+pub(crate) const UID_MASK: u64 = (1 << 53) - 1;
+
+/// This process's random id, with the low 24 bits clear so its document uids (id + counter) don't
+/// run into another instance's.
+pub fn instance_id() -> u64 {
+    static ID: OnceLock<u64> = OnceLock::new();
+    *ID.get_or_init(|| {
+        use std::hash::{BuildHasher, Hasher};
+        let mut h = std::collections::hash_map::RandomState::new().build_hasher();
+        h.write_u32(std::process::id());
+        #[cfg(not(target_arch = "wasm32"))]
+        if let Ok(d) = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH) {
+            h.write_u128(d.as_nanos());
+        }
+        // Never 0: old entries are numbered from 1.
+        ((h.finish() & UID_MASK) & !0xFF_FFFF).max(1 << 24)
+    })
+}
+
+fn lock_path(dir: &Path, owner: u64) -> PathBuf {
+    dir.join(format!("owner-{owner}.lock"))
+}
+
+/// Hold this instance's lock in `dir` for the rest of the process (once per folder).
+fn claim_dir(dir: &Path) {
+    static HELD: Mutex<Vec<(PathBuf, File)>> = Mutex::new(Vec::new());
+    let Ok(mut held) = HELD.lock() else { return };
+    if held.iter().any(|(d, _)| d == dir) {
+        return;
+    }
+    let path = lock_path(dir, instance_id());
+    match File::create(&path).and_then(|f| f.try_lock().map(|()| f).map_err(std::io::Error::other)) {
+        Ok(f) => held.push((dir.to_path_buf(), f)),
+        Err(e) => log::warn!("recovery: could not lock {}: {e}", path.display()),
+    }
+}
+
+/// Whether another running instance holds `owner`'s lock in `dir`.
+fn owner_alive(dir: &Path, owner: u64) -> bool {
+    let Ok(f) = File::open(lock_path(dir, owner)) else { return false };
+    matches!(f.try_lock(), Err(std::fs::TryLockError::WouldBlock))
+}
+
+/// Write `bytes` to `path` through a temporary file and a rename, so a crash never leaves it torn.
+fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
+    let tmp = path.with_extension(format!("{}.tmp", path.extension().and_then(|e| e.to_str()).unwrap_or("")));
+    std::fs::write(&tmp, bytes).and_then(|_| std::fs::rename(&tmp, path)).map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))
+}
+
 fn files(dir: &Path, uid: u64) -> (PathBuf, PathBuf) {
     (dir.join(format!("{uid}.deckcraft")), dir.join(format!("{uid}.json")))
 }
@@ -51,6 +110,7 @@ pub fn discard(dir: &Path, uid: u64) {
 /// were written.
 pub fn save(s: &Session, dir: &Path) -> Result<usize> {
     std::fs::create_dir_all(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?;
+    claim_dir(dir);
     let mut n = 0;
     for d in s.documents() {
         if !d.is_dirty() {
@@ -59,11 +119,9 @@ pub fn save(s: &Session, dir: &Path) -> Result<usize> {
         }
         let (doc, meta) = files(dir, d.uid);
         let bytes = deckcraft_format::save(&d.doc).map_err(|e| EngineError::Other(e.to_string()))?;
-        // Write then rename, so a crash mid-write never leaves a torn file.
-        let tmp = doc.with_extension("tmp");
-        std::fs::write(&tmp, &bytes).and_then(|_| std::fs::rename(&tmp, &doc)).map_err(|e| EngineError::Other(format!("{}: {e}", doc.display())))?;
-        let m = json!({"path": d.path, "title": d.title(), "saved": now_secs()});
-        std::fs::write(&meta, m.to_string()).map_err(|e| EngineError::Other(format!("{}: {e}", meta.display())))?;
+        write_atomic(&doc, &bytes)?;
+        let m = json!({"path": d.path, "title": d.title(), "saved": now_secs(), "owner": instance_id()});
+        write_atomic(&meta, m.to_string().as_bytes())?;
         n += 1;
     }
     Ok(n)
@@ -91,27 +149,67 @@ pub fn list(dir: &Path) -> Vec<(u64, Value)> {
     out
 }
 
-/// Reopen every entry as an unsaved presentation and remove the entries (this session writes its
-/// own). Unreadable entries are left in place. Returns the opened documents' indices.
+/// Reopen every entry left by an instance that is no longer running as an unsaved presentation.
+/// Entries stay on disk, now owned by this instance: the presentation keeps the entry's uid, so
+/// the next recovery save replaces the snapshot and saving or closing it removes it. Entries of a
+/// running instance, unreadable entries and entries already open here are left alone. Returns the
+/// opened documents' indices.
 pub fn open(s: &mut Session, dir: &Path) -> Result<Vec<usize>> {
+    let me = instance_id();
     let mut opened = Vec::new();
-    for (uid, meta) in list(dir) {
-        let (doc, _) = files(dir, uid);
+    for (uid, mut meta) in list(dir) {
+        if s.documents().iter().any(|d| d.uid == uid) {
+            continue;
+        }
+        if let Some(owner) = meta.get("owner").and_then(Value::as_u64)
+            && owner != me
+            && owner_alive(dir, owner)
+        {
+            continue;
+        }
+        let (doc, meta_path) = files(dir, uid);
         let Ok(bytes) = std::fs::read(&doc) else { continue };
         let Ok(mut d) = deckcraft_format::load(&bytes) else { continue };
         deckcraft_format::repair(&mut d);
         let path = meta.get("path").and_then(Value::as_str).map(str::to_string);
-        let title = meta.get("title").and_then(Value::as_str).filter(|t| !t.is_empty()).unwrap_or("Presentation");
+        let title = meta.get("title").and_then(Value::as_str).filter(|t| !t.is_empty()).unwrap_or("Presentation").to_string();
+        // Take the entry over (so another instance starting now leaves it alone), keeping its
+        // snapshot until this one writes a newer one.
+        claim_dir(dir);
+        if let Some(o) = meta.as_object_mut() {
+            o.insert("owner".into(), json!(me));
+        }
+        if let Err(e) = write_atomic(&meta_path, meta.to_string().as_bytes()) {
+            log::warn!("recovery: {e}");
+        }
         let mut st = DocState::new(d, path, format!("{title} (Recovered)"));
+        st.uid = uid;
         // Unsaved: the copy on disk (if any) is older than what was recovered.
         st.saved_doc = std::sync::Arc::new((*st.doc).clone());
         st.revision += 1;
         opened.push(s.add_document(st));
-        discard(dir, uid);
     }
+    remove_stale_locks(dir);
     Ok(opened)
 }
 
+/// Delete the lock files of instances that are no longer running.
+fn remove_stale_locks(dir: &Path) {
+    let me = instance_id();
+    let Ok(rd) = std::fs::read_dir(dir) else { return };
+    for e in rd.flatten() {
+        let name = e.file_name();
+        let Some(owner) =
+            name.to_str().and_then(|n| n.strip_prefix("owner-")).and_then(|n| n.strip_suffix(".lock")).and_then(|n| n.parse::<u64>().ok())
+        else {
+            continue;
+        };
+        if owner != me && !owner_alive(dir, owner) {
+            let _ = std::fs::remove_file(e.path());
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -139,16 +237,67 @@ mod tests {
         assert!(st.is_dirty());
         assert_eq!(st.doc.slides.len(), 2);
         assert!(st.title().ends_with("(Recovered)"));
-        assert!(list(&dir).is_empty(), "entries are consumed");
-        // Saving drops the entry.
-        s.recovery_dir = Some(dir.clone());
-        assert_eq!(save(&s, &dir).unwrap(), 1);
+        assert_eq!(list(&dir).len(), 1, "the snapshot stays until it is replaced or saved");
+        // A second "crash" before any checkpoint: the presentation is recovered again.
+        let mut s3 = Session::new();
+        assert_eq!(open(&mut s3, &dir).unwrap().len(), 1);
+        assert_eq!(s3.doc().unwrap().doc.slides.len(), 2);
+        // Opening again in the same session doesn't duplicate it.
+        assert!(open(&mut s3, &dir).unwrap().is_empty());
+        // Saving the recovered presentation drops the entry.
+        s3.recovery_dir = Some(dir.clone());
         let path = dir.join("saved.deckcraft");
-        s.execute("file.saveAs", &json!({"path": path.to_string_lossy()})).unwrap();
+        s3.execute("file.saveAs", &json!({"path": path.to_string_lossy()})).unwrap();
         assert!(list(&dir).is_empty());
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    #[test]
+    fn uids_are_unique_beyond_this_process() {
+        let a = DocState::new(Default::default(), None, "a".into());
+        let b = DocState::new(Default::default(), None, "b".into());
+        assert_ne!(a.uid, b.uid);
+        // Not the small per-process numbers every instance used to start from.
+        assert!(a.uid >= 1 << 24 && a.uid <= UID_MASK);
+    }
+
+    #[test]
+    fn another_running_instance_keeps_its_entries() {
+        let dir = temp("recovery-shared");
+        std::fs::create_dir_all(&dir).unwrap();
+        // Another instance (its own owner id and uid, its lock held) has an unsaved presentation.
+        let other = instance_id() ^ (1 << 40);
+        let lock = File::create(lock_path(&dir, other)).unwrap();
+        lock.try_lock().unwrap();
+        let mut o = Session::new();
+        o.execute("file.new", &json!({})).unwrap();
+        o.execute("slide.new", &json!({"layout": "blank"})).unwrap();
+        let bytes = deckcraft_format::save(&o.doc().unwrap().doc).unwrap();
+        let uid = other + 5;
+        let (d, m) = files(&dir, uid);
+        std::fs::write(&d, bytes).unwrap();
+        std::fs::write(&m, json!({"title": "Theirs", "owner": other}).to_string()).unwrap();
+        // This instance saving its clean and dirty presentations leaves that entry alone...
+        let mut s = Session::new();
+        s.execute("file.new", &json!({})).unwrap();
+        s.execute("file.new", &json!({})).unwrap();
+        s.execute("slide.new", &json!({"layout": "blank"})).unwrap();
+        assert_eq!(save(&s, &dir).unwrap(), 1);
+        assert!(list(&dir).iter().any(|e| e.0 == uid));
+        // ...and a recovery open here skips it while its owner runs.
+        let mut s2 = Session::new();
+        let opened = open(&mut s2, &dir).unwrap();
+        assert!(s2.documents().iter().all(|d| d.uid != uid), "{opened:?}");
+        assert!(lock_path(&dir, other).exists(), "a live owner's lock stays");
+        // Once the owner is gone, its entry is recovered and its lock file cleaned up.
+        drop(lock);
+        let mut s3 = Session::new();
+        open(&mut s3, &dir).unwrap();
+        assert!(s3.documents().iter().any(|d| d.uid == uid && d.title().starts_with("Theirs")));
+        assert!(!lock_path(&dir, other).exists());
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn corrupt_entries_are_skipped() {
         let dir = temp("recovery-corrupt");