fix(ui): preserve tool commands across document opening

crates/ui-egui/src/lib.rs

@@ -1193,11 +1193,15 @@ impl PdfCraftApp {
             open_worker::ResultKind::Failed(reason) => {
                 self.notify_fmt("Couldn't open {name}: {reason}", &[("name", &pending.name), ("reason", &reason)]);
             }
-            open_worker::ResultKind::Conversion(bytes) => {
-                if let Some(Err(error)) = self.open_converted(&pending.name, &bytes) {
-                    self.notify_fmt("Couldn't open {name}: {e}", &[("name", &pending.name), ("e", &error)]);
+            open_worker::ResultKind::Conversion(bytes) => match self.open_converted(&pending.name, &bytes) {
+                Some(Ok(())) => {
+                    if let Some(command) = pending.then {
+                        self.execute(command);
+                    }
                 }
-            }
+                Some(Err(error)) => self.notify_fmt("Couldn't open {name}: {e}", &[("name", &pending.name), ("e", &error)]),
+                None => {}
+            },
             open_worker::ResultKind::Prepared { bytes, result } => match *result {
                 Ok(prepared) => match self.finish_open(&pending.name, pending.path.clone(), bytes.len(), prepared, pending.activate) {
                     Ok(id) => {
@@ -1727,6 +1731,13 @@ impl PdfCraftApp {
 
     #[cfg(not(target_arch = "wasm32"))]
     pub fn open_path(&mut self, path: &str) {
+        self.open_path_attempt(path);
+    }
+
+    /// Whether this attempt opened a document, created a password prompt, or queued an open.
+    /// Failure can leave an older password prompt intact; it does not belong to this attempt.
+    #[cfg(not(target_arch = "wasm32"))]
+    fn open_path_attempt(&mut self, path: &str) -> bool {
         let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string());
         // Taken before reading: a build that rewrites the file meanwhile still counts as a change.
         let stamp = file_watch::Stamp::at(path);
@@ -1734,20 +1745,26 @@ impl PdfCraftApp {
         // deterministic inline startup and test flows. Once live, reading and preparation use
         // the bounded worker and commit on the UI thread.
         if self.ctx.is_none() || self.run_inline {
-            match std::fs::read(path) {
+            return match std::fs::read(path) {
                 Ok(bytes) => {
                     let before = self.views.len();
                     if let Err(error) = self.open_bytes(&name, Some(path.to_string()), bytes) {
                         self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &error)]);
-                    } else if self.views.len() > before
-                        && let Some(doc) = self.views.last().map(|view| view.id)
-                    {
-                        self.watch.known_as(doc, stamp);
+                        false
+                    } else {
+                        if self.views.len() > before
+                            && let Some(doc) = self.views.last().map(|view| view.id)
+                        {
+                            self.watch.known_as(doc, stamp);
+                        }
+                        true
                     }
                 }
-                Err(error) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &error.to_string())]),
-            }
-            return;
+                Err(error) => {
+                    self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &error.to_string())]);
+                    false
+                }
+            };
         }
         self.startup_superseded = true;
         if let Err(error) = self.schedule_open(OpenRequest {
@@ -1762,6 +1779,9 @@ impl PdfCraftApp {
             then: None,
         }) {
             self.notify_fmt("Couldn't open {name}: {e}", &[("name", &name), ("e", &error)]);
+            false
+        } else {
+            true
         }
     }
 
