From b00d12b623891b942cff7fc7fcacedd3dbcd45ab Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 24 Oct 2026 13:01:33 +0700 Subject: [PATCH] #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}"); +}