Preserve editable clipboard contents when native reads fail

apps/photocraft/src/services.rs

@@ -475,23 +475,35 @@ pub fn native(automation: Option<photocraft_automation::AuthorizedWorkspace>) ->
         clipboard_get_image: Some({
             let clip = clip.clone();
             Box::new(move || {
-                let mut slot = clip.try_borrow_mut().ok()?;
+                let mut slot = clip.try_borrow_mut().map_err(|_| "clipboard is busy".to_string())?;
                 let cb = match slot.as_mut() {
                     Some(c) => c,
-                    None => slot.insert(arboard::Clipboard::new().ok()?),
+                    None => slot.insert(arboard::Clipboard::new().map_err(|e| e.to_string())?),
                 };
                 // Copied files first: Finder also puts the file's icon on the clipboard as an
                 // image, which would otherwise paste instead of the file.
-                if let Some(img) = cb.get().file_list().ok().and_then(|paths| image_from_files(&paths)) {
-                    return Some(img);
+                match cb.get().file_list() {
+                    Ok(paths) => {
+                        if let Some(img) = image_from_files(&paths) {
+                            return Ok(Some(img));
+                        }
+                    }
+                    Err(error @ arboard::Error::ClipboardOccupied) => return Err(error.to_string()),
+                    Err(_) => {}
                 }
-                if let Ok(img) = cb.get_image() {
-                    return Some((img.width as u32, img.height as u32, img.bytes.into_owned()));
+                match cb.get_image() {
+                    Ok(img) => Ok(Some((img.width as u32, img.height as u32, img.bytes.into_owned()))),
+                    Err(error) => {
+                        #[cfg(target_os = "linux")]
+                        if let Some(image) = wayland_clipboard_image() {
+                            return Ok(Some(image));
+                        }
+                        match error {
+                            arboard::Error::ContentNotAvailable => Ok(None),
+                            _ => Err(error.to_string()),
+                        }
+                    }
                 }
-                #[cfg(target_os = "linux")]
-                return wayland_clipboard_image();
-                #[cfg(not(target_os = "linux"))]
-                None
             })
         }),
         load_prefs: Some(Box::new(|| std::fs::read_to_string(prefs_file()?).ok())),
@@ -597,6 +609,57 @@ mod tests {
     use photocraft_ui_egui::{PhotocraftApp, prefs_ui};
     use serde_json::{Value, json};
 
+    /// Regression using the real clipboard on an isolated Windows CI runner.
+    #[cfg(target_os = "windows")]
+    #[test]
+    #[ignore = "requires exclusive use of the Windows clipboard"]
+    fn busy_native_clipboard_preserves_copied_shape() {
+        for id in ["edit.paste", "edit.pasteSpecial.pasteInPlace"] {
+            let mut native = native(None);
+            let services = Services {
+                clipboard_set_image: native.clipboard_set_image.take(),
+                clipboard_get_image: native.clipboard_get_image.take(),
+                ..Default::default()
+            };
+            let mut app = PhotocraftApp::new(Session::new(), services);
+            app.run("file.new", json!({"width": 80, "height": 60})).unwrap();
+            app.run("shape.create", json!({"kind": "rect", "rect": [10, 10, 40, 20], "fill": "#ff0000"})).unwrap();
+            app.run("edit.copy", json!({})).unwrap();
+            assert!(app.session.clipboard.as_ref().unwrap().layers.is_some());
+            assert!(app.services.clipboard_get_image.as_mut().unwrap()().unwrap().is_some(), "native export succeeded");
+            app.run("file.new", json!({"width": 80, "height": 60})).unwrap();
+
+            // Another thread holds the OS clipboard open beyond arboard's bounded retry window.
+            let (ready_tx, ready_rx) = std::sync::mpsc::channel();
+            let (release_tx, release_rx) = std::sync::mpsc::channel();
+            let holder = std::thread::spawn(move || {
+                // A non-null window distinguishes this owner from arboard's null-window opens.
+                let guard = winsafe::HWND::GetDesktopWindow().OpenClipboard().unwrap();
+                ready_tx.send(()).unwrap();
+                release_rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap();
+                drop(guard);
+            });
+            ready_rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap();
+            let contention = arboard::Clipboard::new().unwrap().get_image().err();
+            let first = app.run(id, json!({}));
+            release_tx.send(()).unwrap();
+            holder.join().unwrap();
+            assert!(matches!(contention, Some(arboard::Error::ClipboardOccupied)), "diagnostic must actually occupy the clipboard: {contention:?}");
+            eprintln!("paste while clipboard occupied: {first:?}");
+            assert!(first.is_err(), "report contention rather than a successful no-op");
+            assert!(
+                app.session.clipboard.as_ref().is_some_and(|clip| clip.layers.is_some()),
+                "a temporary OS read failure must not discard the copied editable shape"
+            );
+            app.run(id, json!({})).unwrap();
+            let state = app.session.active().unwrap();
+            assert!(
+                matches!(state.doc.layer(state.active_layer.unwrap()).unwrap().content, photocraft_doc::LayerContent::Shape(_)),
+                "retry keeps the shape editable"
+            );
+        }
+    }
+
     #[cfg(all(unix, not(target_os = "macos")))]
     #[test]
     fn open_filters_cover_uppercase_and_mixed_case_extensions() {

crates/ui-egui/src/control.rs

@@ -1647,7 +1647,7 @@ mod tests {
         let image = clipboard.clone();
         let mut app = PhotocraftApp::new(
             photocraft_engine::Session::new(),
-            crate::Services { clipboard_get_image: Some(Box::new(move || image.lock().ok()?.clone())), ..Default::default() },
+            crate::Services { clipboard_get_image: Some(Box::new(move || Ok(image.lock().unwrap().clone()))), ..Default::default() },
         );
         let ctx = egui::Context::default();
         let opened = call(&mut app, &ctx, "ui.dialog.open", json!({"kind":"newDocument"}));

crates/ui-egui/src/lib.rs

@@ -341,8 +341,9 @@ pub type OpenUrlFn = Box<dyn Fn(&str) -> Result<(), String>>;
 pub type EncodePngFn = Box<dyn Fn(u32, u32, &[u8]) -> Result<Vec<u8>, String>>;
 /// Put an RGBA8 image (width, height, pixels) on the OS clipboard.
 pub type ClipboardSetFn = Box<dyn FnMut(u32, u32, &[u8]) -> Result<(), String>>;
-/// Read an RGBA8 image from the OS clipboard.
-pub type ClipboardGetFn = Box<dyn FnMut() -> Option<(u32, u32, Vec<u8>)>>;
+/// Read an RGBA8 image from the OS clipboard. `Ok(None)` means no image is available;
+/// `Err` means the contents could not be read and must not invalidate the internal copy.
+pub type ClipboardGetFn = Box<dyn FnMut() -> Result<Option<(u32, u32, Vec<u8>)>, String>>;
 /// Shared queue of (file name, bytes) delivered asynchronously.
 pub type Inbox = std::sync::Arc<std::sync::Mutex<Vec<(String, Vec<u8>)>>>;
 /// Read the saved preferences text (`None` when there is none yet).
@@ -1007,15 +1008,11 @@ impl PhotocraftApp {
         if let Some(r) = crate::crop_ui::intercept(self, id) {
             return r;
         }
-        let suppress_events = self.automation_input && self.session.prefs().script_events.enabled;
-        if suppress_events {
-            self.session.edit_prefs(|prefs| prefs.script_events.enabled = false);
-        }
         let t0 = gpu_canvas::now_ms();
         // The OS clipboard is read only on an explicit paste, never in the background (privacy, CPU).
         if matches!(id, "edit.paste" | "edit.pasteSpecial.pasteInPlace" | "file.newFromClipboard") {
             if !clip_read {
-                self.import_os_clipboard();
+                self.import_os_clipboard()?;
             }
             if self.session.clipboard.is_none() && self.services.clipboard_get_image.is_some() {
                 // Enabled on the strength of the OS clipboard, which held no image: a quiet no-op.
@@ -1025,6 +1022,10 @@ impl PhotocraftApp {
                 return Ok(serde_json::json!({"pasted": false}));
             }
         }
+        let suppress_events = self.automation_input && self.session.prefs().script_events.enabled;
+        if suppress_events {
+            self.session.edit_prefs(|prefs| prefs.script_events.enabled = false);
+        }
         // Long commands become background jobs when enabled (`jobs_ui`); the rest run inline.
         let params = self.with_mask_target(id, params);
         let params = vector_ui::with_active_path(self, id, params);
@@ -2446,7 +2447,9 @@ impl PhotocraftApp {
                 }
             }
         }
-        self.import_os_clipboard();
+        if self.import_os_clipboard().is_err() {
+            return f;
+        }
         if let Some(c) = self.session.clipboard.as_ref().filter(|c| !c.bounds.is_empty()) {
             crate::new_doc_ui::set_clipboard(&mut f, c.bounds.width(), c.bounds.height());
         }
@@ -2455,22 +2458,32 @@ impl PhotocraftApp {
 
     /// If the OS clipboard holds an image that isn't the one we put there, make it the session
     /// clipboard (so ⌘V pastes screenshots and images copied in other apps, like Photoshop).
-    /// Returns true when a new external image was imported. Once the OS clipboard no longer holds
+    /// Returns `Ok(true)` when a new external image was imported. Once the OS clipboard no longer holds
     /// the image we mirrored or imported (text or a file was copied since), that image is stale:
     /// it is dropped rather than pasted.
-    pub(crate) fn import_os_clipboard(&mut self) -> bool {
-        let Some(get) = self.services.clipboard_get_image.as_mut() else { return false };
-        let image = get().filter(|(w, h, bytes)| *w > 0 && *h > 0 && bytes.len() == *w as usize * *h as usize * 4);
+    pub(crate) fn import_os_clipboard(&mut self) -> Result<bool, String> {
+        let Some(get) = self.services.clipboard_get_image.as_mut() else { return Ok(false) };
+        // A read failure says nothing about the clipboard contents. Keep our editable copy and
+        // its signature, and abort this paste rather than treating the failure as an empty image.
+        let image = match get() {
+            Ok(image) => image,
+            Err(error) => {
+                self.ui.status = error.clone();
+                self.ui.status_error = true;
+                return Err(error);
+            }
+        };
+        let image = image.filter(|(w, h, bytes)| *w > 0 && *h > 0 && bytes.len() == *w as usize * *h as usize * 4);
         let Some((w, h, bytes)) = image else {
             if self.os_clip_sig.take().is_some() {
                 self.session.clipboard = None;
                 self.clip_external = false;
             }
-            return false;
+            return Ok(false);
         };
         let sig = clip_signature(w, h, &bytes);
         if self.os_clip_sig == Some(sig) && self.session.clipboard.is_some() {
-            return false;
+            return Ok(false);
         }
         let r = photocraft_geom::Rect::new(0, 0, w as i32, h as i32);
         let mut surface = photocraft_raster::Surface::from_interleaved(photocraft_color::PixelFormat::RGBA8, r, &bytes);
@@ -2478,7 +2491,7 @@ impl PhotocraftApp {
         self.session.clipboard = Some(photocraft_engine::edit_cmds::Clip { surface, bounds: r, layers: None, resolution: None });
         self.os_clip_sig = Some(sig);
         self.clip_external = true;
-        true
+        Ok(true)
     }
 }
 
@@ -2611,7 +2624,7 @@ mod clipboard_tests {
                 *a.lock().unwrap() = Some((w, h, px.to_vec()));
                 Ok(())
             })),
-            clipboard_get_image: Some(Box::new(move || b.lock().unwrap().clone())),
+            clipboard_get_image: Some(Box::new(move || Ok(b.lock().unwrap().clone()))),
             ..Default::default()
         };
         let mut app = PhotocraftApp::new(Session::new(), services);
@@ -2622,10 +2635,10 @@ mod clipboard_tests {
         let (w, h, px) = os.lock().unwrap().clone().unwrap();
         assert_eq!((w, h, px.len()), (8, 4, 8 * 4 * 4));
         // Our own image comes back unchanged (no re-import, keeps the original position).
-        assert!(!app.import_os_clipboard());
+        assert!(!app.import_os_clipboard().unwrap());
         // Text copied elsewhere replaces our image: nothing to paste, not the stale image.
         let ours = os.lock().unwrap().take();
-        assert!(!app.import_os_clipboard());
+        assert!(!app.import_os_clipboard().unwrap());
         assert!(app.session.clipboard.is_none());
         *os.lock().unwrap() = ours;
         app.run("edit.copy", serde_json::json!({})).unwrap();
@@ -2659,12 +2672,107 @@ mod clipboard_tests {
 
     type OsClip = Arc<Mutex<Option<(u32, u32, Vec<u8>)>>>;
 
+    #[test]
+    #[ignore = "renders clipboard error and retry evidence with a GPU adapter"]
+    fn clipboard_error_and_retry_visual_evidence() {
+        use std::sync::atomic::{AtomicBool, Ordering};
+        let busy = Arc::new(AtomicBool::new(true));
+        let unavailable = busy.clone();
+        let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).with_max_steps(64).wgpu().build_eframe(move |cc| {
+            PhotocraftApp::setup_context(&cc.egui_ctx, Default::default());
+            let services = Services {
+                clipboard_get_image: Some(Box::new(move || {
+                    if unavailable.load(Ordering::SeqCst) {
+                        Err("The native clipboard is not accessible due to being held by another party.".into())
+                    } else {
+                        Ok(None)
+                    }
+                })),
+                ..Default::default()
+            };
+            let mut app = PhotocraftApp::new(Session::new(), services);
+            app.run("file.new", serde_json::json!({"width": 400, "height": 300, "background": "white"})).unwrap();
+            app.run("shape.create", serde_json::json!({"kind": "rect", "rect": [80, 60, 200, 120], "fill": "#3070c0"})).unwrap();
+            app.run("edit.copy", serde_json::json!({})).unwrap();
+            app.run("file.new", serde_json::json!({"width": 400, "height": 300, "background": "white"})).unwrap();
+            app
+        });
+        let out = std::path::PathBuf::from(std::env::var_os("PHOTOCRAFT_CLIPBOARD_EVIDENCE").expect("evidence directory"));
+        std::fs::create_dir_all(&out).unwrap();
+        h.run_steps(8);
+        let ctx = h.ctx.clone();
+        assert!(crate::menus::invoke(h.state_mut(), &ctx, "edit.paste", serde_json::json!({})).is_err());
+        h.run_steps(2);
+        assert!(h.state().ui.status_error);
+        h.render().unwrap().save(out.join("clipboard-busy.png")).unwrap();
+        busy.store(false, Ordering::SeqCst);
+        crate::menus::invoke(h.state_mut(), &ctx, "edit.paste", serde_json::json!({})).unwrap();
+        h.run_steps(3);
+        assert!(!h.state().ui.status_error);
+        assert!(h.state().ui.status.is_empty(), "retry clears the clipboard error: {}", h.state().ui.status);
+        // Direct harness mutations need a repaint after the command has cleared the error.
+        h.ctx.request_repaint();
+        h.run_steps(2);
+        h.render().unwrap().save(out.join("clipboard-retry.png")).unwrap();
+    }
+
+    #[test]
+    fn failed_clipboard_reads_preserve_editable_contents_until_retry() {
+        use std::sync::atomic::{AtomicBool, Ordering};
+        for id in ["edit.paste", "edit.pasteSpecial.pasteInPlace", "file.newFromClipboard"] {
+            let os = OsClip::default();
+            let busy = Arc::new(AtomicBool::new(false));
+            let (write, read, unavailable) = (os.clone(), os.clone(), busy.clone());
+            let services = Services {
+                clipboard_set_image: Some(Box::new(move |w, h, bytes| {
+                    *write.lock().unwrap() = Some((w, h, bytes.to_vec()));
+                    Ok(())
+                })),
+                clipboard_get_image: Some(Box::new(move || {
+                    if unavailable.load(Ordering::SeqCst) { Err("clipboard is busy".into()) } else { Ok(read.lock().unwrap().clone()) }
+                })),
+                ..Default::default()
+            };
+            let mut app = PhotocraftApp::new(Session::new(), services);
+            app.run("file.new", serde_json::json!({"width": 80, "height": 60})).unwrap();
+            app.run("shape.create", serde_json::json!({"kind": "rect", "rect": [10, 10, 40, 20], "fill": "#ff0000"})).unwrap();
+            app.run("edit.copy", serde_json::json!({})).unwrap();
+            app.run("file.new", serde_json::json!({"width": 80, "height": 60})).unwrap();
+            let signature = app.os_clip_sig;
+            assert!(signature.is_some());
+            let before = app.session.active().unwrap().doc.walk().len();
+            busy.store(true, Ordering::SeqCst);
+            let result = crate::menus::invoke(&mut app, &egui::Context::default(), id, serde_json::json!({}));
+            assert_eq!(result.unwrap_err(), "clipboard is busy", "{id}");
+            assert!(app.ui.status_error);
+            assert_eq!(app.session.documents().len(), 2);
+            assert_eq!(app.session.active().unwrap().doc.walk().len(), before);
+            assert_eq!(app.os_clip_sig, signature);
+            assert!(app.session.clipboard.as_ref().unwrap().layers.is_some());
+            // The New dialog also reads the clipboard; a failed preset probe must preserve it.
+            app.new_document_fields();
+            assert_eq!(app.os_clip_sig, signature);
+            assert!(app.session.clipboard.as_ref().unwrap().layers.is_some());
+            // An automation error must not leave script events disabled by the dispatch guard.
+            app.automation_input = true;
+            app.session.edit_prefs(|prefs| prefs.script_events.enabled = true);
+            assert!(app.run(id, serde_json::json!({})).is_err());
+            assert!(app.session.prefs().script_events.enabled);
+            app.automation_input = false;
+            busy.store(false, Ordering::SeqCst);
+            app.run("edit.pasteSpecial.pasteInPlace", serde_json::json!({})).unwrap();
+            let state = app.session.active().unwrap();
+            assert!(matches!(state.doc.layer(state.active_layer.unwrap()).unwrap().content, photocraft_doc::LayerContent::Shape(_)));
+            assert!(!app.ui.status_error);
+        }
+    }
+
     /// An app whose OS clipboard is `os`, counting every read in `reads`.
     fn app_with_os_clipboard(os: &OsClip, reads: &Arc<std::sync::atomic::AtomicUsize>) -> PhotocraftApp {
         let (b, n) = (os.clone(), reads.clone());
         let get = move || {
             n.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
-            b.lock().unwrap().clone()
+            Ok(b.lock().unwrap().clone())
         };
         let mut app = PhotocraftApp::new(Session::new(), Services { clipboard_get_image: Some(Box::new(get)), ..Default::default() });
         app.session.execute("file.new", serde_json::json!({"width": 64, "height": 64})).unwrap();
@@ -2700,7 +2808,7 @@ mod clipboard_tests {
         let (b, n) = (os.clone(), Arc::clone(&reads));
         let get = move || {
             n.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
-            b.lock().unwrap().clone()
+            Ok(b.lock().unwrap().clone())
         };
         let mut app = PhotocraftApp::new(Session::new(), Services { clipboard_get_image: Some(Box::new(get)), ..Default::default() });
         let ctx = egui::Context::default();
@@ -2750,7 +2858,7 @@ mod clipboard_tests {
             PhotocraftApp::setup_context(&cc.egui_ctx, Default::default());
             let get = move || {
                 n.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
-                b.lock().unwrap().clone()
+                Ok(b.lock().unwrap().clone())
             };
             let mut app = PhotocraftApp::new(Session::new(), Services { clipboard_get_image: Some(Box::new(get)), ..Default::default() });
             app.session.execute("file.new", serde_json::json!({"width": 64, "height": 64})).unwrap();

crates/ui-egui/src/menus.rs

@@ -544,7 +544,7 @@ pub(crate) fn invoke_unguarded(app: &mut PhotocraftApp, ctx: &egui::Context, id:
         "edit.paste" if params.as_object().is_none_or(|o| o.is_empty()) => {
             // Photoshop: paste in place when the copied area is visible, else centred in the view;
             // images from other apps are always centred.
-            app.import_os_clipboard();
+            app.import_os_clipboard()?;
             app.clip_read_for_paste = true;
             let external = app.clip_external;
             let visible = !external