From cc518797a8c13fb00877a24df0aea66f066a92e4 Mon Sep 17 00:00:00 2001
From: Fahri Meral <fahrimeral@outlook.com>
Date: Tue, 13 Oct 2026 18:57:07 +0300
Subject: [PATCH] Fix #1728: require the JSON-RPC 2.0 envelope and correlate
 control replies

The server dispatched requests and notifications without ever checking jsonrpc, so a missing or wrong version could invoke tools or flip notification state; Remote accepted any reply with ok/result/error without comparing ids, so a faulty endpoint could attribute a stray result to the current call. handle() now requires jsonrpc 2.0 before dispatch (other envelopes get -32600, or are ignored for notifications, which have no id for a reply) and Remote::call rejects a reply whose id doesn't echo the request's, dropping the connection so the next call starts fresh, with the usual uncertainty error. Three new tests cover wrong/missing versions (incl. batch entries and a state-changing notification) and the mismatch-reconnect; docs/mcp.md documents both. cargo test -p vectorcraft-mcp passes.
---
 crates/mcp/src/backend.rs        | 10 ++++++++
 crates/mcp/src/server.rs         |  6 +++++
 crates/mcp/src/tests.rs          | 36 ++++++++++++++++++++++++++
 crates/mcp/src/tests_protocol.rs | 44 ++++++++++++++++++++++++++++++++
 docs/mcp.md                      |  7 ++++-
 5 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs
