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

feat(mcp): replace_node write tool — atomic swap at same slot

Browse source 
Eighth write tool in the catalog. Required args: node_id, kind,
name, x, y, width, height. Optional: fill_hex. Builds a fresh
node with a non-colliding id and swaps it into the same parent
slot the target node currently occupies, preserving sibling
order.

Bounded scope: only leaf-style fields land on the replacement.
Full subtree replacement requires a JSON Node parser that
doesn't live on this side yet. The current contract matches TS
`replace_node` for primitives, minus children.

Apply path follows the same pre-validate-then-mutate discipline
as `update_node` and `move_node`: kind / geometry / fill_hex /
target existence / id space — every check before any mutation.
A bad fill_hex never leaves the document half-touched (covered
by `apply_mcp_command_replace_node_atomic_on_invalid_fill_hex`).

Tests live in `mcp/replace_node_tests.rs` (matches the
`copy_node_tests.rs` sibling pattern; keeps `write_tools_tests.rs`
under cap).
This commit is contained in:
Kayshen-X 2026-05-14 21:31:12 +08:00
parent e9254e77c1
commit e12bc4f0bc
5 changed files with 374 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -130,6 +130,43 @@ fn clone_subtree(node: &Node, next_id: &mut u64) -> Option<Node> {
Some(clone)
}
/// Replace the node with `target` id with `replacement` at its
/// current slot. Walks every page; on the first match,
/// swaps in place (`children[idx] = replacement`) so the
/// sibling order is preserved. Returns true on success.
fn replace_node_in_doc(doc: &mut Document, target: NodeId, replacement: Node) -> bool {
// Wrap the replacement in an Option so we can take() it inside
// the recursive walk without giving up the Some-on-failure
// contract (replacement returned untouched is irrelevant here
// — we either consumed it or replace_node already returned).
let mut slot = Some(replacement);
for page in doc.pages.iter_mut() {
if replace_in_subtree(&mut page.children, target, &mut slot) {
return true;
}
}
false
}
fn replace_in_subtree(
children: &mut [Node],
target: NodeId,
slot: &mut Option<Node>,
) -> bool {
if let Some(idx) = children.iter().position(|n| n.id == target) {
if let Some(replacement) = slot.take() {
children[idx] = replacement;
return true;
}
}
for node in children.iter_mut() {
if replace_in_subtree(&mut node.children, target, slot) {
return true;
}
}
false
}
/// 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`).
@ -323,6 +360,51 @@ impl Document {
}
true
}
crate::mcp::McpCommand::ReplaceNode {
node_id,
kind,
name,
x,
y,
width,
height,
fill_hex,
} => {
let Some(target) = NodeId::new_opt(*node_id) else {
return false;
};
let Some(node_kind) = parse_node_kind(kind) else {
return false;
};
// Pre-validate EVERYTHING before mutating (same
// discipline as update_node + move_node — no
// half-mutated state on a bad arg).
if *width < 0 || *height < 0 {
return false;
}
let fill = match fill_hex {
None => None,
Some(hex) => match parse_hex_color(hex) {
Some(c) => Some(c),
None => return false,
},
};
if find_node_in_doc(self, target).is_none() {
return false;
}
let Some(next_id) = self.next_node_id_seed() else {
return false;
};
let mut replacement = Node::leaf(next_id, node_kind, name.clone());
replacement.bounds = crate::Rect::xywh(
*x as f32,
*y as f32,
*width as f32,
*height as f32,
);
replacement.fill = fill;
replace_node_in_doc(self, target, replacement)
}
crate::mcp::McpCommand::MoveNode {
node_id,
target_parent_id,

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

@ -181,7 +181,8 @@ impl VariableTable {
| crate::mcp::McpCommand::UpdateNode { .. }
| crate::mcp::McpCommand::DeleteNode { .. }
| crate::mcp::McpCommand::MoveNode { .. }
| crate::mcp::McpCommand::CopyNode { .. } => {
| crate::mcp::McpCommand::CopyNode { .. }
| crate::mcp::McpCommand::ReplaceNode { .. } => {
// Not VariableTable mutations — Pages-level commands
// live on `Document::apply_mcp_command`. Return false
// so callers with only a VariableTable handle know

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

@ -14,6 +14,7 @@ pub mod tools;
pub mod write_tools;
#[cfg(test)] mod write_tools_tests;
#[cfg(test)] mod copy_node_tests;
#[cfg(test)] mod replace_node_tests;
// Re-export the public surface of submodules so callers can keep
// using `mcp::parse_tool_call` / `mcp::GetDocumentInfo` after the
@ -26,9 +27,9 @@ pub use tools::{
};
pub use write_tools::{
copy_node_snapshot, delete_node_snapshot, insert_node_snapshot, move_node_snapshot,
set_active_axis_value_snapshot, set_variable_color_snapshot, update_node_snapshot,
CopyNode, DeleteNode, InsertNode, MoveNode, SetActiveAxisValue, SetVariableColor,
UpdateNode,
replace_node_snapshot, set_active_axis_value_snapshot, set_variable_color_snapshot,
update_node_snapshot, CopyNode, DeleteNode, InsertNode, MoveNode, ReplaceNode,
SetActiveAxisValue, SetVariableColor, UpdateNode,
};
/// JSON-RPC-style request id. Strings + integers both supported by
@ -180,6 +181,31 @@ pub enum McpCommand {
node_id: u64,
target_parent_id: u64,
},
/// Replace an existing node with a freshly-built one at the
/// same parent slot + same index. Captures the same shape as
/// `InsertNode` (kind / name / bounds / fill_hex) plus the
/// target `node_id` to swap. Useful when the LLM wants to
/// change kind (rect → ellipse) or radically alter the node
/// in one atomic op rather than via incremental `UpdateNode`
/// patches. The new node gets a fresh id past `max_node_id()`
/// so the wire response is still fire-and-forget (callers
/// re-query to learn the new id).
///
/// Bounded scope: today only the leaf-style fields land on
/// the replacement node. A future patch may grow this to
/// carry children / a full subtree once a JSON Node parser
/// lives on the host. The current contract matches what TS
/// `replace_node` accepts for primitives, minus children.
ReplaceNode {
node_id: u64,
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

148
crates/openpencil-shell-core/src/mcp/replace_node_tests.rs Normal file
View file

@ -0,0 +1,148 @@
//! Tests for `mcp::write_tools::ReplaceNode` + the matching
//! `Document::apply_mcp_command(ReplaceNode)` branch. Sibling
//! file (same rationale as `copy_node_tests.rs`) — keeps
//! `write_tools_tests.rs` under the 800-line cap as the write
//! surface grows.
use super::write_tools::*;
use super::{McpCommand, McpTool, ToolErrorCode, ToolOutcome};
use std::collections::BTreeMap;
#[test]
fn replace_node_validates_required_args() {
let tool = replace_node_snapshot();
match tool.call(&BTreeMap::new()) {
ToolOutcome::Err(code, _) => assert_eq!(code, ToolErrorCode::MissingArgument),
_ => panic!(),
}
let mut args = BTreeMap::new();
args.insert("node_id".into(), "11".into());
match tool.call(&args) {
ToolOutcome::Err(code, msg) => {
assert_eq!(code, ToolErrorCode::MissingArgument);
assert!(msg.contains("kind"));
}
_ => panic!(),
}
args.insert("kind".into(), "rect".into());
args.insert("name".into(), "Replacement".into());
args.insert("x".into(), "10".into());
args.insert("y".into(), "20".into());
args.insert("width".into(), "100".into());
args.insert("height".into(), "30".into());
match tool.call(&args) {
ToolOutcome::OkWithCommand(_, McpCommand::ReplaceNode { node_id, kind, name, x, y, width, height, fill_hex }) => {
assert_eq!(node_id, 11);
assert_eq!(kind, "rect");
assert_eq!(name, "Replacement");
assert_eq!(x, 10);
assert_eq!(y, 20);
assert_eq!(width, 100);
assert_eq!(height, 30);
assert!(fill_hex.is_none());
}
other => panic!("expected ReplaceNode, got {other:?}"),
}
}
#[test]
fn replace_node_rejects_bad_kind_and_geometry_and_fill() {
let tool = replace_node_snapshot();
let mut args = BTreeMap::new();
args.insert("node_id".into(), "11".into());
args.insert("kind".into(), "blob".into()); // not in ALLOWED_KINDS
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, _) => assert_eq!(code, ToolErrorCode::InvalidArgument),
_ => panic!(),
}
args.insert("kind".into(), "rect".into());
args.insert("width".into(), "-5".into());
match tool.call(&args) {
ToolOutcome::Err(code, _) => assert_eq!(code, ToolErrorCode::InvalidArgument),
_ => panic!(),
}
args.insert("width".into(), "5".into());
args.insert("fill_hex".into(), "not-a-hex".into());
match tool.call(&args) {
ToolOutcome::Err(code, _) => assert_eq!(code, ToolErrorCode::InvalidArgument),
_ => panic!(),
}
}
#[test]
fn apply_mcp_command_replace_node_swaps_at_same_slot() {
use crate::document::Document;
let mut doc = Document::sample();
let pre_max = doc.max_node_id();
// Title (id 11) is at frame.children[0]. Replace it with a Rect.
let cmd = McpCommand::ReplaceNode {
node_id: 11,
kind: "rect".into(),
name: "Swapped".into(),
x: 0,
y: 0,
width: 50,
height: 50,
fill_hex: Some("#ff0000".into()),
};
assert!(doc.apply_mcp_command(&cmd));
let frame = doc.pages[0].children.iter().find(|n| n.id.raw() == 10).unwrap();
// Slot 0 now holds the replacement; old id is gone.
assert!(frame.children.iter().all(|n| n.id.raw() != 11));
let swapped = &frame.children[0];
assert_eq!(swapped.name, "Swapped");
assert!(swapped.id.raw() > pre_max, "fresh id past pre_max");
// Sibling (Button group, id 12) still at slot 1.
assert_eq!(frame.children[1].id.raw(), 12);
}
#[test]
fn apply_mcp_command_replace_node_rejects_unknown_id() {
use crate::document::Document;
let mut doc = Document::sample();
let pre_root_len = doc.pages[0].children.len();
let cmd = McpCommand::ReplaceNode {
node_id: 99999,
kind: "rect".into(),
name: "Ghost".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: None,
};
assert!(!doc.apply_mcp_command(&cmd));
// Doc unchanged.
assert_eq!(doc.pages[0].children.len(), pre_root_len);
}
#[test]
fn apply_mcp_command_replace_node_atomic_on_invalid_fill_hex() {
// The applier pre-validates fill_hex BEFORE allocating the
// fresh id or mutating pages (same atomicity pattern as
// update_node + move_node).
use crate::document::Document;
let mut doc = Document::sample();
let pre_max = doc.max_node_id();
let cmd = McpCommand::ReplaceNode {
node_id: 11,
kind: "rect".into(),
name: "WouldFail".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: Some("not-hex".into()),
};
assert!(!doc.apply_mcp_command(&cmd));
// Title still in place under Frame.
let frame = doc.pages[0].children.iter().find(|n| n.id.raw() == 10).unwrap();
assert!(frame.children.iter().any(|n| n.id.raw() == 11));
// No id was allocated.
assert_eq!(doc.max_node_id(), pre_max);
}

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

@ -507,6 +507,119 @@ pub fn copy_node_snapshot() -> CopyNode {
CopyNode
}
/// First-party `replace_node` tool — swap an existing node with a
/// freshly-built one at the same parent slot. Required args:
/// node_id, kind, name, x, y, width, height. Optional: fill_hex.
/// Bounded scope: only leaf-style fields are accepted today (the
/// children of the replacement default to empty). TS-equivalent
/// behavior for primitives; subtree-replacement requires a JSON
/// Node parser that doesn't live on this side yet.
pub struct ReplaceNode;
impl McpTool for ReplaceNode {
fn name(&self) -> &str {
"replace_node"
}
fn call(&self, args: &BTreeMap<String, String>) -> ToolOutcome {
let raw = match args.get("node_id") {
Some(s) => s,
None => {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"node_id is required".into(),
);
}
};
let node_id: u64 = match raw.parse() {
Ok(n) if n > 0 => n,
_ => {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!("node_id must be a positive u64, got {raw:?}"),
);
}
};
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(),
);
}
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::ReplaceNode {
node_id,
kind,
name,
x,
y,
width,
height,
fill_hex,
},
)
}
}
pub fn replace_node_snapshot() -> ReplaceNode {
ReplaceNode
}
pub fn set_active_axis_value_snapshot(
doc: &crate::document::Document,
) -> SetActiveAxisValue {