fix(mcp): read a canvas node's facts where the API puts them
`_slim_canvas` filtered `n["data"]`, the React Flow shape the frontend store holds. `GET /api/v1/canvas` has never sent that: a node is reported flat, so the filter matched nothing and `get_canvas` returned an id and a node type per node — no label, no address, no services. The three tests covering it fed a hand-built nested payload, so they passed against a shape the backend does not produce. Fold `data` over the node and read both, so either form slims the same way. `type` leaves `NODE_KEEP` — flat it is the node type, not a device fact, and it is already reported as `node_type`; `parentId` becomes `parent_id`, the key the API uses, normalized from either spelling. The new tests are built on a full `NodeResponse` copied from a real response, and the backend now pins that wire shape from its side so the two move together. ha-relevant: no
This commit is contained in:
Pouzor
committed by
Pouzor - Rémy Jardient
Pouzor - Rémy Jardient
parent
71e29362aa
commit
d40f73b85c
@@ -435,6 +435,77 @@ class TestRoutesKeepTheLinkInStep:
|
||||
canvas_b = (await client.get(f"/api/v1/canvas?design_id={design_b}", headers=headers)).json()
|
||||
assert canvas_b["nodes"][0]["ip"] == "10.0.0.9"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_wire_shape_the_mcp_server_reads_is_unchanged(
|
||||
self, client: AsyncClient, headers, db_session
|
||||
):
|
||||
"""`mcp/app/tools.py` reads the facts flat on the node — keep it that way.
|
||||
|
||||
The MCP server is a thin proxy over these routes: `_slim_canvas` keeps
|
||||
`NODE_KEEP`, `_slim_node_summary` keeps label/type/status, and `get_node`
|
||||
looks a node up by label. None of it knows the inventory row exists, so
|
||||
the split must not show on the wire.
|
||||
"""
|
||||
db_session.add(
|
||||
InventoryDevice(
|
||||
id="d-1",
|
||||
ip="10.0.0.5",
|
||||
hostname="nas.lan",
|
||||
mac="aa:bb:cc:dd:ee:ff",
|
||||
os="TrueNAS",
|
||||
notes="in the garage",
|
||||
cpu_count=8,
|
||||
cpu_model="Xeon",
|
||||
ram_gb=32.0,
|
||||
disk_gb=4000.0,
|
||||
services=[{"port": 22, "protocol": "tcp", "service_name": "ssh"}],
|
||||
properties=[{"key": "Rack", "value": "A1", "icon": None, "visible": True}],
|
||||
status_live="online",
|
||||
)
|
||||
)
|
||||
await db_session.commit()
|
||||
design_id = (await client.post("/api/v1/designs", json={"name": "Net"}, headers=headers)).json()["id"]
|
||||
created = (
|
||||
await client.post(
|
||||
"/api/v1/nodes",
|
||||
json={
|
||||
"type": "nas", "label": "NAS", "ip": "10.0.0.5",
|
||||
"design_id": design_id, "force": True,
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
).json()
|
||||
|
||||
# mcp `_slim_canvas` NODE_KEEP, plus what `_slim_node_summary` reads.
|
||||
expected = {
|
||||
"type": "nas",
|
||||
"label": "NAS",
|
||||
"ip": "10.0.0.5",
|
||||
"hostname": "nas.lan",
|
||||
"mac": "aa:bb:cc:dd:ee:ff",
|
||||
"os": "TrueNAS",
|
||||
"status": "online",
|
||||
"notes": "in the garage",
|
||||
"cpu_count": 8,
|
||||
"cpu_model": "Xeon",
|
||||
"ram_gb": 32.0,
|
||||
"disk_gb": 4000.0,
|
||||
}
|
||||
for source in (
|
||||
created,
|
||||
(await client.get(f"/api/v1/nodes/{created['id']}", headers=headers)).json(),
|
||||
(await client.get("/api/v1/nodes", headers=headers)).json()[0],
|
||||
(await client.get("/api/v1/canvas", headers=headers)).json()["nodes"][0],
|
||||
# mcp `get_node` falls back to a label lookup when given no id.
|
||||
(await client.get("/api/v1/nodes?label=NAS", headers=headers)).json()[0],
|
||||
):
|
||||
assert {k: source[k] for k in expected} == expected
|
||||
assert source["services"] == [{"port": 22, "protocol": "tcp", "service_name": "ssh"}]
|
||||
assert source["properties"] == [
|
||||
{"key": "Rack", "value": "A1", "icon": None, "visible": True}
|
||||
]
|
||||
assert "parent_id" in source
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deleting_a_node_keeps_the_device(self, client: AsyncClient, headers, db_session):
|
||||
db_session.add(InventoryDevice(id="d-1", ip="10.0.0.5"))
|
||||
|
||||
+17
-5
@@ -217,19 +217,31 @@ def register_tools(server: Server):
|
||||
|
||||
|
||||
def _slim_canvas(raw: dict) -> dict:
|
||||
"""Strip React Flow layout/style fields — keep only semantic data for AI use."""
|
||||
"""Strip layout/style fields — keep only semantic data for AI use.
|
||||
|
||||
`GET /api/v1/canvas` reports a node flat (`NodeResponse`): the device facts
|
||||
sit beside `id` and `type`, not under a React Flow `data` key. A payload that
|
||||
does carry `data` — the shape the frontend store holds — is folded in too, so
|
||||
either form slims to the same thing.
|
||||
"""
|
||||
NODE_KEEP = {
|
||||
"id", "type", "label", "ip", "hostname", "mac", "os", "status", "services",
|
||||
"label", "ip", "hostname", "mac", "os", "status", "services",
|
||||
"notes", "description", "properties", "cpu_count", "cpu_model", "ram_gb",
|
||||
"disk_gb", "parentId",
|
||||
"disk_gb", "parent_id",
|
||||
}
|
||||
EDGE_KEEP = {"id", "source", "target", "type", "label"}
|
||||
|
||||
def slim_node(n: dict) -> dict:
|
||||
data = n.get("data", {})
|
||||
out = {k: v for k, v in data.items() if k in NODE_KEEP and v not in (None, "", [])}
|
||||
fields = {**n, **n.get("data", {})}
|
||||
out = {k: v for k, v in fields.items() if k in NODE_KEEP and v not in (None, "", [])}
|
||||
out["id"] = n.get("id")
|
||||
# `type` on the wire is the node type (router, proxmox, ...); it is
|
||||
# reported under its own key so it cannot collide with a device fact.
|
||||
out["node_type"] = n.get("type")
|
||||
# Nesting is `parent_id` on the API and `parentId` in a React Flow payload.
|
||||
parent = out.pop("parent_id", None) or n.get("parentId")
|
||||
if parent:
|
||||
out["parent_id"] = parent
|
||||
return out
|
||||
|
||||
def slim_edge(e: dict) -> dict:
|
||||
|
||||
@@ -197,6 +197,113 @@ async def test_get_canvas_keeps_documentation_fields(mock_backend):
|
||||
assert node["properties"] == [{"name": "rack", "value": "A1"}]
|
||||
|
||||
|
||||
# A node as `GET /api/v1/canvas` actually reports it: `NodeResponse`, flat, with
|
||||
# the device facts beside `id` and `type` rather than under a React Flow `data`
|
||||
# key. Copied from a real response — the backend pins this shape in
|
||||
# `backend/tests/test_inventory_sync.py::
|
||||
# TestRoutesKeepTheLinkInStep::test_the_wire_shape_the_mcp_server_reads_is_unchanged`.
|
||||
_API_CANVAS_NODE = {
|
||||
"id": "n1",
|
||||
"type": "proxmox",
|
||||
"label": "pve1",
|
||||
"design_id": "d-1",
|
||||
"device_id": "dev-1",
|
||||
"ip": "192.168.1.10",
|
||||
"hostname": "pve1.lan",
|
||||
"mac": "aa:bb:cc:dd:ee:ff",
|
||||
"os": "Proxmox VE 8",
|
||||
"status": "online",
|
||||
"notes": "Main hypervisor",
|
||||
"services": [{"port": 8006, "protocol": "tcp", "service_name": "proxmox"}],
|
||||
"properties": [{"key": "Rack", "value": "A1", "icon": None, "visible": True}],
|
||||
"cpu_count": 16,
|
||||
"cpu_model": "Xeon E5",
|
||||
"ram_gb": 64.0,
|
||||
"disk_gb": 4000.0,
|
||||
"show_hardware": True,
|
||||
"check_method": "https",
|
||||
"check_target": "https://192.168.1.10:8006",
|
||||
"parent_id": None,
|
||||
"pos_x": 120.0,
|
||||
"pos_y": 240.0,
|
||||
"width": 160.0,
|
||||
"height": 80.0,
|
||||
"container_mode": False,
|
||||
"custom_colors": None,
|
||||
"custom_icon": None,
|
||||
"show_port_numbers": False,
|
||||
"bottom_handles": 1,
|
||||
"top_handles": 1,
|
||||
"left_handles": 0,
|
||||
"right_handles": 0,
|
||||
"ieee_address": None,
|
||||
"last_seen": "2026-08-14T10:00:00Z",
|
||||
"last_scan": None,
|
||||
"response_time_ms": 12,
|
||||
"created_at": "2026-08-01T10:00:00Z",
|
||||
"updated_at": "2026-08-14T10:00:00Z",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_get_canvas_keeps_the_facts_of_a_flat_api_node(mock_backend):
|
||||
"""The API reports a node flat; slimming must not drop everything but the id."""
|
||||
mock_backend.get = AsyncMock(return_value={"nodes": [_API_CANVAS_NODE], "edges": []})
|
||||
|
||||
node = (await _dispatch("get_canvas", {}))["nodes"][0]
|
||||
|
||||
assert node == {
|
||||
"id": "n1",
|
||||
"node_type": "proxmox",
|
||||
"label": "pve1",
|
||||
"ip": "192.168.1.10",
|
||||
"hostname": "pve1.lan",
|
||||
"mac": "aa:bb:cc:dd:ee:ff",
|
||||
"os": "Proxmox VE 8",
|
||||
"status": "online",
|
||||
"notes": "Main hypervisor",
|
||||
"services": [{"port": 8006, "protocol": "tcp", "service_name": "proxmox"}],
|
||||
"properties": [{"key": "Rack", "value": "A1", "icon": None, "visible": True}],
|
||||
"cpu_count": 16,
|
||||
"cpu_model": "Xeon E5",
|
||||
"ram_gb": 64.0,
|
||||
"disk_gb": 4000.0,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_get_canvas_drops_the_layout_fields_of_a_flat_api_node(mock_backend):
|
||||
mock_backend.get = AsyncMock(return_value={"nodes": [_API_CANVAS_NODE], "edges": []})
|
||||
|
||||
node = (await _dispatch("get_canvas", {}))["nodes"][0]
|
||||
|
||||
for dropped in (
|
||||
"pos_x", "pos_y", "width", "height", "custom_colors", "custom_icon",
|
||||
"bottom_handles", "top_handles", "left_handles", "right_handles",
|
||||
"created_at", "updated_at", "design_id", "device_id", "type",
|
||||
):
|
||||
assert dropped not in node
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_get_canvas_reports_nesting_from_either_shape(mock_backend):
|
||||
mock_backend.get = AsyncMock(
|
||||
return_value={
|
||||
"nodes": [
|
||||
{**_API_CANVAS_NODE, "id": "child", "parent_id": "n1"},
|
||||
{"id": "rf-child", "type": "vm", "parentId": "n1", "data": {"label": "vm1"}},
|
||||
],
|
||||
"edges": [],
|
||||
}
|
||||
)
|
||||
|
||||
nodes = (await _dispatch("get_canvas", {}))["nodes"]
|
||||
|
||||
assert nodes[0]["parent_id"] == "n1"
|
||||
assert nodes[1]["parent_id"] == "n1"
|
||||
assert nodes[1]["label"] == "vm1"
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_get_canvas_with_design_id(mock_backend):
|
||||
mock_backend.get = AsyncMock(return_value={"nodes": [], "edges": []})
|
||||
|
||||
Reference in New Issue
Block a user