fix: remove duplicate move_schematic_component tool registration and add regression test
The PR added a second server.tool("move_schematic_component", ...) at line 1127
without removing the original registration at line 722, causing the server to
fail on startup with "Tool move_schematic_component is already registered".
Also adds tests/test_ts_tool_registry.py which scans all src/tools/**/*.ts files
for duplicate server.tool() names so this class of bug is caught automatically.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -717,48 +717,6 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
// Move schematic component
|
|
||||||
server.tool(
|
|
||||||
"move_schematic_component",
|
|
||||||
"Move a placed symbol to a new position in the schematic.",
|
|
||||||
{
|
|
||||||
schematicPath: z.string().describe("Path to the .kicad_sch file"),
|
|
||||||
reference: z.string().describe("Reference designator (e.g., R1, U1)"),
|
|
||||||
position: z
|
|
||||||
.object({
|
|
||||||
x: z.number(),
|
|
||||||
y: z.number(),
|
|
||||||
})
|
|
||||||
.describe("New position"),
|
|
||||||
},
|
|
||||||
async (args: {
|
|
||||||
schematicPath: string;
|
|
||||||
reference: string;
|
|
||||||
position: { x: number; y: number };
|
|
||||||
}) => {
|
|
||||||
const result = await callKicadScript("move_schematic_component", args);
|
|
||||||
if (result.success) {
|
|
||||||
return {
|
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: `Moved ${args.reference} from (${result.oldPosition.x}, ${result.oldPosition.y}) to (${result.newPosition.x}, ${result.newPosition.y})`,
|
|
||||||
},
|
|
||||||
],
|
|
||||||
};
|
|
||||||
}
|
|
||||||
return {
|
|
||||||
content: [
|
|
||||||
{
|
|
||||||
type: "text",
|
|
||||||
text: `Failed to move component: ${result.message || "Unknown error"}`,
|
|
||||||
},
|
|
||||||
],
|
|
||||||
isError: true,
|
|
||||||
};
|
|
||||||
},
|
|
||||||
);
|
|
||||||
|
|
||||||
// Rotate schematic component
|
// Rotate schematic component
|
||||||
server.tool(
|
server.tool(
|
||||||
"rotate_schematic_component",
|
"rotate_schematic_component",
|
||||||
|
|||||||
61
tests/test_ts_tool_registry.py
Normal file
61
tests/test_ts_tool_registry.py
Normal file
@@ -0,0 +1,61 @@
|
|||||||
|
"""
|
||||||
|
Regression test: no MCP tool name is registered more than once across all
|
||||||
|
TypeScript tool files in src/tools/.
|
||||||
|
|
||||||
|
This caught a real bug where move_schematic_component was registered twice
|
||||||
|
(once in the original code and once in the PR adding wire-preservation),
|
||||||
|
causing the server to fail on startup with:
|
||||||
|
Error: Tool move_schematic_component is already registered
|
||||||
|
"""
|
||||||
|
|
||||||
|
import re
|
||||||
|
from collections import Counter
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
SRC_TOOLS_DIR = Path(__file__).parent.parent / "src" / "tools"
|
||||||
|
|
||||||
|
# Pattern matches the tool-name argument to server.tool(
|
||||||
|
# server.tool(
|
||||||
|
# "some_tool_name",
|
||||||
|
_SERVER_TOOL_RE = re.compile(r'server\.tool\(\s*["\']([a-zA-Z0-9_]+)["\']')
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class TestTsToolRegistry:
|
||||||
|
def _collect_registrations(self):
|
||||||
|
"""Return list of (tool_name, file, line_no) for every server.tool() call."""
|
||||||
|
registrations = []
|
||||||
|
for ts_file in sorted(SRC_TOOLS_DIR.glob("**/*.ts")):
|
||||||
|
text = ts_file.read_text(encoding="utf-8")
|
||||||
|
for m in _SERVER_TOOL_RE.finditer(text):
|
||||||
|
line_no = text[: m.start()].count("\n") + 1
|
||||||
|
registrations.append((m.group(1), ts_file.name, line_no))
|
||||||
|
return registrations
|
||||||
|
|
||||||
|
def test_no_duplicate_tool_names(self):
|
||||||
|
"""Every tool name must appear exactly once across all TS tool files."""
|
||||||
|
registrations = self._collect_registrations()
|
||||||
|
assert registrations, "No server.tool() calls found — check SRC_TOOLS_DIR path"
|
||||||
|
|
||||||
|
counts = Counter(name for name, _, _ in registrations)
|
||||||
|
duplicates = {name: count for name, count in counts.items() if count > 1}
|
||||||
|
|
||||||
|
if duplicates:
|
||||||
|
details = []
|
||||||
|
for dup_name in sorted(duplicates):
|
||||||
|
locations = [
|
||||||
|
f" {fname}:{line}" for name, fname, line in registrations if name == dup_name
|
||||||
|
]
|
||||||
|
details.append(f"{dup_name} ({duplicates[dup_name]}x):\n" + "\n".join(locations))
|
||||||
|
pytest.fail(
|
||||||
|
"Duplicate MCP tool registrations found — server will fail to start:\n\n"
|
||||||
|
+ "\n\n".join(details)
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_tool_files_exist(self):
|
||||||
|
"""Sanity check: src/tools/ directory must be present and contain TS files."""
|
||||||
|
assert SRC_TOOLS_DIR.is_dir(), f"src/tools/ not found at {SRC_TOOLS_DIR}"
|
||||||
|
ts_files = list(SRC_TOOLS_DIR.glob("**/*.ts"))
|
||||||
|
assert ts_files, "No .ts files found in src/tools/"
|
||||||
Reference in New Issue
Block a user