@@ -2064,12 +2084,21 @@ impl PdfCraftApp {
         self.ask_one(pickers::Ask::File(dialog), None, move |app, path| {
             let path = path.to_string_lossy().into_owned();
             let before = app.views.len();
-            app.open_path(&path);
-            if let Some(prompt) = app.password_prompt.as_mut().filter(|p| p.path.as_deref() == Some(path.as_str())) {
-                // Encrypted: the tool runs once the password is in.
-                prompt.then = Some(command);
+            let pending_before = app.pending_opens.len();
+            if !app.open_path_attempt(&path) {
+                return;
+            }
+            if app.ctx.is_some() && !app.run_inline {
+                // UI-thread completion cannot run inside open_path: only its newly queued
+                // request owns this continuation, including through password retries.
+                if let Some(pending) = app.pending_opens.get_mut(pending_before) {
+                    pending.then = Some(command);
+                }
             } else if app.views.len() > before && app.active.is_some() {
                 app.execute(command);
+            } else if let Some(prompt) = app.password_prompt.as_mut().filter(|p| p.path.as_deref() == Some(path.as_str())) {
+                // Encrypted: the tool runs once the password is in.
+                prompt.then = Some(command);
             }
         });
     }

crates/ui-egui/src/open_worker_tests.rs

@@ -89,6 +89,191 @@ fn open_prepare_keeps_control_and_existing_tabs_live_until_atomic_commit() {
     let _ = std::fs::remove_file(path);
 }
 
+#[test]
+fn inline_failed_tool_open_does_not_steal_an_existing_password_prompt() {
+    let mut doc = pdfcraft_cos::Document::open(Arc::new(fixture(2))).unwrap();
+    doc.set_encryption(&pdfcraft_cos::NewEncryption {
+        algorithm: pdfcraft_cos::Algorithm::Aes256,
+        user_password: "reader",
+        owner_password: "owner",
+        permissions: -4,
+        encrypt_metadata: true,
+        seed: [4; 32],
+    })
+    .unwrap();
+    let bytes = pdfcraft_cos::write_full(&doc, &Default::default()).unwrap();
+    let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos();
+    let path = std::env::temp_dir().join(format!("pdfcraft-inline-owner-{}-{nonce}.pdf", std::process::id()));
+    assert!(!path.exists(), "the new opening attempt must fail reading");
+    let mut h = Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_eframe(|_| {
+        let mut app = PdfCraftApp::new();
+        app.set_option("language", "en").unwrap();
+        app.run_inline = true;
+        app.open_bytes("locked.pdf", Some(path.to_string_lossy().into_owned()), bytes).unwrap();
+        app.pick_override = Some(vec![path.to_string_lossy().into_owned()]);
+        app
+    });
+    h.step();
+    let cached = h.state().password_prompt.as_ref().unwrap().bytes.clone();
+    assert!(h.state().password_prompt.as_ref().unwrap().then.is_none());
+    h.state_mut().run_command("export.text");
+    h.step();
+    assert!(h.state().toast.as_ref().is_some_and(|toast| toast.0.starts_with("Couldn't read")));
+    let prompt = h.state().password_prompt.as_ref().unwrap();
+    assert!(Arc::ptr_eq(&prompt.bytes, &cached), "the older encrypted bytes remain available");
+    assert!(prompt.then.is_none(), "a failed attempt must not attach its tool to the older prompt");
+    h.state_mut().submit_password(Some("wrong".into()));
+    assert!(h.state().password_prompt.as_ref().unwrap().then.is_none());
+    h.state_mut().submit_password(Some("reader".into()));
+    assert_eq!(h.state().views.len(), 1, "the older document can still be unlocked");
+    assert!(h.state().password_prompt.is_none());
+    h.run_steps(2);
+    assert!(h.state().dialog.is_none(), "the failed attempt's tool never runs");
+
+    // Accepting the current attempt must still run ordinary/conversion tools and retain
+    // newly created password continuations through retries or discard them on cancellation.
+    for case in ["plain", "converted", "password", "password-cancel"] {
+        let path = path.with_extension(if case == "converted" { "txt" } else { "pdf" });
+        let bytes = match case {
+            "plain" => fixture(2),
+            "converted" => b"Inline text converted to PDF".to_vec(),
+            _ => cached.as_ref().clone(),
+        };
+        std::fs::write(&path, bytes).unwrap();
+        let mut h = Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_eframe(|_| {
+            let mut app = PdfCraftApp::new();
+            app.set_option("language", "en").unwrap();
+            app.run_inline = true;
+            app.pick_override = Some(vec![path.to_string_lossy().into_owned()]);
+            app
+        });
+        h.step();
+        let defaults = h.state().persist();
+        h.state_mut().run_command("export.text");
+        h.step();
+        if case.starts_with("password") {
+            assert_eq!(h.state().password_prompt.as_ref().unwrap().then, Some("export.text"));
+            h.state_mut().submit_password(Some("wrong".into()));
+            assert_eq!(h.state().password_prompt.as_ref().unwrap().then, Some("export.text"));
+            h.state_mut().submit_password((case == "password").then(|| "reader".into()));
+        }
+        assert_eq!(h.state().views.len(), usize::from(case != "password-cancel"));
+        assert_eq!(matches!(h.state().dialog, Some(crate::Dialog::Export(_))), case != "password-cancel");
+        assert!(h.state().password_prompt.is_none());
+        assert_eq!(h.state().view_defaults.layout, PdfCraftApp::new().view_defaults.layout);
+        assert_eq!(h.state().view_defaults.continuous, PdfCraftApp::new().view_defaults.continuous);
+        // Persistence includes recent files; compare only the saved display defaults.
+        let before: Value = serde_json::from_str(&defaults).unwrap();
+        let after: Value = serde_json::from_str(&h.state().persist()).unwrap();
+        assert_eq!(before["default_layout"], after["default_layout"]);
+        let _ = std::fs::remove_file(path);
+    }
+}
+
+#[test]
+fn tool_open_continuation_follows_its_worker_and_password_retries() {
+    for case in ["plain", "converted", "invalid-image", "password", "password-cancel", "wrong-password-cancel", "read-failure", "invalid-pdf"] {
+        let extension = match case {
+            "converted" => "txt",
+            "invalid-image" => "png",
+            _ => "pdf",
+        };
+        let path = std::env::temp_dir().join(format!("pdfcraft-tool-worker-{}-{case}.{extension}", std::process::id()));
+        let encrypted = case.contains("password");
+        if case != "read-failure" {
+            let bytes = if encrypted {
+                let mut doc = pdfcraft_cos::Document::open(Arc::new(fixture(2))).unwrap();
+                doc.set_encryption(&pdfcraft_cos::NewEncryption {
+                    algorithm: pdfcraft_cos::Algorithm::Aes256,
+                    user_password: "reader",
+                    owner_password: "owner",
+                    permissions: -4,
+                    encrypt_metadata: true,
+                    seed: [4; 32],
+                })
+                .unwrap();
+                pdfcraft_cos::write_full(&doc, &Default::default()).unwrap()
+            } else if case == "converted" {
+                b"A text file opened as a PDF".to_vec()
+            } else if case == "invalid-image" {
+                b"\x89PNG\r\n\x1a\n".to_vec()
+            } else if case == "invalid-pdf" {
+                b"not a PDF".to_vec()
+            } else {
+                fixture(2)
+            };
+            std::fs::write(&path, bytes).unwrap();
+        }
+        let mut h = Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_eframe(|_| {
+            let mut app = PdfCraftApp::new();
+            app.set_option("language", "en").unwrap();
+            app.pick_override = Some(vec![path.to_string_lossy().into_owned()]);
+            app
+        });
+        h.step();
+        let (gate, started, release, finished) = open_worker::prepare_gate();
+        h.state_mut().open_prepare_gate = Some(gate);
+        h.state_mut().run_command("export.text");
+        h.step();
+        started.recv_timeout(Duration::from_secs(5)).unwrap();
+        assert_eq!(h.state().pending_opens.len(), 1);
+        assert_eq!(h.state().pending_opens[0].then, Some("export.text"), "{case}: command belongs to the pending request");
+
+        // A different document becoming active must neither run nor inherit the tool.
+        h.state_mut().open_bytes("unrelated.pdf", None, fixture(1)).unwrap();
+        let unrelated = h.state().views[0].id;
+        h.step();
+        assert!(h.state().dialog.is_none());
+        release.send(()).unwrap();
+        finished.recv_timeout(Duration::from_secs(5)).unwrap();
+        h.step();
+
+        if encrypted {
+            assert_eq!(h.state().password_prompt.as_ref().unwrap().then, Some("export.text"));
+            assert!(h.state().dialog.is_none());
+            if case != "password-cancel" {
+                h.state_mut().submit_password(Some("wrong".into()));
+                started.recv_timeout(Duration::from_secs(5)).unwrap();
+                release.send(()).unwrap();
+                finished.recv_timeout(Duration::from_secs(5)).unwrap();
+                h.step();
+                let prompt = h.state().password_prompt.as_ref().unwrap();
+                assert!(prompt.error.is_some());
+                assert_eq!(prompt.then, Some("export.text"));
+                assert!(h.state().dialog.is_none());
+            }
+            if case == "password" {
+                h.state_mut().submit_password(Some("reader".into()));
+                started.recv_timeout(Duration::from_secs(5)).unwrap();
+                release.send(()).unwrap();
+                finished.recv_timeout(Duration::from_secs(5)).unwrap();
+                h.step();
+            } else {
+                h.state_mut().submit_password(None);
+            }
+        }
+        if matches!(case, "plain" | "converted" | "password") {
+            assert_eq!(h.state().views.len(), 2);
+            let app = h.state();
+            let opened = &app.views[app.active.unwrap()];
+            assert_ne!(opened.id, unrelated);
+            assert_eq!(app.session.get(opened.id).unwrap().path.as_deref(), if case == "converted" { None } else { path.to_str() });
+            assert!(matches!(app.dialog, Some(crate::Dialog::Export(_))));
+            // Closing the dialog and processing more frames must not execute it again.
+            h.state_mut().dialog = None;
+        } else {
+            assert_eq!(h.state().views.len(), 1);
+            assert!(h.state().dialog.is_none());
+        }
+        assert!(h.state().pending_opens.is_empty());
+        assert!(h.state().password_prompt.is_none());
+        h.state_mut().open_bytes("later.pdf", None, fixture(1)).unwrap();
+        h.run_steps(2);
+        assert!(h.state().dialog.is_none(), "{case}: continuation cannot run on another open");
+        let _ = std::fs::remove_file(path);
+    }
+}
+
 #[test]
 fn launch_layout_waits_for_cli_opens_and_preserves_restored_tabs() {
     use crate::canvas::PageLayout;

crates/ui-egui/tests/commands.rs

@@ -44,6 +44,15 @@ fn harness() -> Harness<'static, PdfCraftApp> {
     h
 }
 
+fn wait_for(h: &mut Harness<'static, PdfCraftApp>, description: &str, ready: impl Fn(&PdfCraftApp) -> bool) {
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
+    while !ready(h.state()) {
+        assert!(std::time::Instant::now() < deadline, "timed out waiting for {description}");
+        h.step();
+        std::thread::yield_now();
+    }
+}
+
 /// Commands that open a native file picker can't run headless.
 /// (Create ▸ Clipboard reads the system clipboard: tests use `create_from_clip`.)
 const PICKERS: &[&str] = &[
@@ -184,7 +193,7 @@ fn a_tool_chosen_with_no_document_asks_for_one_and_runs_on_it() {
     h.run_steps(2);
     h.state_mut().run_command("export.text");
     assert!(h.state().toast.as_ref().is_none_or(|t| t.0 != "Open a document first"));
-    h.run_steps(3);
+    wait_for(&mut h, "picked document and export dialog", |app| app.active.is_some() && matches!(app.dialog, Some(Dialog::Export(_))));
     assert!(h.state().active.is_some(), "the picked file is open");
     assert!(matches!(h.state().dialog, Some(Dialog::Export(_))), "and the tool runs on it: {:?}", h.state().dialog);
     // Cancelled: nothing opens and nothing runs.
@@ -287,7 +296,11 @@ fn the_open_recent_menu_lists_files_and_opens_one() {
     h.get_by_label("Open Recent ⏵").hover();
     h.run_steps(3);
     h.get_by_label_contains("recent.pdf").click();
-    h.run_steps(4);
+    wait_for(&mut h, "recent document", |app| {
+        app.views.len() == 2
+            && app.active.and_then(|index| app.views.get(index)).and_then(|view| app.session.get(view.id)).and_then(|doc| doc.path.as_deref())
+                == Some(path.as_str())
+    });
     let app = h.state();
     assert_eq!(app.views.len(), 2, "the recent file opened in a new tab");
     let active = app.active.unwrap();