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

fix(mcp): support ts move copy targets

Browse source
This commit is contained in:
Kayshen-X 2026-06-02 09:42:01 +08:00
parent dd34ca8526
commit a26e1b0f48
18 changed files with 555 additions and 118 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
crates/op-cli/src/cli_node_tests.rs
View file

@ -79,3 +79,56 @@ fn parse_args_maps_delete_page_flag_to_ts_page_arg() {
}
);
}
#[test]
fn parse_args_maps_move_page_and_index_flags_to_ts_args() {
let p = parse_args(&[
"move".to_string(),
"n10".to_string(),
"--parent".to_string(),
"n20".to_string(),
"--index".to_string(),
"2".to_string(),
"--page".to_string(),
"page-2".to_string(),
])
.expect("parse move");
assert_eq!(
p.command,
Command::ToolCall {
tool: "move_node".to_string(),
args: vec![
("node_id".to_string(), "n10".to_string()),
("target_parent_id".to_string(), "n20".to_string()),
("index".to_string(), "2".to_string()),
("pageId".to_string(), "page-2".to_string()),
],
}
);
}
#[test]
fn parse_args_maps_copy_page_flag_to_ts_args() {
let p = parse_args(&[
"copy".to_string(),
"n10".to_string(),
"--parent".to_string(),
"n20".to_string(),
"--page".to_string(),
"page-2".to_string(),
])
.expect("parse copy");
assert_eq!(
p.command,
Command::ToolCall {
tool: "copy_node".to_string(),
args: vec![
("node_id".to_string(), "n10".to_string()),
("target_parent_id".to_string(), "n20".to_string()),
("pageId".to_string(), "page-2".to_string()),
],
}
);
}

16
crates/op-cli/src/main.rs
View file

