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

fix(mcp): support insert parent and page targets

Browse source
This commit is contained in:
Kayshen-X 2026-06-02 09:41:57 +08:00
parent bbb26b86e8
commit 79a2d0830c
13 changed files with 303 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
crates/op-cli/src/cli_node_tests.rs Normal file
View file

@ -0,0 +1,33 @@
use super::*;
#[test]
fn parse_args_maps_insert_parent_and_page_flags_to_ts_args() {
let p = parse_args(&[
"insert".to_string(),
r##"{"kind":"rect","name":"Child","x":1,"y":2,"width":3,"height":4,"fill":"#fff"}"##
.to_string(),
"--parent".to_string(),
"n10".to_string(),
"--page".to_string(),
"page-2".to_string(),
])
.expect("parse insert");
assert_eq!(
p.command,
Command::ToolCall {
tool: "insert_node".to_string(),
args: vec![
("kind".to_string(), "rect".to_string()),
("name".to_string(), "Child".to_string()),
("x".to_string(), "1".to_string()),
("y".to_string(), "2".to_string()),
("width".to_string(), "3".to_string()),
("height".to_string(), "4".to_string()),
("fill_hex".to_string(), "#fff".to_string()),
("parent".to_string(), "n10".to_string()),
("pageId".to_string(), "page-2".to_string()),
],
}
);
}

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

