From a6431c152d169ab330948b219555e48a485f2111 Mon Sep 17 00:00:00 2001 From: Alex Chambers Date: Sat, 10 Oct 2026 05:07:57 +0200 Subject: [PATCH 1/3] LR-EXP-PHOTOS: add exported files to Apple Photos (macOS, #236) The Export dialog gets "Add to Apple Photos" with an optional album (macOS only). app.export takes addToPhotos / photosAlbum (dialog, Export with Previous, presets, MCP, lightcraft-cli run and render --opt); the new export.addToPhotos {paths, album?, wait?} adds files already on disk and export.photosImports {job?} reports how imports went. A constant AppleScript run by /usr/bin/osascript asks Photos' import command (Apple Events, not synthetic input) to import the files into a top-level album of exactly that name (made when missing; albums in folders are never used; several of that name: refused), skip check duplicates false, and returns the media item ids. Album names and paths are passed only as separate arguments (on run argv), never spliced into the script. The runner is injectable (Session::apple_photos): with_fs installs the real one on macOS only, and tests use fakes, so no test starts osascript or Photos. Imports are jobs (Session::apple_photos_imports), one at a time. The desktop app (Session::apple_photos_background, set in open_session) never waits for Photos on its UI thread: the background export adds on its worker, app.export without background and export.addToPhotos answer with the running job, and the app announces the outcome. Without a window (MCP, lightcraft-cli) imports wait by default, and both headless front ends (the MCP/CLI backend and the snapshot UI) wait for a running one before the process ends, on every exit path, through one engine helper (Session::wait_for_photos_imports) whose deadline follows the job: a writing allowance while an export still writes, then the import's own time limit from when Photos was asked, under an overall cap. A runner that panics, waiting or on a worker, ends its import as failed and frees the slot (panic guard plus a completion guard). Session::after_export reserves the import slot before an export writes anything and frees it on every exit path (handed the files, cancelled, failed, dropped), so a competing import is refused with the export's job number instead of the export failing its Photos step after writing. Permission (-1743), missing Photos, cancel, timeouts, ambiguous albums, busy imports, other osascript failures and partial imports produce actionable messages; the export's files are kept either way. Hidden on other platforms; the web build compiles it out. Packaging: the hardened-runtime entitlements gain com.apple.security.automation.apple-events (app and CLI) and Info.plist gains NSAppleEventsUsageDescription, without which macOS refuses the events without asking. Not yet tested end to end against a real Photos library: manual steps, with the packaged app, in docs/apple-photos.md. Parity row LR-EXP-PHOTOS is now partial. Co-Authored-By: Claude Opus 5.5 --- apps/lightcraft-cli/src/main.rs | 10 + apps/lightcraft/src/main.rs | 12 + crates/engine/src/apple_photos.rs | 1113 +++++++++++++++++++ crates/engine/src/cmd/export.rs | 88 +- crates/engine/src/export.rs | 143 ++- crates/engine/src/files.rs | 7 +- crates/engine/src/lib.rs | 22 + crates/engine/src/tests_export.rs | 280 +++++ crates/mcp/src/headless.rs | 13 +- crates/mcp/src/tests.rs | 93 ++ crates/mcp/src/tools.rs | 4 + crates/ui-egui/locales/de-formats.json | 5 +- crates/ui-egui/locales/de.json | 6 +- crates/ui-egui/locales/es-formats.json | 5 +- crates/ui-egui/locales/es.json | 6 +- crates/ui-egui/locales/fr-formats.json | 5 +- crates/ui-egui/locales/fr.json | 6 +- crates/ui-egui/locales/ja-formats.json | 5 +- crates/ui-egui/locales/ja.json | 6 +- crates/ui-egui/locales/pt-br-formats.json | 5 +- crates/ui-egui/locales/pt-br.json | 6 +- crates/ui-egui/locales/ru-formats.json | 5 +- crates/ui-egui/locales/ru.json | 6 +- crates/ui-egui/locales/uk-formats.json | 5 +- crates/ui-egui/locales/uk.json | 6 +- crates/ui-egui/locales/zh-hans-formats.json | 5 +- crates/ui-egui/locales/zh-hans.json | 6 +- crates/ui-egui/locales/zh-hant-formats.json | 5 +- crates/ui-egui/locales/zh-hant.json | 6 +- crates/ui-egui/src/control.rs | 14 +- crates/ui-egui/src/export_task.rs | 106 +- crates/ui-egui/src/headless.rs | 230 ++++ crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/panels/dialogs.rs | 21 + docs/apple-photos.md | 159 +++ docs/mcp.md | 18 + docs/parity-checklist.md | 4 +- packaging/macos/Info.plist.in | 4 + packaging/macos/entitlements.plist | 8 +- packaging/macos/package.sh | 5 +- 40 files changed, 2416 insertions(+), 38 deletions(-) create mode 100644 crates/engine/src/apple_photos.rs create mode 100644 docs/apple-photos.md diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index 592a534a..b8ce7e2e 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -62,6 +62,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: @@ -681,6 +683,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)> = @@ -695,6 +699,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 09cb1434..19e19206 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -583,6 +583,8 @@ fn services(ctx: egui::Context, log_file: Option<&std::path::Path>) -> Services 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) } @@ -902,6 +904,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/apple_photos.rs b/crates/engine/src/apple_photos.rs new file mode 100644 index 00000000..6e1b6a16 --- /dev/null +++ b/crates/engine/src/apple_photos.rs @@ -0,0 +1,1113 @@ +//! 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}; + +/// 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: a minute, plus five seconds a file, at most two hours (the first +/// import also waits for the user to answer the permission prompt). +pub fn timeout_for(files: usize) -> Duration { + Duration::from_secs(60u64.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()) +} + +/// Run osascript for a checked request (`album` trimmed; see [`check`]). +fn execute(runner: &Runner, paths: &[String], album: &str) -> Result { + let out = runner(&osascript_args(album, paths), timeout_for(paths.len())).map_err(|e| explain_run_error(&e))?; + if out.status != Some(0) { + return Err(explain(&out, album)); + } + Ok(Imported { ids: parse_ids(&out.stdout), 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) }) + } + + /// [`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 + } +} + +/// Ask Photos to import `paths` (checked; `album` trimmed) for `job`: here, or on a worker thread. +fn launch(runner: &Runner, job: &Arc, paths: Vec, album: String, background: bool) { + 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 { + run_to_completion(runner, job, &paths, &album); + return; + } + let (r, j) = (runner.clone(), job.clone()); + let started = std::thread::Builder::new().name("apple-photos".into()).spawn(move || run_to_completion(&r, &j, &paths, &album)); + 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). 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) -> Result, String> { + let album = check(&paths, album)?; + let job = imports.begin(paths.len(), &album, false)?; + job.quiet.store(!background, Ordering::Relaxed); + launch(runner, &job, paths, album, background); + 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>, +} + +impl Reservation { + 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); + 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); + launch(&self.runner, &job, paths, self.album.clone(), true); + 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")); + let (runner, _) = fake(Err(RunError::Timeout(Duration::from_secs(70)))); + assert!(import(&runner, &paths, "x").unwrap_err().contains("70 s")); + let _ = std::fs::remove_dir_all(dir); + } + + #[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).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).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).unwrap(); + assert!(job.finished() && stopped(&job), "{}", job.json()); + assert!(imports.running().is_none()); + // on a worker + let job = import_job(&panicky, &imports, paths.clone(), "", true).unwrap(); + let 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).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).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).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).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).unwrap(); + assert!(t0.elapsed() < Duration::from_secs(5), "the caller doesn't wait for Photos"); + assert_eq!(job.json(), json!({"job": 1, "running": true, "requested": 2, "album": "Trip"})); + assert_eq!(imports.running().map(|j| j.id), Some(1)); + let e = import_job(&runner, &imports, paths.clone(), "", true).unwrap_err(); + 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).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).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); + } + + #[test] + fn timeouts_grow_with_the_batch_and_stay_bounded() { + assert_eq!(timeout_for(1), Duration::from_secs(65)); + assert_eq!(timeout_for(100), Duration::from_secs(560)); + 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..dafcb9a3 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -78,8 +78,69 @@ 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, + Some(Value::Bool(b)) => *b, + Some(_) => return Err(bad(ID, "`wait` must be true or false")), + }; + let job = crate::apple_photos::import_job(&runner, &s.apple_photos_imports, paths, album, !wait).map_err(|e| bad(ID, e))?; + 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 +184,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 (its control requests are answered on the UI thread). macOS: asks Photos to import the files (the first time, macOS asks the user to allow LightCraft to control Photos). One import runs at a time, and an export adding to Photos reserves it before writing: another is refused until it ends. app.export does this for the files it writes with `addToPhotos: true`", + 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 6a9e9147..8bbfbead 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -605,6 +605,10 @@ pub struct ExportOptions { /// not applied), AVIF as 10-bit Rec. 2020 PQ, 32-bit float TIFF with the highlights above SDR /// white kept. Other formats, and photos without an HDR edit, are SDR. pub hdr: 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 { @@ -629,6 +633,8 @@ impl Default for ExportOptions { color_space: OutputSpace::Srgb, bit_depth: None, hdr: false, + add_to_photos: false, + photos_album: String::new(), } } } @@ -665,8 +671,16 @@ pub const OPTION_PARAMS: &[&str] = &[ "colorSpace", "bitDepth", "hdr", + "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). pub const WATERMARK_PARAMS: &[&str] = &["text", "vertical", "size", "opacity", "anchor", "inset", "color", "shadow", "image", "imageWidth"]; @@ -799,6 +813,18 @@ impl ExportOptions { "dontEnlarge" | "removeLocation" | "background" | "hdr" => { 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)?; } @@ -884,7 +910,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). @@ -930,6 +957,8 @@ impl ExportOptions { color_space: s("colorSpace").and_then(OutputSpace::parse).unwrap_or(d.color_space), bit_depth: u("bitDepth").filter(|b| matches!(b, 8 | 10 | 16 | 32)).map(|b| b as u8), hdr: p.get("hdr").and_then(Value::as_bool).unwrap_or(d.hdr), + 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()), } } @@ -1547,6 +1576,102 @@ 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())?; + AfterExport::apple_photos(runner, &o.photos_album, &self.apple_photos_imports) + } + #[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, @@ -2497,6 +2622,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 31253f66..5b3f293c 100644 --- a/crates/engine/src/files.rs +++ b/crates/engine/src/files.rs @@ -660,13 +660,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 66acfe4a..327d4215 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; @@ -276,6 +278,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 { @@ -355,6 +371,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 b8636c01..7969b799 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -440,3 +440,283 @@ fn hdr_avif_export_is_pq_rec2020() { // a photo without an HDR edit exports as an ordinary (sRGB) AVIF assert!(nclx(&sdr.bytes).is_none_or(|c| c[3] != 16)); } + +/// 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")); + // finished: the next one may start + go.send(()).unwrap(); + assert_eq!(s.execute("export.addToPhotos", &json!({"paths": paths, "wait": true})).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 acbecb60..459fbbed 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(); } } @@ -83,6 +87,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(); @@ -107,6 +113,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) } @@ -131,7 +140,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/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), dontEnlarge?, ppi?, quality?: 1..100, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, hdr? (HDR output for HDR edits: gain map JPEG, 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, inset? (margin, 0..0.4 of the short edge; 0.025), 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/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), dontEnlarge?, ppi?, quality?: 1..100, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, hdr? (HDR output for HDR edits: gain map JPEG, 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, inset? (margin, 0..0.4 of the short edge; 0.025), 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 dd52da40..9e5ab90a 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -259,6 +259,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). 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), AVIF 8|10; 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 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), inset (margin as a fraction of the short edge, 0..0.4; default 0.025), 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"} }), &[], @@ -745,6 +747,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 671da17d..cbb4ddab 100644 --- a/crates/ui-egui/locales/de-formats.json +++ b/crates/ui-egui/locales/de-formats.json @@ -228,5 +228,8 @@ "+{n} more": "+{n} weitere", "1 task is still running: {names}. Quitting stops it.": "1 Aufgabe läuft noch: {names}. Beenden bricht sie ab.", "{n} tasks are still running: {names}. Quitting stops them.": "{n} Aufgaben laufen noch: {names}. Beenden bricht sie ab.", - "{what} merge cancelled": "{what}-Zusammenfügung abgebrochen" + "{what} merge cancelled": "{what}-Zusammenfügung abgebrochen", + "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 6520fb3e..bd97f5ea 100644 --- a/crates/ui-egui/locales/de.json +++ b/crates/ui-egui/locales/de.json @@ -2039,5 +2039,9 @@ "Downloading model": "Modell wird heruntergeladen", "Finding faces": "Gesichter werden gesucht", "Synchronizing folder": "Ordner wird synchronisiert", - "Looking for changes": "Änderungen werden gesucht" + "Looking for changes": "Änderungen werden gesucht", + "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." } diff --git a/crates/ui-egui/locales/es-formats.json b/crates/ui-egui/locales/es-formats.json index 9ba1c8f1..ec8e037d 100644 --- a/crates/ui-egui/locales/es-formats.json +++ b/crates/ui-egui/locales/es-formats.json @@ -228,5 +228,8 @@ "+{n} more": "+{n} más", "1 task is still running: {names}. Quitting stops it.": "1 tarea sigue en curso: {names}. Salir la detiene.", "{n} tasks are still running: {names}. Quitting stops them.": "{n} tareas siguen en curso: {names}. Salir las detiene.", - "{what} merge cancelled": "Combinación {what} cancelada" + "{what} merge cancelled": "Combinación {what} cancelada", + "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 fc53b639..c4c6e1c5 100644 --- a/crates/ui-egui/locales/es.json +++ b/crates/ui-egui/locales/es.json @@ -1884,5 +1884,9 @@ "Downloading model": "Descargando modelo", "Finding faces": "Buscando caras", "Synchronizing folder": "Sincronizando carpeta", - "Looking for changes": "Buscando cambios" + "Looking for changes": "Buscando cambios", + "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." } diff --git a/crates/ui-egui/locales/fr-formats.json b/crates/ui-egui/locales/fr-formats.json index 7ccaf5a3..54e5026a 100644 --- a/crates/ui-egui/locales/fr-formats.json +++ b/crates/ui-egui/locales/fr-formats.json @@ -228,5 +228,8 @@ "+{n} more": "+{n} autres", "1 task is still running: {names}. Quitting stops it.": "1 tâche est encore en cours : {names}. Quitter l'arrête.", "{n} tasks are still running: {names}. Quitting stops them.": "{n} tâches sont encore en cours : {names}. Quitter les arrête.", - "{what} merge cancelled": "Fusion {what} annulée" + "{what} merge cancelled": "Fusion {what} annulée", + "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 53509a50..503b8cba 100644 --- a/crates/ui-egui/locales/fr.json +++ b/crates/ui-egui/locales/fr.json @@ -1884,5 +1884,9 @@ "Panels": "Panels", "Reset Panel Layout": "Reset Panel Layout", "Resize panel window": "Resize panel window", - "Resize panels": "Resize panels" + "Resize panels": "Resize panels", + "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." } diff --git a/crates/ui-egui/locales/ja-formats.json b/crates/ui-egui/locales/ja-formats.json index f1c06351..941e21f1 100644 --- a/crates/ui-egui/locales/ja-formats.json +++ b/crates/ui-egui/locales/ja-formats.json @@ -228,5 +228,8 @@ "+{n} more": "ほか{n}件", "1 task is still running: {names}. Quitting stops it.": "1件のタスクが実行中です：{names}。終了すると停止します。", "{n} tasks are still running: {names}. Quitting stops them.": "{n}件のタスクが実行中です：{names}。終了すると停止します。", - "{what} merge cancelled": "{what}合成をキャンセルしました" + "{what} merge cancelled": "{what}合成をキャンセルしました", + "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 2babd30b..b5b891d0 100644 --- a/crates/ui-egui/locales/ja.json +++ b/crates/ui-egui/locales/ja.json @@ -1886,5 +1886,9 @@ "Downloading model": "モデルをダウンロード中", "Finding faces": "顔を検索中", "Synchronizing folder": "フォルダを同期中", - "Looking for changes": "変更を確認中" + "Looking for changes": "変更を確認中", + "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が確認します。" } diff --git a/crates/ui-egui/locales/pt-br-formats.json b/crates/ui-egui/locales/pt-br-formats.json index 4a9d1168..90decfd4 100644 --- a/crates/ui-egui/locales/pt-br-formats.json +++ b/crates/ui-egui/locales/pt-br-formats.json @@ -228,5 +228,8 @@ "+{n} more": "+{n} mais", "1 task is still running: {names}. Quitting stops it.": "1 tarefa ainda está em andamento: {names}. Sair a interrompe.", "{n} tasks are still running: {names}. Quitting stops them.": "{n} tarefas ainda estão em andamento: {names}. Sair as interrompe.", - "{what} merge cancelled": "{what} mesclagem cancelada" + "{what} merge cancelled": "{what} mesclagem cancelada", + "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 dfe47281..698a274d 100644 --- a/crates/ui-egui/locales/pt-br.json +++ b/crates/ui-egui/locales/pt-br.json @@ -1884,5 +1884,9 @@ "Downloading model": "Baixando modelo", "Finding faces": "Procurando rostos", "Synchronizing folder": "Sincronizando pasta", - "Looking for changes": "Procurando alterações" + "Looking for changes": "Procurando alterações", + "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." } diff --git a/crates/ui-egui/locales/ru-formats.json b/crates/ui-egui/locales/ru-formats.json index ba3560ad..6e1e32c3 100644 --- a/crates/ui-egui/locales/ru-formats.json +++ b/crates/ui-egui/locales/ru-formats.json @@ -228,5 +228,8 @@ "+{n} more": "ещё {n}", "1 task is still running: {names}. Quitting stops it.": "Ещё выполняется задача: {names}. Выход прервёт её.", "{n} tasks are still running: {names}. Quitting stops them.": "Ещё выполняется задач: {n} — {names}. Выход прервёт их.", - "{what} merge cancelled": "{what}: слияние отменено" + "{what} merge cancelled": "{what}: слияние отменено", + "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 fafb65a1..364f07ba 100644 --- a/crates/ui-egui/locales/ru.json +++ b/crates/ui-egui/locales/ru.json @@ -1598,5 +1598,9 @@ "Finding faces": "Поиск лиц", "Quit Anyway": "Всё равно выйти", "Synchronizing folder": "Синхронизация папки", - "Looking for changes": "Поиск изменений" + "Looking for changes": "Поиск изменений", + "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 управлять приложением «Фото»." } diff --git a/crates/ui-egui/locales/uk-formats.json b/crates/ui-egui/locales/uk-formats.json index c33dd0cb..16acf427 100644 --- a/crates/ui-egui/locales/uk-formats.json +++ b/crates/ui-egui/locales/uk-formats.json @@ -228,5 +228,8 @@ "+{n} more": "+{n} ще", "1 task is still running: {names}. Quitting stops it.": "1 завдання ще виконується: {names}. Вихід зупинить його.", "{n} tasks are still running: {names}. Quitting stops them.": "Ще виконуються завдання ({n}): {names}. Вихід зупинить їх.", - "{what} merge cancelled": "Об’єднання {what} скасовано" + "{what} merge cancelled": "Об’єднання {what} скасовано", + "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 7ebad5b1..b76dea8f 100644 --- a/crates/ui-egui/locales/uk.json +++ b/crates/ui-egui/locales/uk.json @@ -2078,5 +2078,9 @@ "Panels": "Panels", "Reset Panel Layout": "Reset Panel Layout", "Resize panel window": "Resize panel window", - "Resize panels": "Resize panels" + "Resize panels": "Resize panels", + "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 керувати програмою «Фото»." } diff --git a/crates/ui-egui/locales/zh-hans-formats.json b/crates/ui-egui/locales/zh-hans-formats.json index eb429af4..97ee818a 100644 --- a/crates/ui-egui/locales/zh-hans-formats.json +++ b/crates/ui-egui/locales/zh-hans-formats.json @@ -228,5 +228,8 @@ "+{n} more": "还有 {n} 项", "1 task is still running: {names}. Quitting stops it.": "仍有 1 个任务在运行：{names}。退出将停止它。", "{n} tasks are still running: {names}. Quitting stops them.": "仍有 {n} 个任务在运行：{names}。退出将停止它们。", - "{what} merge cancelled": "已取消 {what} 合并" + "{what} merge cancelled": "已取消 {what} 合并", + "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 8a0e58b8..b38b91b2 100644 --- a/crates/ui-egui/locales/zh-hans.json +++ b/crates/ui-egui/locales/zh-hans.json @@ -1884,5 +1884,9 @@ "Downloading model": "正在下载模型", "Finding faces": "正在查找人脸", "Synchronizing folder": "正在同步文件夹", - "Looking for changes": "正在查找更改" + "Looking for changes": "正在查找更改", + "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 控制“照片”。" } diff --git a/crates/ui-egui/locales/zh-hant-formats.json b/crates/ui-egui/locales/zh-hant-formats.json index 0b2ec765..9fae26a6 100644 --- a/crates/ui-egui/locales/zh-hant-formats.json +++ b/crates/ui-egui/locales/zh-hant-formats.json @@ -228,5 +228,8 @@ "+{n} more": "還有 {n} 項", "1 task is still running: {names}. Quitting stops it.": "仍有 1 個工作在執行：{names}。結束將停止它。", "{n} tasks are still running: {names}. Quitting stops them.": "仍有 {n} 個工作在執行：{names}。結束將停止它們。", - "{what} merge cancelled": "已取消{what}合併" + "{what} merge cancelled": "已取消{what}合併", + "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 f1cbec3a..073d9f8b 100644 --- a/crates/ui-egui/locales/zh-hant.json +++ b/crates/ui-egui/locales/zh-hant.json @@ -1884,5 +1884,9 @@ "Downloading model": "正在下載模型", "Finding faces": "正在尋找臉孔", "Synchronizing folder": "正在同步資料夾", - "Looking for changes": "正在尋找變更" + "Looking for changes": "正在尋找變更", + "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 控制「照片」。" } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index a48e74cd..6c320f1a 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -472,13 +472,23 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result>, pub cancel: Arc, - rx: Receiver, String>>, + /// The files (per-photo results) and what [`AfterExport`] did with them (`applePhotos`). + rx: Receiver, Option), String>>, /// The export's row in the activity stack (`activity.cancel` sets `cancel`). guard: TaskGuard, } @@ -35,8 +36,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()); } @@ -54,6 +56,15 @@ pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOp } !c.load(Ordering::Relaxed) }); + let r = r.map(|files| { + if !after.is_none() + && let Ok(mut g) = p.lock() + { + g.1 = crate::i18n::tr("Adding to Apple Photos…").to_string(); + } + let photos = after.run(&files, c.load(Ordering::Relaxed)); + (files, photos) + }); let _ = tx.send(r); }; #[cfg(not(target_arch = "wasm32"))] @@ -100,7 +111,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 }; @@ -120,8 +179,10 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) { let cancelled = task.cancel.load(Ordering::Relaxed); 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 +201,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 +317,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 c423bc35..ba533d4f 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -234,6 +234,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 @@ -1804,6 +1814,226 @@ 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); + } + + /// 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 f853fe16..0a711958 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1048,6 +1048,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 aa287c95..fd43b6a4 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -731,6 +731,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..b61ce04c --- /dev/null +++ b/docs/apple-photos.md @@ -0,0 +1,159 @@ +# 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. + +> **Status: not yet tested end to end.** The code and its tests (which use a stand-in for osascript) are in +> place, but no import into a real Photos library has been run yet, and the AppleScript has never been run. +> Please try the [manual test](#manual-test) below, with the packaged app, and report what happens. + +## 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). A failed Photos step does not fail + the export: the files are written either way. +- **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). + +- In the **desktop app** nothing waits for Photos on the window's thread. The Export dialog's background export + adds the files on its worker and reports them in its toast. `app.export` without `background` and + `export.addToPhotos` answer at once with the running job (`running: true`); the app shows a toast when Photos + is done, and `export.photosImports {job}` gives the outcome. (`export.addToPhotos {wait: true}` waits there + too, holding the window until Photos is done.) +- **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. + +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)), +and the MCP server answers the program that started it, so this reaches programs already running as the user. +Treat giving an agent the MCP server or the control port as giving it this too. + +## 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 a minute plus five seconds a file (at most two hours), then stops osascript. 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 81cbc4bd..01fdcf9e 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; 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 09941108..77c9fc38 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%) | @@ -397,7 +397,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. **Not yet tested end to end** against a real Photos library (manual steps, with the packaged app, 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.editInExternal`, `crates/engine/src/cmd/convert.rs` | a 16-bit TIFF `-Edit` copy with the edits (Adobe RGB / ProPhoto / P3 / sRGB) next to the original, added stacked on top of it and opened in the editor set in Settings ▸ General (or the system default) | ## T. Share (SHARE) 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 b43166f1..b21f7e57 100755 --- a/packaging/macos/package.sh +++ b/packaging/macos/package.sh @@ -158,7 +158,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" From c722e6bb64a2f6768570b3f77bb71e476a032ddd Mon Sep 17 00:00:00 2001 From: Alex Chambers Date: Sat, 10 Oct 2026 10:14:33 +0200 Subject: [PATCH 2/3] LR-EXP-PHOTOS: Apple Photos fixes from the live test and review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Ten minutes plus five seconds a file per import (was one minute): the first live run timed out while macOS's permission prompt was still up, and Photos' duplicates question waits for a person too. - Files go to osascript 200 at a time, all within the import's one time limit: one run with every path went over macOS's 1 MiB argument limit at about 10,000 files, and Photos got nothing. A failed later batch says how many files Photos had added. - The progress line "Adding to Apple Photos…" is translated on the UI thread; on the export worker it was always English. - docs/apple-photos.md: who can add what (the control port is reachable by any local program, other users' included), batching, the new limit, and tested end to end (macOS 26.6.2, Photos 11.0, packaged app). The parity row says so; What's New has an entry. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/apple_photos.rs | 105 ++++++++++++++++++++++++++---- crates/ui-egui/src/export_task.rs | 4 +- crates/ui-egui/src/headless.rs | 41 ++++++++++++ docs/apple-photos.md | 22 ++++--- docs/parity-checklist.md | 2 +- docs/whats-new.md | 9 +++ 6 files changed, 161 insertions(+), 22 deletions(-) diff --git a/crates/engine/src/apple_photos.rs b/crates/engine/src/apple_photos.rs index 6e1b6a16..f59468b5 100644 --- a/crates/engine/src/apple_photos.rs +++ b/crates/engine/src/apple_photos.rs @@ -203,10 +203,12 @@ pub fn explain_run_error(e: &RunError) -> String { } } -/// How long one import may take: a minute, plus five seconds a file, at most two hours (the first -/// import also waits for the user to answer the permission prompt). +/// 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(60u64.saturating_add(5u64.saturating_mul(files as u64)).min(7200)) + Duration::from_secs(600u64.saturating_add(5u64.saturating_mul(files as u64)).min(7200)) } /// What an import gave. @@ -258,13 +260,37 @@ pub fn check(paths: &[String], album: &str) -> Result { Ok(album.to_string()) } -/// Run osascript for a checked request (`album` trimmed; see [`check`]). +/// 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 out = runner(&osascript_args(album, paths), timeout_for(paths.len())).map_err(|e| explain_run_error(&e))?; - if out.status != Some(0) { - return Err(explain(&out, album)); + 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: parse_ids(&out.stdout), requested: paths.len(), album: (!album.is_empty()).then(|| album.to_string()) }) + 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, @@ -845,11 +871,68 @@ mod tests { // 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("70 s")); + 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); @@ -1106,8 +1189,8 @@ mod tests { #[test] fn timeouts_grow_with_the_batch_and_stay_bounded() { - assert_eq!(timeout_for(1), Duration::from_secs(65)); - assert_eq!(timeout_for(100), Duration::from_secs(560)); + 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/ui-egui/src/export_task.rs b/crates/ui-egui/src/export_task.rs index cedd4606..b6de2f41 100644 --- a/crates/ui-egui/src/export_task.rs +++ b/crates/ui-egui/src/export_task.rs @@ -48,6 +48,8 @@ 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()); + // 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| { @@ -60,7 +62,7 @@ pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOp if !after.is_none() && let Ok(mut g) = p.lock() { - g.1 = crate::i18n::tr("Adding to Apple Photos…").to_string(); + g.1 = adding; } let photos = after.run(&files, c.load(Ordering::Relaxed)); (files, photos) diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index ba533d4f..1bf18021 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1869,6 +1869,47 @@ mod tests { 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); + h.app.session.apple_photos = Some(Arc::new(move |_: &[String], _| { + 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}"); + let t0 = Instant::now(); + let current = loop { + h.step(); + let current = h.app.export.as_ref().map(|e| e.progress.lock().unwrap().1.clone()).unwrap_or_default(); + if current.contains("Apple") || t0.elapsed() > Duration::from_secs(60) { + break current; + } + }; + assert_eq!(current, "Wird zu Apple Fotos hinzugefügt…"); + release.send(()).unwrap(); + while h.app.export.is_some() && t0.elapsed() < Duration::from_secs(90) { + h.step(); + } + 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. diff --git a/docs/apple-photos.md b/docs/apple-photos.md index b61ce04c..1fc50f65 100644 --- a/docs/apple-photos.md +++ b/docs/apple-photos.md @@ -4,9 +4,9 @@ An export can hand the files it wrote to Apple Photos, optionally into an album `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. -> **Status: not yet tested end to end.** The code and its tests (which use a stand-in for osascript) are in -> place, but no import into a real Photos library has been run yet, and the AppleScript has never been run. -> Please try the [manual test](#manual-test) below, with the packaged app, and report what happens. +> **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 @@ -16,8 +16,9 @@ An export can hand the files it wrote to Apple Photos, optionally into an album - **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). A failed Photos step does not fail - the export: the files are written either way. + 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. @@ -65,7 +66,9 @@ Settings ▸ General ▸ *Copy items to the Photos library*. When it references 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. +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. @@ -91,8 +94,9 @@ not only files it exported. So any client of the control port or the MCP server 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)), -and the MCP server answers the program that started it, so this reaches programs already running as the user. -Treat giving an agent the MCP server or the control port as giving it this too. +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 @@ -102,7 +106,7 @@ Treat giving an agent the MCP server or the control port as giving it this too. | "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 a minute plus five seconds a file (at most two hours), then stops osascript. Photos may still be importing: look in Photos before trying again. | +| "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. | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 77c9fc38..cce814bd 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -397,7 +397,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 | 🟡 | `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. **Not yet tested end to end** against a real Photos library (manual steps, with the packaged app, in `docs/apple-photos.md`). Other platforms' photo libraries are not covered | +| 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.editInExternal`, `crates/engine/src/cmd/convert.rs` | a 16-bit TIFF `-Edit` copy with the edits (Adobe RGB / ProPhoto / P3 / sRGB) next to the original, added stacked on top of it and opened in the editor set in Settings ▸ General (or the system default) | ## T. Share (SHARE) diff --git a/docs/whats-new.md b/docs/whats-new.md index f3fb3a14..629dec18 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,15 @@ ## 2026-10-10 +### 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, From 484c160c13a43025bbb328903e5bdb6f98858119 Mon Sep 17 00:00:00 2001 From: Alex Chambers Date: Sat, 10 Oct 2026 12:11:20 +0200 Subject: [PATCH 3/3] LR-EXP-PHOTOS: Apple Photos imports in the activity stack (#345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto the activity stack (#601). An export adding its files to Photos keeps its row through that step, with "Adding to Apple Photos…" as the detail. The worker turns the row's ✕ off (TaskHandle::set_cancellable) before its last look at the cancel flag and reports what it saw, so a cancel either stops the export before Photos is asked or is refused, never both "cancelled" and imported. activity: a per-task gate orders a cancel's decision against a step switching cancelling off, so a cancel can't be accepted between the switch and the step's look at the flag and then be ignored. An import of its own (export.addToPhotos, or the Photos step left running after a foreground app.export) holds a row of kind applePhotos while Photos works: no ✕, no count, the album as detail. The desktop app refuses export.addToPhotos {wait: true}: its window's thread never waits for Photos. The label is in every catalog; tests assert the rows, the refused cancel and the refused wait. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/activity.rs | 81 ++++++++++++---- crates/engine/src/apple_photos.rs | 142 +++++++++++++++++++++++----- crates/engine/src/cmd/export.rs | 11 ++- crates/engine/src/export.rs | 4 +- crates/engine/src/tests_export.rs | 5 +- crates/ui-egui/locales/de.json | 3 +- crates/ui-egui/locales/es.json | 3 +- crates/ui-egui/locales/fr.json | 3 +- crates/ui-egui/locales/ja.json | 3 +- crates/ui-egui/locales/pt-br.json | 3 +- crates/ui-egui/locales/ru.json | 3 +- crates/ui-egui/locales/uk.json | 3 +- crates/ui-egui/locales/zh-hans.json | 3 +- crates/ui-egui/locales/zh-hant.json | 3 +- crates/ui-egui/src/export_task.rs | 34 ++++--- crates/ui-egui/src/headless.rs | 25 +++-- docs/apple-photos.md | 10 +- docs/mcp.md | 2 +- docs/parity-checklist.md | 4 +- 19 files changed, 266 insertions(+), 79 deletions(-) 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 index f59468b5..fd1c6f24 100644 --- a/crates/engine/src/apple_photos.rs +++ b/crates/engine/src/apple_photos.rs @@ -28,6 +28,8 @@ use std::time::Duration; use serde_json::{Value, json}; +use crate::activity::{Activity, Cancel, TaskGuard}; + /// Run by osascript with `argv` = [`MARKER`], the album name (empty: none), then the files' POSIX /// paths. Prints the imported media items' ids, one per line. /// @@ -469,7 +471,7 @@ impl Imports { return Err("an album name can't contain a NUL character".into()); } let job = self.begin(0, album, true)?; - Ok(Reservation { runner, album: album.to_string(), job: Some(job) }) + Ok(Reservation { runner, album: album.to_string(), job: Some(job), activity: None }) } /// [`Imports::wait_idle_within`] with the [`WaitLimits::default`]. @@ -542,17 +544,39 @@ impl crate::Session { } } -/// Ask Photos to import `paths` (checked; `album` trimmed) for `job`: here, or on a worker thread. -fn launch(runner: &Runner, job: &Arc, paths: Vec, album: String, background: bool) { +/// The activity stack's kind for an Apple Photos import of its own (`activity.list`). +pub const ACTIVITY_KIND: &str = "applePhotos"; +/// ...and its label (translated by the frontend). +pub const ACTIVITY_LABEL: &str = "Adding to Apple Photos"; + +/// A row in the activity stack for an import that runs on its own (not as the last step of an +/// export, whose row covers it): no ✕ (Photos can't be stopped once asked), no count (Photos +/// reports none until it is done), the album as its detail. +fn row(activity: Option<&Activity>, album: &str) -> Option { + 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 || run_to_completion(&r, &j, &paths, &album)); + let started = std::thread::Builder::new().name("apple-photos".into()).spawn(move || { + let _row = row; + run_to_completion(&r, &j, &paths, &album) + }); if let Err(e) = started { job.finish(Outcome::Failed(format!("Couldn't start adding the photos to Apple Photos: {e}"))); } @@ -581,13 +605,22 @@ fn run_to_completion(runner: &Runner, job: &ImportJob, paths: &[String], album: } /// Add `paths` to Photos as a job of `imports`: on a worker thread (`background`; the job is -/// returned running, and announced by the app when it ends) or here (returned finished). An -/// error, and no job, for a bad request or while another import is reserved or running. -pub fn import_job(runner: &Runner, imports: &Imports, paths: Vec, album: &str, background: bool) -> Result, String> { +/// returned running, and announced by the app when it ends) or here (returned finished), with a +/// row in `activity` while Photos works. An error, and no job, for a bad request or while +/// another import is reserved or running. +pub fn import_job( + runner: &Runner, + imports: &Imports, + paths: Vec, + 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); - launch(runner, &job, paths, album, background); + let row = row(activity, &album); + launch(runner, &job, paths, album, background, row); Ok(job) } @@ -598,9 +631,18 @@ pub struct Reservation { runner: Runner, album: String, job: Option>, + /// 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() } @@ -633,7 +675,7 @@ impl Reservation { pub fn run(mut self, files: &[Value], cancelled: bool) -> Value { match self.take(files, cancelled) { Ok((job, paths)) => { - launch(&self.runner, &job, paths, self.album.clone(), false); + launch(&self.runner, &job, paths, self.album.clone(), false, None); job.json() } Err(done) => done, @@ -646,7 +688,8 @@ impl Reservation { match self.take(files, false) { Ok((job, paths)) => { job.quiet.store(false, Ordering::Relaxed); - launch(&self.runner, &job, paths, self.album.clone(), true); + let row = row(self.activity.as_ref(), &self.album); + launch(&self.runner, &job, paths, self.album.clone(), true, row); job.json() } Err(done) => done, @@ -984,7 +1027,7 @@ mod tests { let reserved = imports.reserve(runner.clone(), "Trip").unwrap(); assert_eq!(reserved.job().map(|j| j.json()), Some(json!({"job": 1, "running": true, "exporting": true, "requested": 0, "album": "Trip"}))); // while the export writes: other imports and exports are refused, naming it - let e = import_job(&runner, &imports, paths.clone(), "", false).unwrap_err(); + let e = import_job(&runner, &imports, paths.clone(), "", false, None).unwrap_err(); assert!(e.contains("An export is about to add its files to Apple Photos (import 1)"), "{e}"); assert!(imports.reserve(runner.clone(), "").err().unwrap().contains("(import 1)")); assert!(calls.lock().unwrap().is_empty()); @@ -1003,7 +1046,7 @@ mod tests { assert!(out["error"].as_str().unwrap().contains("no such file"), "{out}"); assert_eq!(calls.lock().unwrap().len(), 1); assert!(imports.running().is_none()); - assert_eq!(import_job(&runner, &imports, paths, "", false).unwrap().id, 4); + assert_eq!(import_job(&runner, &imports, paths, "", false, None).unwrap().id, 4); let _ = std::fs::remove_dir_all(dir); } @@ -1017,11 +1060,11 @@ mod tests { let imports = Imports::default(); let stopped = |j: &ImportJob| j.json()["error"].as_str().is_some_and(|e| e.contains("stopped unexpectedly")); // waiting here - let job = import_job(&panicky, &imports, paths.clone(), "", false).unwrap(); + let job = import_job(&panicky, &imports, paths.clone(), "", false, None).unwrap(); assert!(job.finished() && stopped(&job), "{}", job.json()); assert!(imports.running().is_none()); // on a worker - let job = import_job(&panicky, &imports, paths.clone(), "", true).unwrap(); + let job = import_job(&panicky, &imports, paths.clone(), "", true, None).unwrap(); let t0 = std::time::Instant::now(); while !job.finished() { assert!(t0.elapsed() < Duration::from_secs(20)); @@ -1040,7 +1083,7 @@ mod tests { } assert!(stopped(&job)); // the slot is free: the next import runs - assert_eq!(import_job(&good, &imports, paths, "", false).unwrap().json()["imported"], 1); + assert_eq!(import_job(&good, &imports, paths, "", false, None).unwrap().json()["imported"], 1); let _ = std::fs::remove_dir_all(dir); } @@ -1050,7 +1093,7 @@ mod tests { let (dir, paths) = files("idle", 1); let (runner, go) = delayed(); let imports = Imports::default(); - let job = import_job(&runner, &imports, paths, "", true).unwrap(); + let job = import_job(&runner, &imports, paths, "", true, None).unwrap(); assert!(!job.finished()); let release = std::thread::spawn(move || { std::thread::sleep(Duration::from_millis(200)); @@ -1099,7 +1142,7 @@ mod tests { assert!(t0.elapsed() >= Duration::from_millis(1000), "{:?}", t0.elapsed()); assert_eq!(exporter.join().unwrap()["imported"], 2); // an import past its own limit is given up on at its deadline (not the cap) - let job = import_job(&taking(3000), &imports, paths.clone(), "", true).unwrap(); + let job = import_job(&taking(3000), &imports, paths.clone(), "", true, None).unwrap(); let t0 = std::time::Instant::now(); assert!(!imports.wait_idle_within(limits)); let waited = t0.elapsed(); @@ -1116,7 +1159,7 @@ mod tests { drop(held); assert!(imports.wait_idle_within(limits)); // and the cap holds over everything - let job = import_job(&taking(1000), &imports, paths, "", true).unwrap(); + let job = import_job(&taking(1000), &imports, paths, "", true, None).unwrap(); let t0 = std::time::Instant::now(); assert!(!imports.wait_idle_within(WaitLimits { cap: Duration::from_millis(200), ..limits })); assert!(t0.elapsed() < Duration::from_millis(900)); @@ -1147,11 +1190,11 @@ mod tests { let (runner, go) = delayed(); let imports = Imports::default(); let t0 = std::time::Instant::now(); - let job = import_job(&runner, &imports, paths.clone(), " Trip ", true).unwrap(); + let job = import_job(&runner, &imports, paths.clone(), " Trip ", true, None).unwrap(); assert!(t0.elapsed() < Duration::from_secs(5), "the caller doesn't wait for Photos"); assert_eq!(job.json(), json!({"job": 1, "running": true, "requested": 2, "album": "Trip"})); assert_eq!(imports.running().map(|j| j.id), Some(1)); - let e = import_job(&runner, &imports, paths.clone(), "", true).unwrap_err(); + let e = import_job(&runner, &imports, paths.clone(), "", true, None).unwrap_err(); assert!(e.contains("still adding 2 file(s) from an earlier request (import 1)"), "{e}"); let e = imports.reserve(runner.clone(), "").err().unwrap(); assert!(e.contains("still adding"), "{e}"); @@ -1165,7 +1208,7 @@ mod tests { assert_eq!((done["running"].as_bool(), done["imported"].as_u64(), done["album"].as_str()), (Some(false), Some(2), Some("Trip")), "{done}"); // finished: the next one may start; a failed one says why let (fails, _) = fake(Err(RunError::Spawn("denied".into()))); - let job = import_job(&fails, &imports, paths, "", false).unwrap(); + let job = import_job(&fails, &imports, paths, "", false, None).unwrap(); assert_eq!(job.id, 2); assert!(job.json()["error"].as_str().unwrap().contains("denied")); assert!(imports.running().is_none() && imports.latest().is_some_and(|j| j.id == 2)); @@ -1178,7 +1221,7 @@ mod tests { let (runner, _) = fake(ok("A\n")); let imports = Imports::default(); for _ in 0..KEPT_JOBS + 3 { - import_job(&runner, &imports, paths.clone(), "", false).unwrap(); + import_job(&runner, &imports, paths.clone(), "", false, None).unwrap(); } let all = imports.all(); assert_eq!(all.len(), KEPT_JOBS); @@ -1187,6 +1230,61 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// The activity stack (issue #345): an import of its own shows a row while Photos works (no ✕, + /// no count, the album as detail) and loses it when Photos is done; an export's own import + /// shows none of its own, the export's row covers it. + #[test] + fn imports_of_their_own_show_in_the_activity_stack() { + let (dir, paths) = files("activity", 2); + let activity = Activity::default(); + let seen = Arc::new(Mutex::new(Vec::new())); + // Photos answers when the test lets it (`go`), so no assertion races the import's end + let (go, wait) = std::sync::mpsc::channel::<()>(); + let wait = Mutex::new(wait); + let (a, rows) = (activity.clone(), seen.clone()); + let runner: Runner = Arc::new(move |args: &[String], _| { + rows.lock().unwrap().push(a.list()); + let _ = wait.lock().unwrap().recv_timeout(Duration::from_secs(20)); + let n = args.len() - args.iter().position(|x| x == MARKER).unwrap() - 2; + Ok(Output { status: Some(0), stdout: "ID\n".repeat(n), stderr: String::new() }) + }); + let imports = Imports::default(); + let ended = |job: &ImportJob| { + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while !(job.finished() && activity.list().is_empty()) { + assert!(std::time::Instant::now() < deadline, "the import ends and its row goes"); + std::thread::sleep(Duration::from_millis(5)); + } + }; + // waiting for Photos here + go.send(()).unwrap(); + let job = import_job(&runner, &imports, paths.clone(), "Trip", false, Some(&activity)).unwrap(); + assert_eq!(job.json()["imported"], 2); + assert!(activity.list().is_empty(), "the row goes when Photos is done"); + let during = seen.lock().unwrap().last().cloned().unwrap(); + assert_eq!(during.len(), 1, "{during:?}"); + assert_eq!((during[0].kind, during[0].label.as_str(), during[0].detail.as_str()), (ACTIVITY_KIND, ACTIVITY_LABEL, "Trip")); + assert_eq!((during[0].cancellable, during[0].total), (false, 0), "no ✕ and no count"); + // on a worker thread: the row stays while Photos works, and goes once the import ends + let job = import_job(&runner, &imports, paths.clone(), "", true, Some(&activity)).unwrap(); + assert_eq!(activity.list().first().map(|r| r.kind), Some(ACTIVITY_KIND)); + go.send(()).unwrap(); + ended(&job); + // an export's own import: the export's row covers it + let exported: Vec = 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)); diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index dafcb9a3..e93d0417 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -115,10 +115,17 @@ fn add_to_photos(s: &mut Session, p: &Value) -> Result { // in the middle of the import let wait = match p.get("wait") { None | Some(Value::Null) => !s.apple_photos_background, + // the desktop app answers on its window's thread, which never waits for Photos + Some(Value::Bool(true)) if s.apple_photos_background => { + return Err(bad( + ID, + "`wait: true` isn't available in the desktop app: the import runs in the background; its outcome comes from export.photosImports {job}", + )); + } Some(Value::Bool(b)) => *b, Some(_) => return Err(bad(ID, "`wait` must be true or false")), }; - let job = crate::apple_photos::import_job(&runner, &s.apple_photos_imports, paths, album, !wait).map_err(|e| bad(ID, e))?; + let job = crate::apple_photos::import_job(&runner, &s.apple_photos_imports, paths, album, !wait, Some(&s.activity)).map_err(|e| bad(ID, e))?; match job.outcome() { Some(crate::apple_photos::Outcome::Failed(e)) => Err(bad(ID, e)), _ => Ok(job.json()), @@ -193,7 +200,7 @@ pub fn specs() -> Vec { "Add to Apple Photos", [], None, - "{paths: [absolute file paths], album?: name (an album at the top level of Photos, made when missing; default none), wait?} → once Photos is done, {job, running: false, requested, album, imported, ids: [Photos media item ids], warning?}; with wait: false, at once {job, running: true, requested, album} and the outcome later from export.photosImports {job}. wait defaults to true without a window (lightcraft-cli, the MCP server) and to false in the desktop app (its control requests are answered on the UI thread). macOS: asks Photos to import the files (the first time, macOS asks the user to allow LightCraft to control Photos). One import runs at a time, and an export adding to Photos reserves it before writing: another is refused until it ends. app.export does this for the files it writes with `addToPhotos: true`", + "{paths: [absolute file paths], album?: name (an album at the top level of Photos, made when missing; default none), wait?} → once Photos is done, {job, running: false, requested, album, imported, ids: [Photos media item ids], warning?}; with wait: false, at once {job, running: true, requested, album} and the outcome later from export.photosImports {job}. wait defaults to true without a window (lightcraft-cli, the MCP server) and to false in the desktop app, where wait: true is refused (its control requests are answered on the UI thread, which never waits for Photos). macOS: asks Photos to import the files (the first time, macOS asks the user to allow LightCraft to control Photos). One import runs at a time, and an export adding to Photos reserves it before writing: another is refused until it ends. app.export does this for the files it writes with `addToPhotos: true`", photos_enabled, add_to_photos )); diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index 8bbfbead..b7ffaab0 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -1665,7 +1665,9 @@ impl crate::Session { #[cfg(not(target_arch = "wasm32"))] { let runner = self.apple_photos.clone().ok_or_else(|| crate::apple_photos::unavailable().to_string())?; - AfterExport::apple_photos(runner, &o.photos_album, &self.apple_photos_imports) + let mut after = AfterExport::apple_photos(runner, &o.photos_album, &self.apple_photos_imports)?; + after.apple_photos = after.apple_photos.map(|r| r.shown_in(self.activity.clone())); + Ok(after) } #[cfg(target_arch = "wasm32")] Err("Apple Photos is only available on macOS".into()) diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 7969b799..392cb0e5 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -647,9 +647,12 @@ mod apple_photos { let all = s.execute("export.photosImports", &json!({})).unwrap(); assert_eq!(all["imports"].as_array().map(Vec::len), Some(1)); assert!(s.execute("export.photosImports", &json!({"job": 7})).unwrap_err().to_string().contains("no import 7")); + // the window's thread never waits for Photos: `wait: true` is refused here, before any import + let e = s.execute("export.addToPhotos", &json!({"paths": paths, "wait": true})).unwrap_err().to_string(); + assert!(e.contains("isn't available in the desktop app"), "{e}"); // finished: the next one may start go.send(()).unwrap(); - assert_eq!(s.execute("export.addToPhotos", &json!({"paths": paths, "wait": true})).unwrap()["job"], 2); + assert_eq!(s.execute("export.addToPhotos", &json!({"paths": paths})).unwrap()["job"], 2); } /// Review of #236: an export adding to Photos reserves the import before it writes its files. diff --git a/crates/ui-egui/locales/de.json b/crates/ui-egui/locales/de.json index bd97f5ea..2b130f2e 100644 --- a/crates/ui-egui/locales/de.json +++ b/crates/ui-egui/locales/de.json @@ -2043,5 +2043,6 @@ "Add to Apple Photos": "Zu Apple Fotos hinzufügen", "Adding to Apple Photos…": "Wird zu Apple Fotos hinzugefügt…", "not added to Apple Photos (cancelled)": "nicht zu Apple Fotos hinzugefügt (abgebrochen)", - "The first time, macOS asks whether LightCraft may control Photos.": "Beim ersten Mal fragt macOS, ob LightCraft Fotos steuern darf." + "The first time, macOS asks whether LightCraft may control Photos.": "Beim ersten Mal fragt macOS, ob LightCraft Fotos steuern darf.", + "Adding to Apple Photos": "Wird zu Apple Fotos hinzugefügt" } diff --git a/crates/ui-egui/locales/es.json b/crates/ui-egui/locales/es.json index c4c6e1c5..b9a3b427 100644 --- a/crates/ui-egui/locales/es.json +++ b/crates/ui-egui/locales/es.json @@ -1888,5 +1888,6 @@ "Add to Apple Photos": "Añadir a Fotos de Apple", "Adding to Apple Photos…": "Añadiendo a Fotos de Apple…", "not added to Apple Photos (cancelled)": "no se añadieron a Fotos de Apple (cancelado)", - "The first time, macOS asks whether LightCraft may control Photos.": "La primera vez, macOS pregunta si LightCraft puede controlar Fotos." + "The first time, macOS asks whether LightCraft may control Photos.": "La primera vez, macOS pregunta si LightCraft puede controlar Fotos.", + "Adding to Apple Photos": "Añadiendo a Fotos de Apple" } diff --git a/crates/ui-egui/locales/fr.json b/crates/ui-egui/locales/fr.json index 503b8cba..3a84f7d3 100644 --- a/crates/ui-egui/locales/fr.json +++ b/crates/ui-egui/locales/fr.json @@ -1888,5 +1888,6 @@ "Add to Apple Photos": "Ajouter à Apple Photos", "Adding to Apple Photos…": "Ajout à Apple Photos…", "not added to Apple Photos (cancelled)": "non ajouté à Apple Photos (annulé)", - "The first time, macOS asks whether LightCraft may control Photos.": "La première fois, macOS demande si LightCraft peut contrôler Photos." + "The first time, macOS asks whether LightCraft may control Photos.": "La première fois, macOS demande si LightCraft peut contrôler Photos.", + "Adding to Apple Photos": "Ajout à Apple Photos" } diff --git a/crates/ui-egui/locales/ja.json b/crates/ui-egui/locales/ja.json index b5b891d0..23c4e2c0 100644 --- a/crates/ui-egui/locales/ja.json +++ b/crates/ui-egui/locales/ja.json @@ -1890,5 +1890,6 @@ "Add to Apple Photos": "Appleの写真に追加", "Adding to Apple Photos…": "Appleの写真に追加中…", "not added to Apple Photos (cancelled)": "Appleの写真には追加されませんでした（キャンセル）", - "The first time, macOS asks whether LightCraft may control Photos.": "初回は、LightCraftに写真の操作を許可するかmacOSが確認します。" + "The first time, macOS asks whether LightCraft may control Photos.": "初回は、LightCraftに写真の操作を許可するかmacOSが確認します。", + "Adding to Apple Photos": "Appleの写真に追加中" } diff --git a/crates/ui-egui/locales/pt-br.json b/crates/ui-egui/locales/pt-br.json index 698a274d..cdbf23f4 100644 --- a/crates/ui-egui/locales/pt-br.json +++ b/crates/ui-egui/locales/pt-br.json @@ -1888,5 +1888,6 @@ "Add to Apple Photos": "Adicionar ao Fotos da Apple", "Adding to Apple Photos…": "Adicionando ao Fotos da Apple…", "not added to Apple Photos (cancelled)": "não adicionadas ao Fotos da Apple (cancelado)", - "The first time, macOS asks whether LightCraft may control Photos.": "Na primeira vez, o macOS pergunta se o LightCraft pode controlar o Fotos." + "The first time, macOS asks whether LightCraft may control Photos.": "Na primeira vez, o macOS pergunta se o LightCraft pode controlar o Fotos.", + "Adding to Apple Photos": "Adicionando ao Fotos da Apple" } diff --git a/crates/ui-egui/locales/ru.json b/crates/ui-egui/locales/ru.json index 364f07ba..23963fdc 100644 --- a/crates/ui-egui/locales/ru.json +++ b/crates/ui-egui/locales/ru.json @@ -1602,5 +1602,6 @@ "Add to Apple Photos": "Добавить в Apple Фото", "Adding to Apple Photos…": "Добавление в Apple Фото…", "not added to Apple Photos (cancelled)": "не добавлено в Apple Фото (отменено)", - "The first time, macOS asks whether LightCraft may control Photos.": "В первый раз macOS спросит, может ли LightCraft управлять приложением «Фото»." + "The first time, macOS asks whether LightCraft may control Photos.": "В первый раз macOS спросит, может ли LightCraft управлять приложением «Фото».", + "Adding to Apple Photos": "Добавление в Apple Фото" } diff --git a/crates/ui-egui/locales/uk.json b/crates/ui-egui/locales/uk.json index b76dea8f..8df7036c 100644 --- a/crates/ui-egui/locales/uk.json +++ b/crates/ui-egui/locales/uk.json @@ -2082,5 +2082,6 @@ "Add to Apple Photos": "Додати до Apple Фото", "Adding to Apple Photos…": "Додавання до Apple Фото…", "not added to Apple Photos (cancelled)": "не додано до Apple Фото (скасовано)", - "The first time, macOS asks whether LightCraft may control Photos.": "Першого разу macOS запитає, чи може LightCraft керувати програмою «Фото»." + "The first time, macOS asks whether LightCraft may control Photos.": "Першого разу macOS запитає, чи може LightCraft керувати програмою «Фото».", + "Adding to Apple Photos": "Додавання до Apple Фото" } diff --git a/crates/ui-egui/locales/zh-hans.json b/crates/ui-egui/locales/zh-hans.json index b38b91b2..a82443bc 100644 --- a/crates/ui-egui/locales/zh-hans.json +++ b/crates/ui-egui/locales/zh-hans.json @@ -1888,5 +1888,6 @@ "Add to Apple Photos": "添加到 Apple 照片", "Adding to Apple Photos…": "正在添加到 Apple 照片…", "not added to Apple Photos (cancelled)": "未添加到 Apple 照片（已取消）", - "The first time, macOS asks whether LightCraft may control Photos.": "首次使用时，macOS 会询问是否允许 LightCraft 控制“照片”。" + "The first time, macOS asks whether LightCraft may control Photos.": "首次使用时，macOS 会询问是否允许 LightCraft 控制“照片”。", + "Adding to Apple Photos": "正在添加到 Apple 照片" } diff --git a/crates/ui-egui/locales/zh-hant.json b/crates/ui-egui/locales/zh-hant.json index 073d9f8b..41d50eea 100644 --- a/crates/ui-egui/locales/zh-hant.json +++ b/crates/ui-egui/locales/zh-hant.json @@ -1888,5 +1888,6 @@ "Add to Apple Photos": "加入 Apple 照片", "Adding to Apple Photos…": "正在加入 Apple 照片…", "not added to Apple Photos (cancelled)": "未加入 Apple 照片（已取消）", - "The first time, macOS asks whether LightCraft may control Photos.": "第一次使用時，macOS 會詢問是否允許 LightCraft 控制「照片」。" + "The first time, macOS asks whether LightCraft may control Photos.": "第一次使用時，macOS 會詢問是否允許 LightCraft 控制「照片」。", + "Adding to Apple Photos": "正在加入 Apple 照片" } diff --git a/crates/ui-egui/src/export_task.rs b/crates/ui-egui/src/export_task.rs index b6de2f41..051ad22b 100644 --- a/crates/ui-egui/src/export_task.rs +++ b/crates/ui-egui/src/export_task.rs @@ -22,8 +22,9 @@ pub struct ExportTask { /// Photos started so far and the file in progress. pub progress: Arc>, pub cancel: Arc, - /// The files (per-photo results) and what [`AfterExport`] did with them (`applePhotos`). - rx: Receiver, Option), 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, } @@ -48,6 +49,9 @@ 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 || { @@ -59,13 +63,17 @@ pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOp !c.load(Ordering::Relaxed) }); let r = r.map(|files| { - if !after.is_none() - && let Ok(mut g) = p.lock() - { - g.1 = adding; + if !after.is_none() { + // Photos can't be stopped once asked: no ✕ from here, before the last look at + // the cancel flag, so a cancel either stops the export before Photos or is refused + row.set_cancellable(false); + if let Ok(mut g) = p.lock() { + g.1 = adding; + } } - let photos = after.run(&files, c.load(Ordering::Relaxed)); - (files, photos) + let cancelled = c.load(Ordering::Relaxed); + let photos = after.run(&files, cancelled); + (files, photos, cancelled) }); let _ = tx.send(r); }; @@ -73,8 +81,6 @@ pub fn start(app: &mut LightcraftApp, items: Vec, opts: ExportOp std::thread::Builder::new().name("export".into()).spawn(work).map_err(|e| e.to_string())?; #[cfg(target_arch = "wasm32")] work(); - let guard = app.session.activity.start("export", "Exporting", Cancel::Flag(cancel.clone())); - guard.progress(0, total as u64); app.export = Some(ExportTask { total, progress, cancel, rx, guard }); Ok(json!({"background": true, "total": total})) } @@ -114,7 +120,7 @@ pub fn start_contact_sheet(app: &mut LightcraftApp, params: &Value) -> Result { - let cancelled = task.cancel.load(Ordering::Relaxed); + let cancelled = matches!(&r, Ok((_, _, true))); let total = task.total; app.export = None; // Photos refused (permission, not installed…): leave the way out on screen longer - let photos_failed = matches!(&r, Ok((_, Some(p))) if p.get("error").is_some()); + let photos_failed = matches!(&r, Ok((_, Some(p), _)) if p.get("error").is_some()); let msg = match r { - Ok((files, photos)) => { + Ok((files, photos, _)) => { let sheet = files.first().filter(|f| f["contactSheet"] == true); let ok = files.iter().filter(|f| f.get("path").is_some()).count(); let failed: Vec<&Value> = files.iter().filter(|f| f.get("error").is_some()).collect(); diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 1bf18021..b0cdc465 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1885,7 +1885,10 @@ mod tests { h.app.services.write_shared = Some(Arc::new(|p: &str, bytes: &[u8]| lightcraft_engine::export::write_file(p, bytes))); let (release, held) = std::sync::mpsc::channel::<()>(); let held = std::sync::Mutex::new(held); + let (entered, photos_asked) = std::sync::mpsc::channel::<()>(); + let entered = std::sync::Mutex::new(entered); h.app.session.apple_photos = Some(Arc::new(move |_: &[String], _| { + let _ = entered.lock().unwrap().send(()); let _ = held.lock().unwrap().recv_timeout(Duration::from_secs(30)); Ok(lightcraft_engine::apple_photos::Output { status: Some(0), stdout: "ID\n".into(), stderr: String::new() }) })); @@ -1893,19 +1896,25 @@ mod tests { let params = json!({"ids": ids, "dir": out, "longEdge": 48, "addToPhotos": true, "background": true}); let r = h.request("engine.execute", json!({"command": "app.export", "params": params}), t); assert_eq!(r["ok"], true, "{r}"); - let t0 = Instant::now(); - let current = loop { - h.step(); - let current = h.app.export.as_ref().map(|e| e.progress.lock().unwrap().1.clone()).unwrap_or_default(); - if current.contains("Apple") || t0.elapsed() > Duration::from_secs(60) { - break current; - } - }; + // Photos has been asked, and the window hasn't drawn a frame since (Codex review of #643): + // the export's row already has no ✕, and a cancel is refused rather than half-honoured + photos_asked.recv_timeout(Duration::from_secs(60)).expect("the export asks Photos"); + let row = h.app.session.activity.list().into_iter().find(|r| r.kind == "export").expect("the export's row"); + assert!(!row.cancellable, "{row:?}"); + assert!(h.app.session.activity.cancel(row.id).is_err(), "Photos can't be stopped once asked"); + let current = h.app.export.as_ref().map(|e| e.progress.lock().unwrap().1.clone()).unwrap_or_default(); assert_eq!(current, "Wird zu Apple Fotos hinzugefügt…"); + // the row says so on the next frame (issue #345) + h.step(); + let row = h.app.session.activity.list().into_iter().find(|r| r.kind == "export").expect("the export's row"); + assert_eq!((row.detail.as_str(), row.cancellable), ("Wird zu Apple Fotos hinzugefügt…", false), "{row:?}"); release.send(()).unwrap(); + let t0 = Instant::now(); while h.app.export.is_some() && t0.elapsed() < Duration::from_secs(90) { h.step(); } + let last = h.app.last_export_result.clone().expect("the export reports"); + assert_eq!((last["cancelled"].as_bool(), last["applePhotos"]["imported"].as_u64()), (Some(false), Some(1)), "{last}"); crate::i18n::set_language(crate::i18n::Locale::En); let _ = std::fs::remove_dir_all(&out); } diff --git a/docs/apple-photos.md b/docs/apple-photos.md index 1fc50f65..f0d47c5e 100644 --- a/docs/apple-photos.md +++ b/docs/apple-photos.md @@ -29,11 +29,17 @@ An import can take minutes, and the first one waits for the user to answer macOS import is a job: `{job, running, requested, album}`, plus, once Photos is done, `{imported, ids, warning?}`, `{error}` or `{skipped}` (`exporting: true` while an export that will add its files is still writing them). +While Photos works, the activity stack ([background-tasks.md](background-tasks.md)) shows it. An export adding its +files keeps its own row, which reads "Adding to Apple Photos…" and loses its ✕, since Photos can't be stopped once +asked. An import of its own (`export.addToPhotos`, or the Photos step left running after `app.export` without +`background`) gets a row of kind `applePhotos`, "Adding to Apple Photos", with the album as its detail, no ✕ and no +count, as Photos reports none until it is done. + - In the **desktop app** nothing waits for Photos on the window's thread. The Export dialog's background export adds the files on its worker and reports them in its toast. `app.export` without `background` and `export.addToPhotos` answer at once with the running job (`running: true`); the app shows a toast when Photos - is done, and `export.photosImports {job}` gives the outcome. (`export.addToPhotos {wait: true}` waits there - too, holding the window until Photos is done.) + is done, and `export.photosImports {job}` gives the outcome. (`export.addToPhotos {wait: true}` is refused there: + the window's thread never waits for Photos.) - **Without a window** (the MCP server, `lightcraft-cli`, the headless snapshot) everything waits for Photos by default: `app.export` and `export.addToPhotos` return the finished job, so a one-shot command never ends in the middle of an import. `export.addToPhotos {wait: false}` returns the running job instead (for a diff --git a/docs/mcp.md b/docs/mcp.md index 01fdcf9e..2c9b30ed 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -278,7 +278,7 @@ plus `{imported, ids, warning?}`, `{error}` or `{skipped}` once Photos is done; way. Headless (this server, `lightcraft-cli`) waits for Photos by default; the desktop app (connect mode) answers with the running job and `export.photosImports {job}` gives the outcome. `export.addToPhotos {paths, album?, wait?}` does the same for files already on disk (`wait` defaults to true headless, false in the desktop -app; with `wait: false` headless, the server waits for the import before it exits at end of input). One import +app, which refuses `wait: true`; with `wait: false` headless, the server waits for the import before it exits at end of input). One import runs at a time, and an export adding to Photos reserves it before it writes: another is refused, naming the job in the way. On other platforms `addToPhotos: true` is refused before anything is written, and `export.addToPhotos` is disabled with the reason. The first time, macOS asks the user to allow LightCraft (or diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index cce814bd..7ec59c5b 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -53,9 +53,9 @@ cited here still exists. Formerly docs/parity.md. | Y. Menus | 82 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 75 | 3 | 3 | 1 | 49/52 (94%) | 23/24 (96%) | | Lightroom Classic extras | 33 | 15 | 35 | 9 | — | 23/25 (92%) | -| **Total** | 397 | 49 | 73 | 36 | 192/200 (96%) | 143/155 (92%) | +| **Total** | 397 | 50 | 72 | 36 | 192/200 (96%) | 143/155 (92%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.2%** of 519 in-scope rows — P0 98.0% of 200 · P1 95.2% of 155 · P2 47.2% of 163. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.3%** of 519 in-scope rows — P0 98.0% of 200 · P1 95.2% of 155 · P2 47.5% of 163. ## Top gaps and shortcut notes