diff --git a/apps/photocraft/src/control_server.rs b/apps/photocraft/src/control_server.rs index 33923d9f..932e31ee 100644 --- a/apps/photocraft/src/control_server.rs +++ b/apps/photocraft/src/control_server.rs @@ -5,13 +5,14 @@ use std::io::{BufReader, Write}; use std::net::{TcpListener, TcpStream}; use std::sync::Arc; use std::sync::mpsc::{Receiver, Sender, channel}; -use std::time::{Duration, Instant}; +use std::time::Instant; use photocraft_automation::budgets::write_reply; use photocraft_automation::security::{ ConnectionLimiter, LineRead, MAX_CONNECTIONS, MAX_REQUEST_BYTES, authentication_reply, configure_stream, read_bounded_line, }; use photocraft_ui_egui::ControlRequest; +use photocraft_ui_egui::control::{REPLY_DEADLINE, timeout_error}; use serde_json::{Value, json}; pub fn start(port: u16, token: String, ctx: egui::Context) -> Receiver { @@ -83,14 +84,15 @@ fn serve(stream: TcpStream, token: &str, tx: Sender, ctx: egui:: let id = msg.get("id").cloned().unwrap_or(Value::Null); let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string(); let params = msg.get("params").cloned().unwrap_or(json!({})); - let deadline = Instant::now() + Duration::from_secs(60); + let deadline = Instant::now() + REPLY_DEADLINE; let (mut req, rrx) = ControlRequest::new(method, params); req.deadline = Some(deadline); if tx.send(req).is_err() { break; } ctx.request_repaint(); - let mut r = rrx.recv_timeout(deadline.saturating_duration_since(Instant::now())).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})); + let mut r = + rrx.recv_timeout(deadline.saturating_duration_since(Instant::now())).unwrap_or_else(|_| json!({"ok": false, "error": timeout_error()})); if let Some(o) = r.as_object_mut() { o.insert("id".into(), id); } @@ -111,6 +113,7 @@ fn serve(stream: TcpStream, token: &str, tx: Sender, ctx: egui:: mod tests { use super::*; use std::io::BufRead; + use std::time::Duration; #[test] fn oversized_reply_preserves_framing_id_and_the_next_control_request() { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 357e99cd..fac19f60 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -41,6 +41,19 @@ use crate::state::{DialogKind, Tool}; pub type ControlResponse = Value; +/// The control server's per-request reply deadline. +pub const REPLY_DEADLINE: std::time::Duration = std::time::Duration::from_secs(60); + +/// The error both timeout sites return when a request's [`REPLY_DEADLINE`] passes: the +/// transport thread waiting for a reply, and the UI thread rejecting a request it is only +/// now draining. Built from the same constant, so the text cannot drift from the deadline. +pub fn timeout_error() -> String { + format!( + "timeout: no reply in {}s — the command may still be running, or the app was not drawing frames (display asleep, window fully occluded, or a long operation blocked the frame loop)", + REPLY_DEADLINE.as_secs() + ) +} + pub struct ControlRequest { pub method: String, pub params: Value, @@ -1254,13 +1267,25 @@ mod tests { retry.deadline = Some(Instant::now() + Duration::from_secs(60)); tx.send(retry).unwrap(); app.drain_control(&egui::Context::default()); - assert_eq!(expired_reply.try_recv().unwrap(), json!({"ok": false, "error": "timeout"})); + assert_eq!(expired_reply.try_recv().unwrap(), json!({"ok": false, "error": super::timeout_error()})); assert_eq!(retry_reply.try_recv().unwrap()["ok"], true); let after = app.session.active().unwrap(); assert_eq!(after.doc.layers.len(), layers + 1, "only the live retry adds a layer"); assert_eq!(after.history.past_len(), steps + 1, "the expired edit adds no undo step"); } + #[test] + fn timeout_errors_name_the_likely_causes() { + // #2489: the reply must tell "wait" from "check the machine", and its number + // must come from the deadline it reports. + let error = super::timeout_error(); + assert!(error.starts_with("timeout"), "machines match on the prefix"); + assert!(error.contains(&format!("{}s", super::REPLY_DEADLINE.as_secs())), "the text matches the deadline: {error}"); + assert!(error.contains("may still be running"), "the wait-it-out cause"); + assert!(error.contains("display asleep"), "the check-the-display cause"); + assert!(error.contains("frame"), "the frame-loop cause"); + } + #[test] fn deadline_free_requests_run_even_after_the_receiver_is_dropped() { let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); diff --git a/crates/ui-egui/src/file_dialog/tests.rs b/crates/ui-egui/src/file_dialog/tests.rs index d490e8b1..56eb45b6 100644 --- a/crates/ui-egui/src/file_dialog/tests.rs +++ b/crates/ui-egui/src/file_dialog/tests.rs @@ -292,12 +292,6 @@ fn save_and_export_dialogs_start_beside_the_document() { } } -/// The path `pick_save` suggests for `name` in `dir` on this platform: it joins with `Path::join`, -/// which uses `\` on Windows, so the expected values must be built the same way (#2745). -fn joined(dir: &str, name: &str) -> String { - std::path::Path::new(dir).join(name).to_string_lossy().into_owned() -} - /// #1826: after the user navigates a save or export dialog elsewhere, the document's next dialog /// starts there. Save As of a saved document still suggests its own path; other documents and /// closed documents are unaffected. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index b8085cb0..f8fe21ba 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1252,7 +1252,7 @@ impl PhotocraftApp { while let Ok(req) = rx.try_recv() { let reply = req.reply.clone(); if req.deadline.is_some_and(|deadline| std::time::Instant::now() >= deadline) { - let _ = reply.send(serde_json::json!({"ok": false, "error": "timeout"})); + let _ = reply.send(serde_json::json!({"ok": false, "error": control::timeout_error()})); continue; } match control::handle(self, ctx, &req) { diff --git a/docs/control-protocol.md b/docs/control-protocol.md index ce81dd67..3d67cf67 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -34,7 +34,7 @@ Each reply is one JSON line with the same `id`: {"id": 2, "ok": false, "error": "unknown tool `foo`"} ``` -The desktop server waits up to 60 seconds for a reply. A request still queued at that deadline is rejected before dispatch; a timeout does not cancel work that has already started. +The desktop server waits up to 60 seconds for a reply. A request still queued at that deadline is rejected before dispatch; a timeout does not cancel work that has already started. The timeout error names the likely causes: the command may still be running, or the request was queued while the window was not drawing frames — on Wayland the frame update (and with it the control drain) only runs on compositor frame callbacks, so a display that is asleep or a fully occluded window makes every request time out while the app itself is healthy. The transport is `apps/photocraft/src/control_server.rs`, and the handlers are in `crates/ui-egui/src/control.rs`. The MCP server (`photocraft-cli mcp --bridge 127.0.0.1:`, crate `photocraft-automation`) wraps this same protocol. See [MCP bridge](#mcp-bridge) below.