REC2792: retain original recovery until replacement journal is confirmed

Require the recovered revision and both journal writes before deleting
the original snapshot. Report disabled, failed or disconnected writes
and preserve discoverability when cleanup fails.

Refs #2792.

Co-authored-by: Codex <noreply@openai.com>

crates/engine/src/autosave.rs

@@ -648,7 +648,7 @@ enum Msg {
     Clean,
     Prefs(AutoSavePrefs),
     AutoSaveNow(Sender<Result<PathBuf, String>>),
-    Flush(Sender<()>),
+    Flush(Sender<Result<Option<u64>, String>>),
     Stop,
 }
 
@@ -771,12 +771,15 @@ impl Persistence {
         let _ = self.tx.send(Msg::Prefs(p));
     }
 
-    /// Write any pending journal now and wait for it.
-    pub fn flush(&self) {
+    /// Write any pending journal now and return its confirmed revision (`None` when clean).
+    /// Disabled journaling, failed writes and a stopped/unresponsive worker are errors.
+    pub fn flush(&self) -> Result<Option<u64>, String> {
         let (tx, rx) = channel();
-        if self.tx.send(Msg::Flush(tx)).is_ok() {
-            let _ = rx.recv_timeout(Duration::from_secs(30));
-        }
+        self.tx.send(Msg::Flush(tx)).map_err(|_| "auto-save worker stopped".to_string())?;
+        rx.recv_timeout(Duration::from_secs(30)).map_err(|e| match e {
+            RecvTimeoutError::Timeout => "recovery journal flush timed out".to_string(),
+            RecvTimeoutError::Disconnected => "auto-save worker stopped before confirming the recovery journal".to_string(),
+        })?
     }
 
     /// Write an auto-save now (if there are unsaved changes) and wait for the result.
@@ -818,7 +821,9 @@ impl Persistence {
     /// directory — marked as a clean exit — so the changes are offered next time; otherwise the
     /// directory is removed.
     pub fn close(mut self) {
-        self.flush();
+        if let Err(e) = self.flush() {
+            log::warn!("auto-save at shutdown: {e}");
+        }
         self.stop_worker();
         let snap = self.session_dir.join(SNAPSHOT);
         self.lock.take();
@@ -835,7 +840,7 @@ impl Persistence {
     /// Stop the worker and release the lock *without* cleaning up — what a crash leaves behind.
     #[doc(hidden)]
     pub fn simulate_crash(mut self) {
-        self.flush();
+        let _ = self.flush();
         self.stop_worker();
         self.lock.take();
     }
@@ -946,17 +951,21 @@ impl Worker {
                     let _ = reply.send(r);
                 }
                 Some(Msg::Flush(ack)) => {
-                    if self.journal_due().is_some() {
-                        self.write_journal();
-                    }
-                    let _ = ack.send(());
+                    let result = match self.latest.as_ref() {
+                        None => Ok(None),
+                        Some(_) if !self.prefs.recovery_journal => Err("recovery journal is disabled".to_string()),
+                        Some(s) if s.revision == self.journaled => Ok(Some(self.journaled)),
+                        Some(_) => self.write_journal().map(Some),
+                    };
+                    let _ = ack.send(result);
                 }
                 Some(Msg::Stop) => return,
                 None => {}
             }
             let now = Instant::now();
             if self.journal_due().is_some_and(|d| d <= now) {
-                self.write_journal();
+                // The worker reports write failures through its event channel.
+                let _ = self.write_journal();
             }
             if self.prefs.enabled && self.next_autosave <= now {
                 if self.latest.as_ref().is_some_and(|s| s.revision != self.autosaved) {
@@ -972,8 +981,8 @@ impl Worker {
         let _ = self.events.send(WorkerEvent::Error(e));
     }
 
-    fn write_journal(&mut self) {
-        let Some(s) = &self.latest else { return };
+    fn write_journal(&mut self) -> Result<u64, String> {
+        let Some(s) = &self.latest else { return Err("no unsaved recovery state to write".into()) };
         let t0 = Instant::now();
         let bytes = filmcraft_format::encode(&s.project, false);
         let serialize_ms = t0.elapsed().as_secs_f64() * 1000.0;
@@ -994,12 +1003,15 @@ impl Worker {
                 self.journaled = s.revision;
                 self.first_pending = None;
                 let _ = self.events.send(WorkerEvent::Journaled { revision: s.revision, unix, serialize_ms, write_ms, bytes: bytes.len() });
+                Ok(s.revision)
             }
             Err(e) => {
                 // Try again at the next interval rather than spinning.
                 self.first_pending = Some(Instant::now());
                 self.last_change = Instant::now();
-                self.error(format!("recovery journal: {e}"));
+                let message = format!("recovery journal: {e}");
+                self.error(message.clone());
+                Err(message)
             }
         }
     }
@@ -1056,3 +1068,29 @@ impl Worker {
         }
     }
 }
+
+#[cfg(test)]
+mod durability_tests {
+    use super::*;
+
+    #[test]
+    fn recovery_keeps_original_when_worker_has_stopped() {
+        let d = std::env::temp_dir().join(format!("fc-recover-worker-stopped-{}", std::process::id()));
+        let _ = fs::remove_dir_all(&d);
+        let old_dir = recovery_root(&d).join("1-1");
+        fs::create_dir_all(&old_dir).unwrap();
+        let bytes = filmcraft_format::encode(&Project::default(), false);
+        let original = old_dir.join(SNAPSHOT);
+        fs::write(&original, &bytes).unwrap();
+        let mut session = crate::Session::default();
+        session.start_autosave(AutosaveConfig::new(&d)).unwrap();
+        session.persistence.as_mut().unwrap().stop_worker();
+        let result = session.execute("file.recover", json!({"id": "1-1"}));
+        assert!(original.is_file(), "a stopped worker must not consume the original snapshot");
+        assert_eq!(fs::read(&original).unwrap(), bytes);
+        assert_eq!(session.recovery_candidates().len(), 1);
+        assert!(result.unwrap_err().to_string().contains("worker stopped"));
+        session.shutdown();
+        fs::remove_dir_all(&d).unwrap();
+    }
+}

crates/engine/src/autosave_tests.rs

@@ -231,6 +231,79 @@ fn quitting_with_unsaved_changes_keeps_them_for_next_launch() {
     let _ = std::fs::remove_dir_all(&d);
 }
 
+/// A disabled or unwritable replacement journal must never consume the original recovery copy.
+fn recovery_preserves_original(tag: &str, block: Option<&str>, journal_enabled: bool) {
+    let d = temp_dir(tag);
+    let old_dir = autosave::recovery_root(&d).join("1-1");
+    std::fs::create_dir_all(&old_dir).unwrap();
+    let project = (*demo().project).clone();
+    let bytes = filmcraft_format::encode(&project, false);
+    let original = old_dir.join("snapshot.fcproj");
+    std::fs::write(&original, &bytes).unwrap();
+
+    let mut s = Session::default();
+    let mut cfg = autosave::AutosaveConfig::new(&d);
+    cfg.journal_debounce = std::time::Duration::from_secs(60);
+    s.start_autosave(cfg).unwrap();
+    s.execute("prefs.set", json!({"key": "autoSave.recoveryJournal", "value": journal_enabled})).unwrap();
+    if let Some(name) = block {
+        // A directory where the atomic write needs a file reliably fails on Windows and Unix.
+        std::fs::create_dir(s.persistence.as_ref().unwrap().session_dir.join(name)).unwrap();
+    }
+    let result = s.execute("file.recover", json!({"id": "1-1"}));
+    assert!(original.is_file(), "failed replacement must preserve the original snapshot");
+    assert_eq!(std::fs::read(&original).unwrap(), bytes);
+    assert_eq!(s.recovery_candidates().len(), 1, "the original must remain discoverable");
+    assert!(result.is_err(), "lack of a confirmed replacement must be reported");
+    assert_eq!(*s.project, project, "the recovered project remains open for Save As");
+    assert!(s.is_dirty());
+    s.shutdown();
+    let found = autosave::scan_candidates(&d, None);
+    assert!(found.iter().any(|c| c.id == "1-1"), "another launch can still recover the original");
+    std::fs::remove_dir_all(&d).unwrap();
+}
+
+#[test]
+fn recovery_keeps_original_when_snapshot_write_fails() {
+    recovery_preserves_original("recover-snapshot-fails", Some("snapshot.fcproj"), true);
+}
+
+#[test]
+fn recovery_keeps_original_when_metadata_write_fails() {
+    recovery_preserves_original("recover-meta-fails", Some("session.json"), true);
+}
+
+#[test]
+fn recovery_keeps_original_when_journal_is_disabled() {
+    recovery_preserves_original("recover-journal-disabled", None, false);
+}
+
+#[test]
+fn recovery_can_retry_after_a_failed_journal_write() {
+    let d = temp_dir("recover-retry");
+    let old_dir = autosave::recovery_root(&d).join("1-1");
+    std::fs::create_dir_all(&old_dir).unwrap();
+    let project = (*demo().project).clone();
+    std::fs::write(old_dir.join("snapshot.fcproj"), filmcraft_format::encode(&project, false)).unwrap();
+    let mut s = Session::default();
+    let mut cfg = autosave::AutosaveConfig::new(&d);
+    cfg.journal_debounce = std::time::Duration::from_secs(60);
+    s.start_autosave(cfg).unwrap();
+    let obstruction = s.persistence.as_ref().unwrap().session_dir.join("snapshot.fcproj");
+    std::fs::create_dir(&obstruction).unwrap();
+    assert!(s.execute("file.recover", json!({"id": "1-1"})).is_err());
+    assert!(old_dir.join("snapshot.fcproj").is_file());
+    std::fs::remove_dir(&obstruction).unwrap();
+    s.execute("file.recover", json!({"id": "1-1"})).unwrap();
+    assert!(!old_dir.exists());
+    assert!(s.recovery_candidates().is_empty());
+    let replacement = std::fs::read(&obstruction).unwrap();
+    assert_eq!(filmcraft_format::decode(&replacement).unwrap().project, project);
+    assert_eq!(journaled(&mut s), s.revision);
+    s.shutdown();
+    std::fs::remove_dir_all(&d).unwrap();
+}
+
 #[test]
 fn auto_save_ring_rotates_and_can_save_the_project() {
     let d = temp_dir("ring");

crates/engine/src/commands.rs

@@ -4708,7 +4708,7 @@ fn recover(s: &mut Session, id: Option<&str>) -> Result<Value> {
         Some(id) => per.candidates.iter().position(|c| c.id == id).ok_or_else(|| bad("file.recover", format!("no recovery session `{id}`")))?,
         None => 0,
     };
-    let c = per.candidates[idx].clone();
+    let c = per.candidates.get(idx).cloned().ok_or_else(|| bad("file.recover", "no recovery session"))?;
     let saved_at = per.local_time(c.meta.saved_unix);
     let loaded = crate::autosave::load_candidate(&c).map_err(EngineError::Other)?;
     install_project(s, loaded.project, c.meta.project_path.clone(), false);
@@ -4718,11 +4718,17 @@ fn recover(s: &mut Session, id: Option<&str>) -> Result<Value> {
     crate::relink::on_open(s);
     // Our own journal must hold the recovered state before the old one is deleted.
     s.sync_persistence();
-    if let Some(per) = s.persistence.as_mut() {
-        per.flush();
-        per.candidates.remove(idx);
+    let per = s.persistence.as_mut().ok_or_else(|| EngineError::Other("recovery is not running".into()))?;
+    let preserve = |reason: String| {
+        EngineError::Other(format!(
+            "Recovered project is open, but its new recovery copy was not confirmed: {reason}. The original recovery copy is retained; save the project or use Save As."
+        ))
+    };
+    if per.flush().map_err(preserve)? != Some(s.revision) {
+        return Err(preserve("journal revision does not match the recovered project".into()));
     }
     crate::autosave::discard_candidate(&c).map_err(|e| EngineError::Other(e.to_string()))?;
+    per.candidates.remove(idx);
     s.toast(format!("Recovered unsaved changes to {} from {saved_at}", c.meta.project_name));
     Ok(json!({"id": c.id, "name": c.meta.project_name, "path": c.meta.project_path, "savedAt": saved_at, "revision": c.meta.revision}))
 }

docs/project-files.md

@@ -160,7 +160,10 @@ Premiere's auto-save can lose up to one interval of work. FilmCraft also keeps a
   the remaining candidates in the recovery list for retry. Sessions already deleted stay removed.
 - Recovering loads the snapshot as an **unsaved** project with its original path, so Save writes
   over the original file. The recovered state goes into the new session's own journal before the old
-  journal is deleted.
+  journal is deleted. Both the replacement snapshot and its metadata must be successfully written
+  for that recovered revision. If journaling is disabled, writing fails, or the worker cannot
+  confirm the write, recovery reports an error and retains the original candidate. The recovered
+  project remains open and unsaved; use Save or Save As to keep it.
 - A normal quit with nothing unsaved removes the session directory. Quitting with unsaved changes
   (Quit, Cmd+Q or the window's close button) first asks *"Save changes to 'X' before closing?"*
   with **Cancel**, **Don't Save** and **Save** (Save on a never-saved project opens the Save dialog);