diff --git a/VERSION b/VERSION index e3a4f19..cc6612c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.2.0 \ No newline at end of file +2.3.0 \ No newline at end of file diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index aaa4c8f..390c86a 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -20,6 +20,34 @@ from app.services.zigbee_service import build_zigbee_properties _ZIGBEE_TYPES = {"zigbee_coordinator", "zigbee_router", "zigbee_enddevice"} +def build_mac_property(mac: str | None) -> list[dict[str, Any]]: + """Build a NodeProperty list carrying a device MAC address. + + Shape matches the frontend ``NodeProperty`` type + (``{key, value, icon, visible}``). Hidden by default — the user opts in to + showing it on the canvas card from the right panel. Returns an empty list + when no MAC is known. + """ + if not mac: + return [] + return [{"key": "MAC", "value": mac, "icon": None, "visible": False}] + + +def merge_mac_property( + props: list[dict[str, Any]] | None, mac: str | None +) -> list[dict[str, Any]]: + """Append a MAC NodeProperty to ``props`` unless one is already present. + + Preserves any user-supplied properties (and an existing MAC row's + visibility) untouched. Used on approve so the scanned MAC is not lost. + """ + out = [dict(p) for p in (props or [])] + if not mac or any(p.get("key") == "MAC" for p in out): + return out + out.append({"key": "MAC", "value": mac, "icon": None, "visible": False}) + return out + + class BulkActionRequest(BaseModel): device_ids: list[str] @@ -138,13 +166,14 @@ async def bulk_approve_devices( label=device.hostname or device.friendly_name or device.ip or "device", type=node_type, ip=device.ip, + mac=device.mac, hostname=device.hostname, status="online" if is_zigbee else "unknown", services=device.services or [], ieee_address=device.ieee_address, properties=build_zigbee_properties( device.ieee_address, device.vendor, device.model, device.lqi - ) if is_zigbee else [], + ) if is_zigbee else build_mac_property(device.mac), # Default to ping so the status checker actually polls the new node. # Without this the scheduler skips it (check_method NULL → no check). check_method="none" if is_zigbee else ("ping" if device.ip else None), @@ -245,17 +274,21 @@ async def approve_device( raise HTTPException(status_code=409, detail="Device already processed") device.status = "approved" _is_zigbee = node_data.type in _ZIGBEE_TYPES + # Prefer the MAC discovered during the scan (stored on the pending device); + # fall back to whatever the approve payload carried. + _mac = device.mac or node_data.mac node = Node( label=node_data.label, type=node_data.type, ip=node_data.ip, + mac=_mac, hostname=node_data.hostname, status="online" if _is_zigbee else node_data.status, services=node_data.services or [], ieee_address=device.ieee_address, properties=build_zigbee_properties( device.ieee_address, device.vendor, device.model, device.lqi - ) if _is_zigbee else (node_data.properties or []), + ) if _is_zigbee else merge_mac_property(node_data.properties, _mac), check_method="none" if _is_zigbee else (node_data.check_method or ("ping" if node_data.ip else None)), check_target=None if _is_zigbee else node_data.check_target, design_id=node_design_id, diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index 5a4ee8c..9e69af1 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -228,7 +228,13 @@ async def _persist_pending_import( pending.vendor = n.get("vendor") or pending.vendor if n.get("lqi") is not None: pending.lqi = n.get("lqi") - if pending.status == "hidden": + if pending.status == "approved": + # The device was approved earlier but its canvas Node no longer + # exists (no Node matched the IEEE above) — it was deleted. Revive + # the row to "pending" so it reappears in the Pending list on + # re-import instead of being silently swallowed. (Issue #167) + pending.status = "pending" + elif pending.status == "hidden": # Re-imported a hidden device → leave it hidden, just refresh fields. pass pending_updated += 1 diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 7199ea9..6feb0b9 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -698,6 +698,144 @@ async def test_bulk_approve_zigbee_populates_properties( assert node.check_method == "none" +# --- MAC address propagation on approve (issue #168) --- + +def test_build_mac_property_returns_hidden_row(): + from app.api.routes.scan import build_mac_property + + assert build_mac_property("aa:bb:cc:dd:ee:ff") == [ + {"key": "MAC", "value": "aa:bb:cc:dd:ee:ff", "icon": None, "visible": False} + ] + + +def test_build_mac_property_empty_when_no_mac(): + from app.api.routes.scan import build_mac_property + + assert build_mac_property(None) == [] + assert build_mac_property("") == [] + + +def test_merge_mac_property_appends_when_absent(): + from app.api.routes.scan import merge_mac_property + + existing = [{"key": "Custom", "value": "x", "icon": None, "visible": True}] + merged = merge_mac_property(existing, "aa:bb:cc:dd:ee:ff") + assert {"key": "MAC", "value": "aa:bb:cc:dd:ee:ff", "icon": None, "visible": False} in merged + # Existing prop preserved untouched. + assert existing[0] in merged + + +def test_merge_mac_property_idempotent_and_preserves_visibility(): + from app.api.routes.scan import merge_mac_property + + existing = [{"key": "MAC", "value": "aa:bb:cc:dd:ee:ff", "icon": None, "visible": True}] + merged = merge_mac_property(existing, "aa:bb:cc:dd:ee:ff") + # No duplicate MAC row; user's visible=True choice kept. + macs = [p for p in merged if p["key"] == "MAC"] + assert len(macs) == 1 + assert macs[0]["visible"] is True + + +def test_merge_mac_property_noop_without_mac(): + from app.api.routes.scan import merge_mac_property + + existing = [{"key": "Custom", "value": "x", "icon": None, "visible": True}] + assert merge_mac_property(existing, None) == existing + + +@pytest.mark.asyncio +async def test_approve_device_copies_mac_to_node_and_properties( + client: AsyncClient, headers, pending_device, db_session +): + """Approving a scanned device must carry its MAC onto the node + properties.""" + from sqlalchemy import select + + from app.db.models import Node as NodeModel + # Payload intentionally omits mac — it must come from the pending device. + res = await client.post( + f"/api/v1/scan/pending/{pending_device.id}/approve", + json={"label": "My Server", "type": "server", "ip": "192.168.1.100", "status": "unknown", "services": []}, + headers=headers, + ) + assert res.status_code == 200 + node = ( + await db_session.execute(select(NodeModel).where(NodeModel.ip == "192.168.1.100")) + ).scalar_one() + assert node.mac == "aa:bb:cc:dd:ee:ff" + mac_props = [p for p in node.properties if p["key"] == "MAC"] + assert mac_props == [ + {"key": "MAC", "value": "aa:bb:cc:dd:ee:ff", "icon": None, "visible": False} + ] + + +@pytest.mark.asyncio +async def test_approve_device_does_not_duplicate_mac_property( + client: AsyncClient, headers, pending_device, db_session +): + """If the approve payload already carries a MAC prop, don't add a second one.""" + from sqlalchemy import select + + from app.db.models import Node as NodeModel + res = await client.post( + f"/api/v1/scan/pending/{pending_device.id}/approve", + json={ + "label": "My Server", + "type": "server", + "ip": "192.168.1.100", + "status": "unknown", + "services": [], + "properties": [ + {"key": "MAC", "value": "aa:bb:cc:dd:ee:ff", "icon": None, "visible": True} + ], + }, + headers=headers, + ) + assert res.status_code == 200 + node = ( + await db_session.execute(select(NodeModel).where(NodeModel.ip == "192.168.1.100")) + ).scalar_one() + mac_props = [p for p in node.properties if p["key"] == "MAC"] + assert len(mac_props) == 1 + # User's visibility choice is preserved. + assert mac_props[0]["visible"] is True + + +@pytest.mark.asyncio +async def test_bulk_approve_copies_mac_to_node_and_properties( + client: AsyncClient, headers, db_session +): + """Bulk approve must also propagate the scanned MAC to node + properties.""" + from sqlalchemy import select + + from app.db.models import Node as NodeModel + device = PendingDevice( + id=str(uuid.uuid4()), + ip="192.168.1.55", + mac="11:22:33:44:55:66", + hostname="host-mac", + services=[], + suggested_type="generic", + status="pending", + ) + db_session.add(device) + await db_session.commit() + + res = await client.post( + "/api/v1/scan/pending/bulk-approve", + json={"device_ids": [device.id]}, + headers=headers, + ) + assert res.status_code == 200 + node = ( + await db_session.execute(select(NodeModel).where(NodeModel.ip == "192.168.1.55")) + ).scalar_one() + assert node.mac == "11:22:33:44:55:66" + mac_props = [p for p in node.properties if p["key"] == "MAC"] + assert mac_props == [ + {"key": "MAC", "value": "11:22:33:44:55:66", "icon": None, "visible": False} + ] + + @pytest.mark.asyncio async def test_bulk_approve_sets_default_check_method(client: AsyncClient, headers, two_pending_devices, db_session): """Approved devices with an IP must default to ping; otherwise scheduler skips them.""" diff --git a/backend/tests/test_zigbee_router.py b/backend/tests/test_zigbee_router.py index b24e0b3..5096531 100644 --- a/backend/tests/test_zigbee_router.py +++ b/backend/tests/test_zigbee_router.py @@ -459,6 +459,92 @@ async def test_persist_pending_import_skips_pending_for_approved_node( assert all(p["visible"] is False for p in refreshed.properties) +@pytest.mark.asyncio +async def test_persist_pending_import_revives_orphaned_approved_device( + db_session, +) -> None: + """Regression for #167: approve → delete node → re-import must re-list device. + + When a device was approved (PendingDevice.status="approved") and its canvas + Node was later deleted, the orphaned "approved" row must be reset to + "pending" on re-import so it shows up in the Pending list again — instead of + being silently swallowed (re-import reports "found" but Pending stays empty). + """ + from sqlalchemy import select + + from app.api.routes.zigbee import _persist_pending_import + from app.db.models import PendingDevice + + # Simulate prior approve: a PendingDevice marked approved, but NO matching + # Node exists (the user deleted the canvas node afterwards). + orphan = PendingDevice( + ieee_address="0xR1", + friendly_name="router_1", + hostname="router_1", + suggested_type="zigbee_router", + device_subtype="Router", + model="CC2530", + vendor="TI", + lqi=220, + status="approved", + discovery_source="zigbee", + ) + db_session.add(orphan) + await db_session.commit() + + result = await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) + + # No new row created for 0xR1 — the existing one was updated/revived. + revived = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "0xR1") + ) + ).scalar_one() + assert revived.status == "pending" + # End device 0xE1 is brand new → created as pending; router was updated. + assert result.pending_created == 1 + assert result.pending_updated == 1 + + # It is now visible to the Pending list (status filter == "pending"). + listed = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.status == "pending") + ) + ).scalars().all() + assert {p.ieee_address for p in listed} == {"0xR1", "0xE1"} + + +@pytest.mark.asyncio +async def test_persist_pending_import_keeps_hidden_hidden_on_reimport( + db_session, +) -> None: + """A user-hidden device must stay hidden on re-import (not revived like #167).""" + from sqlalchemy import select + + from app.api.routes.zigbee import _persist_pending_import + from app.db.models import PendingDevice + + hidden = PendingDevice( + ieee_address="0xR1", + friendly_name="router_1", + suggested_type="zigbee_router", + device_subtype="Router", + status="hidden", + discovery_source="zigbee", + ) + db_session.add(hidden) + await db_session.commit() + + await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) + + still_hidden = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "0xR1") + ) + ).scalar_one() + assert still_hidden.status == "hidden" + + @pytest.mark.asyncio async def test_persist_pending_import_preserves_user_visibility(db_session) -> None: """If user has already made props visible, re-import must not flip them back.""" diff --git a/frontend/package-lock.json b/frontend/package-lock.json index f364c60..e4ba3e2 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "frontend", - "version": "2.2.0", + "version": "2.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "frontend", - "version": "2.2.0", + "version": "2.3.0", "dependencies": { "@base-ui/react": "^1.2.0", "@dagrejs/dagre": "^2.0.4", @@ -3237,9 +3237,9 @@ } }, "node_modules/@ts-morph/common/node_modules/brace-expansion": { - "version": "5.0.5", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.5.tgz", - "integrity": "sha512-VZznLgtwhn+Mact9tfiwx64fA9erHH/MCXEUfB/0bX/6Fz6ny5EGTXYltMocqg4xFAQZtnO3DHWWXi8RiuN7cQ==", + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", + "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" @@ -3651,9 +3651,9 @@ } }, "node_modules/@typescript-eslint/typescript-estree/node_modules/brace-expansion": { - "version": "5.0.5", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.5.tgz", - "integrity": "sha512-VZznLgtwhn+Mact9tfiwx64fA9erHH/MCXEUfB/0bX/6Fz6ny5EGTXYltMocqg4xFAQZtnO3DHWWXi8RiuN7cQ==", + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", + "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", "dev": true, "license": "MIT", "dependencies": { @@ -4250,9 +4250,9 @@ } }, "node_modules/brace-expansion": { - "version": "1.1.13", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.13.tgz", - "integrity": "sha512-9ZLprWS6EENmhEOpjCYW2c8VkmOvckIJZfkr7rBW6dObmfgJ/L1GpSYW5Hpo9lDz4D1+n0Ckz8rU7FwHDQiG/w==", + "version": "1.1.15", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.15.tgz", + "integrity": "sha512-EwOCDEex4quD37XhqM3omwtMoJjr//isUZz1JopUNWms+4Z2ViyM/k1YIRePpoVNnQhENnxtFjLaxNHrT7xIUg==", "dev": true, "license": "MIT", "dependencies": { @@ -8040,9 +8040,9 @@ } }, "node_modules/qs": { - "version": "6.15.0", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.0.tgz", - "integrity": "sha512-mAZTtNCeetKMH+pSjrb76NAM8V9a05I9aBZOHztWy/UqcJdQYNsf59vrRKWnojAT9Y+GbIvoTBC++CPHqpDBhQ==", + "version": "6.15.2", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.2.tgz", + "integrity": "sha512-Rzq0KEyX/w/tEybncDgdkZrJgVUsUMk3xjh3t5bv3S1HTAtg+uOYt72+ZfwiQwKdysThkTBdL/rTi6HDmX9Ddw==", "license": "BSD-3-Clause", "dependencies": { "side-channel": "^1.1.0" diff --git a/frontend/package.json b/frontend/package.json index b37ab99..010e6da 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "2.2.0", + "version": "2.3.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/components/modals/PendingDevicesModal.tsx b/frontend/src/components/modals/PendingDevicesModal.tsx index 973b492..1f770ed 100644 --- a/frontend/src/components/modals/PendingDevicesModal.tsx +++ b/frontend/src/components/modals/PendingDevicesModal.tsx @@ -10,6 +10,7 @@ import { toast } from 'sonner' import { PendingDeviceModal, type PendingDevice } from '@/components/modals/PendingDeviceModal' import type { NodeType, ServiceInfo } from '@/types' import { buildZigbeeProperties, isZigbeeType } from '@/utils/zigbeeProperties' +import { buildMacProperty } from '@/utils/macProperty' interface PendingDevicesModalProps { open: boolean @@ -255,11 +256,12 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus const fallbackLabel = deviceLabel(device) const type = (device.suggested_type ?? 'generic') as NodeType const zigbee = isZigbeeType(type) - const properties = zigbee ? buildZigbeeProperties(device) : [] + const properties = zigbee ? buildZigbeeProperties(device) : buildMacProperty(device.mac) const nodeData = { label: fallbackLabel, type, ip: device.ip ?? undefined, + mac: device.mac ?? undefined, hostname: device.hostname ?? undefined, status: zigbee ? 'online' : 'unknown', services: (device.services ?? []) as ServiceInfo[], @@ -325,10 +327,11 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus label: deviceLabel(d), type, ip: d.ip ?? undefined, + mac: d.mac ?? undefined, hostname: d.hostname ?? undefined, status: zigbee ? ('online' as const) : ('unknown' as const), services: (d.services ?? []) as ServiceInfo[], - properties: zigbee ? buildZigbeeProperties(d) : [], + properties: zigbee ? buildZigbeeProperties(d) : buildMacProperty(d.mac), }, }) }) diff --git a/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx b/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx index 6164950..4cbcf97 100644 --- a/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx +++ b/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx @@ -13,6 +13,7 @@ const mockApprove = vi.fn() const mockHide = vi.fn() const mockPending = vi.fn() const mockHidden = vi.fn() +const mockAddNode = vi.fn() vi.mock('@/api/client', () => ({ scanApi: { @@ -69,7 +70,7 @@ const DEVICE_ZIGBEE = { beforeEach(() => { vi.clearAllMocks() vi.mocked(useCanvasStore).mockReturnValue({ - addNode: vi.fn(), + addNode: mockAddNode, scanEventTs: 0, } as unknown as ReturnType) // setState is used by injectAutoEdges @@ -174,6 +175,34 @@ describe('PendingDevicesModal', () => { await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a', 'dev-b'])) }) + it('bulk approve carries the scanned MAC onto the canvas node (#168)', async () => { + render() + await waitFor(() => expect(screen.getByTestId('pending-card-dev-a')).toBeInTheDocument()) + fireEvent.click(screen.getByRole('button', { name: 'Select mode' })) + fireEvent.click(screen.getByTestId('pending-card-dev-a')) + fireEvent.click(screen.getByTestId('pending-card-dev-b')) + fireEvent.click(screen.getByRole('button', { name: /Approve \(2\)/ })) + await waitFor(() => expect(mockAddNode).toHaveBeenCalledTimes(2)) + + // dev-a is an IP device with a MAC → node carries mac + a MAC property row. + const ipNode = mockAddNode.mock.calls + .map((c) => c[0]) + .find((n) => n.id === 'n1') + expect(ipNode.data.mac).toBe('aa:bb:cc:dd:ee:01') + expect(ipNode.data.properties).toContainEqual({ + key: 'MAC', + value: 'aa:bb:cc:dd:ee:01', + icon: null, + visible: false, + }) + + // dev-b is zigbee with no MAC → no MAC property row. + const zbNode = mockAddNode.mock.calls + .map((c) => c[0]) + .find((n) => n.id === 'n2') + expect(zbNode.data.properties.some((p: { key: string }) => p.key === 'MAC')).toBe(false) + }) + it('bulk hide calls API with selected ids', async () => { render() await waitFor(() => expect(screen.getByTestId('pending-card-dev-a')).toBeInTheDocument()) diff --git a/frontend/src/utils/__tests__/macProperty.test.ts b/frontend/src/utils/__tests__/macProperty.test.ts new file mode 100644 index 0000000..166f3fb --- /dev/null +++ b/frontend/src/utils/__tests__/macProperty.test.ts @@ -0,0 +1,16 @@ +import { describe, it, expect } from 'vitest' +import { buildMacProperty } from '../macProperty' + +describe('buildMacProperty', () => { + it('returns a hidden MAC property row for a MAC', () => { + expect(buildMacProperty('aa:bb:cc:dd:ee:ff')).toEqual([ + { key: 'MAC', value: 'aa:bb:cc:dd:ee:ff', icon: null, visible: false }, + ]) + }) + + it('returns an empty array when MAC is null/undefined/empty', () => { + expect(buildMacProperty(null)).toEqual([]) + expect(buildMacProperty(undefined)).toEqual([]) + expect(buildMacProperty('')).toEqual([]) + }) +}) diff --git a/frontend/src/utils/macProperty.ts b/frontend/src/utils/macProperty.ts new file mode 100644 index 0000000..969da81 --- /dev/null +++ b/frontend/src/utils/macProperty.ts @@ -0,0 +1,9 @@ +import type { NodeProperty } from '@/types' + +/** Build the MAC address property row shown in the right panel. + * Hidden by default — the user opts in to showing it on the canvas card. + * Matches backend `build_mac_property`. Returns an empty array when no MAC. */ +export function buildMacProperty(mac?: string | null): NodeProperty[] { + if (!mac) return [] + return [{ key: 'MAC', value: mac, icon: null, visible: false }] +} diff --git a/mcp/app/tools.py b/mcp/app/tools.py index b9b934c..dbddb27 100644 --- a/mcp/app/tools.py +++ b/mcp/app/tools.py @@ -4,87 +4,133 @@ from mcp.types import Tool, TextContent from .backend_client import backend +NODE_TYPES = ["isp", "router", "switch", "server", "proxmox", "vm", "lxc", "nas", "iot", "ap", "generic"] + +# Shared field schemas mirroring backend NodeBase / NodeUpdate (backend/app/schemas/nodes.py). +# create_node and update_node both expose these so the MCP is symmetric with what the +# backend already validates and stores. _dispatch forwards args verbatim, so any field +# advertised here is accepted by the backend. +_NODE_FIELDS = { + "label": {"type": "string"}, + "ip": {"type": "string"}, + "hostname": {"type": "string"}, + "mac": {"type": "string", "description": "MAC address."}, + "os": {"type": "string", "description": "Operating system / distribution."}, + "status": {"type": "string", "enum": ["online", "offline", "unknown", "pending"]}, + "check_method": {"type": "string", "description": "Status check method (ping, http, https, ssh, prometheus, tcp)."}, + "check_target": {"type": "string", "description": "Target host/URL used by the status check."}, + "services": {"type": "array", "items": {"type": "object"}, "description": "Running services detected or documented on the node."}, + "notes": {"type": "string", "description": "Free-text notes / documentation for the node."}, + "parent_id": {"type": "string", "description": "ID of the parent node (e.g. Proxmox host for a VM/LXC). Pass null to detach."}, + "container_mode": {"type": "boolean", "description": "Render this node as a container/group that can hold children."}, + "custom_icon": {"type": "string", "description": "Override icon name for the node."}, + "cpu_count": {"type": "integer", "description": "Number of CPU cores/threads."}, + "cpu_model": {"type": "string", "description": "CPU model name."}, + "ram_gb": {"type": "number", "description": "RAM in gigabytes."}, + "disk_gb": {"type": "number", "description": "Disk capacity in gigabytes."}, + "show_hardware": {"type": "boolean", "description": "Display hardware specs on the node card."}, + "properties": { + "type": "array", + "description": "Arbitrary key/value metadata shown on the node.", + "items": { + "type": "object", + "required": ["name", "value"], + "properties": { + "name": {"type": "string"}, + "value": {"type": "string"}, + }, + }, + }, +} + + +def _build_tools() -> list[Tool]: + create_node_props = { + "type": {"type": "string", "enum": NODE_TYPES}, + **_NODE_FIELDS, + } + create_node_props["status"] = {**_NODE_FIELDS["status"], "default": "unknown"} + + update_node_props = { + "id": {"type": "string"}, + "type": {"type": "string", "enum": NODE_TYPES}, + **_NODE_FIELDS, + } + + return [ + Tool(name="create_node", description="Add a new node to the homelab canvas", inputSchema={ + "type": "object", + "required": ["type", "label"], + "properties": create_node_props, + }), + Tool(name="update_node", description="Update an existing node", inputSchema={ + "type": "object", + "required": ["id"], + "properties": update_node_props, + }), + Tool(name="delete_node", description="Delete a node from the canvas", inputSchema={ + "type": "object", + "required": ["id"], + "properties": {"id": {"type": "string"}}, + }), + Tool(name="create_edge", description="Create a network link between two nodes", inputSchema={ + "type": "object", + "required": ["source", "target"], + "properties": { + "source": {"type": "string"}, + "target": {"type": "string"}, + "type": {"type": "string", "enum": ["ethernet", "wifi", "iot", "vlan", "virtual"], "default": "ethernet"}, + "label": {"type": "string"}, + }, + }), + Tool(name="delete_edge", description="Delete a network link", inputSchema={ + "type": "object", + "required": ["id"], + "properties": {"id": {"type": "string"}}, + }), + Tool(name="trigger_scan", description="Trigger a network discovery scan", inputSchema={ + "type": "object", + "properties": { + "ranges": {"type": "array", "items": {"type": "string"}, "description": "CIDR ranges to scan (uses configured defaults if omitted)"}, + }, + }), + Tool(name="approve_device", description="Approve a pending discovered device and create a node", inputSchema={ + "type": "object", + "required": ["id"], + "properties": { + "id": {"type": "string"}, + "type": {"type": "string", "enum": NODE_TYPES, "default": "generic"}, + "label": {"type": "string"}, + }, + }), + Tool(name="hide_device", description="Hide a pending discovered device", inputSchema={ + "type": "object", + "required": ["id"], + "properties": {"id": {"type": "string"}}, + }), + Tool(name="get_canvas", description="Get the full canvas: all nodes and edges in the homelab topology", inputSchema={ + "type": "object", + "properties": {}, + }), + Tool(name="list_nodes", description="List all nodes (devices) in the homelab", inputSchema={ + "type": "object", + "properties": {}, + }), + Tool(name="list_pending_devices", description="List devices discovered by scan but not yet approved or hidden", inputSchema={ + "type": "object", + "properties": {}, + }), + ] + + +TOOLS = _build_tools() + + def register_tools(server: Server): @server.list_tools() async def list_tools(): - return [ - Tool(name="create_node", description="Add a new node to the homelab canvas", inputSchema={ - "type": "object", - "required": ["type", "label"], - "properties": { - "type": {"type": "string", "enum": ["isp","router","switch","server","proxmox","vm","lxc","nas","iot","ap","generic"]}, - "label": {"type": "string"}, - "ip": {"type": "string"}, - "hostname": {"type": "string"}, - "status": {"type": "string", "enum": ["online","offline","unknown","pending"], "default": "unknown"}, - }, - }), - Tool(name="update_node", description="Update an existing node", inputSchema={ - "type": "object", - "required": ["id"], - "properties": { - "id": {"type": "string"}, - "label": {"type": "string"}, - "ip": {"type": "string"}, - "hostname": {"type": "string"}, - "status": {"type": "string"}, - "parent_id": {"type": "string", "description": "ID of the parent node (e.g. Proxmox host for a VM/LXC). Pass null to detach."}, - }, - }), - Tool(name="delete_node", description="Delete a node from the canvas", inputSchema={ - "type": "object", - "required": ["id"], - "properties": {"id": {"type": "string"}}, - }), - Tool(name="create_edge", description="Create a network link between two nodes", inputSchema={ - "type": "object", - "required": ["source", "target"], - "properties": { - "source": {"type": "string"}, - "target": {"type": "string"}, - "type": {"type": "string", "enum": ["ethernet","wifi","iot","vlan","virtual"], "default": "ethernet"}, - "label": {"type": "string"}, - }, - }), - Tool(name="delete_edge", description="Delete a network link", inputSchema={ - "type": "object", - "required": ["id"], - "properties": {"id": {"type": "string"}}, - }), - Tool(name="trigger_scan", description="Trigger a network discovery scan", inputSchema={ - "type": "object", - "properties": { - "ranges": {"type": "array", "items": {"type": "string"}, "description": "CIDR ranges to scan (uses configured defaults if omitted)"}, - }, - }), - Tool(name="approve_device", description="Approve a pending discovered device and create a node", inputSchema={ - "type": "object", - "required": ["id"], - "properties": { - "id": {"type": "string"}, - "type": {"type": "string", "enum": ["isp","router","switch","server","proxmox","vm","lxc","nas","iot","ap","generic"], "default": "generic"}, - "label": {"type": "string"}, - }, - }), - Tool(name="hide_device", description="Hide a pending discovered device", inputSchema={ - "type": "object", - "required": ["id"], - "properties": {"id": {"type": "string"}}, - }), - Tool(name="get_canvas", description="Get the full canvas: all nodes and edges in the homelab topology", inputSchema={ - "type": "object", - "properties": {}, - }), - Tool(name="list_nodes", description="List all nodes (devices) in the homelab", inputSchema={ - "type": "object", - "properties": {}, - }), - Tool(name="list_pending_devices", description="List devices discovered by scan but not yet approved or hidden", inputSchema={ - "type": "object", - "properties": {}, - }), - ] + return TOOLS @server.call_tool() async def call_tool(name: str, arguments: dict): @@ -94,7 +140,11 @@ def register_tools(server: Server): def _slim_canvas(raw: dict) -> dict: """Strip React Flow layout/style fields — keep only semantic data for AI use.""" - NODE_KEEP = {"id", "type", "label", "ip", "hostname", "status", "services", "description", "parentId"} + NODE_KEEP = { + "id", "type", "label", "ip", "hostname", "mac", "os", "status", "services", + "notes", "description", "properties", "cpu_count", "cpu_model", "ram_gb", + "disk_gb", "parentId", + } EDGE_KEEP = {"id", "source", "target", "type", "label"} def slim_node(n: dict) -> dict: diff --git a/mcp/tests/test_tools.py b/mcp/tests/test_tools.py index c0b4bcf..be1f7b3 100644 --- a/mcp/tests/test_tools.py +++ b/mcp/tests/test_tools.py @@ -1,6 +1,6 @@ import pytest from unittest.mock import AsyncMock, patch -from app.tools import _dispatch +from app.tools import TOOLS, _dispatch @pytest.fixture @@ -32,6 +32,39 @@ async def test_update_node_parent_id(mock_backend): mock_backend.patch.assert_called_once_with("/api/v1/nodes/42", {"parent_id": "proxmox-1"}) +@pytest.mark.anyio +async def test_create_node_full_properties(mock_backend): + args = { + "type": "proxmox", + "label": "pve1", + "os": "Proxmox VE 8", + "notes": "Main hypervisor", + "services": [{"name": "ssh", "port": 22}], + "cpu_count": 16, + "cpu_model": "Ryzen 9 5950X", + "ram_gb": 64, + "disk_gb": 2000, + "show_hardware": True, + "properties": [{"name": "rack", "value": "A1"}], + } + await _dispatch("create_node", dict(args)) + # All extra fields forwarded to the backend unchanged. + mock_backend.post.assert_called_once_with("/api/v1/nodes", args) + + +@pytest.mark.anyio +async def test_update_node_properties(mock_backend): + await _dispatch("update_node", { + "id": "42", + "os": "Debian 12", + "properties": [{"name": "role", "value": "db"}], + }) + mock_backend.patch.assert_called_once_with("/api/v1/nodes/42", { + "os": "Debian 12", + "properties": [{"name": "role", "value": "db"}], + }) + + @pytest.mark.anyio async def test_delete_node(mock_backend): await _dispatch("delete_node", {"id": "42"}) @@ -100,6 +133,55 @@ async def test_get_canvas(mock_backend): assert "viewport" not in result +@pytest.mark.anyio +async def test_get_canvas_keeps_documentation_fields(mock_backend): + mock_backend.get = AsyncMock(return_value={ + "nodes": [ + { + "id": "n1", + "type": "proxmox", + "position": {"x": 0, "y": 0}, + "data": { + "label": "pve1", + "os": "Proxmox VE 8", + "notes": "Main hypervisor", + "cpu_count": 16, + "ram_gb": 64, + "properties": [{"name": "rack", "value": "A1"}], + }, + } + ], + "edges": [], + }) + result = await _dispatch("get_canvas", {}) + node = result["nodes"][0] + assert node["os"] == "Proxmox VE 8" + assert node["notes"] == "Main hypervisor" + assert node["cpu_count"] == 16 + assert node["ram_gb"] == 64 + assert node["properties"] == [{"name": "rack", "value": "A1"}] + + +def _tool_schema(name: str) -> dict: + tool = next(t for t in TOOLS if t.name == name) + return tool.inputSchema["properties"] + + +def test_create_node_schema_exposes_full_node_fields(): + props = _tool_schema("create_node") + for field in ("os", "notes", "services", "cpu_count", "ram_gb", "disk_gb", "properties", "mac"): + assert field in props, f"create_node schema missing {field}" + # type stays an enum of the canonical node types + assert "enum" in props["type"] + + +def test_update_node_schema_exposes_full_node_fields(): + props = _tool_schema("update_node") + for field in ("os", "notes", "services", "cpu_count", "ram_gb", "disk_gb", "properties", "mac"): + assert field in props, f"update_node schema missing {field}" + assert "id" in props + + @pytest.mark.anyio async def test_list_nodes(mock_backend): mock_backend.get = AsyncMock(return_value=[{"id": "1", "label": "Freebox"}])