From 25e4986998f65f119b7957f7d4cdcac883ca460e Mon Sep 17 00:00:00 2001
From: vangel76 <vangel1976@googlemail.com>
Date: Fri, 16 Oct 2026 22:27:46 +0200
Subject: [PATCH] MCP: insert_table tool; insert.spreadsheet takes values,
 header and style (#2242)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---
 ROADMAP.md                     |  1 +
 crates/engine/src/cmd/tools.rs | 71 ++++++++++++++++++++++++++++++++--
 crates/mcp/src/tests.rs        | 41 ++++++++++++++++++++
 crates/mcp/src/tools.rs        | 28 +++++++++++++-
 docs/mcp.md                    |  3 +-
 5 files changed, 138 insertions(+), 6 deletions(-)

diff --git a/ROADMAP.md b/ROADMAP.md
index 40407d18b6..91e8e6b177 100644
--- a/ROADMAP.md
+++ b/ROADMAP.md
@@ -135,6 +135,7 @@ Ranked, to beta (detail and remaining milestones in [docs/roadmap.md](docs/roadm
 ## Progress log
 
 | Date | What landed |
+| 2026-10-16 | MCP `insert_table` tool (#2242): a filled table in one call from `values`, with `header` (repeating first row) and `style`; `insert.spreadsheet` takes the same `values`/`header`/`style` and rejects ragged, oversized or non-array input |
 | 2026-10-14 | DOCX row table border exceptions render and survive saving. Explicit cell borders win; a row exception does not change other rows. |
 | 2026-10-14 | Tab stops edited on the ruler: the corner selector picks Left/Center/Right/Decimal/Bar, a click sets a stop (Alt skips the snap), dragging moves it with a guide line and position tooltip, dragging off the ruler clears it; markers draw per kind and mirror for RTL; one `para.tabs` call and one undo step per gesture |
 | 2026-10-12 | Fields keep `w:dirty` through DOCX saves (#877): a begin marker flagged by a generator that couldn't compute a result (pandoc-style TOC placeholders) keeps the flag on open/save — as a buffered field or a range field — so Word still refreshes it on open; computing a result (Update Field, Update Table) clears it, like Word's F9 |
diff --git a/crates/engine/src/cmd/tools.rs b/crates/engine/src/cmd/tools.rs
index 1243ba4f24..8035e5cb9b 100644
--- a/crates/engine/src/cmd/tools.rs
+++ b/crates/engine/src/cmd/tools.rs
@@ -510,10 +510,10 @@ pub fn specs() -> Vec<CommandSpec> {
         })
         .params(r#"{"path": string}"#),
         CommandSpec::new("insert.spreadsheet", "Spreadsheet Table", "Insert › Tables", |s, v| {
-            let csv = p::str(v, "csv").unwrap_or("A,B,C\n1,2,3\n4,5,6");
-            let rows = super::mailings::parse_csv(csv);
+            let rows = spreadsheet_rows(v)?;
             let cols = rows.iter().map(Vec::len).max().unwrap_or(1).clamp(1, 63);
-            s.run("insert.table", &json!({"rows": rows.len().max(1), "cols": cols, "style": "GridTable1Light"}))?;
+            let style = p::str(v, "style").unwrap_or("GridTable1Light");
+            s.run("insert.table", &json!({"rows": rows.len().max(1), "cols": cols, "style": style}))?;
             let Some((tp, _, _)) = s.sel.focus.path.cell() else { return sel_result(s) };
             let t = s.doc.table_mut(s.sel.focus.story, &tp)?;
             for (r, row) in rows.iter().enumerate() {
@@ -523,9 +523,14 @@ pub fn specs() -> Vec<CommandSpec> {
                     }
                 }
             }
+            if p::bool(v, "header").unwrap_or(false)
+                && let Some(first) = t.rows.first_mut()
+            {
+                first.props.header = true;
+            }
             sel_result(s)
         })
-        .params(r#"{"csv"?: string}"#),
+        .params(r#"{"csv"?: string, "values"?: [[cell, …], …] (rows of equal length, at most 1000 rows × 63 columns; wins over csv), "header"?: bool (first row repeats on each page), "style"?: table style name (default GridTable1Light)}"#),
         CommandSpec::new("layout.linkToPrevious", "Link to Previous", "Header & Footer › Navigation", |s, v| {
             let header = p::bool(v, "footer").map(|f| !f).unwrap_or(true);
             let block = s.sel.focus.path.0.first().copied().unwrap_or(0) as usize;
@@ -1764,6 +1769,39 @@ fn customize_ribbon(s: &mut Session, v: &Value) -> CmdResult {
     Ok(Value::Object(out))
 }
 
+/// The rows of an `insert.spreadsheet` call: `values` (a rectangular array of cells) or `csv`.
+fn spreadsheet_rows(v: &Value) -> Result<Vec<Vec<String>>, CmdError> {
+    let Some(values) = v.get("values").filter(|x| !x.is_null()) else {
+        return Ok(super::mailings::parse_csv(p::str(v, "csv").unwrap_or("A,B,C\n1,2,3\n4,5,6")));
+    };
+    let list = values.as_array().ok_or_else(|| CmdError::Params("`values` must be an array of rows".into()))?;
+    if list.is_empty() || list.len() > 1000 {
+        return Err(CmdError::Params("`values` needs 1 to 1000 rows".into()));
+    }
+    let mut rows: Vec<Vec<String>> = Vec::with_capacity(list.len());
+    for (r, row) in list.iter().enumerate() {
+        let cells = row.as_array().ok_or_else(|| CmdError::Params(format!("`values` row {} is not an array", r + 1)))?;
+        if cells.is_empty() || cells.len() > 63 {
+            return Err(CmdError::Params(format!("`values` row {} needs 1 to 63 cells", r + 1)));
+        }
+        if rows.first().is_some_and(|f| f.len() != cells.len()) {
+            return Err(CmdError::Params(format!("`values` row {} has {} cells, row 1 has {}", r + 1, cells.len(), rows.first().map_or(0, Vec::len))));
+        }
+        let mut out = Vec::with_capacity(cells.len());
+        for cell in cells {
+            out.push(match cell {
+                Value::String(x) => x.clone(),
+                Value::Number(x) => x.to_string(),
+                Value::Bool(x) => x.to_string(),
+                Value::Null => String::new(),
+                _ => return Err(CmdError::Params(format!("`values` row {} holds a cell that is not text, a number or a boolean", r + 1))),
+            });
+        }
+        rows.push(out);
+    }
+    Ok(rows)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1866,4 +1904,29 @@ mod tests {
         assert_eq!(s.doc.styles.default_chr.lang.as_deref(), Some("de-DE"));
         assert!(s.run("review.language", &json!({"lang": "<script>"})).is_err());
     }
+
+    /// #2242: `insert.spreadsheet` fills a table from `values`, with a repeating header and a style.
+    #[test]
+    fn spreadsheet_takes_values_header_and_style() {
+        let mut s = Session::new(Document::from_text(""));
+        s.run("insert.spreadsheet", &json!({"values": [["Month", "Cases"], ["Jan", 12], ["Feb", null]], "header": true, "style": "TableGrid"})).unwrap();
+        let Some(Block::Table(t)) = s.doc.body.iter().map(|b| &**b).find(|b| matches!(b, Block::Table(_))) else { panic!("no table") };
+        assert_eq!((t.rows.len(), t.rows[0].cells.len()), (3, 2));
+        assert!(t.rows[0].props.header && !t.rows[1].props.header);
+        assert_eq!(t.rows[1].cells[1].blocks[0].as_para().unwrap().text, "12");
+        assert_eq!(t.props.style.as_deref(), Some("TableGrid"));
+    }
+
+    /// #2242: malformed `values` is an error, never a panic, and leaves the document alone.
+    #[test]
+    fn spreadsheet_rejects_hostile_values() {
+        let wide: Vec<Value> = (0..64).map(|i| json!(i)).collect();
+        let many: Vec<Value> = (0..1001).map(|_| json!(["x"])).collect();
+        for bad in [json!("x"), json!(5), json!([]), json!(["a"]), json!([["a", "b"], ["c"]]), json!([wide]), json!(many), json!([[{"a": 1}]]), json!([[[1]]]), json!([[]])] {
+            let mut s = Session::new(Document::from_text("keep"));
+            assert!(s.run("insert.spreadsheet", &json!({"values": bad})).is_err(), "{bad}");
+            assert_eq!(s.doc.plain_text(StoryRef::Body), "keep");
+            assert!(!s.doc.body.iter().any(|b| matches!(&**b, Block::Table(_))));
+        }
+    }
 }
diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs
index 781f24d02b..15cb2e02eb 100644
--- a/crates/mcp/src/tests.rs
+++ b/crates/mcp/src/tests.rs
@@ -152,3 +152,44 @@ fn type_text_stops_at_a_refused_paragraph_break() {
     assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("paragraph break refused"), "{r}");
     assert_eq!(*calls.lock().unwrap(), vec!["text.insert", "text.newParagraph"]);
 }
+
+/// #2242: `insert_table` fills a table in one call; the header row and style survive a DOCX round trip.
+#[test]
+fn insert_table_fills_a_table_and_survives_docx() {
+    let mut s = Server::new(Box::new(Headless::default()));
+    tool(&mut s, "new_document", json!({}));
+    tool(&mut s, "insert_table", json!({"values": [["Month", "Cases"], ["January", 12], ["February", 7]], "header": true, "style": "TableGrid"}));
+    let path = std::env::temp_dir().join(format!("wordcraft-mcp-table-{}.docx", std::process::id()));
+    let path_str = path.to_str().expect("utf-8 path").to_string();
+    tool(&mut s, "save_document", json!({"path": path_str}));
+    let mut s2 = Server::new(Box::new(Headless::default()));
+    tool(&mut s2, "open_document", json!({"path": path_str}));
+    let doc = tool(&mut s2, "inspect_document", json!({}));
+    let table = doc["blocks"].as_array().unwrap().iter().find(|b| b["type"] == "table").expect("a table");
+    let text = table.to_string();
+    for cell in ["Month", "Cases", "January", "12", "February", "7"] {
+        assert!(text.contains(cell), "{cell} missing in {text}");
+    }
+    assert!(text.contains("TableGrid"), "{text}");
+    // The header flag is not in `inspect_document`; read it from the reopened document.
+    let mut h = Headless::default();
+    h.session.run("file.open", &json!({"path": path_str})).expect("reopened");
+    let _ = std::fs::remove_file(&path);
+    let table = h.session.doc.body.iter().find_map(|b| match &**b {
+        wordcraft_doc::Block::Table(t) => Some(t.clone()),
+        _ => None,
+    });
+    let table = table.expect("a table");
+    assert!(table.rows[0].props.header && !table.rows[1].props.header);
+}
+
+/// #2242: malformed `insert_table` arguments are tool errors, not panics.
+#[test]
+fn insert_table_rejects_hostile_values() {
+    let mut s = Server::new(Box::new(Headless::default()));
+    let wide: Vec<Value> = (0..64).map(|i| json!(i)).collect();
+    for args in [json!({}), json!({"values": "x"}), json!({"values": [["a", "b"], ["c"]]}), json!({"values": [wide]}), json!({"values": [[{"a": 1}]]}), json!({"values": []})] {
+        let r = call(&mut s, 12, "tools/call", json!({"name": "insert_table", "arguments": args}));
+        assert_eq!(r["result"]["isError"], true, "{args}: {r}");
+    }
+}
diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs
index 623e2d5ed3..2e5263e29f 100644
--- a/crates/mcp/src/tools.rs
+++ b/crates/mcp/src/tools.rs
@@ -64,10 +64,24 @@ pub fn tool_definitions() -> Value {
         tool(
             "execute",
             "Run a command",
-            "Run one command by id with params, e.g. {command:\"format.bold\"} or {command:\"insert.table\", params:{rows:3, cols:4}}.",
+            "Run one command by id with params, e.g. {command:\"format.bold\"} or {command:\"insert.table\", params:{rows:3, cols:4}}. To insert a table that is already filled, use `insert_table`.",
             obj(json!({"command": s("Command id"), "params": {"type": "object", "description": "Command parameters"}}), &["command"]),
             false
         ),
+        tool(
+            "insert_table",
+            "Insert filled table",
+            "Insert a table at the caret, filled from `values` in one call (an array of rows, each an array of cells; all rows the same length, up to 1000 rows and 63 columns). Set `header` to make the first row a repeating header row.",
+            obj(
+                json!({
+                    "values": {"type": "array", "description": "Rows of cells (text, numbers or booleans)", "items": {"type": "array", "items": {"type": ["string", "number", "boolean", "null"]}}},
+                    "header": {"type": "boolean", "description": "First row is a header row that repeats on each page"},
+                    "style": s("Table style name (default GridTable1Light)")
+                }),
+                &["values"]
+            ),
+            false
+        ),
         tool(
             "batch",
             "Run several commands",
@@ -191,6 +205,18 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, a: &Value) -> ToolResult {
             Some(c) => wrap(exec(b, c, a.get("params").cloned().unwrap_or(json!({})))),
             None => ToolResult::error("missing `command`"),
         },
+        "insert_table" => match a.get("values") {
+            Some(values) => {
+                let mut params = json!({"values": values});
+                for k in ["header", "style"] {
+                    if let Some(x) = a.get(k) {
+                        params[k] = x.clone();
+                    }
+                }
+                wrap(exec(b, "insert.spreadsheet", params))
+            }
+            None => ToolResult::error("missing `values`: an array of rows, each an array of cells"),
+        },
         "batch" => {
             let keep = a.get("keepGoing").and_then(Value::as_bool).unwrap_or(false);
             let mut out = Vec::new();
diff --git a/docs/mcp.md b/docs/mcp.md
index 50f8e07b53..4dc0543660 100644
--- a/docs/mcp.md
+++ b/docs/mcp.md
@@ -83,7 +83,8 @@ files it was given rather than guarding against other software on the computer.
 | Tool | What it does |
 |---|---|
 | `list_commands` | every command (filter with `query`) — over 270 of them |
-| `execute` | run one command: `{command: "insert.table", params: {rows: 3, cols: 4}}` |
+| `execute` | run one command: `{command: "insert.table", params: {rows: 3, cols: 4}}` (an empty table; for a filled one use `insert_table`) |
+| `insert_table` | insert a filled table in one call: `{values: [["Month", "Cases"], ["January", 12]], header: true, style?}` (same as `insert.spreadsheet`) |
 | `batch` | run several commands in order |
 | `new_document` | blank, sample, letter, resume, report |
 | `open_document` / `save_document` | docx, doc (open only), odt, rtf, html, md, tex, txt, json; save also pdf, png |