Control/MCP: an export or a save still writes its file, not "applies"; document the 300 s wait

Review of #1989 on this PR: a STEP export that outlasts its wait was said to
"still apply when it finishes", though it only writes a file. control_pending
now tells three effects apart: reads only, writes a file (file.export,
file.save, file.save_as, file.save_mesh, another *.export* command given a
path, ui.render, ui.screenshot with a path) and changes the design. The app's
timeout text and the MCP bridge's still-running texts say "will still write
the file when it finishes" for the second; a request the bridge did not send
itself is no longer said to apply for certain.

docs/mcp.md: a tool call may block up to 300 s (60 s app wait + 240 s
polling) before it returns `still running:`, so MCP clients need a per-call
timeout above that, or should fall back to `pending`.

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

apps/solidcraft/src/control_server.rs

@@ -182,15 +182,21 @@ struct Pending {
 }
 
 /// The reply text for a request that outlasts its wait. A request that only reads changes
-/// nothing when it finishes; any other still applies, as it was not cancelled.
-fn timeout_message(what: &str, reads_only: bool) -> String {
-    if reads_only {
-        format!("timeout: {what} is still running in the app (it only reads: nothing changes); ask `control.pending` for its answer")
-    } else {
-        format!(
+/// nothing when it finishes; an export or a save still writes its file, and any other request
+/// still applies, as it was not cancelled.
+fn timeout_message(what: &str, effect: control_pending::Effect) -> String {
+    match effect {
+        control_pending::Effect::ReadsOnly => {
+            format!("timeout: {what} is still running in the app ({}); ask `control.pending` for its answer", effect.when_done())
+        }
+        control_pending::Effect::WritesFile => format!(
+            "timeout: {what} is still running in the app and will still write the file when it finishes (it was not cancelled: \
+             don't send it again; the design does not change); ask `control.pending` for its result"
+        ),
+        control_pending::Effect::Changes => format!(
             "timeout: {what} is still running in the app and will still apply when it finishes (it was not cancelled: don't send it \
              again); ask `control.pending` for its result"
-        )
+        ),
     }
 }
 
@@ -269,7 +275,7 @@ fn serve_with(stream: TcpStream, token: &str, tx: Sender<ControlRequest>, ctx: e
                     // A batch replaces many short requests: the whole of it may take long (#743).
                     let wait = if patient || method == "engine.batch" { waits.long } else { waits.short };
                     let what = control_pending::describe(&method, &params);
-                    let reads = control_pending::reads_only(&method, &params);
+                    let effect = control_pending::effect(&method, &params);
                     let (req, rrx) = ControlRequest::new(method.clone(), params);
                     if tx.send(req).is_err() {
                         break;
@@ -278,7 +284,7 @@ fn serve_with(stream: TcpStream, token: &str, tx: Sender<ControlRequest>, ctx: e
                     match rrx.recv_timeout(wait) {
                         Ok(r) => r,
                         Err(_) => {
-                            let r = json!({"ok": false, "running": true, "error": timeout_message(&what, reads)});
+                            let r = json!({"ok": false, "running": true, "error": timeout_message(&what, effect)});
                             pending = Some(Pending { id: id.clone(), method, what, reply: rrx });
                             r
                         }
@@ -377,16 +383,23 @@ mod tests {
         assert_eq!(*ran.lock().unwrap(), ["slow", "ui.view"], "the busy request never reached the app");
     }
 
-    /// A timed-out request that only reads is not said to "still apply"; a change is, by name.
+    /// A timed-out request that only reads is not said to "still apply", nor is one that only
+    /// writes a file (an export, #1989); a change is, by name.
     #[test]
     fn timeout_text_tells_reads_from_changes() {
-        let says = |method: &str, p: Value| timeout_message(&control_pending::describe(method, &p), control_pending::reads_only(method, &p));
+        let says = |method: &str, p: Value| timeout_message(&control_pending::describe(method, &p), control_pending::effect(method, &p));
         let read = says("ui.inspect", json!({}));
         assert!(read.starts_with("timeout") && read.contains("only reads") && !read.contains("will still apply"), "{read}");
         let read = says("engine.execute", json!({"command": "sketch.inspect"}));
         assert!(read.contains("only reads") && read.contains("sketch.inspect"), "{read}");
         let change = says("engine.execute", json!({"command": "solid.fillet"}));
         assert!(change.starts_with("timeout") && change.contains("will still apply") && change.contains("solid.fillet"), "{change}");
+        let export = says("engine.execute", json!({"command": "file.export", "params": {"path": "/tmp/wall.step"}}));
+        assert!(export.contains("will still write the file") && !export.contains("apply") && export.contains("file.export"), "{export}");
+        let save = says("engine.execute", json!({"command": "file.save"}));
+        assert!(save.contains("will still write the file") && !save.contains("apply"), "{save}");
+        let render = says("ui.render", json!({"path": "/tmp/a.png"}));
+        assert!(render.contains("will still write the file"), "{render}");
     }
 
     /// #205: a connection runs nothing until a request carries the launch's token; a wrong or

crates/engine/src/control_pending.rs

@@ -1,10 +1,35 @@
 //! What a client is told about a control request that outlasts its wait (docs/control-protocol.md,
-//! #1618): the app's control server and the bridged MCP server both say whether the request
-//! still changes something when it finishes, or only reads.
+//! #1618): the app's control server and the bridged MCP server both say what the request still
+//! does when it finishes: nothing (it only reads), write a file (an export or a save), or change
+//! the design.
 
 use serde_json::Value;
 
+/// What a request still running does when it finishes.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum Effect {
+    /// It only reads (the design, the window or the command list): nothing changes.
+    ReadsOnly,
+    /// It writes a file (an export, a save, a render to a path); the design stays as it is.
+    WritesFile,
+    /// It changes the design (or may: anything not known to read or only write a file).
+    Changes,
+}
+
+impl Effect {
+    /// What happens when the request finishes, for a message: "it only reads: nothing changes",
+    /// "it was not cancelled and will still write the file when it finishes", …
+    pub fn when_done(self) -> &'static str {
+        match self {
+            Effect::ReadsOnly => "it only reads: nothing changes",
+            Effect::WritesFile => "it was not cancelled and will still write the file when it finishes; the design does not change",
+            Effect::Changes => "it was not cancelled and will still apply when it finishes",
+        }
+    }
+}
+
 /// Control methods that only read: a timed-out one changes nothing when it finishes.
+/// (`ui.screenshot` with a `path`, and `ui.render`, write a file: see [`effect`].)
 const READ_METHODS: &[&str] = &[
     "control.pending",
     "document.inspect",
@@ -12,14 +37,16 @@ const READ_METHODS: &[&str] = &[
     "ui.at",
     "ui.handles",
     "ui.inspect",
-    "ui.render",
-    "ui.screenshot",
     "ui.selection",
     "ui.sketchToScreen",
     "ui.triad",
     "ui.worldToScreen",
 ];
 
+/// Commands that write a file and leave the design as it is. `file.save` and `file.save_as`
+/// count: they mark the design saved, but change nothing in it.
+const FILE_COMMANDS: &[&str] = &["file.export", "file.save", "file.save_as", "file.save_mesh"];
+
 /// Commands that only read the design (inspection, lists, measurements).
 pub fn command_reads_only(id: &str) -> bool {
     let id = crate::find_command(id).map_or(id, |c| c.id);
@@ -30,15 +57,42 @@ pub fn command_reads_only(id: &str) -> bool {
         || matches!(id, "document.inspect" | "engine.commands" | "render.status" | "render.backends" | "render.settings.get" | "file.slicers")
 }
 
-/// Whether the request `method` with `params` only reads (the design, the window or the
-/// command list): renders and screenshots count, as they change nothing in the design.
-pub fn reads_only(method: &str, params: &Value) -> bool {
-    match method {
-        "engine.execute" | "command" => params.get("command").or_else(|| params.get("id")).and_then(Value::as_str).is_some_and(command_reads_only),
-        m => READ_METHODS.contains(&m),
+/// What the command `id` with `params` does: an export that is given a `path` (`*.export*`, as
+/// `sketch.export_dxf` or `parameters.export`) or a save writes a file; without a path an export
+/// only answers with its contents.
+pub fn command_effect(id: &str, params: &Value) -> Effect {
+    let id = crate::find_command(id).map_or(id, |c| c.id);
+    let exports = id.split('.').skip(1).any(|part| part.starts_with("export"));
+    if FILE_COMMANDS.contains(&id) || (exports && params.get("path").is_some_and(|p| !p.is_null())) {
+        Effect::WritesFile
+    } else if command_reads_only(id) || exports {
+        Effect::ReadsOnly
+    } else {
+        Effect::Changes
     }
 }
 
+/// What the request `method` with `params` does when it finishes. Renders and screenshots
+/// change nothing in the design; given a `path` they write it.
+pub fn effect(method: &str, params: &Value) -> Effect {
+    match method {
+        "engine.execute" | "command" => match params.get("command").or_else(|| params.get("id")).and_then(Value::as_str) {
+            Some(id) => command_effect(id, params.get("params").unwrap_or(&Value::Null)),
+            None => Effect::Changes,
+        },
+        "ui.render" => Effect::WritesFile,
+        "ui.screenshot" if params.get("path").is_some_and(|p| !p.is_null()) => Effect::WritesFile,
+        "ui.screenshot" => Effect::ReadsOnly,
+        m if READ_METHODS.contains(&m) => Effect::ReadsOnly,
+        _ => Effect::Changes,
+    }
+}
+
+/// Whether the request `method` with `params` only reads.
+pub fn reads_only(method: &str, params: &Value) -> bool {
+    effect(method, params) == Effect::ReadsOnly
+}
+
 /// The request in a message: "`engine.execute` (solid.fillet)", or just "`ui.inspect`".
 pub fn describe(method: &str, params: &Value) -> String {
     let command = matches!(method, "engine.execute" | "command")
@@ -60,12 +114,15 @@ mod tests {
     fn reads_are_told_from_changes() {
         for (m, p) in [
             ("ui.inspect", json!({})),
+            ("ui.screenshot", json!({})),
             ("document.inspect", json!({"measure": true})),
             ("engine.execute", json!({"command": "sketch.inspect"})),
             ("engine.execute", json!({"command": "inspect.measure"})),
             ("engine.execute", json!({"command": "model.edges", "params": {"body": "Body1"}})),
+            ("engine.execute", json!({"command": "parameters.export", "params": {"format": "csv"}})),
         ] {
             assert!(reads_only(m, &p), "{m} {p}");
+            assert_eq!(effect(m, &p), Effect::ReadsOnly, "{m} {p}");
         }
         for (m, p) in [
             ("engine.execute", json!({"command": "solid.fillet"})),
@@ -75,8 +132,30 @@ mod tests {
             ("app.quit", json!({})),
         ] {
             assert!(!reads_only(m, &p), "{m} {p}");
+            assert_eq!(effect(m, &p), Effect::Changes, "{m} {p}");
         }
         assert_eq!(describe("engine.execute", &json!({"command": "solid.fillet"})), "`engine.execute` (solid.fillet)");
         assert_eq!(describe("ui.inspect", &json!({})), "`ui.inspect`");
     }
+
+    /// An export or a save writes a file and leaves the design as it is (#1989: a STEP export
+    /// is not said to "still apply").
+    #[test]
+    fn file_writers_are_told_apart() {
+        for (m, p) in [
+            ("engine.execute", json!({"command": "file.export", "params": {"path": "/tmp/a.step"}})),
+            ("engine.execute", json!({"command": "file.save", "params": {}})),
+            ("engine.execute", json!({"command": "file.save_as", "params": {"path": "/tmp/a.solidcraft"}})),
+            ("engine.execute", json!({"command": "file.save_mesh", "params": {"path": "/tmp/a.stl"}})),
+            ("engine.execute", json!({"command": "sketch.export_dxf", "params": {"path": "/tmp/a.dxf"}})),
+            ("ui.render", json!({"path": "/tmp/a.png"})),
+            ("ui.screenshot", json!({"path": "/tmp/a.png"})),
+        ] {
+            assert_eq!(effect(m, &p), Effect::WritesFile, "{m} {p}");
+            assert!(!reads_only(m, &p), "{m} {p}");
+        }
+        assert!(Effect::WritesFile.when_done().contains("will still write the file") && !Effect::WritesFile.when_done().contains("apply"));
+        assert!(Effect::Changes.when_done().contains("will still apply"));
+        assert!(Effect::ReadsOnly.when_done().contains("nothing changes"));
+    }
 }

crates/mcp/src/backend.rs

@@ -64,7 +64,8 @@ pub(crate) const STILL_RUNNING: &str = "still running:";
 /// through `control.pending`, so the connection is kept for it.
 struct Outstanding {
     what: String,
-    reads_only: bool,
+    /// What it does when it finishes; `None`: not known (a request this bridge did not send).
+    effect: Option<control_pending::Effect>,
     since: Instant,
 }
 
@@ -317,7 +318,7 @@ impl Remote {
     /// The tool error for the outstanding request still running after its wait.
     fn still_running(&self, earlier: bool) -> String {
         let Some(o) = &self.outstanding else { return format!("{STILL_RUNNING} a request is still running in the app") };
-        let effect = if o.reads_only { "it only reads: nothing changes" } else { "it was not cancelled and will still apply when it finishes" };
+        let effect = o.effect.map_or("it was not cancelled and may still change the design when it finishes", control_pending::Effect::when_done);
         let settle = self.waits.settle.as_secs();
         if earlier {
             format!(
@@ -376,7 +377,7 @@ impl Remote {
     fn call_once(&mut self, method: &str, params: Value, retry_busy: bool) -> Result<Value, String> {
         self.settle_earlier()?;
         let what = control_pending::describe(method, &params);
-        let reads_only = control_pending::reads_only(method, &params);
+        let effect = control_pending::effect(method, &params);
         let again = retry_busy.then(|| params.clone());
         let sent = Instant::now();
         let line = self.request_line(method, params);
@@ -394,7 +395,7 @@ impl Remote {
         let v: Value = serde_json::from_str(&reply).map_err(|e| format!("bad reply from the app: {e}"))?;
         // Timed out in the app, still running (#1618): wait for its result on this connection.
         if v.get("running").and_then(Value::as_bool) == Some(true) {
-            self.outstanding = Some(Outstanding { what: what.clone(), reads_only, since: sent });
+            self.outstanding = Some(Outstanding { what: what.clone(), effect: Some(effect), since: sent });
             eprintln!(
                 "solidcraft-mcp: {what} is still running in the app after {} s; waiting up to {} s more for it",
                 sent.elapsed().as_secs(),
@@ -413,7 +414,7 @@ impl Remote {
         if v.get("busy").and_then(Value::as_bool) == Some(true)
             && let Some(params) = again
         {
-            self.outstanding = Some(Outstanding { what: "an earlier request".into(), reads_only: false, since: Instant::now() });
+            self.outstanding = Some(Outstanding { what: "an earlier request".into(), effect: None, since: Instant::now() });
             self.settle_earlier()?;
             return self.call_once(method, params, false);
         }

crates/mcp/src/tests.rs

@@ -1624,7 +1624,7 @@ fn a_timed_out_request_is_waited_for_and_returns_its_result() {
 }
 
 /// A request still running after the wait is left to the `pending` tool: the error says so
-/// (and that it will still apply), later calls are not sent behind it, and `pending` returns
+/// (and that it will still apply, or only write its file), later calls are not sent behind it, and `pending` returns
 /// its result once it lands, on the same connection.
 #[test]
 fn a_request_still_running_is_left_to_the_pending_tool() {
@@ -1674,6 +1674,14 @@ fn a_request_still_running_is_left_to_the_pending_tool() {
     done.store(false, Ordering::SeqCst);
     let e = remote.call("engine.execute", json!({"command": "sketch.inspect"})).unwrap_err();
     assert!(e.contains("only reads") && !e.contains("will still apply"), "{e}");
+
+    // An export only writes its file (#1989): it is not said to apply either.
+    let mut other = crate::Remote::connect(&addr).unwrap();
+    other.set_waits(quick_waits(60, 60));
+    let e = other.call("engine.execute", json!({"command": "file.export", "params": {"path": "/tmp/wall.step"}})).unwrap_err();
+    assert!(e.starts_with(STILL_RUNNING) && e.contains("will still write the file") && !e.contains("apply"), "{e}");
+    let e = other.call("document.inspect", json!({})).unwrap_err();
+    assert!(e.contains("earlier request") && e.contains("will still write the file"), "{e}");
 }
 
 /// The headless session never leaves a request running.

docs/control-protocol.md

@@ -53,8 +53,11 @@ limit applies to the MCP server.
 
 A timed-out request has not failed: it is still running in the app, and a change still applies
 when it finishes (a long fillet, say), so don't simply send it again. The timeout's text says
-which: a request that only reads (`ui.inspect`, `document.inspect`, an inspection command)
-"only reads: nothing changes". The connection keeps its reply (#1618). Ask for it
+what it still does: a request that only reads (`ui.inspect`, `document.inspect`, an inspection
+command) "only reads: nothing changes"; one that only writes a file (`file.export`,
+`file.save`, `file.save_as`, `file.save_mesh`, another export given a `path`, `ui.render`, or
+`ui.screenshot` with a `path`) "will still write the file when it finishes" and leaves the design
+as it is; any other request "will still apply when it finishes". The connection keeps its reply (#1618). Ask for it
 with `control.pending`: `{"pending": <id>, "method": …, "done": false}` while it runs, then
 `{"pending": <id>, "method": …, "done": true, "reply": {…}}` (its own reply, once), and
 `{"pending": null}` when nothing is outstanding. Until it lands, any other request on that

docs/mcp.md

@@ -197,13 +197,20 @@ not cancelled. The bridged server then keeps the connection, where the reply wil
 asks the app (`control.pending`) every half second for up to 240 seconds more, saying on stderr
 how long it waits; when the request finishes, the tool call returns its real result. If it still
 runs after that, the tool call fails with an error starting `still running:` that names the
-request and says whether it will still apply (a request that only reads changes nothing). Don't
+request and says what it still does: it will still apply when it finishes, or will still write
+the file (an export or a save: the design does not change), or only reads (nothing changes). Don't
 send it again: call the `pending` tool, which waits up to 60 seconds for its result each time.
 Any other tool call first waits up to 60 seconds for it too, and is not sent while it still runs
 (so nothing is queued to apply unseen behind it); a request that finishes in that wait keeps its
 result for `pending`. If the connection to the app is lost meanwhile, the result is lost with it,
 and `pending` says so: inspect the design before sending a change again.
 
+So a single tool call may block for up to 300 seconds (the app's 60 s wait plus 240 s of polling)
+before it returns `still running:`, and a call made while an earlier request still runs for up to
+60 seconds more. An MCP client needs a per-call timeout above that (say 330 s) to see that error;
+a client that gives up sooner only sees its own timeout, though the request goes on running in the
+app: then call `pending` (or `inspect_design`) before sending anything again.
+
 A `batch` step that is still running stops the batch like a failure. Its rollback first waits for
 the step (as above), so a restored checkpoint takes the step back too; if it still runs, the
 reply says `rolled_back: false`, marks the step `still_running` and lists it in `applied`, as it