diff --git a/crates/openpencil-desktop/src/mcp_serve.rs b/crates/openpencil-desktop/src/mcp_serve.rs index cfd80752c..476921ca3 100644 --- a/crates/openpencil-desktop/src/mcp_serve.rs +++ b/crates/openpencil-desktop/src/mcp_serve.rs @@ -27,8 +27,9 @@ use openpencil_shell_core::mcp::{ batch_design_snapshot, copy_node_snapshot, delete_node_snapshot, design_content_snapshot, design_refine_snapshot, design_skeleton_snapshot, add_page_snapshot, create_component_snapshot, delete_component_snapshot, - document_info_snapshot, get_component_snapshot, instantiate_component_snapshot, - list_components_snapshot, rename_component_snapshot, set_active_page_snapshot, + delete_page_snapshot, document_info_snapshot, duplicate_page_snapshot, + get_component_snapshot, instantiate_component_snapshot, list_components_snapshot, + rename_component_snapshot, rename_page_snapshot, set_active_page_snapshot, get_active_theme_snapshot, get_node_snapshot, insert_node_snapshot, list_pages_snapshot, list_variables_snapshot, move_node_snapshot, replace_node_snapshot, run_stdio_with_applier, selection_snapshot, @@ -164,6 +165,9 @@ fn rebuild_registry(doc: &Document) -> ToolRegistry { r.register(Box::new(rename_component_snapshot())); r.register(Box::new(set_active_page_snapshot())); r.register(Box::new(add_page_snapshot())); + r.register(Box::new(rename_page_snapshot())); + r.register(Box::new(delete_page_snapshot())); + r.register(Box::new(duplicate_page_snapshot())); r } @@ -393,6 +397,9 @@ const TOOL_SCHEMAS: &[&str] = &[ r#"{"name":"rename_component","description":"Rename a registered component. Name must be non-empty / non-whitespace.","inputSchema":{"type":"object","properties":{"component_id":{"type":"string","description":"positive u64 component id"},"name":{"type":"string"}},"required":["component_id","name"]}}"#, r#"{"name":"set_active_page","description":"Switch which page is the active target for subsequent inserts / batch_design / design_* commands. index is 0-based.","inputSchema":{"type":"object","properties":{"index":{"type":"string","description":"0-based page index"}},"required":["index"]}}"#, r#"{"name":"add_page","description":"Append a fresh empty page and switch the active page to it. Returns false on id-space exhaustion.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, + r#"{"name":"rename_page","description":"Set a page's display name. Name must be non-empty / non-whitespace.","inputSchema":{"type":"object","properties":{"index":{"type":"string","description":"0-based page index"},"name":{"type":"string"}},"required":["index","name"]}}"#, + r#"{"name":"delete_page","description":"Remove a page by index. The applier keeps the active page valid (clamps if needed).","inputSchema":{"type":"object","properties":{"index":{"type":"string","description":"0-based page index"}},"required":["index"]}}"#, + r#"{"name":"duplicate_page","description":"Clone the page at index and switch the active page to the clone.","inputSchema":{"type":"object","properties":{"index":{"type":"string","description":"0-based page index"}},"required":["index"]}}"#, 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":"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"]}}"#, @@ -446,7 +453,7 @@ mod tests { } #[test] - fn tools_list_response_includes_all_twenty_nine_tools() { + fn tools_list_response_includes_all_thirty_two_tools() { let r = tools_list_response("3"); // Exact-count assertion: any tool added without // updating this test will trip the count first. Codex @@ -455,7 +462,7 @@ mod tests { // without being added to the list below. assert_eq!( TOOL_SCHEMAS.len(), - 29, + 32, "tools/list catalog count must match the registered tools — add the new tool to this test" ); for name in [ @@ -473,6 +480,9 @@ mod tests { "rename_component", "set_active_page", "add_page", + "rename_page", + "delete_page", + "duplicate_page", "set_variable_color", "set_active_axis_value", "insert_node", diff --git a/crates/openpencil-shell-core/src/document/mcp_apply.rs b/crates/openpencil-shell-core/src/document/mcp_apply.rs index 2213e5c0d..fa0e8ec73 100644 --- a/crates/openpencil-shell-core/src/document/mcp_apply.rs +++ b/crates/openpencil-shell-core/src/document/mcp_apply.rs @@ -510,6 +510,15 @@ impl Document { self.set_active_page(*index as usize) } crate::mcp::McpCommand::AddPage => self.add_page().is_some(), + crate::mcp::McpCommand::RenamePage { index, name } => { + self.rename_page(*index as usize, name.clone()) + } + crate::mcp::McpCommand::DeletePage { index } => { + self.remove_page(*index as usize) + } + crate::mcp::McpCommand::DuplicatePage { index } => { + self.duplicate_page(*index as usize).is_some() + } crate::mcp::McpCommand::BatchInsert { items } => { // Validate EVERY descriptor before any mutation. // A single bad entry rejects the entire batch so diff --git a/crates/openpencil-shell-core/src/document/page_mutators.rs b/crates/openpencil-shell-core/src/document/page_mutators.rs index bc79379ce..3f4333e34 100644 --- a/crates/openpencil-shell-core/src/document/page_mutators.rs +++ b/crates/openpencil-shell-core/src/document/page_mutators.rs @@ -88,6 +88,23 @@ impl Document { true } + /// Set a page's name directly (MCP-friendly entry point — + /// the UI rename flow uses `start_rename_page` + + /// `rename_append`). Rejects out-of-range indices and empty + /// / whitespace-only names so list_pages never returns a + /// page label a user can't recognize. + pub fn rename_page(&mut self, idx: usize, name: impl Into) -> bool { + let name: String = name.into(); + if name.trim().is_empty() { + return false; + } + let Some(page) = self.pages.get_mut(idx) else { + return false; + }; + page.name = name; + true + } + /// Start inline rename on a page row. pub fn start_rename_page(&mut self, idx: usize) -> bool { let Some(page) = self.pages.get(idx) else { diff --git a/crates/openpencil-shell-core/src/document/variables.rs b/crates/openpencil-shell-core/src/document/variables.rs index f600c2bfb..f857e4550 100644 --- a/crates/openpencil-shell-core/src/document/variables.rs +++ b/crates/openpencil-shell-core/src/document/variables.rs @@ -199,7 +199,10 @@ impl VariableTable { | crate::mcp::McpCommand::DeleteComponent { .. } | crate::mcp::McpCommand::RenameComponent { .. } | crate::mcp::McpCommand::SetActivePage { .. } - | crate::mcp::McpCommand::AddPage => { + | crate::mcp::McpCommand::AddPage + | crate::mcp::McpCommand::RenamePage { .. } + | crate::mcp::McpCommand::DeletePage { .. } + | crate::mcp::McpCommand::DuplicatePage { .. } => { // Not VariableTable mutations — Pages-level commands // live on `Document::apply_mcp_command`. Return false // so callers with only a VariableTable handle know diff --git a/crates/openpencil-shell-core/src/mcp.rs b/crates/openpencil-shell-core/src/mcp.rs index 49039f061..5e351aca6 100644 --- a/crates/openpencil-shell-core/src/mcp.rs +++ b/crates/openpencil-shell-core/src/mcp.rs @@ -40,9 +40,10 @@ pub use write_tools::{ }; pub use component_tools::{ add_page_snapshot, create_component_snapshot, delete_component_snapshot, - instantiate_component_snapshot, rename_component_snapshot, set_active_page_snapshot, - AddPage, CreateComponent, DeleteComponent, InstantiateComponent, RenameComponent, - SetActivePage, + delete_page_snapshot, duplicate_page_snapshot, instantiate_component_snapshot, + rename_component_snapshot, rename_page_snapshot, set_active_page_snapshot, AddPage, + CreateComponent, DeleteComponent, DeletePage, DuplicatePage, InstantiateComponent, + RenameComponent, RenamePage, SetActivePage, }; pub use batch_design::{ batch_design_snapshot, design_content_snapshot, design_refine_snapshot, @@ -299,6 +300,23 @@ pub enum McpCommand { /// id-space exhaustion (same guard as the rest of the write /// tools). AddPage, + /// Set a page's display name. Rejects out-of-range indices + /// and empty / whitespace-only names. + RenamePage { + index: u32, + name: String, + }, + /// Remove a page by index. The applier reuses the existing + /// `Document::remove_page` which keeps the active page valid. + DeletePage { + index: u32, + }, + /// Duplicate the page at `index` (clone + insert right after). + /// Switches active page to the clone. Mirrors TS + /// `duplicatePage(idx)`. + DuplicatePage { + index: u32, + }, } /// Wire-friendly value payload for `McpCommand::SetVariableScalar`. diff --git a/crates/openpencil-shell-core/src/mcp/component_tools.rs b/crates/openpencil-shell-core/src/mcp/component_tools.rs index 93b51bdcb..dcd8263d0 100644 --- a/crates/openpencil-shell-core/src/mcp/component_tools.rs +++ b/crates/openpencil-shell-core/src/mcp/component_tools.rs @@ -239,3 +239,122 @@ pub fn add_page_snapshot() -> AddPage { AddPage } +/// First-party `rename_page` tool — set a page's display name. +/// Rejects out-of-range indices + empty / whitespace-only names. +pub struct RenamePage; + +impl McpTool for RenamePage { + fn name(&self) -> &str { + "rename_page" + } + fn call(&self, args: &BTreeMap) -> ToolOutcome { + let Some(raw) = args.get("index") else { + return ToolOutcome::Err( + ToolErrorCode::MissingArgument, + "index is required (0-based page index)".into(), + ); + }; + let index: u32 = match raw.parse() { + Ok(n) => n, + _ => { + return ToolOutcome::Err( + ToolErrorCode::InvalidArgument, + format!("index must be a u32, got {raw:?}"), + ); + } + }; + let Some(name) = args.get("name") else { + return ToolOutcome::Err( + ToolErrorCode::MissingArgument, + "name is required".into(), + ); + }; + if name.trim().is_empty() { + return ToolOutcome::Err( + ToolErrorCode::InvalidArgument, + "name must not be empty / whitespace-only".into(), + ); + } + let mut out = BTreeMap::new(); + out.insert("wrote".into(), "true".into()); + ToolOutcome::OkWithCommand( + out, + McpCommand::RenamePage { + index, + name: name.clone(), + }, + ) + } +} + +pub fn rename_page_snapshot() -> RenamePage { + RenamePage +} + +/// First-party `delete_page` tool — remove a page by index. +pub struct DeletePage; + +impl McpTool for DeletePage { + fn name(&self) -> &str { + "delete_page" + } + fn call(&self, args: &BTreeMap) -> ToolOutcome { + let Some(raw) = args.get("index") else { + return ToolOutcome::Err( + ToolErrorCode::MissingArgument, + "index is required (0-based page index)".into(), + ); + }; + let index: u32 = match raw.parse() { + Ok(n) => n, + _ => { + return ToolOutcome::Err( + ToolErrorCode::InvalidArgument, + format!("index must be a u32, got {raw:?}"), + ); + } + }; + let mut out = BTreeMap::new(); + out.insert("wrote".into(), "true".into()); + ToolOutcome::OkWithCommand(out, McpCommand::DeletePage { index }) + } +} + +pub fn delete_page_snapshot() -> DeletePage { + DeletePage +} + +/// First-party `duplicate_page` tool — clone the page at `index` +/// and switch the active page to the clone. +pub struct DuplicatePage; + +impl McpTool for DuplicatePage { + fn name(&self) -> &str { + "duplicate_page" + } + fn call(&self, args: &BTreeMap) -> ToolOutcome { + let Some(raw) = args.get("index") else { + return ToolOutcome::Err( + ToolErrorCode::MissingArgument, + "index is required (0-based page index)".into(), + ); + }; + let index: u32 = match raw.parse() { + Ok(n) => n, + _ => { + return ToolOutcome::Err( + ToolErrorCode::InvalidArgument, + format!("index must be a u32, got {raw:?}"), + ); + } + }; + let mut out = BTreeMap::new(); + out.insert("wrote".into(), "true".into()); + ToolOutcome::OkWithCommand(out, McpCommand::DuplicatePage { index }) + } +} + +pub fn duplicate_page_snapshot() -> DuplicatePage { + DuplicatePage +} +