diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index f2021cc6..1c2ab141 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -1,7 +1,7 @@ //! Where MCP tool calls end up: a control-channel method call, answered either by an in-process //! engine session ([`crate::Headless`]) or by a running FigCraft app ([`Attached`]). -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::net::{TcpStream, ToSocketAddrs}; use std::time::Duration; @@ -76,17 +76,23 @@ pub fn decode_data(v: &Value) -> Result, String> { base64::engine::general_purpose::STANDARD.decode(d.trim()).map_err(|e| format!("bad base64 from the app: {e}")) } +/// The longest reply line we read from the app (512 MiB). The app's window captures alone reach +/// 256 MiB as base64 (192 MiB PNGs); anything longer is refused without reading the rest. +const MAX_REPLY: usize = 512 * 1024 * 1024; + /// A running FigCraft app, reached through its loopback control port (`figcraft --control PORT`). pub struct Attached { addr: String, conn: Option<(BufReader, TcpStream)>, next_id: u64, + /// [`MAX_REPLY`]; smaller in tests. + max_reply: usize, } impl Attached { /// Connect to `addr` (e.g. `127.0.0.1:7979`), failing fast when nothing is listening. pub fn connect(addr: &str) -> std::io::Result { - let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 }; + let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, max_reply: MAX_REPLY }; r.reconnect()?; Ok(r) } @@ -133,17 +139,25 @@ impl Attached { } fn roundtrip(&mut self, line: &str) -> std::io::Result { + let max_reply = self.max_reply; let Some((reader, writer)) = self.conn.as_mut() else { return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "not connected to the app")); }; writer.write_all(line.as_bytes())?; writer.write_all(b"\n")?; writer.flush()?; - let mut reply = String::new(); - if reader.read_line(&mut reply)? == 0 { + let mut reply = Vec::new(); + if reader.take((max_reply + 1) as u64).read_until(b'\n', &mut reply)? == 0 { return Err(std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "control channel closed")); } - Ok(reply) + if reply.last() == Some(&b'\n') { + reply.pop(); + } + if reply.len() > max_reply { + let mib = max_reply / (1024 * 1024); + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, format!("reply larger than {mib} MiB"))); + } + String::from_utf8(reply).map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) } } @@ -181,3 +195,7 @@ impl Backend for Attached { format!("attached to the FigCraft app at {}", self.addr) } } + +#[cfg(test)] +#[path = "backend_tests.rs"] +mod tests; diff --git a/crates/mcp/src/backend_tests.rs b/crates/mcp/src/backend_tests.rs new file mode 100644 index 00000000..ec4fc473 --- /dev/null +++ b/crates/mcp/src/backend_tests.rs @@ -0,0 +1,77 @@ +//! The attached backend reads the app's replies with a bound (#1113). + +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; +use std::sync::mpsc; +use std::time::Duration; + +use serde_json::json; + +use super::{Attached, Backend}; + +/// A fake app that takes one connection per reply, reads one request on it and answers with the +/// reply. It keeps the connections open until [`finish`] (so an unterminated reply never reaches +/// EOF), then returns how many requests it read. +fn fake_app(replies: Vec>) -> (u16, mpsc::Sender<()>, std::thread::JoinHandle) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let (done, wait) = mpsc::channel::<()>(); + let app = std::thread::spawn(move || { + let mut open = Vec::new(); + for reply in &replies { + let (stream, _) = listener.accept().unwrap(); + let mut out = stream.try_clone().unwrap(); + let mut line = String::new(); + BufReader::new(stream).read_line(&mut line).unwrap(); + out.write_all(reply).unwrap(); + out.flush().unwrap(); + open.push(out); + } + wait.recv_timeout(Duration::from_secs(30)).ok(); + open.len() + }); + (port, done, app) +} + +/// Let the fake app close its connections, and check it read `requests` requests. +fn finish(done: mpsc::Sender<()>, app: std::thread::JoinHandle, requests: usize) { + drop(done); + assert_eq!(app.join().unwrap(), requests); +} + +fn ok_reply(text: &str) -> Vec { + format!("{}\n", json!({"id": 1, "ok": true, "result": text})).into_bytes() +} + +#[test] +fn an_oversized_reply_is_refused_and_the_next_call_reconnects() { + let (port, done, fake) = fake_app(vec![vec![b'x'; 200], ok_reply("fine")]); + let mut app = Attached::port(port).unwrap(); + app.max_reply = 64; + let err = app.call("engine.commands", json!({})).unwrap_err(); + assert!(err.contains("reply larger than"), "{err}"); + assert!(err.contains("may have completed"), "{err}"); + assert!(app.conn.is_none()); + assert_eq!(app.call("engine.commands", json!({})).unwrap(), json!("fine")); + finish(done, fake, 2); +} + +#[test] +fn a_reply_at_the_limit_is_read() { + let reply = ok_reply("at the limit"); + let (port, done, fake) = fake_app(vec![reply.clone()]); + let mut app = Attached::port(port).unwrap(); + app.max_reply = reply.len() - 1; // the newline doesn't count + assert_eq!(app.call("engine.commands", json!({})).unwrap(), json!("at the limit")); + finish(done, fake, 1); +} + +#[test] +fn a_reply_that_is_not_utf8_is_an_error() { + let (port, done, fake) = fake_app(vec![b"{\"ok\": true, \"result\": \"\xff\"}\n".to_vec()]); + let mut app = Attached::port(port).unwrap(); + let err = app.call("engine.commands", json!({})).unwrap_err(); + assert!(err.contains("failed"), "{err}"); + assert!(app.conn.is_none()); + finish(done, fake, 1); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 448a9979..2f211950 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -29,6 +29,8 @@ active connections**; additional connections are closed immediately. Socket read have a **60 s inactivity timeout**, so idle clients and clients that stop reading release their worker. This bounds blocked socket operations, not the total duration of a frame arriving slowly. Clients can reconnect after an idle timeout (the attached MCP backend already does). +Replies have no size limit on the app's side. The attached MCP backend reads them up to **512 MiB +before the newline** and drops the connection on a longer one (see [MCP server](mcp.md)). Both the socket queue and the UI queue hold at most **8 requests** each, including requests waiting for another window's frame. A full socket queue replies with `"control request queue is full"` diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..cbf989cf 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -23,6 +23,10 @@ Attached calls are never automatically replayed after a transport failure: the a applied the request before its reply was lost. The error reports that the operation may have completed; inspect the document before retrying an edit. The next explicit call reconnects. If that connection cannot be established, the error says the request was not sent. +A reply from the app is read up to **512 MiB before the newline** (the largest window capture is +256 MiB as base64). The backend stops reading a longer reply at the first byte past the limit and +treats it like a lost reply: the connection is closed, the operation may have completed, and the +next call reconnects. For larger exports, give `export` a `path` or lower the scale. A connection the app closed while it sat idle is noticed before the next request is written and replaced then, so a pause between calls is not an error. diff --git a/docs/roadmap.md b/docs/roadmap.md index 14d734ba..d9df3e02 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # FigCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (the attached MCP server reads the app's replies with a 512 MiB bound (#1113); Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) Forward-looking plan. Where FigCraft stands today: [`ROADMAP.md`](../ROADMAP.md) and [`target-app-parity.md`](target-app-parity.md); the full ranked list: [`gaps.md`](gaps.md). Hours @@ -75,6 +75,7 @@ blockers. | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | Bounded attached MCP replies (M8, #1113): `figcraft-cli mcp --attach` read each reply from the app's control port with no size limit, so a process listening on that port could make it buffer until memory ran out. Replies are now read up to 512 MiB (an app window capture is at most 256 MiB as base64); reading stops at the first byte past that, the connection is closed and the call is reported like any other lost reply, and the next call reconnects. Regression tests in the MCP backend tests. Numbers unchanged | | 2026-10-26 | minor | Dragging a selected unfilled shape (M1, #1370): a press inside a selected stroke-only shape over a filled layer stacked below it moved that layer; the drag-from-inside fallback of #910 now treats any layer painted below the selection like the frame or canvas behind it, so the selection drags, while a layer painted above it still takes the press. Regression test in the selection behaviour suite. Numbers unchanged | | 2026-10-26 | minor | Text truncation (M4, #1375, #1376, #1377): the last line shown takes as much of the next word as fits before the ellipsis ("Lorem consec…", not "Lorem…"); an explicit height, `transform.resize`, a resize handle or fixed sizing on a truncated text keeps it truncating instead of turning it fixed size; Max lines on auto-height text hugs up to that many lines and ends in an ellipsis (`text.maxLines`, `node.set {maxLines}`, Type settings, `.fig` export and import), and `node.get` / `get_node` report `maxLines`. Regression tests in the text and engine behaviour suites. Numbers unchanged | | 2026-10-24 | minor | Installed weights of an embedded font family (M7, #1246): release builds embed some families with only a few styles (Poppins Regular, Italic and Bold), and the first font scan dropped every installed face of such a family, so an installed Poppins SemiBold or Medium was never offered (a rescan did offer them). The first scan now keeps the embedded faces, which still win for the styles they cover, and offers the installed styles they lack beside them. Regression test in the font database tests. Numbers unchanged |