Wait for recovery cleanup before the app exits (#3140)

A save makes the document clean, which queues discarding this
session's now-redundant recovery copy — but that's only queued, on a
background worker thread, the same as a print job. Closing the app
right after Save (the very next frame, with no regular recovery_tick
in between to notice and queue the discard, or with the discard
queued but not yet finished by the worker) could race the process
exiting before the copy was actually removed, so the next launch
wrongly offered to recover a document that was in fact saved cleanly.

Added Recovery::finish and Session::finish_recovery, mirroring the
print spooler's own finish_pending_jobs, and call it from the same
places: on_exit and the macOS hard-quit path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

apps/wordcraft/src/main.rs

@@ -77,7 +77,7 @@ impl eframe::App for App {
             {
                 ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose);
                 self.0.quit_requested = false;
-                schedule_macos_terminate(&self.0, self.2.as_ref());
+                schedule_macos_terminate(&mut self.0, self.2.as_ref());
             }
             #[cfg(not(target_os = "macos"))]
             if self.0.quit_requested {
@@ -94,6 +94,7 @@ impl eframe::App for App {
     }
     fn on_exit(&mut self) {
         print::finish_pending_jobs();
+        self.0.session.finish_recovery();
         save_prefs(&self.0);
         if let Some(key_file) = &self.2 {
             key_file.remove();
@@ -109,7 +110,7 @@ impl eframe::App for App {
 /// removed first, as `on_exit` does, because a fallback [`std::process::exit`]
 /// skips `on_exit`. A second close while a quit is already scheduled is ignored.
 #[cfg(target_os = "macos")]
-fn schedule_macos_terminate(app: &WordApp, key_file: Option<&control_server::KeyFile>) {
+fn schedule_macos_terminate(app: &mut WordApp, key_file: Option<&control_server::KeyFile>) {
     use std::sync::atomic::{AtomicBool, Ordering};
     if app.printing.busy() {
         return;
@@ -121,6 +122,7 @@ fn schedule_macos_terminate(app: &WordApp, key_file: Option<&control_server::Key
     log::info!("window close: quitting");
     // This path can bypass on_exit. Join the owned workers before termination and cleanup.
     print::finish_pending_jobs();
+    app.session.finish_recovery();
     save_prefs(app);
     if let Some(key_file) = key_file {
         key_file.remove();

crates/engine/src/recovery.rs

@@ -88,6 +88,21 @@ impl Session {
         }
         None
     }
+    /// Waits for recovery to finish anything already under way, discarding this session's own
+    /// copy first if it no longer needs one (the document was saved, say). Desktop hosts call
+    /// this right before the process exits, the same as they wait out pending print jobs:
+    /// otherwise a save just before close can race the process exiting before a now-redundant
+    /// recovery copy actually gets removed, so the next launch wrongly offers to recover a
+    /// document that was in fact saved cleanly.
+    pub fn finish_recovery(&mut self) {
+        #[cfg(not(target_arch = "wasm32"))]
+        if let Some(mut r) = self.recovery.take() {
+            if let Err(e) = r.finish(self) {
+                log::warn!("recovery: {e}");
+            }
+            self.recovery = Some(r);
+        }
+    }
     /// Only an explicit protect/save action waits for the worker and removes plaintext copies.
     /// A failure prevents applying the password; typing never waits.
     pub(crate) fn clear_recovery_before_password(&mut self) -> Result<(), CmdError> {

crates/engine/src/recovery/desktop.rs

@@ -549,6 +549,34 @@ impl Recovery {
         self.revision = None;
         Ok(())
     }
+    /// Waits out any job already in flight (an autosave queued just before shutdown, say), then,
+    /// if the document turns out to need no recovery copy kept (not dirty, or password-protected
+    /// — [`tick`](Self::tick)'s own condition), discards it and waits for that too. Desktop hosts
+    /// call this before the process exits, the same as the print spooler's own pending-job wait
+    /// (`finish_pending_jobs`): otherwise a save that lands right before close can race the
+    /// process exiting before the now-redundant recovery copy is actually removed, so the *next*
+    /// launch wrongly offers to recover a document that was in fact saved cleanly.
+    pub fn finish(&mut self, s: &Session) -> Result<(), String> {
+        if self.busy {
+            let e = self.rx.recv().map_err(|e| e.to_string())?;
+            self.documents = e.documents;
+            self.busy = false;
+            self.ready = true;
+            e.outcome?;
+        }
+        if let Some(id) = self.id.clone()
+            && (!s.dirty || s.password.is_some())
+        {
+            self.queue(Job::Discard(id))?;
+            let e = self.rx.recv().map_err(|e| e.to_string())?;
+            self.busy = false;
+            self.documents = e.documents;
+            e.outcome?;
+            self.id = None;
+            self.revision = None;
+        }
+        Ok(())
+    }
 }
 
 #[cfg(test)]
@@ -628,6 +656,31 @@ mod tests {
         settle(s);
     }
 
+    /// #3140: a save that lands right before the app closes can race the process exiting before
+    /// the now-redundant recovery copy is actually removed, so the next launch wrongly offers to
+    /// recover a document that was in fact saved cleanly. `finish_recovery` (desktop hosts call it
+    /// on exit, the same as they wait for pending print jobs) must discard it synchronously
+    /// instead, with no regular tick in between — simulating closing right after Save, not after
+    /// however long it'd take the next normal autosave tick to notice and clean up.
+    #[test]
+    fn finish_recovery_discards_its_own_copy_for_a_document_saved_right_before_exit() {
+        let profile = Temp::new();
+        let mut s = session(&profile.0);
+        s.run("document.setText", &json!({"text": "About to be saved"})).unwrap();
+        save_now(&mut s);
+        assert_eq!(s.recovery_documents().len(), 1, "a recovery copy exists for the dirty document");
+        // A real Save does this, then the app can close on the very next frame, with no regular
+        // recovery_tick() in between to notice `!s.dirty` and queue the discard itself.
+        s.dirty = false;
+        assert_eq!(
+            s.recovery_documents().len(),
+            1,
+            "confirms the race: setting dirty=false alone (what Save does) doesn't discard anything by itself"
+        );
+        s.finish_recovery();
+        assert!(s.recovery_documents().is_empty(), "finish_recovery should have discarded the now-redundant copy before returning");
+    }
+
     #[test]
     fn recovery_commands_only_address_known_profile_records() {
         use crate::file_access::{AutomationRoots, confine};