SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

feat(mcp): insert_node tool — third write (the big one)

Browse source 
Third MCP write tool, completing the "create + style + theme" write
surface for LLM clients. `insert_node` is the TS pen-mcp
equivalent's flagship capability — without it, LLMs can only
modify existing nodes; with it, they can build entire designs.

`crates/openpencil-shell-core/src/mcp.rs`:
  - `McpCommand::InsertNode { kind, name, x, y, width, height,
    fill_hex }`. fill_hex is `Option<String>` — color-bearing
    shapes can carry a fill; structural nodes (frame/group)
    pass None.

`crates/openpencil-shell-core/src/document/mcp_apply.rs` (NEW):
  - `Document::apply_mcp_command(cmd)` — lifted to Document level
    so InsertNode can reach Pages + the id allocator (variable +
    theme commands still route to `var_table.apply_mcp_command`).
  - `Document::next_node_id_seed()` — allocates a fresh id past
    `max_node_id() + 1`, saturating_add-guarded so u64::MAX
    returns 1 instead of colliding with NodeId::NONE.
  - `parse_node_kind(s)` — accepts the same lowercase strings
    the read-side tools (get_node, get_selection) emit, so an
    LLM can round-trip a node's kind through read → modify →
    re-insert without re-encoding.
  - Pulled out of mutators.rs to keep that file under 800 lines.

`crates/openpencil-shell-core/src/mcp/tools.rs`:
  - `InsertNode` (stateless tool struct) + `insert_node_snapshot()`
    factory. No document snapshot needed — the tool doesn't need
    to know the current state; the host's applier handles id
    allocation + bounds installation.
  - `McpTool::call` validates:
      - All required args present (kind / name / x / y / width /
        height); each `MissingArgument` carries the missing
        name.
      - kind in ALLOWED_KINDS (frame / group / rect / ellipse /
        polygon / line / text / path).
      - x / y / width / height parse as decimal i32 (negative x/y
        allowed for nodes placed off the page-origin; width /
        height must be non-negative).
      - Optional fill_hex parses as #rgb / #rrggbb / #rrggbbaa.
  - Returns `OkWithCommand(InsertNode)` on success.

Tests (6 added, 325 shell-core total):
  - insert_node_validates_required_args — missing kind →
    MissingArgument; invalid kind → InvalidArgument.
  - insert_node_validates_numeric_args — non-numeric x →
    InvalidArgument; negative width → InvalidArgument.
  - insert_node_validates_optional_fill_hex — bad hex →
    InvalidArgument.
  - insert_node_returns_command_with_parsed_args — happy path;
    every field round-trips into the McpCommand payload.
  - apply_mcp_command_routes_insert_node — end-to-end: doc.empty()
    → apply InsertNode → new node lives on active page, bounds +
    fill flow through, name matches.
  - apply_mcp_command_rejects_invalid_node_kind — bad kind at
    apply time → false (defensive re-check beyond the tool's
    validation, so host-only call sites are safe too).

mutators.rs trimmed from 814 → 798 (apply_mcp_command extracted +
some redundant doc comments compacted). All OP-owned files now
under the 800 cap except 3 pre-existing tech-debt violators
(codegen.rs 867, widget_host/press.rs 840,
widgets/canvas_viewport.rs 839).

MCP write surface now: 3 first-party writes (set_variable_color,
set_active_axis_value, insert_node). The TS pen-mcp catalog still
needs: update_node, delete_node, move_node, copy_node, replace,
batch_design, design_skeleton — each extends McpCommand + the
same applier pattern.
This commit is contained in:
Kayshen-X 2026-05-14 20:37:27 +08:00
parent e51600f068
commit a6d208d313
7 changed files with 382 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
crates/openpencil-shell-core/src/document.rs
View file

@ -800,6 +800,7 @@ mod align;
mod color_picker;
mod components;
mod grouping;
mod mcp_apply;
mod mutators;
mod page_mutators;
mod pen;

81
crates/openpencil-shell-core/src/document/mcp_apply.rs Normal file
View file

