Fix #1231: keep autosave completion tied to its originating project (#1330)

crates/engine/src/autosave.rs

@@ -47,6 +47,11 @@ pub struct AutoSaveState {
 #[derive(Clone)]
 pub struct PendingWrite {
     pub path: PathBuf,
+    pub(crate) slot: u32,
+    pub(crate) revision: u64,
+    /// Originating project identity: the path changes on Save As, the epoch on Open/New.
+    pub(crate) project_path: Option<String>,
+    pub(crate) project_epoch: u64,
     handle: std::sync::Arc<std::sync::Mutex<Option<std::thread::JoinHandle<std::io::Result<()>>>>>,
 }
 
@@ -57,9 +62,16 @@ impl std::fmt::Debug for PendingWrite {
 }
 
 impl PendingWrite {
-    pub fn spawn(path: PathBuf, work: impl FnOnce() -> std::io::Result<()> + Send + 'static) -> std::io::Result<PendingWrite> {
+    pub fn spawn(
+        path: PathBuf,
+        slot: u32,
+        revision: u64,
+        project_path: Option<String>,
+        project_epoch: u64,
+        work: impl FnOnce() -> std::io::Result<()> + Send + 'static,
+    ) -> std::io::Result<PendingWrite> {
         let h = std::thread::Builder::new().name("autosave".into()).spawn(work)?;
-        Ok(PendingWrite { path, handle: std::sync::Arc::new(std::sync::Mutex::new(Some(h))) })
+        Ok(PendingWrite { path, slot, revision, project_path, project_epoch, handle: std::sync::Arc::new(std::sync::Mutex::new(Some(h))) })
     }
     pub fn is_finished(&self) -> bool {
         self.handle.lock().map(|h| h.as_ref().is_none_or(|h| h.is_finished())).unwrap_or(true)

crates/engine/src/session_settings.rs

@@ -225,36 +225,50 @@ impl Session {
         let prefs = self.autosave_prefs();
         let fail = |e: std::io::Error| EngineError::Other(format!("auto-save failed: {e}"));
         let plan = autosave::plan_in(self.file_ops(), &prefs, self.path.as_deref(), root.as_deref(), self.autosave.last_slot).map_err(fail)?;
+        let path = plan.path.to_string_lossy().into_owned();
         if self.autosave.background && !cfg!(target_arch = "wasm32") {
             let project = self.project.clone();
             let config = self.config.clone();
             let job = plan.clone();
-            let pending = autosave::PendingWrite::spawn(plan.path.clone(), move || {
-                let json = project.to_json_compact();
-                match &config {
-                    Some(c) => job.write(c.files(), json.as_bytes()),
-                    None => job.write(&crate::config::StdFiles, json.as_bytes()),
-                }
-            })
-            .map_err(fail)?;
+            let pending =
+                autosave::PendingWrite::spawn(plan.path.clone(), plan.slot, self.revision, self.path.clone(), self.script_ui.project_epoch, move || {
+                    let json = project.to_json_compact();
+                    match &config {
+                        Some(c) => job.write(c.files(), json.as_bytes()),
+                        None => job.write(&crate::config::StdFiles, json.as_bytes()),
+                    }
+                })
+                .map_err(fail)?;
             self.autosave.pending = Some(pending);
         } else {
             let json = self.project.to_json_compact();
             plan.write(self.file_ops(), json.as_bytes()).map_err(fail)?;
+            self.commit_autosave(path.clone(), plan.slot, self.revision);
         }
-        let (path, slot) = (plan.path, plan.slot);
-        let p = path.to_string_lossy().to_string();
+        Ok(path)
+    }
+
+    /// Advance recovery bookkeeping only for a successfully written snapshot.
+    fn commit_autosave(&mut self, path: String, slot: u32, revision: u64) {
         self.autosave.last_slot = Some(slot);
-        self.autosave.saved_revision = Some(self.revision);
-        self.autosave.last_path = Some(p.clone());
+        self.autosave.saved_revision = Some(revision);
+        self.autosave.last_path = Some(path);
         self.update_sentinel();
-        Ok(p)
     }
 
     /// Wait for a background auto-save ([`autosave::AutoSaveState::background`]) to land.
     pub fn autosave_wait(&mut self) -> Result<()> {
         match self.autosave.pending.take() {
-            Some(w) => w.join().map_err(|e| EngineError::Other(format!("auto-save failed: {e}"))),
+            Some(w) => {
+                w.join().map_err(|e| EngineError::Other(format!("auto-save failed: {e}")))?;
+                // Open/New can replace the project before note_project_path collects this
+                // write. Save As changes only its path. Never publish the old snapshot as the
+                // new project's recovery record, even when that record's final reset fails.
+                if w.project_path == self.path && w.project_epoch == self.script_ui.project_epoch {
+                    self.commit_autosave(w.path.to_string_lossy().into_owned(), w.slot, w.revision);
+                }
+                Ok(())
+            }
             None => Ok(()),
         }
     }
@@ -278,7 +292,7 @@ impl Session {
             return None;
         }
         self.autosave.last = Some(now);
-        if !self.is_dirty() || self.autosave.saved_revision == Some(self.revision) || self.render_job.is_some() {
+        if !self.is_dirty() || self.autosave.pending.is_some() || self.autosave.saved_revision == Some(self.revision) || self.render_job.is_some() {
             return None;
         }
         if self.path.is_none() && self.prefs.auto_save.location != "custom" && self.default_autosave_root().is_none() && self.autosave_folder_override.is_none()

crates/engine/src/tests_settings.rs

@@ -608,6 +608,216 @@ fn atomic_write_does_not_follow_a_link_at_the_old_temp_name() {
     let _ = std::fs::remove_dir_all(d);
 }
 
+struct AutosaveTestStore {
+    config: DirConfig,
+    fail_next: std::sync::atomic::AtomicBool,
+    gate: std::sync::Mutex<Option<(std::sync::mpsc::Sender<()>, std::sync::mpsc::Receiver<()>)>>,
+    release_on_prefs: std::sync::Mutex<Option<std::sync::mpsc::Sender<()>>>,
+    fail_reset_for: std::sync::Mutex<Option<String>>,
+    failed_resets: std::sync::atomic::AtomicUsize,
+}
+
+impl ConfigStore for AutosaveTestStore {
+    fn read(&self, name: &str) -> Option<String> {
+        self.config.read(name)
+    }
+    fn write(&self, name: &str, data: &str) -> std::io::Result<()> {
+        // note_project_path saves preferences after the project/path switch, before joining
+        // the pending autosave. Release that writer at this exact transition, without sleeps.
+        if name == PREFS_FILE
+            && let Some(release) = self.release_on_prefs.lock().unwrap().take()
+        {
+            release.send(()).unwrap();
+        }
+        if name == autosave::SENTINEL
+            && let Some(path) = self.fail_reset_for.lock().unwrap().as_ref()
+        {
+            let record: autosave::Sentinel = serde_json::from_str(data).unwrap();
+            if record.project.as_ref() == Some(path) && record.autosave.is_none() {
+                self.failed_resets.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
+                return Err(std::io::Error::other("injected final recovery reset failure"));
+            }
+        }
+        self.config.write(name, data)
+    }
+    fn remove(&self, name: &str) -> std::io::Result<()> {
+        self.config.remove(name)
+    }
+    fn dir(&self) -> Option<PathBuf> {
+        self.config.dir()
+    }
+    fn files(&self) -> &dyn crate::config::FileOps {
+        self
+    }
+}
+
+impl crate::config::FileOps for AutosaveTestStore {
+    fn list(&self, dir: &std::path::Path) -> Vec<(String, std::time::SystemTime)> {
+        crate::config::StdFiles.list(dir)
+    }
+    fn write(&self, path: &std::path::Path, data: &[u8]) -> std::io::Result<()> {
+        if let Some((started, release)) = self.gate.lock().unwrap().take() {
+            started.send(()).unwrap();
+            release.recv().unwrap();
+        }
+        if self.fail_next.swap(false, std::sync::atomic::Ordering::SeqCst) {
+            return Err(std::io::Error::other("injected autosave write failure"));
+        }
+        crate::config::StdFiles.write(path, data)
+    }
+    fn remove(&self, path: &std::path::Path) -> std::io::Result<()> {
+        crate::config::StdFiles.remove(path)
+    }
+    fn is_file(&self, path: &std::path::Path) -> bool {
+        crate::config::StdFiles.is_file(path)
+    }
+}
+
+fn background_autosave_session(dir: &std::path::Path) -> (Session, Arc<AutosaveTestStore>) {
+    let store = Arc::new(AutosaveTestStore {
+        config: DirConfig::new(dir.join("config")),
+        fail_next: Default::default(),
+        gate: Default::default(),
+        release_on_prefs: Default::default(),
+        fail_reset_for: Default::default(),
+        failed_resets: Default::default(),
+    });
+    let mut s = session_with_comp();
+    s.config = Some(store.clone());
+    s.prefs.auto_save.enabled = true;
+    s.prefs.auto_save.interval_minutes = 1;
+    s.prefs.auto_save.location = "custom".into();
+    s.prefs.auto_save.folder = dir.join("saves").to_string_lossy().into_owned();
+    s.autosave.background = true;
+    s.begin_recovery();
+    (s, store)
+}
+
+#[test]
+fn background_autosave_failure_retries_the_same_revision_and_retains_recovery() {
+    let dir = tmp("autosave-retry");
+    let (mut s, store) = background_autosave_session(&dir);
+    let first = s.autosave_now().unwrap();
+    s.autosave_wait().unwrap();
+    let committed = s.autosave.saved_revision;
+    s.execute("layer.newNull", json!({"name": "work to recover"})).unwrap();
+    let revision = s.revision;
+    store.fail_next.store(true, std::sync::atomic::Ordering::SeqCst);
+    assert!(s.autosave_tick(1000.0).is_none());
+    let failed_path = s.autosave_tick(1061.0).unwrap().unwrap();
+    assert!(s.autosave_wait().unwrap_err().to_string().contains("injected autosave write failure"));
+    assert_eq!(s.autosave.saved_revision, committed);
+    assert_eq!(s.autosave.last_slot, Some(1));
+    assert_eq!(s.autosave.last_path.as_deref(), Some(first.as_str()));
+    let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap();
+    assert_eq!(sentinel.autosave.as_deref(), Some(first.as_str()));
+    assert!(!std::path::Path::new(&failed_path).is_file());
+    let retried = s.autosave_tick(1122.0).expect("retry without another edit").unwrap();
+    assert_eq!(retried, failed_path, "failed writes do not rotate the slot");
+    s.autosave_wait().unwrap();
+    assert_eq!(s.revision, revision);
+    assert_eq!(s.autosave.saved_revision, Some(revision));
+    assert_eq!(s.autosave.last_slot, Some(2));
+    assert!(std::fs::read_to_string(&retried).unwrap().contains("work to recover"));
+    let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap();
+    assert_eq!(sentinel.autosave.as_deref(), Some(retried.as_str()));
+    assert!(s.autosave_tick(1183.0).is_none(), "committed revision needs no duplicate write");
+    std::fs::remove_dir_all(dir).unwrap();
+}
+
+#[test]
+fn background_autosave_commits_only_the_snapshot_that_finished() {
+    let dir = tmp("autosave-pending");
+    let (mut s, store) = background_autosave_session(&dir);
+    let (started, started_rx) = std::sync::mpsc::channel();
+    let (release, release_rx) = std::sync::mpsc::channel();
+    *store.gate.lock().unwrap() = Some((started, release_rx));
+    let revision = s.revision;
+    let path = s.autosave_now().unwrap();
+    started_rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap();
+    assert!(s.autosave_tick(1000.0).is_none());
+    let saved = s.autosave.saved_revision;
+    let last_path = s.autosave.last_path.clone();
+    let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap();
+    s.execute("layer.newNull", json!({"name": "edited during save"})).unwrap();
+    assert!(s.autosave_tick(1061.0).is_none(), "a pending write must not block the next tick or launch a duplicate");
+    release.send(()).unwrap();
+    s.autosave_wait().unwrap();
+    assert_eq!(saved, None, "an in-flight save is not committed");
+    assert_eq!(last_path, None);
+    assert_eq!(sentinel.autosave, None);
+    assert_eq!(s.autosave.saved_revision, Some(revision), "completion must not mark newer edits as saved");
+    assert_eq!(s.autosave.last_path.as_deref(), Some(path.as_str()));
+    assert!(!std::fs::read_to_string(&path).unwrap().contains("edited during save"));
+    let newer = s.autosave_tick(1122.0).unwrap().unwrap();
+    s.autosave_wait().unwrap();
+    assert_eq!(s.autosave.saved_revision, Some(s.revision));
+    assert!(std::fs::read_to_string(newer).unwrap().contains("edited during save"));
+    std::fs::remove_dir_all(dir).unwrap();
+}
+
+fn autosave_project_transition_with_failed_reset(command: &str, reopen: bool) {
+    let dir = tmp("autosave-project-transition");
+    let a = dir.join("A.ecproj").to_string_lossy().into_owned();
+    let b = if reopen { a.clone() } else { dir.join("B.ecproj").to_string_lossy().into_owned() };
+    let (mut s, store) = background_autosave_session(&dir);
+    s.execute("file.saveAs", json!({"path": a})).unwrap();
+    if !reopen {
+        let mut other = session_with_comp();
+        other.execute("layer.newNull", json!({"name": "project B"})).unwrap();
+        other.execute("file.saveAs", json!({"path": b})).unwrap();
+    }
+    let b_bytes = std::fs::read(&b).unwrap();
+    s.execute("layer.newNull", json!({"name": "unsaved project A"})).unwrap();
+    s.update_sentinel();
+    let (started, started_rx) = std::sync::mpsc::channel();
+    let (release, release_rx) = std::sync::mpsc::channel();
+    *store.gate.lock().unwrap() = Some((started, release_rx));
+    let snapshot = s.autosave_now().unwrap();
+    started_rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap();
+    assert!(!s.autosave.pending.as_ref().unwrap().is_finished());
+    let previous_record = store.read(autosave::SENTINEL).unwrap();
+    *store.fail_reset_for.lock().unwrap() = Some(b.clone());
+    *store.release_on_prefs.lock().unwrap() = Some(release);
+    s.execute(command, json!({"path": b})).unwrap();
+    assert_eq!(s.path.as_deref(), Some(b.as_str()));
+    assert_eq!(store.failed_resets.load(std::sync::atomic::Ordering::SeqCst), 1, "the final recovery reset really failed");
+    assert_eq!(
+        store.read(autosave::SENTINEL).as_deref(),
+        Some(previous_record.as_str()),
+        "completion must not publish A's snapshot under the switched project"
+    );
+    assert_eq!(s.autosave.last_path, None);
+    assert_eq!(s.autosave.saved_revision, None);
+    if command == "file.open" {
+        assert_eq!(std::fs::read(&b).unwrap(), b_bytes, "opening B must not alter its file bytes");
+    }
+    let prefs = s.prefs.clone();
+    drop(s); // Simulate an unclean exit, leaving the failed-reset sentinel in place.
+    let mut restart = Session { config: Some(store), prefs, ..Default::default() };
+    let recovery = restart.begin_recovery().expect("previous project remains recoverable");
+    assert_eq!(recovery.project.as_deref(), Some(a.as_str()));
+    assert_eq!(recovery.autosave.as_deref(), Some(snapshot.as_str()));
+    assert!(recovery.dirty);
+    assert!(std::fs::read_to_string(snapshot).unwrap().contains("unsaved project A"));
+    std::fs::remove_dir_all(dir).unwrap();
+}
+
+#[test]
+fn opening_another_project_during_autosave_keeps_recovery_consistent_if_the_reset_fails() {
+    autosave_project_transition_with_failed_reset("file.open", false);
+}
+
+#[test]
+fn reopening_the_same_path_during_autosave_keeps_recovery_consistent_if_the_reset_fails() {
+    autosave_project_transition_with_failed_reset("file.open", true);
+}
+
+#[test]
+fn save_as_during_autosave_keeps_recovery_consistent_if_the_reset_fails() {
+    autosave_project_transition_with_failed_reset("file.saveAs", false);
+}
+
 #[test]
 fn atomic_write_survives_a_crash_mid_write() {
     let d = tmp("atomic");