LR-EXP-PHOTOS: Apple Photos imports in the activity stack (#345)

Rebased onto the activity stack (#601). An export adding its files to
Photos keeps its row through that step, with "Adding to Apple Photos…" as
the detail. The worker turns the row's ✕ off (TaskHandle::set_cancellable)
before its last look at the cancel flag and reports what it saw, so a
cancel either stops the export before Photos is asked or is refused, never
both "cancelled" and imported. activity: a per-task gate orders a cancel's
decision against a step switching cancelling off, so a cancel can't be
accepted between the switch and the step's look at the flag and then be
ignored. An import of its own (export.addToPhotos, or
the Photos step left running after a foreground app.export) holds a row of
kind applePhotos while Photos works: no ✕, no count, the album as detail.
The desktop app refuses export.addToPhotos {wait: true}: its window's
thread never waits for Photos. The label is in every catalog; tests assert
the rows, the refused cancel and the refused wait.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/engine/src/activity.rs

@@ -73,7 +73,7 @@ impl Unit {
 pub struct TaskInfo {
     pub id: u64,
     /// A stable id: `import`, `export`, `scan`, `previews`, `smartPreviews`, `lightroom`, `merge`, `download`,
-    /// `faces`, `denoise`, `findMissing`.
+    /// `faces`, `denoise`, `findMissing`, `applePhotos`.
     pub kind: &'static str,
     /// English; the frontend translates it.
     pub label: String,
@@ -100,6 +100,12 @@ struct Entry {
     detail: Mutex<String>,
     cancel: Option<Arc<AtomicBool>>,
     cancellable: AtomicBool,
+    /// Held while a cancel is decided ([`Activity::cancel`], [`Activity::cancel_all`]) and while
+    /// cancelling is switched ([`TaskGuard::set_cancellable`], [`TaskHandle::set_cancellable`]): a
+    /// step that turns it off and then reads the cancel flag either sees a cancel that was
+    /// accepted, or makes the cancel fail. Without it a cancel could pass the check, the step
+    /// start, and the flag be set after the step's last look at it: accepted but ignored.
+    gate: Mutex<()>,
 }
 
 fn lock<T>(m: &Mutex<T>) -> MutexGuard<'_, T> {
@@ -111,6 +117,23 @@ fn capped(text: &str) -> String {
 }
 
 impl Entry {
+    fn set_cancellable(&self, yes: bool) {
+        let _gate = lock(&self.gate);
+        self.cancellable.store(yes, Ordering::Relaxed);
+    }
+
+    /// Set the cancel flag if the task can be cancelled now (see [`Entry::gate`]).
+    fn try_cancel(&self) -> bool {
+        let _gate = lock(&self.gate);
+        match &self.cancel {
+            Some(flag) if self.is_cancellable() => {
+                flag.store(true, Ordering::Relaxed);
+                true
+            }
+            _ => false,
+        }
+    }
+
     fn is_cancellable(&self) -> bool {
         self.cancel.is_some() && self.cancellable.load(Ordering::Relaxed)
     }
@@ -165,6 +188,7 @@ impl Activity {
             detail: Mutex::new(String::new()),
             cancellable: AtomicBool::new(cancel.is_some()),
             cancel,
+            gate: Mutex::new(()),
         });
         lock(&self.inner.tasks).push(entry.clone());
         TaskGuard { activity: self.clone(), entry }
@@ -180,26 +204,13 @@ impl Activity {
     pub fn cancel(&self, id: u64) -> Result<(), String> {
         let tasks = lock(&self.inner.tasks);
         let entry = tasks.iter().find(|e| e.id == id).ok_or_else(|| format!("no task {id}"))?;
-        match (&entry.cancel, entry.is_cancellable()) {
-            (Some(flag), true) => {
-                flag.store(true, Ordering::Relaxed);
-                Ok(())
-            }
-            _ => Err(format!("{} can't be cancelled", entry.label)),
-        }
+        if entry.try_cancel() { Ok(()) } else { Err(format!("{} can't be cancelled", entry.label)) }
     }
 
     /// Ask every cancellable task to stop; returns how many were asked.
     pub fn cancel_all(&self) -> usize {
         let tasks = lock(&self.inner.tasks);
-        let mut n = 0;
-        for e in tasks.iter().filter(|e| e.is_cancellable()) {
-            if let Some(flag) = &e.cancel {
-                flag.store(true, Ordering::Relaxed);
-                n += 1;
-            }
-        }
-        n
+        tasks.iter().filter(|e| e.try_cancel()).count()
     }
 
     /// Tasks that quitting would cut short: cancellable ones not already stopping.
@@ -235,7 +246,7 @@ impl TaskGuard {
 
     /// A step that must not be cut short (the Lightroom import's commit) turns cancelling off for its duration.
     pub fn set_cancellable(&self, yes: bool) {
-        self.entry.cancellable.store(yes, Ordering::Relaxed);
+        self.entry.set_cancellable(yes);
     }
 
     pub fn cancel_flag(&self) -> Option<Arc<AtomicBool>> {
@@ -283,6 +294,13 @@ impl TaskHandle {
     pub fn is_cancelled(&self) -> bool {
         self.entry.is_cancelled()
     }
+
+    /// [`TaskGuard::set_cancellable`] from the worker, at the moment its step starts (an export
+    /// handing its files to Apple Photos, which can't be stopped once asked). Read the cancel flag
+    /// after turning it off: a cancel accepted before is seen then, any later one is refused.
+    pub fn set_cancellable(&self, yes: bool) {
+        self.entry.set_cancellable(yes);
+    }
 }
 
 #[cfg(test)]
@@ -380,6 +398,35 @@ mod tests {
         assert_eq!(a.list().len(), 1);
     }
 
+    /// Codex review of #643: a worker that turns cancelling off and then reads the cancel flag
+    /// (an export handing its files to Apple Photos) must either see a cancel that was accepted or
+    /// make it fail, never let one be accepted and then ignored. Here the test holds the task's gate
+    /// as a cancel does while it decides: the worker's switch waits for it, and then sees the cancel.
+    #[test]
+    fn a_step_that_turns_cancelling_off_sees_or_refuses_every_cancel() {
+        let a = Activity::default();
+        let flag = Arc::new(AtomicBool::new(false));
+        let g = a.start("export", "Exporting", Cancel::Flag(flag.clone()));
+        let handle = g.handle();
+        let gate = lock(&g.entry.gate); // a cancel deciding
+        let (tx, rx) = std::sync::mpsc::channel();
+        let (h, f) = (handle.clone(), flag.clone());
+        let worker = std::thread::spawn(move || {
+            h.set_cancellable(false);
+            tx.send(f.load(Ordering::Relaxed)).unwrap();
+        });
+        assert!(rx.recv_timeout(std::time::Duration::from_millis(100)).is_err(), "the switch waits for the cancel");
+        flag.store(true, Ordering::Relaxed); // the cancel goes through...
+        drop(gate);
+        assert!(rx.recv_timeout(std::time::Duration::from_secs(10)).unwrap(), "...and the step sees it");
+        worker.join().unwrap();
+        // once it is off, cancels are refused
+        let g2 = a.start("export", "Exporting", Cancel::Yes);
+        g2.handle().set_cancellable(false);
+        assert!(a.cancel(g2.id()).is_err());
+        assert_eq!(a.cancel_all(), 0, "{:?}", a.list());
+    }
+
     #[test]
     fn handle_updates_from_another_thread() {
         let a = Activity::default();

crates/engine/src/apple_photos.rs

@@ -28,6 +28,8 @@ use std::time::Duration;
 
 use serde_json::{Value, json};
 
+use crate::activity::{Activity, Cancel, TaskGuard};
+
 /// Run by osascript with `argv` = [`MARKER`], the album name (empty: none), then the files' POSIX
 /// paths. Prints the imported media items' ids, one per line.
 ///
@@ -469,7 +471,7 @@ impl Imports {
             return Err("an album name can't contain a NUL character".into());
         }
         let job = self.begin(0, album, true)?;
-        Ok(Reservation { runner, album: album.to_string(), job: Some(job) })
+        Ok(Reservation { runner, album: album.to_string(), job: Some(job), activity: None })
     }
 
     /// [`Imports::wait_idle_within`] with the [`WaitLimits::default`].
@@ -542,17 +544,39 @@ impl crate::Session {
     }
 }
 
-/// Ask Photos to import `paths` (checked; `album` trimmed) for `job`: here, or on a worker thread.
-fn launch(runner: &Runner, job: &Arc<ImportJob>, paths: Vec<String>, album: String, background: bool) {
+/// The activity stack's kind for an Apple Photos import of its own (`activity.list`).
+pub const ACTIVITY_KIND: &str = "applePhotos";
+/// ...and its label (translated by the frontend).
+pub const ACTIVITY_LABEL: &str = "Adding to Apple Photos";
+
+/// A row in the activity stack for an import that runs on its own (not as the last step of an
+/// export, whose row covers it): no ✕ (Photos can't be stopped once asked), no count (Photos
+/// reports none until it is done), the album as its detail.
+fn row(activity: Option<&Activity>, album: &str) -> Option<TaskGuard> {
+    let row = activity?.start(ACTIVITY_KIND, ACTIVITY_LABEL, Cancel::No);
+    row.progress(0, 0);
+    if !album.is_empty() {
+        row.detail(album);
+    }
+    Some(row)
+}
+
+/// Ask Photos to import `paths` (checked; `album` trimmed) for `job`: here, or on a worker thread,
+/// holding `row` (the activity stack's) until Photos is done.
+fn launch(runner: &Runner, job: &Arc<ImportJob>, paths: Vec<String>, album: String, background: bool, row: Option<TaskGuard>) {
     job.requested.store(paths.len(), Ordering::Relaxed);
     *job.started.lock().unwrap_or_else(PoisonError::into_inner) = Some(std::time::Instant::now());
     job.exporting.store(false, Ordering::Relaxed);
     if !background {
+        let _row = row;
         run_to_completion(runner, job, &paths, &album);
         return;
     }
     let (r, j) = (runner.clone(), job.clone());
-    let started = std::thread::Builder::new().name("apple-photos".into()).spawn(move || run_to_completion(&r, &j, &paths, &album));
+    let started = std::thread::Builder::new().name("apple-photos".into()).spawn(move || {
+        let _row = row;
+        run_to_completion(&r, &j, &paths, &album)
+    });
     if let Err(e) = started {
         job.finish(Outcome::Failed(format!("Couldn't start adding the photos to Apple Photos: {e}")));
     }
@@ -581,13 +605,22 @@ fn run_to_completion(runner: &Runner, job: &ImportJob, paths: &[String], album:
 }
 
 /// Add `paths` to Photos as a job of `imports`: on a worker thread (`background`; the job is
-/// returned running, and announced by the app when it ends) or here (returned finished). An
-/// error, and no job, for a bad request or while another import is reserved or running.
-pub fn import_job(runner: &Runner, imports: &Imports, paths: Vec<String>, album: &str, background: bool) -> Result<Arc<ImportJob>, String> {
+/// returned running, and announced by the app when it ends) or here (returned finished), with a
+/// row in `activity` while Photos works. An error, and no job, for a bad request or while
+/// another import is reserved or running.
+pub fn import_job(
+    runner: &Runner,
+    imports: &Imports,
+    paths: Vec<String>,
+    album: &str,
+    background: bool,
+    activity: Option<&Activity>,
+) -> Result<Arc<ImportJob>, String> {
     let album = check(&paths, album)?;
     let job = imports.begin(paths.len(), &album, false)?;
     job.quiet.store(!background, Ordering::Relaxed);
-    launch(runner, &job, paths, album, background);
+    let row = row(activity, &album);
+    launch(runner, &job, paths, album, background, row);
     Ok(job)
 }
 
@@ -598,9 +631,18 @@ pub struct Reservation {
     runner: Runner,
     album: String,
     job: Option<Arc<ImportJob>>,
+    /// Where [`Reservation::start`]'s import shows while Photos works.
+    activity: Option<Activity>,
 }
 
 impl Reservation {
+    /// Show the import [`Reservation::start`] leaves running in `activity`. ([`Reservation::run`]
+    /// runs on the export's worker, and the export's own row covers it.)
+    pub fn shown_in(mut self, activity: Activity) -> Self {
+        self.activity = Some(activity);
+        self
+    }
+
     pub fn job(&self) -> Option<&Arc<ImportJob>> {
         self.job.as_ref()
     }
@@ -633,7 +675,7 @@ impl Reservation {
     pub fn run(mut self, files: &[Value], cancelled: bool) -> Value {
         match self.take(files, cancelled) {
             Ok((job, paths)) => {
-                launch(&self.runner, &job, paths, self.album.clone(), false);
+                launch(&self.runner, &job, paths, self.album.clone(), false, None);
                 job.json()
             }
             Err(done) => done,
@@ -646,7 +688,8 @@ impl Reservation {
         match self.take(files, false) {
             Ok((job, paths)) => {
                 job.quiet.store(false, Ordering::Relaxed);
-                launch(&self.runner, &job, paths, self.album.clone(), true);
+                let row = row(self.activity.as_ref(), &self.album);
+                launch(&self.runner, &job, paths, self.album.clone(), true, row);
                 job.json()
             }
             Err(done) => done,
@@ -984,7 +1027,7 @@ mod tests {
         let reserved = imports.reserve(runner.clone(), "Trip").unwrap();
         assert_eq!(reserved.job().map(|j| j.json()), Some(json!({"job": 1, "running": true, "exporting": true, "requested": 0, "album": "Trip"})));
         // while the export writes: other imports and exports are refused, naming it
-        let e = import_job(&runner, &imports, paths.clone(), "", false).unwrap_err();
+        let e = import_job(&runner, &imports, paths.clone(), "", false, None).unwrap_err();
         assert!(e.contains("An export is about to add its files to Apple Photos (import 1)"), "{e}");
         assert!(imports.reserve(runner.clone(), "").err().unwrap().contains("(import 1)"));
         assert!(calls.lock().unwrap().is_empty());
@@ -1003,7 +1046,7 @@ mod tests {
         assert!(out["error"].as_str().unwrap().contains("no such file"), "{out}");
         assert_eq!(calls.lock().unwrap().len(), 1);
         assert!(imports.running().is_none());
-        assert_eq!(import_job(&runner, &imports, paths, "", false).unwrap().id, 4);
+        assert_eq!(import_job(&runner, &imports, paths, "", false, None).unwrap().id, 4);
         let _ = std::fs::remove_dir_all(dir);
     }
 
@@ -1017,11 +1060,11 @@ mod tests {
         let imports = Imports::default();
         let stopped = |j: &ImportJob| j.json()["error"].as_str().is_some_and(|e| e.contains("stopped unexpectedly"));
         // waiting here
-        let job = import_job(&panicky, &imports, paths.clone(), "", false).unwrap();
+        let job = import_job(&panicky, &imports, paths.clone(), "", false, None).unwrap();
         assert!(job.finished() && stopped(&job), "{}", job.json());
         assert!(imports.running().is_none());
         // on a worker
-        let job = import_job(&panicky, &imports, paths.clone(), "", true).unwrap();
+        let job = import_job(&panicky, &imports, paths.clone(), "", true, None).unwrap();
         let t0 = std::time::Instant::now();
         while !job.finished() {
             assert!(t0.elapsed() < Duration::from_secs(20));
@@ -1040,7 +1083,7 @@ mod tests {
         }
         assert!(stopped(&job));
         // the slot is free: the next import runs
-        assert_eq!(import_job(&good, &imports, paths, "", false).unwrap().json()["imported"], 1);
+        assert_eq!(import_job(&good, &imports, paths, "", false, None).unwrap().json()["imported"], 1);
         let _ = std::fs::remove_dir_all(dir);
     }
 
@@ -1050,7 +1093,7 @@ mod tests {
         let (dir, paths) = files("idle", 1);
         let (runner, go) = delayed();
         let imports = Imports::default();
-        let job = import_job(&runner, &imports, paths, "", true).unwrap();
+        let job = import_job(&runner, &imports, paths, "", true, None).unwrap();
         assert!(!job.finished());
         let release = std::thread::spawn(move || {
             std::thread::sleep(Duration::from_millis(200));
@@ -1099,7 +1142,7 @@ mod tests {
         assert!(t0.elapsed() >= Duration::from_millis(1000), "{:?}", t0.elapsed());
         assert_eq!(exporter.join().unwrap()["imported"], 2);
         // an import past its own limit is given up on at its deadline (not the cap)
-        let job = import_job(&taking(3000), &imports, paths.clone(), "", true).unwrap();
+        let job = import_job(&taking(3000), &imports, paths.clone(), "", true, None).unwrap();
         let t0 = std::time::Instant::now();
         assert!(!imports.wait_idle_within(limits));
         let waited = t0.elapsed();
@@ -1116,7 +1159,7 @@ mod tests {
         drop(held);
         assert!(imports.wait_idle_within(limits));
         // and the cap holds over everything
-        let job = import_job(&taking(1000), &imports, paths, "", true).unwrap();
+        let job = import_job(&taking(1000), &imports, paths, "", true, None).unwrap();
         let t0 = std::time::Instant::now();
         assert!(!imports.wait_idle_within(WaitLimits { cap: Duration::from_millis(200), ..limits }));
         assert!(t0.elapsed() < Duration::from_millis(900));
@@ -1147,11 +1190,11 @@ mod tests {
         let (runner, go) = delayed();
         let imports = Imports::default();
         let t0 = std::time::Instant::now();
-        let job = import_job(&runner, &imports, paths.clone(), " Trip ", true).unwrap();
+        let job = import_job(&runner, &imports, paths.clone(), " Trip ", true, None).unwrap();
         assert!(t0.elapsed() < Duration::from_secs(5), "the caller doesn't wait for Photos");
         assert_eq!(job.json(), json!({"job": 1, "running": true, "requested": 2, "album": "Trip"}));
         assert_eq!(imports.running().map(|j| j.id), Some(1));
-        let e = import_job(&runner, &imports, paths.clone(), "", true).unwrap_err();
+        let e = import_job(&runner, &imports, paths.clone(), "", true, None).unwrap_err();
         assert!(e.contains("still adding 2 file(s) from an earlier request (import 1)"), "{e}");
         let e = imports.reserve(runner.clone(), "").err().unwrap();
         assert!(e.contains("still adding"), "{e}");
@@ -1165,7 +1208,7 @@ mod tests {
         assert_eq!((done["running"].as_bool(), done["imported"].as_u64(), done["album"].as_str()), (Some(false), Some(2), Some("Trip")), "{done}");
         // finished: the next one may start; a failed one says why
         let (fails, _) = fake(Err(RunError::Spawn("denied".into())));
-        let job = import_job(&fails, &imports, paths, "", false).unwrap();
+        let job = import_job(&fails, &imports, paths, "", false, None).unwrap();
         assert_eq!(job.id, 2);
         assert!(job.json()["error"].as_str().unwrap().contains("denied"));
         assert!(imports.running().is_none() && imports.latest().is_some_and(|j| j.id == 2));
@@ -1178,7 +1221,7 @@ mod tests {
         let (runner, _) = fake(ok("A\n"));
         let imports = Imports::default();
         for _ in 0..KEPT_JOBS + 3 {
-            import_job(&runner, &imports, paths.clone(), "", false).unwrap();
+            import_job(&runner, &imports, paths.clone(), "", false, None).unwrap();
         }
         let all = imports.all();
         assert_eq!(all.len(), KEPT_JOBS);
@@ -1187,6 +1230,61 @@ mod tests {
         let _ = std::fs::remove_dir_all(dir);
     }
 
+    /// The activity stack (issue #345): an import of its own shows a row while Photos works (no ✕,
+    /// no count, the album as detail) and loses it when Photos is done; an export's own import
+    /// shows none of its own, the export's row covers it.
+    #[test]
+    fn imports_of_their_own_show_in_the_activity_stack() {
+        let (dir, paths) = files("activity", 2);
+        let activity = Activity::default();
+        let seen = Arc::new(Mutex::new(Vec::new()));
+        // Photos answers when the test lets it (`go`), so no assertion races the import's end
+        let (go, wait) = std::sync::mpsc::channel::<()>();
+        let wait = Mutex::new(wait);
+        let (a, rows) = (activity.clone(), seen.clone());
+        let runner: Runner = Arc::new(move |args: &[String], _| {
+            rows.lock().unwrap().push(a.list());
+            let _ = wait.lock().unwrap().recv_timeout(Duration::from_secs(20));
+            let n = args.len() - args.iter().position(|x| x == MARKER).unwrap() - 2;
+            Ok(Output { status: Some(0), stdout: "ID\n".repeat(n), stderr: String::new() })
+        });
+        let imports = Imports::default();
+        let ended = |job: &ImportJob| {
+            let deadline = std::time::Instant::now() + Duration::from_secs(20);
+            while !(job.finished() && activity.list().is_empty()) {
+                assert!(std::time::Instant::now() < deadline, "the import ends and its row goes");
+                std::thread::sleep(Duration::from_millis(5));
+            }
+        };
+        // waiting for Photos here
+        go.send(()).unwrap();
+        let job = import_job(&runner, &imports, paths.clone(), "Trip", false, Some(&activity)).unwrap();
+        assert_eq!(job.json()["imported"], 2);
+        assert!(activity.list().is_empty(), "the row goes when Photos is done");
+        let during = seen.lock().unwrap().last().cloned().unwrap();
+        assert_eq!(during.len(), 1, "{during:?}");
+        assert_eq!((during[0].kind, during[0].label.as_str(), during[0].detail.as_str()), (ACTIVITY_KIND, ACTIVITY_LABEL, "Trip"));
+        assert_eq!((during[0].cancellable, during[0].total), (false, 0), "no ✕ and no count");
+        // on a worker thread: the row stays while Photos works, and goes once the import ends
+        let job = import_job(&runner, &imports, paths.clone(), "", true, Some(&activity)).unwrap();
+        assert_eq!(activity.list().first().map(|r| r.kind), Some(ACTIVITY_KIND));
+        go.send(()).unwrap();
+        ended(&job);
+        // an export's own import: the export's row covers it
+        let exported: Vec<Value> = paths.iter().map(|p| json!({"path": p})).collect();
+        go.send(()).unwrap();
+        imports.reserve(runner.clone(), "").unwrap().shown_in(activity.clone()).run(&exported, false);
+        assert!(seen.lock().unwrap().last().unwrap().is_empty(), "no row of its own while the export's worker waits");
+        // ...unless it is left running on its own once the export is done
+        let started = imports.reserve(runner.clone(), "").unwrap().shown_in(activity.clone()).start(&exported);
+        assert_eq!(started["running"], true);
+        assert_eq!(activity.list().first().map(|r| r.kind), Some(ACTIVITY_KIND));
+        go.send(()).unwrap();
+        let job = imports.get(started["job"].as_u64().unwrap()).unwrap();
+        ended(&job);
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
     #[test]
     fn timeouts_grow_with_the_batch_and_stay_bounded() {
         assert_eq!(timeout_for(1), Duration::from_secs(605));

crates/engine/src/cmd/export.rs

@@ -115,10 +115,17 @@ fn add_to_photos(s: &mut Session, p: &Value) -> Result<Value> {
     // in the middle of the import
     let wait = match p.get("wait") {
         None | Some(Value::Null) => !s.apple_photos_background,
+        // the desktop app answers on its window's thread, which never waits for Photos
+        Some(Value::Bool(true)) if s.apple_photos_background => {
+            return Err(bad(
+                ID,
+                "`wait: true` isn't available in the desktop app: the import runs in the background; its outcome comes from export.photosImports {job}",
+            ));
+        }
         Some(Value::Bool(b)) => *b,
         Some(_) => return Err(bad(ID, "`wait` must be true or false")),
     };
-    let job = crate::apple_photos::import_job(&runner, &s.apple_photos_imports, paths, album, !wait).map_err(|e| bad(ID, e))?;
+    let job = crate::apple_photos::import_job(&runner, &s.apple_photos_imports, paths, album, !wait, Some(&s.activity)).map_err(|e| bad(ID, e))?;
     match job.outcome() {
         Some(crate::apple_photos::Outcome::Failed(e)) => Err(bad(ID, e)),
         _ => Ok(job.json()),
@@ -193,7 +200,7 @@ pub fn specs() -> Vec<CommandSpec> {
         "Add to Apple Photos",
         [],
         None,
-        "{paths: [absolute file paths], album?: name (an album at the top level of Photos, made when missing; default none), wait?} → once Photos is done, {job, running: false, requested, album, imported, ids: [Photos media item ids], warning?}; with wait: false, at once {job, running: true, requested, album} and the outcome later from export.photosImports {job}. wait defaults to true without a window (lightcraft-cli, the MCP server) and to false in the desktop app (its control requests are answered on the UI thread). macOS: asks Photos to import the files (the first time, macOS asks the user to allow LightCraft to control Photos). One import runs at a time, and an export adding to Photos reserves it before writing: another is refused until it ends. app.export does this for the files it writes with `addToPhotos: true`",
+        "{paths: [absolute file paths], album?: name (an album at the top level of Photos, made when missing; default none), wait?} → once Photos is done, {job, running: false, requested, album, imported, ids: [Photos media item ids], warning?}; with wait: false, at once {job, running: true, requested, album} and the outcome later from export.photosImports {job}. wait defaults to true without a window (lightcraft-cli, the MCP server) and to false in the desktop app, where wait: true is refused (its control requests are answered on the UI thread, which never waits for Photos). macOS: asks Photos to import the files (the first time, macOS asks the user to allow LightCraft to control Photos). One import runs at a time, and an export adding to Photos reserves it before writing: another is refused until it ends. app.export does this for the files it writes with `addToPhotos: true`",
         photos_enabled,
         add_to_photos
     ));

crates/engine/src/export.rs

@@ -1665,7 +1665,9 @@ impl crate::Session {
         #[cfg(not(target_arch = "wasm32"))]
         {
             let runner = self.apple_photos.clone().ok_or_else(|| crate::apple_photos::unavailable().to_string())?;
-            AfterExport::apple_photos(runner, &o.photos_album, &self.apple_photos_imports)
+            let mut after = AfterExport::apple_photos(runner, &o.photos_album, &self.apple_photos_imports)?;
+            after.apple_photos = after.apple_photos.map(|r| r.shown_in(self.activity.clone()));
+            Ok(after)
         }
         #[cfg(target_arch = "wasm32")]
         Err("Apple Photos is only available on macOS".into())

crates/engine/src/tests_export.rs

@@ -647,9 +647,12 @@ mod apple_photos {
         let all = s.execute("export.photosImports", &json!({})).unwrap();
         assert_eq!(all["imports"].as_array().map(Vec::len), Some(1));
         assert!(s.execute("export.photosImports", &json!({"job": 7})).unwrap_err().to_string().contains("no import 7"));
+        // the window's thread never waits for Photos: `wait: true` is refused here, before any import
+        let e = s.execute("export.addToPhotos", &json!({"paths": paths, "wait": true})).unwrap_err().to_string();
+        assert!(e.contains("isn't available in the desktop app"), "{e}");
         // finished: the next one may start
         go.send(()).unwrap();
-        assert_eq!(s.execute("export.addToPhotos", &json!({"paths": paths, "wait": true})).unwrap()["job"], 2);
+        assert_eq!(s.execute("export.addToPhotos", &json!({"paths": paths})).unwrap()["job"], 2);
     }
 
     /// Review of #236: an export adding to Photos reserves the import before it writes its files.

crates/ui-egui/locales/de.json

@@ -2043,5 +2043,6 @@
   "Add to Apple Photos": "Zu Apple Fotos hinzufügen",
   "Adding to Apple Photos…": "Wird zu Apple Fotos hinzugefügt…",
   "not added to Apple Photos (cancelled)": "nicht zu Apple Fotos hinzugefügt (abgebrochen)",
-  "The first time, macOS asks whether LightCraft may control Photos.": "Beim ersten Mal fragt macOS, ob LightCraft Fotos steuern darf."
+  "The first time, macOS asks whether LightCraft may control Photos.": "Beim ersten Mal fragt macOS, ob LightCraft Fotos steuern darf.",
+  "Adding to Apple Photos": "Wird zu Apple Fotos hinzugefügt"
 }

crates/ui-egui/locales/es.json

@@ -1888,5 +1888,6 @@
   "Add to Apple Photos": "Añadir a Fotos de Apple",
   "Adding to Apple Photos…": "Añadiendo a Fotos de Apple…",
   "not added to Apple Photos (cancelled)": "no se añadieron a Fotos de Apple (cancelado)",
-  "The first time, macOS asks whether LightCraft may control Photos.": "La primera vez, macOS pregunta si LightCraft puede controlar Fotos."
+  "The first time, macOS asks whether LightCraft may control Photos.": "La primera vez, macOS pregunta si LightCraft puede controlar Fotos.",
+  "Adding to Apple Photos": "Añadiendo a Fotos de Apple"
 }

crates/ui-egui/locales/fr.json

@@ -1888,5 +1888,6 @@
   "Add to Apple Photos": "Ajouter à Apple Photos",
   "Adding to Apple Photos…": "Ajout à Apple Photos…",
   "not added to Apple Photos (cancelled)": "non ajouté à Apple Photos (annulé)",
-  "The first time, macOS asks whether LightCraft may control Photos.": "La première fois, macOS demande si LightCraft peut contrôler Photos."
+  "The first time, macOS asks whether LightCraft may control Photos.": "La première fois, macOS demande si LightCraft peut contrôler Photos.",
+  "Adding to Apple Photos": "Ajout à Apple Photos"
 }

crates/ui-egui/locales/ja.json

@@ -1890,5 +1890,6 @@
   "Add to Apple Photos": "Appleの写真に追加",
   "Adding to Apple Photos…": "Appleの写真に追加中…",
   "not added to Apple Photos (cancelled)": "Appleの写真には追加されませんでした（キャンセル）",
-  "The first time, macOS asks whether LightCraft may control Photos.": "初回は、LightCraftに写真の操作を許可するかmacOSが確認します。"
+  "The first time, macOS asks whether LightCraft may control Photos.": "初回は、LightCraftに写真の操作を許可するかmacOSが確認します。",
+  "Adding to Apple Photos": "Appleの写真に追加中"
 }

crates/ui-egui/locales/pt-br.json

@@ -1888,5 +1888,6 @@
   "Add to Apple Photos": "Adicionar ao Fotos da Apple",
   "Adding to Apple Photos…": "Adicionando ao Fotos da Apple…",
   "not added to Apple Photos (cancelled)": "não adicionadas ao Fotos da Apple (cancelado)",
-  "The first time, macOS asks whether LightCraft may control Photos.": "Na primeira vez, o macOS pergunta se o LightCraft pode controlar o Fotos."
+  "The first time, macOS asks whether LightCraft may control Photos.": "Na primeira vez, o macOS pergunta se o LightCraft pode controlar o Fotos.",
+  "Adding to Apple Photos": "Adicionando ao Fotos da Apple"
 }

crates/ui-egui/locales/ru.json

@@ -1602,5 +1602,6 @@
   "Add to Apple Photos": "Добавить в Apple Фото",
   "Adding to Apple Photos…": "Добавление в Apple Фото…",
   "not added to Apple Photos (cancelled)": "не добавлено в Apple Фото (отменено)",
-  "The first time, macOS asks whether LightCraft may control Photos.": "В первый раз macOS спросит, может ли LightCraft управлять приложением «Фото»."
+  "The first time, macOS asks whether LightCraft may control Photos.": "В первый раз macOS спросит, может ли LightCraft управлять приложением «Фото».",
+  "Adding to Apple Photos": "Добавление в Apple Фото"
 }

crates/ui-egui/locales/uk.json

@@ -2082,5 +2082,6 @@
   "Add to Apple Photos": "Додати до Apple Фото",
   "Adding to Apple Photos…": "Додавання до Apple Фото…",
   "not added to Apple Photos (cancelled)": "не додано до Apple Фото (скасовано)",
-  "The first time, macOS asks whether LightCraft may control Photos.": "Першого разу macOS запитає, чи може LightCraft керувати програмою «Фото»."
+  "The first time, macOS asks whether LightCraft may control Photos.": "Першого разу macOS запитає, чи може LightCraft керувати програмою «Фото».",
+  "Adding to Apple Photos": "Додавання до Apple Фото"
 }

crates/ui-egui/locales/zh-hans.json

@@ -1888,5 +1888,6 @@
   "Add to Apple Photos": "添加到 Apple 照片",
   "Adding to Apple Photos…": "正在添加到 Apple 照片…",
   "not added to Apple Photos (cancelled)": "未添加到 Apple 照片（已取消）",
-  "The first time, macOS asks whether LightCraft may control Photos.": "首次使用时，macOS 会询问是否允许 LightCraft 控制“照片”。"
+  "The first time, macOS asks whether LightCraft may control Photos.": "首次使用时，macOS 会询问是否允许 LightCraft 控制“照片”。",
+  "Adding to Apple Photos": "正在添加到 Apple 照片"
 }

crates/ui-egui/locales/zh-hant.json

@@ -1888,5 +1888,6 @@
   "Add to Apple Photos": "加入 Apple 照片",
   "Adding to Apple Photos…": "正在加入 Apple 照片…",
   "not added to Apple Photos (cancelled)": "未加入 Apple 照片（已取消）",
-  "The first time, macOS asks whether LightCraft may control Photos.": "第一次使用時，macOS 會詢問是否允許 LightCraft 控制「照片」。"
+  "The first time, macOS asks whether LightCraft may control Photos.": "第一次使用時，macOS 會詢問是否允許 LightCraft 控制「照片」。",
+  "Adding to Apple Photos": "正在加入 Apple 照片"
 }

crates/ui-egui/src/export_task.rs

@@ -22,8 +22,9 @@ pub struct ExportTask {
     /// Photos started so far and the file in progress.
     pub progress: Arc<Mutex<(usize, String)>>,
     pub cancel: Arc<AtomicBool>,
-    /// The files (per-photo results) and what [`AfterExport`] did with them (`applePhotos`).
-    rx: Receiver<Result<(Vec<Value>, Option<Value>), String>>,
+    /// The files (per-photo results), what [`AfterExport`] did with them (`applePhotos`) and
+    /// whether the export was cancelled (as the worker saw it when the batch ended).
+    rx: Receiver<Result<(Vec<Value>, Option<Value>, bool), String>>,
     /// The export's row in the activity stack (`activity.cancel` sets `cancel`).
     guard: TaskGuard,
 }
@@ -48,6 +49,9 @@ pub fn start(app: &mut LightcraftApp, items: Vec<PreparedExport>, opts: ExportOp
     let cancel = Arc::new(AtomicBool::new(false));
     let (tx, rx) = channel();
     let (p, c) = (progress.clone(), cancel.clone());
+    let guard = app.session.activity.start("export", "Exporting", Cancel::Flag(cancel.clone()));
+    guard.progress(0, total as u64);
+    let row = guard.handle();
     // translated here: the language is the UI thread's, and the worker would show English
     let adding = crate::i18n::tr("Adding to Apple Photos…").to_string();
     let work = move || {
@@ -59,13 +63,17 @@ pub fn start(app: &mut LightcraftApp, items: Vec<PreparedExport>, opts: ExportOp
             !c.load(Ordering::Relaxed)
         });
         let r = r.map(|files| {
-            if !after.is_none()
-                && let Ok(mut g) = p.lock()
-            {
-                g.1 = adding;
+            if !after.is_none() {
+                // Photos can't be stopped once asked: no ✕ from here, before the last look at
+                // the cancel flag, so a cancel either stops the export before Photos or is refused
+                row.set_cancellable(false);
+                if let Ok(mut g) = p.lock() {
+                    g.1 = adding;
+                }
             }
-            let photos = after.run(&files, c.load(Ordering::Relaxed));
-            (files, photos)
+            let cancelled = c.load(Ordering::Relaxed);
+            let photos = after.run(&files, cancelled);
+            (files, photos, cancelled)
         });
         let _ = tx.send(r);
     };
@@ -73,8 +81,6 @@ pub fn start(app: &mut LightcraftApp, items: Vec<PreparedExport>, opts: ExportOp
     std::thread::Builder::new().name("export".into()).spawn(work).map_err(|e| e.to_string())?;
     #[cfg(target_arch = "wasm32")]
     work();
-    let guard = app.session.activity.start("export", "Exporting", Cancel::Flag(cancel.clone()));
-    guard.progress(0, total as u64);
     app.export = Some(ExportTask { total, progress, cancel, rx, guard });
     Ok(json!({"background": true, "total": total}))
 }
@@ -114,7 +120,7 @@ pub fn start_contact_sheet(app: &mut LightcraftApp, params: &Value) -> Result<Va
                 Ok(vec![json!({"path": path, "pages": doc.pages, "photos": doc.photos, "bytes": doc.bytes.len(), "contactSheet": true})])
             });
         // a PDF isn't something Photos imports: contact sheets never add to Apple Photos
-        let _ = tx.send(result.map(|files| (files, None)));
+        let _ = tx.send(result.map(|files| (files, None, c.load(Ordering::Relaxed))));
     };
     #[cfg(not(target_arch = "wasm32"))]
     std::thread::Builder::new().name("contact-sheet".into()).spawn(work).map_err(|e| e.to_string())?;
@@ -178,13 +184,13 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) {
     let Some(task) = &app.export else { return };
     match task.rx.try_recv() {
         Ok(r) => {
-            let cancelled = task.cancel.load(Ordering::Relaxed);
+            let cancelled = matches!(&r, Ok((_, _, true)));
             let total = task.total;
             app.export = None;
             // Photos refused (permission, not installed…): leave the way out on screen longer
-            let photos_failed = matches!(&r, Ok((_, Some(p))) if p.get("error").is_some());
+            let photos_failed = matches!(&r, Ok((_, Some(p), _)) if p.get("error").is_some());
             let msg = match r {
-                Ok((files, photos)) => {
+                Ok((files, photos, _)) => {
                     let sheet = files.first().filter(|f| f["contactSheet"] == true);
                     let ok = files.iter().filter(|f| f.get("path").is_some()).count();
                     let failed: Vec<&Value> = files.iter().filter(|f| f.get("error").is_some()).collect();

crates/ui-egui/src/headless.rs

@@ -1885,7 +1885,10 @@ mod tests {
         h.app.services.write_shared = Some(Arc::new(|p: &str, bytes: &[u8]| lightcraft_engine::export::write_file(p, bytes)));
         let (release, held) = std::sync::mpsc::channel::<()>();
         let held = std::sync::Mutex::new(held);
+        let (entered, photos_asked) = std::sync::mpsc::channel::<()>();
+        let entered = std::sync::Mutex::new(entered);
         h.app.session.apple_photos = Some(Arc::new(move |_: &[String], _| {
+            let _ = entered.lock().unwrap().send(());
             let _ = held.lock().unwrap().recv_timeout(Duration::from_secs(30));
             Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID\n".into(), stderr: String::new() })
         }));
@@ -1893,19 +1896,25 @@ mod tests {
         let params = json!({"ids": ids, "dir": out, "longEdge": 48, "addToPhotos": true, "background": true});
         let r = h.request("engine.execute", json!({"command": "app.export", "params": params}), t);
         assert_eq!(r["ok"], true, "{r}");
-        let t0 = Instant::now();
-        let current = loop {
-            h.step();
-            let current = h.app.export.as_ref().map(|e| e.progress.lock().unwrap().1.clone()).unwrap_or_default();
-            if current.contains("Apple") || t0.elapsed() > Duration::from_secs(60) {
-                break current;
-            }
-        };
+        // Photos has been asked, and the window hasn't drawn a frame since (Codex review of #643):
+        // the export's row already has no ✕, and a cancel is refused rather than half-honoured
+        photos_asked.recv_timeout(Duration::from_secs(60)).expect("the export asks Photos");
+        let row = h.app.session.activity.list().into_iter().find(|r| r.kind == "export").expect("the export's row");
+        assert!(!row.cancellable, "{row:?}");
+        assert!(h.app.session.activity.cancel(row.id).is_err(), "Photos can't be stopped once asked");
+        let current = h.app.export.as_ref().map(|e| e.progress.lock().unwrap().1.clone()).unwrap_or_default();
         assert_eq!(current, "Wird zu Apple Fotos hinzugefügt…");
+        // the row says so on the next frame (issue #345)
+        h.step();
+        let row = h.app.session.activity.list().into_iter().find(|r| r.kind == "export").expect("the export's row");
+        assert_eq!((row.detail.as_str(), row.cancellable), ("Wird zu Apple Fotos hinzugefügt…", false), "{row:?}");
         release.send(()).unwrap();
+        let t0 = Instant::now();
         while h.app.export.is_some() && t0.elapsed() < Duration::from_secs(90) {
             h.step();
         }
+        let last = h.app.last_export_result.clone().expect("the export reports");
+        assert_eq!((last["cancelled"].as_bool(), last["applePhotos"]["imported"].as_u64()), (Some(false), Some(1)), "{last}");
         crate::i18n::set_language(crate::i18n::Locale::En);
         let _ = std::fs::remove_dir_all(&out);
     }

docs/apple-photos.md

@@ -29,11 +29,17 @@ An import can take minutes, and the first one waits for the user to answer macOS
 import is a job: `{job, running, requested, album}`, plus, once Photos is done, `{imported, ids, warning?}`,
 `{error}` or `{skipped}` (`exporting: true` while an export that will add its files is still writing them).
 
+While Photos works, the activity stack ([background-tasks.md](background-tasks.md)) shows it. An export adding its
+files keeps its own row, which reads "Adding to Apple Photos…" and loses its ✕, since Photos can't be stopped once
+asked. An import of its own (`export.addToPhotos`, or the Photos step left running after `app.export` without
+`background`) gets a row of kind `applePhotos`, "Adding to Apple Photos", with the album as its detail, no ✕ and no
+count, as Photos reports none until it is done.
+
 - In the **desktop app** nothing waits for Photos on the window's thread. The Export dialog's background export
   adds the files on its worker and reports them in its toast. `app.export` without `background` and
   `export.addToPhotos` answer at once with the running job (`running: true`); the app shows a toast when Photos
-  is done, and `export.photosImports {job}` gives the outcome. (`export.addToPhotos {wait: true}` waits there
-  too, holding the window until Photos is done.)
+  is done, and `export.photosImports {job}` gives the outcome. (`export.addToPhotos {wait: true}` is refused there:
+  the window's thread never waits for Photos.)
 - **Without a window** (the MCP server, `lightcraft-cli`, the headless snapshot) everything waits for Photos by
   default: `app.export` and `export.addToPhotos` return the finished job, so a one-shot command never ends in
   the middle of an import. `export.addToPhotos {wait: false}` returns the running job instead (for a

docs/mcp.md

@@ -278,7 +278,7 @@ plus `{imported, ids, warning?}`, `{error}` or `{skipped}` once Photos is done;
 way. Headless (this server, `lightcraft-cli`) waits for Photos by default; the desktop app (connect mode)
 answers with the running job and `export.photosImports {job}` gives the outcome. `export.addToPhotos {paths,
 album?, wait?}` does the same for files already on disk (`wait` defaults to true headless, false in the desktop
-app; with `wait: false` headless, the server waits for the import before it exits at end of input). One import
+app, which refuses `wait: true`; with `wait: false` headless, the server waits for the import before it exits at end of input). One import
 runs at a time, and an export adding to Photos reserves it before it writes: another is refused, naming the
 job in the way. On other platforms `addToPhotos: true` is refused before anything is written, and
 `export.addToPhotos` is disabled with the reason. The first time, macOS asks the user to allow LightCraft (or

docs/parity-checklist.md

@@ -53,9 +53,9 @@ cited here still exists. Formerly docs/parity.md.
 | Y. Menus | 82 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) |
 | Z. Keyboard shortcuts (desktop) | 75 | 3 | 3 | 1 | 49/52 (94%) | 23/24 (96%) |
 | Lightroom Classic extras | 33 | 15 | 35 | 9 | — | 23/25 (92%) |
-| **Total** | 397 | 49 | 73 | 36 | 192/200 (96%) | 143/155 (92%) |
+| **Total** | 397 | 50 | 72 | 36 | 192/200 (96%) | 143/155 (92%) |
 
-Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.2%** of 519 in-scope rows — P0 98.0% of 200 · P1 95.2% of 155 · P2 47.2% of 163.
+Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.3%** of 519 in-scope rows — P0 98.0% of 200 · P1 95.2% of 155 · P2 47.5% of 163.
 <!-- /parity:summary -->
 
 ## Top gaps and shortcut notes