diff --git a/src/tools/design-rules.ts b/src/tools/design-rules.ts index 030c739..7678614 100644 --- a/src/tools/design-rules.ts +++ b/src/tools/design-rules.ts @@ -7,6 +7,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; import { logger } from "../logger.js"; +import { formatKicadResult } from "./tool-response.js"; // Command function type for KiCAD script calls type CommandFunction = (command: string, params: Record) => Promise; @@ -52,14 +53,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm logger.debug("Setting design rules"); const result = await callKicadScript("set_design_rules", params); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -74,14 +68,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm logger.debug("Getting design rules"); const result = await callKicadScript("get_design_rules", {}); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -98,14 +85,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm logger.debug("Running DRC check"); const result = await callKicadScript("run_drc", { reportPath }); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -162,14 +142,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm nets, }); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -190,14 +163,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm netClass, }); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -224,14 +190,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm minViaDrill, }); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -284,14 +243,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm item2, }); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); @@ -311,14 +263,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm logger.debug("Getting DRC violations"); const result = await callKicadScript("get_drc_violations", { severity }); - return { - content: [ - { - type: "text", - text: JSON.stringify(result), - }, - ], - }; + return formatKicadResult(result); }, ); diff --git a/src/tools/tool-response.ts b/src/tools/tool-response.ts new file mode 100644 index 0000000..28bb42e --- /dev/null +++ b/src/tools/tool-response.ts @@ -0,0 +1,30 @@ +export type McpTextResult = { + content: Array<{ + type: "text"; + text: string; + }>; + isError?: true; +}; + +function isKicadFailure(result: unknown): boolean { + return ( + typeof result === "object" && + result !== null && + "success" in result && + (result as { success?: unknown }).success === false + ); +} + +export function formatKicadResult(result: unknown): McpTextResult { + const text = JSON.stringify(result) ?? String(result); + + return { + content: [ + { + type: "text", + text, + }, + ], + ...(isKicadFailure(result) ? { isError: true as const } : {}), + }; +} diff --git a/tests/test_mcp_error_wrapping_static.py b/tests/test_mcp_error_wrapping_static.py new file mode 100644 index 0000000..9c43723 --- /dev/null +++ b/tests/test_mcp_error_wrapping_static.py @@ -0,0 +1,51 @@ +""" +Regression tests for MCP error wrapping in TypeScript tool adapters. + +KiCad backend commands may report domain failures as JSON payloads such as +{"success": false, "message": "No board is loaded"}. The MCP tool result must +also be marked with isError so clients do not treat the failed command as OK. +""" + +from pathlib import Path + +import pytest + +ROOT = Path(__file__).parent.parent +TOOLS_DIR = ROOT / "src" / "tools" +DESIGN_RULES_TS = TOOLS_DIR / "design-rules.ts" +TOOL_RESPONSE_TS = TOOLS_DIR / "tool-response.ts" + + +@pytest.mark.unit +class TestMcpErrorWrapping: + def test_helper_marks_only_explicit_failure_payloads_as_mcp_errors(self): + """A KiCad success:false payload must become an MCP isError result.""" + helper = TOOL_RESPONSE_TS.read_text(encoding="utf-8") + + assert "export function formatKicadResult" in helper + assert "success === false" in helper + assert "isError: true" in helper + + def test_design_rule_tools_use_shared_error_wrapper(self): + """DRC/design-rule wrappers must not return success:false as a plain OK result.""" + source = DESIGN_RULES_TS.read_text(encoding="utf-8") + + assert 'import { formatKicadResult } from "./tool-response.js";' in source + + for command in ( + "set_design_rules", + "get_design_rules", + "run_drc", + "add_net_class", + "assign_net_to_class", + "set_layer_constraints", + "check_clearance", + "get_drc_violations", + ): + marker = f'callKicadScript("{command}"' + command_index = source.find(marker) + assert command_index != -1, f"{command} wrapper not found" + + next_tool_index = source.find("server.tool(", command_index + len(marker)) + wrapper_body = source[command_index : next_tool_index if next_tool_index != -1 else None] + assert "return formatKicadResult(result);" in wrapper_body