Batch r2-2 review: data.fill refused before it copies or writes in a read-only live file; an oversized child is passed over in a marker-only page

crates/engine/src/datafill.rs

@@ -8,7 +8,7 @@ use serde_json::{Value, json};
 
 use crate::cmd::CommandSpec;
 use crate::params::{get, id, id_of, num_or, string};
-use crate::{Result, Session, bad, spec};
+use crate::{EngineError, Result, Session, bad, spec};
 
 const CMD: &str = "data.fill";
 const MAX_ROWS: usize = 500;
@@ -31,6 +31,10 @@ fn file_name(name: &str) -> String {
 }
 
 fn fill(s: &mut Session, p: &Value) -> Result<Value> {
+    // Refused before any copy or file is made: a rollback afterwards wouldn't take files back.
+    if let Some(why) = s.live_read_only(s.ds()?.uid) {
+        return Err(EngineError::Disabled(CMD.into(), why.into()));
+    }
     let src = id(p, "id").ok_or_else(|| bad(CMD, "`id` (the frame to repeat) required"))?;
     let rows = match get(p, "rows") {
         Some(Value::Array(a)) if a.iter().all(Value::is_object) && a.len() <= MAX_ROWS => a.clone(),

crates/engine/src/live_access_tests.rs

@@ -105,6 +105,13 @@ fn a_view_link_makes_the_file_read_only() {
     refused(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 5, "height": 5}), VIEW_ONLY);
     refused(&mut s, "node.set", json!({"ids": [card.0], "props": {"name": "Mine"}}), VIEW_ONLY);
     refused(&mut s, "edit.delete", json!({"ids": [card.0]}), VIEW_ONLY);
+    // Refused before it makes copies or writes files, which no rollback takes back.
+    let dir = std::env::temp_dir().join(format!("figcraft-view-fill-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    refused(&mut s, "data.fill", json!({"id": card.0, "rows": [{"name": "Copy"}], "dir": dir.to_string_lossy()}), VIEW_ONLY);
+    assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 0, "no file was written");
+    let _ = std::fs::remove_dir_all(&dir);
     refused(&mut s, "tool.set", json!({"tool": "text"}), VIEW_ONLY);
     assert!(matches!(s.begin_interaction("Move"), Err(EngineError::Disabled(..))));
     assert_eq!(*s.doc().unwrap(), before);

crates/mcp/src/tools.rs

@@ -754,7 +754,10 @@ fn get_node(b: &mut dyn Backend, a: &Map<String, Value>) -> Result<ToolResult, S
     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);
+    // A child too large to fit on its own is passed over (a page of just the marker), so paging
+    // moves on and every answer stays within maxBytes; `get_node` on the child reads it.
+    let after = offset + shown.max(1);
+    let next = (after < total).then_some(after);
     let used_wire = wire_size(&value);
     let mut out = with_marker(
         value,
@@ -792,9 +795,8 @@ fn children_window(value: &mut Value, offset: usize, budget: usize) -> bool {
     if fits(value, rest.len()) {
         return rest.len() != all.len();
     }
-    // The largest window that fits, but at least one child, so paging moves on past a child too
-    // large to fit (the marker then says so).
-    let (mut lo, mut hi) = (usize::from(!rest.is_empty()), rest.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 }

crates/mcp/tests/it/inspect_response_limit.rs

@@ -153,8 +153,8 @@ fn get_node(c: &mut Client, args: Value) -> (Value, usize) {
     (serde_json::from_str(text).expect("the block is JSON"), wire)
 }
 
-/// A child too large for the limit on its own is still shown, one per page, so paging moves on,
-/// and the marker says the answer was cut.
+/// 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();
@@ -165,8 +165,9 @@ fn children_too_large_to_fit_page_one_at_a_time() {
     let mut offset = Some(0u64);
     let mut pages = 0;
     while let Some(at) = offset {
-        let (page, _) = get_node(&mut c, json!({"id": frame, "depth": 1, "maxBytes": 1024, "offset": at}));
-        assert_eq!(page["children"].as_array().map(Vec::len), Some(1), "{}", page["truncated"]);
+        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}");