mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-31 13:38:14 -06:00
Merge track r3-t06
# Conflicts: # docs/parity-checklist.md # docs/whats-new.md
This commit is contained in:
aiongg
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+102
-11
@@ -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<String> = std::env::args().skip(1).collect();
|
||||
let mut args: Vec<String> = 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<Value> = 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<Vec<Value>, 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<Value> = 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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<u16> = "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<Value>, 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<Value>, 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}");
|
||||
// `<host>:<port>` 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=<bare-port>` 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())
|
||||
|
||||
@@ -87,7 +87,7 @@ fn serve(stream: TcpStream, tx: &Sender<ControlRequest>, 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<ControlRequest>, 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<Value>) {
|
||||
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<u8>, max: usize) -> Result<bool, String> {
|
||||
fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec<u8>, max: usize) -> Result<bool, Closing> {
|
||||
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<u8>, 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<R: Read, W: Write>(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<R: Read, W: Write>(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:?}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -104,9 +104,14 @@ fn base64_encode(bytes: &[u8]) -> String {
|
||||
fn base64_decode(s: &str) -> Option<Vec<u8>> {
|
||||
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<Vec<u8>> {
|
||||
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]
|
||||
|
||||
@@ -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<Item = (PhotoId, &'a str)>) -> 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<Vec<(PhotoId, String)>> {
|
||||
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<Value> {
|
||||
|
||||
pub fn specs() -> Vec<CommandSpec> {
|
||||
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",
|
||||
|
||||
@@ -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]
|
||||
|
||||
+176
-55
@@ -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<Value, String> {
|
||||
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>, 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<TcpStream>) -> 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]
|
||||
|
||||
@@ -20,9 +20,14 @@ pub fn base64_encode(bytes: &[u8]) -> String {
|
||||
pub fn base64_decode(text: &str) -> Option<Vec<u8>> {
|
||||
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<Vec<u8>> {
|
||||
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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<std::fs::Metadata>)>,
|
||||
dies_mid_write: bool,
|
||||
}
|
||||
|
||||
impl Backend for PngWriter {
|
||||
fn call(&mut self, method: &str, params: Value) -> Result<Value, String> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
+55
-9
@@ -2,6 +2,7 @@
|
||||
//! per command in the engine's command registry (`cmd_<id with . → _>`), 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: `<tag>.png` in a new
|
||||
/// `lightcraft-mcp-<pid>-<random>` 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<Self, String> {
|
||||
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),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<lightcraft_engine::originals::WriteTarget>,
|
||||
@@ -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<Value, String>) -> 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<Value, String
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
/// The largest coordinate or offset a gesture takes, in points (far past any screen).
|
||||
const MAX_GESTURE_NUMBER: f64 = 1e6;
|
||||
|
||||
/// Every number of a gesture's parameters (and of `ui.pointer`'s events) is finite and within
|
||||
/// ±[`MAX_GESTURE_NUMBER`]. A larger one turns into an infinite or NaN `f32` position or scroll
|
||||
/// delta: egui asserts on NaN positions, and a release build would hand them to every widget.
|
||||
/// `count` and `steps` are clamped where they are read instead, and `ui.zoom`'s `factor` (a
|
||||
/// multiplier, not a position) is checked and clamped by its own branch. Checked when a request
|
||||
/// is admitted and again here, for callers that resolve a gesture directly.
|
||||
pub(crate) fn check_gesture_numbers(method: &str, p: &Value) -> 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<Value, String
|
||||
/// has pushed the events onto `app.synthetic`; the app answers the request once the UI consumed
|
||||
/// them.
|
||||
pub(crate) fn gesture_events(app: &mut LightcraftApp, ctx: &egui::Context, method: &str, p: &Value) -> Result<GestureOutcome, String> {
|
||||
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 {
|
||||
|
||||
@@ -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<std::sync::mpsc::Receiver<Value>, 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();
|
||||
|
||||
@@ -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<String>,
|
||||
/// Native file dialogs up for commands (`pick`): each command runs again when its closes.
|
||||
pub(crate) pending_picks: Vec<pick::Pending>,
|
||||
/// `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.
|
||||
|
||||
@@ -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<Value>)>,
|
||||
/// A finished check's answer that no request received.
|
||||
kept: Option<Kept>,
|
||||
}
|
||||
|
||||
/// 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<Value>) -> 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<Option<Vec<(PhotoId, String)>>, 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
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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<AtomicBool>,
|
||||
on_ui: Arc<AtomicUsize>,
|
||||
calls: Arc<AtomicUsize>,
|
||||
}
|
||||
|
||||
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<Value> {
|
||||
h.send("engine.execute", json!({"command": "library.missing"})).unwrap()
|
||||
}
|
||||
|
||||
/// Frames until `rx` has an answer.
|
||||
fn answer(h: &mut Headless, rx: &Receiver<Value>) -> 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<lightcraft_engine::activity::TaskInfo> {
|
||||
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<u64> = reply["result"].as_array().unwrap().iter().map(|r| r["id"].as_u64().unwrap()).collect();
|
||||
assert_eq!(ids, (1..=10).collect::<Vec<_>>(), "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<Photo> {
|
||||
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<u64> = reply["result"].as_array().unwrap().iter().map(|r| r["id"].as_u64().unwrap()).collect();
|
||||
assert_eq!(ids, (4..=10).collect::<Vec<_>>(), "{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);
|
||||
}
|
||||
@@ -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:<id>`, ✕ = `activity:cancel:<id>`, `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 |
|
||||
|
||||
+3
-1
@@ -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),
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user