From 3d9dab0f20f543cf2bcaec169911a63fd8cd8480 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Thu, 14 May 2026 23:30:47 +0800 Subject: [PATCH] fix(desktop/mcp): tighten design_* schema descriptions + exact-count test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex stop-gate review on 7cee053d flagged two concerns: 1. design_skeleton/content/refine tools/list schema descriptions could mislead LLM clients into inferring differentiated apply semantics from the phase label. Per the internal comment in batch_design.rs, all three currently dispatch to BatchInsert with phase metadata only — but the schema description is what clients see, not the comment. Add explicit "Apply behavior is identical to batch_design today (phase is metadata only)" to each of the three schema descriptions. 2. The new tools_list test only verified each tool name via `contains`. A future tool addition could silently pass this test without updating the expected count. Add an `assert_eq!(TOOL_SCHEMAS.len(), 21)` exact-count guard with a clear error message ("add the new tool to this test"). --- crates/openpencil-desktop/src/mcp_serve.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/openpencil-desktop/src/mcp_serve.rs b/crates/openpencil-desktop/src/mcp_serve.rs index d033bf33a..8a965f070 100644 --- a/crates/openpencil-desktop/src/mcp_serve.rs +++ b/crates/openpencil-desktop/src/mcp_serve.rs @@ -369,9 +369,9 @@ const TOOL_SCHEMAS: &[&str] = &[ // --- write tools --- r##"{"name":"set_variable_color","description":"Set a Color-kind variable's value.","inputSchema":{"type":"object","properties":{"name":{"type":"string"},"hex":{"type":"string","description":"#rgb / #rrggbb / #rrggbbaa"}},"required":["name","hex"]}}"##, r#"{"name":"batch_design","description":"Insert N leaf nodes on the active page in one atomic shot. nodes_json must be a JSON array string of {kind,name,x,y,width,height,fill_hex?} descriptors.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, - r#"{"name":"design_skeleton","description":"Layered design workflow phase 1: insert structural scaffolding. Same wire shape as batch_design; response carries phase=skeleton so the client can phase its prompting.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, - r#"{"name":"design_content","description":"Layered design workflow phase 2: fill content into the scaffold. Same wire shape as batch_design; response carries phase=content.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, - r#"{"name":"design_refine","description":"Layered design workflow phase 3: polish details. Same wire shape as batch_design; response carries phase=refine.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, + r#"{"name":"design_skeleton","description":"Layered design workflow phase 1: insert structural scaffolding. Apply behavior is identical to batch_design today (phase is metadata only); the response carries phase=skeleton so the client can phase its prompting. A future patch may add per-phase apply semantics.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, + r#"{"name":"design_content","description":"Layered design workflow phase 2: fill content into the scaffold. Apply behavior is identical to batch_design today (phase is metadata only); the response carries phase=content. A future patch may add per-phase apply semantics.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, + r#"{"name":"design_refine","description":"Layered design workflow phase 3: polish details. Apply behavior is identical to batch_design today (phase is metadata only); the response carries phase=refine. A future patch may add per-phase apply semantics.","inputSchema":{"type":"object","properties":{"nodes_json":{"type":"string","description":"JSON array of node descriptors"}},"required":["nodes_json"]}}"#, r#"{"name":"set_variable_number","description":"Set a Number-kind variable's value (decimal, may be negative or fractional).","inputSchema":{"type":"object","properties":{"name":{"type":"string"},"value":{"type":"string"}},"required":["name","value"]}}"#, r#"{"name":"set_variable_string","description":"Set a String-kind variable's value (free-form text).","inputSchema":{"type":"object","properties":{"name":{"type":"string"},"value":{"type":"string"}},"required":["name","value"]}}"#, r#"{"name":"set_variable_boolean","description":"Set a Boolean-kind variable's value (\"true\" or \"false\").","inputSchema":{"type":"object","properties":{"name":{"type":"string"},"value":{"type":"string","enum":["true","false"]}},"required":["name","value"]}}"#, @@ -430,6 +430,16 @@ mod tests { #[test] fn tools_list_response_includes_all_twenty_one_tools() { let r = tools_list_response("3"); + // 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. + assert_eq!( + TOOL_SCHEMAS.len(), + 21, + "tools/list catalog count must match the registered tools — add the new tool to this test" + ); for name in [ "get_document_info", "get_selection",