From 44f3141c3dc9a94262ea63eaedc59b89139e99ee Mon Sep 17 00:00:00 2001
From: KnottyDyes <47539659+KnottyDyes@users.noreply.github.com>
Date: Sun, 11 Oct 2026 09:31:39 -0600
Subject: [PATCH] MCP: lost replies report uncertainty instead of replaying
 edits

Drop a failed control connection and reconnect for the next request, never resend an operation that may already have completed.

A loopback regression applies a rectangle then drops its reply. It reproduces two edits before the fix and verifies one edit, an error, and successful inspection afterward.
---
 crates/mcp/src/backend.rs | 15 +++--------
 crates/mcp/src/tests.rs   | 55 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs
index e969e8a5..c607216f 100644
--- a/crates/mcp/src/backend.rs
+++ b/crates/mcp/src/backend.rs
@@ -78,17 +78,10 @@ impl Backend for Remote {
         let id = self.next_id;
         self.next_id += 1;
         let line = json!({"id": id, "method": method, "params": params}).to_string();
-        // One retry with a fresh connection (the app may have restarted).
-        let reply = match self.roundtrip(&line) {
-            Ok(r) => r,
-            Err(_) => {
-                self.conn = None;
-                self.roundtrip(&line).map_err(|e| {
-                    self.conn = None;
-                    format!("VectorCraft app at {} is not reachable: {e}", self.addr)
-                })?
-            }
-        };
+        let reply = self.roundtrip(&line).map_err(|e| {
+            self.conn = None;
+            format!("VectorCraft app at {}: `{method}` failed: {e}; operation may have completed; inspect state before retrying", self.addr)
+        })?;
         let v: Value = serde_json::from_str(reply.trim()).map_err(|e| format!("bad reply from app: {e}"))?;
         if v.get("ok").and_then(Value::as_bool) == Some(true) {
             Ok(v.get("result").cloned().unwrap_or(Value::Null))
diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs
index 0b2117ab..8f474ec4 100644
--- a/crates/mcp/src/tests.rs
+++ b/crates/mcp/src/tests.rs
@@ -430,6 +430,61 @@ fn remote_forwards_methods() {
     assert_eq!(seen[1]["params"], json!({"command": "object.group", "params": {}}));
 }
 
+#[test]
+fn remote_does_not_replay_when_edit_reply_is_lost() {
+    let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+    listener.set_nonblocking(true).unwrap();
+    let addr = listener.local_addr().unwrap().to_string();
+    let h = std::thread::spawn(move || {
+        let timeout = std::time::Duration::from_secs(5);
+        let mut backend = Headless::with_document();
+        let base = backend.call("document.inspect", json!({})).unwrap()["objects"].as_u64().unwrap();
+        for connection in 0..2 {
+            let deadline = std::time::Instant::now() + timeout;
+            let stream = loop {
+                match listener.accept() {
+                    Ok((stream, _)) => break stream,
+                    Err(e) if e.kind() == std::io::ErrorKind::WouldBlock && std::time::Instant::now() < deadline => std::thread::yield_now(),
+                    Err(e) => panic!("accept failed: {e}"),
+                }
+            };
+            stream.set_nonblocking(false).unwrap();
+            stream.set_read_timeout(Some(timeout)).unwrap();
+            stream.set_write_timeout(Some(timeout)).unwrap();
+            let mut out = stream.try_clone().unwrap();
+            let mut reader = BufReader::new(stream);
+            for _ in 0..2 {
+                let mut line = String::new();
+                reader.read_line(&mut line).unwrap();
+                let msg: Value = serde_json::from_str(&line).unwrap();
+                let method = msg["method"].as_str().unwrap();
+                let result = backend.call(method, msg["params"].clone()).unwrap();
+                if connection == 0 {
+                    assert_eq!(method, "engine.execute");
+                    break;
+                }
+                let reply = json!({"id": msg["id"], "ok": true, "result": result});
+                writeln!(out, "{reply}").unwrap();
+                if method == "document.inspect" {
+                    return result["objects"].as_u64().unwrap() - base;
+                }
+                assert_eq!(method, "engine.execute");
+            }
+        }
+        panic!("document.inspect was not received");
+    });
+
+    let mut remote = Remote::connect(&addr).unwrap();
+    let first = remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {"x": 10, "y": 10, "width": 20, "height": 20}}));
+    let next = remote.call("document.inspect", json!({}));
+    let edits = h.join().unwrap();
+
+    assert_eq!(edits, 1, "a lost reply must not duplicate the edit");
+    let error = first.expect_err("the lost reply must report uncertainty");
+    assert!(error.contains("operation may have completed"), "{error}");
+    assert!(next.is_ok(), "the next call must reconnect: {next:?}");
+}
+
 #[test]
 fn remote_connect_fails_fast() {
     // A port this test holds, bound but not listening, so connecting to it is refused. (A port