@ -353,13 +353,19 @@ fn map_reparent(tool: &str, positionals: &[String], flags: &Flags) -> Result<Com
let id = required_pos(
positionals,
1,
"Usage: op move/copy <node-id> [--parent <parent-id>]",
"Usage: op move/copy <node-id> [--parent <parent-id>] [--page PAGE]",
)?;
let parent = flag_value(flags, "parent").unwrap_or_default();
tool_call(
tool,
vec![pair("node_id", id), pair("target_parent_id", parent)],
)
let mut pairs = vec![pair("node_id", id), pair("target_parent_id", parent)];
if tool == "move_node" {
if let Some(index) = flag_value(flags, "index") {
pairs.push(pair("index", index));
}
}
if let Some(page) = flag_value(flags, "page") {
pairs.push(pair("pageId", page));
}
tool_call(tool, pairs)
}
fn map_design_like(tool: &str, payload: Option<&String>) -> Result<Command, String> {

5
crates/op-cli/src/usage.txt
View file

@ -20,8 +20,9 @@ COMMON COMMANDS:
patch x/y/width/height/name/fill
op delete <id> [--page PAGE] delete a node
op read-nodes [ids] [--depth N] [--vars] [--page P] [--file F]
op move <id> [--parent P] reparent a node (empty parent = page root)
op copy <id> [--parent P] deep-copy a node
op move <id> [--parent P] [--index N] [--page PAGE]
reparent a node (empty parent = page root)
op copy <id> [--parent P] [--page PAGE] deep-copy a node
op replace <id> <json|@file|-> replace with a leaf node
op design <json-array|@file|-> call batch_design(nodes_json)
op design:skeleton <json-array|@file|->

8
crates/op-editor-core/src/command.rs
View file

@ -157,12 +157,20 @@ pub enum EditorCommand {
MoveNode {
node_id: NodeId,
target_parent: NodeId,
/// `None` moves on the active page; `Some` targets a page id
/// or legacy page index.
page_id: Option<String>,
/// Optional insertion index within the target parent/root.
index: Option<usize>,
},
/// Deep-clone a node + subtree under a new parent (`NONE` = active
/// page root). Fresh ids minted past the id space.
CopyNode {
node_id: NodeId,
target_parent: NodeId,
/// `None` copies on the active page; `Some` targets a page id
/// or legacy page index.
page_id: Option<String>,
},
/// Replace an existing node with a freshly-built leaf at the same
/// slot. `drop_children` is the destructive-swap guard: replacing a

35
crates/op-editor-core/src/command_apply.rs
View file

@ -133,7 +133,7 @@ fn apply_import_svg_on_active_page(
else {
return false;
};
imported_root.is_real() && state.cmd_move_node(&imported_root, target_parent)
imported_root.is_real() && state.cmd_move_node(&imported_root, target_parent, None)
} else {
true
}
@ -220,11 +220,40 @@ impl EditorState {
EditorCommand::MoveNode {
node_id,
target_parent,
} => self.cmd_move_node(&node_id, &target_parent),
page_id,
index,
} => {
let Some(target_page_index) = command_page_index(self, page_id.as_deref()) else {
return false;
};
let original_page_index = self.ui.active_page_index;
if page_id.is_some() {
self.ui.active_page_index = target_page_index;
}
let changed = self.cmd_move_node(&node_id, &target_parent, index);
if page_id.is_some() && target_page_index != original_page_index {
self.ui.active_page_index = original_page_index;
}
changed
}
EditorCommand::CopyNode {
node_id,
target_parent,
} => self.cmd_copy_node(&node_id, &target_parent),
page_id,
} => {
let Some(target_page_index) = command_page_index(self, page_id.as_deref()) else {
return false;
};
let original_page_index = self.ui.active_page_index;
if page_id.is_some() {
self.ui.active_page_index = target_page_index;
}
let changed = self.cmd_copy_node(&node_id, &target_parent);
if page_id.is_some() && target_page_index != original_page_index {
self.ui.active_page_index = original_page_index;
}
changed
}
EditorCommand::ReplaceNode {
node_id,
kind,

79
crates/op-editor-core/src/command_node.rs
View file

@ -236,6 +236,50 @@ pub fn replace_node_in_children(
false
}
#[allow(clippy::result_large_err)]
fn insert_into_parent_or_root(
children: &mut Vec<PenNode>,
parent: &NodeId,
node: PenNode,
index: Option<usize>,
) -> Result<(), PenNode> {
if !parent.is_real() {
let idx = index.unwrap_or(children.len()).min(children.len());
children.insert(idx, node);
return Ok(());
}
insert_into_parent(children, parent, node, index)
}
#[allow(clippy::result_large_err)]
fn insert_into_parent(
children: &mut [PenNode],
parent: &NodeId,
node: PenNode,
index: Option<usize>,
) -> Result<(), PenNode> {
if let Some(idx) = children.iter().position(|n| n.id_str() == parent.as_str()) {
match children[idx].children_mut() {
Some(grand) => {
let insert_idx = index.unwrap_or(grand.len()).min(grand.len());
grand.insert(insert_idx, node);
return Ok(());
}
None => return Err(node),
}
}
let mut carry = node;
for child in children.iter_mut() {
if let Some(grand) = child.children_mut() {
match insert_into_parent(grand, parent, carry, index) {
Ok(()) => return Ok(()),
Err(returned) => carry = returned,
}
}
}
Err(carry)
}
impl EditorState {
/// Compute the numeric seed for the next editor-minted `n{N}` id —
/// `max_node_id() + 1`. `None` on `u64` exhaustion.
@ -374,7 +418,12 @@ impl EditorState {
/// `MoveNode` — reparent a node. A `NONE` target reparents to the
/// active page root; a real target must resolve + must not create
/// a cycle (target is a descendant of the moved node).
pub(crate) fn cmd_move_node(&mut self, node_id: &NodeId, target_parent: &NodeId) -> bool {
pub(crate) fn cmd_move_node(
&mut self,
node_id: &NodeId,
target_parent: &NodeId,
index: Option<usize>,
) -> bool {
if !node_id.is_real() || target_parent == node_id {
return false;
}
@ -389,7 +438,10 @@ impl EditorState {
if walkers::descendant_contains(src, target_parent) {
return false;
}
if walkers::find_node(children, target_parent).is_none() {
let Some(target) = walkers::find_node(children, target_parent) else {
return false;
};
if target.children().is_none() {
return false;
}
}
@ -398,12 +450,7 @@ impl EditorState {
let Some(detached) = walkers::extract_node(children, node_id) else {
return false;
};
if target_parent.is_real() {
walkers::append_into(children, target_parent, detached).is_ok()
} else {
children.push(detached);
true
}
insert_into_parent_or_root(children, target_parent, detached, index).is_ok()
}
/// `CopyNode` — deep-clone a node + subtree under a new parent
@ -418,8 +465,13 @@ impl EditorState {
if walkers::find_node(children, node_id).is_none() {
return false;
}
if target_parent.is_real() && walkers::find_node(children, target_parent).is_none() {
return false;
if target_parent.is_real() {
let Some(target) = walkers::find_node(children, target_parent) else {
return false;
};
if target.children().is_none() {
return false;
}
}
}
let Some(mut next_id) = self.next_node_id_seed() else {
@ -433,12 +485,7 @@ impl EditorState {
walkers::deep_clone_with_new_ids(src, &mut next_id, &mut taken)
};
let children = self.active_children_mut();
if target_parent.is_real() {
walkers::append_into(children, target_parent, clone).is_ok()
} else {
children.push(clone);
true
}
insert_into_parent_or_root(children, target_parent, clone, None).is_ok()
}
/// `ReplaceNode` — swap an existing node for a freshly-built leaf at

108
crates/op-editor-core/src/command_reparent_tests.rs Normal file
View file

@ -0,0 +1,108 @@
//! `EditorCommand::MoveNode` / `CopyNode` page targeting tests.
#![cfg(test)]
use crate::command::EditorCommand;
use crate::node_id::NodeId;
use crate::pen_node_ext::PenNodeExt;
use crate::test_support::{frame, rect, state_with};
use jian_ops_schema::page::PenPage;
fn id(s: &str) -> NodeId {
NodeId::new(s)
}
#[test]
fn move_node_can_target_requested_page_without_switching_active_page() {
let mut s = state_with(vec![]);
s.doc.pages = Some(vec![
PenPage {
id: "page-1".into(),
name: "Page 1".into(),
children: vec![rect("n1", "Current", 0.0, 0.0, 10.0, 10.0)],
state: None,
lifecycle: None,
},
PenPage {
id: "page-2".into(),
name: "Page 2".into(),
children: vec![
frame("n2", "Target", 0.0, 0.0, 100.0, 100.0, Vec::new()),
rect("n3", "Moved", 0.0, 0.0, 10.0, 10.0),
],
state: None,
lifecycle: None,
},
]);
s.ui.active_page_index = 0;
assert!(s.apply(EditorCommand::MoveNode {
node_id: id("n3"),
target_parent: id("n2"),
page_id: Some("page-2".into()),
index: None,
}));
let pages = s.doc.pages.as_ref().expect("pages");
let target_children = pages[1].children[0].children().expect("frame children");
assert_eq!(target_children.len(), 1);
assert_eq!(target_children[0].id_str(), "n3");
assert_eq!(s.ui.active_page_index, 0);
}
#[test]
fn move_node_can_insert_at_requested_root_index() {
let mut s = state_with(vec![
rect("n1", "A", 0.0, 0.0, 10.0, 10.0),
rect("n2", "B", 0.0, 0.0, 10.0, 10.0),
rect("n3", "C", 0.0, 0.0, 10.0, 10.0),
]);
assert!(s.apply(EditorCommand::MoveNode {
node_id: id("n3"),
target_parent: NodeId::NONE,
page_id: None,
index: Some(1),
}));
let ids: Vec<&str> = s.active_children().iter().map(|n| n.id_str()).collect();
assert_eq!(ids, vec!["n1", "n3", "n2"]);
}
#[test]
fn copy_node_can_target_requested_page_without_switching_active_page() {
let mut s = state_with(vec![]);
s.doc.pages = Some(vec![
PenPage {
id: "page-1".into(),
name: "Page 1".into(),
children: vec![rect("n1", "Current", 0.0, 0.0, 10.0, 10.0)],
state: None,
lifecycle: None,
},
PenPage {
id: "page-2".into(),
name: "Page 2".into(),
children: vec![
frame("n2", "Target", 0.0, 0.0, 100.0, 100.0, Vec::new()),
rect("n3", "Source", 0.0, 0.0, 10.0, 10.0),
],
state: None,
lifecycle: None,
},
]);
s.ui.active_page_index = 0;
assert!(s.apply(EditorCommand::CopyNode {
node_id: id("n3"),
target_parent: id("n2"),
page_id: Some("page-2".into()),
}));
let pages = s.doc.pages.as_ref().expect("pages");
let target_children = pages[1].children[0].children().expect("frame children");
assert_eq!(target_children.len(), 1);
assert_ne!(target_children[0].id_str(), "n3");
assert_eq!(target_children[0].base().name.as_deref(), Some("Source"));
assert_eq!(s.ui.active_page_index, 0);
}

8
crates/op-editor-core/src/command_tests.rs
View file

@ -29,6 +29,8 @@ fn move_node_reparents_into_container() {
assert!(s.apply(EditorCommand::MoveNode {
node_id: id("n11"),
target_parent: id("n12"),
page_id: None,
index: None,
}));
let group = find_node(s.active_children(), &id("n12")).unwrap();
assert!(group
@ -45,6 +47,8 @@ fn move_node_rejects_cycle() {
assert!(!s.apply(EditorCommand::MoveNode {
node_id: id("n10"),
target_parent: id("n12"),
page_id: None,
index: None,
}));
// n10 still at root.
assert!(s.active_children().iter().any(|c| c.id_str() == "n10"));
@ -56,6 +60,8 @@ fn move_node_to_page_root_with_none_target() {
assert!(s.apply(EditorCommand::MoveNode {
node_id: id("n11"),
target_parent: NodeId::NONE,
page_id: None,
index: None,
}));
// n11 now at the page root, alongside n10.
assert!(s.active_children().iter().any(|c| c.id_str() == "n11"));
@ -70,6 +76,7 @@ fn copy_node_clones_subtree_with_fresh_ids() {
assert!(s.apply(EditorCommand::CopyNode {
node_id: id("n12"),
target_parent: NodeId::NONE,
page_id: None,
}));
// A clone landed at the page root; its id is fresh.
assert!(s.max_node_id() > pre_max);
@ -83,6 +90,7 @@ fn copy_node_rejects_unknown_source() {
assert!(!s.apply(EditorCommand::CopyNode {
node_id: id("ghost"),
target_parent: NodeId::NONE,
page_id: None,
}));
}

2
crates/op-editor-core/src/lib.rs
View file

@ -63,6 +63,8 @@ mod command_delete_tests;
#[cfg(test)]
mod command_insert_tests;
#[cfg(test)]
mod command_reparent_tests;
#[cfg(test)]
mod command_style_replace_tests;
#[cfg(test)]
mod command_subtree_tests;

4
crates/op-host-desktop/src/mcp_serve.rs
View file

@ -782,8 +782,8 @@ const TOOL_SCHEMAS: &[&str] = &[
r#"{"name":"update_node","description":"Patch fields on an existing node. Accepts Rust flat fields or TS-style nodeId + data object.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"nodeId":{"type":"string"},"x":{"type":"string"},"y":{"type":"string"},"width":{"type":"string"},"height":{"type":"string"},"name":{"type":"string"},"fill_hex":{"type":"string"},"data":{"type":"object","description":"TS-style patch data: name/x/y/width/height/fill"},"pageId":{"type":"string","description":"optional target page id or legacy page index; omitted = active page"}}}}"#,
r#"{"name":"import_svg","description":"Parse an SVG document and insert the resulting nodes on the active or requested page. Supports rect/circle/ellipse/line/polyline/polygon and path (M/L/H/V/C/S/Q/T/Z); <g>/transforms/CSS are skipped. x/y (optional, default 0) offset the imported nodes in doc-px; parent inserts under a container node.","inputSchema":{"type":"object","properties":{"svg":{"type":"string","description":"SVG document text"},"x":{"type":"string","description":"i32 doc-px x offset (default 0)"},"y":{"type":"string","description":"i32 doc-px y offset (default 0)"},"parent":{"type":"string","description":"optional parent node id; empty/0/root omitted = page root"},"pageId":{"type":"string","description":"optional target page id or legacy page index; omitted = active page"}},"required":["svg"]}}"#,
r#"{"name":"delete_node","description":"Remove a node + descendants from its parent. Accepts Rust node_id or TS nodeId, plus optional pageId.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"nodeId":{"type":"string"},"pageId":{"type":"string","description":"optional target page id or legacy page index; omitted = active page"}}}}"#,
r#"{"name":"move_node","description":"Reparent a node. target_parent_id=0 puts it at the active page root.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"target_parent_id":{"type":"string"}},"required":["node_id","target_parent_id"]}}"#,
r#"{"name":"copy_node","description":"Deep-clone a subtree with fresh ids under a new parent.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"target_parent_id":{"type":"string"}},"required":["node_id","target_parent_id"]}}"#,
r#"{"name":"move_node","description":"Reparent a node. Accepts Rust node_id/target_parent_id or TS nodeId/parent, plus optional index and pageId.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"nodeId":{"type":"string"},"target_parent_id":{"type":"string"},"parent":{"type":"string","description":"target parent node id; empty/0/null/root omitted = page root"},"index":{"type":"string","description":"optional insertion index within target parent/root"},"pageId":{"type":"string","description":"optional target page id or legacy page index; omitted = active page"}}}}"#,
r#"{"name":"copy_node","description":"Deep-clone a subtree with fresh ids under a new parent. Accepts Rust node_id or TS sourceId/nodeId, plus optional parent/pageId.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"sourceId":{"type":"string"},"nodeId":{"type":"string"},"target_parent_id":{"type":"string"},"parent":{"type":"string","description":"target parent node id; empty/0/null/root omitted = page root"},"pageId":{"type":"string","description":"optional target page id or legacy page index; omitted = active page"}}}}"#,
r#"{"name":"replace_node","description":"Swap an existing node at the same parent slot with a freshly-built leaf. Set drop_children=true to discard a container's subtree.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"kind":{"type":"string","enum":["frame","group","rect","ellipse","polygon","line","text","path"]},"name":{"type":"string"},"x":{"type":"string"},"y":{"type":"string"},"width":{"type":"string"},"height":{"type":"string"},"fill_hex":{"type":"string"},"drop_children":{"type":"string","enum":["true","false"]}},"required":["node_id","kind","name","x","y","width","height"]}}"#,
];

26
crates/op-mcp/src/batch_design_tests.rs
View file

@ -177,11 +177,15 @@ fn batch_design_accepts_single_move_operation_without_index() {
EditorCommand::MoveNode {
node_id,
target_parent,
page_id,
index,
},
) => {
assert_eq!(result.get("count"), Some(&"1".to_string()));
assert_eq!(node_id.as_str(), "n14");
assert!(!target_parent.is_real());
assert!(page_id.is_none());
assert!(index.is_none());
}
other => panic!("expected MoveNode command, got {other:?}"),
}
@ -238,6 +242,28 @@ fn batch_design_rejects_malformed_json() {
}
}
#[test]
fn batch_design_accepts_single_move_operation_with_index() {
let tool = batch_design_snapshot();
let mut args = BTreeMap::new();
args.insert("operations".into(), r##"M("n14", "n10", 2)"##.into());
match tool.call(&args) {
ToolOutcome::OkWithCommand(
_,
EditorCommand::MoveNode {
target_parent,
index,
..
},
) => {
assert_eq!(target_parent.as_str(), "n10");
assert_eq!(index, Some(2));
}
other => panic!("expected indexed MoveNode command, got {other:?}"),
}
}
#[test]
fn batch_insert_command_adds_all_nodes() {
let mut s = sample();

18
crates/op-mcp/src/batch_direct_ops.rs
View file

@ -78,12 +78,22 @@ fn parse_move_operation(body: &str) -> Result<EditorCommand, String> {
return Err("M() requires node id and parent id".into());
};
let rest = body[comma + 1..].trim();
if find_top_level_char(rest, ',').is_some() {
return Err("M() index argument is not supported by the Rust MoveNode command".into());
}
let (parent_raw, index) = if let Some(index_comma) = find_top_level_char(rest, ',') {
let raw_index = rest[index_comma + 1..]
.trim()
.trim_matches(|c| c == '"' || c == '\'');
let index = raw_index
.parse::<usize>()
.map_err(|_| format!("M() index must be a non-negative integer, got {raw_index:?}"))?;
(rest[..index_comma].trim(), Some(index))
} else {
(rest, None)
};
Ok(EditorCommand::MoveNode {
node_id: NodeId::new(&parse_ref_token(body[..comma].trim())?),
target_parent: parse_parent_node_id(rest)?,
target_parent: parse_parent_node_id(parent_raw)?,
page_id: None,
index,
})
}

31
crates/op-mcp/src/copy_node_tests.rs
View file

@ -5,8 +5,8 @@
//! plus end-to-end `EditorState::apply` checks; the apply-path
//! correctness is covered by `op-editor-core`'s `command_tests.rs`.
use super::reparent_tools::*;
use super::test_fixtures::sample;
use super::write_tools::*;
use super::{EditorCommand, McpTool, ToolErrorCode, ToolOutcome};
use op_editor_core::NodeId;
use std::collections::BTreeMap;
@ -36,10 +36,12 @@ fn copy_node_validates_args() {
EditorCommand::CopyNode {
node_id,
target_parent,
page_id,
},
) => {
assert_eq!(node_id.as_str(), "n10");
assert_eq!(target_parent.as_str(), "n10");
assert!(page_id.is_none());
}
other => panic!("expected CopyNode, got {other:?}"),
}
@ -59,6 +61,30 @@ fn copy_node_maps_zero_target_to_page_root() {
}
}
#[test]
fn copy_node_accepts_ts_source_parent_and_page_args() {
let tool = copy_node_snapshot();
let mut args = BTreeMap::new();
args.insert("sourceId".into(), "n12".into());
args.insert("parent".into(), "n10".into());
args.insert("pageId".into(), "page-2".into());
match tool.call(&args) {
ToolOutcome::OkWithCommand(
_,
EditorCommand::CopyNode {
node_id,
target_parent,
page_id,
},
) => {
assert_eq!(node_id.as_str(), "n12");
assert_eq!(target_parent.as_str(), "n10");
assert_eq!(page_id.as_deref(), Some("page-2"));
}
other => panic!("expected CopyNode command with TS args, got {other:?}"),
}
}
#[test]
fn copy_node_command_clones_to_page_root() {
let mut s = sample();
@ -66,6 +92,7 @@ fn copy_node_command_clones_to_page_root() {
assert!(s.apply(EditorCommand::CopyNode {
node_id: NodeId::new("n12"),
target_parent: NodeId::NONE,
page_id: None,
}));
assert_eq!(s.active_children().len(), pre_root_len + 1);
}
@ -76,9 +103,11 @@ fn copy_node_command_rejects_unknown_source_or_target() {
assert!(!s.apply(EditorCommand::CopyNode {
node_id: NodeId::new("n99999"),
target_parent: NodeId::NONE,
page_id: None,
}));
assert!(!s.apply(EditorCommand::CopyNode {
node_id: NodeId::new("n11"),
target_parent: NodeId::new("n99999"),
page_id: None,
}));
}

9
crates/op-mcp/src/lib.rs
View file

@ -63,6 +63,7 @@ pub mod read_nodes;
#[cfg(test)]
mod read_nodes_tests;
pub mod read_tools;
pub mod reparent_tools;
#[cfg(test)]
mod replace_node_tests;
pub mod scalar_vars;
@ -157,6 +158,7 @@ pub use page_tools::{
};
pub use parser::parse_tool_call;
pub use read_nodes::{read_nodes_snapshot, ReadNodes};
pub use reparent_tools::{copy_node_snapshot, move_node_snapshot, CopyNode, MoveNode};
pub use scalar_vars::{
create_variable_snapshot, delete_variable_snapshot, rename_variable_snapshot,
set_variable_boolean_snapshot, set_variable_number_snapshot, set_variable_string_snapshot,
@ -194,10 +196,9 @@ pub use tools::{
ListVariables, NodeRecord, SnapshotLayout, VariableRecord,
};
pub use write_tools::{
copy_node_snapshot, delete_node_snapshot, import_svg_snapshot, insert_node_snapshot,
move_node_snapshot, replace_node_snapshot, set_active_axis_value_snapshot,
set_variable_color_snapshot, update_node_snapshot, CopyNode, DeleteNode, ImportSvg, InsertNode,
MoveNode, ReplaceNode, SetActiveAxisValue, SetVariableColor, UpdateNode,
delete_node_snapshot, import_svg_snapshot, insert_node_snapshot, replace_node_snapshot,
set_active_axis_value_snapshot, set_variable_color_snapshot, update_node_snapshot, DeleteNode,
ImportSvg, InsertNode, ReplaceNode, SetActiveAxisValue, SetVariableColor, UpdateNode,
};
/// JSON-RPC-style request id. Strings + integers both supported by

153
crates/op-mcp/src/reparent_tools.rs Normal file
View file

@ -0,0 +1,153 @@
//! MCP move/copy tools.
use std::collections::BTreeMap;
use op_editor_core::NodeId;
use crate::write_tools::root_or_node_id;
use crate::{EditorCommand, McpTool, ToolErrorCode, ToolOutcome};
/// First-party `move_node` tool — reparent a node. An empty
/// `target_parent_id` reparents to the active page root.
pub struct MoveNode;
impl McpTool for MoveNode {
fn name(&self) -> &str {
"move_node"
}
fn call(&self, args: &BTreeMap<String, String>) -> ToolOutcome {
let node_id = match parse_node_id_any(args, &["node_id", "nodeId"], "node_id") {
Ok(v) => v,
Err(e) => return e,
};
let target_parent = match parse_target_parent_arg(args) {
Ok(v) => v,
Err(e) => return e,
};
if target_parent.is_real() && target_parent == node_id {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
"node_id and target_parent_id must differ".into(),
);
}
let index = match parse_optional_usize_arg(args, "index") {
Ok(v) => v,
Err(e) => return e,
};
let page_id = optional_page_id(args);
let mut out = BTreeMap::new();
out.insert("wrote".into(), "true".into());
ToolOutcome::OkWithCommand(
out,
EditorCommand::MoveNode {
node_id,
target_parent,
page_id,
index,
},
)
}
}
pub fn move_node_snapshot() -> MoveNode {
MoveNode
}
/// First-party `copy_node` tool — deep-clone a node + subtree under a
/// new parent.
pub struct CopyNode;
impl McpTool for CopyNode {
fn name(&self) -> &str {
"copy_node"
}
fn call(&self, args: &BTreeMap<String, String>) -> ToolOutcome {
let node_id = match parse_node_id_any(args, &["sourceId", "node_id", "nodeId"], "node_id") {
Ok(v) => v,
Err(e) => return e,
};
let target_parent = match parse_target_parent_arg(args) {
Ok(v) => v,
Err(e) => return e,
};
let page_id = optional_page_id(args);
let mut out = BTreeMap::new();
out.insert("wrote".into(), "true".into());
ToolOutcome::OkWithCommand(
out,
EditorCommand::CopyNode {
node_id,
target_parent,
page_id,
},
)
}
}
pub fn copy_node_snapshot() -> CopyNode {
CopyNode
}
#[allow(clippy::result_large_err)]
fn parse_node_id_any(
args: &BTreeMap<String, String>,
keys: &[&str],
message_key: &str,
) -> Result<NodeId, ToolOutcome> {
for key in keys {
if let Some(raw) = args.get(*key) {
return NodeId::new_opt(raw.as_str()).ok_or_else(|| {
ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!("{key} must be a non-empty node id"),
)
});
}
}
Err(ToolOutcome::Err(
ToolErrorCode::MissingArgument,
format!("{message_key} is required"),
))
}
#[allow(clippy::result_large_err)]
fn parse_target_parent_arg(args: &BTreeMap<String, String>) -> Result<NodeId, ToolOutcome> {
let Some(raw_target) = args
.get("parent")
.or_else(|| args.get("parent_id"))
.or_else(|| args.get("target_parent_id"))
else {
return Err(ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"target_parent_id is required (\"\" or \"0\" = page root)".into(),
));
};
Ok(root_or_node_id(raw_target))
}
#[allow(clippy::result_large_err)]
fn parse_optional_usize_arg(
args: &BTreeMap<String, String>,
key: &str,
) -> Result<Option<usize>, ToolOutcome> {
let Some(raw) = args.get(key) else {
return Ok(None);
};
raw.parse::<usize>().map(Some).map_err(|_| {
ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
format!("{key} must be a non-negative integer, got {raw:?}"),
)
})
}
fn optional_page_id(args: &BTreeMap<String, String>) -> Option<String> {
args.get("pageId")
.or_else(|| args.get("page_id"))
.or_else(|| args.get("page"))
.map(|s| s.trim())
.filter(|s| !s.is_empty())
.map(str::to_string)
}

82
crates/op-mcp/src/write_tools.rs
View file

@ -459,56 +459,12 @@ pub fn delete_node_snapshot() -> DeleteNode {
DeleteNode
}
/// First-party `move_node` tool — reparent a node. An empty
/// `target_parent_id` reparents to the active page root.
pub struct MoveNode;
impl McpTool for MoveNode {
fn name(&self) -> &str {
"move_node"
}
fn call(&self, args: &BTreeMap<String, String>) -> ToolOutcome {
let node_id = match parse_node_id(args, "node_id") {
Ok(v) => v,
Err(e) => return e,
};
// `target_parent_id` is required; an empty string ("" or "0")
// means "the active page root" (the NONE sentinel).
let Some(raw_target) = args.get("target_parent_id") else {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"target_parent_id is required (\"\" or \"0\" = page root)".into(),
);
};
let target_parent = root_or_node_id(raw_target);
if target_parent.is_real() && target_parent == node_id {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
"node_id and target_parent_id must differ".into(),
);
}
let mut out = BTreeMap::new();
out.insert("wrote".into(), "true".into());
ToolOutcome::OkWithCommand(
out,
EditorCommand::MoveNode {
node_id,
target_parent,
},
)
}
}
pub fn move_node_snapshot() -> MoveNode {
MoveNode
}
/// Resolve a `target_parent_id`-style arg. The legacy wire used `"0"`
/// for "page root"; the canonical model uses the empty `NodeId::NONE`
/// sentinel. Both `""` and `"0"` map to `NONE` so older clients keep
/// working. `"root"` is also accepted because the generated tool
/// schema uses that wording for page-root inserts.
fn root_or_node_id(raw: &str) -> NodeId {
pub(super) fn root_or_node_id(raw: &str) -> NodeId {
let trimmed = raw.trim();
if trimmed.is_empty()
|| trimmed == "0"
@ -521,42 +477,6 @@ fn root_or_node_id(raw: &str) -> NodeId {
}
}
/// First-party `copy_node` tool — deep-clone a node + subtree under a
/// new parent.
pub struct CopyNode;
impl McpTool for CopyNode {
fn name(&self) -> &str {
"copy_node"
}
fn call(&self, args: &BTreeMap<String, String>) -> ToolOutcome {
let node_id = match parse_node_id(args, "node_id") {
Ok(v) => v,
Err(e) => return e,
};
let Some(raw_target) = args.get("target_parent_id") else {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"target_parent_id is required (\"\" or \"0\" = page root)".into(),
);
};
let target_parent = root_or_node_id(raw_target);
let mut out = BTreeMap::new();
out.insert("wrote".into(), "true".into());
ToolOutcome::OkWithCommand(
out,
EditorCommand::CopyNode {
node_id,
target_parent,
},
)
}
}
pub fn copy_node_snapshot() -> CopyNode {
CopyNode
}
/// First-party `replace_node` tool — swap an existing node for a
/// freshly-built one at the same parent slot.
pub struct ReplaceNode;

34
crates/op-mcp/src/write_tools_tests.rs
View file

@ -7,6 +7,7 @@
//! apply-path correctness itself is exhaustively covered by
//! `op-editor-core`'s own `command_tests.rs`.
use super::reparent_tools::move_node_snapshot;
use super::test_fixtures::{add_theme_axis, add_variable, sample, state_with};
use super::write_tools::*;
use super::{EditorCommand, McpTool, ToolErrorCode, ToolOutcome};
@ -615,15 +616,46 @@ fn move_node_returns_command_with_empty_target_for_page_root() {
EditorCommand::MoveNode {
node_id,
target_parent,
page_id,
index,
},
) => {
assert_eq!(node_id.as_str(), "n11");
assert!(!target_parent.is_real(), "0 maps to the page-root NONE");
assert!(page_id.is_none());
assert!(index.is_none());
}
other => panic!("expected MoveNode, got {other:?}"),
}
}
#[test]
fn move_node_accepts_ts_node_parent_page_and_index_args() {
let tool = move_node_snapshot();
let mut args = BTreeMap::new();
args.insert("nodeId".into(), "n11".into());
args.insert("parent".into(), "n10".into());
args.insert("pageId".into(), "page-2".into());
args.insert("index".into(), "2".into());
match tool.call(&args) {
ToolOutcome::OkWithCommand(
_,
EditorCommand::MoveNode {
node_id,
target_parent,
page_id,
index,
},
) => {
assert_eq!(node_id.as_str(), "n11");
assert_eq!(target_parent.as_str(), "n10");
assert_eq!(page_id.as_deref(), Some("page-2"));
assert_eq!(index, Some(2));
}
other => panic!("expected MoveNode command with TS args, got {other:?}"),
}
}
#[test]
fn move_node_command_applies_through_editor_state() {
let mut s = sample();
@ -631,6 +663,8 @@ fn move_node_command_applies_through_editor_state() {
assert!(s.apply(EditorCommand::MoveNode {
node_id: op_editor_core::NodeId::new("n11"),
target_parent: op_editor_core::NodeId::NONE,
page_id: None,
index: None,
}));
assert!(s
.active_children()

2
crates/op-orchestrator/src/dashboard_columns/height_reorder.rs
View file

@ -167,6 +167,8 @@ pub(crate) fn reorder_dashboard_main_children(
.map(|id| EditorCommand::MoveNode {
node_id: NodeId::new(id),
target_parent: main_node_id.clone(),
page_id: None,
index: None,
})
.collect()
}