Fix MCP error wrapping for DRC payloads
This commit is contained in:
@@ -7,6 +7,7 @@
|
|||||||
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
|
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
|
||||||
import { z } from "zod";
|
import { z } from "zod";
|
||||||
import { logger } from "../logger.js";
|
import { logger } from "../logger.js";
|
||||||
|
import { formatKicadResult } from "./tool-response.js";
|
||||||
|
|
||||||
// Command function type for KiCAD script calls
|
// Command function type for KiCAD script calls
|
||||||
type CommandFunction = (command: string, params: Record<string, unknown>) => Promise<any>;
|
type CommandFunction = (command: string, params: Record<string, unknown>) => Promise<any>;
|
||||||
@@ -52,14 +53,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
logger.debug("Setting design rules");
|
logger.debug("Setting design rules");
|
||||||
const result = await callKicadScript("set_design_rules", params);
|
const result = await callKicadScript("set_design_rules", params);
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -74,14 +68,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
logger.debug("Getting design rules");
|
logger.debug("Getting design rules");
|
||||||
const result = await callKicadScript("get_design_rules", {});
|
const result = await callKicadScript("get_design_rules", {});
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -98,14 +85,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
logger.debug("Running DRC check");
|
logger.debug("Running DRC check");
|
||||||
const result = await callKicadScript("run_drc", { reportPath });
|
const result = await callKicadScript("run_drc", { reportPath });
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -162,14 +142,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
nets,
|
nets,
|
||||||
});
|
});
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -190,14 +163,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
netClass,
|
netClass,
|
||||||
});
|
});
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -224,14 +190,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
minViaDrill,
|
minViaDrill,
|
||||||
});
|
});
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -284,14 +243,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
item2,
|
item2,
|
||||||
});
|
});
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -311,14 +263,7 @@ export function registerDesignRuleTools(server: McpServer, callKicadScript: Comm
|
|||||||
logger.debug("Getting DRC violations");
|
logger.debug("Getting DRC violations");
|
||||||
const result = await callKicadScript("get_drc_violations", { severity });
|
const result = await callKicadScript("get_drc_violations", { severity });
|
||||||
|
|
||||||
return {
|
return formatKicadResult(result);
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: JSON.stringify(result),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
30
src/tools/tool-response.ts
Normal file
30
src/tools/tool-response.ts
Normal file
@@ -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 } : {}),
|
||||||
|
};
|
||||||
|
}
|
||||||
51
tests/test_mcp_error_wrapping_static.py
Normal file
51
tests/test_mcp_error_wrapping_static.py
Normal file
@@ -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
|
||||||
Reference in New Issue
Block a user