index 0fe84b2b..863ace29 100644
--- a/crates/mcp/src/backend.rs
+++ b/crates/mcp/src/backend.rs
@@ -94,6 +94,16 @@ impl Backend for Remote {
             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("id") != Some(&json!(id)) {
+            // Not the answer to this call: the stream is desynchronised (or the endpoint faulty),
+            // so the connection is dropped and the next call starts a fresh one. The operation may
+            // still have completed, as with any reply that never arrived whole (#1728).
+            self.conn = None;
+            return Err(format!(
+                "VectorCraft app at {}: reply id mismatch (expected {id}); connection dropped; operation may have completed; inspect state before retrying",
+                self.addr
+            ));
+        }
         if v.get("ok").and_then(Value::as_bool) == Some(true) {
             Ok(v.get("result").cloned().unwrap_or(Value::Null))
         } else {
diff --git a/crates/mcp/src/server.rs b/crates/mcp/src/server.rs
index b7d6978e..74004283 100644
--- a/crates/mcp/src/server.rs
+++ b/crates/mcp/src/server.rs
@@ -198,6 +198,12 @@ impl Server {
             }
             return Some(error(id.unwrap_or(Value::Null), INVALID_REQUEST, "missing `method`"));
         };
+        // JSON-RPC 2.0 is the only envelope this server speaks: a request or notification in any
+        // other envelope is never dispatched and never changes state (#1728).
+        if o.get("jsonrpc").and_then(Value::as_str) != Some("2.0") {
+            // A notification has no id to address a reply to; ignoring it is the only answer.
+            return id.map(|id| error(id, INVALID_REQUEST, "expected `\"jsonrpc\": \"2.0\"`"));
+        };
         let params = o.get("params").cloned().unwrap_or(Value::Null);
         let Some(id) = id else {
             self.notification(method, &params);
diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs
index d788026a..7682f380 100644
--- a/crates/mcp/src/tests.rs
+++ b/crates/mcp/src/tests.rs
@@ -506,6 +506,42 @@ fn remote_does_not_replay_when_edit_reply_is_lost() {
     assert!(next.is_ok(), "the next call must reconnect: {next:?}");
 }
 
+#[test]
+fn remote_rejects_a_reply_with_a_mismatched_id_and_reconnects() {
+    // A fake control server that answers the first request with another call's id, then serves
+    // correctly: the client must reject the stray reply, drop the connection, and reconnect.
+    let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+    let addr = listener.local_addr().unwrap().to_string();
+    let h = std::thread::spawn(move || {
+        let mut seen = vec![];
+        for stream in listener.incoming().flatten().take(2) {
+            stream.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap();
+            let mut out = stream.try_clone().unwrap();
+            let mut reader = BufReader::new(stream);
+            let mut line = String::new();
+            if reader.read_line(&mut line).unwrap_or(0) == 0 {
+                continue;
+            }
+            let msg: Value = serde_json::from_str(&line).unwrap();
+            // First connection: somebody else's answer. Later ones: the right echo.
+            let id = if seen.is_empty() { json!(msg["id"].as_u64().unwrap_or(0) + 1000) } else { msg["id"].clone() };
+            let reply = json!({"ok": true, "result": {"echo": msg["params"]}, "id": id});
+            writeln!(out, "{reply}").unwrap();
+            seen.push(msg);
+        }
+        seen
+    });
+    let mut remote = Remote::connect(&addr).unwrap();
+    let err = remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {}})).expect_err("a stray reply must not be trusted");
+    assert!(err.contains("mismatch"), "{err}");
+    let good = remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {}}));
+    assert!(good.is_ok(), "the next call reconnects: {good:?}");
+    drop(remote);
+    let seen = h.join().unwrap();
+    assert_eq!(seen.len(), 2, "one request per connection: {seen:?}");
+    assert_ne!(seen[0]["id"], seen[1]["id"]);
+}
+
 #[test]
 fn remote_connect_fails_fast() {
     // A port this test holds, bound but not listening, so connecting to it is refused. (A port
diff --git a/crates/mcp/src/tests_protocol.rs b/crates/mcp/src/tests_protocol.rs
index 40d656aa..1fe41c3e 100644
--- a/crates/mcp/src/tests_protocol.rs
+++ b/crates/mcp/src/tests_protocol.rs
@@ -97,6 +97,50 @@ fn prompt_argument_errors_are_invalid_params() {
     assert_eq!(err_code(&mut s, 3, "prompts/get", json!({})), -32602);
 }
 
+// ---------- envelope ----------
+
+/// Requests and notifications outside a `"jsonrpc": "2.0"` envelope are never dispatched (#1728).
+#[test]
+fn envelopes_without_version_20_are_not_dispatched() {
+    let mut s = server();
+    // A request in no envelope: invalid, and nothing ran.
+    let line = json!({"id": 1, "method": "ping", "params": {}}).to_string();
+    let v: Value = serde_json::from_str(&s.handle_line(&line).unwrap()).unwrap();
+    assert_eq!(v["error"]["code"], -32600, "{v}");
+    assert!(v["error"]["message"].as_str().unwrap_or_default().contains("2.0"), "{v}");
+    // A wrong version is the same.
+    let line = json!({"jsonrpc": "1.0", "id": 2, "method": "ping", "params": {}}).to_string();
+    let v: Value = serde_json::from_str(&s.handle_line(&line).unwrap()).unwrap();
+    assert_eq!(v["error"]["code"], -32600, "{v}");
+    // In a batch the unversioned message is an error entry, not a dispatch; the 2.0 one answers.
+    let batch = json!([
+        {"jsonrpc": "2.0", "id": 3, "method": "ping", "params": {}},
+        {"id": 4, "method": "ping", "params": {}},
+    ])
+    .to_string();
+    let v: Value = serde_json::from_str(&s.handle_line(&batch).unwrap()).unwrap();
+    assert_eq!(v.as_array().map(Vec::len), Some(2), "{v}");
+    assert!(v[0].get("result").is_some(), "{v}");
+    assert_eq!(v[1]["error"]["code"], -32600, "{v}");
+    // The 2.0 request still works.
+    assert_eq!(result(&mut s, 5, "ping", json!({})), json!({}));
+}
+
+#[test]
+fn notification_without_version_20_changes_nothing() {
+    let mut s = server();
+    assert!(!s.is_initialized());
+    // No envelope, no id: there is no address for a reply, so it is ignored — and it must not
+    // flip the server into its initialised state.
+    let line = json!({"method": "notifications/initialized", "params": {}}).to_string();
+    assert!(s.handle_line(&line).is_none());
+    assert!(!s.is_initialized(), "an unversioned notification changed state");
+    // The 2.0 notification still works.
+    let line = json!({"jsonrpc": "2.0", "method": "notifications/initialized", "params": {}}).to_string();
+    assert!(s.handle_line(&line).is_none());
+    assert!(s.is_initialized());
+}
+
 // ---------- completions ----------
 
 #[test]
diff --git a/docs/mcp.md b/docs/mcp.md
index bfadc69f..2dfe5232 100644
--- a/docs/mcp.md
+++ b/docs/mcp.md
@@ -105,7 +105,12 @@ given rather than guarding against other software on the computer.
 ## Protocol
 
 Newline-delimited JSON-RPC 2.0 on stdio. The revision is **`2025-06-18`**; `2025-03-26` and `2024-11-05` are
-accepted too. Clients requesting `2026-07-28` through `initialize`, or through a request's
+accepted too. Every request and notification must carry `"jsonrpc": "2.0"`: anything else is
+refused with `-32600` (requests) or ignored (notifications, which have no id to address a reply
+to) before it is dispatched. When a tool call reaches a running app, it is one control-channel
+request, and the app echoes its `id` in the reply: a reply with any other `id` is rejected, the
+connection dropped, and the next call starts a fresh one — the call reports uncertainty like any
+reply that never arrived whole. Clients requesting `2026-07-28` through `initialize`, or through a request's
 `_meta["io.modelcontextprotocol/protocolVersion"]`, receive `resultType: "complete"`,
 `ttlMs` and `cacheScope: "private"` on tool/resource/template/prompt lists and resource/prompt reads.
 List TTL is 600000 ms; live reads use zero. Older clients keep the original response shapes.