From fd7ac2fe4fc7b7fd369c5a67cd8d069b35d5a0c5 Mon Sep 17 00:00:00 2001 From: jahvari <75337667+jahvari@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:16:05 -0500 Subject: [PATCH] REC2792: retain recovery candidates when discard fails (#2993) Stop deleting after the first filesystem error and restore failed, unprocessed and unselected candidates before returning it. Keep completed deletions removed. Three real filesystem regressions reproduced list loss. Fourteen autosave tests, formatting and strict engine lint pass; full CI runs on GitHub. Refs #2792. Co-authored-by: Codex --- crates/engine/src/autosave_tests.rs | 96 +++++++++++++++++++++++++++++ crates/engine/src/commands.rs | 17 +++-- docs/project-files.md | 2 + 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/autosave_tests.rs b/crates/engine/src/autosave_tests.rs index 4ac6346a4..aad4adc20 100644 --- a/crates/engine/src/autosave_tests.rs +++ b/crates/engine/src/autosave_tests.rs @@ -35,6 +35,102 @@ fn journaled(s: &mut Session) -> u64 { s.persistence.as_ref().unwrap().status.journaled_revision } +fn discard_fixture(tag: &str) -> (Session, std::path::PathBuf) { + let d = temp_dir(tag); + let mut s = Session::default(); + start(&mut s, &d); + let candidates = ["first", "second", "third"] + .into_iter() + .map(|id| { + let dir = d.join(id); + std::fs::create_dir(&dir).unwrap(); + std::fs::write(dir.join("snapshot.fcproj"), b"retained snapshot").unwrap(); + autosave::RecoveryCandidate { id: id.into(), dir, meta: Default::default() } + }) + .collect(); + s.persistence.as_mut().unwrap().candidates = candidates; + (s, d) +} + +fn block_discard(dir: &std::path::Path) { + // An ordinary file cannot be removed as a directory, on Windows or Unix. + std::fs::remove_dir_all(dir).unwrap(); + std::fs::write(dir, b"occupied directory path").unwrap(); +} + +fn recovery_ids(s: &mut Session) -> Vec { + s.execute("file.recoveryList", json!({})).unwrap().as_array().unwrap().iter().map(|v| v["id"].as_str().unwrap().to_owned()).collect() +} + +#[test] +fn discard_failure_preserves_candidates_and_allows_retry() { + let (mut s, d) = discard_fixture("discard-first-error"); + block_discard(&d.join("first")); + assert!(s.execute("file.discardRecovery", json!({"all": true})).is_err()); + assert_eq!(recovery_ids(&mut s), ["first", "second", "third"]); + assert!(s.is_enabled("file.discardRecovery")); + assert!(d.join("second/snapshot.fcproj").exists()); + assert!(d.join("third/snapshot.fcproj").exists()); + std::fs::remove_file(d.join("first")).unwrap(); + std::fs::create_dir(d.join("first")).unwrap(); + assert_eq!(s.execute("file.discardRecovery", json!({"all": true})).unwrap()["discarded"], 3); + assert!(recovery_ids(&mut s).is_empty()); + s.shutdown(); + std::fs::remove_dir_all(d).unwrap(); +} + +#[test] +fn discard_failure_keeps_only_not_yet_deleted_candidates() { + let (mut s, d) = discard_fixture("discard-second-error"); + block_discard(&d.join("second")); + assert!(s.execute("file.discardRecovery", json!({"all": true})).is_err()); + assert_eq!(recovery_ids(&mut s), ["second", "third"]); + assert!(!d.join("first").exists()); + assert!(d.join("third/snapshot.fcproj").exists()); + s.shutdown(); + std::fs::remove_dir_all(d).unwrap(); +} + +#[test] +fn discard_selected_failure_keeps_unselected_candidates() { + let (mut s, d) = discard_fixture("discard-selected-error"); + block_discard(&d.join("second")); + assert!(s.execute("file.discardRecovery", json!({"id": "second"})).is_err()); + assert_eq!(recovery_ids(&mut s), ["first", "second", "third"]); + assert!(d.join("first/snapshot.fcproj").exists()); + assert!(d.join("third/snapshot.fcproj").exists()); + s.shutdown(); + std::fs::remove_dir_all(d).unwrap(); +} + +#[test] +fn discard_default_failure_does_not_select_another_candidate() { + let (mut s, d) = discard_fixture("discard-default-error"); + block_discard(&d.join("first")); + let error = s.execute("file.discardRecovery", json!({})).unwrap_err().to_string(); + assert!(error.contains("first"), "{error}"); + assert_eq!(recovery_ids(&mut s), ["first", "second", "third"]); + assert!(d.join("second/snapshot.fcproj").exists()); + s.shutdown(); + std::fs::remove_dir_all(d).unwrap(); +} + +#[test] +fn discard_selection_and_unknown_id_preserve_other_candidates() { + let (mut s, d) = discard_fixture("discard-selection"); + assert!(s.execute("file.discardRecovery", json!({"id": "unknown"})).is_err()); + assert_eq!(recovery_ids(&mut s), ["first", "second", "third"]); + assert_eq!(s.execute("file.discardRecovery", json!({"id": "second"})).unwrap()["discarded"], 1); + assert_eq!(recovery_ids(&mut s), ["first", "third"]); + assert!(!d.join("second").exists()); + assert_eq!(s.execute("file.discardRecovery", json!({})).unwrap()["discarded"], 1); + assert_eq!(recovery_ids(&mut s), ["third"]); + assert!(!d.join("first").exists()); + assert!(d.join("third/snapshot.fcproj").exists()); + s.shutdown(); + std::fs::remove_dir_all(d).unwrap(); +} + #[test] fn preferences_get_set_and_persist() { let d = temp_dir("prefs"); diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 5c564e69e..e08003673 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -1521,18 +1521,27 @@ fn build() -> Vec { let all = bool_p(p, "all").unwrap_or(false); let id = str_p(p, "id").map(str::to_string); let Some(per) = s.persistence.as_mut() else { return Ok(json!({"discarded": 0})) }; - let mut n = 0; + let mut n = 0usize; let mut keep = Vec::new(); + let mut failure = None; for (i, c) in std::mem::take(&mut per.candidates).into_iter().enumerate() { let hit = all || id.as_deref().map_or(i == 0, |want| want == c.id); - if hit { - crate::autosave::discard_candidate(&c).map_err(|e| EngineError::Other(e.to_string()))?; - n += 1; + if hit && failure.is_none() { + match crate::autosave::discard_candidate(&c) { + Ok(()) => n = n.saturating_add(1), + Err(e) => { + failure = Some(EngineError::Other(format!("could not discard recovery session {}: {e}", c.id))); + keep.push(c); + } + } } else { keep.push(c); } } per.candidates = keep; + if let Some(e) = failure { + return Err(e); + } if n == 0 { return Err(bad("file.discardRecovery", "no such recovery session")); } diff --git a/docs/project-files.md b/docs/project-files.md index 7878eadc5..2e2c380df 100644 --- a/docs/project-files.md +++ b/docs/project-files.md @@ -156,6 +156,8 @@ Premiere's auto-save can lose up to one interval of work. FilmCraft also keeps a Recover unsaved changes from 2026-10-01 00:02:09?"*, with the buttons **Recover**, **Not Now** and **Discard**. Stale directories without a snapshot are removed. Journals of running sessions are never touched. +- If discarding a recovery session fails, FilmCraft reports the error and keeps that session and + 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.