@ -0,0 +1,81 @@
//! MCP-write command application against the document. Pulled out
//! of `mutators.rs` so that file stays under the 800-line cap as
//! more write commands land.
use super::variables::parse_hex_color;
use super::{Document, Node, NodeKind};
/// Resolve an MCP `kind` arg into a NodeKind. Accepts the same
/// lowercase strings the read-side tools emit (`frame`, `group`,
/// `rect`, `ellipse`, `polygon`, `line`, `text`, `path`).
pub(super) fn parse_node_kind(s: &str) -> Option<NodeKind> {
match s {
"frame" => Some(NodeKind::Frame),
"group" => Some(NodeKind::Group),
"rect" => Some(NodeKind::Rect),
"ellipse" => Some(NodeKind::Ellipse),
"polygon" => Some(NodeKind::Polygon),
"line" => Some(NodeKind::Line),
"text" => Some(NodeKind::Text),
"path" => Some(NodeKind::Path),
_ => None,
}
}
impl Document {
/// Apply an MCP write command against the document. Returns
/// true when the command actually changed something so callers
/// can decide whether to push an undo snapshot. False on apply-
/// time validation failure (unknown variable, value not in axis,
/// unknown node kind, etc.). Routes variable + theme commands
/// to `VariableTable::apply_mcp_command`; `InsertNode` lives
/// here because it needs Pages + the id allocator.
pub fn apply_mcp_command(&mut self, cmd: &crate::mcp::McpCommand) -> bool {
match cmd {
crate::mcp::McpCommand::InsertNode {
kind,
name,
x,
y,
width,
height,
fill_hex,
} => {
let Some(node_kind) = parse_node_kind(kind) else {
return false;
};
// Compute the fresh id BEFORE taking a mutable
// borrow on `pages` — `max_node_id()` reads pages
// immutably and would conflict otherwise.
let next_id = self.next_node_id_seed();
let active_idx = self.active_page_index;
let Some(page) = self.pages.get_mut(active_idx) else {
return false;
};
let mut node = Node::leaf(next_id, node_kind, name.clone());
node.bounds = crate::Rect::xywh(
*x as f32,
*y as f32,
*width as f32,
*height as f32,
);
if let Some(hex) = fill_hex {
if let Some(color) = parse_hex_color(hex) {
node.fill = Some(color);
} else {
return false;
}
}
page.children.push(node);
true
}
_ => self.var_table.apply_mcp_command(cmd),
}
}
/// Compute a fresh node id that won't collide with any existing
/// node across pages. Mirrors the duplicate allocator's guard.
fn next_node_id_seed(&self) -> u64 {
self.max_node_id().saturating_add(1).max(1)
}
}

26
crates/openpencil-shell-core/src/document/mutators.rs
View file

