From b1f0eafbf7e226b29ecb211b2fe8cee1efce00e5 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 16:29:32 +0500 Subject: [PATCH 1/6] MCP: inspect_document answers within what a client can read MCP stdio is newline-delimited JSON, so one tool result is one message and has to fit the client's read buffer whole; there is no streaming of part of one. The TypeScript SDK's stdio transport defaults to 10 MiB and hosts rarely raise it, so a document whose tree went past that killed the server: the client stopped reading, the pipe broke, and every later call in the session failed. The tree is now reported at the deepest depth that fits, found by bisection, with a `truncated` object giving the depth asked for, the depth used, both sizes, the limit and a pointer to get_node for reading one subtree deeply. `maxBytes` lets a caller that knows its own buffer say so; the default is 8 MiB, leaving the envelope 2 MiB of the usual 10. The size is measured as the transport sends it -- pretty-printed and then escaped inside the text block, which adds about a tenth (342,136 bytes of tree became 379,954 on the wire in the fixture). A kilobyte is reserved for the marker itself, which is added after the measurement. A limit below even the shallowest tree still answers, at depth 0, with the marker saying so: there is nothing smaller to send, and an error would leave the caller with nothing at all. The tool description now says that each level of depth multiplies the response and points at get_node, which the report notes would have been enough on its own to avoid the crash. Fixes #912. --- crates/mcp/src/tools.rs | 87 +++++++++++++++++++++- crates/mcp/tests/inspect_response_limit.rs | 72 ++++++++++++++++++ 2 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 crates/mcp/tests/inspect_response_limit.rs diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 7714da5a..92773e03 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -173,9 +173,13 @@ pub fn tool_definitions() -> Vec { tool( "inspect_document", "Inspect document", - "The active document as JSON: name, path, dirty flag, current page, selection, the current page's layer tree (id, name, type, x/y relative to the parent frame (groups don't count), width, height, absoluteBounds in page coordinates, fills, strokes, effects, cornerRadius, layout, sizing, text properties, component links, overrides, bound variables), other pages summarised, styles, variable count and the undo history. Use it to verify every change.", + "The active document as JSON: name, path, dirty flag, current page, selection, the current page's layer tree (id, name, type, x/y relative to the parent frame (groups don't count), width, height, absoluteBounds in page coordinates, fills, strokes, effects, cornerRadius, layout, sizing, text properties, component links, overrides, bound variables), other pages summarised, styles, variable count and the undo history. Use it to verify every change. Each level of `depth` multiplies the response, and `allPages` multiplies it again: on a large document the tree is reported at a shallower depth than asked for, with a `truncated` object saying so. Use get_node to read one subtree deeply instead.", obj( - json!({"depth": int("Tree depth for the current page (default 4)"), "allPages": boolean("Expand every page, not only the current one")}), + json!({ + "depth": int("Tree depth for the current page (default 4)"), + "allPages": boolean("Expand every page, not only the current one"), + "maxBytes": int("Largest response to send, in bytes on the wire (default 8388608, which fits the usual 10 MiB client buffer)"), + }), &[], ), true, @@ -562,6 +566,27 @@ fn set_properties(b: &mut dyn Backend, a: &Map) -> Result Result<(Value, usize), String> { + let value = b.call("document.inspect", json!({"depth": depth, "allPages": all}))?; + let text = serde_json::to_string_pretty(&value).unwrap_or_default(); + let wire = serde_json::to_string(&text).map_or(text.len(), |s| s.len()); + Ok((value, wire)) +} + fn inspect_document(b: &mut dyn Backend, a: &Map) -> Result { let depth = n_arg(a, "depth")?.unwrap_or(4.0).clamp(0.0, 64.0) as u64; let all = match a.get("allPages") { @@ -569,7 +594,63 @@ fn inspect_document(b: &mut dyn Backend, a: &Map) -> Result *x, Some(_) => return Err("`allPages` must be a boolean".into()), }; - b.call("document.inspect", json!({"depth": depth, "allPages": all})).map(|v| ToolResult::json(&v)) + // A caller that knows its own buffer can say so; 1 KiB is below the shallowest tree, so the + // floor only keeps the arithmetic sane. + let max_bytes = match n_arg(a, "maxBytes")? { + Some(v) if v < 1024.0 => return Err("`maxBytes` must be at least 1024".into()), + Some(v) => v.min(usize::MAX as f64) as usize, + None => MAX_RESPONSE, + }; + + let (value, wire) = inspect_at(b, depth, all)?; + if wire <= max_bytes || depth == 0 { + return Ok(ToolResult::json(&value)); + } + + // The deepest tree that fits, by bisection: a deeper tree holds every node of a shallower one, + // so the size does not fall as the depth grows. Bisecting keeps this to about six more walks + // of the document rather than one per level. + let (mut lo, mut hi) = (0u64, depth - 1); + let mut best: Option<(u64, Value, usize)> = None; + while lo <= hi { + let mid = lo + (hi - lo) / 2; + let (v, w) = inspect_at(b, mid, all)?; + if w + MARKER_RESERVE <= max_bytes { + best = Some((mid, v, w)); + if mid == hi { + break; + } + lo = mid + 1; + } else if mid == 0 { + // Not even the shallowest tree fits. Send it anyway and say so: there is nothing + // smaller to send, and an error would leave the caller with nothing at all. + best = Some((0, v, w)); + break; + } else { + hi = mid - 1; + } + } + + let Some((used, mut value, used_wire)) = best else { + return Ok(ToolResult::json(&value)); + }; + if let Some(obj) = value.as_object_mut() { + obj.insert( + "truncated".into(), + json!({ + "requestedDepth": depth, + "depth": used, + "requestedDepthBytes": wire, + "bytes": used_wire, + "maxBytes": max_bytes, + "reason": format!( + "the tree at depth {depth} would be {wire} bytes on the wire, over the {max_bytes}-byte limit a client can read in one message; it is reported at depth {used} instead" + ), + "hint": "get_node({id, depth}) walks one subtree at a time, so a deep tree can be read in parts without a size limit", + }), + ); + } + Ok(ToolResult::json(&value)) } fn get_node(b: &mut dyn Backend, a: &Map) -> Result { diff --git a/crates/mcp/tests/inspect_response_limit.rs b/crates/mcp/tests/inspect_response_limit.rs new file mode 100644 index 00000000..0d9f9e6d --- /dev/null +++ b/crates/mcp/tests/inspect_response_limit.rs @@ -0,0 +1,72 @@ +//! `inspect_document` must not emit a message larger than a client can read in one piece: MCP +//! stdio is newline-delimited JSON, so one result is one message, and a client whose read buffer +//! it exceeds closes the pipe and kills the server for the rest of the session (#912). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod common; +use common::Client; +use serde_json::{Value, json}; + +/// A document whose tree is far larger than the limits used below. +fn busy() -> Client { + let mut c = Client::new(); + for i in 0..40 { + let frame = c.create(json!({"type": "frame", "x": i * 10, "y": 0, "width": 200, "height": 200})); + for j in 0..12 { + c.create(json!({"type": "rectangle", "parent": frame, "x": j * 5, "y": 5, "width": 20, "height": 20})); + } + } + c +} + +/// The response and what its text block takes on the wire, escaped as the transport sends it. +fn inspect(c: &mut Client, args: Value) -> (Value, usize) { + let r = c.call("inspect_document", args.clone()); + assert_eq!(r["isError"], false, "{args}: {}", r["content"][0]["text"]); + let text = r["content"][0]["text"].as_str().expect("a text block"); + let wire = serde_json::to_string(&text).unwrap().len(); + (serde_json::from_str(text).expect("the block is JSON"), wire) +} + +#[test] +fn a_tree_too_large_for_the_client_is_reported_shallower_and_says_so() { + let mut c = busy(); + let (full, full_wire) = inspect(&mut c, json!({"depth": 6, "allPages": true})); + assert!(full["truncated"].is_null(), "a tree within the limit is not truncated: {}", full["truncated"]); + assert!(full_wire > 30_000, "the fixture has to outgrow the limits below: {full_wire}"); + + // Asked for more than the caller can read: a shallower tree, inside the limit, and a marker. + let (cut, cut_wire) = inspect(&mut c, json!({"depth": 6, "allPages": true, "maxBytes": 30_000})); + assert!(cut_wire <= 30_000, "the response still exceeds maxBytes: {cut_wire}"); + let t = &cut["truncated"]; + assert_eq!(t["requestedDepth"], 6, "{t}"); + assert!(t["depth"].as_u64().unwrap() < 6, "the depth was reduced: {t}"); + assert_eq!(t["maxBytes"], 30_000, "{t}"); + assert_eq!(t["requestedDepthBytes"], full_wire, "the marker says what the full tree would have taken: {t}"); + // The marker points at the way to read the rest. + assert!(t["hint"].as_str().unwrap_or("").contains("get_node"), "{t}"); + // Still a usable document, not just a marker: the page and its top-level layers are there. + assert!(!cut["currentPage"].is_null(), "{cut}"); + assert!(cut["pages"][0]["children"].as_array().is_some_and(|c| !c.is_empty()), "{cut}"); + + // The deepest tree that fits is the one sent: a limit that admits the whole tree is not cut. + let (whole, _) = inspect(&mut c, json!({"depth": 6, "allPages": true, "maxBytes": full_wire})); + assert!(whole["truncated"].is_null(), "a limit at the exact size is not a truncation: {}", whole["truncated"]); +} + +#[test] +fn a_limit_below_the_shallowest_tree_still_answers() { + // Nothing smaller than depth 0 exists, so it is sent with the marker rather than an error: + // an error would leave the caller with nothing at all. + let mut c = busy(); + let (v, _) = inspect(&mut c, json!({"depth": 6, "allPages": true, "maxBytes": 1024})); + assert_eq!(v["truncated"]["depth"], 0, "{}", v["truncated"]); + assert_eq!(v["truncated"]["requestedDepth"], 6, "{}", v["truncated"]); +} + +#[test] +fn maxbytes_is_checked() { + let mut c = Client::new(); + let r = c.call("inspect_document", json!({"depth": 1, "maxBytes": 10})); + assert_eq!(r["isError"], true, "{r}"); + assert!(r["content"][0]["text"].as_str().unwrap_or("").contains("at least 1024"), "{r}"); +} From 3df5673627026deba90fcfbf089c0ac1fa0d49dc Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 20:56:08 +0500 Subject: [PATCH 2/6] MCP: maxBytes is the whole message, and depth 0 says when it is over Two things the limit got wrong, both reported by CodeRabbit on #915. maxBytes was compared with the escaped text block alone, but what goes on the wire is that block inside a content object, inside the JSON-RPC result, with the request id and the newline the transport writes: a block measured at exactly maxBytes left as a larger message than the caller said it could read. A kilobyte of the caller's figure is now kept for that wrapping. About ninety bytes of it is fixed and the rest is the request id, so the figure holds for any id up to roughly 900 bytes -- every id in practice, a number or a UUID. Sizing the real envelope here would mean handing the transport's request id to every tool, and the assumption is written down instead, in the schema and at the constant. A depth: 0 request over the limit returned early, so the one response that cannot be made any smaller was the one sent without the truncated marker -- while a bisection that walked down to depth 0 sent the same tree with it. Depth 0 now takes the same path as the bisection: the tree goes out, over the limit, with the marker saying so. Without the first fix the test reports a limit at the exact size of the tree as no truncation; without the second the depth-0 marker is absent (left: (None, None), right: (Some(0), Some(0))). --- crates/mcp/src/tools.rs | 68 ++++++++++++++-------- crates/mcp/tests/inspect_response_limit.rs | 29 ++++++++- 2 files changed, 70 insertions(+), 27 deletions(-) diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 92773e03..d04ea620 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -178,7 +178,9 @@ pub fn tool_definitions() -> Vec { json!({ "depth": int("Tree depth for the current page (default 4)"), "allPages": boolean("Expand every page, not only the current one"), - "maxBytes": int("Largest response to send, in bytes on the wire (default 8388608, which fits the usual 10 MiB client buffer)"), + "maxBytes": int( + "Largest response to send, in bytes of the whole JSON-RPC message (default 8388608, which fits the usual 10 MiB client buffer); 1024 of it is kept for the envelope and the request id" + ), }), &[], ), @@ -578,6 +580,14 @@ const MAX_RESPONSE: usize = 8 << 20; /// otherwise be able to push a response that only just fitted back over the limit. const MARKER_RESERVE: usize = 1 << 10; +/// Room kept for everything the text block is wrapped in: the content object and `isError`, the +/// JSON-RPC envelope (`jsonrpc`, `id`, `result`) and the newline the transport writes after it. +/// About ninety bytes of that is fixed and the rest is the request id, so `maxBytes` holds for +/// any id up to roughly 900 bytes -- which is every id in practice, a number or a UUID. An id +/// larger than that is the one case where the message can pass the caller's figure, and sizing +/// the real envelope here would mean handing the transport's request id to every tool. +const ENVELOPE_RESERVE: usize = 1 << 10; + /// The document at `depth`, and what its text block would take on the wire. The tree is sent as a /// text block, so it is measured pretty-printed *and* escaped -- escaping adds about a tenth. fn inspect_at(b: &mut dyn Backend, depth: u64, all: bool) -> Result<(Value, usize), String> { @@ -602,37 +612,45 @@ fn inspect_document(b: &mut dyn Backend, a: &Map) -> Result MAX_RESPONSE, }; + // What the tree itself may take: the caller's figure is for the whole message. + let budget = max_bytes.saturating_sub(ENVELOPE_RESERVE); + let (value, wire) = inspect_at(b, depth, all)?; - if wire <= max_bytes || depth == 0 { + if wire <= budget { return Ok(ToolResult::json(&value)); } - // The deepest tree that fits, by bisection: a deeper tree holds every node of a shallower one, - // so the size does not fall as the depth grows. Bisecting keeps this to about six more walks - // of the document rather than one per level. - let (mut lo, mut hi) = (0u64, depth - 1); - let mut best: Option<(u64, Value, usize)> = None; - while lo <= hi { - let mid = lo + (hi - lo) / 2; - let (v, w) = inspect_at(b, mid, all)?; - if w + MARKER_RESERVE <= max_bytes { - best = Some((mid, v, w)); - if mid == hi { + let (used, mut value, used_wire) = if depth == 0 { + // Depth 0 is the shallowest tree there is, so there is nothing smaller to send: it goes + // out over the limit with the marker saying so, the same as a bisection that reaches + // depth 0. An error would leave the caller with nothing at all. + (0, value, wire) + } else { + // The deepest tree that fits, by bisection: a deeper tree holds every node of a shallower + // one, so the size does not fall as the depth grows. Bisecting keeps this to about six + // more walks of the document rather than one per level. + let (mut lo, mut hi) = (0u64, depth - 1); + let mut best: Option<(u64, Value, usize)> = None; + while lo <= hi { + let mid = lo + (hi - lo) / 2; + let (v, w) = inspect_at(b, mid, all)?; + if w + MARKER_RESERVE <= budget { + best = Some((mid, v, w)); + if mid == hi { + break; + } + lo = mid + 1; + } else if mid == 0 { + best = Some((0, v, w)); break; + } else { + hi = mid - 1; } - lo = mid + 1; - } else if mid == 0 { - // Not even the shallowest tree fits. Send it anyway and say so: there is nothing - // smaller to send, and an error would leave the caller with nothing at all. - best = Some((0, v, w)); - break; - } else { - hi = mid - 1; } - } - - let Some((used, mut value, used_wire)) = best else { - return Ok(ToolResult::json(&value)); + match best { + Some(found) => found, + None => return Ok(ToolResult::json(&value)), + } }; if let Some(obj) = value.as_object_mut() { obj.insert( diff --git a/crates/mcp/tests/inspect_response_limit.rs b/crates/mcp/tests/inspect_response_limit.rs index 0d9f9e6d..5e36b5a9 100644 --- a/crates/mcp/tests/inspect_response_limit.rs +++ b/crates/mcp/tests/inspect_response_limit.rs @@ -6,6 +6,10 @@ mod common; use common::Client; use serde_json::{Value, json}; +/// The allowance `inspect_document` keeps for the content object, the JSON-RPC envelope and the +/// request id (`ENVELOPE_RESERVE`). +const ENVELOPE: usize = 1024; + /// A document whose tree is far larger than the limits used below. fn busy() -> Client { let mut c = Client::new(); @@ -49,8 +53,29 @@ fn a_tree_too_large_for_the_client_is_reported_shallower_and_says_so() { assert!(cut["pages"][0]["children"].as_array().is_some_and(|c| !c.is_empty()), "{cut}"); // The deepest tree that fits is the one sent: a limit that admits the whole tree is not cut. - let (whole, _) = inspect(&mut c, json!({"depth": 6, "allPages": true, "maxBytes": full_wire})); - assert!(whole["truncated"].is_null(), "a limit at the exact size is not a truncation: {}", whole["truncated"]); + // maxBytes is the whole message, so the tree fits under it with room for the envelope, not + // at it: a text block of exactly maxBytes would be sent inside a larger message than that. + let (whole, _) = inspect(&mut c, json!({"depth": 6, "allPages": true, "maxBytes": full_wire + ENVELOPE})); + assert!(whole["truncated"].is_null(), "a limit that leaves room for the envelope is not a truncation: {}", whole["truncated"]); + let (tight, _) = inspect(&mut c, json!({"depth": 6, "allPages": true, "maxBytes": full_wire})); + assert_eq!(tight["truncated"]["requestedDepth"], 6, "a limit at the exact size of the tree leaves none for the envelope: {tight}"); +} + +/// Every over-limit response carries the marker, however the depth was arrived at: a caller that +/// asked for depth 0 has no other way to learn that its limit was passed. +#[test] +fn a_depth_0_response_over_the_limit_says_so_too() { + let mut c = busy(); + let (deep, _) = inspect(&mut c, json!({"depth": 0, "allPages": true})); + assert!(deep["truncated"].is_null(), "within the default limit: {}", deep["truncated"]); + let (v, wire) = inspect(&mut c, json!({"depth": 0, "allPages": true, "maxBytes": 1024})); + let t = &v["truncated"]; + assert_eq!((t["requestedDepth"].as_u64(), t["depth"].as_u64()), (Some(0), Some(0)), "{t}"); + // `bytes` is the tree, measured before the marker was added to it, so the message that goes + // out is larger still -- there being nothing smaller to send. + assert_eq!(t["bytes"], t["requestedDepthBytes"], "{t}"); + assert!(wire > 1024, "the depth-0 tree is sent over the limit, with the marker saying so: {wire}"); + assert!(!v["pages"].is_null(), "the tree is still sent, there being nothing smaller: {v}"); } #[test] From 1aa1d61bd4aeb2aa93bfc04e2a3156227631bab4 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 21 Oct 2026 00:29:14 +0500 Subject: [PATCH 3/6] MCP: get_node keeps to the limit too, and windows a wide node inspect_document sends a shallower tree when the document's is too large and points at get_node for the rest, so get_node was the way out of that limit -- and it had none of its own. Measured against a document with a frame of 15,000 children, through figcraft-cli mcp: inspect_document depth 64, allPages 14.29 MiB (8.00 MiB cap not applied before #915) get_node depth 64 10.51 MiB Both are over the 10 MiB buffer the report is about, and for get_node no depth makes it smaller: the width is at depth 1, so depth 1 is already 9.12 MiB of text and depth 0 has no children at all. get_node now takes the same maxBytes, cuts the subtree by depth as the document's tree is cut, and then takes a window over the children when even depth 1 is too large. The marker says how many children were shown of how many, and the offset to ask for next; `offset` is a new argument for reading the rest. After this: get_node depth 64 8.00 MiB, children 11427 of 15000 from 0, next 11427 The hint in inspect_document's marker no longer promises that get_node has no size limit, since that was the hole: it now says a wide node answers with a window and an offset. The test pages through a windowed node and asserts every child is reported exactly once, in order, with every page inside the limit. Unbounded, the first page is 23,003 bytes against a 7,667-byte limit. --- crates/mcp/src/tools.rs | 185 +++++++++++++++++++-- crates/mcp/tests/inspect_response_limit.rs | 57 +++++++ 2 files changed, 224 insertions(+), 18 deletions(-) diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index d04ea620..19f2be35 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -189,8 +189,18 @@ pub fn tool_definitions() -> Vec { tool( "get_node", "Get layer", - "One layer as JSON (same fields as inspect_document) with its children to `depth`.", - obj(json!({"id": id_schema("Layer id"), "depth": int("Child depth (default 1)")}), &["id"]), + "One layer as JSON (same fields as inspect_document) with its children to `depth`. A subtree too large for one message is reported at a shallower depth, or with a window of the children, and a `truncated` object says so and gives the `offset` to ask for next.", + obj( + json!({ + "id": id_schema("Layer id"), + "depth": int("Child depth (default 1)"), + "offset": int("First child to report (default 0), for reading a node with more children than fit in one message"), + "maxBytes": int( + "Largest response to send, in bytes of the whole JSON-RPC message (default 8388608, which fits the usual 10 MiB client buffer); 1024 of it is kept for the envelope and the request id" + ), + }), + &["id"], + ), true, ), tool( @@ -588,12 +598,38 @@ const MARKER_RESERVE: usize = 1 << 10; /// the real envelope here would mean handing the transport's request id to every tool. const ENVELOPE_RESERVE: usize = 1 << 10; +/// What the caller says it can read, as the bytes its *tree* may take: the figure is for the whole +/// message, so the envelope's share comes off it here. +fn budget(a: &Map) -> Result<(usize, usize), String> { + // A caller that knows its own buffer can say so; 1 KiB is below the shallowest tree, so the + // floor only keeps the arithmetic sane. + let max_bytes = match n_arg(a, "maxBytes")? { + Some(v) if v < 1024.0 => return Err("`maxBytes` must be at least 1024".into()), + Some(v) => v.min(usize::MAX as f64) as usize, + None => MAX_RESPONSE, + }; + Ok((max_bytes, max_bytes.saturating_sub(ENVELOPE_RESERVE))) +} + +/// What a value's text block would take on the wire. It is sent as a text block, so it is measured +/// pretty-printed *and* escaped -- escaping adds about a tenth. +fn wire_size(value: &Value) -> usize { + let text = serde_json::to_string_pretty(value).unwrap_or_default(); + serde_json::to_string(&text).map_or(text.len(), |s| s.len()) +} + /// The document at `depth`, and what its text block would take on the wire. The tree is sent as a /// text block, so it is measured pretty-printed *and* escaped -- escaping adds about a tenth. fn inspect_at(b: &mut dyn Backend, depth: u64, all: bool) -> Result<(Value, usize), String> { let value = b.call("document.inspect", json!({"depth": depth, "allPages": all}))?; - let text = serde_json::to_string_pretty(&value).unwrap_or_default(); - let wire = serde_json::to_string(&text).map_or(text.len(), |s| s.len()); + let wire = wire_size(&value); + Ok((value, wire)) +} + +/// One layer's subtree at `depth`, and what its text block would take on the wire. +fn node_at(b: &mut dyn Backend, id: u64, depth: u64) -> Result<(Value, usize), String> { + let value = b.call("node.get", json!({"id": id, "depth": depth}))?; + let wire = wire_size(&value); Ok((value, wire)) } @@ -604,16 +640,7 @@ fn inspect_document(b: &mut dyn Backend, a: &Map) -> Result *x, Some(_) => return Err("`allPages` must be a boolean".into()), }; - // A caller that knows its own buffer can say so; 1 KiB is below the shallowest tree, so the - // floor only keeps the arithmetic sane. - let max_bytes = match n_arg(a, "maxBytes")? { - Some(v) if v < 1024.0 => return Err("`maxBytes` must be at least 1024".into()), - Some(v) => v.min(usize::MAX as f64) as usize, - None => MAX_RESPONSE, - }; - - // What the tree itself may take: the caller's figure is for the whole message. - let budget = max_bytes.saturating_sub(ENVELOPE_RESERVE); + let (max_bytes, budget) = budget(a)?; let (value, wire) = inspect_at(b, depth, all)?; if wire <= budget { @@ -664,21 +691,143 @@ fn inspect_document(b: &mut dyn Backend, a: &Map) -> Result) -> Result { let id = id_arg(a.get("id").ok_or("`id` is required")?)?; let depth = n_arg(a, "depth")?.unwrap_or(1.0).clamp(0.0, 64.0) as u64; - let v = b.call("node.get", json!({"id": id, "depth": depth}))?; - if v.is_null() { + let offset = n_arg(a, "offset")?.unwrap_or(0.0).max(0.0).min(u32::MAX as f64) as usize; + let (max_bytes, budget) = budget(a)?; + + let (value, wire) = node_at(b, id, depth)?; + if value.is_null() { return Err(format!("no such layer {id}")); } - Ok(ToolResult::json(&v)) + if wire <= budget && offset == 0 { + return Ok(ToolResult::json(&value)); + } + + // The deepest subtree that fits, by bisection, as the document's tree is cut. A node asked for + // at a depth of its own children only (1) cannot be made shallower without losing them all, + // so that is where the window takes over. + let (mut used, mut value, mut used_wire) = (depth, value, wire); + if depth > 1 && offset == 0 { + let (mut lo, mut hi) = (1u64, depth - 1); + while lo <= hi { + let mid = lo + (hi - lo) / 2; + let (v, w) = node_at(b, id, mid)?; + if w + MARKER_RESERVE <= budget { + (used, value, used_wire) = (mid, v, w); + if mid == hi { + break; + } + lo = mid + 1; + } else if mid == 1 { + (used, value, used_wire) = (1, v, w); + break; + } else { + hi = mid - 1; + } + } + if used_wire + MARKER_RESERVE <= budget { + return Ok(ToolResult::json(&with_marker(value, depth, used, wire, used_wire, max_bytes, None))); + } + } + + // Still over, or a window was asked for: the children from `offset` that fit. + let total = value.get("children").and_then(Value::as_array).map_or(0, Vec::len); + let window = children_window(&mut value, offset, budget.saturating_sub(MARKER_RESERVE)); + let shown = value.get("children").and_then(Value::as_array).map_or(0, Vec::len); + let next = (offset + shown < total).then_some(offset + shown); + let used_wire = wire_size(&value); + let mut out = with_marker( + value, + depth, + used, + wire, + used_wire, + max_bytes, + Some(json!({ + "from": offset, + "shown": shown, + "total": total, + "nextOffset": next, + })), + ); + if window && next.is_none() && offset == 0 && used == depth { + // Nothing was left out after all (an offset of zero that fitted whole). + out.as_object_mut().map(|o| o.remove("truncated")); + } + Ok(ToolResult::json(&out)) +} + +/// Keep the children from `offset` that fit in `budget`, and say whether any were left out. The +/// window is found by bisection: a node with thousands of children would otherwise be serialised +/// once per child. +fn children_window(value: &mut Value, offset: usize, budget: usize) -> bool { + let Some(all) = value.get("children").and_then(Value::as_array).cloned() else { return false }; + let rest: Vec = all.iter().skip(offset).cloned().collect(); + let fits = |value: &mut Value, n: usize| { + if let Some(obj) = value.as_object_mut() { + obj.insert("children".into(), Value::Array(rest.iter().take(n).cloned().collect())); + } + wire_size(value) <= budget + }; + if fits(value, rest.len()) { + return rest.len() != all.len(); + } + // The largest window that fits, between none and all of the rest. + let (mut lo, mut hi) = (0usize, rest.len()); + while lo < hi { + let mid = lo + (hi - lo).div_ceil(2); + if fits(value, mid) { lo = mid } else { hi = mid - 1 } + } + fits(value, lo); + true +} + +/// `value` with the `truncated` object saying how it was cut. `children`: the window, when the +/// children were cut as well as (or instead of) the depth. +fn with_marker(mut value: Value, asked: u64, used: u64, asked_wire: usize, used_wire: usize, max_bytes: usize, children: Option) -> Value { + if let Some(obj) = value.as_object_mut() { + let mut marker = json!({ + "requestedDepth": asked, + "depth": used, + "requestedDepthBytes": asked_wire, + "bytes": used_wire, + "maxBytes": max_bytes, + }); + match &children { + Some(window) => { + marker["children"] = window.clone(); + marker["reason"] = json!(format!( + "the subtree at depth {asked} would be {asked_wire} bytes on the wire, over the {max_bytes}-byte limit a client can read in one message; its children are reported from {} instead", + window.get("from").and_then(Value::as_u64).unwrap_or(0) + )); + marker["hint"] = json!("ask again with the `offset` in `children.nextOffset` for the rest, or get_node({id, depth}) on one child"); + } + None => { + marker["reason"] = json!(format!( + "the subtree at depth {asked} would be {asked_wire} bytes on the wire, over the {max_bytes}-byte limit a client can read in one message; it is reported at depth {used} instead" + )); + marker["hint"] = json!("get_node({id, depth}) on a child reads deeper, one subtree at a time"); + } + } + obj.insert("truncated".into(), marker); + } + value } fn select(b: &mut dyn Backend, a: &Map) -> Result { diff --git a/crates/mcp/tests/inspect_response_limit.rs b/crates/mcp/tests/inspect_response_limit.rs index 5e36b5a9..b6d8b92e 100644 --- a/crates/mcp/tests/inspect_response_limit.rs +++ b/crates/mcp/tests/inspect_response_limit.rs @@ -95,3 +95,60 @@ fn maxbytes_is_checked() { assert_eq!(r["isError"], true, "{r}"); assert!(r["content"][0]["text"].as_str().unwrap_or("").contains("at least 1024"), "{r}"); } + +/// `inspect_document` sends a shallower tree and points at `get_node` for the rest, so `get_node` +/// must keep to the same limit -- it was unbounded. Measured on `main`: a frame with 15,000 +/// children answers with a 10.5 MiB message at depth 1, over the 10 MiB buffer the report is +/// about, and no depth makes that smaller because the width is at depth 1. The children are +/// windowed instead, with the offset to ask for next. +#[test] +fn a_node_with_more_children_than_fit_is_windowed() { + let mut c = Client::new(); + let frame = c.create(json!({"type": "frame", "x": 0, "y": 0, "width": 400, "height": 400})); + for i in 0..40 { + c.create(json!({"type": "rectangle", "parent": frame, "x": i * 5, "y": 5, "width": 20, "height": 20})); + } + + // Whole, to measure against: every child, no marker. + let (whole, whole_wire) = get_node(&mut c, json!({"id": frame, "depth": 1})); + let total = whole["children"].as_array().expect("children").len(); + assert_eq!(total, 40, "{whole}"); + assert!(whole["truncated"].is_null(), "within the default limit: {}", whole["truncated"]); + + // A limit that holds only some of them: a window, and the marker says how many and what next. + let limit = whole_wire / 3; + assert!(limit >= 1024, "the fixture has to outgrow the floor: {whole_wire}"); + let (cut, cut_wire) = get_node(&mut c, json!({"id": frame, "depth": 1, "maxBytes": limit})); + assert!(cut_wire <= limit, "the response still exceeds maxBytes: {cut_wire} > {limit}"); + let window = &cut["truncated"]["children"]; + let shown = cut["children"].as_array().expect("children").len(); + assert_eq!(window["from"], json!(0), "{window}"); + assert_eq!(window["shown"], json!(shown), "{window}"); + assert_eq!(window["total"], json!(total), "{window}"); + assert!(shown > 0 && shown < total, "some children, not all: {shown} of {total}"); + assert_eq!(window["nextOffset"], json!(shown), "the offset to ask for next: {window}"); + // The layer itself is still there, not just a marker. + assert_eq!(cut["id"], whole["id"]); + + // Paging through with that offset reaches every child exactly once, in order. + let mut seen: Vec = Vec::new(); + let mut offset = Some(0u64); + while let Some(at) = offset { + let (page, wire) = get_node(&mut c, json!({"id": frame, "depth": 1, "maxBytes": limit, "offset": at})); + assert!(wire <= limit, "page at {at} exceeds maxBytes: {wire}"); + seen.extend(page["children"].as_array().cloned().unwrap_or_default()); + offset = page["truncated"]["children"]["nextOffset"].as_u64(); + assert!(seen.len() <= total, "a page repeated a child: {} of {total}", seen.len()); + } + assert_eq!(seen.len(), total, "every child was reported once"); + assert_eq!(seen, whole["children"].as_array().cloned().unwrap_or_default(), "in the same order"); +} + +/// The response and what its text block takes on the wire, for one `get_node` call. +fn get_node(c: &mut Client, args: Value) -> (Value, usize) { + let r = c.call("get_node", args.clone()); + assert_eq!(r["isError"], false, "{args}: {}", r["content"][0]["text"]); + let text = r["content"][0]["text"].as_str().expect("a text block"); + let wire = serde_json::to_string(&text).unwrap().len() + ENVELOPE; + (serde_json::from_str(text).expect("the block is JSON"), wire) +} From 91af3782e05fd9ee4b7f606939600f890c83d895 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 24 Oct 2026 12:57:58 +0700 Subject: [PATCH 4/6] #915: the inspect response-limit tests join the MCP crate's one test binary (tests/it, as on main since #1208) --- crates/mcp/tests/{ => it}/inspect_response_limit.rs | 4 ++-- crates/mcp/tests/it/main.rs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) rename crates/mcp/tests/{ => it}/inspect_response_limit.rs (99%) diff --git a/crates/mcp/tests/inspect_response_limit.rs b/crates/mcp/tests/it/inspect_response_limit.rs similarity index 99% rename from crates/mcp/tests/inspect_response_limit.rs rename to crates/mcp/tests/it/inspect_response_limit.rs index b6d8b92e..8e52514b 100644 --- a/crates/mcp/tests/inspect_response_limit.rs +++ b/crates/mcp/tests/it/inspect_response_limit.rs @@ -1,8 +1,8 @@ //! `inspect_document` must not emit a message larger than a client can read in one piece: MCP //! stdio is newline-delimited JSON, so one result is one message, and a client whose read buffer //! it exceeds closes the pipe and kills the server for the rest of the session (#912). -#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] -mod common; +use crate::common; + use common::Client; use serde_json::{Value, json}; diff --git a/crates/mcp/tests/it/main.rs b/crates/mcp/tests/it/main.rs index b5e3d89d..b16fc98e 100644 --- a/crates/mcp/tests/it/main.rs +++ b/crates/mcp/tests/it/main.rs @@ -13,6 +13,8 @@ mod export_hidden_roots; #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod garbage; #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod inspect_response_limit; +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod protocol; #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod screenshot_pixel_limit; From b6f5c29d190a530f362ba66a9ba8f7dd2dc7d7ff Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 24 Oct 2026 12:58:10 +0700 Subject: [PATCH 5/6] #915: MCP get_node pages past a child too large for the limit and keeps the truncation marker --- crates/mcp/src/tools.rs | 7 +++-- crates/mcp/tests/it/inspect_response_limit.rs | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 19f2be35..4feebbb8 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -750,7 +750,10 @@ fn get_node(b: &mut dyn Backend, a: &Map) -> Result) -> Result (Value, usize) { let wire = serde_json::to_string(&text).unwrap().len() + ENVELOPE; (serde_json::from_str(text).expect("the block is JSON"), wire) } + +/// A child too large for the limit on its own is passed over, one per page of just the marker, +/// so paging moves on and every page stays within the limit. +#[test] +fn children_too_large_to_fit_page_one_at_a_time() { + let mut c = Client::new(); + let frame = c.create(json!({"type": "frame", "x": 0, "y": 0, "width": 400, "height": 400})); + for i in 0..3 { + c.create(json!({"type": "rectangle", "parent": frame, "x": i * 5, "y": 5, "width": 20, "height": 20, "name": "n".repeat(3000)})); + } + let mut offset = Some(0u64); + let mut pages = 0; + while let Some(at) = offset { + let (page, wire) = get_node(&mut c, json!({"id": frame, "depth": 1, "maxBytes": 2500, "offset": at})); + assert!(wire <= 2500, "page at {at} exceeds maxBytes: {wire}"); + assert_eq!(page["children"].as_array().map_or(0, Vec::len), 0, "{}", page["truncated"]); + assert!(!page["truncated"].is_null(), "the marker stays: page at {at}"); + offset = page["truncated"]["children"]["nextOffset"].as_u64(); + assert!(offset.is_none_or(|o| o > at), "paging moves on from {at}"); + pages += 1; + assert!(pages <= 3); + } + assert_eq!(pages, 3); + + // A sole child passed over still says the answer was cut. + let single = c.create(json!({"type": "frame", "x": 500, "y": 0, "width": 40, "height": 40})); + c.create(json!({"type": "rectangle", "parent": single, "x": 0, "y": 0, "width": 20, "height": 20, "name": "n".repeat(3000)})); + let (page, _) = get_node(&mut c, json!({"id": single, "depth": 1, "maxBytes": 2500})); + assert_eq!(page["children"].as_array().map_or(0, Vec::len), 0); + assert_eq!(page["truncated"]["children"]["total"], json!(1), "{}", page["truncated"]); +} From b00d12b623891b942cff7fc7fcacedd3dbcd45ab Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 24 Oct 2026 13:01:33 +0700 Subject: [PATCH 6/6] #915: get_node refuses a layer too large for maxBytes on its own; a window only an offset made says it is paging --- crates/mcp/src/tools.rs | 43 +++++++++++++++---- crates/mcp/tests/it/inspect_response_limit.rs | 33 ++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 4feebbb8..32ad8832 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -742,7 +742,7 @@ fn get_node(b: &mut dyn Backend, a: &Map) -> Result) -> Result budget { + // Even with every child left out, the layer's own fields (a very long name, say) pass the + // limit: an answer would be larger than the caller can read, so there is none. + return Err(format!( + "layer {id} takes {used_wire} bytes on the wire without its children, more than fit in maxBytes {max_bytes}; ask again with a larger `maxBytes`" + )); + } + // Children left out after the window were cut by size; a window that only `offset` moved, of a + // subtree within the limit, is paging. + let over = wire > budget || next.is_some(); let mut out = with_marker( value, depth, @@ -762,6 +772,7 @@ fn get_node(b: &mut dyn Backend, a: &Map) -> Result) -> Result bool { true } -/// `value` with the `truncated` object saying how it was cut. `children`: the window, when the +/// `value` with the `truncated` object saying how it was cut. `over`: the subtree asked for passed +/// the limit (else the window is only the `offset` asked for). `children`: the window, when the /// children were cut as well as (or instead of) the depth. -fn with_marker(mut value: Value, asked: u64, used: u64, asked_wire: usize, used_wire: usize, max_bytes: usize, children: Option) -> Value { +fn with_marker( + mut value: Value, + asked: u64, + used: u64, + asked_wire: usize, + used_wire: usize, + max_bytes: usize, + over: bool, + children: Option, +) -> Value { if let Some(obj) = value.as_object_mut() { let mut marker = json!({ "requestedDepth": asked, @@ -815,10 +836,16 @@ fn with_marker(mut value: Value, asked: u64, used: u64, asked_wire: usize, used_ match &children { Some(window) => { marker["children"] = window.clone(); - marker["reason"] = json!(format!( - "the subtree at depth {asked} would be {asked_wire} bytes on the wire, over the {max_bytes}-byte limit a client can read in one message; its children are reported from {} instead", - window.get("from").and_then(Value::as_u64).unwrap_or(0) - )); + let from = window.get("from").and_then(Value::as_u64).unwrap_or(0); + marker["reason"] = json!(if over { + format!( + "the subtree at depth {asked} would be {asked_wire} bytes on the wire, over the {max_bytes}-byte limit a client can read in one message; its children are reported from {from} instead" + ) + } else { + format!( + "its children are reported from {from}, the `offset` asked for; the subtree at depth {asked} ({asked_wire} bytes) is within the {max_bytes}-byte limit" + ) + }); marker["hint"] = json!("ask again with the `offset` in `children.nextOffset` for the rest, or get_node({id, depth}) on one child"); } None => { diff --git a/crates/mcp/tests/it/inspect_response_limit.rs b/crates/mcp/tests/it/inspect_response_limit.rs index 764e6a50..347aa96e 100644 --- a/crates/mcp/tests/it/inspect_response_limit.rs +++ b/crates/mcp/tests/it/inspect_response_limit.rs @@ -127,6 +127,7 @@ fn a_node_with_more_children_than_fit_is_windowed() { assert_eq!(window["total"], json!(total), "{window}"); assert!(shown > 0 && shown < total, "some children, not all: {shown} of {total}"); assert_eq!(window["nextOffset"], json!(shown), "the offset to ask for next: {window}"); + assert!(cut["truncated"]["reason"].as_str().unwrap_or("").contains("over the"), "cut by size: {}", cut["truncated"]); // The layer itself is still there, not just a marker. assert_eq!(cut["id"], whole["id"]); @@ -183,3 +184,35 @@ fn children_too_large_to_fit_page_one_at_a_time() { assert_eq!(page["children"].as_array().map_or(0, Vec::len), 0); assert_eq!(page["truncated"]["children"]["total"], json!(1), "{}", page["truncated"]); } + +/// A layer whose own fields pass the limit with every child left out is refused: an answer would +/// be over maxBytes, and the marker would only make it larger. +#[test] +fn a_layer_too_large_on_its_own_is_refused() { + let mut c = Client::new(); + let rect = c.create(json!({"type": "rectangle", "x": 0, "y": 0, "width": 20, "height": 20, "name": "n".repeat(3000)})); + let r = c.call("get_node", json!({"id": rect, "maxBytes": 2500})); + assert_eq!(r["isError"], true, "{r}"); + assert!(r["content"][0]["text"].as_str().unwrap_or("").contains("maxBytes"), "{r}"); + // A layer of ordinary size answers within the same limit. + let small = c.create(json!({"type": "rectangle", "x": 0, "y": 0, "width": 20, "height": 20})); + let (v, wire) = get_node(&mut c, json!({"id": small, "maxBytes": 2500})); + assert!(wire <= 2500 && v["truncated"].is_null(), "{wire}: {v}"); +} + +/// A window that only an `offset` made, of a subtree within the limit, is paging: the marker +/// doesn't say the limit was passed. +#[test] +fn an_offset_alone_pages_without_claiming_the_limit() { + let mut c = Client::new(); + let frame = c.create(json!({"type": "frame", "x": 0, "y": 0, "width": 400, "height": 400})); + for i in 0..3 { + c.create(json!({"type": "rectangle", "parent": frame, "x": i * 5, "y": 5, "width": 20, "height": 20})); + } + let (page, _) = get_node(&mut c, json!({"id": frame, "depth": 1, "offset": 1})); + let t = &page["truncated"]; + assert_eq!(page["children"].as_array().map_or(0, Vec::len), 2, "{t}"); + assert!(t["children"]["nextOffset"].is_null(), "{t}"); + let reason = t["reason"].as_str().unwrap_or(""); + assert!(reason.contains("offset") && !reason.contains("over the"), "{reason}"); +}