diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index 16f0d70b..63a4deb7 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -63,6 +63,8 @@ USAGE: string), e.g. --opt colorSpace=displayP3 --opt bitDepth=16 --opt percent=50 --opt shortEdge=1080 --opt ppi=300 --opt format=original (copy + XMP sidecar) --opt metadata=none + --opt addToPhotos=true --opt photosAlbum=NAME (macOS: then add + the file to Apple Photos, into that album) lightcraft-cli snapshot [OPTIONS] [FILES/FOLDERS…] Run the full app UI headlessly (no window, no GPU: CPU-rasterized egui) and write PNGs. Options: @@ -698,6 +700,8 @@ fn render(args: &[String]) -> Result<(), String> { o.format = ExportFormat::parse(&ext) .ok_or_else(|| format!("{output}: unknown extension (use .jpg .png .tif .webp .avif .dng or --opt format=…)"))?; } + // Add to Apple Photos: refused before anything is written when it can't be done + let after = s.after_export(&o)?; let e = export_photo(&mut s, lightcraft_engine::catalog::PhotoId(id), &o, 1)?; // never over the input (or its sidecar), however it is spelled: `render IMG.jpg -o IMG.jpg` let sidecars: Vec<(String, &Vec)> = @@ -712,6 +716,12 @@ fn render(args: &[String]) -> Result<(), String> { eprintln!("lightcraft-cli: wrote {sc}"); } eprintln!("lightcraft-cli: wrote {output} ({}×{})", e.width, e.height); + if let Some(photos) = after.run(&[json!({"path": output})], false) { + if let Some(err) = photos.get("error").and_then(Value::as_str) { + return Err(format!("render: {output} was written, but {err}")); + } + eprintln!("lightcraft-cli: added to Apple Photos: {photos}"); + } Ok(()) } diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index 770a30df..cc71cc06 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -595,6 +595,8 @@ fn services(cc: &eframe::CreationContext<'_>, log_file: Option<&std::path::Path> fn open_session(in_memory: bool, dir: Option, seed_demo: bool) -> (Session, Option) { let (mut s, problem) = open_library_session(in_memory, dir, seed_demo); s.face_models_dir = lightcraft_engine::config::default_face_models_dir(); + // Control requests are answered on the UI thread: Add to Apple Photos must not wait there + s.apple_photos_background = true; (s, problem) } @@ -945,6 +947,16 @@ mod tests { let _ = std::fs::remove_dir_all(&home); } + /// Issue #236: the window answers control requests on its UI thread, so the desktop session + /// adds to Apple Photos on a worker (headless sessions wait instead). + #[test] + fn the_desktop_session_adds_to_apple_photos_in_the_background() { + let (s, problem) = open_session(true, None, false); + assert!(problem.is_none()); + assert!(s.apple_photos_background); + assert!(!lightcraft_engine::Session::new().apple_photos_background); + } + /// Issue #103: a damaged ui.json (which holds the library location) is kept aside and /// reported, not silently replaced by the defaults. #[test] diff --git a/crates/engine/src/activity.rs b/crates/engine/src/activity.rs index 654b6db4..6fc61803 100644 --- a/crates/engine/src/activity.rs +++ b/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, cancel: Option>, 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(m: &Mutex) -> 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> { @@ -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(); diff --git a/crates/engine/src/apple_photos.rs b/crates/engine/src/apple_photos.rs new file mode 100644 index 00000000..fd1c6f24 --- /dev/null +++ b/crates/engine/src/apple_photos.rs @@ -0,0 +1,1294 @@ +//! Add exported files to Apple Photos (macOS; issue #236). +//! +//! Photos is asked through its scripting interface, the way Apple documents for importing: a +//! constant AppleScript ([`SCRIPT`]) run by `/usr/bin/osascript` imports the files, into an album +//! (made when missing) if one is named, and returns the new media items' ids. Album names and file +//! paths are never written into the script: they reach it as separate arguments (`on run argv`), +//! so no name or path can change what the script does and nothing needs escaping. +//! +//! This sends Apple Events to Photos' import command; it does not synthesize keystrokes or clicks. +//! The first time, macOS asks the user whether LightCraft may control Photos (System Settings › +//! Privacy & Security › Automation). A packaged app needs the `com.apple.security.automation. +//! apple-events` entitlement and an `NSAppleEventsUsageDescription` for that (packaging/macos). +//! +//! An import can take minutes (and waits for the user to answer that prompt), so each one is a job +//! ([`ImportJob`], kept in [`Imports`]) that a front end may run on a worker and look at later +//! (`export.photosImports`). One runs at a time, and an export that will add its files reserves +//! the slot before it writes them ([`Reservation`]): a retry while one is reserved or running is +//! refused, not queued, so files are never sent twice by accident and an export is never turned +//! away after writing its files. +//! +//! The process runner is injectable ([`Runner`]): [`crate::Session::apple_photos`] holds the real one +//! on macOS (installed by [`crate::Session::with_fs`]) and nothing elsewhere; tests use fakes and +//! never start osascript. Not in the web build. + +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex, PoisonError}; +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. +/// +/// The album is looked for by its exact name among the albums at the top level of the library +/// (Photos' `album` elements are every album, folders included, in no set order): albums inside +/// folders are never used. None there: a new top-level album is made. More than one: nothing is +/// imported (error [`AMBIGUOUS_ALBUM`]). +pub const SCRIPT: &str = r#"on run argv + if (count of argv) < 3 then error "LightCraft: no files to import" number -50 + set albumName to item 2 of argv + set mediaFiles to {} + repeat with i from 3 to (count of argv) + set end of mediaFiles to (POSIX file (item i of argv)) as alias + end repeat + set importedItems to {} + tell application "Photos" + with timeout of 7200 seconds + if albumName is not "" then + set topLevel to {} + repeat with candidate in (every album whose name is albumName) + set candidateName to name of candidate + set candidateParent to missing value + try + set candidateParent to parent of candidate + on error errText number errNum + -- no parent folder: the album is at the top level + if {-1728, -2763} does not contain errNum then error errText number errNum + end try + considering case + set sameName to (candidateName is albumName) + end considering + if sameName and candidateParent is missing value then set end of topLevel to contents of candidate + end repeat + if (count of topLevel) > 1 then error "LightCraft: several top-level albums have this name" number 23601 + if (count of topLevel) is 1 then + set targetAlbum to item 1 of topLevel + else + set targetAlbum to make new album named albumName + end if + end if + try + if albumName is "" then + set importedItems to import mediaFiles skip check duplicates false + else + set importedItems to import mediaFiles into targetAlbum skip check duplicates false + end if + on error errText number errNum + -- -2763: Photos returned nothing (it imported nothing); anything else is a real error + if errNum is not -2763 then error errText number errNum + end try + end timeout + end tell + try + if importedItems is missing value then set importedItems to {} + on error + set importedItems to {} + end try + set mediaIds to {} + tell application "Photos" + repeat with mediaItem in importedItems + set end of mediaIds to (id of mediaItem) + end repeat + end tell + set AppleScript's text item delimiters to linefeed + set output to mediaIds as text + set AppleScript's text item delimiters to "" + return output +end run"#; + +/// The first script argument, always the same: osascript's option parsing stops at the first +/// argument that isn't an option, so an album name or path starting with `-` is never read as one. +pub const MARKER: &str = "lightcraft-photos-import"; + +/// The error number [`SCRIPT`] raises when more than one top-level album has the name asked for. +pub const AMBIGUOUS_ALBUM: i64 = 23601; + +/// Where macOS keeps Photos. +pub const PHOTOS_APP: &str = "/System/Applications/Photos.app"; + +/// How many finished imports a session remembers (`export.photosImports`). +pub const KEPT_JOBS: usize = 32; + +/// Why there is no Photos runner in this session. +pub fn unavailable() -> &'static str { + if cfg!(target_os = "macos") { + "Apple Photos isn't connected in this session (it is in the desktop app and lightcraft-cli)" + } else { + "Apple Photos is only available on macOS" + } +} + +/// What osascript gave back. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct Output { + /// The exit code (`None`: ended by a signal). + pub status: Option, + pub stdout: String, + pub stderr: String, +} + +/// Why osascript gave nothing back. +#[derive(Clone, Debug, PartialEq)] +pub enum RunError { + /// Photos isn't installed. + NoPhotos, + /// osascript couldn't be started. + Spawn(String), + /// Still running after this long: stopped. + Timeout(Duration), +} + +/// Runs osascript with these arguments, stopping it after the timeout. +pub type Runner = Arc Result + Send + Sync>; + +/// osascript's arguments: the script, one `-e` per line, then [`MARKER`], the album name and the +/// paths, each a whole argument of its own, unchanged. +pub fn osascript_args(album: &str, paths: &[String]) -> Vec { + let mut args = Vec::with_capacity(SCRIPT.lines().count() * 2 + 2 + paths.len()); + for line in SCRIPT.lines() { + args.push("-e".to_string()); + args.push(line.to_string()); + } + args.push(MARKER.to_string()); + args.push(album.to_string()); + args.extend(paths.iter().cloned()); + args +} + +/// The media item ids the script printed (one per line; blank lines ignored). +pub fn parse_ids(stdout: &str) -> Vec { + stdout.lines().map(str::trim).filter(|l| !l.is_empty()).map(str::to_string).collect() +} + +/// The AppleScript error number at the end of osascript's message (`… (-1743)`). +fn error_number(stderr: &str) -> Option { + let s = stderr.trim_end(); + let open = s.rfind('(')?; + s.get(open + 1..)?.strip_suffix(')')?.trim().parse().ok() +} + +/// What to tell the user when osascript failed importing into `album`. +pub fn explain(out: &Output, album: &str) -> String { + match error_number(&out.stderr) { + Some(-1743 | -1744) => "LightCraft isn't allowed to control Photos. Allow it in System Settings › Privacy & Security › Automation \ + (turn on Photos under LightCraft, or under your terminal app for lightcraft-cli), then try again." + .into(), + Some(-600 | -10810 | -10814) => "Apple Photos couldn't be found or opened on this Mac.".into(), + Some(-128) => "The import was cancelled in Photos.".into(), + Some(-1712) => "Photos didn't answer in time; it may still be importing. Check Photos before trying again.".into(), + Some(-43) => "Photos couldn't find a file to add (it was moved or deleted after the export).".into(), + Some(AMBIGUOUS_ALBUM) => format!( + "More than one album at the top level of Photos is named “{album}”, so nothing was added. Rename one of them in Photos, or use another album name." + ), + _ => { + let detail = out.stderr.lines().map(str::trim).find(|l| !l.is_empty()); + match (detail, out.status) { + (Some(d), _) => format!("Adding to Apple Photos failed: {d}"), + (None, Some(code)) => format!("Adding to Apple Photos failed (osascript exited with {code})."), + (None, None) => "Adding to Apple Photos failed (osascript was stopped).".into(), + } + } + } +} + +/// What to tell the user when osascript gave nothing back. +pub fn explain_run_error(e: &RunError) -> String { + match e { + RunError::NoPhotos => "Apple Photos couldn't be found on this Mac.".into(), + RunError::Spawn(why) => format!("Couldn't run /usr/bin/osascript to add the photos to Apple Photos: {why}"), + RunError::Timeout(t) => { + format!("Photos didn't finish adding the photos within {} s; it may still be importing. Check Photos before trying again.", t.as_secs()) + } + } +} + +/// How long one import may take: ten minutes, plus five seconds a file, at most two hours. The ten +/// minutes are for a person: the first import waits for the user to answer macOS's permission +/// prompt, and Photos may ask about duplicates; a first try with one minute timed out while the +/// prompt was still up. +pub fn timeout_for(files: usize) -> Duration { + Duration::from_secs(600u64.saturating_add(5u64.saturating_mul(files as u64)).min(7200)) +} + +/// What an import gave. +#[derive(Clone, Debug, PartialEq)] +pub struct Imported { + /// The new media items in Photos. + pub ids: Vec, + /// Files asked for. + pub requested: usize, + pub album: Option, +} + +impl Imported { + /// When Photos added fewer files than asked: why that can be. + pub fn warning(&self) -> Option { + (self.ids.len() < self.requested).then(|| { + format!("Photos added {} of {} files; the others may be duplicates it skipped or files it couldn't read.", self.ids.len(), self.requested) + }) + } + + pub fn to_json(&self) -> Value { + let mut v = json!({"imported": self.ids.len(), "requested": self.requested, "ids": self.ids, "album": self.album}); + if let Some(w) = self.warning() { + v["warning"] = json!(w); + } + v + } +} + +/// `paths` (absolute paths of existing files) and `album` checked for an import; the album name +/// trimmed. Nothing is started for a bad request. +pub fn check(paths: &[String], album: &str) -> Result { + if paths.is_empty() { + return Err("no files to add to Apple Photos".into()); + } + if let Some(p) = paths.iter().find(|p| !std::path::Path::new(p.as_str()).is_absolute()) { + return Err(format!("not an absolute path: {p}")); + } + if let Some(p) = paths.iter().find(|p| p.contains('\0')) { + return Err(format!("a path can't contain a NUL character: {}", p.escape_debug())); + } + if let Some(p) = paths.iter().find(|p| !std::path::Path::new(p.as_str()).is_file()) { + return Err(format!("no such file: {p}")); + } + let album = album.trim(); + if album.contains('\0') { + return Err("an album name can't contain a NUL character".into()); + } + Ok(album.to_string()) +} + +/// Files per osascript run. The paths are arguments, and macOS limits a process's arguments to +/// 1 MiB in all: 200 paths of the longest length macOS opens (1,024 bytes; [`check`] finds no +/// file at a longer one) stay below a quarter of that. +pub const BATCH: usize = 200; + +/// Run osascript for a checked request (`album` trimmed; see [`check`]): [`BATCH`] files at a +/// time, all within [`timeout_for`] the whole request. The first run makes the album when it is +/// missing; the others find it. +fn execute(runner: &Runner, paths: &[String], album: &str) -> Result { + let limit = timeout_for(paths.len()); + let deadline = std::time::Instant::now() + limit; + let mut ids = Vec::new(); + for (i, batch) in paths.chunks(BATCH).enumerate() { + let left = deadline.saturating_duration_since(std::time::Instant::now()); + let out = if left.is_zero() { Err(RunError::Timeout(limit)) } else { runner(&osascript_args(album, batch), left) }; + let failed = match out { + Ok(out) if out.status == Some(0) => { + ids.extend(parse_ids(&out.stdout)); + continue; + } + Ok(out) => explain(&out, album), + // the request's limit, not what was left of it for this batch + Err(RunError::Timeout(_)) => explain_run_error(&RunError::Timeout(limit)), + Err(e) => explain_run_error(&e), + }; + if i == 0 { + return Err(failed); + } + return Err(format!("{failed} (Photos had added {} of the {} files before that.)", ids.len(), paths.len())); + } + Ok(Imported { ids, requested: paths.len(), album: (!album.is_empty()).then(|| album.to_string()) }) +} + +/// Add `paths` (absolute paths of existing files) to Photos, into `album` when it isn't blank, +/// here and now. +pub fn import(runner: &Runner, paths: &[String], album: &str) -> Result { + let album = check(paths, album)?; + execute(runner, paths, &album) +} + +/// How an import ended. +#[derive(Clone, Debug, PartialEq)] +pub enum Outcome { + Imported(Imported), + /// Photos (or osascript) failed: why, for the user. + Failed(String), + /// Photos wasn't asked: the export that reserved the import was cancelled, wrote no files or + /// stopped early. + Skipped(String), +} + +/// One import: reserved by an export still writing its files, running, or finished. +#[derive(Debug)] +pub struct ImportJob { + pub id: u64, + pub album: Option, + /// Files asked for (0 while an export is still writing them). + requested: AtomicUsize, + /// Reserved by an export that hasn't handed over its files yet. + exporting: AtomicBool, + /// Whoever started it reports the outcome itself (an export's toast or result): the app + /// doesn't announce it again. + quiet: AtomicBool, + outcome: Mutex>, + announced: AtomicBool, + /// When Photos was asked (the import started): `None` while an export still writes its files. + started: Mutex>, +} + +impl ImportJob { + /// When Photos was asked; `None` while the export that reserved the job still writes. + pub fn started(&self) -> Option { + *self.started.lock().unwrap_or_else(PoisonError::into_inner) + } + + pub fn finished(&self) -> bool { + self.outcome.lock().unwrap_or_else(PoisonError::into_inner).is_some() + } + + /// Record the outcome, unless there already is one. + fn finish(&self, o: Outcome) { + let mut g = self.outcome.lock().unwrap_or_else(PoisonError::into_inner); + if g.is_none() { + *g = Some(o); + } + self.exporting.store(false, Ordering::Relaxed); + } + + pub fn outcome(&self) -> Option { + self.outcome.lock().unwrap_or_else(PoisonError::into_inner).clone() + } + + pub fn requested(&self) -> usize { + self.requested.load(Ordering::Relaxed) + } + + pub fn quiet(&self) -> bool { + self.quiet.load(Ordering::Relaxed) + } + + /// True once: when the job has finished and nobody has announced it yet (never for a quiet one). + pub fn take_announcement(&self) -> bool { + !self.quiet() && self.finished() && !self.announced.swap(true, Ordering::Relaxed) + } + + /// Why another import can't start now. + fn busy(&self) -> String { + if self.exporting.load(Ordering::Relaxed) { + format!( + "An export is about to add its files to Apple Photos (import {}). Wait for it to finish (export.photosImports), then try again.", + self.id + ) + } else { + format!( + "Apple Photos is still adding {} file(s) from an earlier request (import {}). Wait for it to finish (export.photosImports), then try again.", + self.requested(), + self.id + ) + } + } + + /// `{job, running, requested, album}` (`exporting: true` while an export is still writing its + /// files), and once finished `{imported, ids, warning?}`, `{error}` or `{skipped}`. + pub fn json(&self) -> Value { + let mut v = json!({"job": self.id, "running": true, "requested": self.requested(), "album": self.album}); + if self.exporting.load(Ordering::Relaxed) { + v["exporting"] = json!(true); + } + match self.outcome() { + None => {} + Some(Outcome::Imported(i)) => { + if let (Some(o), Some(done)) = (v.as_object_mut(), i.to_json().as_object()) { + o.extend(done.clone()); + } + v["running"] = json!(false); + } + Some(Outcome::Failed(e)) => { + v["running"] = json!(false); + v["error"] = json!(e); + } + Some(Outcome::Skipped(why)) => { + v["running"] = json!(false); + v["skipped"] = json!(why); + } + } + v + } +} + +/// A session's imports (the last [`KEPT_JOBS`], oldest first), shared with the threads that run +/// them. +#[derive(Clone, Debug, Default)] +pub struct Imports(Arc>)>>); + +impl Imports { + fn jobs(&self) -> std::sync::MutexGuard<'_, (u64, Vec>)> { + self.0.lock().unwrap_or_else(PoisonError::into_inner) + } + + pub fn all(&self) -> Vec> { + self.jobs().1.clone() + } + + pub fn get(&self, id: u64) -> Option> { + self.jobs().1.iter().find(|j| j.id == id).cloned() + } + + pub fn latest(&self) -> Option> { + self.jobs().1.last().cloned() + } + + /// The import that is reserved or running, if any (there is at most one). + pub fn running(&self) -> Option> { + self.jobs().1.iter().find(|j| !j.finished()).cloned() + } + + /// A new job, unless one is reserved or running: two would race in Photos, and a retry of one + /// that is still going would add its files twice. + fn begin(&self, requested: usize, album: &str, exporting: bool) -> Result, String> { + let mut g = self.jobs(); + if let Some(r) = g.1.iter().find(|j| !j.finished()) { + return Err(r.busy()); + } + g.0 = g.0.saturating_add(1); + let job = Arc::new(ImportJob { + id: g.0, + album: (!album.is_empty()).then(|| album.to_string()), + requested: AtomicUsize::new(requested), + exporting: AtomicBool::new(exporting), + quiet: AtomicBool::new(true), + outcome: Mutex::new(None), + announced: AtomicBool::new(false), + started: Mutex::new(None), + }); + g.1.push(job.clone()); + let excess = g.1.len().saturating_sub(KEPT_JOBS); + g.1.drain(..excess); + Ok(job) + } + + /// Reserve the import slot for an export that will add the files it writes: from now until + /// the export hands them over ([`Reservation::run`] / [`Reservation::start`]) or ends (the + /// reservation dropped), other imports are refused. An error while one is reserved or running. + pub fn reserve(&self, runner: Runner, album: &str) -> Result { + let album = album.trim(); + if album.contains('\0') { + 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), activity: None }) + } + + /// [`Imports::wait_idle_within`] with the [`WaitLimits::default`]. + pub fn wait_idle(&self) -> bool { + self.wait_idle_within(WaitLimits::default()) + } + + /// Wait until no import is reserved or running: a process about to end (lightcraft-cli, an + /// MCP server at EOF, a snapshot script) calls this so it never abandons osascript or an + /// import's result. The deadline follows the job as it goes: while an export still writes + /// the files it reserved the import for, `limits.writing` from the start of the wait; once + /// Photos was asked, the import's own time limit (`limits.import`, the one osascript runs + /// under) plus `limits.margin` from when it started; never longer than `limits.cap` in all. + /// True when nothing is left reserved or running. + pub fn wait_idle_within(&self, limits: WaitLimits) -> bool { + let t0 = std::time::Instant::now(); + loop { + let Some(job) = self.running() else { return true }; + let waited = t0.elapsed(); + if waited >= limits.cap { + return false; + } + let expired = match job.started() { + None => waited >= limits.writing, + Some(at) => at.elapsed() >= (limits.import)(job.requested()).saturating_add(limits.margin), + }; + if expired { + return false; + } + std::thread::sleep(Duration::from_millis(20)); + } + } +} + +/// How long [`Imports::wait_idle_within`] waits. +#[derive(Clone, Copy, Debug)] +pub struct WaitLimits { + /// An export still writing the files it will add: how much longer it may write. + pub writing: Duration, + /// A running import's time limit for this many files. + pub import: fn(usize) -> Duration, + /// Added to that limit. + pub margin: Duration, + /// The longest the whole wait may take. + pub cap: Duration, +} + +impl Default for WaitLimits { + /// An hour of writing (what `lightcraft-cli snapshot` gives a background export), then the + /// import's osascript limit ([`timeout_for`], at most two hours) and half a minute; three hours + /// and a minute in all. + fn default() -> Self { + Self { writing: Duration::from_secs(3600), import: timeout_for, margin: Duration::from_secs(30), cap: Duration::from_secs(3600 + 7200 + 60) } + } +} + +impl crate::Session { + /// Before a front end without a window ends (lightcraft-cli, the MCP server, a headless + /// snapshot), on every exit path: wait for an Apple Photos import still reserved or running + /// (`export.addToPhotos {wait: false}`, a background export still writing), bounded + /// ([`Imports::wait_idle`], [`WaitLimits::default`]), so the process never abandons osascript + /// or the import's result. True when it had to wait. + pub fn wait_for_photos_imports(&self) -> bool { + let Some(job) = self.apple_photos_imports.running() else { return false }; + log::warn!("Apple Photos: waiting for import {} to finish before exiting", job.id); + if !self.apple_photos_imports.wait_idle() { + log::warn!("Apple Photos: gave up waiting for import {}; Photos may still be importing", job.id); + } + true + } +} + +/// 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 { + 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, paths: Vec, album: String, background: bool, row: Option) { + 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 || { + 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}"))); + } +} + +/// The message for an import whose runner panicked. +const STOPPED: &str = "Adding to Apple Photos stopped unexpectedly."; + +/// Finishes its job as failed when dropped without an outcome: however the work ends (a panic +/// unwinding through it included), the job finishes and the import slot is freed. +struct Completion<'a>(&'a ImportJob); + +impl Drop for Completion<'_> { + fn drop(&mut self) { + self.0.finish(Outcome::Failed(STOPPED.into())); + } +} + +/// Run the import for `job` and record its outcome, on this thread. A panic in the runner is +/// that import's error, never a job that stays "running" (holding the slot) or a panic for the +/// caller. +fn run_to_completion(runner: &Runner, job: &ImportJob, paths: &[String], album: &str) { + let _done = Completion(job); + let out = crate::guard::catch("the Apple Photos import", || execute(runner, paths, album)).unwrap_or_else(|_| Err(STOPPED.into())); + job.finish(out.map_or_else(Outcome::Failed, Outcome::Imported)); +} + +/// 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), 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, + album: &str, + background: bool, + activity: Option<&Activity>, +) -> Result, String> { + let album = check(&paths, album)?; + let job = imports.begin(paths.len(), &album, false)?; + job.quiet.store(!background, Ordering::Relaxed); + let row = row(activity, &album); + launch(runner, &job, paths, album, background, row); + Ok(job) +} + +/// The import slot an export holds while it writes its files ([`Imports::reserve`]). Handed the +/// files with [`Reservation::run`] or [`Reservation::start`]; dropped unused (the export failed, +/// was refused or never started) it frees the slot, the job ending as skipped. +pub struct Reservation { + runner: Runner, + album: String, + job: Option>, + /// Where [`Reservation::start`]'s import shows while Photos works. + activity: Option, +} + +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> { + self.job.as_ref() + } + + /// The absolute paths of the files an export wrote (`files`, as `export_batch` reports them: + /// entries with a `path`; skipped and failed ones are left out), or why there is nothing to do. + fn take(&mut self, files: &[Value], cancelled: bool) -> Result<(Arc, Vec), Value> { + let Some(job) = self.job.take() else { return Err(json!({"error": "this import was already handed its files"})) }; + let paths: Vec = files.iter().filter_map(|f| f.get("path").and_then(Value::as_str)).map(absolute).collect(); + let skipped = if cancelled { + Some("the export was cancelled") + } else if paths.is_empty() { + Some("the export wrote no files") + } else { + None + }; + if let Some(why) = skipped { + job.finish(Outcome::Skipped(why.into())); + return Err(job.json()); + } + if let Err(e) = check(&paths, &self.album) { + job.finish(Outcome::Failed(e)); + return Err(job.json()); + } + Ok((job, paths)) + } + + /// Add the files an export wrote and wait for Photos (a worker thread, or a caller without a + /// window). The finished job's [`ImportJob::json`]; never an error for the export. + 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, None); + job.json() + } + Err(done) => done, + } + } + + /// Like [`Reservation::run`] without waiting: Photos works on a worker thread; the running + /// job is returned, and the app announces its outcome. + pub fn start(mut self, files: &[Value]) -> Value { + match self.take(files, false) { + Ok((job, paths)) => { + job.quiet.store(false, Ordering::Relaxed); + let row = row(self.activity.as_ref(), &self.album); + launch(&self.runner, &job, paths, self.album.clone(), true, row); + job.json() + } + Err(done) => done, + } + } +} + +impl Drop for Reservation { + fn drop(&mut self) { + if let Some(job) = self.job.take() { + job.finish(Outcome::Skipped("the export ended before writing its files".into())); + } + } +} + +/// `path` made absolute against the working directory (export folders may be given relative). +fn absolute(path: &str) -> String { + let p = std::path::Path::new(path); + if p.is_absolute() { + return path.to_string(); + } + std::env::current_dir().map(|d| d.join(p).to_string_lossy().to_string()).unwrap_or_else(|_| path.to_string()) +} + +/// The real runner: `/usr/bin/osascript`, stopped after the timeout. +#[cfg(target_os = "macos")] +pub fn osascript() -> Runner { + Arc::new(|args: &[String], timeout: Duration| { + use std::io::Read; + use std::process::{Command, Stdio}; + if !std::path::Path::new(PHOTOS_APP).exists() { + return Err(RunError::NoPhotos); + } + let mut child = Command::new("/usr/bin/osascript") + .args(args) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| RunError::Spawn(e.to_string()))?; + // read both pipes while it runs: a long list of ids must not fill a pipe and stall it + fn drain(r: Option) -> std::io::Result> { + std::thread::Builder::new().name("osascript-output".into()).spawn(move || { + let mut buf = Vec::new(); + if let Some(mut r) = r { + let _ = r.read_to_end(&mut buf); + } + String::from_utf8_lossy(&buf).into_owned() + }) + } + let pipes = drain(child.stdout.take()).and_then(|out| Ok((out, drain(child.stderr.take())?))); + let (stdout, stderr) = match pipes { + Ok(p) => p, + Err(e) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(RunError::Spawn(e.to_string())); + } + }; + let deadline = std::time::Instant::now().checked_add(timeout); + let status = loop { + match child.try_wait() { + Ok(Some(status)) => break status, + Ok(None) if deadline.is_some_and(|d| std::time::Instant::now() >= d) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(RunError::Timeout(timeout)); + } + Ok(None) => std::thread::sleep(Duration::from_millis(100)), + Err(e) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(RunError::Spawn(e.to_string())); + } + } + }; + Ok(Output { status: status.code(), stdout: stdout.join().unwrap_or_default(), stderr: stderr.join().unwrap_or_default() }) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A runner that records the arguments it got and answers `answer`. + fn fake(answer: Result) -> (Runner, Arc>>>) { + let calls = Arc::new(Mutex::new(Vec::new())); + let c = calls.clone(); + let r: Runner = Arc::new(move |args: &[String], _| { + c.lock().unwrap().push(args.to_vec()); + answer.clone() + }); + (r, calls) + } + + fn ok(stdout: &str) -> Result { + Ok(Output { status: Some(0), stdout: stdout.into(), stderr: String::new() }) + } + + fn failed(stderr: &str) -> Output { + Output { status: Some(1), stdout: String::new(), stderr: stderr.into() } + } + + /// The script part of an argument list: everything before the marker. + fn script_of(args: &[String]) -> &[String] { + let at = args.iter().position(|a| a == MARKER).unwrap(); + &args[..at] + } + + #[test] + fn names_and_paths_are_arguments_never_script() { + let hostile = [ + "", + "Holiday \"2026\"", + "back\\slash\\", + "line\nbreak\r\nand\ttab", + "end tell\ndo shell script \"rm -rf ~\"", + "-e", + "--help", + "日本の写真 📷 Ünïcödé", + "'single' & \"double\" » «", + ]; + let base = osascript_args("", &["/tmp/a.jpg".into()]); + for album in hostile { + let paths = vec!["/tmp/with \"quotes\".jpg".to_string(), "/tmp/new\nline/é\\x.jpg".to_string(), format!("/tmp/-{album}.jpg")]; + let args = osascript_args(album, &paths); + // the script is the same whatever the input… + assert_eq!(script_of(&args), script_of(&base)); + assert!(script_of(&args).chunks(2).all(|c| c[0] == "-e" && SCRIPT.lines().any(|l| l == c[1]))); + // …and the input follows the marker as whole arguments, unchanged + let at = args.iter().position(|a| a == MARKER).unwrap(); + assert_eq!(&args[at + 1], album); + assert_eq!(&args[at + 2..], &paths[..]); + } + // the script reads them from argv only + assert!(SCRIPT.starts_with("on run argv") && SCRIPT.contains("item 2 of argv") && SCRIPT.contains("item i of argv")); + assert!(!SCRIPT.contains("do shell script")); + // it imports into the album (made when missing) without skipping Photos' duplicate check + assert!(SCRIPT.contains("make new album named albumName")); + assert!(SCRIPT.contains("import mediaFiles into targetAlbum skip check duplicates false")); + } + + /// Photos' `album` elements are every album, those in folders too, in no set order: the script + /// keeps the top-level ones of exactly that name and refuses when there are several. + #[test] + fn the_album_is_a_single_top_level_one() { + let lookup = SCRIPT.find("every album whose name is albumName").unwrap(); + let choose = SCRIPT.find("set targetAlbum to item 1 of topLevel").unwrap(); + let make = SCRIPT.find("make new album named albumName").unwrap(); + let import = SCRIPT.find("import mediaFiles into targetAlbum").unwrap(); + assert!(lookup < choose && choose < make && make < import); + let filter = &SCRIPT[lookup..choose]; + assert!(filter.contains("parent of candidate") && filter.contains("candidateParent is missing value"), "{filter}"); + assert!(filter.contains("considering case"), "exact names: {filter}"); + assert!(filter.contains(&format!( + "if (count of topLevel) > 1 then error \"LightCraft: several top-level albums have this name\" number {AMBIGUOUS_ALBUM}" + ))); + let m = explain(&failed(&format!("execution error: LightCraft: several top-level albums have this name ({AMBIGUOUS_ALBUM})")), "Trip"); + assert!(m.contains("More than one album at the top level of Photos is named “Trip”") && m.contains("nothing was added"), "{m}"); + } + + #[test] + fn ids_are_one_per_line() { + assert_eq!(parse_ids("A1B2/L0/001\nC3D4/L0/001\n"), vec!["A1B2/L0/001", "C3D4/L0/001"]); + assert_eq!(parse_ids(" X/L0/001 \r\n\r\n Y/L0/001"), vec!["X/L0/001", "Y/L0/001"]); + assert!(parse_ids("").is_empty() && parse_ids("\n \n").is_empty()); + } + + #[test] + fn errors_say_what_to_do() { + let says = |stderr: &str, part: &str| { + let m = explain(&failed(stderr), "Trip"); + assert!(m.contains(part), "{stderr:?}: {m}"); + }; + says("146:180: execution error: Not authorized to send Apple events to Photos. (-1743)", "System Settings › Privacy & Security › Automation"); + says("execution error: Photos got an error: … (-1744)", "Automation"); + says("execution error: Can’t get application \"Photos\". (-10814)", "couldn't be found"); + says("execution error: Photos got an error: Application isn’t running. (-600)", "couldn't be found or opened"); + says("execution error: User canceled. (-128)", "cancelled"); + says("execution error: Photos got an error: AppleEvent timed out. (-1712)", "didn't answer in time"); + says("execution error: File not found (-43)", "couldn't find a file"); + says("execution error: Photos got an error: Something odd happened. (-2700)", "Something odd happened"); + assert!(explain(&Output { status: Some(3), ..Default::default() }, "").contains("exited with 3")); + assert!(explain(&Output { status: None, ..Default::default() }, "").contains("stopped")); + assert!(explain_run_error(&RunError::NoPhotos).contains("couldn't be found")); + assert!(explain_run_error(&RunError::Spawn("denied".into())).contains("denied")); + assert!(explain_run_error(&RunError::Timeout(Duration::from_secs(65))).contains("65 s")); + // a stray parenthesis is not an error number + assert!(explain(&failed("execution error: (not a number)"), "").contains("not a number")); + } + + fn files(tag: &str, n: usize) -> (std::path::PathBuf, Vec) { + let dir = std::env::temp_dir().join(format!("lc-photos-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let paths = (0..n) + .map(|i| { + let p = dir.join(format!("photo {i} é.jpg")); + std::fs::write(&p, b"jpeg").unwrap(); + p.to_string_lossy().to_string() + }) + .collect(); + (dir, paths) + } + + #[test] + fn an_import_passes_the_files_and_returns_the_ids() { + let (dir, paths) = files("import", 2); + let (runner, calls) = fake(ok("ID-1/L0/001\nID-2/L0/001\n")); + let got = import(&runner, &paths, " Trip “2026” ").unwrap(); + assert_eq!(got, Imported { ids: vec!["ID-1/L0/001".into(), "ID-2/L0/001".into()], requested: 2, album: Some("Trip “2026”".into()) }); + assert_eq!(got.warning(), None); + let args = calls.lock().unwrap()[0].clone(); + let at = args.iter().position(|a| a == MARKER).unwrap(); + assert_eq!(&args[at + 1..], &[vec!["Trip “2026”".to_string()], paths.clone()].concat()[..]); + // fewer back than asked: a partial import, said so + let (runner, _) = fake(ok("ID-1/L0/001\n")); + let got = import(&runner, &paths, "").unwrap(); + assert_eq!((got.ids.len(), got.album.clone()), (1, None)); + assert!(got.warning().unwrap().contains("1 of 2")); + assert_eq!(got.to_json()["warning"], json!(got.warning().unwrap())); + // a failed run is the mapped message + let (runner, _) = fake(Ok(failed("execution error: Not authorized to send Apple events to Photos. (-1743)"))); + assert!(import(&runner, &paths, "x").unwrap_err().contains("Automation")); + // a timeout names the request's limit + let (runner, _) = fake(Err(RunError::Timeout(Duration::from_secs(70)))); + assert!(import(&runner, &paths, "x").unwrap_err().contains(&format!("{} s", timeout_for(2).as_secs()))); + let _ = std::fs::remove_dir_all(dir); + } + + /// Codex review of #643: every path was an argument of one osascript run, so a big export + /// (10,000 files) went over macOS's 1 MiB argument limit and Photos got nothing. + #[test] + fn big_imports_run_in_batches_within_the_argument_limit() { + let long = format!("/{}", "d/".repeat(500)); // about 1,000 bytes, near the longest path macOS opens + let paths: Vec = (0..10_000).map(|i| format!("{long}{i:05}.jpg")).collect(); + let runs = Arc::new(Mutex::new(Vec::<(usize, usize, Duration)>::new())); + let r = runs.clone(); + let runner: Runner = Arc::new(move |args: &[String], limit| { + let files = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + r.lock().unwrap().push((files, args.iter().map(|a| a.len() + 1).sum(), limit)); + Ok(Output { status: Some(0), stdout: (0..files).map(|i| format!("ID-{i}\n")).collect(), stderr: String::new() }) + }); + let got = execute(&runner, &paths, "Big").unwrap(); + assert_eq!((got.ids.len(), got.requested, got.warning()), (10_000, 10_000, None)); + let runs = runs.lock().unwrap().clone(); + assert_eq!(runs.len(), 10_000 / BATCH); + assert!(runs.iter().all(|&(files, _, _)| files == BATCH)); + let biggest = runs.iter().map(|&(_, bytes, _)| bytes).max().unwrap(); + assert!(biggest < 1 << 18, "{biggest} bytes of arguments in one run"); + // every run gets what is left of the request's limit, never more + assert!(runs.iter().all(|&(_, _, limit)| limit <= timeout_for(10_000))); + + // a later batch failing: the error says how far Photos got + let calls = Arc::new(Mutex::new(0)); + let c = calls.clone(); + let runner: Runner = Arc::new(move |args: &[String], _| { + let mut n = c.lock().unwrap(); + *n += 1; + let files = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + if *n == 2 { + return Ok(failed("execution error: Photos got an error: User canceled. (-128)")); + } + Ok(Output { status: Some(0), stdout: (0..files).map(|i| format!("ID-{i}\n")).collect(), stderr: String::new() }) + }); + let e = execute(&runner, &paths[..450], "Big").unwrap_err(); + assert!(e.contains("cancelled") && e.contains("200 of the 450 files"), "{e}"); + assert_eq!(*calls.lock().unwrap(), 2, "nothing more is sent after a failed batch"); + + // the batches share the request's one deadline: time a batch takes comes off the next one's + let limits = Arc::new(Mutex::new(Vec::::new())); + let l = limits.clone(); + let runner: Runner = Arc::new(move |args: &[String], limit| { + l.lock().unwrap().push(limit); + std::thread::sleep(Duration::from_millis(300)); + let files = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + Ok(Output { status: Some(0), stdout: (0..files).map(|i| format!("ID-{i}\n")).collect(), stderr: String::new() }) + }); + execute(&runner, &paths[..450], "Big").unwrap(); + let limits = limits.lock().unwrap().clone(); + assert_eq!(limits.len(), 3); + let whole = timeout_for(450); + assert!(limits[0] <= whole && limits[0] > whole - Duration::from_millis(250), "{limits:?}"); + assert!(limits[1] <= whole - Duration::from_millis(300) && limits[2] <= whole - Duration::from_millis(600), "{limits:?}"); + } + + #[test] + fn bad_requests_never_reach_osascript() { + let (dir, paths) = files("bad", 1); + let (runner, calls) = fake(ok("")); + assert!(import(&runner, &[], "").unwrap_err().contains("no files")); + assert!(import(&runner, &["relative/a.jpg".into()], "").unwrap_err().contains("absolute")); + assert!(import(&runner, &[format!("{}/missing.jpg", dir.display())], "").unwrap_err().contains("no such file")); + assert!(import(&runner, &[format!("{}/a\0b.jpg", dir.display())], "").unwrap_err().contains("NUL")); + assert!(import(&runner, &paths, "a\0b").unwrap_err().contains("NUL")); + assert!(calls.lock().unwrap().is_empty()); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn after_an_export_the_written_files_are_added() { + let (dir, paths) = files("after", 2); + let exported = vec![ + json!({"path": paths[0], "width": 10}), + json!({"skipped": "/elsewhere/x.jpg"}), + json!({"photo": 3, "error": "x"}), + json!({"path": paths[1]}), + ]; + let (runner, calls) = fake(ok("A/L0/001\nB/L0/001")); + let imports = Imports::default(); + let out = imports.reserve(runner.clone(), " Album ").unwrap().run(&exported, false); + assert_eq!(out["imported"], 2); + assert_eq!(out["album"], "Album"); + assert_eq!((out["job"].as_u64(), out["running"].as_bool()), (Some(1), Some(false))); + let args = calls.lock().unwrap()[0].clone(); + assert_eq!(&args[args.len() - 2..], &paths[..], "only the files written, in order"); + // nothing written, or cancelled: Photos isn't asked, and the slot is free again + let out = imports.reserve(runner.clone(), "").unwrap().run(&[json!({"skipped": "x"})], false); + assert!(out["skipped"].as_str().unwrap().contains("wrote no files"), "{out}"); + let out = imports.reserve(runner.clone(), "").unwrap().run(&exported, true); + assert!(out["skipped"].as_str().unwrap().contains("cancelled"), "{out}"); + assert_eq!(calls.lock().unwrap().len(), 1); + assert!(imports.running().is_none()); + let _ = std::fs::remove_dir_all(dir); + } + + /// An export that will add its files reserves the slot before writing them: a competing + /// import is refused, naming the export's job, and the export is never turned away after + /// writing. Every way the export can end frees the slot. + #[test] + fn an_export_reserves_the_import_before_writing() { + let (dir, paths) = files("reserve", 1); + let (runner, calls) = fake(ok("A\n")); + let imports = Imports::default(); + 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, 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()); + // the export hands over its files: they are imported under its job + let out = reserved.run(&[json!({"path": paths[0]})], false); + assert_eq!((out["job"].as_u64(), out["imported"].as_u64(), out.get("exporting")), (Some(1), Some(1), None), "{out}"); + // an export that fails or never runs (its reservation dropped) frees the slot + let dropped = imports.reserve(runner.clone(), "").unwrap(); + let id = dropped.job().unwrap().id; + drop(dropped); + let j = imports.get(id).unwrap(); + assert!(j.finished() && !j.take_announcement(), "skipped quietly"); + assert!(j.json()["skipped"].as_str().unwrap().contains("ended before writing")); + // a file the export reported but that is gone: Photos isn't asked, the slot is freed + let out = imports.reserve(runner.clone(), "").unwrap().run(&[json!({"path": format!("{}/gone.jpg", dir.display())})], false); + 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, None).unwrap().id, 4); + let _ = std::fs::remove_dir_all(dir); + } + + /// Review of #236: a runner that panics, here or on a worker, ends its import as failed and + /// frees the slot; it never stays "running", refusing every later import. + #[test] + fn a_panicking_runner_frees_the_slot() { + let (dir, paths) = files("panic", 1); + let panicky: Runner = Arc::new(|_: &[String], _| panic!("runner exploded")); + let (good, _) = fake(ok("A\n")); + 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, 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, None).unwrap(); + let t0 = std::time::Instant::now(); + while !job.finished() { + assert!(t0.elapsed() < Duration::from_secs(20)); + std::thread::sleep(Duration::from_millis(5)); + } + assert!(stopped(&job), "{}", job.json()); + // an export's reservation, waiting and not + let out = imports.reserve(panicky.clone(), "").unwrap().run(&[json!({"path": paths[0]})], false); + assert!(out["error"].as_str().unwrap().contains("stopped unexpectedly"), "{out}"); + assert!(imports.running().is_none()); + let started = imports.reserve(panicky, "").unwrap().start(&[json!({"path": paths[0]})]); + let job = imports.get(started["job"].as_u64().unwrap()).unwrap(); + while !job.finished() { + assert!(t0.elapsed() < Duration::from_secs(20)); + std::thread::sleep(Duration::from_millis(5)); + } + assert!(stopped(&job)); + // the slot is free: the next import runs + assert_eq!(import_job(&good, &imports, paths, "", false, None).unwrap().json()["imported"], 1); + let _ = std::fs::remove_dir_all(dir); + } + + /// A process about to end waits for an import still running, so it never abandons osascript. + #[test] + fn waiting_for_idle_outlasts_a_running_import() { + let (dir, paths) = files("idle", 1); + let (runner, go) = delayed(); + let imports = Imports::default(); + 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)); + go.send(()).unwrap(); + }); + imports.wait_idle(); + assert!(job.finished() && imports.running().is_none()); + release.join().unwrap(); + // nothing running: returns at once + imports.wait_idle(); + let _ = std::fs::remove_dir_all(dir); + } + + /// Review of #236: shutdown while an export is still writing the files it reserved the import + /// for. The deadline follows the job: the write allowance while the export writes, then the + /// import's own time limit from when Photos was asked, not one deadline fixed at the start + /// (which gave up with the import still inside its limit). Scaled down: 300 ms of writing, + /// imports limited to 1.5 s (+ 0.1 s). + #[test] + fn shutdown_follows_an_export_into_its_import() { + let (dir, paths) = files("shutdown", 2); + let limits = WaitLimits { + writing: Duration::from_millis(300), + import: |_| Duration::from_millis(1500), + margin: Duration::from_millis(100), + cap: Duration::from_secs(20), + }; + let taking = |ms: u64| -> Runner { + Arc::new(move |args: &[String], _| { + std::thread::sleep(Duration::from_millis(ms)); + let n = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + Ok(Output { status: Some(0), stdout: "ID\n".repeat(n), stderr: String::new() }) + }) + }; + let imports = Imports::default(); + // the export is still writing when the wait begins; Photos then takes 1 s, past the write + // allowance but inside the import's own limit + let reserved = imports.reserve(taking(1000), "").unwrap(); + let written: Vec = paths.iter().map(|p| json!({"path": p})).collect(); + let exporter = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(150)); + reserved.run(&written, false) + }); + let t0 = std::time::Instant::now(); + assert!(imports.wait_idle_within(limits), "waited for the import, not just the write allowance"); + 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, None).unwrap(); + let t0 = std::time::Instant::now(); + assert!(!imports.wait_idle_within(limits)); + let waited = t0.elapsed(); + assert!(waited >= Duration::from_millis(1500) && waited < Duration::from_millis(2900), "{waited:?}"); + while !job.finished() { + std::thread::sleep(Duration::from_millis(20)); + } + // an export that never hands its files over: the write allowance, then it's left + let held = imports.reserve(taking(0), "").unwrap(); + let t0 = std::time::Instant::now(); + assert!(!imports.wait_idle_within(limits)); + let waited = t0.elapsed(); + assert!(waited >= Duration::from_millis(300) && waited < Duration::from_millis(1500), "{waited:?}"); + drop(held); + assert!(imports.wait_idle_within(limits)); + // and the cap holds over everything + 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)); + while !job.finished() { + std::thread::sleep(Duration::from_millis(20)); + } + let _ = std::fs::remove_dir_all(dir); + } + + /// A runner that waits for `go` (at most 20 s, so a regression fails instead of hanging), then + /// answers one id per file. + fn delayed() -> (Runner, std::sync::mpsc::Sender<()>) { + let (go, wait) = std::sync::mpsc::channel::<()>(); + let wait = Mutex::new(wait); + let r: Runner = Arc::new(move |args: &[String], _| { + let _ = wait.lock().unwrap().recv_timeout(Duration::from_secs(20)); + let files = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + Ok(Output { status: Some(0), stdout: (0..files).map(|i| format!("ID-{i}\n")).collect(), stderr: String::new() }) + }); + (r, go) + } + + /// Imports run as jobs: in the background the caller gets the job at once and its outcome + /// later; while one runs a second is refused (a retry must not add the files twice). + #[test] + fn a_slow_import_runs_as_a_job_and_refuses_a_second_one() { + let (dir, paths) = files("job", 2); + let (runner, go) = delayed(); + let imports = Imports::default(); + let t0 = std::time::Instant::now(); + 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, 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}"); + go.send(()).unwrap(); + while !job.finished() { + assert!(t0.elapsed() < Duration::from_secs(20), "the job finishes"); + std::thread::sleep(Duration::from_millis(5)); + } + assert!(job.take_announcement() && !job.take_announcement(), "announced once"); + let done = imports.get(1).unwrap().json(); + 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, 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)); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn a_session_remembers_its_last_imports() { + let (dir, paths) = files("kept", 1); + let (runner, _) = fake(ok("A\n")); + let imports = Imports::default(); + for _ in 0..KEPT_JOBS + 3 { + import_job(&runner, &imports, paths.clone(), "", false, None).unwrap(); + } + let all = imports.all(); + assert_eq!(all.len(), KEPT_JOBS); + assert_eq!((all[0].id, all[KEPT_JOBS - 1].id), (4, KEPT_JOBS as u64 + 3)); + assert!(imports.get(1).is_none()); + 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 = 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)); + assert_eq!(timeout_for(100), Duration::from_secs(1100)); + assert_eq!(timeout_for(usize::MAX), Duration::from_secs(7200)); + } +} diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index e08d0d17..e93d0417 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -78,8 +78,76 @@ fn delete(s: &mut Session, p: &Value) -> Result { Ok(list(s)) } +/// `export.addToPhotos`: there is a runner (the desktop app and lightcraft-cli on macOS). +#[cfg(not(target_arch = "wasm32"))] +fn photos_enabled(s: &Session) -> std::result::Result<(), String> { + match &s.apple_photos { + Some(_) => Ok(()), + None => Err(crate::apple_photos::unavailable().into()), + } +} + +/// `export.addToPhotos {paths, album?, wait?}`: add files already on disk to Apple Photos, as a +/// job: here (the default without a window: lightcraft-cli, the MCP server, scripts), or on a +/// worker thread (the default in the desktop app, which answers control requests on its UI +/// thread and must not wait minutes for Photos; see [`Session::apple_photos_background`]). +#[cfg(not(target_arch = "wasm32"))] +fn add_to_photos(s: &mut Session, p: &Value) -> Result { + const ID: &str = "export.addToPhotos"; + let runner = s.apple_photos.clone().ok_or_else(|| bad(ID, crate::apple_photos::unavailable()))?; + if let Some(k) = p.as_object().and_then(|o| o.keys().find(|k| !matches!(k.as_str(), "paths" | "album" | "wait"))) { + return Err(bad(ID, format!("unknown parameter `{k}` (one of paths, album, wait)"))); + } + let paths: Vec = match p.get("paths") { + Some(Value::Array(a)) => a + .iter() + .map(|v| v.as_str().map(str::to_string)) + .collect::>() + .ok_or_else(|| bad(ID, "`paths` must be an array of file paths"))?, + _ => return Err(bad(ID, "missing `paths` (an array of absolute file paths)")), + }; + let album = match p.get("album") { + None | Some(Value::Null) => "", + Some(Value::String(a)) => a.as_str(), + Some(_) => return Err(bad(ID, "`album` must be a string")), + }; + // a window (UI thread) doesn't wait by default; a one-shot process must, or it would end + // 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, 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()), + } +} + +/// `export.photosImports {job?}`: this session's Apple Photos imports, or one of them. +#[cfg(not(target_arch = "wasm32"))] +fn photos_imports(s: &mut Session, p: &Value) -> Result { + const ID: &str = "export.photosImports"; + match p.get("job") { + None | Some(Value::Null) => Ok(json!({"imports": s.apple_photos_imports.all().iter().map(|j| j.json()).collect::>()})), + Some(v) => { + let id = v.as_u64().ok_or_else(|| bad(ID, "`job` must be an import number"))?; + s.apple_photos_imports.get(id).map(|j| j.json()).ok_or_else(|| bad(ID, format!("no import {id} (see export.photosImports)"))) + } + } +} + pub fn specs() -> Vec { - vec![ + #[allow(unused_mut)] + let mut v = vec![ cmd!(query "export.contactSheet", "Export Contact Sheet PDF", [], None, "{path, ids?: photo ids (default selection), paper?: a4|letter, landscape?: bool, columns?: 1..8, rows?: 1..10, captions?: bool} → {path, pages, photos, bytes}; replaces an existing output, never an original", always, |s, p| { @@ -123,5 +191,28 @@ pub fn specs() -> Vec { Ok(json!({"path": path})) } ), - ] + ]; + // Apple Events to Photos' import command (macOS); not in the web build. Not journaled: a + // replay must not import again. + #[cfg(not(target_arch = "wasm32"))] + v.push(cmd!( + query "export.addToPhotos", + "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, 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 + )); + #[cfg(not(target_arch = "wasm32"))] + v.push(cmd!( + query "export.photosImports", + "Apple Photos Imports", + [], + None, + "{job?} → {imports: [{job, running, exporting?, requested, album, imported?, ids?, warning?, error?, skipped?}]} (this session's, oldest first), or the one job: how Add to Apple Photos is getting on (exporting: reserved by an export still writing its files)", + always, + photos_imports + )); + v } diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index 0d02a4b0..f356d6e4 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -677,6 +677,10 @@ pub struct ExportOptions { pub jxl_effort: JxlEffort, /// JPEG XL: progressive file (a coarse image first, refined as it loads). pub jxl_progressive: bool, + /// After the export, add the files written to Apple Photos (macOS only; see [`AfterExport`]). + pub add_to_photos: bool, + /// The Photos album they go into, made when missing (empty = none). + pub photos_album: String, } impl Default for ExportOptions { @@ -704,6 +708,8 @@ impl Default for ExportOptions { jxl_lossless: false, jxl_effort: JxlEffort::Normal, jxl_progressive: false, + add_to_photos: false, + photos_album: String::new(), } } } @@ -743,8 +749,16 @@ pub const OPTION_PARAMS: &[&str] = &[ "jxlLossless", "jxlEffort", "jxlProgressive", + "addToPhotos", + "photosAlbum", ]; +/// Why an export can't add its files to Apple Photos on this platform (`None`: it can, when the +/// session has the runner; see [`crate::Session::after_export`]). +pub fn apple_photos_unsupported() -> Option<&'static str> { + (!cfg!(target_os = "macos")).then_some("Apple Photos is only available on macOS") +} + /// The keys of a `watermark` object ([`Watermark`], camelCase). `inset` sets `insetX` and `insetY` /// together (what versions before the two margins wrote); either of those, when given, wins. pub const WATERMARK_PARAMS: &[&str] = @@ -892,6 +906,18 @@ impl ExportOptions { "dontEnlarge" | "removeLocation" | "background" | "hdr" | "jxlLossless" | "jxlProgressive" => { boolean(k, v)?; } + "addToPhotos" => { + if boolean(k, v)? + && let Some(why) = apple_photos_unsupported() + { + return Err(bad(format!("`{k}`: {why}"))); + } + } + "photosAlbum" => { + if string(cmd, k, v)?.contains('\0') { + return Err(bad(format!("`{k}` can't contain a NUL character"))); + } + } "naming" | "subfolder" | "path" | "dir" | "preset" => { string(cmd, k, v)?; } @@ -983,7 +1009,8 @@ impl ExportOptions { } /// Read options from command params (`format`, `quality`, `limitKb`, `sharpen`, `sharpenAmount`, - /// `naming`, `metadata`, `removeLocation`, `watermark`, `colorSpace`, `bitDepth`, `ppi`) and the + /// `naming`, `metadata`, `removeLocation`, `watermark`, `colorSpace`, `bitDepth`, `ppi`, + /// `addToPhotos`, `photosAlbum`) and the /// size: one of `longEdge`, `shortEdge`, `width`, `height` (both = fit inside W × H), /// `megapixels`, `percent` (absent or 0 = full size), or a [`Resize`] object as `resize`; plus /// `dontEnlarge` (default true). @@ -1032,6 +1059,8 @@ impl ExportOptions { jxl_lossless: p.get("jxlLossless").and_then(Value::as_bool).unwrap_or(d.jxl_lossless), jxl_effort: s("jxlEffort").and_then(JxlEffort::parse).unwrap_or(d.jxl_effort), jxl_progressive: p.get("jxlProgressive").and_then(Value::as_bool).unwrap_or(d.jxl_progressive), + add_to_photos: p.get("addToPhotos").and_then(Value::as_bool).unwrap_or(d.add_to_photos), + photos_album: s("photosAlbum").map_or(d.photos_album, |a| a.trim().to_string()), } } @@ -1713,6 +1742,104 @@ pub struct Destination { pub exact: Option, } +/// What happens after an export has written its files (Lightroom's Post-Processing): nothing, or +/// adding them to Apple Photos ([`ExportOptions::add_to_photos`]). Made by +/// [`crate::Session::after_export`] before the export starts, so an export asking for something +/// this session can't do fails before writing anything; for Photos it holds the import slot +/// (a [`crate::apple_photos::Reservation`]) until the files are handed over, and frees it when +/// dropped unused (the export failed or never ran). `Send`: a background export takes it to its +/// worker. +#[derive(Default)] +pub struct AfterExport { + #[cfg(not(target_arch = "wasm32"))] + apple_photos: Option, +} + +impl AfterExport { + /// Add the files to Apple Photos with `runner`, into `album` when it isn't blank, reserving + /// the import slot of `imports` now. (Front ends get this from + /// [`crate::Session::after_export`], which checks the platform first.) + #[cfg(not(target_arch = "wasm32"))] + pub fn apple_photos(runner: crate::apple_photos::Runner, album: &str, imports: &crate::apple_photos::Imports) -> Result { + Ok(Self { apple_photos: Some(imports.reserve(runner, album)?) }) + } + + /// Nothing to do. + #[cfg(not(target_arch = "wasm32"))] + pub fn is_none(&self) -> bool { + self.apple_photos.is_none() + } + + /// Nothing to do. + #[cfg(target_arch = "wasm32")] + pub fn is_none(&self) -> bool { + true + } + + /// Do it now for an export's per-photo results ([`run_batch`]'s: only the files written + /// count), waiting for Photos: for a worker thread (the app's background export) or a caller + /// without a window (MCP, lightcraft-cli). `None` when there was nothing to do, else the + /// outcome to report as the export's `applePhotos`: the finished job (`{job, running: false, + /// requested, album}` with `{imported, ids, warning?}`, `{error}`, or `{skipped}` when the + /// export was cancelled or wrote nothing). Never fails the export: its files are written + /// either way. + pub fn run(self, files: &[serde_json::Value], cancelled: bool) -> Option { + #[cfg(not(target_arch = "wasm32"))] + if let Some(r) = self.apple_photos { + return Some(r.run(files, cancelled)); + } + let _ = (files, cancelled); + None + } + + /// Like [`AfterExport::run`], without waiting: Photos works on a worker thread and the + /// outcome is the running job (`{job, running: true, requested, album}`; its result comes + /// from `export.photosImports`, and the desktop app announces it). For a caller on the UI + /// thread, which must not wait minutes for Photos. + pub fn start(self, files: &[serde_json::Value]) -> Option { + #[cfg(not(target_arch = "wasm32"))] + if let Some(r) = self.apple_photos { + return Some(r.start(files)); + } + let _ = files; + None + } + + /// [`AfterExport::start`] where the session answers on a UI thread + /// ([`crate::Session::apple_photos_background`]), else [`AfterExport::run`]. + pub fn finish(self, s: &crate::Session, files: &[serde_json::Value]) -> Option { + #[cfg(not(target_arch = "wasm32"))] + if s.apple_photos_background { + return self.start(files); + } + let _ = s; + self.run(files, false) + } +} + +impl crate::Session { + /// What an export with `o` does after writing its files; an error, before anything is written, + /// when it asks for Apple Photos and this platform or session has no way to reach it, or + /// another import is reserved or running. For Photos the import slot is reserved from now on. + pub fn after_export(&self, o: &ExportOptions) -> Result { + if !o.add_to_photos { + return Ok(AfterExport::default()); + } + if let Some(why) = apple_photos_unsupported() { + return Err(why.into()); + } + #[cfg(not(target_arch = "wasm32"))] + { + let runner = self.apple_photos.clone().ok_or_else(|| crate::apple_photos::unavailable().to_string())?; + 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()) + } +} + /// Export `ids` in order ([`prepare_batch`] + [`run_batch`]), stopping at the first error. pub fn export_batch( session: &mut crate::Session, @@ -2860,6 +2987,22 @@ mod tests { j[k] = v; } assert_eq!(ExportOptions::from_params(&j).unwrap(), full); + // Add to Apple Photos (issue #236): a switch and an album name; on only where Photos is + assert!(err(json!({"addToPhotos": "yes"})).contains("`addToPhotos` must be true or false")); + assert!(err(json!({"photosAlbum": 3})).contains("`photosAlbum` must be a string")); + assert!(err(json!({"photosAlbum": "a\0b"})).contains("NUL")); + assert!(err(json!({"photosAlbm": "x"})).contains("did you mean `photosAlbum`")); + let album = ExportOptions { photos_album: "Trip “2026” \\ \"x\"".into(), ..Default::default() }; + assert_eq!(ExportOptions::from_params(&album.to_json()).unwrap(), album); + let on = ExportOptions { add_to_photos: true, ..album }; + if cfg!(target_os = "macos") { + assert_eq!(ExportOptions::from_params(&on.to_json()).unwrap(), on); + } else { + assert!(err(on.to_json()).contains("Apple Photos is only available on macOS")); + // stored settings (a preset or Export with Previous made on a Mac) still read + assert!(ExportOptions::from_json(&on.to_json()).add_to_photos); + } + assert_eq!(ExportOptions::from_json(&json!({"photosAlbum": " Trip "})).photos_album, "Trip"); assert_eq!(ExportOptions::from_params(&ExportOptions::default().to_json()).unwrap(), ExportOptions::default()); // null is "not given", as a dropped key is ExportOptions::from_params(&json!({"quality": 92, "longEdge": null, "watermark": null})).unwrap(); diff --git a/crates/engine/src/files.rs b/crates/engine/src/files.rs index 8ff84ef5..c48aaff7 100644 --- a/crates/engine/src/files.rs +++ b/crates/engine/src/files.rs @@ -862,13 +862,18 @@ pub fn fs_hooks() -> (FileLoader, FileProbe) { } impl crate::Session { - /// Install the filesystem file hooks (desktop, CLI, MCP headless). + /// Install the filesystem file hooks (desktop, CLI, MCP headless), and on macOS the Apple Photos + /// runner ([`crate::apple_photos::osascript`]; installed, not run). pub fn with_fs(mut self) -> Self { let (l, p) = fs_hooks(); self.media.file_loader = Some(l); self.media.file_probe = Some(p); self.media.preview_loader = Some(fs_preview_loader()); self.media.denoise.loader = Some(fs_pair_loader()); + #[cfg(target_os = "macos")] + { + self.apple_photos = Some(crate::apple_photos::osascript()); + } self } } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 32761a59..3bc6c431 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -15,6 +15,8 @@ extern crate lightcraft_denoise_core as lightcraft_denoise; pub mod activity; +#[cfg(not(target_arch = "wasm32"))] +pub mod apple_photos; pub mod availability; pub(crate) mod camera_preview; pub mod camera_profiles; @@ -293,6 +295,20 @@ pub struct Session { /// Untouched Local records of folders not browsed for this many days are forgotten when the /// library opens (0 = never; persisted in prefs.json). See `cmd/browse.rs`. pub forget_local_days: u32, + /// Adds exported files to Apple Photos ([`apple_photos`]): `/usr/bin/osascript`, installed by + /// [`Session::with_fs`] on macOS; `None` elsewhere and in sessions without a file system. Tests + /// put a fake here. + #[cfg(not(target_arch = "wasm32"))] + pub apple_photos: Option, + /// This session's Apple Photos imports, running or finished (`export.photosImports`). + #[cfg(not(target_arch = "wasm32"))] + pub apple_photos_imports: apple_photos::Imports, + /// Apple Photos imports run on a worker by default (`export.addToPhotos` without `wait`, + /// `app.export` without `background`): set by the desktop app, which answers control requests + /// on its UI thread. Off (the default), they wait for Photos, so a one-shot process + /// (lightcraft-cli, a script) never ends in the middle of one. + #[cfg(not(target_arch = "wasm32"))] + pub apple_photos_background: bool, } impl Default for Session { @@ -374,6 +390,12 @@ impl Session { cache_mb: 0, smart_previews_dir: None, forget_local_days: lightcraft_catalog::DEFAULT_FORGET_DAYS, + #[cfg(not(target_arch = "wasm32"))] + apple_photos: None, + #[cfg(not(target_arch = "wasm32"))] + apple_photos_imports: Default::default(), + #[cfg(not(target_arch = "wasm32"))] + apple_photos_background: false, } } diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 53d7cf1e..b57bd471 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -576,3 +576,286 @@ fn jxl_export_writes_metadata() { assert!(d.xmp.as_deref().is_some_and(|x| x.contains("Golden horizon")), "title in the XMP box"); assert!(d.exif.is_some(), "EXIF box"); } + +/// Issue #236: Add to Apple Photos after an export. A fake runner stands in for osascript: tests +/// never start it, nor Photos. +#[cfg(not(target_arch = "wasm32"))] +mod apple_photos { + use std::sync::{Arc, Mutex}; + + use serde_json::{Value, json}; + + use super::disk_batch; + use crate::Session; + use crate::apple_photos::{Imports, MARKER, Output, Runner}; + use crate::export::{AfterExport, Destination, ExportOptions}; + use crate::tests_xmp::temp_dir; + + type Calls = Arc>>>; + + /// Records the arguments; Photos "imports" the first `keep` files (all: `usize::MAX`). + fn fake(keep: usize) -> (Runner, Calls) { + let calls: Calls = Arc::default(); + let c = calls.clone(); + let r: Runner = Arc::new(move |args: &[String], _| { + c.lock().unwrap().push(args.to_vec()); + let files = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + let ids: Vec = (0..files.min(keep)).map(|i| format!("ID-{i}/L0/001")).collect(); + Ok(Output { status: Some(0), stdout: ids.join("\n") + "\n", stderr: String::new() }) + }); + (r, calls) + } + + /// The paths after the album: the files Photos was asked to import. + fn imported_paths(args: &[String]) -> Vec { + args[args.iter().position(|a| a == MARKER).unwrap() + 2..].to_vec() + } + + fn export_two(s: &mut Session, tag: &str) -> Vec { + let dir = temp_dir(tag); + let ids: Vec<_> = s.visible().iter().copied().take(2).collect(); + let o = ExportOptions::from_json(&json!({"longEdge": 48, "naming": "{name}-{seq}"})); + let to = Destination { dir: dir.to_string_lossy().into(), exact: None }; + let mut files = Vec::new(); + for id in ids { + files.extend(disk_batch(s, id, &o, &to).unwrap()); + } + files + } + + #[test] + fn exported_files_go_to_photos_in_order() { + let mut s = Session::with_demo(); + let files = export_two(&mut s, "photos-flow"); + let written: Vec = files.iter().map(|f| f["path"].as_str().unwrap().to_string()).collect(); + assert_eq!(written.len(), 2); + let (runner, calls) = fake(usize::MAX); + let imports = Imports::default(); + let after = AfterExport::apple_photos(runner.clone(), " Trip \"2026\"\n ", &imports).unwrap(); + // a skipped file and a failed photo are not sent + let mut results = files.clone(); + results.insert(1, json!({"skipped": "/elsewhere/x.jpg"})); + results.push(json!({"photo": 9, "file": "y", "error": "no source"})); + let out = after.run(&results, false).unwrap(); + assert_eq!(out["imported"], 2, "{out}"); + assert_eq!(out["requested"], 2); + assert_eq!(out["ids"], json!(["ID-0/L0/001", "ID-1/L0/001"])); + assert_eq!(out["album"], "Trip \"2026\""); + assert!(out.get("warning").is_none()); + let args = calls.lock().unwrap()[0].clone(); + assert_eq!(imported_paths(&args), written, "the files written, in order, as whole arguments"); + assert!(written.iter().all(|p| std::path::Path::new(p).is_absolute())); + assert_eq!(args[args.iter().position(|a| a == MARKER).unwrap() + 1], "Trip \"2026\""); + // a cancelled export adds nothing + let out = AfterExport::apple_photos(runner.clone(), "", &imports).unwrap().run(&files, true).unwrap(); + assert!(out["skipped"].as_str().unwrap().contains("cancelled")); + assert_eq!(calls.lock().unwrap().len(), 1); + // Photos took one of the two: said so, not an error + let (runner, _) = fake(1); + let out = AfterExport::apple_photos(runner, "", &Imports::default()).unwrap().run(&files, false).unwrap(); + assert_eq!((out["imported"].as_u64(), out["requested"].as_u64()), (Some(1), Some(2))); + assert!(out["warning"].as_str().unwrap().contains("1 of 2"), "{out}"); + assert!(out["album"].is_null()); + // permission refused: the outcome says where to allow it; the files stay written + let denied: Runner = Arc::new(|_: &[String], _| { + Ok(Output { + status: Some(1), + stdout: String::new(), + stderr: "execution error: Not authorized to send Apple events to Photos. (-1743)".into(), + }) + }); + let out = AfterExport::apple_photos(denied, "", &Imports::default()).unwrap().run(&files, false).unwrap(); + assert!(out["error"].as_str().unwrap().contains("Privacy & Security › Automation"), "{out}"); + assert!(written.iter().all(|p| std::path::Path::new(p).is_file())); + // nothing asked: nothing done + assert!(AfterExport::default().is_none()); + assert_eq!(AfterExport::default().run(&files, false), None); + } + + #[test] + fn photos_is_refused_before_anything_is_written_when_it_cannot_be_reached() { + let mut s = Session::with_demo(); + assert!(s.apple_photos.is_none(), "only with_fs installs the runner"); + let on = ExportOptions { add_to_photos: true, ..Default::default() }; + let e = s.after_export(&on).err().unwrap(); + if cfg!(target_os = "macos") { + assert!(e.contains("isn't connected in this session"), "{e}"); + } else { + assert_eq!(e, "Apple Photos is only available on macOS"); + } + assert!(s.after_export(&ExportOptions::default()).unwrap().is_none()); + // with a runner: macOS adds, other platforms still refuse + let (runner, calls) = fake(usize::MAX); + s.apple_photos = Some(runner); + let after = s.after_export(&on); + assert_eq!(after.is_ok(), cfg!(target_os = "macos"), "{:?}", after.as_ref().err()); + match after { + Ok(after) => { + let files = export_two(&mut s, "photos-session"); + assert_eq!(after.run(&files, false).unwrap()["imported"], 2); + assert_eq!(calls.lock().unwrap().len(), 1); + } + Err(e) => assert!(e.contains("only available on macOS"), "{e}"), + } + } + + #[test] + fn the_add_to_photos_command_needs_a_runner_and_checks_its_input() { + let mut s = Session::with_demo(); + let files = export_two(&mut s, "photos-cmd"); + let paths: Vec = files.iter().map(|f| f["path"].as_str().unwrap().to_string()).collect(); + let info = |s: &Session| s.commands().into_iter().find(|c| c.id == "export.addToPhotos").unwrap(); + // no runner: listed, disabled, with the reason + let i = info(&s); + assert!(!i.enabled); + let why = i.disabled_reason.unwrap(); + assert!(why.contains(if cfg!(target_os = "macos") { "isn't connected" } else { "only available on macOS" }), "{why}"); + let e = s.execute("export.addToPhotos", &json!({"paths": paths})).unwrap_err().to_string(); + assert!(e.contains("Apple Photos"), "{e}"); + let (runner, calls) = fake(usize::MAX); + s.apple_photos = Some(runner); + assert!(info(&s).enabled); + // no window: it waits for Photos by default + let r = s.execute("export.addToPhotos", &json!({"paths": paths, "album": "Läufe/日本 \\ \"q\""})).unwrap(); + assert_eq!((r["imported"].as_u64(), r["running"].as_bool(), r["job"].as_u64()), (Some(2), Some(false), Some(1)), "{r}"); + assert_eq!(r["album"], "Läufe/日本 \\ \"q\""); + assert_eq!(imported_paths(&calls.lock().unwrap()[0]), paths); + // bad input never reaches the runner (paths from the temp folder: absolute on every platform) + let missing = std::env::temp_dir().join("lc-photos-nonexistent").join("a.jpg").to_string_lossy().into_owned(); + for (p, msg) in [ + (json!({}), "missing `paths`"), + (json!({"paths": "a.jpg"}), "missing `paths`"), + (json!({"paths": [1]}), "array of file paths"), + (json!({"paths": []}), "no files"), + (json!({"paths": ["relative.jpg"]}), "absolute"), + (json!({"paths": [missing]}), "no such file"), + (json!({"paths": paths, "album": 3}), "`album` must be a string"), + (json!({"paths": paths, "wait": "yes"}), "`wait` must be true or false"), + (json!({"paths": paths, "albm": "x"}), "unknown parameter `albm`"), + ] { + let e = s.execute("export.addToPhotos", &p).unwrap_err().to_string(); + assert!(e.contains(msg), "{p}: {e}"); + } + assert_eq!(calls.lock().unwrap().len(), 1); + // not journaled: replaying actions must not import again + assert!(!s.journal.iter().any(|(id, _)| id == "export.addToPhotos")); + } + + /// The desktop app answers control requests on its UI thread: `export.addToPhotos` must not + /// wait there for Photos (minutes, or a permission prompt). It returns the running job; the + /// outcome comes from `export.photosImports`; a second import, or an export asking for Photos, + /// is refused until it ends, so a retry never adds the files twice. + #[test] + fn a_slow_photos_import_runs_in_the_background() { + let mut s = Session::with_demo(); + let files = export_two(&mut s, "photos-slow"); + let paths: Vec = files.iter().map(|f| f["path"].as_str().unwrap().to_string()).collect(); + let (go, wait) = std::sync::mpsc::channel::<()>(); + let wait = Mutex::new(wait); + let runner: Runner = Arc::new(move |args: &[String], _| { + // a regression (waiting on the caller's thread) fails after 20 s instead of hanging + let _ = wait.lock().unwrap().recv_timeout(std::time::Duration::from_secs(20)); + let n = args.len() - args.iter().position(|a| a == MARKER).unwrap() - 2; + Ok(Output { status: Some(0), stdout: (0..n).map(|i| format!("ID-{i}\n")).collect(), stderr: String::new() }) + }); + s.apple_photos = Some(runner); + s.apple_photos_background = true; // as in the desktop app + let t0 = std::time::Instant::now(); + let r = s.execute("export.addToPhotos", &json!({"paths": paths, "album": "Slow"})).unwrap(); + assert!(t0.elapsed() < std::time::Duration::from_secs(5), "answered without waiting for Photos"); + assert_eq!(r, json!({"job": 1, "running": true, "requested": 2, "album": "Slow"})); + let status = |s: &mut Session| s.execute("export.photosImports", &json!({"job": 1})).unwrap(); + assert_eq!(status(&mut s)["running"], true); + // a retry is refused, and says why + let e = s.execute("export.addToPhotos", &json!({"paths": paths})).unwrap_err().to_string(); + assert!(e.contains("still adding 2 file(s) from an earlier request (import 1)"), "{e}"); + if cfg!(target_os = "macos") { + let on = ExportOptions { add_to_photos: true, ..Default::default() }; + assert!(s.after_export(&on).err().unwrap().contains("still adding"), "refused before writing anything"); + } + go.send(()).unwrap(); + while status(&mut s)["running"] == true { + assert!(t0.elapsed() < std::time::Duration::from_secs(20), "the import finishes"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let done = status(&mut s); + assert_eq!((done["imported"].as_u64(), done["album"].as_str()), (Some(2), Some("Slow")), "{done}"); + 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})).unwrap()["job"], 2); + } + + /// Review of #236: an export adding to Photos reserves the import before it writes its files. + /// While it renders, `export.addToPhotos` is refused, naming the export's job, instead of + /// taking Photos and leaving the export to fail its Photos step after writing. + #[test] + fn an_export_holds_photos_from_before_it_writes() { + let mut s = Session::with_demo(); + let (runner, calls) = fake(usize::MAX); + s.apple_photos = Some(runner.clone()); + let elsewhere = export_two(&mut s, "photos-hold-other"); + let other: Vec = elsewhere.iter().map(|f| f["path"].as_str().unwrap().to_string()).collect(); + // what Session::after_export does on macOS (it refuses other platforms first) + let after = AfterExport::apple_photos(runner, "Held", &s.apple_photos_imports).unwrap(); + let job = s.execute("export.photosImports", &json!({"job": 1})).unwrap(); + assert_eq!((job["running"].as_bool(), job["exporting"].as_bool()), (Some(true), Some(true)), "{job}"); + let e = s.execute("export.addToPhotos", &json!({"paths": other})).unwrap_err().to_string(); + assert!(e.contains("An export is about to add its files to Apple Photos (import 1)"), "{e}"); + if cfg!(target_os = "macos") { + let on = ExportOptions { add_to_photos: true, ..Default::default() }; + assert!(s.after_export(&on).err().unwrap().contains("(import 1)"), "a second export is refused before writing"); + } + assert!(calls.lock().unwrap().is_empty()); + // the export writes its files and hands them over: imported under its own job + let files = export_two(&mut s, "photos-hold"); + let out = after.run(&files, false).unwrap(); + assert_eq!((out["job"].as_u64(), out["imported"].as_u64(), out["album"].as_str()), (Some(1), Some(2), Some("Held")), "{out}"); + // then the slot is free + assert_eq!(s.execute("export.addToPhotos", &json!({"paths": other})).unwrap()["job"], 2); + // an export that fails (its AfterExport dropped) frees it too + drop(AfterExport::apple_photos(fake(usize::MAX).0, "", &s.apple_photos_imports).unwrap()); + assert!(s.apple_photos_imports.running().is_none()); + assert_eq!(s.execute("export.addToPhotos", &json!({"paths": other})).unwrap()["job"], 4); + } + + /// Review of #236: a runner that panics while the command or an export waits for it is that + /// import's error; the slot is freed, so the next import isn't refused as "still running" + /// and a process ending doesn't wait for it. + #[test] + fn a_panicking_runner_never_holds_photos() { + let mut s = Session::with_demo(); + let files = export_two(&mut s, "photos-panic"); + let paths: Vec = files.iter().map(|f| f["path"].as_str().unwrap().to_string()).collect(); + s.apple_photos = Some(Arc::new(|_: &[String], _| panic!("runner exploded"))); + // the command (no window: it waits) + let e = s.execute("export.addToPhotos", &json!({"paths": paths})).unwrap_err().to_string(); + assert!(e.contains("stopped unexpectedly"), "{e}"); + assert!(s.apple_photos_imports.running().is_none()); + assert_eq!(s.execute("export.photosImports", &json!({"job": 1})).unwrap()["running"], false); + // an export's reservation + let runner = s.apple_photos.clone().unwrap(); + let out = AfterExport::apple_photos(runner, "", &s.apple_photos_imports).unwrap().run(&files, false).unwrap(); + assert!(out["error"].as_str().unwrap().contains("stopped unexpectedly"), "{out}"); + assert!(s.apple_photos_imports.running().is_none()); + assert!(!s.wait_for_photos_imports(), "nothing left to wait for"); + // and the next import runs + let (good, _) = fake(usize::MAX); + s.apple_photos = Some(good); + assert_eq!(s.execute("export.addToPhotos", &json!({"paths": paths})).unwrap()["imported"], 2); + } + + /// The desktop app, lightcraft-cli and the MCP server get the real runner from `with_fs`, on + /// macOS only. (Installed, never run here.) + #[test] + fn with_fs_installs_the_runner_on_macos_only() { + let s = Session::new().with_fs(); + assert_eq!(s.apple_photos.is_some(), cfg!(target_os = "macos")); + assert_eq!(s.commands().into_iter().find(|c| c.id == "export.addToPhotos").unwrap().enabled, cfg!(target_os = "macos")); + } +} diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index 663f6c83..8678b1f7 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -24,8 +24,12 @@ pub struct Headless { } impl Drop for Headless { - /// Snapshot a persistent library on exit (a no-op for in-memory sessions). + /// Snapshot a persistent library on exit (a no-op for in-memory sessions), after any Apple + /// Photos import still running (`export.addToPhotos {wait: false}`) has ended: the process + /// must not end in the middle of one, abandoning osascript and the import's result. fn drop(&mut self) { + #[cfg(not(target_arch = "wasm32"))] + self.session.wait_for_photos_imports(); let _ = self.session.close_library(); } } @@ -85,6 +89,8 @@ impl Headless { }; let dir = p.get("dir").and_then(Value::as_str).unwrap_or(""); let write = &mut lightcraft_engine::export::write_file; + // Add to Apple Photos: refused before anything is written when it can't be done + let after = self.session.after_export(&opts)?; let items = prepare_batch(&mut self.session, &ids, &opts)?; let total = items.len(); let mut hook = self.progress.take(); @@ -109,6 +115,9 @@ impl Headless { } // Single-photo exports also report path/width/height at the top level (back-compat). let mut out = files.first().cloned().unwrap_or_else(|| json!({})); + if let Some(photos) = after.run(&files, false) { + out["applePhotos"] = photos; + } out["files"] = json!(files); Ok(out) } @@ -133,7 +142,7 @@ impl Backend for Headless { "engine.commands" => { let mut v: Vec = self.session.commands().into_iter().map(|c| serde_json::to_value(c).unwrap_or_default()).collect(); v.push(json!({"id": "app.export", "label": "Export Now", "menu": [], "shortcut": null, - "params": "{path?: output file (.jpg/.png/.tif/.webp/.avif/.jxl/.psd/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|jxl|psd|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), dontEnlarge?, ppi?, quality?: 1..100, jxlLossless?, jxlEffort?: fast|normal, jxlProgressive?, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, hdr? (HDR output for HDR edits: gain map JPEG, PQ AVIF / PNG / JPEG XL, float TIFF), sharpen?: none|screen|matte|glossy, sharpenAmount?: low|standard|high, metadata?: all|allExceptCamera|copyright|none, removeLocation?, naming?, startNumber?, subfolder?, conflict?: unique|overwrite|skip, tiffCompression?: none|lzw|zip, dngCompression?: lossless|deflate|uncompressed, watermark?: text | {text?, vertical? (upright columns, right to left), size? (text height, 0.005..0.5 of the short edge; default 0.035), opacity? (0..1; 0.7), anchor?: topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight, insetX? (margin from the left/right edge, 0..0.4 of the short edge; 0.025), insetY? (from the top/bottom edge; 0.025), inset? (sets both), rotation?: 0|90|180|270 (degrees clockwise; 0), color? [r,g,b] sRGB, shadow?, image? (graphic drawn instead of the text), imageWidth? (0.01..1 of the photo's width; 0.2)}} — an unknown parameter, an out-of-range watermark size or a value of the wrong kind is an error, not a default", + "params": "{path?: output file (.jpg/.png/.tif/.webp/.avif/.jxl/.psd/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|jxl|psd|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), dontEnlarge?, ppi?, quality?: 1..100, jxlLossless?, jxlEffort?: fast|normal, jxlProgressive?, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, hdr? (HDR output for HDR edits: gain map JPEG, PQ AVIF / PNG / JPEG XL, float TIFF), sharpen?: none|screen|matte|glossy, sharpenAmount?: low|standard|high, metadata?: all|allExceptCamera|copyright|none, removeLocation?, naming?, startNumber?, subfolder?, conflict?: unique|overwrite|skip, tiffCompression?: none|lzw|zip, dngCompression?: lossless|deflate|uncompressed, addToPhotos?: bool (macOS: then add the files written to Apple Photos; outcome in `applePhotos`), photosAlbum?: top-level album name (made when missing), watermark?: text | {text?, vertical? (upright columns, right to left), size? (text height, 0.005..0.5 of the short edge; default 0.035), opacity? (0..1; 0.7), anchor?: topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight, insetX? (margin from the left/right edge, 0..0.4 of the short edge; 0.025), insetY? (from the top/bottom edge; 0.025), inset? (sets both), rotation?: 0|90|180|270 (degrees clockwise; 0), color? [r,g,b] sRGB, shadow?, image? (graphic drawn instead of the text), imageWidth? (0.01..1 of the photo's width; 0.2)}} — an unknown parameter, an out-of-range watermark size or a value of the wrong kind is an error, not a default", "enabled": self.session.active().is_some()})); Ok(Value::Array(v)) } diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index e09c9d4c..9975ef06 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -152,6 +152,99 @@ fn export_tool_forwards_its_advertised_options() { let _ = std::fs::remove_dir_all(&base); } +/// Issue #236: `app.export {addToPhotos, photosAlbum}` through the headless backend (the MCP +/// `export` tool, `lightcraft-cli run`): the files written go to Photos and the result says how +/// it went; where Photos isn't (not macOS), it is refused before anything is written. A fake +/// runner replaces the real one first: tests never start osascript or Photos. +#[test] +fn export_adds_to_apple_photos_where_there_is_photos() { + use std::sync::{Arc, Mutex}; + let mut b = Headless::demo(); + let calls = Arc::new(Mutex::new(Vec::>::new())); + let c = calls.clone(); + b.session.apple_photos = Some(Arc::new(move |args: &[String], _| { + c.lock().unwrap().push(args.to_vec()); + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID-1/L0/001\n".into(), stderr: String::new() }) + })); + let base = std::env::temp_dir().join(format!("lc-mcp-export-photos-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let r = call_tool(&mut b, "export", &json!({"dir": base.to_string_lossy(), "longEdge": 32, "addToPhotos": true, "photosAlbum": "From \"MCP\""})); + if cfg!(target_os = "macos") { + assert!(!r.is_error, "{r:?}"); + let out = r.structured.unwrap(); + assert_eq!(out["applePhotos"]["imported"], 1, "{out}"); + assert_eq!(out["applePhotos"]["album"], "From \"MCP\""); + let written = out["files"][0]["path"].as_str().unwrap().to_string(); + let args = calls.lock().unwrap().clone(); + assert_eq!(args.len(), 1); + assert_eq!(args[0].last(), Some(&written)); + } else { + assert!(r.is_error, "{r:?}"); + assert!(format!("{:?}", r.content).contains("only available on macOS"), "{r:?}"); + assert!(!base.exists(), "nothing written"); + assert!(calls.lock().unwrap().is_empty()); + } + let _ = std::fs::remove_dir_all(&base); +} + +/// Review of #236: a one-shot process (`lightcraft-cli run`, an MCP server reaching EOF) must not +/// end in the middle of an Apple Photos import, abandoning osascript and the result. The test runs +/// itself as a child process (`LC_PHOTOS_CHILD`): the child adds a file with a fake runner that +/// takes a while and then leaves a marker, prints the reply and ends as the CLI does (dropping +/// its backend). The parent checks the import had finished: by default the call waited for +/// Photos, and with `wait: false` the backend waited before the process ended. Never osascript: +/// the child's session has no file-system hooks and only the fake runner. +#[test] +fn a_one_shot_process_never_ends_during_a_photos_import() { + use std::sync::Arc; + use std::time::Duration; + if let (Ok(dir), Ok(mode)) = (std::env::var("LC_PHOTOS_CHILD"), std::env::var("LC_PHOTOS_CHILD_MODE")) { + let dir = std::path::PathBuf::from(dir); + let file = dir.join("a.jpg"); + let marker = dir.join("imported"); + let mut h = Headless::new(lightcraft_engine::Session::with_demo()); + h.session.apple_photos = Some(Arc::new(move |args: &[String], _| { + std::thread::sleep(Duration::from_millis(400)); + std::fs::write(&marker, args.last().cloned().unwrap_or_default()).unwrap(); + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID-1\n".into(), stderr: String::new() }) + })); + let mut p = json!({"paths": [file.to_string_lossy()]}); + if mode == "nowait" { + p["wait"] = json!(false); + } + let r = h.call("engine.execute", json!({"command": "export.addToPhotos", "params": p})).unwrap(); + println!("CHILD-REPLY {r}"); + drop(h); // what lightcraft-cli does before it exits + return; + } + for mode in ["default", "nowait"] { + let dir = std::env::temp_dir().join(format!("lc-mcp-photos-child-{mode}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("a.jpg"), b"jpeg").unwrap(); + let out = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "tests::a_one_shot_process_never_ends_during_a_photos_import", "--nocapture", "--test-threads=1"]) + .env("LC_PHOTOS_CHILD", &dir) + .env("LC_PHOTOS_CHILD_MODE", mode) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(out.status.success(), "{mode}: {stdout}\n{}", String::from_utf8_lossy(&out.stderr)); + let reply: Value = stdout + .lines() + .find_map(|l| l.split_once("CHILD-REPLY ").map(|(_, j)| serde_json::from_str(j).unwrap())) + .unwrap_or_else(|| panic!("{mode}: no reply in {stdout}")); + let marker = std::fs::read_to_string(dir.join("imported")).unwrap_or_else(|_| panic!("{mode}: the process ended before the import did")); + assert!(marker.ends_with("a.jpg"), "{marker}"); + if mode == "default" { + assert_eq!((reply["running"].as_bool(), reply["imported"].as_u64()), (Some(false), Some(1)), "waited: {reply}"); + } else { + assert_eq!(reply["running"], true, "{reply}"); + } + let _ = std::fs::remove_dir_all(&dir); + } +} + /// The `import` helper moves: renamed into the folder template, the source removed. #[test] fn import_tool_moves_with_a_folder_template() { diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 2f51d64d..93b0567d 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -262,6 +262,8 @@ pub fn helper_tools(has_ui: bool) -> Vec { "colorSpace": {"type": "string", "enum": ["srgb", "displayP3", "adobeRgb", "proPhoto", "rec2020"], "description": "Output colour space (default sRGB; AVIF is always sRGB; lossy JPEG XL writes adobeRgb/proPhoto as rec2020). adobeRgb = Adobe RGB (1998) compatible; the embedded ICC profile is generated from the published primaries"}, "bitDepth": {"type": "integer", "enum": [8, 10, 16, 32], "description": "Bits per channel: PNG 8|16 (default 8), TIFF 8|16|32 (default 16; 32 = linear float with a linear profile), PSD 8|16 (default 16), AVIF 8|10, JPEG XL 8|16; JPEG/WebP are 8-bit"}, "hdr": {"type": "boolean", "description": "HDR output for photos edited in HDR (develop.hdr): JPEG as an ISO 21496-1 gain map JPEG, AVIF as 10-bit, PNG and JPEG XL as 16-bit Rec. 2020 PQ, 32-bit TIFF with highlights above 1; other formats and SDR edits export as usual"}, + "addToPhotos": {"type": "boolean", "description": "macOS: after writing the files, add them to Apple Photos (Apple Events to Photos' import; the first time, macOS asks the user to allow LightCraft to control Photos). The outcome is `applePhotos` in the result, the import job: {job, running, requested, album, imported?, ids?, warning?, error?}; headless waits for Photos, the desktop app returns the running job (outcome: command export.photosImports {job}). The files are written either way. An error before exporting on other platforms, or while another import is reserved or running"}, + "photosAlbum": {"type": "string", "description": "With addToPhotos: the album to add them to, a top-level Photos album of exactly this name, made when missing (several of that name: nothing is added); default none"}, "watermark": {"description": "Text, or {text, vertical (boolean; defaults to false: upright columns right to left), size (text height as a fraction of the short edge, 0.005..0.5; default 0.035), opacity (0..1; default 0.7), anchor (topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight), insetX (margin from the left or right edge as a fraction of the short edge, 0..0.4; default 0.025), insetY (margin from the top or bottom edge, same units), inset (sets both), rotation (degrees clockwise: 0|90|180|270; default 0; the watermark is sized before it is turned), color [r,g,b] (sRGB 0..255), shadow (boolean), image (path of a graphic drawn instead of the text), imageWidth (fraction of the photo's width, 0.01..1; default 0.2)}. Unknown keys and out-of-range sizes are errors"} }), &[], @@ -748,6 +750,8 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { "conflict", "tiffCompression", "dngCompression", + "addToPhotos", + "photosAlbum", ], ), ) diff --git a/crates/ui-egui/locales/de-formats.json b/crates/ui-egui/locales/de-formats.json index b9a6debe..ca0b6705 100644 --- a/crates/ui-egui/locales/de-formats.json +++ b/crates/ui-egui/locales/de-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "{what}-Zusammenfügung abgebrochen", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "Übersprungen: {n} lokale Fotos{0:.0} – Alben enthalten Bibliotheksfotos (zuerst zu „Meine Fotos“ hinzufügen)", "New Folder in “{name}”": "Neuer Ordner in „{name}“", - "Moved {} photo(s)": "{} Foto(s) verschoben" + "Moved {} photo(s)": "{} Foto(s) verschoben", + "not added to Apple Photos: {e}": "nicht zu Apple Fotos hinzugefügt: {e}", + "{n} added to Apple Photos album “{album}”": "{n} zum Apple-Fotos-Album „{album}“ hinzugefügt", + "{n} added to Apple Photos": "{n} zu Apple Fotos hinzugefügt" } diff --git a/crates/ui-egui/locales/de.json b/crates/ui-egui/locales/de.json index 5c7d81a9..aa591b05 100644 --- a/crates/ui-egui/locales/de.json +++ b/crates/ui-egui/locales/de.json @@ -2105,5 +2105,10 @@ "Appearance Settings…": "Erscheinungsbild-Einstellungen…", "Add to favorites": "Zu Favoriten hinzufügen", "Remove from favorites": "Aus Favoriten entfernen", - "{n} photos opened for editing; each is stacked with its original": "{n} Fotos zur Bearbeitung geöffnet; jedes ist mit seinem Original gestapelt" + "{n} photos opened for editing; each is stacked with its original": "{n} Fotos zur Bearbeitung geöffnet; jedes ist mit seinem Original gestapelt", + "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.", + "Adding to Apple Photos": "Wird zu Apple Fotos hinzugefügt" } diff --git a/crates/ui-egui/locales/es-formats.json b/crates/ui-egui/locales/es-formats.json index c6132475..3cb8047d 100644 --- a/crates/ui-egui/locales/es-formats.json +++ b/crates/ui-egui/locales/es-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "Combinación {what} cancelada", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "Fotos locales omitidas: {n}{0:.0} — los álbumes contienen fotos de la biblioteca (añádelas primero a Mis fotos)", "New Folder in “{name}”": "Nueva carpeta en «{name}»", - "Moved {} photo(s)": "{} foto(s) movida(s)" + "Moved {} photo(s)": "{} foto(s) movida(s)", + "not added to Apple Photos: {e}": "no se añadieron a Fotos de Apple: {e}", + "{n} added to Apple Photos album “{album}”": "{n} añadidas al álbum “{album}” de Fotos de Apple", + "{n} added to Apple Photos": "{n} añadidas a Fotos de Apple" } diff --git a/crates/ui-egui/locales/es.json b/crates/ui-egui/locales/es.json index 5b9e12b1..716e420d 100644 --- a/crates/ui-egui/locales/es.json +++ b/crates/ui-egui/locales/es.json @@ -1947,5 +1947,10 @@ "Appearance Settings…": "Ajustes de apariencia…", "Add to favorites": "Añadir a favoritos", "Remove from favorites": "Quitar de favoritos", - "{n} photos opened for editing; each is stacked with its original": "{n} fotos abiertas para editar; cada una está apilada con su original" + "{n} photos opened for editing; each is stacked with its original": "{n} fotos abiertas para editar; cada una está apilada con su original", + "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.", + "Adding to Apple Photos": "Añadiendo a Fotos de Apple" } diff --git a/crates/ui-egui/locales/fr-formats.json b/crates/ui-egui/locales/fr-formats.json index 0ac18fea..c076d09e 100644 --- a/crates/ui-egui/locales/fr-formats.json +++ b/crates/ui-egui/locales/fr-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "Fusion {what} annulée", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "Photos locales ignorées : {n}{0:.0} — les albums contiennent des photos de la bibliothèque (ajoutez-les d’abord à Mes photos)", "New Folder in “{name}”": "Nouveau dossier dans « {name} »", - "Moved {} photo(s)": "{} photo(s) déplacée(s)" + "Moved {} photo(s)": "{} photo(s) déplacée(s)", + "not added to Apple Photos: {e}": "non ajouté à Apple Photos : {e}", + "{n} added to Apple Photos album “{album}”": "{n} ajouté(s) à l'album Apple Photos « {album} »", + "{n} added to Apple Photos": "{n} ajouté(s) à Apple Photos" } diff --git a/crates/ui-egui/locales/fr.json b/crates/ui-egui/locales/fr.json index 9054e3fd..d4fcff46 100644 --- a/crates/ui-egui/locales/fr.json +++ b/crates/ui-egui/locales/fr.json @@ -1947,5 +1947,10 @@ "Midnight": "Minuit", "Silver": "Argent", "Paper": "Papier", - "{n} photos opened for editing; each is stacked with its original": "{n} photos ouvertes pour modification ; chacune est empilée avec son original" + "{n} photos opened for editing; each is stacked with its original": "{n} photos ouvertes pour modification ; chacune est empilée avec son original", + "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.", + "Adding to Apple Photos": "Ajout à Apple Photos" } diff --git a/crates/ui-egui/locales/ja-formats.json b/crates/ui-egui/locales/ja-formats.json index 3dabbde1..f5964ece 100644 --- a/crates/ui-egui/locales/ja-formats.json +++ b/crates/ui-egui/locales/ja-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "{what}合成をキャンセルしました", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "ローカル写真{n}枚をスキップしました{0:.0}：アルバムに入れられるのはライブラリの写真です（先にマイフォトに追加してください）", "New Folder in “{name}”": "「{name}」に新規フォルダ", - "Moved {} photo(s)": "{} 枚の写真を移動しました" + "Moved {} photo(s)": "{} 枚の写真を移動しました", + "not added to Apple Photos: {e}": "Appleの写真に追加されませんでした：{e}", + "{n} added to Apple Photos album “{album}”": "{n}件をAppleの写真のアルバム「{album}」に追加しました", + "{n} added to Apple Photos": "{n}件をAppleの写真に追加しました" } diff --git a/crates/ui-egui/locales/ja.json b/crates/ui-egui/locales/ja.json index 4c503519..8f387d97 100644 --- a/crates/ui-egui/locales/ja.json +++ b/crates/ui-egui/locales/ja.json @@ -1949,5 +1949,10 @@ "Appearance Settings…": "外観設定…", "Add to favorites": "お気に入りに追加", "Remove from favorites": "お気に入りから削除", - "{n} photos opened for editing; each is stacked with its original": "{n}枚の写真を編集用に開きました。それぞれ元の写真とスタックされています" + "{n} photos opened for editing; each is stacked with its original": "{n}枚の写真を編集用に開きました。それぞれ元の写真とスタックされています", + "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が確認します。", + "Adding to Apple Photos": "Appleの写真に追加中" } diff --git a/crates/ui-egui/locales/pt-br-formats.json b/crates/ui-egui/locales/pt-br-formats.json index b76d794c..74a0c7a8 100644 --- a/crates/ui-egui/locales/pt-br-formats.json +++ b/crates/ui-egui/locales/pt-br-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "{what} mesclagem cancelada", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "Fotos locais ignoradas: {n}{0:.0} — álbuns contêm fotos da biblioteca (adicione-as às Minhas fotos primeiro)", "New Folder in “{name}”": "Nova pasta em “{name}”", - "Moved {} photo(s)": "{} foto(s) movida(s)" + "Moved {} photo(s)": "{} foto(s) movida(s)", + "not added to Apple Photos: {e}": "não adicionadas ao Fotos da Apple: {e}", + "{n} added to Apple Photos album “{album}”": "{n} adicionadas ao álbum “{album}” do Fotos da Apple", + "{n} added to Apple Photos": "{n} adicionadas ao Fotos da Apple" } diff --git a/crates/ui-egui/locales/pt-br.json b/crates/ui-egui/locales/pt-br.json index b992e6cb..a6d42d26 100644 --- a/crates/ui-egui/locales/pt-br.json +++ b/crates/ui-egui/locales/pt-br.json @@ -1947,5 +1947,10 @@ "Appearance Settings…": "Configurações de aparência…", "Add to favorites": "Adicionar aos favoritos", "Remove from favorites": "Remover dos favoritos", - "{n} photos opened for editing; each is stacked with its original": "{n} fotos abertas para edição; cada uma está empilhada com a original" + "{n} photos opened for editing; each is stacked with its original": "{n} fotos abertas para edição; cada uma está empilhada com a original", + "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.", + "Adding to Apple Photos": "Adicionando ao Fotos da Apple" } diff --git a/crates/ui-egui/locales/ru-formats.json b/crates/ui-egui/locales/ru-formats.json index 4a4f0e90..403b474a 100644 --- a/crates/ui-egui/locales/ru-formats.json +++ b/crates/ui-egui/locales/ru-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "{what}: слияние отменено", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "Пропущено локальных фото: {n}{0:.0} — в альбомах только фото из библиотеки (сначала добавьте их в «Мои фото»)", "New Folder in “{name}”": "Новая папка в «{name}»", - "Moved {} photo(s)": "Перемещено фото: {}" + "Moved {} photo(s)": "Перемещено фото: {}", + "not added to Apple Photos: {e}": "не добавлено в Apple Фото: {e}", + "{n} added to Apple Photos album “{album}”": "Добавлено в альбом «{album}» Apple Фото: {n}", + "{n} added to Apple Photos": "Добавлено в Apple Фото: {n}" } diff --git a/crates/ui-egui/locales/ru.json b/crates/ui-egui/locales/ru.json index e03913c2..b7620212 100644 --- a/crates/ui-egui/locales/ru.json +++ b/crates/ui-egui/locales/ru.json @@ -2139,5 +2139,10 @@ "≤ at most": "≤ не более", "≥ at least": "≥ не менее", "⌘-drag to straighten, ⇧G Guided Upright, a grid while transforming.": "⌘+перетаскивание — выровнять, ⇧G — направляемое выравнивание, сетка при трансформации.", - "{n} photos opened for editing; each is stacked with its original": "Открыто для редактирования фото: {n}; каждое объединено в стопку с оригиналом" + "{n} photos opened for editing; each is stacked with its original": "Открыто для редактирования фото: {n}; каждое объединено в стопку с оригиналом", + "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 управлять приложением «Фото».", + "Adding to Apple Photos": "Добавление в Apple Фото" } diff --git a/crates/ui-egui/locales/uk-formats.json b/crates/ui-egui/locales/uk-formats.json index b9b5bca7..a5a3824e 100644 --- a/crates/ui-egui/locales/uk-formats.json +++ b/crates/ui-egui/locales/uk-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "Об’єднання {what} скасовано", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "Пропущено локальних фото: {n}{0:.0} — в альбомах лише фото з бібліотеки (спершу додайте їх до моїх фото)", "New Folder in “{name}”": "Нова тека в «{name}»", - "Moved {} photo(s)": "Переміщено фото: {}" + "Moved {} photo(s)": "Переміщено фото: {}", + "not added to Apple Photos: {e}": "не додано до Apple Фото: {e}", + "{n} added to Apple Photos album “{album}”": "Додано до альбому Apple Фото «{album}»: {n}", + "{n} added to Apple Photos": "Додано до Apple Фото: {n}" } diff --git a/crates/ui-egui/locales/uk.json b/crates/ui-egui/locales/uk.json index 1d70b360..3d983ea8 100644 --- a/crates/ui-egui/locales/uk.json +++ b/crates/ui-egui/locales/uk.json @@ -2141,5 +2141,10 @@ "Midnight": "Опівніч", "Silver": "Срібло", "Paper": "Папір", - "{n} photos opened for editing; each is stacked with its original": "Відкрито для редагування фото: {n}; кожне об’єднано в стос з оригіналом" + "{n} photos opened for editing; each is stacked with its original": "Відкрито для редагування фото: {n}; кожне об’єднано в стос з оригіналом", + "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 керувати програмою «Фото».", + "Adding to Apple Photos": "Додавання до Apple Фото" } diff --git a/crates/ui-egui/locales/zh-hans-formats.json b/crates/ui-egui/locales/zh-hans-formats.json index 8f62003f..2020e26c 100644 --- a/crates/ui-egui/locales/zh-hans-formats.json +++ b/crates/ui-egui/locales/zh-hans-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "已取消 {what} 合并", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "已跳过 {n} 张本地照片{0:.0}：相册只能收录图库中的照片（请先添加到我的照片）", "New Folder in “{name}”": "在“{name}”中新建文件夹", - "Moved {} photo(s)": "已移动 {} 张照片" + "Moved {} photo(s)": "已移动 {} 张照片", + "not added to Apple Photos: {e}": "未添加到 Apple 照片：{e}", + "{n} added to Apple Photos album “{album}”": "已将 {n} 个添加到 Apple 照片相册“{album}”", + "{n} added to Apple Photos": "已将 {n} 个添加到 Apple 照片" } diff --git a/crates/ui-egui/locales/zh-hans.json b/crates/ui-egui/locales/zh-hans.json index b47aba48..7cb6091b 100644 --- a/crates/ui-egui/locales/zh-hans.json +++ b/crates/ui-egui/locales/zh-hans.json @@ -1985,5 +1985,10 @@ "save goes through.": "才能完成保存。", "smart album value|No": "智能相册取值|否", "smart album value|Yes": "智能相册取值|是", - "{n} photos opened for editing; each is stacked with its original": "已打开 {n} 张照片进行编辑；每张都与其原片堆叠在一起" + "{n} photos opened for editing; each is stacked with its original": "已打开 {n} 张照片进行编辑；每张都与其原片堆叠在一起", + "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 控制“照片”。", + "Adding to Apple Photos": "正在添加到 Apple 照片" } diff --git a/crates/ui-egui/locales/zh-hant-formats.json b/crates/ui-egui/locales/zh-hant-formats.json index 390359ff..3b95e938 100644 --- a/crates/ui-egui/locales/zh-hant-formats.json +++ b/crates/ui-egui/locales/zh-hant-formats.json @@ -240,5 +240,8 @@ "{what} merge cancelled": "已取消{what}合併", "{n} Local photo{} skipped: albums hold library photos (Add to My Photos first)": "已略過 {n} 張本機照片{0:.0}：相簿只能收錄圖庫中的照片（請先加入我的照片）", "New Folder in “{name}”": "在「{name}」中新增資料夾", - "Moved {} photo(s)": "已移動 {} 張照片" + "Moved {} photo(s)": "已移動 {} 張照片", + "not added to Apple Photos: {e}": "未加入 Apple 照片：{e}", + "{n} added to Apple Photos album “{album}”": "已將 {n} 個加入 Apple 照片相簿「{album}」", + "{n} added to Apple Photos": "已將 {n} 個加入 Apple 照片" } diff --git a/crates/ui-egui/locales/zh-hant.json b/crates/ui-egui/locales/zh-hant.json index 92974711..96630878 100644 --- a/crates/ui-egui/locales/zh-hant.json +++ b/crates/ui-egui/locales/zh-hant.json @@ -1947,5 +1947,10 @@ "Appearance Settings…": "外觀設定…", "Add to favorites": "加入最愛", "Remove from favorites": "從最愛移除", - "{n} photos opened for editing; each is stacked with its original": "已開啟 {n} 張相片進行編輯；每張都與其原始相片堆疊在一起" + "{n} photos opened for editing; each is stacked with its original": "已開啟 {n} 張相片進行編輯；每張都與其原始相片堆疊在一起", + "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 控制「照片」。", + "Adding to Apple Photos": "正在加入 Apple 照片" } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 360893be..cf2dda38 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -494,13 +494,23 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result>, pub cancel: Arc, - rx: Receiver, 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, Option, bool), String>>, /// The export's row in the activity stack (`activity.cancel` sets `cancel`). guard: TaskGuard, } @@ -35,8 +37,9 @@ impl ExportTask { } } -/// Start exporting `items` in the background. Errors per photo are collected, not fatal. -pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOptions, to: Destination) -> Result { +/// Start exporting `items` in the background, then do `after` with the files written (Add to Apple +/// Photos). Errors per photo are collected, not fatal. +pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOptions, to: Destination, after: AfterExport) -> Result { if app.export.is_some() { return Err("an export is already running".into()); } @@ -46,6 +49,11 @@ pub fn start(app: &mut LightcraftApp, items: Vec, 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 || { let mut w = |path: &str, bytes: &[u8]| write(path, bytes); let r = run_batch(items, &opts, &to, &mut w, &|path| std::path::Path::new(path).exists(), false, &mut |done, name| { @@ -54,14 +62,25 @@ pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOp } !c.load(Ordering::Relaxed) }); + let r = r.map(|files| { + 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 cancelled = c.load(Ordering::Relaxed); + let photos = after.run(&files, cancelled); + (files, photos, cancelled) + }); let _ = tx.send(r); }; #[cfg(not(target_arch = "wasm32"))] 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})) } @@ -100,7 +119,8 @@ pub fn start_contact_sheet(app: &mut LightcraftApp, params: &Value) -> Result Result String { + if let Some(e) = outcome.get("error").and_then(Value::as_str) { + return crate::i18n::tr_format!("not added to Apple Photos: {e}", e = e); + } + if let Some(why) = outcome.get("skipped").and_then(Value::as_str) { + if why.contains("cancelled") { + return crate::i18n::tr("not added to Apple Photos (cancelled)").to_string(); + } + return crate::i18n::tr_format!("not added to Apple Photos: {e}", e = why); + } + let n = outcome.get("imported").and_then(Value::as_u64).unwrap_or(0); + let mut m = match outcome.get("album").and_then(Value::as_str) { + Some(album) => crate::i18n::tr_format!("{n} added to Apple Photos album “{album}”", n = n, album = album), + None => crate::i18n::tr_format!("{n} added to Apple Photos", n = n), + }; + if let Some(w) = outcome.get("warning").and_then(Value::as_str) { + m += &format!(" ({w})"); + } + m +} + +/// Per frame: when an Apple Photos import that no export task reports finishes (`app.export` +/// without `background`, `export.addToPhotos`), say how it went. +#[cfg(not(target_arch = "wasm32"))] +pub fn poll_photos(app: &mut LightcraftApp, ctx: &egui::Context) { + let jobs = app.session.apple_photos_imports.all(); + if jobs.iter().any(|j| !j.finished()) { + ctx.request_repaint_after(std::time::Duration::from_millis(250)); + } + for job in jobs { + if job.take_announcement() { + let outcome = job.json(); + let msg = apple_photos_note(&outcome); + if outcome.get("error").is_some() { + app.toast_for(ctx, msg, 15.0); + } else { + app.toast(ctx, msg); + } + } + } +} + +/// No Apple Photos on the web. +#[cfg(target_arch = "wasm32")] +pub fn poll_photos(_: &mut LightcraftApp, _: &egui::Context) {} + /// Per frame: keep the activity row up to date; when the batch finishes, report it. 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 msg = match r { - Ok(files) => { + 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(); @@ -140,12 +209,20 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) { if cancelled { m += " · cancelled"; } - app.last_export_result = Some(json!({"files": files, "cancelled": cancelled})); + if let Some(photos) = &photos { + m += " · "; + m += &apple_photos_note(photos); + } + app.last_export_result = Some(json!({"files": files, "cancelled": cancelled, "applePhotos": photos})); m } Err(e) => e, }; - app.toast(ctx, msg); + if photos_failed { + app.toast_for(ctx, msg, 15.0); + } else { + app.toast(ctx, msg); + } } Err(std::sync::mpsc::TryRecvError::Empty) => { let (done, current) = task.progress.lock().map(|g| g.clone()).unwrap_or_default(); @@ -248,3 +325,24 @@ mod contact_sheet_tests { assert!(task.rx.recv_timeout(Duration::from_secs(20)).unwrap().is_err()); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// The toast's Apple Photos part (issue #236): the count and album, a partial import's + /// warning, an error with what to do, a cancelled export. + #[test] + fn apple_photos_outcomes_read_well() { + assert_eq!(apple_photos_note(&json!({"imported": 3, "requested": 3, "ids": [], "album": null})), "3 added to Apple Photos"); + assert_eq!(apple_photos_note(&json!({"imported": 1, "album": "Trip"})), "1 added to Apple Photos album “Trip”"); + let partial = apple_photos_note(&json!({"imported": 1, "requested": 2, "warning": "Photos added 1 of 2 files"})); + assert_eq!(partial, "1 added to Apple Photos (Photos added 1 of 2 files)"); + let denied = apple_photos_note( + &json!({"error": "LightCraft isn't allowed to control Photos. Allow it in System Settings › Privacy & Security › Automation"}), + ); + assert!(denied.starts_with("not added to Apple Photos: ") && denied.contains("Automation"), "{denied}"); + assert_eq!(apple_photos_note(&json!({"skipped": "the export was cancelled"})), "not added to Apple Photos (cancelled)"); + assert_eq!(apple_photos_note(&json!({"skipped": "the export wrote no files"})), "not added to Apple Photos: the export wrote no files"); + } +} diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index d6e2c20d..8cb91b9b 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -247,6 +247,16 @@ pub struct Headless { pub last_cursor: egui::CursorIcon, } +/// A headless run (`lightcraft-cli snapshot`, tests) is a one-shot process: however it ends, it +/// first waits for an Apple Photos import its script left running (`export.addToPhotos {wait: +/// false}`), bounded, so it never abandons osascript or the import's result. +impl Drop for Headless { + fn drop(&mut self) { + #[cfg(not(target_arch = "wasm32"))] + self.app.session.wait_for_photos_imports(); + } +} + impl Headless { /// Wrap `app` (its control channel is replaced by the driver's). /// Invalid dimensions fall back to 1600×1000 at scale 1; external callers can use @@ -1840,6 +1850,276 @@ mod tests { assert!(last["files"][0]["width"].as_u64().is_some_and(|w| w <= 64)); } + /// Issue #236: the Export dialog's Add to Apple Photos is there on macOS only; there the + /// background export hands the files it wrote to Photos (a fake runner: tests never start + /// osascript or Photos) and the toast says how it went. + #[test] + fn export_dialog_adds_to_apple_photos_on_macos() { + let mut h = demo([1200.0, 1100.0]); + let t = Duration::from_secs(10); + let out = std::env::temp_dir().join(format!("lc-photos-dialog-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&out); + h.app.services.write_shared = Some(Arc::new(|p: &str, bytes: &[u8]| lightcraft_engine::export::write_file(p, bytes))); + let calls = Arc::new(std::sync::Mutex::new(Vec::>::new())); + let c = calls.clone(); + h.app.session.apple_photos = Some(Arc::new(move |args: &[String], _| { + c.lock().unwrap().push(args.to_vec()); + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "A/L0/001\nB/L0/001\n".into(), stderr: String::new() }) + })); + let ids: Vec = h.app.session.visible_cloned().iter().take(2).map(|p| p.0).collect(); + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": ids}}), t); + h.request("engine.execute", json!({"command": "dialog.export", "params": {}}), t); + h.settle(SETTLE); + let shown = h.request("ui.widgets", json!({"filter": "exportAddToPhotos"}), t)["result"].as_array().map_or(0, Vec::len); + assert_eq!(shown > 0, cfg!(target_os = "macos"), "the option is offered where Photos is"); + if !cfg!(target_os = "macos") { + return; + } + h.request("ui.clickWidget", json!({"id": "exportAddToPhotos"}), t); + h.settle(SETTLE); + if let Some(crate::state::Dialog::Export { opts, full_size, resize, dir, .. }) = &mut h.app.ui.dialog { + assert!(opts.add_to_photos, "the checkbox turns it on"); + opts.photos_album = "Trip “2026”".into(); + *full_size = false; + *resize = lightcraft_engine::export::Resize::long_edge(48); + *dir = out.to_string_lossy().into_owned(); + } + let r = h.request("ui.dialog.confirm", json!({}), t); + assert_eq!(r["ok"], true, "{r}"); + let t0 = Instant::now(); + while h.app.export.is_some() && t0.elapsed() < Duration::from_secs(60) { + h.step(); + } + let last = h.request("ui.inspect", json!({}), t)["result"]["export"]["last"].clone(); + let written: Vec = last["files"].as_array().unwrap().iter().map(|f| f["path"].as_str().unwrap().to_string()).collect(); + assert_eq!(written.len(), 2, "{last}"); + assert_eq!(last["applePhotos"]["imported"], 2, "{last}"); + assert_eq!(last["applePhotos"]["album"], "Trip “2026”"); + let args = calls.lock().unwrap().clone(); + assert_eq!(args.len(), 1); + assert_eq!(args[0][args[0].len() - 2..], written[..], "the files written go to Photos"); + let toast = h.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default(); + assert!(toast.contains("2 added to Apple Photos album “Trip “2026””"), "{toast}"); + // Export with Previous repeats it + assert_eq!(h.app.session.last_export.as_ref().unwrap()["addToPhotos"], true); + let _ = std::fs::remove_dir_all(&out); + } + + /// Codex review of #643: the progress line while Photos imports was translated on the export's + /// worker thread, whose language is always English. + #[test] + fn the_photos_progress_line_is_in_the_users_language() { + if !cfg!(target_os = "macos") { + return; + } + let mut h = demo([1200.0, 900.0]); + let t = Duration::from_secs(10); + let out = std::env::temp_dir().join(format!("lc-photos-language-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&out); + h.app.ui.language = crate::i18n::Locale::De; + crate::i18n::set_language(crate::i18n::Locale::De); + 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() }) + })); + let ids: Vec = h.app.session.visible_cloned().iter().take(1).map(|p| p.0).collect(); + 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}"); + // 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); + } + + /// Issue #236 review: control requests run on the UI thread, which must not wait minutes for + /// Photos. `export.addToPhotos` and `app.export {addToPhotos}` without `background` answer at + /// once with the running job; the app announces the outcome when Photos is done. + #[test] + fn a_slow_photos_import_never_blocks_the_window() { + let mut h = demo([1000.0, 700.0]); + let t = Duration::from_secs(30); + let out = std::env::temp_dir().join(format!("lc-photos-slow-ui-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&out); + std::fs::create_dir_all(&out).unwrap(); + let file = out.join("a.jpg"); + std::fs::write(&file, b"jpeg").unwrap(); + h.app.services.write = Some(Box::new(|p: &str, bytes: &[u8]| lightcraft_engine::export::write_file(p, bytes))); + let (go, wait) = std::sync::mpsc::channel::<()>(); + let wait = std::sync::Mutex::new(wait); + h.app.session.apple_photos_background = true; // as the desktop app sets it + h.app.session.apple_photos = Some(Arc::new(move |args: &[String], _| { + // a regression (Photos on the UI thread) fails the timing below instead of hanging + let _ = wait.lock().unwrap().recv_timeout(Duration::from_secs(20)); + let n = args.len() - args.iter().position(|a| a == lightcraft_engine::apple_photos::MARKER).unwrap() - 2; + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID\n".repeat(n), stderr: String::new() }) + })); + let t0 = Instant::now(); + let r = + h.request("engine.execute", json!({"command": "export.addToPhotos", "params": {"paths": [file.to_string_lossy()], "album": "Slow"}}), t); + assert!(t0.elapsed() < Duration::from_secs(5), "answered at once: {r}"); + assert_eq!(r["result"]["running"], true, "{r}"); + go.send(()).unwrap(); + while !h.app.ui.toast.as_ref().is_some_and(|t| t.0.contains("Apple Photos")) { + assert!(t0.elapsed() < Duration::from_secs(20), "the outcome is announced"); + h.step(); + } + assert_eq!(h.app.ui.toast.as_ref().unwrap().0, "1 added to Apple Photos album “Slow”"); + if cfg!(target_os = "macos") { + let t1 = Instant::now(); + let p = json!({"dir": out.to_string_lossy(), "longEdge": 32, "addToPhotos": true}); + let r = h.request("engine.execute", json!({"command": "app.export", "params": p}), t); + assert!(t1.elapsed() < Duration::from_secs(10), "the export answers without waiting for Photos: {r}"); + assert_eq!(r["result"]["applePhotos"]["running"], true, "{r}"); + assert_eq!(r["result"]["applePhotos"]["job"], 2, "{r}"); + go.send(()).unwrap(); + while !h.app.ui.toast.as_ref().is_some_and(|t| t.0.starts_with("1 added to Apple Photos") && !t.0.contains("Slow")) { + assert!(t1.elapsed() < Duration::from_secs(20), "the outcome is announced"); + h.step(); + } + } + let _ = std::fs::remove_dir_all(&out); + } + + /// Review of #236: a headless run (snapshot scripts) ending while an import it started with + /// `wait: false` is still running waits for it, on every exit path (dropping the `Headless`). + #[test] + fn a_headless_run_waits_for_its_photos_import_before_ending() { + let mut h = demo([800.0, 600.0]); + let t = Duration::from_secs(10); + let dir = std::env::temp_dir().join(format!("lc-photos-headless-drop-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("a.jpg"); + std::fs::write(&file, b"jpeg").unwrap(); + let marker = dir.join("imported"); + let m = marker.clone(); + // Photos "imports" when `go` arrives (at most 20 s): released 0.8 s into the drop below + let (go, wait) = std::sync::mpsc::channel::<()>(); + let wait = std::sync::Mutex::new(wait); + h.app.session.apple_photos = Some(Arc::new(move |_: &[String], _| { + let _ = wait.lock().unwrap().recv_timeout(Duration::from_secs(20)); + std::fs::write(&m, b"done").unwrap(); + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID\n".into(), stderr: String::new() }) + })); + let r = + h.request("engine.execute", json!({"command": "export.addToPhotos", "params": {"paths": [file.to_string_lossy()], "wait": false}}), t); + assert_eq!(r["result"]["running"], true, "{r}"); + assert!(!marker.exists(), "still importing"); + let release = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(800)); + let _ = go.send(()); + }); + drop(h); + assert!(marker.exists(), "the import finished before the run ended"); + release.join().unwrap(); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Review of #236: a snapshot that ends (an error in its script, say) while its background + /// export is still writing waits through the writing and then the import the export starts. + #[test] + fn a_headless_run_waits_for_its_exports_photos_import() { + if !cfg!(target_os = "macos") { + return; // addToPhotos is refused before exporting elsewhere + } + let mut h = demo([800.0, 600.0]); + let t = Duration::from_secs(10); + let dir = std::env::temp_dir().join(format!("lc-photos-headless-export-drop-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let marker = dir.join("imported"); + h.app.services.write_shared = Some(Arc::new(|p: &str, bytes: &[u8]| { + std::thread::sleep(Duration::from_millis(300)); // a slow disk: still writing at the drop + lightcraft_engine::export::write_file(p, bytes) + })); + let m = marker.clone(); + h.app.session.apple_photos = Some(Arc::new(move |_: &[String], _| { + std::thread::sleep(Duration::from_millis(500)); + std::fs::write(&m, b"done").unwrap(); + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID\n".into(), stderr: String::new() }) + })); + let p = json!({"dir": dir.join("out").to_string_lossy(), "longEdge": 32, "addToPhotos": true, "background": true}); + let r = h.request("engine.execute", json!({"command": "app.export", "params": p}), t); + assert_eq!(r["ok"], true, "{r}"); + assert!(h.app.session.apple_photos_imports.running().is_some(), "reserved while writing"); + drop(h); + assert!(marker.exists(), "the export's import finished before the run ended"); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Review of #236: a background export adding to Photos holds the import from before it + /// writes. While its files are being written, `export.addToPhotos` is refused with the + /// export's job number; the export then adds its files under that job. + #[test] + fn a_background_export_holds_photos_while_it_writes() { + if !cfg!(target_os = "macos") { + return; // addToPhotos is refused before exporting elsewhere + } + let mut h = demo([1000.0, 700.0]); + let t = Duration::from_secs(10); + let out = std::env::temp_dir().join(format!("lc-photos-hold-ui-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&out); + std::fs::create_dir_all(&out).unwrap(); + let other = out.join("other.jpg"); + std::fs::write(&other, b"jpeg").unwrap(); + // the writer waits for `go`: the export is still writing while we try another import + let (go, wait) = std::sync::mpsc::channel::<()>(); + let wait = std::sync::Mutex::new(wait); + h.app.services.write_shared = Some(Arc::new(move |p: &str, bytes: &[u8]| { + let _ = wait.lock().unwrap().recv_timeout(Duration::from_secs(20)); + lightcraft_engine::export::write_file(p, bytes) + })); + let calls = Arc::new(std::sync::Mutex::new(Vec::>::new())); + let c = calls.clone(); + h.app.session.apple_photos_background = true; + h.app.session.apple_photos = Some(Arc::new(move |args: &[String], _| { + c.lock().unwrap().push(args.to_vec()); + Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID\n".into(), stderr: String::new() }) + })); + let p = json!({"dir": out.join("export").to_string_lossy(), "longEdge": 32, "addToPhotos": true, "photosAlbum": "Held", "background": true}); + let r = h.request("engine.execute", json!({"command": "app.export", "params": p}), t); + assert_eq!(r["ok"], true, "{r}"); + assert!(h.app.export.is_some(), "writing in the background"); + let r = h.request("engine.execute", json!({"command": "export.addToPhotos", "params": {"paths": [other.to_string_lossy()]}}), t); + assert_eq!(r["ok"], false, "{r}"); + assert!(r["error"].as_str().unwrap().contains("An export is about to add its files to Apple Photos (import 1)"), "{r}"); + go.send(()).unwrap(); + let t0 = Instant::now(); + while h.app.export.is_some() { + assert!(t0.elapsed() < Duration::from_secs(20), "the export finishes"); + h.step(); + } + let last = h.app.last_export_result.clone().unwrap(); + assert_eq!((last["applePhotos"]["job"].as_u64(), last["applePhotos"]["imported"].as_u64()), (Some(1), Some(1)), "{last}"); + let args = calls.lock().unwrap().clone(); + assert_eq!(args.len(), 1, "only the export's files went to Photos"); + assert!(args[0].last().unwrap().contains("export"), "{:?}", args[0].last()); + let _ = std::fs::remove_dir_all(&out); + } + /// Dragging grid photos onto an album row adds them to the album. #[test] fn drag_photos_onto_an_album() { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8f837a82..9a387c77 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1312,6 +1312,7 @@ impl LightcraftApp { panels::library_problem::show(self, &ctx); export_task::poll(self, &ctx); panels::activity::show(self, &ctx); + export_task::poll_photos(self, &ctx); pick::poll(self, &ctx); panels::grid::drag_feedback(self, &ctx); panels::left::album_drag_feedback(self, &ctx); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 2426efe0..72528fca 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -921,6 +921,27 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { &[(K::Unique, "Add number"), (K::Overwrite, "Overwrite"), (K::Skip, "Skip")], &mut opts.conflict, ); + // After export: Apple Photos imports the files written (macOS only) + #[cfg(target_os = "macos")] + { + let r = ui.checkbox(&mut opts.add_to_photos, crate::i18n::tr("Add to Apple Photos")); + crate::widgets::register(ui.ctx(), "exportAddToPhotos", r.rect); + if opts.add_to_photos { + field(ui, "Album", |ui| { + ui.add(egui::TextEdit::singleline(&mut opts.photos_album).hint_text(crate::i18n::tr("none")).desired_width(f32::INFINITY)) + }); + ui.label( + egui::RichText::new(crate::i18n::tr("The first time, macOS asks whether LightCraft may control Photos.")) + .size(11.0) + .color(t.text_dim), + ); + } + } + // nowhere to add them: a setting carried over from a Mac is dropped, not refused + #[cfg(not(target_os = "macos"))] + { + opts.add_to_photos = false; + } ui.add_space(4.0); field(ui, "Save preset", |ui| { trailing_button_row(ui, |ui| { diff --git a/docs/apple-photos.md b/docs/apple-photos.md new file mode 100644 index 00000000..f0d47c5e --- /dev/null +++ b/docs/apple-photos.md @@ -0,0 +1,169 @@ +# Add to Apple Photos (macOS) + +An export can hand the files it wrote to Apple Photos, optionally into an album (issue #236, parity row +`LR-EXP-PHOTOS`). macOS only: on other platforms the option is hidden in the Export dialog and refused by +`app.export`. The web build doesn't include it. + +> **Tested end to end** on macOS 26.6.2 with Photos 11.0, using the ad-hoc-signed packaged app and a new, empty +> Photos library: the [manual test](#manual-test) below, except the two `lightcraft-cli` steps (11 and 12). Please +> run it again after changing the script or the packaging. + +## Using it + +- **Export dialog:** tick **Add to Apple Photos** under *If file exists* and, if you like, type an **Album** + name (empty = no album; see [Which album](#which-album)). The files are exported as usual, into the folder + you chose, and then added to Photos. The toast says how many Photos took, or why it took none. +- **Export with Previous** and saved export presets repeat it (the setting is part of the export options). +- **Agents and scripts:** `app.export {…, addToPhotos: true, photosAlbum?: "Name"}` (desktop app, MCP `export` + tool, `lightcraft-cli run`); `lightcraft-cli render in.dng -o out.jpg --opt addToPhotos=true --opt + photosAlbum=Name`. The export's result carries `applePhotos` (below). The files are written either way: a + failed Photos step doesn't fail `app.export` (its `applePhotos` has the `error`), while `lightcraft-cli render` + says the file was written and exits with the error. +- **Files already on disk:** `export.addToPhotos {paths: [absolute paths], album?, wait?}`. +- **How an import went:** `export.photosImports` lists this session's imports, oldest first; + `export.photosImports {job}` gives one. + +### Waiting, or not + +An import can take minutes, and the first one waits for the user to answer macOS's permission prompt. Each +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}` 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 + long-running MCP server that polls `export.photosImports`); a process that is about to end (the CLI after + its last command, the MCP server at end of input, a snapshot script, also when they end with an error) + first waits for such an import to finish. The wait follows the import: while an export is still writing the + files it will add, up to an hour more of writing; once Photos is asked, the import's own time limit (see the + table below) and half a minute; never more than three hours and one minute in all. +- **One import at a time.** An export that will add to Photos reserves the import before it writes anything + and holds it until it hands its files over (or is cancelled, fails or writes nothing). While an import is + reserved or running, another (`export.addToPhotos`, or an export asking for Photos, refused before it writes + anything) is refused with the number of the one in the way, so a retry never sends the same files twice and + an export is never turned away after writing its files. When LightCraft stops waiting (see the table below), + Photos may still be importing: look in Photos before trying again. + +### Which album + +The album is looked for by its exact name (case included) among the albums at the **top level** of the +Photos library. Albums inside folders are never used: when the only album of that name is in a folder, a new +top-level album is made. When none is there, a new top-level album is made. When **more than one** top-level +album has the name, nothing is imported and LightCraft says so: rename one of them in Photos, or use another +name. (Photos' own list of albums includes those in folders, in no particular order, so the script checks +each one's parent folder.) + +Photos either copies the files into its library or references them where they are, as set in Photos ▸ +Settings ▸ General ▸ *Copy items to the Photos library*. When it references them, keep the exported files. + +## How it works + +LightCraft runs `/usr/bin/osascript` with a constant AppleScript (`SCRIPT` in +`crates/engine/src/apple_photos.rs`) that asks Photos, through its scripting interface, to import the files: +`import … into album … skip check duplicates false`. The album is found or made first ([Which +album](#which-album)). The script returns the new media items' ids. Files go to Photos 200 at a time (each path is an osascript argument, and +macOS limits a program's arguments to 1 MiB), one run after another within the import's time limit; the first +run makes the album when it is missing. + +This is Apple Events to Photos' own import command. It is not the OS-level synthetic input that AGENTS.md +forbids: nothing is typed or clicked, and no other window is touched. + +The album name and the file paths are never written into the script. They follow it as separate arguments +(`on run argv`), each passed to osascript unchanged, so quotes, backslashes, line breaks or a leading `-` in a +name or path can't change what the script does. A constant first argument keeps osascript from reading a +name that starts with `-` as one of its own options. + +The first time, macOS asks whether LightCraft may control Photos. The answer is kept in System Settings ▸ +Privacy & Security ▸ Automation. macOS asks about the app that started LightCraft's work: the LightCraft app +when it was opened from the Finder or with `open`, but the **terminal** for `lightcraft-cli` and for a +LightCraft started from a terminal (`cargo run`). + +**Packaged app.** The release app is signed with the hardened runtime. For it to be allowed to ask, it carries +the `com.apple.security.automation.apple-events` entitlement (`packaging/macos/entitlements.plist`, also used +for `lightcraft-cli`), and its `Info.plist` has the `NSAppleEventsUsageDescription` text macOS shows in the +prompt (`packaging/macos/Info.plist.in`). Without them macOS refuses the events (`-1743`) without asking. +`cargo run` doesn't test this configuration; the manual test below uses the packaged app. + +**Who can add what.** `export.addToPhotos {paths}` adds any file LightCraft can read, given its absolute path, +not only files it exported. So any client of the control port or the MCP server can put any readable image or +video on the Mac into the user's Photos library, and from there into iCloud Photos when that is on. macOS asks +for the Automation permission once per app, not per import, so after the first Allow nothing asks again. The +control port listens on 127.0.0.1 only and has no authentication ([control-protocol.md](control-protocol.md)), +so any program on this Mac that can connect to it can do this, including programs of other users logged in to +the Mac. The MCP server talks only to the program that started it (stdio). Treat giving an agent the MCP server +or the control port as giving it this too, and turn the control port on only when you need it. + +## When it doesn't work + +| What you see | Why, and what to do | +|---|---| +| "LightCraft isn't allowed to control Photos…" | Permission refused (`-1743`). Turn on Photos under LightCraft (or your terminal app) in System Settings ▸ Privacy & Security ▸ Automation, then try again. | +| "Apple Photos couldn't be found…" | Photos isn't installed or couldn't be opened (`-600`, `-10810`, `-10814`, or no `/System/Applications/Photos.app`). | +| "The import was cancelled in Photos." | Someone pressed Cancel in a Photos dialog (`-128`). | +| "More than one album at the top level of Photos is named …" | Several top-level albums have that name; nothing was added. Rename one in Photos, or use another name. | +| "Photos didn't finish adding the photos within N s…" / "didn't answer in time" | LightCraft waits ten minutes plus five seconds a file (at most two hours), then stops osascript. The ten minutes leave time to answer macOS's permission prompt and Photos' duplicates question. Photos may still be importing: look in Photos before trying again. | +| "Apple Photos is still adding N file(s) from an earlier request (import K)…" / "An export is about to add its files to Apple Photos (import K)…" | One import at a time: wait for import K (`export.photosImports {job: K}`), then try again. | +| "Photos added 2 of 3 files…" | A partial import: Photos skipped files it found to be duplicates (it may ask first) or couldn't read. | +| "Apple Photos is only available on macOS" | The setting came from a Mac (a preset or Export with Previous); the Export dialog drops it on other platforms. | + +## Manual test + +Run this once on a Mac whose Photos library you are happy to add a few test images to (or switch Photos to a +new empty library first: hold ⌥ while opening Photos ▸ Create New). + +1. Package the app (ad-hoc signed unless `MACOS_SIGN_IDENTITY` is set): `packaging/macos/package.sh --arch + aarch64` (or `x86_64` on an Intel Mac). Check its entitlement: `codesign -d --entitlements - + target/macos-package/LightCraft.app` lists `com.apple.security.automation.apple-events`. With a Developer ID + build, also run the notarized DMG from `dist/release/`. +2. Open it from the Finder, or with `open target/macos-package/LightCraft.app --args --demo` (not by running + the binary in a terminal, which makes the terminal the app macOS asks about). +3. Select three photos, choose File ▸ Export…, set a folder (e.g. `~/Pictures/LightCraft Exports`), tick **Add + to Apple Photos**, type the album `LightCraft test "1"` and press OK. +4. macOS asks whether **LightCraft** may control Photos, showing the usage text from `Info.plist`: allow it. + Expected: the toast reads "Exported 3 of 3 photos · 3 added to Apple Photos album “LightCraft test "1"”", + and a top-level album of that name holds the three images. +5. Export the same photos again with the same settings. Expected: Photos asks about duplicates or imports them + again; the toast reports the count Photos returned (a partial import shows its warning). +6. Albums: in Photos make a folder holding an album `Nested`, then export into `Nested`. Expected: a new + top-level album `Nested` gets the images, the one in the folder doesn't. Then make a second top-level album + `Nested` and export again. Expected: nothing is added, and the toast says more than one album has the name. +7. Permission refused: in System Settings ▸ Privacy & Security ▸ Automation turn off Photos under LightCraft, + export again. Expected: the files are written, and the toast says to allow LightCraft there. Turn it back + on. +8. No album: keep Add to Apple Photos ticked, clear the Album field and export. Expected: the images land in the + library, in no album. +9. Hostile names: album `-e`, then `a\b`, and an export folder whose name contains `"` and `'`; a name with a + line break through the CLI: `lightcraft-cli run --demo app.export dir=/tmp/lc-photos addToPhotos=true + "photosAlbum=$(printf 'two\nlines')"`. Expected: albums named exactly so; nothing else happens. +10. The window stays responsive: start the app with `open target/macos-package/LightCraft.app --args --demo + --control 18999`, then send `{"method": "engine.execute", "params": {"command": "export.addToPhotos", + "params": {"paths": ["/tmp/lc-photos/.jpg"]}}}` to port 18999 (`docs/control-protocol.md`). + Expected: an immediate `{"job": …, "running": true}`; the app keeps responding; a toast when Photos is done; + `export.photosImports` then shows the outcome. +11. Agents: `lightcraft-cli run --demo app.export dir=/tmp/lc-photos addToPhotos=true photosAlbum=CLI` (allow + the terminal under Automation when asked). Expected: `applePhotos.imported` in the printed result. +12. Files on disk: `lightcraft-cli run export.addToPhotos 'paths=["/tmp/lc-photos/.jpg"]' album=CLI`. + Expected: the command waits for Photos and prints `{"job": …, "running": false, "imported": 1, …}`. + +Report the toast or JSON, and the macOS and Photos versions. + +## Limitations + +- Albums inside folders are never used ([Which album](#which-album)). +- Photos' duplicate check stays on, so Photos may ask about duplicates; what it skips shows as a partial import. +- Cancelling a running export cancels the Photos step too; a Photos import already started can't be stopped + from LightCraft. +- One import runs (or is reserved by an export) at a time; a session remembers its last 32. +- Quitting the desktop app while Photos is importing doesn't wait for it: Photos may still finish the import, + but LightCraft won't show the outcome. diff --git a/docs/mcp.md b/docs/mcp.md index 7c072e6b..e1fb0737 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -255,6 +255,7 @@ lightcraft-cli run --demo --script steps.jsonl --keep-going lightcraft-cli render in.dng -o out.tif --opt colorSpace=displayP3 --opt bitDepth=16 --opt percent=50 lightcraft-cli render in.dng -o out.jpg --set light.exposure=0.5 --set light.contrast=20 --size 2048 lightcraft-cli render in.jpg -o out.png --settings look.json --preset +lightcraft-cli render in.dng -o out.jpg --opt addToPhotos=true --opt photosAlbum=Trip # macOS: then into Apple Photos lightcraft-cli commands [--json] # the command registry lightcraft-cli controls [--json] # develop control ids and ranges lightcraft-cli calibrate --max 300 ~/Pictures/2026 # camera colour profiles (docs/camera-preview-colour.md) @@ -268,6 +269,23 @@ lightcraft-cli calibrate --max 300 ~/Pictures/2026 # camera colour profiles (d - `apps/lightcraft-cli/tests/cli.rs` — spawns `lightcraft-cli mcp` with real pipes; `render`; `commands`. +## Add to Apple Photos (macOS) + +`app.export` (MCP `export` tool, `lightcraft-cli run`, the desktop app) takes `addToPhotos: true` and +`photosAlbum` to hand the files it wrote to Apple Photos, into that album (a top-level album of exactly that +name, made when missing). The result's `applePhotos` is the import job: `{job, running, requested, album}` +plus `{imported, ids, warning?}`, `{error}` or `{skipped}` once Photos is done; the files are written either +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, 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 +the terminal running `lightcraft-cli`) to control Photos. After that, any client can add any file LightCraft can +read to the user's Photos library (and so to iCloud Photos when that is on), not only exported ones: see [Who +can add what](apple-photos.md#how-it-works). + ## Export progress and cancellation Headless direct `export`, `command_run` / `run_command` with `app.export`, and `cmd_app_export` diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 0d1d4dcf..26a8f396 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -44,7 +44,7 @@ cited here still exists. Formerly docs/parity.md. | P. Enhance (ENH) | 0 | 0 | 2 | 0 | — | — | | Q. HDR (HDR) | 2 | 3 | 0 | 0 | — | — | | R. Video (VID) | 0 | 0 | 5 | 1 | — | 0/2 (0%) | -| S. Export (EXP) | 15 | 2 | 1 | 0 | 7/7 (100%) | 7/7 (100%) | +| S. Export (EXP) | 15 | 3 | 0 | 0 | 7/7 (100%) | 7/7 (100%) | | T. Share (SHARE) | 0 | 0 | 0 | 4 | — | — | | U. Map & location (MAP) | 0 | 1 | 1 | 0 | — | 0/1 (0%) | | V. Preferences (PREF) | 6 | 2 | 2 | 2 | 1/1 (100%) | 4/5 (80%) | @@ -399,7 +399,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EXP-PREVIOUS | Export with previous settings | P0 | ✅ | `cmd:app.exportPrevious`, `cmd:dialog.export` | last options persist in prefs.json; dialog prefilled; no named export presets yet | | LR-EXP-DNGOPT | DNG options | P2 | 🟡 | `cmd:app.export` (`dngCompression`), `crates/engine/src/export.rs` | compression: lossless JPEG (default), ZIP or none; no embedded JPEG preview size, no lossy DNG output | | LR-EXP-ORIGINAL | Original + XMP | P1 | ✅ | `cmd:app.export` (`format: original`), `crates/engine/src/tests_export.rs` | file copied byte for byte, sidecar named after the output and subject to the conflict policy | -| LR-EXP-PHOTOS | Export to the system photo library | P2 | ⬜ | | | +| LR-EXP-PHOTOS | Export to the system photo library | P2 | 🟡 | `cmd:app.export` (`addToPhotos`, `photosAlbum`), `cmd:export.addToPhotos`, `cmd:export.photosImports`, `crates/engine/src/apple_photos.rs`, `crates/ui-egui/src/panels/dialogs.rs`, `packaging/macos/entitlements.plist`, `docs/apple-photos.md` | macOS: the exported files go to Apple Photos through Apple Events to its import command (a constant AppleScript run by `/usr/bin/osascript`; album names and paths are separate arguments), into a top-level album of exactly that name, made when missing (several of that name: refused); the Export dialog's Add to Apple Photos, Export with Previous, presets, MCP and `lightcraft-cli` all reach it; imports are jobs, off the UI thread in the desktop app and awaited in one-shot headless runs, one at a time, reserved by an export before it writes; permission, missing Photos, timeouts and partial imports are reported; the packaged app carries the Apple Events entitlement and usage text. Tested end to end with the packaged app on macOS 26.6.2 / Photos 11.0 (manual steps in `docs/apple-photos.md`). Other platforms' photo libraries are not covered | | LR-EXP-PSD | Round trip to an external editor | P2 | ✅ | `cmd:photo.editExternal`, `cmd:photo.editInPhotoCraft`, `cmd:photo.editInExternal`, `crates/engine/src/cmd/convert.rs`, `apps/lightcraft/src/editors.rs` | an `-Edit` copy of each selected photo (up to 50 at once, issue #920) with the edits (PSD or TIFF, 16- or 8-bit, Adobe RGB / ProPhoto / P3 / sRGB, chosen per editor in Settings ▸ General ▸ External Editing) next to the original, added stacked on top of it and opened in PhotoCraft (found where its installers put it; a PSD by default, which PhotoCraft saves back to in place) or the other editor set there (or the system default); also on the photo's right-click menu (Edit In). Re-read when LightCraft is focused again. No "Edit a copy / Edit original" choice for non-raw files yet | ## T. Share (SHARE) diff --git a/docs/whats-new.md b/docs/whats-new.md index a364df03..fb8b285a 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -101,6 +101,15 @@ Appearance lists the modes and themes. On Linux, Auto follows the desktop portal's colour scheme as it changes, without polling. +### Add to Apple Photos (macOS) +- The Export dialog has **Add to Apple Photos**, with an optional album. The files are exported to your folder as + usual and then added to Photos, into a top-level album of that name (made when it's missing). The toast says how + many Photos added, or why it added none. Export with Previous and export presets repeat it. +- The first time, macOS asks whether LightCraft may control Photos. Photos may ask about duplicates; files it skips + show as a partial import. +- Agents: `app.export {addToPhotos, photosAlbum}`, `export.addToPhotos {paths}` for files already on disk, and + `export.photosImports`. See [Add to Apple Photos](apple-photos.md). + ### Activity stack - Long-running tasks show in one place, top left under the top bar, as in Lightroom Classic (issue #345): imports and folder scans, exports (and the contact sheet PDF), Synchronize Folder, Build / Smart Previews, Lightroom catalog import, Photo Merge, model downloads, the face scan, diff --git a/packaging/macos/Info.plist.in b/packaging/macos/Info.plist.in index f571d18e..03220ab0 100644 --- a/packaging/macos/Info.plist.in +++ b/packaging/macos/Info.plist.in @@ -42,6 +42,10 @@ NSRequiresAquaSystemAppearance + + NSAppleEventsUsageDescription + LightCraft asks Photos to import the photos you export when you choose Add to Apple Photos in the Export dialog. NSHumanReadableCopyright Copyright © the LightCraft authors. MIT OR Apache-2.0. diff --git a/packaging/macos/entitlements.plist b/packaging/macos/entitlements.plist index 8b8cd128..5771451c 100644 --- a/packaging/macos/entitlements.plist +++ b/packaging/macos/entitlements.plist @@ -1,9 +1,15 @@ - + + com.apple.security.automation.apple-events + diff --git a/packaging/macos/package.sh b/packaging/macos/package.sh index c6fd5e09..b26aa96d 100755 --- a/packaging/macos/package.sh +++ b/packaging/macos/package.sh @@ -210,7 +210,10 @@ CLI_DIR="$WORK/lightcraft-cli-$VERSION-macos-$ARCH" mkdir -p "$CLI_DIR" cp "$WORK/bin/lightcraft-cli" "$CLI_DIR/" copy_docs "$CLI_DIR" -sign --options runtime "$CLI_DIR/lightcraft-cli" +# Same entitlements as the app: `--opt addToPhotos=true` / `app.export {addToPhotos}` send Apple +# Events to Photos through osascript too (macOS asks about the app that started the CLI, usually +# the terminal). +sign --options runtime --entitlements "$HERE/entitlements.plist" "$CLI_DIR/lightcraft-cli" codesign --verify --strict --verbose=2 "$CLI_DIR/lightcraft-cli" rm -f "$CLI_ZIP" ditto -c -k --keepParent "$CLI_DIR" "$CLI_ZIP"