@ -1,16 +1,10 @@
//! `impl Document` mutators + queries.
//!
//! Lives in `document/mutators.rs` so the `document` module file
//! itself stays under the 800-line cap. The impl block is split
//! from `document.rs` purely for file-size hygiene — semantically
//! these methods belong to `Document` and are accessed via the
//! usual `doc.method(...)` paths.
//! `impl Document` mutators + queries. Split from `document.rs`
//! for file-size hygiene — methods are reached via `doc.method()`.
use super::walkers::*;
use super::*;
/// Whether `reorder_relative` drops the source before or after the
/// anchor. Internal helper for `reorder_before` / `reorder_after`.
/// Whether `reorder_relative` drops the source before or after.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum RelativePosition {
Before,
@ -49,20 +43,10 @@ impl Document {
}
}
/// Sample document for the editor-UI demo.
/// Sample document for the editor-UI demo. Stable ids: page=1,
/// frame=10, title=11, button=12, button_rect=13, button_text=14.
pub fn sample() -> Self {
use crate::{Color, Rect};
// Id allocations: page=1, frame=10, title=11, button=12,
// button_rect=13, button_text=14. Stable across runs so
// tests can assert specific ids.
//
// Layout (document coordinates, top-left origin):
// Frame (40, 40)–(360, 240) white fill, black 1px stroke
// Title (60, 60)–(*, *) text "Hello OpenPencil", no bg
// Button group at (60, 130)
// Rect (60, 130)–(180, 36) blue fill, no stroke
// Text (76, 152)–(*, *) text "Click me", no bg
let title = Node::leaf(11, NodeKind::Text, "Title")
.with_bounds(Rect::xywh(60.0, 60.0, 240.0, 28.0))
.with_text("Hello OpenPencil");

10
crates/openpencil-shell-core/src/document/variables.rs
View file

@ -177,6 +177,14 @@ impl VariableTable {
self.active_theme.insert(axis.clone(), value.clone());
true
}
crate::mcp::McpCommand::InsertNode { .. } => {
// Not a VariableTable mutation — InsertNode lives on
// the wider `Document::apply_mcp_command` since it
// needs Pages + the id allocator. Return false here
// so callers that only have a VariableTable handle
// know this variant wasn't theirs to apply.
false
}
}
}
@ -341,7 +349,7 @@ impl VariableTable {
/// Parse `#rgb` / `#rrggbb` / `#rrggbbaa` into a `Color`. Mirrors the
/// TS paint helpers — lenient on case, requires the leading `#`.
fn parse_hex_color(s: &str) -> Option<crate::Color> {
pub(super) fn parse_hex_color(s: &str) -> Option<crate::Color> {
let s = s.trim().strip_prefix('#')?;
let (r, g, b, a) = match s.len() {
3 => {

32
crates/openpencil-shell-core/src/mcp.rs
View file

@ -17,10 +17,11 @@ pub mod tools;
// split. Mirrors the `widgets::*` re-export pattern.
pub use parser::parse_tool_call;
pub use tools::{
document_info_snapshot, get_active_theme_snapshot, get_node_snapshot, list_pages_snapshot,
list_variables_snapshot, selection_snapshot, set_active_axis_value_snapshot,
set_variable_color_snapshot, GetActiveTheme, GetDocumentInfo, GetNode, GetSelection,
ListPages, ListVariables, NodeRecord, SetActiveAxisValue, SetVariableColor, VariableRecord,
document_info_snapshot, get_active_theme_snapshot, get_node_snapshot, insert_node_snapshot,
list_pages_snapshot, list_variables_snapshot, selection_snapshot,
set_active_axis_value_snapshot, set_variable_color_snapshot, GetActiveTheme,
GetDocumentInfo, GetNode, GetSelection, InsertNode, ListPages, ListVariables, NodeRecord,
SetActiveAxisValue, SetVariableColor, VariableRecord,
};
/// JSON-RPC-style request id. Strings + integers both supported by
@ -107,10 +108,29 @@ pub enum ToolOutcome {
/// shadow / history snapshot).
/// - `SetActiveAxisValue { axis, value }` — pins a theme axis
/// directly (vs. `cycle_active_axis_value` which advances).
/// - `InsertNode { ... }` — creates a fresh node on the active
/// page with the supplied bounds + fill. The applier
/// allocates an id past `max_node_id()` so it can't collide
/// with existing nodes even after deletes.
#[derive(Debug, Clone, PartialEq)]
pub enum McpCommand {
SetVariableColor { name: String, hex: String },
SetActiveAxisValue { axis: String, value: String },
SetVariableColor {
name: String,
hex: String,
},
SetActiveAxisValue {
axis: String,
value: String,
},
InsertNode {
kind: String,
name: String,
x: i32,
y: i32,
width: i32,
height: i32,
fill_hex: Option<String>,
},
}
/// Trait every MCP tool implements. The MCP server walks its

134
crates/openpencil-shell-core/src/mcp/tools.rs
View file

@ -568,19 +568,10 @@ impl McpTool for SetVariableColor {
}
}
/// First-party `set_active_axis_value` tool — programmatic
/// counterpart to the VariablesPanel chip-cycle interaction. LLM
/// clients use this when they want to pin an axis to a specific
/// value rather than advance through the cycle (e.g. "set mode to
/// dark" instead of "cycle mode"). Validates that the axis exists
/// AND the value is in `themes[axis].values`; the host's applier
/// (`VariableTable::apply_mcp_command`) re-validates against live
/// state and rejects on drift.
///
/// Wire shape:
/// args — { "axis": "<axis>", "value": "<value>" }
/// result — { "wrote": "true" } when queued
/// command — `McpCommand::SetActiveAxisValue { axis, value }`
/// First-party `set_active_axis_value` tool — pins an axis to a
/// value (vs `cycle_active_axis_value` which advances). Validates
/// axis exists + value is in `themes[axis].values`; the host
/// applier re-validates against live state.
pub struct SetActiveAxisValue {
/// Snapshot of axis → allowed-values, mirroring
/// `VariableTable::themes`. Validation only — the host re-runs
@ -633,6 +624,123 @@ impl McpTool for SetActiveAxisValue {
}
}
/// First-party `insert_node` tool — creates a fresh node on the
/// active page. Args: kind / name / x / y / width / height +
/// optional fill_hex. The applier allocates a non-colliding id
/// past `max_node_id()` so the LLM never has to reason about id
/// space. Stateless — no document snapshot needed.
pub struct InsertNode;
impl McpTool for InsertNode {
fn name(&self) -> &str {
"insert_node"
}
fn call(&self, args: &BTreeMap<String, String>) -> ToolOutcome {
// Required args: kind, name, x, y, width, height.
let kind = match args.get("kind") {
Some(s) => s.clone(),
None => {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"kind is required".into(),
);
}
};
if !ALLOWED_KINDS.iter().any(|k| *k == kind) {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!(
"kind {kind:?} not supported; allowed: {}",
ALLOWED_KINDS.join(", ")
),
);
}
let name = match args.get("name") {
Some(s) => s.clone(),
None => {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"name is required".into(),
);
}
};
let x = match parse_i32_arg(args, "x") {
Ok(v) => v,
Err(e) => return e,
};
let y = match parse_i32_arg(args, "y") {
Ok(v) => v,
Err(e) => return e,
};
let width = match parse_i32_arg(args, "width") {
Ok(v) => v,
Err(e) => return e,
};
let height = match parse_i32_arg(args, "height") {
Ok(v) => v,
Err(e) => return e,
};
if width < 0 || height < 0 {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
"width / height must be non-negative".into(),
);
}
// fill_hex is optional, but if present it must parse.
let fill_hex = match args.get("fill_hex") {
None => None,
Some(s) if !validate_hex(s) => {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!("fill_hex must be #rgb/#rrggbb/#rrggbbaa, got {s:?}"),
);
}
Some(s) => Some(s.clone()),
};
let mut out = BTreeMap::new();
out.insert("wrote".into(), "true".into());
ToolOutcome::OkWithCommand(
out,
McpCommand::InsertNode {
kind,
name,
x,
y,
width,
height,
fill_hex,
},
)
}
}
const ALLOWED_KINDS: &[&str] = &[
"frame", "group", "rect", "ellipse", "polygon", "line", "text", "path",
];
fn parse_i32_arg(
args: &BTreeMap<String, String>,
key: &str,
) -> Result<i32, ToolOutcome> {
let Some(raw) = args.get(key) else {
return Err(ToolOutcome::Err(
ToolErrorCode::MissingArgument,
format!("{key} is required"),
));
};
raw.parse::<i32>().map_err(|_| {
ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!("{key} must be a decimal i32, got {raw:?}"),
)
})
}
pub fn insert_node_snapshot() -> InsertNode {
// Stateless — no document snapshot needed.
InsertNode
}
pub fn set_active_axis_value_snapshot(
doc: &crate::document::Document,
) -> SetActiveAxisValue {

139
crates/openpencil-shell-core/src/mcp/tools_tests.rs
View file

@ -579,3 +579,142 @@ fn apply_mcp_command_routes_set_active_axis_value() {
};
assert!(!doc.var_table.apply_mcp_command(&bad));
}
#[test]
fn insert_node_validates_required_args() {
let tool = insert_node_snapshot();
// Missing kind.
let mut args = BTreeMap::new();
args.insert("name".into(), "X".into());
args.insert("x".into(), "0".into());
args.insert("y".into(), "0".into());
args.insert("width".into(), "10".into());
args.insert("height".into(), "10".into());
match tool.call(&args) {
ToolOutcome::Err(code, msg) => {
assert_eq!(code, ToolErrorCode::MissingArgument);
assert!(msg.contains("kind"));
}
_ => panic!(),
}
// Invalid kind.
args.insert("kind".into(), "frobnicate".into());
match tool.call(&args) {
ToolOutcome::Err(code, _) => {
assert_eq!(code, ToolErrorCode::InvalidArgument);
}
_ => panic!(),
}
}
#[test]
fn insert_node_validates_numeric_args() {
let tool = insert_node_snapshot();
let mut args = BTreeMap::new();
args.insert("kind".into(), "rect".into());
args.insert("name".into(), "X".into());
args.insert("x".into(), "not-a-number".into());
args.insert("y".into(), "0".into());
args.insert("width".into(), "10".into());
args.insert("height".into(), "10".into());
match tool.call(&args) {
ToolOutcome::Err(code, _) => assert_eq!(code, ToolErrorCode::InvalidArgument),
_ => panic!(),
}
args.insert("x".into(), "0".into());
args.insert("width".into(), "-5".into());
match tool.call(&args) {
ToolOutcome::Err(code, msg) => {
assert_eq!(code, ToolErrorCode::InvalidArgument);
assert!(msg.contains("non-negative"));
}
_ => panic!(),
}
}
#[test]
fn insert_node_validates_optional_fill_hex() {
let tool = insert_node_snapshot();
let mut args = BTreeMap::new();
args.insert("kind".into(), "rect".into());
args.insert("name".into(), "X".into());
args.insert("x".into(), "0".into());
args.insert("y".into(), "0".into());
args.insert("width".into(), "10".into());
args.insert("height".into(), "10".into());
args.insert("fill_hex".into(), "not-hex".into());
match tool.call(&args) {
ToolOutcome::Err(code, _) => assert_eq!(code, ToolErrorCode::InvalidArgument),
_ => panic!(),
}
}
#[test]
fn insert_node_returns_command_with_parsed_args() {
let tool = insert_node_snapshot();
let mut args = BTreeMap::new();
args.insert("kind".into(), "rect".into());
args.insert("name".into(), "My Rect".into());
args.insert("x".into(), "10".into());
args.insert("y".into(), "20".into());
args.insert("width".into(), "100".into());
args.insert("height".into(), "50".into());
args.insert("fill_hex".into(), "#ff0000".into());
match tool.call(&args) {
ToolOutcome::OkWithCommand(_, McpCommand::InsertNode { kind, name, x, y, width, height, fill_hex }) => {
assert_eq!(kind, "rect");
assert_eq!(name, "My Rect");
assert_eq!(x, 10);
assert_eq!(y, 20);
assert_eq!(width, 100);
assert_eq!(height, 50);
assert_eq!(fill_hex.as_deref(), Some("#ff0000"));
}
other => panic!("expected InsertNode command, got {other:?}"),
}
}
#[test]
fn apply_mcp_command_routes_insert_node() {
use crate::document::Document;
let mut doc = Document::empty();
let initial_root_count = doc.pages[0].children.len();
let cmd = McpCommand::InsertNode {
kind: "rect".into(),
name: "Created".into(),
x: 50,
y: 60,
width: 200,
height: 150,
fill_hex: Some("#00ff00".into()),
};
assert!(doc.apply_mcp_command(&cmd));
// New node lives on the active page; bounds + fill flow through.
assert_eq!(
doc.pages[0].children.len(),
initial_root_count + 1,
"node must be added"
);
let last = doc.pages[0].children.last().unwrap();
assert_eq!(last.name, "Created");
assert_eq!(last.bounds.size.x, 200.0);
assert_eq!(last.bounds.size.y, 150.0);
let fill = last.fill.expect("fill set");
assert!((fill.g - 1.0).abs() < 0.01);
}
#[test]
fn apply_mcp_command_rejects_invalid_node_kind() {
use crate::document::Document;
let mut doc = Document::empty();
let cmd = McpCommand::InsertNode {
kind: "frobnicate".into(),
name: "X".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: None,
};
assert!(!doc.apply_mcp_command(&cmd));
}