Merge pull request #1862 from storytold/fix/audit-ids-batch-control

Fix #1718, #1697, #1725, #1717: id exhaustion, batch rollback, timed-out control requests, RGBA size before decoding

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/doc/src/lib.rs

@@ -31,6 +31,7 @@ pub mod overprint;
 pub mod pattern;
 pub mod perspective;
 mod pixels;
+pub use pixels::MAX_RASTER_ALLOC;
 pub mod placed_document;
 pub mod profiles;
 pub mod puppet;
@@ -775,7 +776,9 @@ impl Document {
     /// Allocate a fresh node id.
     pub fn alloc_id(&mut self) -> NodeId {
         let id = NodeId(self.next_id);
-        self.next_id += 1;
+        // Never reached from a counter [`Self::fix_next_id`] has seen (it stays under
+        // [`MAX_NEXT_ID`]); saturating so that no counter can overflow.
+        self.next_id = self.next_id.saturating_add(1);
         id
     }
     pub fn peek_next_id(&self) -> u64 {
@@ -783,14 +786,22 @@ impl Document {
     }
     /// Ensure `next_id` is above every id in the tree (after deserializing foreign data).
     pub fn fix_next_id(&mut self) {
+        // Ids from [`MAX_NEXT_ID`] up (a damaged or hand-made file: #1718) are left out, as is a
+        // counter up there: new ids start under them and never reach them, where counting on from
+        // the largest would run out and hand out ids the document already uses.
         let mut max = 0;
+        let mut see = |id: u64| {
+            if id < MAX_NEXT_ID {
+                max = max.max(id);
+            }
+        };
         for l in &self.layers {
-            l.walk(&mut |n| max = max.max(n.id.0));
+            l.walk(&mut |n| see(n.id.0));
         }
-        max = self.slices.iter().fold(max, |m, s| m.max(s.id.0));
-        max = self.assets.iter().fold(max, |m, a| m.max(a.id));
-        // An id of u64::MAX (a damaged file) can't overflow: ids after it are reused at worst.
-        self.next_id = self.next_id.max(max.saturating_add(1));
+        self.slices.iter().for_each(|s| see(s.id.0));
+        self.assets.iter().for_each(|a| see(a.id));
+        let counter = Some(self.next_id).filter(|n| *n <= MAX_NEXT_ID).unwrap_or(0);
+        self.next_id = counter.max(max.saturating_add(1));
     }
 
     /// Find a node anywhere in the tree.
@@ -1376,7 +1387,7 @@ impl Document {
         self.guides.extend(copies);
     }
     pub fn next_artboard_id(&self) -> u32 {
-        self.artboards.iter().map(|a| a.id).max().unwrap_or(0) + 1
+        free_id(self.artboards.iter().map(|a| a.id))
     }
     pub fn pattern(&self, name: &str) -> Option<&PatternDef> {
         self.patterns.iter().find(|p| p.name == name)
@@ -1460,7 +1471,7 @@ impl Document {
     }
     /// An id no graphic style has.
     pub fn next_graphic_style_id(&self) -> u32 {
-        self.graphic_styles.iter().map(|g| g.id).max().unwrap_or(0) + 1
+        free_id(self.graphic_styles.iter().map(|g| g.id))
     }
     /// The id of graphic style `index`, assigning one if it has none yet.
     pub fn graphic_style_id(&mut self, index: usize) -> u32 {
@@ -1499,11 +1510,29 @@ fn default_graphic_styles() -> Vec<GraphicStyle> {
     .collect()
 }
 
+/// Node, slice and asset ids are handed out below this: no document gets near it by counting, so
+/// larger ids in a file are damage (or made by hand) and the counter never follows them (#1718).
+const MAX_NEXT_ID: u64 = 1 << 62;
+
+/// An id (never 0) none of `used` has: one past the largest, or, when that is the largest there
+/// is (a damaged file), the smallest one free.
+fn free_id(used: impl Iterator<Item = u32>) -> u32 {
+    let mut used: Vec<u32> = used.collect();
+    if let Some(next) = used.iter().max().copied().unwrap_or(0).checked_add(1) {
+        return next;
+    }
+    used.sort_unstable();
+    used.dedup();
+    // Ids 1, 2, 3… as far as they are all taken: the first that isn't follows them.
+    let taken = used.iter().filter(|&&id| id != 0).zip(1u32..).take_while(|&(&id, n)| id == n).count();
+    u32::try_from(taken).map_or(u32::MAX, |n| n.saturating_add(1))
+}
+
 impl Document {
     /// Make the next id allocated at least `next`: ids handed out outside the layer tree (such
     /// as an importer's opacity mask and pattern art), which [`Document::fix_next_id`] doesn't see.
     pub fn reserve_ids(&mut self, next: u64) {
-        self.next_id = self.next_id.max(next);
+        self.next_id = self.next_id.max(next.min(MAX_NEXT_ID));
     }
 }
 
@@ -1628,6 +1657,45 @@ mod tests {
         assert_eq!(d.layer_color(l2), LAYER_COLORS[1].1);
     }
 
+    /// #1718: a file whose ids or counters are at their type's largest value still hands out ids
+    /// nothing in it uses (and never 0), instead of overflowing.
+    #[test]
+    fn exhausted_ids_in_a_file_dont_run_out() {
+        let (d, a, b) = doc_with_rects();
+        let mut v = serde_json::to_value(&d).unwrap();
+        v["next_id"] = serde_json::json!(u64::MAX);
+        // Object `a` gets the largest id there is.
+        fn renumber(v: &mut serde_json::Value, from: u64) {
+            match v {
+                serde_json::Value::Object(o) => {
+                    if o.get("id").and_then(serde_json::Value::as_u64) == Some(from) {
+                        o.insert("id".into(), serde_json::json!(u64::MAX));
+                    }
+                    o.values_mut().for_each(|c| renumber(c, from));
+                }
+                serde_json::Value::Array(a) => a.iter_mut().for_each(|c| renumber(c, from)),
+                _ => {}
+            }
+        }
+        renumber(&mut v["layers"], a.0);
+        let mut d: Document = serde_json::from_value(v).unwrap();
+        assert!(d.node(NodeId(u64::MAX)).is_some(), "the file's largest id is kept");
+        d.fix_next_id();
+        let fresh: Vec<NodeId> = (0..3).map(|_| d.alloc_id()).collect();
+        assert!(fresh.iter().all(|id| d.node(*id).is_none() && id.0 > b.0 && id.0 < MAX_NEXT_ID), "{fresh:?}");
+        assert!(fresh[0] != fresh[1] && fresh[1] != fresh[2]);
+        // A counter left at the end by other means stays there rather than wrapping to 0.
+        d.next_id = u64::MAX;
+        assert_eq!((d.alloc_id(), d.peek_next_id()), (NodeId(u64::MAX), u64::MAX));
+        d.reserve_ids(u64::MAX);
+        // Artboards and graphic styles: one past the largest, else the smallest id free.
+        assert_eq!(free_id([1, 2, 7].into_iter()), 8);
+        assert_eq!(free_id([u32::MAX, 0, 1, 2, 4].into_iter()), 3);
+        assert_eq!(free_id([u32::MAX].into_iter()), 1);
+        d.artboards[0].id = u32::MAX;
+        assert_eq!(d.next_artboard_id(), 1);
+    }
+
     #[test]
     fn serde_roundtrip() {
         let (d, _, _) = doc_with_rects();

crates/doc/src/pixels.rs

@@ -6,6 +6,22 @@ use std::sync::Arc;
 
 use crate::ImageBlob;
 
+/// The most memory the decoded RGBA pixels of one image may take: the allowance the importers
+/// and the renderer decode within (a 60 in sheet at 300 ppi is ~475 MB). The web app has 4 GB to
+/// address and aborts when an allocation fails, so it keeps the decoder's own 512 MB.
+pub const MAX_RASTER_ALLOC: u64 = if cfg!(target_arch = "wasm32") { 512 << 20 } else { 2 << 30 };
+
+/// The pixels of encoded image `bytes` as RGBA; `None` when they can't be decoded or would take
+/// more than `max` bytes as RGBA. The size is read from the header first: the decoder's own limit
+/// is on the file's samples, and a grey image is a quarter of what it becomes here (#1717).
+fn rgba_within(bytes: &[u8], max: u64) -> Option<image::RgbaImage> {
+    let (w, h) = image::ImageReader::new(Cursor::new(bytes)).with_guessed_format().ok()?.into_dimensions().ok()?;
+    if u64::from(w) * u64::from(h) * 4 > max {
+        return None;
+    }
+    Some(image::load_from_memory(bytes).ok()?.to_rgba8())
+}
+
 impl ImageBlob {
     /// A PNG blob of `png` bytes.
     pub fn png(png: Vec<u8>) -> Self {
@@ -21,7 +37,7 @@ impl ImageBlob {
     /// pixels left alone, re-encoded as PNG. `f` runs once per distinct colour. `None` when the
     /// image can't be decoded or no pixel changed.
     pub fn map_rgb(&self, mut f: impl FnMut([u8; 3]) -> [u8; 3]) -> Option<ImageBlob> {
-        let mut img = image::load_from_memory(&self.bytes).ok()?.to_rgba8();
+        let mut img = rgba_within(&self.bytes, MAX_RASTER_ALLOC)?;
         let mut memo: HashMap<[u8; 3], [u8; 3]> = HashMap::new();
         let mut changed = false;
         for px in img.pixels_mut().filter(|p| p.0[3] > 0) {
@@ -67,7 +83,7 @@ impl ImageBlob {
         LAST.with_borrow_mut(|last| match last {
             Some((k, img)) if *k == key => Some(img.clone()),
             _ => {
-                let img = Arc::new(image::load_from_memory(&self.bytes).ok()?.to_rgba8());
+                let img = Arc::new(rgba_within(&self.bytes, MAX_RASTER_ALLOC)?);
                 *last = Some((key, img.clone()));
                 Some(img)
             }
@@ -115,6 +131,18 @@ mod tests {
         image::load_from_memory(&b.bytes).unwrap().to_rgba8().pixels().map(|p| p.0).collect()
     }
 
+    /// #1717: the RGBA size is checked from the header, before anything is decoded: a grey image
+    /// the decoder's own limit lets through is four times that as RGBA.
+    #[test]
+    fn pixels_past_the_allowance_are_not_decoded() {
+        let grey = image::GrayImage::from_raw(4, 2, vec![7; 8]).unwrap();
+        let mut png = Vec::new();
+        grey.write_to(&mut Cursor::new(&mut png), image::ImageFormat::Png).unwrap();
+        assert!(rgba_within(&png, 31).is_none(), "8 grey bytes are 32 as RGBA");
+        assert_eq!(rgba_within(&png, 32).map(|i| i.dimensions()), Some((4, 2)));
+        assert!(rgba_within(b"not an image", u64::MAX).is_none());
+    }
+
     #[test]
     fn samples_a_pixel_or_the_average_around_it() {
         let b = blob(&[[255, 0, 0, 255], [0, 255, 0, 255], [0, 0, 255, 255], [9, 9, 9, 0]], 4, 1);

crates/engine/src/cmd/edit.rs

@@ -265,7 +265,7 @@ fn undo_group_end(s: &mut Session, p: &Value) -> Result<Value> {
 /// under an interaction that would then preview on, or cancel to, the document from before (#1719).
 /// A typing session also ends in the Type tool, whose caret and marked text (IME) must follow the
 /// document.
-fn settle_interaction(s: &mut Session) -> Result<()> {
+pub(crate) fn settle_interaction(s: &mut Session) -> Result<()> {
     let typing = typing_in_progress(s);
     s.commit_interaction()?;
     if typing {

crates/engine/src/cmd/fileio/batch.rs

@@ -13,14 +13,22 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result<Value> {
     // What an error rolls back besides the documents: session-level state a step may change
     // (paint defaults, drawing mode, clipboard, the Untitled-N count).
     let saved = (s.paint.clone(), s.fill_active, s.draw_mode, s.draw_inside, s.clipboard.clone(), s.untitled_counter);
-    let (start, active): (Vec<u64>, _) = (s.docs.iter().map(|d| d.uid).collect(), s.active);
+    // A drag or typing still in progress is a step of its own, as before Undo: the batch's steps
+    // would otherwise run inside it and take its baseline as theirs (#1697).
+    if s.active().is_some() {
+        super::super::edit::settle_interaction(s)?;
+    }
+    // The documents as they are now, whole: what a failed step brings back. Their art, history,
+    // file, format and saved state share their data with the open ones until a step changes them.
+    let start = s.docs.clone();
+    let active = s.active;
     s.batch_stash = Some(vec![]);
     let r = run_steps(s, &label, artboard, &mut cmds);
     let stash = s.batch_stash.take().unwrap_or_default();
     let results = match r {
         Ok(results) => results,
         Err(e) => {
-            roll_back(s, &start, stash);
+            roll_back(s, start, &stash);
             s.active = active;
             (s.paint, s.fill_active, s.draw_mode, s.draw_inside, s.clipboard, s.untitled_counter) = saved;
             s.pending_paint = None;
@@ -81,22 +89,17 @@ fn run_steps(s: &mut Session, label: &str, artboard: Option<u64>, cmds: &mut [Va
     Ok(results)
 }
 
-/// The documents as they were before the batch (`start`: their uids in order): the ones it opened
-/// close, the ones it closed or replaced (`stash`) come back, and each one's interaction is undone.
-fn roll_back(s: &mut Session, start: &[u64], mut stash: Vec<DocState>) {
-    let mut open = std::mem::take(&mut s.docs);
-    for &uid in start {
-        // The newest revision the document reached, so the restored one redraws.
-        let newest = open.iter().chain(&stash).filter(|d| d.uid == uid).map(|d| d.revision).max().unwrap_or_default();
-        // Its state before the batch: the first one a step closed or replaced, else the open one.
-        let st = match stash.iter().position(|d| d.uid == uid) {
-            Some(i) => Some(stash.remove(i)),
-            None => open.iter().position(|d| d.uid == uid).map(|i| open.remove(i)),
-        };
-        if let Some(mut st) = st {
-            st.undo_interaction();
-            st.revision = newest.saturating_add(1);
-            s.docs.push(st);
-        }
+/// The documents as they were before the batch (`start`), whatever the steps did to them: opened
+/// ones close, closed or replaced ones (`stash`) come back, and each has its art, selection,
+/// history, file, format and saved state again (#1697: undoing the batch's interaction alone left
+/// a Save As step's new file and an Undo step's history in place). Files the steps wrote stay
+/// written.
+fn roll_back(s: &mut Session, start: Vec<DocState>, stash: &[DocState]) {
+    let open = std::mem::take(&mut s.docs);
+    s.docs = start;
+    for st in &mut s.docs {
+        // Past the newest revision the document reached, so the restored one redraws.
+        let newest = open.iter().chain(stash).filter(|d| d.uid == st.uid).map(|d| d.revision).max().unwrap_or(st.revision);
+        st.revision = newest.saturating_add(1);
     }
 }

crates/engine/src/cmd/fileio/load.rs

@@ -419,7 +419,7 @@ pub(super) fn new_from_template(s: &mut Session, p: &Value) -> Result<Value> {
 /// The most memory decoding one image may take: a 60 in print sheet at 300 ppi (18000 × 6600 px)
 /// is ~475 MB as RGBA, past the decoder's 512 MB default once it is converted. The web app keeps
 /// that default: wasm32 has 4 GB to address and aborts when an allocation fails.
-pub(crate) const MAX_RASTER_ALLOC: u64 = if cfg!(target_arch = "wasm32") { 512 << 20 } else { 2 << 30 };
+pub(crate) use vectorcraft_doc::MAX_RASTER_ALLOC;
 
 /// What to say of encoded image `bytes` that need more than [`MAX_RASTER_ALLOC`] to decode (its
 /// header says so; the pixels aren't read): the renderer, which has the same allowance, draws

crates/engine/tests/model_based.rs

@@ -121,6 +121,44 @@ fn batch_is_one_undo_step_and_rolls_back_on_error() {
     assert_eq!(s.doc().unwrap().doc.node_count(), 1);
 }
 
+/// #1697: a failed batch brings back the whole document, not only its art: the file a Save As
+/// step named and the history an Undo step moved; and a batch leaves a drag in progress its own
+/// step.
+#[test]
+fn a_failed_batch_restores_the_file_the_history_and_a_drag_in_progress() {
+    let mut s = fixtures::session();
+    let rect = |x: i32| json!({"command": "shape.rectangle", "params": {"x": x, "y": 0, "width": 10, "height": 10}});
+    let fail = json!({"command": "no.such.command", "params": {}});
+    exec(&mut s, "shape.rectangle", json!({"x": 50, "y": 0, "width": 10, "height": 10}));
+    let before = (doc_json(&s.doc().unwrap().doc), s.doc().unwrap().history.undo.len(), s.doc().unwrap().history.redo.len());
+    let state = |s: &vectorcraft_engine::Session| {
+        let st = s.doc().unwrap();
+        (doc_json(&st.doc), st.history.undo.len(), st.history.redo.len())
+    };
+    // Save As, then a failing step: the document keeps no file and stays modified.
+    let path = vectorcraft_testkit::temp_dir("batch-rollback").join("named.vectorcraft").to_string_lossy().to_string();
+    let save = json!({"command": "file.saveAs", "params": {"path": path}});
+    assert!(s.execute("command.batch", &json!({"commands": [rect(0), save, fail]})).is_err());
+    assert_eq!(state(&s), before);
+    assert_eq!((s.doc().unwrap().path.as_deref(), s.doc().unwrap().is_dirty()), (None, true));
+    // Undo, then a failing step: the art and both history stacks are as before.
+    let undo = json!({"command": "edit.undo", "params": {}});
+    assert!(s.execute("command.batch", &json!({"commands": [undo, fail]})).is_err());
+    assert_eq!(state(&s), before);
+    // A batch during a drag: the drag is kept as a step, the batch is the next one.
+    exec(&mut s, "select.all", json!({}));
+    s.begin_interaction("Move").unwrap();
+    s.preview("object.move", &json!({"dx": 5, "dy": 0})).unwrap();
+    let moved = doc_json(&s.doc().unwrap().doc);
+    exec(&mut s, "command.batch", json!({"commands": [rect(100)]}));
+    assert!(!s.in_interaction());
+    assert_eq!(s.doc().unwrap().history.undo.len(), before.1 + 2);
+    exec(&mut s, "edit.undo", json!({}));
+    assert_eq!(doc_json(&s.doc().unwrap().doc), moved, "undoing the batch leaves the drag's move");
+    exec(&mut s, "edit.undo", json!({}));
+    assert_eq!(doc_json(&s.doc().unwrap().doc), before.0);
+}
+
 #[test]
 fn empty_batch_is_harmless() {
     let mut s = fixtures::session();

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