diff --git a/README.md b/README.md index 1e9291be5..a7b71e3ef 100644 --- a/README.md +++ b/README.md @@ -259,7 +259,8 @@ lightcraft --control 7980 ~/Pictures/trip - **Command registry.** `engine.commands` lists the available commands; `develop.controls` lists every slider's range, default and current value. - **MCP server.** `lightcraft-cli mcp` exposes the command registry to Claude (or any MCP client), alongside - helpers for import, query, develop, mask, render (returned as an image) and export. It runs headless, or attached + helpers for import, query, develop, render (returned as an image) and export. Masks have no helper of their own: + create and edit them through `run_command` (e.g. `mask.add`). It runs headless, or attached to the running app with screenshots, clicks and gestures. See [docs/mcp.md](docs/mcp.md). ```sh diff --git a/README.zh-hans.md b/README.zh-hans.md index fd4f17132..9a0f4764e 100644 --- a/README.zh-hans.md +++ b/README.zh-hans.md @@ -241,7 +241,7 @@ lightcraft --control 7980 ~/Pictures/trip - **命令注册表。** `engine.commands` 列出可用命令；`develop.controls` 列出每个滑块的范围、 默认值和当前值。 - **MCP 服务器。** `lightcraft-cli mcp` 把命令注册表开放给 Claude（或任何 MCP 客户端）， - 同时提供导入、查询、显影、蒙版、渲染（以图像返回）和导出的辅助工具。它既能无界面运行，也能附着 + 同时提供导入、查询、显影、渲染（以图像返回）和导出的辅助工具。蒙版没有专门的辅助工具，请通过 `run_command` 使用（例如 `mask.add`）。它既能无界面运行，也能附着 到正在运行的应用上，支持截图、点击和手势。见 [docs/mcp.md](docs/mcp.md)。 ```sh diff --git a/README.zh-hant.md b/README.zh-hant.md index ab742f905..f2c223897 100644 --- a/README.zh-hant.md +++ b/README.zh-hant.md @@ -240,7 +240,7 @@ lightcraft --control 7980 ~/Pictures/trip - **指令註冊表。** `engine.commands` 列出可用的指令；`develop.controls` 列出每個滑桿的範圍、 預設值與目前值。 - **MCP 伺服器。** `lightcraft-cli mcp` 把指令註冊表開放給 Claude（或任何 MCP 用戶端）， - 同時提供匯入、查詢、顯影、遮色片、算繪（以影像回傳）與匯出的輔助工具。它可以無介面執行，也可以附掛 + 同時提供匯入、查詢、顯影、算繪（以影像回傳）與匯出的輔助工具。遮色片沒有專屬的輔助工具，請透過 `run_command` 使用（例如 `mask.add`）。它可以無介面執行，也可以附掛 到正在執行的應用程式上，支援截圖、點擊與手勢。詳見 [docs/mcp.md](docs/mcp.md)。 ```sh diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index c1f9d258c..dcb5280f6 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -155,7 +155,16 @@ fn main() -> ExitCode { // Warnings (a GPU render redone on the CPU, an unknown backend name) on stderr; LIGHTCRAFT_LOG // or RUST_LOG picks another level (#168). lightcraft_engine::logging::install("lightcraft-cli"); - let args: Vec = std::env::args().skip(1).collect(); + let mut args: Vec = Vec::new(); + for (index, arg) in std::env::args_os().skip(1).enumerate() { + match arg.into_string() { + Ok(arg) => args.push(arg), + Err(_) => { + eprintln!("lightcraft-cli: argument {} is not valid Unicode", index + 1); + return ExitCode::FAILURE; + } + } + } let r = match args.first().map(String::as_str) { Some("run") => run(&args[1..]), Some("mcp") => mcp(&args[1..]), @@ -557,6 +566,8 @@ fn run(args: &[String]) -> Result<(), String> { } else { std::fs::read_to_string(path).map_err(|e| format!("{path}: {e}"))? }; + // Windows PowerShell 5.1 writes a UTF-8 BOM; it is not part of the first JSON line. + let text = text.strip_prefix('\u{feff}').unwrap_or(&text); for (n, line) in text.lines().enumerate().filter(|(_, l)| !l.trim().is_empty() && !l.trim_start().starts_with('#')) { let v: Value = serde_json::from_str(line).map_err(|e| format!("{path}:{}: {e}", n + 1))?; // `{"command": id, "params": …}` or a control request `{"method": m, "params": …}` @@ -565,7 +576,11 @@ fn run(args: &[String]) -> Result<(), String> { .or(v.get("method")) .and_then(Value::as_str) .ok_or_else(|| format!("{path}:{}: needs `command` or `method`", n + 1))?; - let params = v.get("params").cloned().filter(|p| p.is_object()).unwrap_or_else(|| json!({})); + let params = match v.get("params") { + None | Some(Value::Null) => json!({}), + Some(Value::Object(params)) => Value::Object(params.clone()), + Some(_) => return Err(format!("{path}:{}: `params` must be a JSON object", n + 1)), + }; steps.push(Step { id: id.to_string(), params }); } } @@ -594,14 +609,7 @@ fn run(args: &[String]) -> Result<(), String> { None => Headless::default(), }; if !imports.is_empty() { - let paths = expand_paths(&imports); - let r = h.session.execute("library.import", &json!({"paths": paths})).map_err(|e| e.to_string())?; - // what follows acts on the imported photos (already-known files are reported as duplicates) - let mut ids: Vec = r["imported"].as_array().cloned().unwrap_or_default(); - ids.extend(r["duplicates"].as_array().into_iter().flatten().filter_map(|d| d.get("existing").filter(|v| v.is_u64()).cloned())); - if let Some(first) = ids.first().cloned() { - h.session.execute("library.select", &json!({"ids": ids, "active": first})).map_err(|e| e.to_string())?; - } + import_and_select(&mut h.session, &imports)?; } Box::new(h) } @@ -626,6 +634,34 @@ fn run(args: &[String]) -> Result<(), String> { if failed > 0 { Err(format!("{failed} command(s) failed")) } else { Ok(()) } } +fn import_and_select(session: &mut Session, imports: &[String]) -> Result<(), String> { + let selection = session.selection.clone(); + let paths = expand_paths(imports); + let result = (|| { + let r = session.execute("library.import", &json!({"paths": paths})).map_err(|e| format!("import failed: {e}; no commands were run"))?; + let ids = import_targets(&r)?; + let first = ids.first().cloned().ok_or("import found no photos to select; no commands were run")?; + session.execute("library.select", &json!({"ids": ids, "active": first})).map_err(|e| e.to_string())?; + Ok::<(), String>(()) + })(); + if result.is_err() { + session.selection = selection; + } + result +} + +fn import_targets(report: &Value) -> Result, String> { + if let Some(failed) = report["failed"].as_array().filter(|failed| !failed.is_empty()) { + return Err(format!("import failed for {} item(s): {}; no commands were run", failed.len(), json!(failed))); + } + let mut ids: Vec = report["imported"].as_array().cloned().unwrap_or_default(); + ids.extend(report["duplicates"].as_array().into_iter().flatten().filter_map(|d| d.get("existing").filter(|v| v.is_u64()).cloned())); + ids.extend(report["restored"].as_array().into_iter().flatten().filter(|id| id.is_u64()).cloned()); + let mut seen = std::collections::HashSet::new(); + ids.retain(|id| id.as_u64().is_none_or(|id| seen.insert(id))); + if ids.is_empty() { Err("import found no photos to select; no commands were run".into()) } else { Ok(ids) } +} + fn take_value<'a>(args: &'a [String], i: &mut usize, flag: &str) -> Result<&'a str, String> { *i += 1; args.get(*i).map(String::as_str).ok_or_else(|| format!("{flag} needs a value")) @@ -683,7 +719,9 @@ fn render(args: &[String]) -> Result<(), String> { "--set" => { let kv = take_value(args, &mut i, "--set")?; let (k, v) = kv.split_once('=').ok_or_else(|| format!("--set expects control=value, got `{kv}`"))?; - let v: f64 = v.trim().parse().map_err(|_| format!("--set {k}: `{v}` is not a number"))?; + // `inf` / `NaN` parse as f64 but become JSON null, which the control would silently ignore. + let v: f64 = + v.trim().parse().ok().filter(|n: &f64| n.is_finite()).ok_or_else(|| format!("--set {k}: `{v}` is not a finite number"))?; values.insert(k.trim().to_string(), json!(v)); } "--settings" => settings = Some(read_settings(take_value(args, &mut i, "--settings")?)?), @@ -852,6 +890,8 @@ fn snapshot(args: &[String]) -> Result<(), String> { let mut failed = 0usize; if let Some(script) = &script { let text = std::fs::read_to_string(script).map_err(|e| format!("{script}: {e}"))?; + // Windows PowerShell 5.1 writes a UTF-8 BOM; it is not part of the first JSON line. + let text = text.strip_prefix('\u{feff}').unwrap_or(&text); let mut out = std::io::stdout().lock(); for (n, line) in text.lines().enumerate() { let line = line.trim(); @@ -1028,3 +1068,54 @@ fn denoise_serve(args: &[String]) -> Result<(), String> { fn denoise_serve(_: &[String]) -> Result<(), String> { Err("denoise-serve: this build has no denoise support (build with the `denoise` feature)".into()) } + +#[cfg(test)] +mod import_target_tests { + use super::{import_and_select, import_targets, json}; + + #[test] + fn empty_reports_and_duplicates_without_ids_have_no_target() { + assert!(import_targets(&json!({})).is_err()); + assert!(import_targets(&json!({"imported": [], "duplicates": [{"existing": null}]})).is_err()); + } + + #[test] + fn restored_only_and_mixed_reports_keep_target_order() { + assert_eq!(import_targets(&json!({"restored": [9]})).unwrap(), vec![json!(9)]); + assert_eq!( + import_targets(&json!({"imported": [1], "duplicates": [{"existing": null}, {"existing": 2}], "restored": [3]})).unwrap(), + vec![json!(1), json!(2), json!(3)] + ); + } + + #[test] + fn repeated_targets_are_selected_once_in_first_occurrence_order() { + let report = json!({"imported": [5, 7], "duplicates": [{"existing": 5}, {"existing": 8}, {"existing": 8}, {"existing": 7}, {"existing": null}], "restored": [8, 9, 5]}); + assert_eq!(import_targets(&report).unwrap(), vec![json!(5), json!(7), json!(8), json!(9)]); + } + + #[test] + fn failed_items_reject_even_when_targets_exist() { + let error = import_targets(&json!({"imported": [1], "failed": [["broken.png", "invalid"]]})).unwrap_err(); + assert!(error.contains("1") && error.contains("broken.png") && error.contains("no commands were run"), "{error}"); + } + + #[test] + fn failed_partial_import_restores_empty_selection() { + let dir = std::env::temp_dir().join(format!("lightcraft-import-selection-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let input = dir.join("valid.png"); + let broken = dir.join("broken.png"); + let img = lightcraft_raster::Rgba8::from_fn(2, 2, |_, _| [20, 40, 60, 255]); + let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &Default::default()).unwrap(); + std::fs::write(&input, png).unwrap(); + std::fs::write(&broken, b"not a PNG").unwrap(); + let mut headless = lightcraft_mcp::Headless::default(); + let session = &mut headless.session; + let selection = session.selection.clone(); + assert!(selection.ids.is_empty() && selection.active.is_none()); + assert!(import_and_select(session, &[input.to_string_lossy().into_owned(), broken.to_string_lossy().into_owned()]).is_err()); + assert_eq!(session.selection, selection); + let _ = std::fs::remove_dir_all(dir); + } +} diff --git a/apps/lightcraft-cli/tests/it/cli.rs b/apps/lightcraft-cli/tests/it/cli.rs index 8a11f5c9a..ceebce2d3 100644 --- a/apps/lightcraft-cli/tests/it/cli.rs +++ b/apps/lightcraft-cli/tests/it/cli.rs @@ -162,6 +162,43 @@ fn mcp_over_stdio_sets_exposure_and_renders() { assert!(b > a + 10.0, "{a} → {b}"); } +#[cfg(unix)] +#[test] +fn non_utf8_argument_fails_without_a_panic() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + + let out = tmp("non-utf8-output.png"); + let _ = std::fs::remove_file(&out); + let output = Command::new(binary()).arg("render").arg(OsString::from_vec(b"bad-\xff.png".to_vec())).arg("-o").arg(&out).output().unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(1), "{stderr}"); + assert!(stderr.contains("argument 2"), "{stderr}"); + assert!(stderr.contains("is not valid Unicode"), "{stderr}"); + assert!(!stderr.contains("panicked"), "{stderr}"); + assert!(!out.exists()); + assert!(Command::new(binary()).arg("--version").status().unwrap().success()); +} + +#[cfg(windows)] +#[test] +fn non_unicode_argument_fails_without_a_panic_on_windows() { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + + let out = tmp("non-unicode-output.png"); + let _ = std::fs::remove_file(&out); + let name: Vec = "bad-".encode_utf16().chain([0xD800]).chain(".png".encode_utf16()).collect(); + let output = Command::new(binary()).arg("render").arg(OsString::from_wide(&name)).arg("-o").arg(&out).output().unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(1), "{stderr}"); + assert!(stderr.contains("argument 2"), "{stderr}"); + assert!(stderr.contains("is not valid Unicode"), "{stderr}"); + assert!(!stderr.contains("panicked"), "{stderr}"); + assert!(!out.exists()); + assert!(Command::new(binary()).arg("--version").status().unwrap().success()); +} + #[test] fn render_subcommand() { let input = tmp("r-in.png"); @@ -184,6 +221,12 @@ fn render_subcommand() { let o = Command::new(binary()).args(["render", input.to_str().unwrap(), "-o", out.to_str().unwrap(), "--set", "nope=1"]).output().unwrap(); assert!(!o.status.success()); assert!(String::from_utf8_lossy(&o.stderr).contains("unknown control")); + // Values that parse as f64 but are not finite fail cleanly too. + for bad in ["light.exposure=inf", "light.exposure=-inf", "light.exposure=NaN", "light.exposure=1e999"] { + let o = Command::new(binary()).args(["render", input.to_str().unwrap(), "-o", out.to_str().unwrap(), "--set", bad]).output().unwrap(); + assert!(!o.status.success(), "{bad} must be refused"); + assert!(String::from_utf8_lossy(&o.stderr).contains("not a finite number"), "{bad}"); + } } /// Issue #1730: `render --settings` took a `.lcpreset` from `preset.export`, ignored its wrapper @@ -502,6 +545,29 @@ fn snapshot_starts_without_a_gpu() { } } +/// A loopback port nothing listens on, for tests that need `--connect` to be refused (#3070). +/// A fixed port is not safe: agents give their apps' control servers ports in 18000–19999 (and +/// 7980 is the default), so a test could reach someone's running app and drive it. The OS hands +/// out a free port for `:0`; the listener is dropped so connecting to it is refused. Another +/// process could bind the port between the drop and the connect. That window is a few +/// milliseconds, and `:0` draws from the OS's ephemeral range (from 32768 on Linux, 49152 on +/// macOS and Windows), where nobody puts a control server (7980, 18000–19999): the standard +/// trade-off for a test that needs a refused connection. +/// +/// Within this binary the window is closed: the tests that take an ephemeral port and let it go +/// hold [`ephemeral_ports`], so `denoise-serve` can't start on a port a refused-connection test +/// picked while it runs. +fn unused_port() -> u16 { + std::net::TcpListener::bind("127.0.0.1:0").unwrap().local_addr().unwrap().port() +} + +/// Held by each test that picks an ephemeral port and releases it before use, for the whole test. +fn ephemeral_ports() -> std::sync::MutexGuard<'static, ()> { + static PORTS: std::sync::Mutex<()> = std::sync::Mutex::new(()); + // a test that failed while holding it doesn't fail the others + PORTS.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + fn run_cli(args: &[&str], stdin: Option<&str>) -> (bool, Vec, String) { let mut child = Command::new(binary()).arg("run").args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); if let Some(text) = stdin { @@ -513,6 +579,230 @@ fn run_cli(args: &[&str], stdin: Option<&str>) -> (bool, Vec, String) { (o.status.success(), lines, String::from_utf8_lossy(&o.stderr).to_string()) } +/// Windows PowerShell 5.1 writes a UTF-8 BOM; `--script` (file and stdin) must accept it. +#[test] +fn run_script_accepts_a_utf8_bom() { + let text = "\u{feff}{\"command\": \"library.info\", \"params\": {}}\n"; + let script = tmp("bom-steps.jsonl"); + std::fs::write(&script, text).unwrap(); + let (ok, lines, err) = run_cli(&["--demo", "--script", script.to_str().unwrap()], None); + assert!(ok, "file: {err}"); + assert_eq!(lines.len(), 1, "{lines:?}"); + let (ok, lines, err) = run_cli(&["--demo", "--script", "-"], Some(text)); + assert!(ok, "stdin: {err}"); + assert_eq!(lines.len(), 1, "{lines:?}"); +} + +#[test] +fn run_script_params_validate_before_any_commands() { + let path = tmp("run-invalid-params.jsonl"); + let path_str = path.to_str().unwrap(); + for params in [json!([]), json!("invalid"), json!(42), json!(true)] { + let script = format!("{}\n{}\n", json!({"command": "library.info"}), json!({"command": "develop.reset", "params": params})); + std::fs::write(&path, &script).unwrap(); + for (source, stdin) in [("-", Some(script.as_str())), (path_str, None)] { + let (ok, lines, err) = run_cli(&["--script", source], stdin); + assert!(!ok, "{source}: {params}: {lines:?}"); + assert!(lines.is_empty(), "{source}: {params}: {lines:?}"); + assert!(err.contains(&format!("{source}:2")), "{err}"); + assert!(err.contains("`params` must be a JSON object"), "{err}"); + } + } + for script in + ["{\"command\":\"library.info\"}\n", "{\"command\":\"library.info\",\"params\":null}\n", "{\"command\":\"library.info\",\"params\":{}}\n"] + { + let (ok, lines, err) = run_cli(&["--script", "-"], Some(script)); + assert!(ok, "{script}: {err}"); + assert_eq!(lines.len(), 1, "{script}: {lines:?}"); + assert_eq!(lines[0]["ok"], true, "{script}: {lines:?}"); + } + std::fs::remove_file(path).unwrap(); +} + +#[test] +fn run_failed_import_never_edits_existing_selection() { + let dir = tmp("failed-import"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let lib = dir.join("library"); + let missing = dir.join("missing.png"); + let empty = dir.join("empty"); + std::fs::create_dir(&empty).unwrap(); + assert!(!missing.exists()); + let lib = lib.to_str().unwrap(); + let (ok, lines, err) = run_cli(&["--demo", "--library", lib, "library.state"], None); + assert!(ok, "{err}"); + let id = lines[0]["result"]["selection"]["active"].as_u64().unwrap(); + let (ok, _, err) = run_cli(&["--library", lib, "photo.rate", &format!("ids=[{id}]"), "rating=0"], None); + assert!(ok, "{err}"); + for path in [&missing, &empty] { + let (ok, lines, err) = run_cli(&["--library", lib, "--import", path.to_str().unwrap(), "photo.rate", "rating=5"], None); + assert!(!ok, "import unexpectedly succeeded for {}: {lines:?}", path.display()); + assert!(lines.is_empty(), "commands ran: {lines:?}"); + assert!(err.contains("import") && err.contains("no commands were run"), "{err}"); + if path == &missing { + assert!(err.contains("failed") && err.contains("missing.png"), "{err}"); + } + let (ok, lines, err) = run_cli(&["--library", lib, "photo.inspect", &format!("id={id}")], None); + assert!(ok, "{err}"); + assert_eq!(lines[0]["result"]["rating"], 0); + } +} + +#[test] +fn run_partial_import_stops_commands_and_import_targets_remain_usable() { + let dir = tmp("partial-import"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let input = dir.join("valid.png"); + let broken = dir.join("broken.png"); + gradient_png(&input); + std::fs::write(&broken, b"not a PNG").unwrap(); + let mixed_lib = dir.join("mixed"); + let mixed_lib = mixed_lib.to_str().unwrap(); + let input = input.to_str().unwrap(); + let (ok, lines, err) = run_cli(&["--demo", "--library", mixed_lib, "catalog.query"], None); + assert!(ok, "{err}"); + let photos = lines[0]["result"]["photos"].as_array().unwrap(); + let a = photos[0]["id"].as_u64().unwrap(); + let b = photos[1]["id"].as_u64().unwrap(); + let (ok, lines, err) = run_cli( + &[ + "--library", + mixed_lib, + "library.select", + &format!("ids=[{a},{b}]"), + &format!("active={a}"), + "photo.rate", + &format!("ids=[{a}]"), + "rating=0", + "library.state", + ], + None, + ); + assert!(ok, "{err}"); + let selection = lines[2]["result"]["selection"].clone(); + assert_eq!(selection["ids"], json!([a, b])); + assert_eq!(selection["active"], a); + let (ok, lines, err) = + run_cli(&["--library", mixed_lib, "--import", input, "--import", broken.to_str().unwrap(), "photo.rate", "rating=5"], None); + let (query_ok, after, query_err) = + run_cli(&["--library", mixed_lib, "library.state", "photo.inspect", &format!("id={a}"), "catalog.query"], None); + assert!(query_ok, "{query_err}"); + assert_eq!(after[0]["result"]["selection"], selection, "selection changed after partial import"); + assert!(!ok, "partial import unexpectedly succeeded: {lines:?}"); + assert!(lines.is_empty() && err.contains("failed") && err.contains("no commands were run"), "{err}: {lines:?}"); + assert_eq!(after[1]["result"]["rating"], 0); + assert!(after[2]["result"]["photos"].as_array().unwrap().iter().any(|p| p["fileName"] == "valid.png")); + + let lib = dir.join("duplicates"); + let lib = lib.to_str().unwrap(); + let (ok, lines, err) = run_cli(&["--demo", "--library", lib, "library.state"], None); + assert!(ok, "{err}"); + let previous = lines[0]["result"]["selection"]["active"].as_u64().unwrap(); + let (ok, lines, err) = run_cli(&["--library", lib, "--import", input, "photo.rate", "rating=4", "library.state"], None); + assert!(ok, "{err}"); + let id = lines[1]["result"]["selection"]["active"].as_u64().unwrap(); + assert_ne!(id, previous); + let (ok, _, err) = run_cli(&["--library", lib, "library.select", &format!("ids=[{previous}]"), "photo.rate", "rating=0"], None); + assert!(ok, "{err}"); + let (ok, _, err) = run_cli(&["--library", lib, "--import", input, "photo.rate", "rating=3"], None); + assert!(ok, "{err}"); + let (ok, lines, err) = run_cli(&["--library", lib, "photo.inspect", &format!("id={id}"), "photo.inspect", &format!("id={previous}")], None); + assert!(ok, "{err}"); + assert_eq!(lines[0]["result"]["rating"], 3); + assert_eq!(lines[1]["result"]["rating"], 0); +} + +#[test] +fn run_restored_import_is_selected() { + let dir = tmp("restored-import"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let input = dir.join("valid.png"); + gradient_png(&input); + let lib = dir.join("library"); + let (lib, input) = (lib.to_str().unwrap(), input.to_str().unwrap()); + let (ok, lines, err) = run_cli(&["--library", lib, "--import", input, "library.state"], None); + assert!(ok, "{err}"); + let id = lines[0]["result"]["selection"]["active"].as_u64().unwrap(); + let (ok, _, err) = run_cli(&["--library", lib, "photo.delete"], None); + assert!(ok, "{err}"); + let (ok, _, err) = run_cli(&["--library", lib, "--import", input, "photo.rate", "rating=4"], None); + assert!(ok, "{err}"); + let (ok, lines, err) = run_cli(&["--library", lib, "catalog.query"], None); + assert!(ok, "{err}"); + assert_eq!(lines[0]["result"]["photos"].as_array().unwrap().len(), 1); + assert_eq!(lines[0]["result"]["photos"][0]["id"], id); + assert_eq!(lines[0]["result"]["photos"][0]["rating"], 4); +} + +#[test] +fn run_same_batch_duplicates_selects_photo_once() { + let dir = tmp("same-batch-duplicates"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let first = dir.join("first.png"); + let second = dir.join("second.png"); + gradient_png(&first); + std::fs::copy(&first, &second).unwrap(); + let library = dir.join("library"); + let (ok, lines, err) = run_cli( + &[ + "--library", + library.to_str().unwrap(), + "--import", + first.to_str().unwrap(), + "--import", + second.to_str().unwrap(), + "photo.rate", + "rating=4", + "library.state", + "catalog.query", + ], + None, + ); + assert!(ok, "{err}"); + let selection = &lines[1]["result"]["selection"]; + let ids = selection["ids"].as_array().unwrap(); + assert_eq!(ids.len(), 1, "same photo selected more than once: {selection}"); + assert_eq!(selection["active"], ids[0]); + let photos = lines[2]["result"]["photos"].as_array().unwrap(); + assert_eq!(photos.len(), 1); + assert_eq!(photos[0]["id"], ids[0]); + assert_eq!(photos[0]["rating"], 4); + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn run_mixed_new_and_restored_import_selects_both() { + let dir = tmp("mixed-restored-import"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let restored = dir.join("restored.png"); + let new = dir.join("new.png"); + gradient_png(&restored); + let image = lightcraft_raster::Rgba8::from_fn(120, 80, |x, y| [(x * 2) as u8, (y * 3) as u8, 200, 255]); + let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&image), &Default::default()).unwrap(); + std::fs::write(&new, png).unwrap(); + let lib = dir.join("library"); + let (lib, restored, new) = (lib.to_str().unwrap(), restored.to_str().unwrap(), new.to_str().unwrap()); + let (ok, lines, err) = run_cli(&["--library", lib, "--import", restored, "library.state"], None); + assert!(ok, "{err}"); + let restored_id = lines[0]["result"]["selection"]["active"].as_u64().unwrap(); + let (ok, _, err) = run_cli(&["--library", lib, "photo.delete"], None); + assert!(ok, "{err}"); + let (ok, lines, err) = run_cli(&["--library", lib, "--import", new, "--import", restored, "photo.rate", "rating=4", "library.state"], None); + assert!(ok, "{err}"); + assert_eq!(lines[1]["result"]["selection"]["ids"].as_array().unwrap().len(), 2); + let (ok, lines, err) = run_cli(&["--library", lib, "catalog.query"], None); + assert!(ok, "{err}"); + let photos = lines[0]["result"]["photos"].as_array().unwrap(); + assert_eq!(photos.len(), 2); + assert!(photos.iter().any(|p| p["id"] == restored_id && p["rating"] == 4)); + assert!(photos.iter().any(|p| p["fileName"] == "new.png" && p["id"] != restored_id && p["rating"] == 4)); +} + #[test] fn run_subcommand_chains_commands_and_persists_a_library() { let input = tmp("run-in.png"); @@ -614,9 +904,11 @@ fn a_library_open_in_another_process_is_refused() { /// ADDR actually names — following the old fixed "7980" left the retry connecting elsewhere. #[test] fn connect_failure_hint_names_the_attempted_port() { - let (ok, _lines, err) = run_cli(&["--connect", "127.0.0.1:18437", "ui.inspect"], None); + let _ports = ephemeral_ports(); + let port = unused_port(); + let (ok, _lines, err) = run_cli(&["--connect", &format!("127.0.0.1:{port}"), "ui.inspect"], None); assert!(!ok); - assert!(err.contains("--control 18437"), "the hint must name the port it tried: {err}"); + assert!(err.contains(&format!("--control {port}")), "the hint must name the port it tried: {err}"); assert!(!err.contains("--control 7980"), "{err}"); } @@ -626,20 +918,23 @@ fn connect_failure_hint_names_the_attempted_port() { /// works; non-numeric, non-`ADDR`-shaped tokens (e.g. a real command id) keep falling through. #[test] fn connect_bare_port_is_treated_as_loopback_port() { + let _ports = ephemeral_ports(); + let port = unused_port(); + let hint = format!("--control {port}"); // Bare port: the failure hint names that port. - let (ok, _lines, err) = run_cli(&["--connect", "18437", "ui.inspect"], None); + let (ok, _lines, err) = run_cli(&["--connect", &port.to_string(), "ui.inspect"], None); assert!(!ok); - assert!(err.contains("--control 18437"), "bare port must reach the hint: {err}"); + assert!(err.contains(&hint), "bare port must reach the hint: {err}"); assert!(!err.contains("--control 7980"), "{err}"); // `:` still works (issue #167 case). - let (ok, _lines, err) = run_cli(&["--connect", "127.0.0.1:18437", "ui.inspect"], None); + let (ok, _lines, err) = run_cli(&["--connect", &format!("127.0.0.1:{port}"), "ui.inspect"], None); assert!(!ok); - assert!(err.contains("--control 18437"), "{err}"); + assert!(err.contains(&hint), "{err}"); // `--connect=` already worked (the `a.starts_with("--connect=")` arm); spot-check // the hint matches its port too. - let (ok, _lines, err) = run_cli(&["--connect=18437", "ui.inspect"], None); + let (ok, _lines, err) = run_cli(&[&format!("--connect={port}"), "ui.inspect"], None); assert!(!ok); - assert!(err.contains("--control 18437"), "{err}"); + assert!(err.contains(&hint), "{err}"); } /// The CLI logs warnings on stderr (#168): with `RUST_LOG=warn` an unknown GPU backend name is @@ -785,7 +1080,8 @@ fn denoise_serve_runs_tiles_for_another_computer() { Command::new(binary()).args(["denoise-serve", "--models", models.to_str().unwrap(), "--token-env", "LC_TEST_NO_SUCH_VAR"]).output().unwrap(); assert!(!o.status.success() && String::from_utf8_lossy(&o.stderr).contains("LC_TEST_NO_SUCH_VAR")); - let port = std::net::TcpListener::bind("127.0.0.1:0").unwrap().local_addr().unwrap().port(); + let _ports = ephemeral_ports(); + let port = unused_port(); let addr = format!("127.0.0.1:{port}"); let token = "cli-test-token-0123456789"; let mut child = Command::new(binary()) diff --git a/apps/lightcraft/src/control_server.rs b/apps/lightcraft/src/control_server.rs index 95aa10896..f60669640 100644 --- a/apps/lightcraft/src/control_server.rs +++ b/apps/lightcraft/src/control_server.rs @@ -87,7 +87,7 @@ fn serve(stream: TcpStream, tx: &Sender, ctx: &egui::Context, id log::warn!("control connection closed: could not set idle read timeout: {e}"); return; } - serve_lines(&stream, &stream, |method, params| { + serve_stream(stream, |method, params| { let (req, rrx) = ControlRequest::new(method, params); let state = Arc::clone(&req.state); tx.send(req).ok()?; @@ -96,6 +96,40 @@ fn serve(stream: TcpStream, tx: &Sender, ctx: &egui::Context, id }); } +/// The longest a closing connection is drained, however slowly its peer keeps sending. +const DRAIN_TIME: Duration = Duration::from_secs(1); + +/// [`serve_lines`] on a socket, then close it so the peer reads every reply. +fn serve_stream(stream: TcpStream, handle: impl FnMut(String, Value) -> Option) { + serve_lines(&stream, &stream, handle); + close_gracefully(&stream); +} + +/// Close a connection the server is done with without destroying its last reply. A socket closed +/// while the peer's later lines are unread, or arrive after it, answers them with a reset, and a +/// reset can discard the replies the peer has not read yet (on Windows it does): the client sees +/// "connection reset" instead of why. Send FIN, then read and drop what is left (at most +/// [`MAX_LINE`] bytes, 500 ms per read and [`DRAIN_TIME`] in all) before closing. From #1970 +/// (LochLogic). +fn close_gracefully(stream: &TcpStream) { + let _ = stream.shutdown(std::net::Shutdown::Write); + let start = std::time::Instant::now(); + let mut sink = [0u8; 8192]; + let mut left = MAX_LINE; + let mut reader = stream; + while left > 0 { + // a peer that keeps trickling bytes must not hold the connection slot past DRAIN_TIME + let wait = DRAIN_TIME.saturating_sub(start.elapsed()).min(Duration::from_millis(500)); + if wait.is_zero() || stream.set_read_timeout(Some(wait)).is_err() { + break; + } + match reader.read(&mut sink) { + Ok(0) | Err(_) => break, + Ok(n) => left = left.saturating_sub(n), + } + } +} + /// Wait for a request's reply. While the app has not picked the request up and `frames` (its /// frame counter) stands still for `no_frames`, the request is abandoned (it will not run late) /// and the reply says why; a request the app is running gets the whole `timeout`. @@ -170,15 +204,40 @@ fn parse_line(bytes: &[u8]) -> Line { } } +/// The error of the notice sent when an idle connection is closed. Clients match the reply's +/// `"closed": "idle"` marker; `crates/mcp/src/backend.rs` also matches this text, for apps from +/// before the marker, so keep the two in step. +const IDLE_CLOSE_ERROR: &str = "control connection idle read timed out; closing the connection"; + +/// Why the server closes a connection; its error reply says so. +#[derive(Debug, PartialEq)] +enum Closing { + /// Nothing complete arrived within the idle timeout. Nothing more is read, so a request that + /// crosses the notice is never run: the reply carries `"closed": "idle"` to tell clients it is + /// safe to send that request again on a new connection. + Idle, + /// Junk, an overlong line or a failed read. + Error(String), +} + +impl Closing { + fn reply(&self) -> Value { + match self { + Self::Idle => json!({"ok": false, "error": IDLE_CLOSE_ERROR, "closed": "idle"}), + Self::Error(e) => json!({"ok": false, "error": e}), + } + } +} + /// Read one `\n`-terminated line of at most `max` bytes into `buf` (newline stripped). /// `Ok(false)` at end of stream; `Err` when the line is too long or the read fails. -fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec, max: usize) -> Result { +fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec, max: usize) -> Result { buf.clear(); // `take` bounds the bytes buffered for one line: a peer can't make us allocate more than `max`. let limit = u64::try_from(max).unwrap_or(u64::MAX).saturating_add(1); let n = reader.take(limit).read_until(b'\n', buf).map_err(|e| match e.kind() { - std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut => "control connection idle read timed out; closing the connection".to_string(), - _ => e.to_string(), + std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut => Closing::Idle, + _ => Closing::Error(e.to_string()), })?; if n == 0 { return Ok(false); @@ -191,7 +250,7 @@ fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec, max: usize) - return Ok(true); } if buf.len() > max { - return Err(format!("request line longer than {max} bytes; closing the connection")); + return Err(Closing::Error(format!("request line longer than {max} bytes; closing the connection"))); } // Last line without a newline, then end of stream. Ok(true) @@ -204,12 +263,12 @@ fn serve_lines(read: R, mut out: W, mut handle: impl FnMut(St let mut reader = BufReader::new(read); let mut buf = Vec::new(); loop { - let reject = match read_bounded_line(&mut reader, &mut buf, MAX_LINE) { + let closing = match read_bounded_line(&mut reader, &mut buf, MAX_LINE) { Ok(false) => return, - Err(e) => e, + Err(closing) => closing, Ok(true) => match parse_line(&buf) { Line::Blank => continue, - Line::Reject(e) => e, + Line::Reject(e) => Closing::Error(e), Line::Request { id, method, params } => { let Some(mut r) = handle(method, params) else { return }; if let Some(o) = r.as_object_mut() { @@ -222,7 +281,7 @@ fn serve_lines(read: R, mut out: W, mut handle: impl FnMut(St } }, }; - let _ = writeln!(out, "{}", json!({"ok": false, "error": reject})).and_then(|()| out.flush()); + let _ = writeln!(out, "{}", closing.reply()).and_then(|()| out.flush()); return; } } @@ -287,7 +346,9 @@ mod tests { assert!(connection.requests.try_recv().is_err(), "incomplete input never runs"); let reply: Value = serde_json::from_str(&reply).unwrap(); assert_eq!(reply["ok"], false); - assert!(reply["error"].as_str().unwrap().contains("idle read timed out")); + assert_eq!(reply["error"], IDLE_CLOSE_ERROR); + assert_eq!(reply["closed"], "idle", "clients may resend a request that crossed this notice"); + assert!(reply.get("id").is_none()); // A subsequent connection is admitted through the same accounting path. let mut next = Connection::new(&open, Duration::from_secs(1), |c| c.shutdown(std::net::Shutdown::Write).unwrap()); @@ -413,6 +474,7 @@ mod tests { assert_eq!(r.len(), 1); assert_eq!(r[0]["ok"], false); assert!(r[0]["error"].as_str().unwrap().contains("closing the connection")); + assert!(r[0].get("closed").is_none(), "only an idle close invites a resend: {}", r[0]); } #[test] @@ -502,18 +564,52 @@ mod tests { let addr = listener.local_addr().unwrap(); let server = std::thread::spawn(move || { let (s, _) = listener.accept().unwrap(); - let read = s.try_clone().unwrap(); - serve_lines(read, s, |_, _| Some(json!({"ok": true}))); + serve_stream(s, |_, _| Some(json!({"ok": true}))); }); let mut c = TcpStream::connect(addr).unwrap(); c.set_read_timeout(Some(Duration::from_secs(10))).unwrap(); - c.write_all(b"{\"id\":7,\"method\":\"x\"}\nhello\n{\"method\":\"y\"}\n").unwrap(); - let mut text = String::new(); - c.read_to_string(&mut text).unwrap(); // returns at EOF: the server closed the connection + c.write_all(b"{\"id\":7,\"method\":\"x\"}\nhello\n").unwrap(); + let mut replies = BufReader::new(c.try_clone().unwrap()); + let (mut first, mut second) = (String::new(), String::new()); + replies.read_line(&mut first).unwrap(); + replies.read_line(&mut second).unwrap(); + assert!(first.contains("\"id\":7"), "{first}"); + assert!(second.contains("closing the connection"), "{second}"); + // A client that keeps sending after the junk: its next line reaches the server after the + // rejection. A socket closed by then answers that line with a reset, which the client reads + // as "connection reset" (os error 10053 / 10054 on Windows) instead of the end of the + // replies. The pause lets a reset arrive before the read. + c.write_all(b"{\"method\":\"y\"}\n").unwrap(); + std::thread::sleep(Duration::from_millis(100)); + let mut rest = String::new(); + let read = replies.read_to_string(&mut rest); server.join().unwrap(); - let lines: Vec<&str> = text.lines().collect(); - assert_eq!(lines.len(), 2, "{text}"); - assert!(lines[0].contains("\"id\":7")); - assert!(lines[1].contains("closing the connection")); + assert!(read.is_ok(), "the close must be clean, not a reset: {read:?}"); + assert_eq!(rest, "", "nothing follows the rejection"); + } + + #[test] + fn a_closing_connection_is_not_held_open_by_a_trickling_peer() { + let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let addr = listener.local_addr().unwrap(); + let done = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let server_done = Arc::clone(&done); + let server = std::thread::spawn(move || { + let (s, _) = listener.accept().unwrap(); + let t0 = std::time::Instant::now(); + close_gracefully(&s); + server_done.store(true, Ordering::SeqCst); + t0.elapsed() + }); + let mut c = TcpStream::connect(addr).unwrap(); + // one byte every 100 ms: each read returns well within its 500 ms timeout + for _ in 0..100 { + if done.load(Ordering::SeqCst) || c.write_all(b"x").is_err() { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + let drained = server.join().unwrap(); + assert!(drained < DRAIN_TIME + Duration::from_secs(1), "drained for {drained:?}"); } } diff --git a/crates/develop/src/segmask.rs b/crates/develop/src/segmask.rs index 172d53fce..de0aede09 100644 --- a/crates/develop/src/segmask.rs +++ b/crates/develop/src/segmask.rs @@ -104,9 +104,14 @@ fn base64_encode(bytes: &[u8]) -> String { fn base64_decode(s: &str) -> Option> { let mut out = Vec::with_capacity(s.len() / 4 * 3); let (mut acc, mut bits) = (0u32, 0u32); + let mut padded = false; for c in s.bytes().filter(|c| !c.is_ascii_whitespace()) { if c == b'=' { - break; + padded = true; + continue; + } + if padded { + return None; } let v = B64.iter().position(|b| *b == c)? as u32; acc = (acc << 6) | v; @@ -117,6 +122,9 @@ fn base64_decode(s: &str) -> Option> { acc &= (1 << bits) - 1; } } + if bits == 6 { + return None; + } Some(out) } @@ -133,6 +141,10 @@ mod tests { assert_eq!(base64_encode(b"Man"), "TWFu"); assert_eq!(base64_encode(b"Ma"), "TWE="); assert!(base64_decode("TW!u").is_none()); + assert!(base64_decode("Z").is_none()); + assert!(base64_decode("Zm9vY").is_none()); + assert!(base64_decode("Zg==!!!not-base64").is_none()); + assert_eq!(base64_decode("Zg==\n").unwrap(), b"f"); } #[test] diff --git a/crates/engine/src/cmd/missing.rs b/crates/engine/src/cmd/missing.rs index c06997eef..e5a17ab5d 100644 --- a/crates/engine/src/cmd/missing.rs +++ b/crates/engine/src/cmd/missing.rs @@ -41,7 +41,77 @@ pub fn missing(s: &Session) -> Vec<(PhotoId, String)> { if cfg!(target_arch = "wasm32") { return Vec::new(); } - missing_with(&s.catalog, |f| Path::new(f).exists()) + let exists = s.media.availability.probe(); + missing_with(&s.catalog, |f| exists(f)) +} + +/// Every in-scope photo with its file ([`checked_path`]), without touching the disk: what a +/// sweep ([`sweep`]) goes through, so a caller can take the list on its own thread and check +/// the files on another. +pub fn checked(cat: &Catalog) -> Vec<(PhotoId, String)> { + cat.photos().filter_map(|p| checked_path(p).map(|f| (p.id, f.to_string()))).collect() +} + +fn fingerprint_of<'a>(entries: impl Iterator) -> u64 { + use std::hash::{DefaultHasher, Hash, Hasher}; + entries.fold(0u64, |sum, entry| { + let mut h = DefaultHasher::new(); + entry.hash(&mut h); + sum.wrapping_add(h.finish()) + }) +} + +/// What [`checked`] lists, as one number (the photos in scope and the file each one points at, +/// whatever their order), without building the list. A caller that took `checked` and checked its +/// files on another thread compares it with [`fingerprint_of_checked`] to know whether the photos +/// changed meanwhile. +pub fn fingerprint(cat: &Catalog) -> u64 { + fingerprint_of(cat.photos().filter_map(|p| checked_path(p).map(|f| (p.id, f)))) +} + +/// [`fingerprint`] of the list [`checked`] gave. +pub fn fingerprint_of_checked(files: &[(PhotoId, String)]) -> u64 { + fingerprint_of(files.iter().map(|(id, f)| (*id, f.as_str()))) +} + +/// What of `gone` (a sweep's answer, of the catalog as it was) still holds in `cat` as it is now: the +/// photos still in scope that still point at the file that was not found. A photo removed, moved to +/// Recently Deleted or relinked since is not reported as missing; one added since was not checked. +pub fn still_missing(cat: &Catalog, gone: Vec<(PhotoId, String)>) -> Vec<(PhotoId, String)> { + gone.into_iter().filter(|(id, file)| cat.photo(*id).and_then(|p| checked_path(p)) == Some(file.as_str())).collect() +} + +/// How many files [`sweep`] checks between two reports of its progress. +const PROGRESS_EVERY: usize = 64; + +/// The files among `files` that `exists` doesn't find, in order: [`missing`] for a caller that +/// checks on a thread of its own. `stop` is asked before every check (`None`: it said yes, and +/// the rest was not looked at); `progress` is told how many files are done every few dozen. +pub fn sweep( + files: Vec<(PhotoId, String)>, + exists: impl Fn(&str) -> bool, + stop: impl Fn() -> bool, + progress: impl Fn(usize), +) -> Option> { + let mut gone = Vec::new(); + for (done, (id, file)) in files.into_iter().enumerate() { + if stop() { + return None; + } + if done % PROGRESS_EVERY == 0 { + progress(done); + } + if !exists(&file) { + gone.push((id, file)); + } + } + // (a stop that came during the last check is honoured too) + (!stop()).then_some(gone) +} + +/// `library.missing`'s answer: `[{id, path}]`. +pub fn missing_json(gone: Vec<(PhotoId, String)>) -> Value { + Value::Array(gone.into_iter().map(|(id, path)| json!({"id": id.0, "path": path})).collect()) } /// Whether photo `id` is in scope and its file is gone (the Missing Photos view checks only @@ -418,8 +488,8 @@ fn find_missing(s: &mut Session, p: &Value) -> Result { pub fn specs() -> Vec { vec![ - cmd!(query "library.missing", "Missing Photos", [], None, "{} → [{id, path}] — photos whose original file isn't where the library expects it", always, |s, _| { - Ok(Value::Array(missing(s).into_iter().map(|(id, path)| json!({"id": id.0, "path": path})).collect())) + cmd!(query "library.missing", "Missing Photos", [], None, "{} → [{id, path}] — photos whose original file isn't where the library expects it (every file is looked at: on a network share that takes minutes, so the desktop app, asked over its control channel, checks on a worker thread, with a row in the activity stack that can stop it, and answers when it is done)", always, |s, _| { + Ok(missing_json(missing(s))) }), cmd!( "photo.relink", diff --git a/crates/engine/src/tests/import.rs b/crates/engine/src/tests/import.rs index 478f99b1d..72cfd7ef7 100644 --- a/crates/engine/src/tests/import.rs +++ b/crates/engine/src/tests/import.rs @@ -1473,6 +1473,76 @@ fn missing_photos_skip_local_browse_records() { assert_eq!(s.visible_cloned().len(), 4, "the view lists what the count counts"); } +/// `library.missing` asks the session's existence check (the one the availability cache and +/// the tests replace), and a sweep on another thread answers what the command does: the same +/// files in the same order, progress on the way, and a stop that leaves the rest unchecked. +#[test] +fn missing_sweep_answers_what_the_command_does_and_can_stop() { + use lightcraft_catalog::{Op, Photo, PhotoId, Source}; + use std::sync::atomic::{AtomicUsize, Ordering}; + let mut s = Session::new(); + let mut ops = Vec::new(); + for i in 0..200u64 { + let path = format!("/share/IMG_{i:04}.jpg"); + let p = Photo::new(PhotoId(i + 1), Source::File { path }, &format!("IMG_{i:04}.jpg"), "JPEG", 60, 40, "2026-01-01T00:00:00"); + ops.push(Op::AddPhoto { photo: Box::new(p) }); + } + s.commit("Add", Op::Batch { ops }).unwrap(); + // the first hundred files are gone + s.media.availability.set_probe(std::sync::Arc::new(|f| !f.contains("_00"))); + let asked = s.execute("library.missing", &json!({})).unwrap(); + assert_eq!(asked.as_array().map(Vec::len), Some(100), "the command used the session's check: {asked}"); + + let files = crate::cmd::missing::checked(&s.catalog); + assert_eq!(files.len(), 200); + let reports = AtomicUsize::new(0); + let probe = s.media.availability.probe(); + let swept = crate::cmd::missing::sweep( + files.clone(), + |f| probe(f), + || false, + |_| { + reports.fetch_add(1, Ordering::Relaxed); + }, + ); + assert_eq!(crate::cmd::missing::missing_json(swept.unwrap()), asked); + assert!(reports.load(Ordering::Relaxed) >= 2, "progress is reported on the way"); + + // stopped after 10 files: nothing is answered, and the rest is not looked at + let looked = AtomicUsize::new(0); + let stopped = crate::cmd::missing::sweep( + files, + |_| { + looked.fetch_add(1, Ordering::Relaxed); + true + }, + || looked.load(Ordering::Relaxed) >= 10, + |_| {}, + ); + assert_eq!((stopped, looked.load(Ordering::Relaxed)), (None, 10)); +} + +/// A stop that arrives while the last file is being checked is honoured: no answer. +#[test] +fn missing_sweep_honours_a_stop_during_the_last_check() { + use lightcraft_catalog::PhotoId; + use std::sync::atomic::{AtomicBool, Ordering}; + let stop = AtomicBool::new(false); + let files = vec![(PhotoId(1), "/a".to_string()), (PhotoId(2), "/b".to_string())]; + let swept = crate::cmd::missing::sweep( + files, + |f| { + if f == "/b" { + stop.store(true, Ordering::Relaxed); + } + false + }, + || stop.load(Ordering::Relaxed), + |_| {}, + ); + assert_eq!(swept, None); +} + /// `library.browse` follows `library.source`'s rule for the active photo (issue #1447): a library /// photo the folder doesn't show gives way to the folder's first photo; one it shows stays. #[test] diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 6a2c2efb9..a621f4037 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -136,39 +136,58 @@ fn peer_closed(stream: &TcpStream) -> bool { stream.set_nonblocking(false).is_err() || !open } +/// The app's idle-close notice (`apps/lightcraft/src/control_server.rs`), matched when a reply lacks +/// the `closed` marker (an app from before it). Keep the two in step. +const IDLE_CLOSE_ERROR: &str = "control connection idle read timed out; closing the connection"; + +/// True for the app's idle-close notice: an id-less error the app writes when it closes a connection +/// on which it read nothing for its idle timeout. Read in reply to a request, it means the app closed +/// the connection without reading the request (the request crossed the notice), so nothing ran. +fn is_idle_close(reply: &Value) -> bool { + reply.get("id").is_none() + && reply.get("ok").and_then(Value::as_bool) == Some(false) + && (reply.get("closed").and_then(Value::as_str) == Some("idle") || reply.get("error").and_then(Value::as_str) == Some(IDLE_CLOSE_ERROR)) +} + impl Backend for Remote { fn call(&mut self, method: &str, params: Value) -> Result { let id = self.next_id; self.next_id += 1; let line = json!({"id": id, "method": method, "params": params}).to_string(); - // Retry once with a fresh connection (the app may have restarted), but only when the request - // never reached the app. After a full write the command may have run, so resending it could - // apply a mutation twice (toggle, undo, ...): report the unknown outcome instead. - let reply = match self.roundtrip(&line) { - Ok(r) => r, - Err((_, false)) => { - self.conn = None; - self.roundtrip(&line).map_err(|(e, sent)| { + // Send again once, on a fresh connection, but only when the request never reached the app: it + // could not be written (the app restarted), or the app's idle-close notice crossed it (the app + // stopped reading before it arrived). After a full write anything else may mean the command + // ran, so resending it could apply a mutation twice (toggle, undo, ...): report the unknown + // outcome instead. One resend at most, whatever the reason, so this can't loop. + let mut resent = false; + let v = loop { + let reply = match self.roundtrip(&line) { + Ok(r) => r, + Err((e, sent)) => { self.conn = None; if sent { - self.unknown_outcome(method, &e) - } else { - format!("LightCraft app at {} is not reachable: {e} (start it with `lightcraft --control PORT`)", self.addr) + return Err(self.unknown_outcome(method, &e)); } - })? - } - Err((e, true)) => { + if resent { + return Err(format!("LightCraft app at {} is not reachable: {e} (start it with `lightcraft --control PORT`)", self.addr)); + } + resent = true; + continue; + } + }; + let v = parse_reply(&reply, id).map_err(|e| { self.conn = None; - return Err(self.unknown_outcome(method, &e)); + self.unknown_outcome(method, &e) + })?; + if v.get("id").is_none() { + self.conn = None; + if !resent && is_idle_close(&v) { + resent = true; + continue; + } } + break v; }; - let v = parse_reply(&reply, id).map_err(|e| { - self.conn = None; - self.unknown_outcome(method, &e) - })?; - if v.get("id").is_none() { - self.conn = None; - } if v.get("ok").and_then(Value::as_bool) == Some(true) { Ok(v.get("result").cloned().unwrap_or(Value::Null)) } else { @@ -275,6 +294,11 @@ mod tests { ("{\"id\":1,\"ok\":true,\"result\":null}\r\n", Ok(Value::Null)), ("{\"id\":1,\"ok\":false,\"error\":\"unknown command\"}\n", Err("unknown command".to_string())), ("{\"ok\":false,\"error\":\"too many control connections\"}\n", Err("too many control connections".to_string())), + // The idle-close text in reply to the request (it has the id) is the command's own error. + ( + "{\"id\":1,\"ok\":false,\"error\":\"control connection idle read timed out; closing the connection\"}\n", + Err(IDLE_CLOSE_ERROR.to_string()), + ), ] { with_reply(reply.as_bytes(), |remote, result| { assert_eq!(result, expected); @@ -380,51 +404,148 @@ mod tests { assert_eq!(seen.load(Ordering::SeqCst), 2); } + /// Accept the next connection within 5 s, as a blocking stream with a 3 s read timeout. + fn accept(listener: &TcpListener) -> (BufReader, TcpStream) { + let deadline = std::time::Instant::now() + Duration::from_secs(5); + let stream = loop { + match listener.accept() { + Ok((stream, _)) => break stream, + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock && std::time::Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(5)); + } + Err(e) => panic!("accept: {e}"), + } + }; + // BSD kernels (macOS, FreeBSD) let an accepted socket inherit the listener's non-blocking flag. + stream.set_nonblocking(false).unwrap(); + stream.set_read_timeout(Some(Duration::from_secs(3))).unwrap(); + (BufReader::new(stream.try_clone().unwrap()), stream) + } + + fn read_request(reader: &mut BufReader) -> Value { + let mut line = String::new(); + reader.read_line(&mut line).unwrap(); + serde_json::from_str(&line).unwrap() + } + + /// The app's idle-close notice, as `apps/lightcraft/src/control_server.rs` writes it. + fn idle_notice() -> Value { + json!({"ok": false, "error": IDLE_CLOSE_ERROR, "closed": "idle"}) + } + + /// Issue #3069. The app closes an idle connection with an id-less notice. Either the notice + /// arrives before the next request (the client sees the close and reconnects before sending), + /// or the request crosses it (sent just as the app stopped reading): the notice then comes + /// back as the request's reply although the app never ran it, so the client sends it once more + /// on a new connection. Both orders are forced here, so the test does not depend on timing. #[test] fn idle_close_reconnects_without_replaying_a_command() { + let _serial = crate::serial_sockets_and_children(); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); listener.set_nonblocking(true).unwrap(); let addr = listener.local_addr().unwrap().to_string(); let (closed, expired) = std::sync::mpsc::channel(); let server = std::thread::spawn(move || { - let mut ids = Vec::new(); - for _ in 0..2 { - let deadline = std::time::Instant::now() + Duration::from_secs(5); - let stream = loop { - match listener.accept() { - Ok((stream, _)) => break stream, - Err(e) if e.kind() == std::io::ErrorKind::WouldBlock && std::time::Instant::now() < deadline => { - std::thread::sleep(Duration::from_millis(5)); - } - Err(e) => panic!("accept: {e}"), - } - }; - stream.set_read_timeout(Some(Duration::from_secs(3))).unwrap(); - let mut reader = BufReader::new(stream.try_clone().unwrap()); - let mut writer = stream; - let mut line = String::new(); - reader.read_line(&mut line).unwrap(); - let request: Value = serde_json::from_str(&line).unwrap(); - ids.push(request["id"].as_u64().unwrap()); - writeln!(writer, "{}", json!({"id": request["id"], "ok": true, "result": ids.len()})).unwrap(); - writer.set_read_timeout(Some(Duration::from_millis(50))).unwrap(); - line.clear(); - let error = reader.read_line(&mut line).unwrap_err(); - assert!(matches!(error.kind(), std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut)); - assert!(line.is_empty(), "no command is resent while idle"); - writeln!(writer, "{}", json!({"ok": false, "error": "control connection idle read timed out; closing the connection"})).unwrap(); - drop(reader); - drop(writer); - closed.send(()).unwrap(); - } - ids + let mut ran = Vec::new(); + let mut run = |request: &Value, writer: &mut TcpStream| { + ran.push(request["id"].as_u64().unwrap()); + writeln!(writer, "{}", json!({"id": request["id"], "ok": true, "result": ran.len()})).unwrap(); + }; + + // 1. Idle with nothing sent: the notice (in an older app's form, without `closed`) and + // the close reach the client before its next request. + let (mut reader, mut writer) = accept(&listener); + run(&read_request(&mut reader), &mut writer); + writer.set_read_timeout(Some(Duration::from_millis(50))).unwrap(); + let mut line = String::new(); + let error = reader.read_line(&mut line).unwrap_err(); + assert!(matches!(error.kind(), std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut)); + assert!(line.is_empty(), "no command is resent while idle"); + writeln!(writer, "{}", json!({"ok": false, "error": IDLE_CLOSE_ERROR})).unwrap(); + drop((reader, writer)); + closed.send(()).unwrap(); + + // 2. The client reconnected before sending. Its next request crosses the notice: the + // app's read timed out just before the request arrived, so it is not run. (Reading + // it here only shows that it was sent on this connection and keeps the close clean.) + let (mut reader, mut writer) = accept(&listener); + run(&read_request(&mut reader), &mut writer); + let crossed = read_request(&mut reader); + writeln!(writer, "{}", idle_notice()).unwrap(); + drop((reader, writer)); + + // 3. The crossed request, sent once more on a new connection, and nothing after it. + let (mut reader, mut writer) = accept(&listener); + let resent = read_request(&mut reader); + assert_eq!(resent, crossed, "the same request is sent again"); + run(&resent, &mut writer); + line.clear(); + assert_eq!(reader.read_line(&mut line).unwrap(), 0, "nothing else arrives: {line}"); + ran }); let mut remote = Remote::lazy(&addr); assert_eq!(remote.call("engine.execute", json!({"command": "edit.undo"})).unwrap(), json!(1)); expired.recv_timeout(Duration::from_secs(5)).unwrap(); + let deadline = std::time::Instant::now() + Duration::from_secs(5); + while !remote.conn.as_ref().is_some_and(|(_, writer)| peer_closed(writer)) { + assert!(std::time::Instant::now() < deadline, "the idle close never reached the client"); + std::thread::sleep(Duration::from_millis(1)); + } assert_eq!(remote.call("engine.execute", json!({"command": "edit.undo"})).unwrap(), json!(2)); - expired.recv_timeout(Duration::from_secs(5)).unwrap(); - assert_eq!(server.join().unwrap(), [1, 2], "each command ran once on a separate connection"); + assert_eq!(remote.call("engine.execute", json!({"command": "edit.undo"})).unwrap(), json!(3)); + drop(remote); + assert_eq!(server.join().unwrap(), [1, 2, 3], "each command ran once"); + } + + /// The resend after an idle close happens once: a second notice is the call's error. + #[test] + fn idle_close_is_resent_only_once() { + let _serial = crate::serial_sockets_and_children(); + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let server = std::thread::spawn(move || { + for _ in 0..2 { + let (mut reader, mut writer) = accept(&listener); + assert_eq!(read_request(&mut reader)["id"], 1); + writeln!(writer, "{}", idle_notice()).unwrap(); + } + listener + }); + let mut remote = Remote::lazy(&addr); + assert_eq!(remote.call("engine.execute", json!({"command": "edit.undo"})), Err(IDLE_CLOSE_ERROR.to_string())); + assert!(remote.conn.is_none(), "the closed connection is not reused"); + drop(remote); + let listener = server.join().unwrap(); + assert_eq!(listener.accept().unwrap_err().kind(), std::io::ErrorKind::WouldBlock, "a third connection was opened"); + } + + #[test] + fn only_the_idle_close_notice_counts_as_undelivered() { + let idle = [ + json!({"ok": false, "error": IDLE_CLOSE_ERROR, "closed": "idle"}), + // an app from before the `closed` marker + json!({"ok": false, "error": IDLE_CLOSE_ERROR}), + // the marker decides when present, whatever the wording + json!({"ok": false, "error": "idle, closing", "closed": "idle"}), + ]; + for reply in idle { + assert!(is_idle_close(&reply), "{reply}"); + } + let delivered = [ + // a reply to the request itself + json!({"id": 1, "ok": false, "error": IDLE_CLOSE_ERROR}), + json!({"id": 1, "ok": false, "error": IDLE_CLOSE_ERROR, "closed": "idle"}), + // other refusals and closes + json!({"ok": false, "error": "too many control connections"}), + json!({"ok": false, "error": "bad JSON; closing the connection"}), + json!({"ok": false, "error": "bad JSON; closing the connection", "closed": "junk"}), + json!({"ok": false, "error": format!("{IDLE_CLOSE_ERROR}.")}), + json!({"ok": true, "result": IDLE_CLOSE_ERROR, "closed": "idle"}), + ]; + for reply in delivered { + assert!(!is_idle_close(&reply), "{reply}"); + } } #[test] diff --git a/crates/mcp/src/base64.rs b/crates/mcp/src/base64.rs index 9c02ac327..12ccf45bb 100644 --- a/crates/mcp/src/base64.rs +++ b/crates/mcp/src/base64.rs @@ -20,9 +20,14 @@ pub fn base64_encode(bytes: &[u8]) -> String { pub fn base64_decode(text: &str) -> Option> { let mut out = Vec::with_capacity(text.len() / 4 * 3); let (mut acc, mut bits) = (0u32, 0u32); + let mut padded = false; for c in text.bytes().filter(|c| !c.is_ascii_whitespace()) { if c == b'=' { - break; + padded = true; + continue; + } + if padded { + return None; } let v = ALPHABET.iter().position(|&a| a == c)? as u32; acc = (acc << 6) | v; @@ -33,6 +38,9 @@ pub fn base64_decode(text: &str) -> Option> { acc &= (1 << bits) - 1; } } + if bits == 6 { + return None; + } Some(out) } @@ -57,4 +65,12 @@ mod tests { } assert!(base64_decode("not base64!").is_none()); } + + #[test] + fn rejects_malformed() { + assert!(base64_decode("Z").is_none()); + assert!(base64_decode("Zm9vY").is_none()); + assert!(base64_decode("Zg==!!!not-base64").is_none()); + assert_eq!(base64_decode("Zg==\n").unwrap(), b"f"); + } } diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 59e7ba3eb..7cc61a318 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -539,3 +539,66 @@ fn the_clipboard_tool_cuts_copies_and_pastes_in_the_app() { assert!(!r.is_error, "{r:?}"); assert_eq!(b.0, vec![("ui.clipboard".to_string(), json!({"action": "paste", "text": "travel"}))]); } + +/// A running app that writes the PNG `ui.render` / `ui.screenshot` ask for. It notes each path and +/// what its folder looked like then; with `dies_mid_write` it leaves a half-written temp file and +/// fails instead, like an app that quits part-way through. +struct PngWriter { + seen: Vec<(std::path::PathBuf, Option)>, + dies_mid_write: bool, +} + +impl Backend for PngWriter { + fn call(&mut self, method: &str, params: Value) -> Result { + assert!(matches!(method, "ui.render" | "ui.screenshot"), "{method}"); + let path = std::path::PathBuf::from(params["path"].as_str().unwrap()); + self.seen.push((path.clone(), path.parent().and_then(|d| std::fs::symlink_metadata(d).ok()))); + if self.dies_mid_write { + std::fs::write(path.with_file_name(".lightcraft-mcp-test.tmp"), b"half a png").unwrap(); + return Err("connection closed".into()); + } + crate::write_image(&path, &lightcraft_raster::Rgba8::new(1, 1), 92)?; + Ok(json!({"width": 1, "height": 1})) + } + fn has_ui(&self) -> bool { + true + } + fn describe(&self) -> String { + "png writer".into() + } +} + +/// Renders and screenshots reach the agent through a PNG the app writes. In a shared /tmp other +/// users could read it or take its predictable name first and make the render fail, so it goes in +/// a new folder of its own, private on Unix, that is gone afterwards even when the write fails +/// (#2573). +#[test] +fn render_and_screenshot_pngs_go_through_a_private_folder() { + let mut b = PngWriter { seen: vec![], dies_mid_write: false }; + for (tool, args) in [("render_photo", json!({"id": 1})), ("render_preview", json!({"id": 1})), ("screenshot", json!({}))] { + let r = call_tool(&mut b, tool, &args); + assert!(!r.is_error, "{tool}: {r:?}"); + assert_eq!(r.image().map(|(_, mime)| mime), Some("image/png"), "{tool}: {r:?}"); + } + b.dies_mid_write = true; + assert!(call_tool(&mut b, "screenshot", &json!({})).is_error); + assert_eq!(b.seen.len(), 4); + let temp = std::env::temp_dir(); + let mut folders = vec![]; + for (file, folder) in &b.seen { + let dir = file.parent().unwrap(); + let meta = folder.as_ref().unwrap(); + assert!(meta.is_dir() && !meta.file_type().is_symlink(), "{}", dir.display()); + assert_ne!(dir, temp, "{} is right in the shared temp dir", file.display()); + assert_eq!(dir.parent(), Some(temp.as_path()), "{}", file.display()); + assert!(dir.file_name().unwrap().to_string_lossy().starts_with("lightcraft-mcp-"), "{}", dir.display()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(meta.permissions().mode() & 0o777, 0o700, "{}", dir.display()); + } + assert!(!dir.exists(), "{} was left behind", dir.display()); + assert!(!folders.contains(&dir), "{} was used twice", dir.display()); + folders.push(dir); + } +} diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index b542b2b2f..1c7ae3701 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -2,6 +2,7 @@ //! per command in the engine's command registry (`cmd_`), all forwarded to a //! [`Backend`] as control-channel methods. +use std::hash::{BuildHasher, Hasher, RandomState}; use std::sync::atomic::{AtomicU64, Ordering}; use serde_json::{Map, Value, json}; @@ -437,9 +438,48 @@ fn activate(b: &mut dyn Backend, args: &Value) -> Result<(), String> { Ok(()) } -fn temp_path(tag: &str) -> std::path::PathBuf { - static N: AtomicU64 = AtomicU64::new(0); - std::env::temp_dir().join(format!("lightcraft-mcp-{}-{}-{tag}.png", std::process::id(), N.fetch_add(1, Ordering::Relaxed))) +/// Where the backend writes a render or screenshot for us to read back: `.png` in a new +/// `lightcraft-mcp--` folder in the temp dir, private to this user on Unix (0700). +/// Straight in a shared /tmp, other users could read the PNG, and a name they took first made the +/// render fail (#2573). Dropping this removes the folder and whatever is in it. +struct TempPng { + dir: std::path::PathBuf, + file: std::path::PathBuf, +} + +impl TempPng { + fn new(tag: &str) -> Result { + static N: AtomicU64 = AtomicU64::new(0); + let temp = std::env::temp_dir(); + for _ in 0..100 { + let mut h = RandomState::new().build_hasher(); + h.write_u64(N.fetch_add(1, Ordering::Relaxed)); + let dir = temp.join(format!("lightcraft-mcp-{}-{:016x}", std::process::id(), h.finish())); + #[cfg(unix)] + let builder = { + use std::os::unix::fs::DirBuilderExt; + let mut b = std::fs::DirBuilder::new(); + b.mode(0o700); + b + }; + #[cfg(not(unix))] + let builder = std::fs::DirBuilder::new(); + // Never recursive: a name someone else made first fails and is not used. + match builder.create(&dir) { + Ok(()) => return Ok(Self { file: dir.join(format!("{tag}.png")), dir }), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(e) => return Err(format!("cannot make a private temporary folder in {}: {e}", temp.display())), + } + } + Err(format!("cannot make a private temporary folder in {}: every name tried is taken", temp.display())) + } +} + +impl Drop for TempPng { + fn drop(&mut self) { + // Also removes the temp file a backend that died mid-write leaves next to the PNG. + let _ = std::fs::remove_dir_all(&self.dir); + } } /// Read a PNG the backend wrote, optionally downscale / re-encode as JPEG, and wrap it as image @@ -512,26 +552,32 @@ fn render_photo(b: &mut dyn Backend, args: &Value) -> ToolResult { }, }; let Some(id) = id else { return ToolResult::error("no active photo: import or select one, or pass `id`") }; - let file = temp_path("render"); - let path = file.to_string_lossy().to_string(); + let tmp = match TempPng::new("render") { + Ok(t) => t, + Err(e) => return ToolResult::error(e), + }; + let path = tmp.file.to_string_lossy().to_string(); let meta = match b.call("ui.render", json!({"id": id, "size": size, "path": path})) { Ok(m) => m, Err(e) => return ToolResult::error(e), }; let mut meta = meta; meta["id"] = json!(id); - image_result(&file, None, args.get("format").and_then(Value::as_str).unwrap_or("png"), args.get("path").and_then(Value::as_str), meta) + image_result(&tmp.file, None, args.get("format").and_then(Value::as_str).unwrap_or("png"), args.get("path").and_then(Value::as_str), meta) } fn screenshot(b: &mut dyn Backend, args: &Value) -> ToolResult { if let Err(e) = check_save_path(b, args) { return ToolResult::error(e); } - let file = temp_path("screenshot"); - let path = file.to_string_lossy().to_string(); + let tmp = match TempPng::new("screenshot") { + Ok(t) => t, + Err(e) => return ToolResult::error(e), + }; + let path = tmp.file.to_string_lossy().to_string(); match b.call("ui.screenshot", json!({"path": path})) { Ok(meta) => image_result( - &file, + &tmp.file, Some(args.get("maxSize").and_then(Value::as_u64).unwrap_or(1600).clamp(64, 8192) as u32), args.get("format").and_then(Value::as_str).unwrap_or("png"), args.get("path").and_then(Value::as_str), diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index bdcf202f2..7100a9603 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -73,6 +73,7 @@ impl LightcraftApp { drop_pending: vec![], copied: None, pending_picks: vec![], + missing: Default::default(), synthetic_mods: egui::Modifiers::NONE, synthetic_mods_release: false, host_mods: egui::Modifiers::NONE, diff --git a/crates/ui-egui/src/app/control.rs b/crates/ui-egui/src/app/control.rs index 525c5419c..763524101 100644 --- a/crates/ui-egui/src/app/control.rs +++ b/crates/ui-egui/src/app/control.rs @@ -18,6 +18,7 @@ impl LightcraftApp { control::Outcome::Done(v) => { let _ = reply.send(v); } + control::Outcome::Later => {} control::Outcome::Screenshot { target, headless } => { if let Err(error) = self.admit_shot(ctx, target, headless, reply.clone()) { let _ = reply.send(serde_json::json!({"ok": false, "error": error})); @@ -53,6 +54,8 @@ impl LightcraftApp { if self.pending_gestures.len() >= MAX_PENDING_GESTURES { return Err(format!("{}: {MAX_PENDING_GESTURES} control gestures are already pending", method)); } + // refused at once, without waiting for a layout or taking a pending slot + crate::control::check_gesture_numbers(&method, ¶ms)?; let now = self.tick_now_ms(); self.pending_gestures.push(PendingGesture { method, diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 0adf0ef5b..9011a4702 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -59,6 +59,8 @@ impl ControlRequest { pub enum Outcome { Done(Value), + /// Answered later, by whoever holds the request's `reply` (a task that is still running). + Later, /// A capture was admitted: `target` pins the destination identity until the write. Screenshot { target: Option, @@ -72,11 +74,19 @@ pub enum Outcome { }, } +/// A reply that says it worked: `result`. +pub(crate) fn success(v: Value) -> Value { + json!({"ok": true, "result": v}) +} +/// A reply that says it failed: `error`. +pub(crate) fn failure(e: impl std::fmt::Display) -> Value { + json!({"ok": false, "error": e.to_string()}) +} fn ok(v: Value) -> Outcome { - Outcome::Done(json!({"ok": true, "result": v})) + Outcome::Done(success(v)) } fn err(e: impl std::fmt::Display) -> Outcome { - Outcome::Done(json!({"ok": false, "error": e.to_string()})) + Outcome::Done(failure(e)) } fn wrap(r: Result) -> Outcome { match r { @@ -329,6 +339,12 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest "engine.execute" | "ui.menu.invoke" | "command" => { let Some(id) = s("command").or(s("id")) else { return err("missing `command`") }; let params = p.get("params").cloned().filter(|v| !v.is_null()).unwrap_or(json!({})); + if id == "library.missing" { + // looks at every photo's file: on a worker thread, answered when it is done + let started = crate::missing_sweep::ask(app, req.reply.clone()); + ctx.request_repaint(); + return started; + } let r = app.run(id, params); ctx.request_repaint(); wrap(r) @@ -603,6 +619,30 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result Result<(), String> { + fn check(method: &str, key: &str, v: &Value) -> Result<(), String> { + match v { + Value::Number(n) if !matches!(key, "count" | "steps" | "factor") => match n.as_f64() { + Some(x) if x.abs() <= MAX_GESTURE_NUMBER => Ok(()), + _ => Err(format!("{method}: `{key}` must be a number between -1000000 and 1000000")), + }, + Value::Array(a) => a.iter().try_for_each(|v| check(method, key, v)), + Value::Object(o) => o.iter().try_for_each(|(k, v)| check(method, k, v)), + _ => Ok(()), + } + } + check(method, "", p) +} + /// Resolve one queued synthetic-input control request against the **current** layout. /// /// Called by the app only after a completed UI pass newer than admission (plan §4.5). Widget rects @@ -610,6 +650,7 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result Result { + check_gesture_numbers(method, p)?; let s = |k: &str| p.get(k).and_then(Value::as_str); let f = |k: &str| p.get(k).and_then(Value::as_f64); match method { diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index ab49563cb..d9a90d546 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -466,13 +466,21 @@ impl Headless { } } + /// Queue a control-protocol request without running a frame: the answer comes on the returned + /// channel once a frame ([`Self::step`]) has taken the request up, and the work it starts is done. + pub(crate) fn send(&mut self, method: &str, params: Value) -> Result, Value> { + let (req, rx) = ControlRequest::new(method, params); + self.control.send(req).map_err(|_| json!({"ok": false, "error": "control channel closed"}))?; + Ok(rx) + } + /// Send a control-protocol request (see [`crate::control`]) and run frames until it is /// answered, then until its input has been consumed. Returns `{"ok": …, "result"|"error": …}`. pub fn request(&mut self, method: &str, params: Value, timeout: Duration) -> Value { - let (req, rx) = ControlRequest::new(method, params); - if self.control.send(req).is_err() { - return json!({"ok": false, "error": "control channel closed"}); - } + let rx = match self.send(method, params) { + Ok(rx) => rx, + Err(e) => return e, + }; let t0 = Instant::now(); let reply = loop { self.step(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1ec2a408c..186de15f4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -32,6 +32,7 @@ pub mod icons; pub mod lightroom_import; pub mod links; pub mod menus; +mod missing_sweep; mod model_setup; pub mod multi_select; pub mod panels; @@ -243,6 +244,9 @@ pub struct LightcraftApp { pub copied: Option, /// Native file dialogs up for commands (`pick`): each command runs again when its closes. pub(crate) pending_picks: Vec, + /// `library.missing` requests from the control channel and the check that answers them on a + /// worker thread ([`missing_sweep`]). + pub(crate) missing: missing_sweep::State, /// Modifiers announced for synthetic input (held from a button down to its release). This is /// only the app's synthetic ownership; the real host state is tracked separately in /// `host_mods`, so a canceled synthetic gesture never clears a physically held key. diff --git a/crates/ui-egui/src/missing_sweep.rs b/crates/ui-egui/src/missing_sweep.rs new file mode 100644 index 000000000..1afd0a280 --- /dev/null +++ b/crates/ui-egui/src/missing_sweep.rs @@ -0,0 +1,120 @@ +//! `library.missing` in the app. The engine's command looks at every photo's file on the calling +//! thread, which here is the UI's: on a share with hundreds of thousands of photos that froze the +//! window for minutes. A request from the control channel (and so an agent, `mcp --connect`) goes +//! the way of the person's own long jobs instead: the files are checked on a worker thread, under +//! a row in the activity stack that counts them and can stop the check, and the request is +//! answered with the same `[{id, path}]` once the check ends. A second request meanwhile joins the +//! check that is running and gets its answer, rather than starting another walk of the share. +//! +//! A check on a big share can outlast what a caller waits for (the control server gives a request +//! 60 s). An answer that nobody was waiting for any more is kept for a few minutes and given to +//! the next request, so that asking again after a timeout is not another walk of the share that +//! times out again. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::Sender; +use std::time::Duration; + +use web_time::Instant; + +use lightcraft_catalog::PhotoId; +use lightcraft_engine::cmd::missing; +use serde_json::Value; + +use crate::LightcraftApp; +use crate::control::{self, Outcome}; + +/// The activity stack's label (the command's name, already translated). +pub const LABEL: &str = "Missing Photos"; + +/// The activity row's kind. +const KIND: &str = "missing"; + +/// How long an answer nobody was waiting for is kept. +const KEEP: Duration = Duration::from_secs(10 * 60); + +/// The requests waiting for a check, and the answer kept for the next one. +#[derive(Default)] +pub struct State { + /// Each request with the library ([`lightcraft_engine::Session::library_generation`]) it asked about. + pub(crate) waiting: Vec<(u64, Sender)>, + /// A finished check's answer that no request received. + kept: Option, +} + +/// An answer nobody received: its library, the photos it was of ([`missing::fingerprint`]), when it was kept. +struct Kept { + library: u64, + snapshot: u64, + at: Instant, + answer: Value, +} + +impl State { + /// Send `answer` to every request waiting on `library`; `true` if any caller was still there. + /// (a request whose caller gave up has no one to send to) + fn answer(&mut self, library: u64, answer: &Value) -> bool { + let (theirs, others): (Vec<_>, Vec<_>) = std::mem::take(&mut self.waiting).into_iter().partition(|(l, _)| *l == library); + self.waiting = others; + theirs.into_iter().fold(false, |received, (_, waiting)| waiting.send(answer.clone()).is_ok() || received) + } + + /// A check for `library` could not start: fail its requests, and only those (another library's + /// check may be running for the others). + pub(crate) fn fail(&mut self, library: u64, error: &str) { + self.answer(library, &control::failure(error)); + } +} + +/// Answer `library.missing` from the control channel: give the answer kept from a check nobody +/// waited for, or start the check, or join the one running for this library. The answer goes to +/// `reply` when the check ends. +pub fn ask(app: &mut LightcraftApp, reply: Sender) -> Outcome { + let library = app.session.library_generation(); + if let Some(kept) = app.missing.kept.take() + && kept.library == library + && kept.at.elapsed() < KEEP + && kept.snapshot == missing::fingerprint(&app.session.catalog) + { + return Outcome::Done(kept.answer); + } + app.missing.waiting.push((library, reply)); + if app.tasks.is_running(LABEL) && app.missing.waiting.iter().filter(|(l, _)| *l == library).count() > 1 { + return Outcome::Later; + } + let files = missing::checked(&app.session.catalog); + let snapshot = missing::fingerprint_of_checked(&files); + let total = files.len() as u64; + let exists = app.session.media.availability.probe(); + let cancel = Arc::new(AtomicBool::new(false)); + let flag = cancel.clone(); + let work = move |task: lightcraft_engine::activity::TaskHandle| { + task.progress(0, total); + // (a panic is this check's failure, answered to the callers: they would wait for it in vain) + lightcraft_engine::guard::catch("Missing Photos", || { + missing::sweep(files, |f| exists(f), || flag.load(Ordering::Relaxed), |done| task.progress(done as u64, total)) + }) + }; + let done = move |app: &mut LightcraftApp, _ctx: &egui::Context, checked: Result>, String>| { + let (answer, complete) = match checked { + _ if app.session.library_generation() != library => { + (control::failure("Missing Photos: another library was opened before the check ended"), false) + } + Err(e) => (control::failure(e), false), + Ok(None) => (control::failure("Missing Photos: the check was stopped"), false), + Ok(Some(gone)) if missing::fingerprint(&app.session.catalog) == snapshot => (control::success(missing::missing_json(gone)), true), + // (photos changed while the files were checked, as an import does: the answer is what of the + // check still holds, and is not kept, for the next request must see the photos as they are then) + Ok(Some(gone)) => (control::success(missing::missing_json(missing::still_missing(&app.session.catalog, gone))), false), + }; + let received = app.missing.answer(library, &answer); + if complete && !received { + app.missing.kept = Some(Kept { library, snapshot, at: Instant::now(), answer }); + } + }; + if let Err(e) = crate::tasks::spawn_tracked(app, LABEL, KIND, cancel, work, done) { + app.missing.fail(library, &e); + } + Outcome::Later +} diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index e1ffd1dd2..9db6e38cf 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -55,6 +55,7 @@ mod masking; mod masking_layout; mod menubar; mod merge_preview; +mod missing_sweep; mod model_setup; mod move_folder; mod offline; diff --git a/crates/ui-egui/src/tests/lifecycle_037.rs b/crates/ui-egui/src/tests/lifecycle_037.rs index bb6f9d860..d994b9300 100644 --- a/crates/ui-egui/src/tests/lifecycle_037.rs +++ b/crates/ui-egui/src/tests/lifecycle_037.rs @@ -564,7 +564,7 @@ fn one_synthetic_batch_until_a_completed_pass_and_drops_wait_for_the_ui_phase() #[test] fn first_headless_frame_runs_logic_and_uses_its_simulated_clock() { - let mut h = Headless::new(app(), [800.0, 600.0], 1.0); + let mut h = Headless::new(self::app(), [800.0, 600.0], 1.0); assert_eq!(h.time(), 0.0); h.step(); assert!(h.app.styled, "the first frame at t=0 runs logic and installs fonts"); @@ -903,6 +903,48 @@ fn ui_scroll_carries_validated_unit_phase_and_source() { assert!(app.synthetic.is_empty()); } +/// A coordinate or delta too large for an `f32` became an infinite or NaN pointer position or +/// scroll delta: egui panicked on the NaN in debug builds ("Comparing NaN vectors"), and a release +/// build handed it to every widget. Such a gesture is refused and queues nothing. +#[test] +fn gestures_refuse_numbers_that_are_not_screen_positions() { + let (mut app, ctx) = (app(), egui::Context::default()); + app.widgets.push(("finite:test".into(), egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(20.0, 20.0)))); + app.image_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(100.0, 100.0))); + let refused = [ + ("ui.click", serde_json::json!({"x": 1e308})), + ("ui.click", serde_json::json!({"y": -1e308})), + ("ui.move", serde_json::json!({"x": 1e300, "y": 1.0})), + ("ui.drag", serde_json::json!({"x": 1.0, "toY": 1e308})), + ("ui.dragWidget", serde_json::json!({"id": "finite:test", "dx": -1e308})), + ("ui.scroll", serde_json::json!({"dy": 1e308})), + ("ui.pointer", serde_json::json!({"events": [{"kind": "down", "x": 0.5}, {"kind": "move", "x": 1e308}]})), + ]; + for (method, p) in refused { + let e = crate::control::gesture_events(&mut app, &ctx, method, &p).err().unwrap_or_else(|| panic!("{method} {p} was accepted")); + assert!(e.starts_with(method) && e.contains("between"), "{e}"); + assert!(app.synthetic.is_empty(), "{method} {p} queued events"); + } + // ordinary values still work, and count / steps are still clamped rather than refused + crate::control::gesture_events(&mut app, &ctx, "ui.click", &serde_json::json!({"x": 10.0, "y": -5.0, "count": u64::MAX})).unwrap(); + crate::control::gesture_events(&mut app, &ctx, "ui.drag", &serde_json::json!({"x": 1.0, "toX": 999_999.0, "steps": 1e12})).unwrap(); + assert!(!app.synthetic.is_empty()); + + // refused when the request is admitted: no pending slot, no wait for a layout + let (tx, rx) = reply_channel(); + let e = app.admit_gesture(&ctx, "ui.click".into(), serde_json::json!({"x": 1e308}), tx).expect_err("refused at admission"); + assert!(e.contains("between"), "{e}"); + assert!(app.pending_gestures.is_empty()); + drop(rx); + + // through the whole control channel, as an agent sends it: an error, not a panic + let mut h = Headless::new(self::app(), [800.0, 600.0], 1.0); + h.step(); + let r = h.request("ui.click", serde_json::json!({"x": 1e308}), Duration::from_secs(5)); + assert_eq!(r["ok"], false, "{r}"); + h.step(); +} + #[test] fn synthetic_pointer_requests_bound_clicks_and_drag_steps() { let (mut app, ctx) = (app(), egui::Context::default()); diff --git a/crates/ui-egui/src/tests/missing_sweep.rs b/crates/ui-egui/src/tests/missing_sweep.rs new file mode 100644 index 000000000..62b15a9d1 --- /dev/null +++ b/crates/ui-egui/src/tests/missing_sweep.rs @@ -0,0 +1,285 @@ +//! `library.missing` from the control channel (an agent, `mcp --connect`) looks at every photo's +//! file, which on a share with hundreds of thousands of photos took minutes on the UI thread and +//! froze the window: the files are checked on a worker thread under a row of the activity stack. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::mpsc::Receiver; +use std::time::{Duration, Instant}; + +use lightcraft_catalog::{Op, Photo, PhotoId, Source}; +use serde_json::{Value, json}; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(60); +const PHOTOS: u64 = 40; + +/// Photos `IMG_0000` … on a share; the first ten are gone. The existence check waits for `open` +/// (a share that answers slowly), at most `guard`, and counts its calls, and those made on the UI thread. +struct Share { + open: Arc, + on_ui: Arc, + calls: Arc, +} + +fn app_on_a_slow_share(guard: Duration) -> (Headless, Share) { + let mut app = LightcraftApp::new(lightcraft_engine::Session::new(), Services { png: None, ..Default::default() }); + for i in 0..PHOTOS { + let path = format!("/share/IMG_{i:04}.jpg"); + let p = Photo::new(PhotoId(i + 1), Source::File { path }, &format!("IMG_{i:04}.jpg"), "JPEG", 60, 40, "2026-01-01T00:00:00"); + app.session.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + } + let share = Share { open: Arc::new(AtomicBool::new(false)), on_ui: Arc::new(AtomicUsize::new(0)), calls: Arc::new(AtomicUsize::new(0)) }; + let (ui, open, on_ui, calls) = (std::thread::current().id(), share.open.clone(), share.on_ui.clone(), share.calls.clone()); + let deadline = Instant::now() + guard; + app.session.media.availability.set_probe(Arc::new(move |f| { + calls.fetch_add(1, Ordering::SeqCst); + if std::thread::current().id() == ui { + on_ui.fetch_add(1, Ordering::SeqCst); + } + // (a hang guard: the check goes on after it, so a regression fails the test instead of hanging it) + while !open.load(Ordering::SeqCst) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(1)); + } + !f.contains("_000") + })); + (Headless::new(app, [1200.0, 800.0], 1.0), share) +} + +fn ask(h: &mut Headless) -> Receiver { + h.send("engine.execute", json!({"command": "library.missing"})).unwrap() +} + +/// Frames until `rx` has an answer. +fn answer(h: &mut Headless, rx: &Receiver) -> Value { + let t0 = Instant::now(); + loop { + h.step(); + if let Ok(v) = rx.try_recv() { + return v; + } + assert!(t0.elapsed() < T, "never answered"); + std::thread::sleep(Duration::from_millis(1)); + } +} + +fn missing_rows(h: &Headless) -> Vec { + h.app.session.activity.list().into_iter().filter(|r| r.kind == "missing").collect() +} + +#[test] +fn library_missing_is_checked_off_the_ui_thread_and_answered_when_done() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let t0 = Instant::now(); + let rx = ask(&mut h); + for _ in 0..3 { + h.step(); + } + // the share is still answering nothing: the window took its frames all the same + assert!(t0.elapsed() < Duration::from_secs(2), "the frames waited for the files: {:?}", t0.elapsed()); + assert!(rx.try_recv().is_err(), "no answer before the files are checked"); + let rows = missing_rows(&h); + assert_eq!(rows.len(), 1, "{rows:?}"); + assert!(rows[0].cancellable, "the row can stop the check"); + assert_eq!(rows[0].label, crate::missing_sweep::LABEL); + + share.open.store(true, Ordering::SeqCst); + let reply = answer(&mut h, &rx); + assert_eq!(share.on_ui.load(Ordering::SeqCst), 0, "a file was looked at on the UI thread"); + assert_eq!(reply["ok"], true, "{reply}"); + let ids: Vec = reply["result"].as_array().unwrap().iter().map(|r| r["id"].as_u64().unwrap()).collect(); + assert_eq!(ids, (1..=10).collect::>(), "the same [{{id, path}}] the command gives"); + assert_eq!(reply["result"][0]["path"], "/share/IMG_0000.jpg"); + assert!(h.step_until(T, |h| h.app.tasks.is_empty())); + assert!(missing_rows(&h).is_empty(), "the row goes with the check"); +} + +/// A request that comes while a check runs gets that check's answer: the share isn't walked twice. +#[test] +fn a_second_request_joins_the_check_that_is_running() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let (first, second) = (ask(&mut h), ask(&mut h)); + for _ in 0..3 { + h.step(); + } + assert_eq!(missing_rows(&h).len(), 1, "one check for both"); + share.open.store(true, Ordering::SeqCst); + let (a, b) = (answer(&mut h, &first), answer(&mut h, &second)); + assert_eq!(a["ok"], true, "{a}"); + assert_eq!(a, b); + assert_eq!(share.on_ui.load(Ordering::SeqCst), 0); +} + +/// ✕ on the row (or `activity.cancel`) stops the check; the request is told so. +#[test] +fn stopping_the_check_answers_the_request_with_an_error() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let rx = ask(&mut h); + for _ in 0..3 { + h.step(); + } + let id = missing_rows(&h)[0].id; + let r = h.request("engine.execute", json!({"command": "activity.cancel", "params": {"id": id}}), T); + assert_eq!(r["ok"], true, "{r}"); + share.open.store(true, Ordering::SeqCst); + let reply = answer(&mut h, &rx); + assert_eq!(reply["ok"], false, "{reply}"); + assert!(reply["error"].as_str().is_some_and(|e| e.contains("stopped")), "{reply}"); + assert!(h.step_until(T, |h| h.app.tasks.is_empty())); + assert!(missing_rows(&h).is_empty()); +} + +/// The control server gives a request 60 s, a check on a big share takes longer: the caller asks +/// again after its timeout and is given the answer the check ended with, not another walk. +#[test] +fn an_answer_nobody_waited_for_is_given_to_the_next_request() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let gave_up = ask(&mut h); + for _ in 0..3 { + h.step(); + } + drop(gave_up); + share.open.store(true, Ordering::SeqCst); + assert!(h.step_until(T, |h| h.app.tasks.is_empty()), "the check ends without a caller"); + assert_eq!(share.calls.load(Ordering::SeqCst), PHOTOS as usize); + + let again = ask(&mut h); + let reply = answer(&mut h, &again); + assert_eq!(reply["ok"], true, "{reply}"); + assert_eq!(reply["result"].as_array().map(Vec::len), Some(10), "{reply}"); + assert_eq!(share.calls.load(Ordering::SeqCst), PHOTOS as usize, "no second walk of the share"); + // given once: the next request checks again + let third = ask(&mut h); + assert_eq!(answer(&mut h, &third)["ok"], true); + assert_eq!(share.calls.load(Ordering::SeqCst), 2 * PHOTOS as usize); +} + +/// A check that panics is a failure told to the caller, not a request left waiting for its timeout. +#[test] +fn a_check_that_panics_is_answered_with_an_error() { + let (mut h, _share) = app_on_a_slow_share(Duration::from_secs(3)); + h.app.session.media.availability.set_probe(Arc::new(|_| panic!("a file system that panics"))); + let rx = ask(&mut h); + let reply = answer(&mut h, &rx); + assert_eq!(reply["ok"], false, "{reply}"); + assert!(reply["error"].as_str().is_some_and(|e| e.contains("failed unexpectedly")), "{reply}"); + assert!(h.step_until(T, |h| h.app.tasks.is_empty())); + // and the next request is checked afresh + h.app.session.media.availability.set_probe(Arc::new(|_| true)); + let rx = ask(&mut h); + assert_eq!(answer(&mut h, &rx)["result"], json!([])); +} + +fn new_photo(id: u64, path: &str) -> Box { + Box::new(Photo::new(PhotoId(id), Source::File { path: path.into() }, "x.jpg", "JPEG", 60, 40, "2026-01-01T00:00:00")) +} + +/// Photos change while the files are being checked (auto-import adds some every few seconds, a share check +/// takes minutes): the answer is the check's, kept to the photos that are still where it looked, so asking +/// again never has to be the answer. A photo that was missing and has been removed, moved to Recently +/// Deleted or relinked since is not reported; one added since was not checked and is not either. +#[test] +fn a_check_over_changing_photos_answers_what_still_holds() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let rx = ask(&mut h); + for _ in 0..3 { + h.step(); + } + let cat = &mut h.app.session.catalog; + cat.apply(Op::RemovePhoto { id: PhotoId(1) }).unwrap(); + cat.apply(Op::SetDeleted { id: PhotoId(2), deleted: true }).unwrap(); + let relinked = + Op::Relink { id: PhotoId(3), file_name: "y.jpg".into(), source: Source::File { path: "/share/IMG_9998.jpg".into() }, format: None }; + cat.apply(relinked).unwrap(); + cat.apply(Op::AddPhoto { photo: new_photo(PHOTOS + 1, "/share/IMG_0000x.jpg") }).unwrap(); + share.open.store(true, Ordering::SeqCst); + let reply = answer(&mut h, &rx); + assert_eq!(reply["ok"], true, "{reply}"); + let ids: Vec = reply["result"].as_array().unwrap().iter().map(|r| r["id"].as_u64().unwrap()).collect(); + assert_eq!(ids, (4..=10).collect::>(), "{reply}"); +} + +/// A stop is a stop, whatever the photos did meanwhile. +#[test] +fn a_stopped_check_says_stopped_even_if_the_photos_changed() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let rx = ask(&mut h); + for _ in 0..3 { + h.step(); + } + h.app.session.catalog.apply(Op::AddPhoto { photo: new_photo(PHOTOS + 1, "/share/IMG_9999.jpg") }).unwrap(); + let id = missing_rows(&h)[0].id; + let r = h.request("engine.execute", json!({"command": "activity.cancel", "params": {"id": id}}), T); + assert_eq!(r["ok"], true, "{r}"); + share.open.store(true, Ordering::SeqCst); + let reply = answer(&mut h, &rx); + assert!(reply["error"].as_str().is_some_and(|e| e.contains("stopped")), "{reply}"); +} + +/// A check nobody waited for, over photos that changed while it ran, is not kept: the next request +/// checks afresh (and then sees the photo that came meanwhile). +#[test] +fn a_check_over_changed_photos_is_not_kept_for_a_later_request() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + drop(ask(&mut h)); + for _ in 0..3 { + h.step(); + } + h.app.session.catalog.apply(Op::AddPhoto { photo: new_photo(PHOTOS + 1, "/share/IMG_0000x.jpg") }).unwrap(); + share.open.store(true, Ordering::SeqCst); + assert!(h.step_until(T, |h| h.app.tasks.is_empty())); + let calls = share.calls.load(Ordering::SeqCst); + let again = ask(&mut h); + let reply = answer(&mut h, &again); + assert_eq!(reply["result"].as_array().map(Vec::len), Some(11), "{reply}"); + assert!(share.calls.load(Ordering::SeqCst) > calls, "checked afresh"); +} + +/// An edit that leaves the photos' files alone (a rating) does not spoil the check. +#[test] +fn an_edit_that_leaves_the_files_alone_does_not_spoil_the_check() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + let rx = ask(&mut h); + for _ in 0..3 { + h.step(); + } + h.app.session.catalog.apply(Op::SetRating { id: PhotoId(1), rating: 3 }).unwrap(); + share.open.store(true, Ordering::SeqCst); + assert_eq!(answer(&mut h, &rx)["ok"], true); +} + +/// A check that can't start for one library fails only the requests for that library. +#[test] +fn a_failed_start_fails_only_that_librarys_requests() { + let mut state = crate::missing_sweep::State::default(); + let (a_tx, a_rx) = std::sync::mpsc::channel(); + let (b_tx, b_rx) = std::sync::mpsc::channel(); + state.waiting.push((1, a_tx)); + state.waiting.push((2, b_tx)); + state.fail(2, "Missing Photos: could not start"); + assert!(b_rx.try_recv().is_ok_and(|v| v["ok"] == false)); + assert!(a_rx.try_recv().is_err(), "the other library's request keeps waiting"); + assert_eq!(state.waiting.len(), 1); +} + +/// An answer kept for a request that never came is not given once the photos have changed. +#[test] +fn a_kept_answer_is_not_given_after_the_photos_changed() { + let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3)); + drop(ask(&mut h)); + for _ in 0..3 { + h.step(); + } + share.open.store(true, Ordering::SeqCst); + assert!(h.step_until(T, |h| h.app.tasks.is_empty())); + let p = + Photo::new(PhotoId(PHOTOS + 1), Source::File { path: "/share/IMG_0000x.jpg".into() }, "IMG_0000x.jpg", "JPEG", 60, 40, "2026-01-01T00:00:00"); + h.app.session.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + let calls = share.calls.load(Ordering::SeqCst); + let again = ask(&mut h); + let reply = answer(&mut h, &again); + assert_eq!(reply["result"].as_array().map(Vec::len), Some(11), "{reply}"); + assert!(share.calls.load(Ordering::SeqCst) > calls); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index d6df09113..bf8ecb56e 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -21,15 +21,25 @@ headless snapshots or `lightcraft-cli mcp` without `--connect`. A request that h Connections close after five minutes without incoming bytes while the server is waiting for input, including in the middle of an unfinished request line. This allows pauses between -automation commands while releasing abandoned connection slots. An idle read timeout gets an -`ok: false` error saying the connection is closing; incomplete input is never dispatched. +automation commands while releasing abandoned connection slots. An idle read timeout gets one +reply without an `id`, then the connection closes; incomplete input is never dispatched: + +```text +← {"ok": false, "error": "control connection idle read timed out; closing the connection", "closed": "idle"} +``` + This is a socket read inactivity limit, not a total connection lifetime: a peer that keeps sending bytes can keep it open. Waiting for an already-dispatched command's reply does not count as idle reading and retains the request timeout above. -Reconnect before sending another request after an idle close. MCP connect mode detects closed -connections before sending and reconnects; if a connection is lost after sending a request, -its outcome remains unknown and the client must not blindly replay it. +Reconnect before sending another request after an idle close. A request sent just as the +connection expires crosses the notice: the client reads the notice where it expects the reply. +The server reads nothing after its idle timeout, so that request never ran, and `"closed": "idle"` +(only on this notice) says it is safe to send it again, once, on a new connection. Apps from +before the marker send the same notice without it; match its exact `error` text for them. MCP +connect mode does both: it detects closed connections before sending and reconnects, and +resends a request that crossed the idle notice once. Any other lost connection after sending a +request leaves its outcome unknown, and the client must not blindly replay it. **Only requests are read.** Every line must be a JSON object with a string `method` (`id` and `params` are optional; blank lines are skipped). Anything else — text that isn't JSON, a JSON array or number, an object without @@ -82,7 +92,7 @@ filmstrip is visible. | `ui.inspect` | — | UI state, window, `loupe` (`source`, `pending`, `failed` = why its render failed, else `null`, and `region: {full, window, pending}` when a zoomed view also renders the window on screen at no more than 100 % — `full` is the frame it is cut from, `window` its `[x, y, w, h]` — and `regionBefore` for the Before side of a Before/After view), canvas/image rects, `scroll: {grid, filmstrip}` (scroll offsets in points, `null` until drawn), active photo, selection, perf (`frameMs` = layout, `logicMs` = per-frame logic before it, `updateMs` = both, `maxUpdateMs`, `fps`, render queue …, `gpu` = adapter in use, `gpuReason` = why renders don't use the GPU, `gpuFallback` = latest render redone on the CPU and why — see `docs/gpu-pipeline.md`), status, memory (bytes per cache, see `library.memory`; plus stage caches — `budgetBytes`, `trimmed`, `sharedSourceBytes` (the one device copy of a big original) — and textures), `export: {running: {total, done, current} \| null, last}`, `notices` (warnings waiting to be shown, e.g. a damaged settings file; OK = `button:noticeOk`), `quitPrompt` (why quitting was stopped: tasks that would be cut short are running — `button:quitAnyway` cancels them and quits, `button:quitCancel` keeps working — or unsaved changes: `button:quitRetry` / `button:quitAnyway` / `button:quitCancel`), `import: {done, total, imported, cancelled} \| null` (an import runs on a worker thread), `tasks` (other background work: `Find Missing Photos`, `Auto Import`), `activity` (every long-running task in flight as the activity stack in the top-left corner shows it, same shape as `activity.list`; a row appears once its task is 0.5 s old, ids `activity:row:`, ✕ = `activity:cancel:`, `activity:more` expands past 3 rows), `fileDialogs` (commands waiting on a native file dialog, which runs off the UI thread; they run again with the answer when it closes), `copied` (the text the UI last put on the clipboard, `null` until something is copied) | | `ui.widgets` | `{filter?}` | On-screen widgets `{id, rect: [x, y, w, h]}` (screen points) | | `ui.clickWidget` / `ui.dragWidget` / `ui.hoverWidget` | `{id, count?, button?, fx?, fy?}` (`button: "right"` right-clicks; `count` is capped at 16) / `{id, toX?, toY?, dx?, dy?, steps?}` (steps are capped at 1000) / `{id, fx?, fy?}` | Real egui input on a widget (hover: the pointer rests on it, e.g. for preset/profile previews). `fx`/`fy` are fractions of the registered rect and may lie outside `[0,1]`; the resulting point must still be inside egui's clipped interaction rect, and a non-finite fraction or resolved point is rejected before any input is queued. Geometry comes from the latest completed layout; if the widget moves before the first event or before a press, or its latest clipped rect no longer contains the point, the request fails with an error instead of clicking another widget or retargeting. A target below an ordinary modal dialog may succeed as a no-op or a backdrop click, because egui routes that input to the dialog; a target covered by a non-modal overlay fails. The blocking library-problem/launch-list window is explicit: its dim takes the modal layer, but a background target fails instead of reporting a no-op success, and only that window's own controls (`button:library…`) pass. Its creation frame is shielded and redrawn with empty input like a prompt's, so background text/pointer input cannot act while the new modal areas settle. A quit prompt or settings notice is a real egui modal scope with a foreground backdrop. Only its own buttons (`button:quit…`, `button:noticeOk`) accept a `ui.clickWidget`; background targets fail, and background keystrokes, text/clipboard input, wheel/zoom input and AccessKit actions are dropped before panels see them. The prompt itself takes Tab/Shift+Tab, Enter/Space on the focused button, and Escape/Enter for the safe default Cancel; AccessKit Focus/Click works when targeted at a prompt button. Its first creation frame (including a Tasks→Unsaved or notice→next-notice transition) is shielded and redrawn with empty input, so a press/release in that frame, or a held press from before it, cannot act underneath it; a synthetic drag already in progress is canceled. Once settled, raw `ui.click`/`ui.drag`/`ui.pointer` and backdrop clicks are still delivered to the prompt backdrop as no-ops, and the prompt's own button clicks work normally. The area/layer check cannot see a different widget covering the point in the same layer. A `ui.clickWidget` whose widget disappears before its release also fails, after canceling the pointer press without a click or drag (the same rules apply when a click spans several host frames) | -| `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points (the same click and step caps apply) | +| `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points (the same click and step caps apply). In every pointer and scroll gesture a number other than `count` and `steps` must lie between −1,000,000 and 1,000,000, or the request is refused and queues nothing | | `ui.pointer` | `{events: [{kind: down\|drag\|up, x, y}], alt?, shift?, cmd?}` | Gesture in normalized image coordinates (Detail view) | | `ui.key` | `{key, action?: press\|down\|up, cmd?, shift?, alt?, ctrl?}` | Key press (default), or the key going `down` and staying held until an `up` (Space held over a `ui.pointer` drag); while a prompt is up only its Tab/Shift+Tab, Enter, Space and Escape are delivered, every other key is dropped | | `ui.text` | `{text}` | Text input | diff --git a/docs/mcp.md b/docs/mcp.md index 2c155e72c..4df3e0eaf 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -22,7 +22,9 @@ it open, `--library` on the same folder fails with "This library is already open `--demo` starts the headless session with the procedurally generated demo library; `--compact` lists only the helper tools (see below). In connect mode the server starts even when the app is not running yet and connects on the first call (and reconnects if the app restarts). -A request is sent again only when it never reached the app. If the connection drops after a request +A request is sent again (once) only when it never reached the app: it could not be sent, or the app's +idle-close notice came back in its place (the app closed the idle connection without reading it; see +[control-protocol.md](control-protocol.md)). If the connection drops after a request was sent but before its reply, the tool fails with "the outcome is unknown" and nothing is resent, so a command such as undo can't run twice: check the app's state before retrying. Replies must be newline-terminated, at most 64 MiB before the newline (including encoded previews/screenshots), diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 8bebf2bfa..cf9bec6cc 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (LR-LIB-ALBUM: album commands leave out photos the library no longer has and record nothing when they add nothing; LR-BEHAV-DRAGDROP: a right-click on a text field while photos are carried from the grid opens no Cut / Copy / Paste menu; LR-BEHAV-DRAGDROP: a right-click while photos are carried from the grid opens no context menu; LR-LIB-VIRTUALCOPY: default numbering is preflighted across the requested batch and explicit names bypass suffix allocation; LR-BEHAV-DRAGDROP: only the primary button drags photos out of the grid and drops them; KEYC-RATING: deleting a keyword while Enable Filters has the filter put aside no longer leaves an empty one there; LR-BEHAV-DRAGDROP: a drop's destination is named with its parent when another folder or album has the same name; LR-VIEW-FILMSTRIP: photos drag from the filmstrip to the same sidebar targets as from the grid; LRC-LIB-LIBFOLDERS: a deep row's count follows its name in a narrow panel; LR-FILT-FLAG / LRC-LIB-FILTERBAR: the flag toggles combine, Picked + Unflagged, #2588; LR-VIEW-PHOTOGRID, LR-VIEW-FILMSTRIP, LR-VIEW-COMPARE, LR-VIEW-SURVEY, LRC-LIB-REFVIEW, LRC-LIB-PEOPLE: the active photo's selection ring shows on light themes and in Compare, Survey and the face tiles, and needs a loaded thumbnail; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LR-META-LOCATION: Info GPS text is fully validated before metadata changes; LR-MAP-TRACKLOG decodes GPX character references in point fields; malformed references error; CDATA stays literal; LR-BEHAV-ACCESS: the Thumbnail size control is a slider to screen readers; LR-BEHAV-ACCESS: the Detail photo is named with its file name; LR-VIEW-PHOTOGRID: the grid's empty-view message names its shortcut as this platform writes it and as it is bound; LR-BEHAV-ACCESS: the grid's date headers are named as they read; LR-BEHAV-ACCESS: the sidebar's disclosure triangles name their row; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌘⌥⇧K Set Keyword Shortcut and ⇧K; KEYC-METADATA: ⌘⇧K / Ctrl+Shift+K Edit Keywords; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LRC-LIB-FOLDERS: Local's built-in places (Pictures, Desktop, Downloads, Home) offer no Rename Folder… or Move Folder To…, and `folder.rename` / `folder.move` refuse the home folder and its Pictures, Desktop and Downloads; LRC-LIB-CATALOG-SWITCH: a library's catalog.snap or folder given at launch opens that library; LR-IMP-ADD-DIALOG / LR-IMP-MOVE: checked rename-sequence exhaustion; LR-IMP-ADD-DIALOG: an import asks the disk about the probes a scan left, and for each photo's sidecar, up to 16 files at a time; LRC-LIB-LIBFOLDERS: Don't ask again in the Move Photos question is kept only when the move started; a refused move leaves the question on; LR-IMP-ADD-DIALOG: remembered import thumbnail size, density-aware bounded previews and one job per candidate/size bucket (#2237, part of #2056); import loupe remains open; counts unchanged; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LR-MERGE-HDR / LR-MERGE-PANO / LR-MERGE-HDRPANO: the dialog makes a new preview each time it opens, also after a failed one, #2708; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (LR-LIB-ALBUM: album commands leave out photos the library no longer has and record nothing when they add nothing; LR-BEHAV-DRAGDROP: a right-click on a text field while photos are carried from the grid opens no Cut / Copy / Paste menu; LR-BEHAV-DRAGDROP: a right-click while photos are carried from the grid opens no context menu; LR-LIB-VIRTUALCOPY: default numbering is preflighted across the requested batch and explicit names bypass suffix allocation; LR-BEHAV-DRAGDROP: only the primary button drags photos out of the grid and drops them; KEYC-RATING: deleting a keyword while Enable Filters has the filter put aside no longer leaves an empty one there; LR-BEHAV-DRAGDROP: a drop's destination is named with its parent when another folder or album has the same name; LR-VIEW-FILMSTRIP: photos drag from the filmstrip to the same sidebar targets as from the grid; LRC-LIB-LIBFOLDERS: a deep row's count follows its name in a narrow panel; LR-FILT-FLAG / LRC-LIB-FILTERBAR: the flag toggles combine, Picked + Unflagged, #2588; LR-VIEW-PHOTOGRID, LR-VIEW-FILMSTRIP, LR-VIEW-COMPARE, LR-VIEW-SURVEY, LRC-LIB-REFVIEW, LRC-LIB-PEOPLE: the active photo's selection ring shows on light themes and in Compare, Survey and the face tiles, and needs a loaded thumbnail; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LR-META-LOCATION: Info GPS text is fully validated before metadata changes; LR-MAP-TRACKLOG decodes GPX character references in point fields; malformed references error; CDATA stays literal; LR-BEHAV-ACCESS: the Thumbnail size control is a slider to screen readers; LR-BEHAV-ACCESS: the Detail photo is named with its file name; LR-VIEW-PHOTOGRID: the grid's empty-view message names its shortcut as this platform writes it and as it is bound; LR-BEHAV-ACCESS: the grid's date headers are named as they read; LR-BEHAV-ACCESS: the sidebar's disclosure triangles name their row; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌘⌥⇧K Set Keyword Shortcut and ⇧K; KEYC-METADATA: ⌘⇧K / Ctrl+Shift+K Edit Keywords; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LRC-LIB-FOLDERS: Local's built-in places (Pictures, Desktop, Downloads, Home) offer no Rename Folder… or Move Folder To…, and `folder.rename` / `folder.move` refuse the home folder and its Pictures, Desktop and Downloads; LRC-LIB-CATALOG-SWITCH: a library's catalog.snap or folder given at launch opens that library; LR-IMP-ADD-DIALOG / LR-IMP-MOVE: checked rename-sequence exhaustion; LR-IMP-ADD-DIALOG: an import asks the disk about the probes a scan left, and for each photo's sidecar, up to 16 files at a time; LRC-LIB-LIBFOLDERS: Don't ask again in the Move Photos question is kept only when the move started; a refused move leaves the question on; LR-IMP-ADD-DIALOG: remembered import thumbnail size, density-aware bounded previews and one job per candidate/size bucket (#2237, part of #2056); import loupe remains open; counts unchanged; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LR-MERGE-HDR / LR-MERGE-PANO / LR-MERGE-HDRPANO: the dialog makes a new preview each time it opens, also after a failed one, #2708; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; LRC-LIB-MISSING: `library.missing` from the control channel checks the files on a worker thread instead of the UI thread; LR-FILT-SEARCH-META: `>=` / `<=` in fielded search tokens; MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say Close Window quits the app, after the same check as Quit; MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Edit lists Select Only Active Photo after Deselect All and Paste Settings from Previous after Paste Selected Settings…; on macOS no doubled separators are left where Settings… moved to the app menu; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -726,7 +726,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-PEOPLE | People view | P2 | 🟡 | `cmd:view.people`, `cmd:view.person`, `cmd:faces.person`, `cmd:faces.unnamed`, `cmd:faces.nameFaces`, `crates/ui-egui/src/panels/people.rs`, `crates/ui-egui/src/panels/person.rs`, `crates/ui-egui/src/panels/unnamed.rs`, `crates/engine/src/face_tiles.rs` | Named People cards (face close-up, the photo count on the picture, name) for the photos the active filters let through, with the filter chips shown above them, sized by the thumbnail slider. Below them the **Unnamed faces**: every face nobody has named as a cropped picture, look-alikes next to each other once recognition has looked at them, a suggested name on a face the named ones recognise; click or Shift-click to select, type a name, Enter names the whole group in one undo step. A click on a person opens their page: only cropped faces, then **More** (unnamed faces that look like them, best first; click to confirm one, × to hide for the session, or ↑ / hovered-or-focused A key to confirm all currently shown non-hidden suggestions at or above that face's score, including ties, in one undo step). Every face is shown by the detector's box where the scan has one, so faces are equally close. Until recognition is set up a line at the top offers the next step (Settings ▸ Faces, or Turn on), as does the loupe's name box. Face pictures of unedited raws are cut from the largest embedded preview (planned read: header and JPEG only, never the sensor data) when it holds at least ¾ of the tile's pixels across the face, else developed as before; all are kept in the thumbnail disk cache at 16 px size steps, so a later launch shows them without reading the originals (#1440, `docs/faces.md`). No clusters with separators between groups, no persistent reject, no merge of two people | LRC-LIB-COMMENTS | Comments panel | P2 | ⬜ | | | | LRC-LIB-VISUALSEARCH | Find similar photos | P2 | ✅ | `cmd:library.findSimilar`, `crates/pipeline/src/cull.rs` (`signature`) | photo menu / Photo ▸ Find Similar Photos: look-alike photos (composition and tones), most similar first, as a filter (`only`) | -| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`), `crates/ui-egui/src/panels/left.rs` (`locate_missing_item`) | File → Find Missing Photos… (anywhere in a folder: same name, size and content hash; renamed files by size + content hash; same name and size alone only when unambiguous — look-alikes are skipped and reported; searched on a worker thread, one undo step), photo menu → Locate Missing File…; a folder or disk row's Locate Missing Files… chooses its new location and scopes the same recursive background finder to that folder and its subfolders. Unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread; the view, the Info panel, the grid's context menu and thumbnails read file availability from a cache a worker thread fills, so an offline NAS never stalls a frame); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike | +| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`), `crates/ui-egui/src/missing_sweep.rs`, `crates/ui-egui/src/panels/left.rs` (`locate_missing_item`) | File → Find Missing Photos… (anywhere in a folder: same name, size and content hash; renamed files by size + content hash; same name and size alone only when unambiguous — look-alikes are skipped and reported; searched on a worker thread, one undo step), photo menu → Locate Missing File…; a folder or disk row's Locate Missing Files… chooses its new location and scopes the same recursive background finder to that folder and its subfolders. Unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread; the view, the Info panel, the grid's context menu and thumbnails read file availability from a cache a worker thread fills, so an offline NAS never stalls a frame); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike; `library.missing` from the control channel or `mcp --connect` checks the files on a worker thread (a "Missing Photos" row in the activity stack, ✕ stops it) and answers when it ends, so a large library on a network share never stalls the window; the same request meanwhile shares the running check | | LRC-LIB-CONVERT | Convert to DNG | P2 | ✅ | `cmd:photo.convertToDng`, `crates/engine/src/cmd/convert.rs` | Photo ▸ Convert to DNG: lossless DNG next to the raw with the settings embedded, photo (and its virtual copies) relinked; originals kept; undoable; the DNG is decoded and compared with the raw data, written to a temp file, synced, read back and only then given a free name (never replacing a file) — a failed write leaves no DNG and the photo on its raw (issue #106) | | LRC-LIB-PREVIEWS | Build / discard previews | P1 | ✅ | `cmd:library.buildPreviews`, `cmd:library.previewProgress`, `cmd:library.cancelPreviews`, `cmd:library.clearPreviews`, `crates/engine/src/cmd/previews.rs`, `crates/preview/src/lib.rs` | File ▸ Previews: build standard-sized (Settings → Performance size) or 1:1 previews of the selected / visible photos in the background (a row in the activity stack, ✕ to stop), discard the cache; disk thumbnail + view cache; no smart previews (offline proxies) | | LRC-LIB-SLIDESHOW-IMPROMPTU | Impromptu slideshow | P2 | ✅ | `cmd:view.slideshow` | see LR-VIEW-SLIDESHOW | @@ -987,6 +987,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-22 | minor | LR-CROP-AUTO: Auto straighten turns by at most 10° and only on at least 3 agreeing lines, and says when it finds nothing to level by; measured on 68 photos straightened by hand and 300 left as shot, unwanted turns of more than 1° fell from 79 to 19 of the 300. Downgraded ✅ → 🟡: it still leaves most tilted photos alone | | 2026-10-23 | patch | LRC-LIB-FILTERBAR: showing an album (`library.source`) while the filter holds an album drops the filter's album, as showing a library folder already dropped its folder filter; before, the old album stayed in the filter unseen (the filter bar has no album chip) and narrowed the next source such as All Photos; the row's note says so; status unchanged (✅) | | 2026-10-28 | patch | LR-VIEW-PHOTOGRID: the grid's empty-view message (shown when a view other than the library is empty) used a fixed "Cmd+Shift+I" (or a translation's fixed copy of it) whatever the platform or the binding, on one line that ran out of a narrow window; it now shows Import Photos' shortcut as the menu bar writes it and as it is bound on its own line, and none when it is unbound, and the sentence wraps; status unchanged (✅) | +| 2026-10-22 | patch | LRC-LIB-MISSING: `library.missing` from the control channel (an agent, `mcp --connect`) checks the photos' files on a worker thread under a "Missing Photos" row in the activity stack (counts the files, ✕ stops it) and is answered when the check ends, instead of freezing the window while the UI thread looked at every file (minutes on a large library on a network share); status unchanged (✅) | | 2026-10-27 | patch | MENU-EDIT-SELECTNONE, MENU-EDIT-PASTESELECTED: Select Only Active Photo and Paste Settings from Previous were not in the Edit menu's layout and ended up after Settings…, near the end of the menu; they now sit with the other Select and Paste rows. On macOS the Edit menu no longer shows two separators in a row where Settings… moved to the app menu; status unchanged (✅) | | 2026-10-27 | patch | MENU-FILE-CLOSE, KEY-CLOSEWINDOW: the notes say that Close Window and the red close button of the main window quit the app, after the check Quit makes; kept on purpose; a user page says so; status unchanged (✅) | | 2026-10-17 | minor | LR-VIEW-PHOTOGRID, LR-VIEW-SQUAREGRID, LR-VIEW-FILMSTRIP, LR-VIEW-SURVEY, LR-VIEW-COMPARE, LRC-LIB-REFVIEW, LRC-LIB-PEOPLE: selection rings are theme tokens (3:1 or more against the grid; the active photo's 3 pt two-tone ring against the others' 1.5 pt), so the active photo is no longer a white ring lost on the light themes' grid; Compare, Reference and Survey frame their active photo and the People view its chosen person the same way; statuses unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index a2e1647a8..1fe84bab1 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -25,6 +25,11 @@ - Export ▸ Image Sizing kept pixel sizes only in steps of 16, so a typed 1080 became 1088 and 3000 became 3008. The long edge, short edge, width and height fields now keep any whole number of pixels. +### The control channel refuses pointer positions that aren't on any screen +- `ui.click`, `ui.move`, `ui.drag`, `ui.dragWidget`, `ui.pointer` and `ui.scroll` with a coordinate or delta far past + any screen (such as `1e308`) used to queue an infinite or NaN position. A debug build panicked on it and a release + build handed it to every widget. Numbers outside ±1,000,000 are now an error, and nothing is queued. + ## 2026-10-29 ### A person's page opens in seconds in a large library @@ -250,6 +255,18 @@ that no longer existed. The library's lock is now released as soon as everything is saved, as it already was on the other systems. +### Asking for the missing photos no longer freezes the window +- `library.missing` (the control channel and `mcp --connect`) looked at every photo's file on the thread that draws + the window, so on a library of hundreds of thousands of photos on a network share the app stopped answering for + minutes. The files are now checked in the background: a "Missing Photos" row in the activity stack counts them and + its ✕ stops the check, and the request is answered with the same list when the check ends. A second request meanwhile + shares the running check. A caller that stops waiting (the control server gives a request 60 s) gets the answer of + that check when it asks again within ten minutes, instead of another walk of the share, as long as the photos have + not changed since; otherwise the files are checked again. If photos change while the files are being checked (an + import adds some, others are removed, moved to Recently Deleted or relinked), the answer leaves out the photos that + no longer apply, a photo added meanwhile is not part of that check, and the answer is not kept for a later request. + + ### Choose where a Lightroom catalog goes - File ▸ Import Lightroom Catalog… now asks first: merge the catalog's photos, collections and edits into the library open now, or create a new library for them, named after the catalog and placed next to the open library