Control: timeout errors name the likely causes (#2489) (#2497)

* Control: timeout text derives from the reply deadline (#2489 review)

The server deadline and both timeout replies now share one constant:
REPLY_DEADLINE drives the serve thread's clock and the text is built from
it, so the message can never claim a number the server no longer uses.

* Tests: drop the duplicate joined() helper left by #2782 and #2853 both landing

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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<ControlRequest> {
@@ -83,14 +84,15 @@ fn serve(stream: TcpStream, token: &str, tx: Sender<ControlRequest>, 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<ControlRequest>, 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() {

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());

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.

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) {

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:<port>`, crate `photocraft-automation`) wraps this same protocol. See [MCP bridge](#mcp-bridge) below.