@ -232,7 +232,7 @@ fn command_from_positionals(positionals: &[String], flags: &Flags) -> Result<Com
}
"get" => map_get(flags),
"selection" => tool_call("get_selection", vec![]),
"insert" => map_insert(positionals),
"insert" => map_insert(positionals, flags),
"update" => map_update(positionals),
"delete" => map_one_id("delete_node", positionals),
"read-nodes" => map_read_nodes(positionals, flags),
@ -292,9 +292,16 @@ fn map_get(flags: &Flags) -> Result<Command, String> {
tool_call("batch_get", pairs)
}
fn map_insert(positionals: &[String]) -> Result<Command, String> {
fn map_insert(positionals: &[String], flags: &Flags) -> Result<Command, String> {
let raw = resolve_arg(positionals.get(1).map(String::as_str))?;
tool_call("insert_node", insert_pairs(&raw)?)
let mut pairs = insert_pairs(&raw)?;
if let Some(parent) = flag_value(flags, "parent") {
pairs.push(pair("parent", parent));
}
if let Some(page) = flag_value(flags, "page") {
pairs.push(pair("pageId", page));
}
tool_call("insert_node", pairs)
}
fn map_update(positionals: &[String]) -> Result<Command, String> {
@ -723,4 +730,6 @@ fn version_json() -> String {
#[cfg(test)]
mod cli_import_tests;
#[cfg(test)]
mod cli_node_tests;
#[cfg(test)]
mod tests;

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

@ -14,7 +14,8 @@ COMMON COMMANDS:
op status check whether MCP HTTP is reachable
op get [--type T] [--name N] [--id ID] [--depth N] [--parent P]
op selection get current selection
op insert <json|@file|-> insert a leaf node
op insert <json|@file|-> [--parent P] [--page PAGE]
insert a leaf node
op update <id> <json|@file|-> patch x/y/width/height/name/fill
op delete <id> delete a node
op read-nodes [ids] [--depth N] [--vars] [--page P] [--file F]

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

@ -126,6 +126,11 @@ pub enum EditorCommand {
width: i32,
height: i32,
fill_hex: Option<String>,
/// `NodeId::NONE` inserts at the active/requested page root.
target_parent: NodeId,
/// `None` inserts on the active page; `Some` targets a page id
/// or legacy page index.
page_id: Option<String>,
},
/// Patch fields on an existing node — every field optional.
UpdateNode {

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

@ -70,7 +70,7 @@ fn parse_variable_kind(s: &str) -> Option<VariableKind> {
}
}
fn import_svg_page_index(state: &EditorState, page_id: Option<&str>) -> Option<usize> {
fn command_page_index(state: &EditorState, page_id: Option<&str>) -> Option<usize> {
let Some(raw) = page_id.map(str::trim).filter(|s| !s.is_empty()) else {
return Some(
state
@ -88,6 +88,21 @@ fn import_svg_page_index(state: &EditorState, page_id: Option<&str>) -> Option<u
}
}
#[allow(clippy::too_many_arguments)]
fn apply_insert_node_on_active_page(
state: &mut EditorState,
kind: &str,
name: &str,
x: i32,
y: i32,
width: i32,
height: i32,
fill_hex: &Option<String>,
target_parent: &NodeId,
) -> bool {
state.cmd_insert_node(kind, name, x, y, width, height, fill_hex, target_parent)
}
fn apply_import_svg_on_active_page(
state: &mut EditorState,
svg: &str,
@ -139,7 +154,32 @@ impl EditorState {
width,
height,
fill_hex,
} => self.cmd_insert_node(&kind, &name, x, y, width, height, &fill_hex),
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 = apply_insert_node_on_active_page(
self,
&kind,
&name,
x,
y,
width,
height,
&fill_hex,
&target_parent,
);
if page_id.is_some() && target_page_index != original_page_index {
self.ui.active_page_index = original_page_index;
}
changed
}
EditorCommand::UpdateNode {
node_id,
x,
@ -453,8 +493,7 @@ impl EditorState {
target_parent,
page_id,
} => {
let Some(target_page_index) = import_svg_page_index(self, page_id.as_deref())
else {
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;

129
crates/op-editor-core/src/command_insert_tests.rs Normal file
View file

@ -0,0 +1,129 @@
//! `EditorCommand::InsertNode` parent/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, state_with};
use jian_ops_schema::page::PenPage;
#[test]
fn insert_node_appends_a_fresh_leaf() {
let mut s = state_with(vec![]);
assert!(s.apply(EditorCommand::InsertNode {
kind: "rect".into(),
name: "Box".into(),
x: 10,
y: 20,
width: 100,
height: 60,
fill_hex: Some("#ff0000".into()),
target_parent: NodeId::NONE,
page_id: None,
}));
assert_eq!(s.active_children().len(), 1);
let node = &s.active_children()[0];
assert_eq!(node.base().name.as_deref(), Some("Box"));
assert_eq!(node.base().x, Some(10.0));
}
#[test]
fn insert_node_rejects_unknown_kind() {
let mut s = state_with(vec![]);
assert!(!s.apply(EditorCommand::InsertNode {
kind: "bogus".into(),
name: "X".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: None,
target_parent: NodeId::NONE,
page_id: None,
}));
assert!(s.active_children().is_empty());
}
#[test]
fn insert_node_atomic_on_bad_fill_hex() {
let mut s = state_with(vec![]);
assert!(!s.apply(EditorCommand::InsertNode {
kind: "rect".into(),
name: "X".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: Some("not-hex".into()),
target_parent: NodeId::NONE,
page_id: None,
}));
assert!(s.active_children().is_empty());
assert_eq!(s.max_node_id(), 0);
}
#[test]
fn insert_node_can_insert_under_parent_container() {
let mut s = state_with(vec![frame("n1", "Frame", 0.0, 0.0, 100.0, 100.0, vec![])]);
assert!(s.apply(EditorCommand::InsertNode {
kind: "rect".into(),
name: "Child".into(),
x: 1,
y: 2,
width: 3,
height: 4,
fill_hex: None,
target_parent: NodeId::new("n1"),
page_id: None,
}));
assert_eq!(s.active_children().len(), 1);
let children = s.active_children()[0].children().expect("parent children");
assert_eq!(children.len(), 1);
assert_eq!(children[0].base().name.as_deref(), Some("Child"));
}
#[test]
fn insert_node_can_insert_on_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![frame("n1", "Current", 0.0, 0.0, 100.0, 100.0, vec![])],
state: None,
lifecycle: None,
},
PenPage {
id: "page-2".into(),
name: "Page 2".into(),
children: Vec::new(),
state: None,
lifecycle: None,
},
]);
s.ui.active_page_index = 0;
assert!(s.apply(EditorCommand::InsertNode {
kind: "rect".into(),
name: "Other Page".into(),
x: 1,
y: 2,
width: 3,
height: 4,
fill_hex: None,
target_parent: NodeId::NONE,
page_id: Some("page-2".into()),
}));
let pages = s.doc.pages.as_ref().expect("pages");
assert_eq!(pages[0].children.len(), 1);
assert_eq!(pages[1].children.len(), 1);
assert_eq!(
pages[1].children[0].base().name.as_deref(),
Some("Other Page")
);
assert_eq!(s.ui.active_page_index, 0);
}

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

@ -264,6 +264,7 @@ impl EditorState {
width: i32,
height: i32,
fill_hex: &Option<String>,
target_parent: &NodeId,
) -> bool {
if !kind_is_valid(kind) || width < 0 || height < 0 {
return false;
@ -274,6 +275,12 @@ impl EditorState {
return false;
}
}
if target_parent.is_real() {
match walkers::find_node(self.active_children(), target_parent) {
Some(parent) if parent.is_container() => {}
_ => return false,
}
}
let Some(new_id) = self.next_node_id() else {
return false;
};
@ -284,7 +291,18 @@ impl EditorState {
if let Some(hex) = fill_hex {
set_primary_fill_hex(&mut node, hex);
}
self.active_children_mut().push(node);
if target_parent.is_real() {
let root = self.active_children_mut();
let Some(parent) = walkers::find_node_mut(root, target_parent) else {
return false;
};
let Some(children) = parent.children_mut() else {
return false;
};
children.push(node);
} else {
self.active_children_mut().push(node);
}
true
}

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

@ -22,58 +22,6 @@ fn id(s: &str) -> NodeId {
NodeId::new(s)
}
// --- InsertNode ------------------------------------------------------
#[test]
fn insert_node_appends_a_fresh_leaf() {
let mut s = state_with(vec![]);
assert!(s.apply(EditorCommand::InsertNode {
kind: "rect".into(),
name: "Box".into(),
x: 10,
y: 20,
width: 100,
height: 60,
fill_hex: Some("#ff0000".into()),
}));
assert_eq!(s.active_children().len(), 1);
let node = &s.active_children()[0];
assert_eq!(node.base().name.as_deref(), Some("Box"));
assert_eq!(node.base().x, Some(10.0));
}
#[test]
fn insert_node_rejects_unknown_kind() {
let mut s = state_with(vec![]);
assert!(!s.apply(EditorCommand::InsertNode {
kind: "bogus".into(),
name: "X".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: None,
}));
assert!(s.active_children().is_empty());
}
#[test]
fn insert_node_atomic_on_bad_fill_hex() {
let mut s = state_with(vec![]);
assert!(!s.apply(EditorCommand::InsertNode {
kind: "rect".into(),
name: "X".into(),
x: 0,
y: 0,
width: 10,
height: 10,
fill_hex: Some("not-hex".into()),
}));
// No id minted, no node appended.
assert!(s.active_children().is_empty());
assert_eq!(s.max_node_id(), 0);
}
// --- UpdateNode ------------------------------------------------------
#[test]

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

@ -57,6 +57,8 @@ pub mod walkers;
#[cfg(test)]
mod command_attr_tests;
#[cfg(test)]
mod command_insert_tests;
#[cfg(test)]
mod command_style_replace_tests;
#[cfg(test)]
mod command_subtree_tests;

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

@ -778,7 +778,7 @@ const TOOL_SCHEMAS: &[&str] = &[
r#"{"name":"duplicate_page","description":"Clone a page and switch the active page to the clone. Accepts TS-style pageId or legacy index; optional name overrides the clone name.","inputSchema":{"type":"object","properties":{"pageId":{"type":"string","description":"Page id returned by list_pages"},"index":{"type":"string","description":"0-based page index (legacy)"},"name":{"type":"string","description":"Optional clone name"}}}}"#,
r#"{"name":"reorder_page","description":"Move a page. Accepts TS-style pageId + index or legacy from + to. Target is clamped to [0, page_count).","inputSchema":{"type":"object","properties":{"pageId":{"type":"string","description":"Page id returned by list_pages"},"index":{"type":"string","description":"0-based target page index"},"from":{"type":"string","description":"0-based source page index (legacy)"},"to":{"type":"string","description":"0-based target page index (legacy)"}}}}"#,
r#"{"name":"set_active_axis_value","description":"Pin a theme axis to one of its allowed values.","inputSchema":{"type":"object","properties":{"axis":{"type":"string"},"value":{"type":"string"}},"required":["axis","value"]}}"#,
r#"{"name":"insert_node","description":"Create a new leaf node on the active page.","inputSchema":{"type":"object","properties":{"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"}},"required":["kind","name","x","y","width","height"]}}"#,
r#"{"name":"insert_node","description":"Create a new leaf node on the active or requested page.","inputSchema":{"type":"object","properties":{"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"},"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":["kind","name","x","y","width","height"]}}"#,
r#"{"name":"update_node","description":"Patch fields on an existing node. Pass any subset of x/y/width/height/name/fill_hex.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"},"x":{"type":"string"},"y":{"type":"string"},"width":{"type":"string"},"height":{"type":"string"},"name":{"type":"string"},"fill_hex":{"type":"string"}},"required":["node_id"]}}"#,
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.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string"}},"required":["node_id"]}}"#,

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

@ -225,6 +225,8 @@ fn find_empty_space_returns_padded_position_from_active_page_bounds() {
width: 100,
height: 50,
fill_hex: None,
target_parent: op_editor_core::NodeId::NONE,
page_id: None,
}));
assert!(state.apply(EditorCommand::InsertNode {
kind: "rect".into(),
@ -234,6 +236,8 @@ fn find_empty_space_returns_padded_position_from_active_page_bounds() {
width: 50,
height: 40,
fill_hex: None,
target_parent: op_editor_core::NodeId::NONE,
page_id: None,
}));
let line = r#"{"jsonrpc":"2.0","id":9,"method":"tools/call","params":{"name":"find_empty_space","arguments":{"direction":"right","width":"320","height":"240"}}}"#;
let response = process_message_with_applier(&mut state, line, |_, _| false)
@ -255,6 +259,8 @@ fn read_nodes_accepts_structured_ids_over_mcp() {
width: 100,
height: 50,
fill_hex: None,
target_parent: op_editor_core::NodeId::NONE,
page_id: None,
}));
let node_id = state.active_children()[0].base().id.clone();
let line = format!(

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

@ -188,6 +188,19 @@ impl McpTool for InsertNode {
}
Some(s) => Some(s.clone()),
};
let target_parent = args
.get("parent")
.or_else(|| args.get("parent_id"))
.or_else(|| args.get("target_parent_id"))
.map(|s| root_or_node_id(s))
.unwrap_or(NodeId::NONE);
let page_id = 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);
let mut out = BTreeMap::new();
out.insert("wrote".into(), "true".into());
ToolOutcome::OkWithCommand(
@ -200,6 +213,8 @@ impl McpTool for InsertNode {
width,
height,
fill_hex,
target_parent,
page_id,
},
)
}

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

@ -11,7 +11,7 @@ use super::test_fixtures::{add_theme_axis, add_variable, sample, state_with};
use super::write_tools::*;
use super::{EditorCommand, McpTool, ToolErrorCode, ToolOutcome};
use jian_ops_schema::variable::{VariableKind, VariableScalar};
use op_editor_core::EditorState;
use op_editor_core::{EditorState, NodeId};
use std::collections::BTreeMap;
fn state_with_color_var(name: &str, hex: &str) -> EditorState {
@ -297,6 +297,8 @@ fn insert_node_returns_command_with_parsed_args() {
width,
height,
fill_hex,
target_parent,
page_id,
},
) => {
assert_eq!(kind, "rect");
@ -306,11 +308,42 @@ fn insert_node_returns_command_with_parsed_args() {
assert_eq!(width, 100);
assert_eq!(height, 50);
assert_eq!(fill_hex.as_deref(), Some("#ff0000"));
assert!(!target_parent.is_real());
assert_eq!(page_id, None);
}
other => panic!("expected InsertNode command, got {other:?}"),
}
}
#[test]
fn insert_node_accepts_ts_parent_and_page_args() {
let tool = insert_node_snapshot();
let mut args = BTreeMap::new();
args.insert("kind".into(), "rect".into());
args.insert("name".into(), "Nested".into());
args.insert("x".into(), "1".into());
args.insert("y".into(), "2".into());
args.insert("width".into(), "3".into());
args.insert("height".into(), "4".into());
args.insert("parent".into(), "n10".into());
args.insert("pageId".into(), "page-2".into());
match tool.call(&args) {
ToolOutcome::OkWithCommand(
_,
EditorCommand::InsertNode {
target_parent,
page_id,
..
},
) => {
assert_eq!(target_parent.as_str(), "n10");
assert_eq!(page_id.as_deref(), Some("page-2"));
}
other => panic!("expected InsertNode command with parent/page, got {other:?}"),
}
}
#[test]
fn insert_node_command_applies_through_editor_state() {
let mut s = state_with(vec![]);
@ -323,6 +356,8 @@ fn insert_node_command_applies_through_editor_state() {
width: 200,
height: 150,
fill_hex: Some("#00ff00".into()),
target_parent: NodeId::NONE,
page_id: None,
}));
assert_eq!(s.active_children().len(), before + 1);
}