diff --git a/crates/op-host-desktop/src/mcp_serve.rs b/crates/op-host-desktop/src/mcp_serve.rs index 9b684412a..be7c1bbc9 100644 --- a/crates/op-host-desktop/src/mcp_serve.rs +++ b/crates/op-host-desktop/src/mcp_serve.rs @@ -33,31 +33,30 @@ use op_mcp::{ add_node_effect_snapshot, add_page_snapshot, align_selected_snapshot, batch_design_snapshot, clear_selection_snapshot, copy_node_snapshot, copy_selected_snapshot, count_nodes_snapshot, create_component_snapshot, create_variable_snapshot, cut_selected_snapshot, - cycle_active_axis_value_snapshot, debug_validation_report_snapshot, delete_component_snapshot, - delete_node_snapshot, - delete_page_snapshot, delete_selected_snapshot, delete_variable_snapshot, - design_content_snapshot, design_refine_snapshot, design_skeleton_snapshot, - document_info_snapshot, duplicate_page_snapshot, duplicate_selected_snapshot, - find_node_by_name_snapshot, get_active_theme_snapshot, get_canvas_bounds_snapshot, - get_component_snapshot, get_history_depth_snapshot, get_node_children_snapshot, - get_node_parent_snapshot, get_node_snapshot, get_selection_set_snapshot, get_viewport_snapshot, - group_selected_snapshot, import_svg_snapshot, insert_node_snapshot, - instantiate_component_snapshot, list_components_snapshot, list_node_kinds_snapshot, - list_pages_snapshot, list_variables_snapshot, move_node_snapshot, nudge_selected_snapshot, - paste_clipboard_snapshot, redo_snapshot, remove_node_effect_snapshot, - rename_component_snapshot, rename_page_snapshot, rename_variable_snapshot, - reorder_page_snapshot, reorder_selected_snapshot, replace_node_snapshot, - run_stdio_with_applier, selection_snapshot, set_active_axis_value_snapshot, - set_active_page_snapshot, set_active_tool_snapshot, set_ellipse_arc_snapshot, - set_node_collapsed_snapshot, set_node_corner_radius_snapshot, set_node_fill_hex_snapshot, - set_node_flip_snapshot, set_node_font_size_snapshot, set_node_font_weight_snapshot, - set_node_hidden_snapshot, set_node_locked_snapshot, set_node_name_snapshot, - set_node_rotation_snapshot, set_node_stroke_hex_snapshot, set_node_stroke_width_snapshot, - set_node_text_snapshot, set_selection_set_snapshot, set_selection_snapshot, - set_variable_boolean_snapshot, set_variable_color_snapshot, set_variable_number_snapshot, - set_variable_string_snapshot, set_viewport_snapshot, snapshot_layout_snapshot, - toggle_node_selection_snapshot, undo_snapshot, ungroup_selected_snapshot, update_node_snapshot, - ToolRegistry, + cycle_active_axis_value_snapshot, debug_tools_enabled, debug_validation_report_snapshot, + delete_component_snapshot, delete_node_snapshot, delete_page_snapshot, + delete_selected_snapshot, delete_variable_snapshot, design_content_snapshot, + design_refine_snapshot, design_skeleton_snapshot, document_info_snapshot, + duplicate_page_snapshot, duplicate_selected_snapshot, find_node_by_name_snapshot, + get_active_theme_snapshot, get_canvas_bounds_snapshot, get_component_snapshot, + get_history_depth_snapshot, get_node_children_snapshot, get_node_parent_snapshot, + get_node_snapshot, get_selection_set_snapshot, get_viewport_snapshot, group_selected_snapshot, + import_svg_snapshot, insert_node_snapshot, instantiate_component_snapshot, + list_components_snapshot, list_node_kinds_snapshot, list_pages_snapshot, + list_variables_snapshot, move_node_snapshot, nudge_selected_snapshot, paste_clipboard_snapshot, + redo_snapshot, remove_node_effect_snapshot, rename_component_snapshot, rename_page_snapshot, + rename_variable_snapshot, reorder_page_snapshot, reorder_selected_snapshot, + replace_node_snapshot, run_stdio_with_applier, selection_snapshot, + set_active_axis_value_snapshot, set_active_page_snapshot, set_active_tool_snapshot, + set_ellipse_arc_snapshot, set_node_collapsed_snapshot, set_node_corner_radius_snapshot, + set_node_fill_hex_snapshot, set_node_flip_snapshot, set_node_font_size_snapshot, + set_node_font_weight_snapshot, set_node_hidden_snapshot, set_node_locked_snapshot, + set_node_name_snapshot, set_node_rotation_snapshot, set_node_stroke_hex_snapshot, + set_node_stroke_width_snapshot, set_node_text_snapshot, set_selection_set_snapshot, + set_selection_snapshot, set_variable_boolean_snapshot, set_variable_color_snapshot, + set_variable_number_snapshot, set_variable_string_snapshot, set_viewport_snapshot, + snapshot_layout_snapshot, toggle_node_selection_snapshot, undo_snapshot, + ungroup_selected_snapshot, update_node_snapshot, ToolRegistry, }; /// Load a `.op` file into an `EditorState`. The `.op` format is plain @@ -331,7 +330,12 @@ fn rebuild_registry(doc: &EditorState) -> ToolRegistry { r.register(Box::new(get_history_depth_snapshot(doc))); r.register(Box::new(get_viewport_snapshot(doc))); r.register(Box::new(get_selection_set_snapshot(doc))); - r.register(Box::new(debug_validation_report_snapshot(doc))); + // Debug tools stay out of the production registry unless the + // isolation flag is set — matches the TS design where debug tools + // ship only in a debug build, never in the production catalog. + if debug_tools_enabled() { + r.register(Box::new(debug_validation_report_snapshot(doc))); + } r.register(Box::new(clear_selection_snapshot())); r.register(Box::new(set_selection_snapshot())); r.register(Box::new(set_viewport_snapshot())); @@ -584,9 +588,14 @@ fn tools_list_response(id_raw: &str, state: &EditorState) -> String { // client to render a tool picker + validate calls. Dynamic // UIKit element tools (one per kit component) are appended // alongside the static schemas — the kit set lives on - // `EditorState`, so they're computed per call. + // `EditorState`, so they're computed per call. Debug-tool + // schemas are appended only when the isolation flag is set, + // so they stay invisible to a production client. let mut entries: Vec = TOOL_SCHEMAS.iter().map(|s| (*s).to_string()).collect(); entries.extend(op_mcp::element_tools::element_tool_schemas(state)); + if debug_tools_enabled() { + entries.extend(DEBUG_TOOL_SCHEMAS.iter().map(|s| (*s).to_string())); + } format!( r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"tools":[{}]}}}}"#, entries.join(",") @@ -616,7 +625,6 @@ const TOOL_SCHEMAS: &[&str] = &[ r#"{"name":"get_history_depth","description":"Return undo + redo stack sizes. Useful before bulk rollback to know how many steps are available.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, r#"{"name":"get_viewport","description":"Return current canvas pan + zoom. pan_x/pan_y are i32 doc-px; zoom_percent is the zoom * 100 as int.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, r#"{"name":"get_selection_set","description":"Return every id in the multi-select set (vs get_selection which returns only the anchor). Result: count + comma-separated ids + anchor.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, - r#"{"name":"debug_validation_report","description":"Run the op-design-lint detectors over the active page and return the design-issue list. Read-only, no parameters. Result: count + categories (`;`-separated `category|count`) + issues (JSON-serialized Issue array). Gated behind the OPENPENCIL_DEBUG_TOOLS=1 env flag — ToolFailed when unset.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, r#"{"name":"clear_selection","description":"Drop the current multi-select. No args.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, r#"{"name":"set_selection","description":"Set selection to a single node by id (scoped to the ACTIVE page only). Rejects unknown ids and ids that live on a non-active page — switch the active page first with set_active_page.","inputSchema":{"type":"object","properties":{"node_id":{"type":"string","description":"positive u64 node id on the active page"}},"required":["node_id"]}}"#, r#"{"name":"set_viewport","description":"Set canvas pan + zoom. Pass any subset of pan_x / pan_y / zoom_percent — omitted axes are left unchanged. zoom_percent clamps to [10, 2000].","inputSchema":{"type":"object","properties":{"pan_x":{"type":"string","description":"i32 doc-px"},"pan_y":{"type":"string","description":"i32 doc-px"},"zoom_percent":{"type":"string","description":"int * 100 (100 == 1.0×)"}}}}"#, @@ -684,5 +692,12 @@ const TOOL_SCHEMAS: &[&str] = &[ 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"]}}"#, ]; +/// Debug-only tool schemas — appended to `tools/list` only when +/// `OPENPENCIL_DEBUG_TOOLS=1`. Kept separate from `TOOL_SCHEMAS` so the +/// production catalog never advertises them. +const DEBUG_TOOL_SCHEMAS: &[&str] = &[ + r#"{"name":"debug_validation_report","description":"Run the op-design-lint detectors over the active page and return the design-issue list. Read-only, no parameters. Result: count + categories (`;`-separated `category|count`) + issues (JSON-serialized Issue array). Gated behind the OPENPENCIL_DEBUG_TOOLS=1 env flag.","inputSchema":{"type":"object","properties":{},"additionalProperties":false}}"#, +]; + #[cfg(test)] mod tests; diff --git a/crates/op-host-desktop/src/mcp_serve/tests.rs b/crates/op-host-desktop/src/mcp_serve/tests.rs index c594a818b..f7155d2f0 100644 --- a/crates/op-host-desktop/src/mcp_serve/tests.rs +++ b/crates/op-host-desktop/src/mcp_serve/tests.rs @@ -43,18 +43,28 @@ fn initialize_response_includes_protocol_and_capabilities() { #[test] fn tools_list_response_includes_all_registered_tools() { + // The debug isolation flag is process-global; remove it + // explicitly so this test is deterministic under cargo's + // parallel runner (an earlier test may have set it). + std::env::remove_var("OPENPENCIL_DEBUG_TOOLS"); let state = op_editor_core::EditorState::new(); let r = tools_list_response("3", &state); - // Exact-count assertion: any tool added without - // updating this test will trip the count first. Codex - // stop-gate: previous `contains`-only checks would have - // silently passed if a new tool slipped into TOOL_SCHEMAS - // without being added to the list below. + // The production catalog excludes debug tools. Exact-count + // assertion: any tool added without updating this test trips + // the count first. Codex stop-gate: previous `contains`-only + // checks would have silently passed if a new tool slipped into + // TOOL_SCHEMAS without being added to the list below. assert_eq!( TOOL_SCHEMAS.len(), - 83, + 82, "tools/list catalog count must match the registered tools — add the new tool to this test" ); + // Production catalog excludes debug tools (we removed the + // env var above to ensure deterministic gate-off behaviour). + assert!( + !r.contains("debug_validation_report"), + "production tools/list must not advertise the debug tool: {r}" + ); // UIKit element tools are appended dynamically — one per // built-in starter-kit component (6) — and ride alongside // the static schemas in the tools/list response. @@ -95,7 +105,6 @@ fn tools_list_response_includes_all_registered_tools() { "get_history_depth", "get_viewport", "get_selection_set", - "debug_validation_report", "clear_selection", "set_selection", "set_viewport", @@ -163,6 +172,15 @@ fn tools_list_response_includes_all_registered_tools() { ] { assert!(r.contains(name), "tools/list must include {name}: {r}"); } + + // Gate open — the debug tool joins the catalog. + std::env::set_var("OPENPENCIL_DEBUG_TOOLS", "1"); + let r_debug = tools_list_response("3", &state); + assert!( + r_debug.contains("debug_validation_report"), + "debug tools/list must advertise debug_validation_report: {r_debug}" + ); + std::env::remove_var("OPENPENCIL_DEBUG_TOOLS"); } /// In-memory `Read + Write` stand-in for a `TcpStream` so the HTTP diff --git a/crates/op-mcp/src/debug_tools.rs b/crates/op-mcp/src/debug_tools.rs index 125850d88..4ffa22ba6 100644 --- a/crates/op-mcp/src/debug_tools.rs +++ b/crates/op-mcp/src/debug_tools.rs @@ -34,7 +34,12 @@ use super::{McpTool, ToolErrorCode, ToolOutcome}; const DEBUG_TOOLS_ENV: &str = "OPENPENCIL_DEBUG_TOOLS"; /// Returns whether the debug tools are enabled (`OPENPENCIL_DEBUG_TOOLS=1`). -fn debug_tools_enabled() -> bool { +/// +/// Public so the host MCP server can keep the debug tool out of the +/// production catalog entirely — both `rebuild_registry` and +/// `tools/list` consult this, so a client with the flag unset never +/// sees `debug_validation_report` at all (not just `ToolFailed` on call). +pub fn debug_tools_enabled() -> bool { std::env::var(DEBUG_TOOLS_ENV).is_ok_and(|v| v == "1") } diff --git a/crates/op-mcp/src/lib.rs b/crates/op-mcp/src/lib.rs index a355e8277..4534b6801 100644 --- a/crates/op-mcp/src/lib.rs +++ b/crates/op-mcp/src/lib.rs @@ -82,7 +82,9 @@ pub use component_tools::{ rename_component_snapshot, set_node_collapsed_snapshot, CreateComponent, DeleteComponent, InstantiateComponent, RenameComponent, SetNodeCollapsed, }; -pub use debug_tools::{debug_validation_report_snapshot, DebugValidationReport}; +pub use debug_tools::{ + debug_tools_enabled, debug_validation_report_snapshot, DebugValidationReport, +}; pub use extra_read_tools::{get_node_children_snapshot, ChildRecord, GetNodeChildren}; pub use json_serializer::response_to_json; pub use node_attr_tools::{