Fix #1725: a control request its caller timed out on is withdrawn, not run later

The worker answered "timeout" after 60 s and left the request on the queue, so
the app ran it whenever it next drained the queue: an edit nobody was waiting
for, and a second one if the caller had retried. A request now leaves the
queue once: taken by the app, or given up by its caller, whichever comes
first. The timeout reply says which ("will not run" / "still running").

apps/vectorcraft/src/control_server.rs

@@ -29,6 +29,21 @@ pub const MAX_CONNECTIONS: usize = 16;
 /// Size in bytes of the buffer that [`write_line`] writes a reply line through.
 const REPLY_CHUNK: usize = 64 * 1024;
 
+/// How long a request waits for the app's reply.
+const REPLY_WAIT: Duration = Duration::from_secs(60);
+
+/// The reply to a request the app didn't answer in [`REPLY_WAIT`]. One it had not started is
+/// withdrawn, so it never runs after its caller was told it failed; one it is running can't be
+/// stopped, and the reply says its outcome isn't known.
+fn timed_out(ticket: &vectorcraft_ui_egui::ControlTicket) -> Value {
+    let error = if ticket.give_up() {
+        "timeout: the app did not start the request in time; it was withdrawn and will not run"
+    } else {
+        "timeout: the app is still running the request; it may yet complete"
+    };
+    json!({"ok": false, "error": error})
+}
+
 #[derive(Clone, Copy)]
 struct ConnectionTimeouts {
     idle: Duration,
@@ -106,9 +121,10 @@ fn spawn_connection(
         let _slot = slot;
         serve_socket(stream, timeouts, |method, params| {
             let (req, rrx) = ControlRequest::new(method, params);
+            let ticket = req.ticket();
             tx.send(req).ok()?;
             ctx.request_repaint();
-            Some(rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})))
+            Some(rrx.recv_timeout(REPLY_WAIT).unwrap_or_else(|_| timed_out(&ticket)))
         });
     })
 }
@@ -455,6 +471,19 @@ mod tests {
         json!({"ok": true, "result": {"layers": [{"children": paths, "id": 1, "kind": "Layer", "name": "Layer 1"}]}})
     }
 
+    /// #1725: a request its caller gave up before the app took it never runs; one the app took
+    /// first can't be withdrawn, and the timeout says so.
+    #[test]
+    fn a_timed_out_request_is_withdrawn_unless_it_started() {
+        let (waiting, _rx) = ControlRequest::new("engine.execute", json!({}));
+        assert!(timed_out(&waiting.ticket())["error"].as_str().unwrap().contains("will not run"));
+        assert!(!waiting.claim(), "the app skips a withdrawn request");
+        let (running, _rx) = ControlRequest::new("engine.execute", json!({}));
+        assert!(running.claim());
+        assert!(timed_out(&running.ticket())["error"].as_str().unwrap().contains("still running"));
+        assert!(!running.claim(), "a request is taken once");
+    }
+
     #[test]
     fn requests_are_answered_in_order_with_ids() {
         let (r, calls) = run(b"{\"id\":1,\"method\":\"a\"}\n\n{\"id\":2,\"method\":\"b\",\"params\":{}}\r\n{\"method\":\"c\"}");

crates/ui-egui/src/control.rs

@@ -24,6 +24,8 @@
 //! - `app.export {path?, format?, …document.export options}`: encoded by the engine, written through
 //!   the host; the document keeps its path. No path → `{dataBase64, format, bytes}` (as headless)
 
+use std::sync::Arc;
+use std::sync::atomic::{AtomicU8, Ordering};
 use std::sync::mpsc::Sender;
 
 use serde_json::{Value, json};
@@ -63,12 +65,41 @@ pub struct ControlRequest {
     pub method: String,
     pub params: Value,
     pub reply: Sender<ControlResponse>,
+    state: Arc<AtomicU8>,
+}
+
+/// A request's state: waiting to run, taken by the app ([`ControlRequest::claim`]), or given up
+/// by its caller before that ([`Ticket::give_up`]). It leaves `QUEUED` once, one way or the other.
+const QUEUED: u8 = 0;
+const STARTED: u8 = 1;
+const GIVEN_UP: u8 = 2;
+
+/// The caller's side of a queued [`ControlRequest`].
+pub struct Ticket(Arc<AtomicU8>);
+
+impl Ticket {
+    /// Stop waiting for the request → `true` when it had not started, and so never runs: a caller
+    /// told "timeout" can then retry without the edit being applied twice, or applied later when
+    /// nobody expects it (#1725). `false`: the app is running it (or has); its outcome is unknown.
+    pub fn give_up(&self) -> bool {
+        self.0.compare_exchange(QUEUED, GIVEN_UP, Ordering::SeqCst, Ordering::SeqCst).is_ok()
+    }
 }
 
 impl ControlRequest {
     pub fn new(method: impl Into<String>, params: Value) -> (Self, std::sync::mpsc::Receiver<ControlResponse>) {
         let (tx, rx) = std::sync::mpsc::channel();
-        (Self { method: method.into(), params, reply: tx }, rx)
+        (Self { method: method.into(), params, reply: tx, state: Arc::new(AtomicU8::new(QUEUED)) }, rx)
+    }
+
+    /// The caller's handle on this request, to give it up with.
+    pub fn ticket(&self) -> Ticket {
+        Ticket(self.state.clone())
+    }
+
+    /// Take the request to run it → `false` when its caller gave it up meanwhile (don't run it).
+    pub fn claim(&self) -> bool {
+        self.state.compare_exchange(QUEUED, STARTED, Ordering::SeqCst, Ordering::SeqCst).is_ok()
     }
 }
 

crates/ui-egui/src/lib.rs

@@ -162,7 +162,7 @@ use vectorcraft_engine::cmd::fileio;
 use vectorcraft_engine::file_access::{self, AutomationRoots};
 use vectorcraft_engine::{Session, ViewInfo};
 
-pub use control::{ControlRequest, ControlResponse};
+pub use control::{ControlRequest, ControlResponse, Ticket as ControlTicket};
 pub use state::{UiState, View};
 pub use sysclip::{ClipboardProbeFactory, SystemClipboard};
 
@@ -769,6 +769,11 @@ impl VectorcraftApp {
         let mut handled = 0;
         while handled < control::MAX_CONTROL_REQUESTS_PER_FRAME {
             let Ok(req) = rx.try_recv() else { break };
+            // Its caller stopped waiting (the app was busy or its window not drawn for too long)
+            // and was told it won't run: an edit applied now would come from nowhere (#1725).
+            if !req.claim() {
+                continue;
+            }
             handled += 1;
             let reply = req.reply.clone();
             // Guarded per request: a panic must not drop the channel (taken out of `self` above).

docs/control-protocol.md

@@ -43,6 +43,12 @@ and does not append a partial gesture. Clients can retry once earlier input has
 scheduled. These limits cover synthetic input admission, separately from socket transport and
 command-processing timeouts.
 
+A request the app hasn't answered after **60 s** gets `{"ok": false, "error": "timeout: …"}`. If the
+app had not started it (it was busy, or its window wasn't being drawn), the request is withdrawn
+and never runs, and the error says "will not run": retrying is safe. If the app was already
+running it, the error says "still running": it may yet complete, so check the document before
+retrying.
+
 ## Confining file access
 
 ```sh