diff --git a/backend/app/api/routes/edges.py b/backend/app/api/routes/edges.py index ba49256..4d63530 100644 --- a/backend/app/api/routes/edges.py +++ b/backend/app/api/routes/edges.py @@ -4,7 +4,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps import get_current_user from app.db.database import get_db -from app.db.models import Edge +from app.db.models import Design, Edge from app.schemas.edges import EdgeCreate, EdgeResponse, EdgeUpdate router = APIRouter() @@ -18,7 +18,14 @@ async def list_edges(db: AsyncSession = Depends(get_db), _: str = Depends(get_cu @router.post("", response_model=EdgeResponse, status_code=status.HTTP_201_CREATED) async def create_edge(body: EdgeCreate, db: AsyncSession = Depends(get_db), _: str = Depends(get_current_user)) -> Edge: - edge = Edge(**body.model_dump()) + data = body.model_dump() + # Same reconciliation as nodes: clients omitting design_id (MCP write tools) + # would create design_id=null edges that never render until a restart. + # Fall back to the first design so the edge attaches to a canvas. + if data.get("design_id") is None: + first_design = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar() + data["design_id"] = first_design.id if first_design else None + edge = Edge(**data) db.add(edge) await db.commit() await db.refresh(edge) diff --git a/backend/app/api/routes/nodes.py b/backend/app/api/routes/nodes.py index a773995..30d3f28 100644 --- a/backend/app/api/routes/nodes.py +++ b/backend/app/api/routes/nodes.py @@ -4,7 +4,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps import get_current_user from app.db.database import get_db -from app.db.models import Node +from app.db.models import Design, Node from app.schemas.nodes import NodeCreate, NodeResponse, NodeUpdate router = APIRouter() @@ -18,7 +18,15 @@ async def list_nodes(db: AsyncSession = Depends(get_db), _: str = Depends(get_cu @router.post("", response_model=NodeResponse, status_code=status.HTTP_201_CREATED) async def create_node(body: NodeCreate, db: AsyncSession = Depends(get_db), _: str = Depends(get_current_user)) -> Node: - node = Node(**body.model_dump()) + data = body.model_dump() + # Attach to a design so the node lands on a canvas. Clients that don't send a + # design_id (e.g. the MCP write tools) would otherwise create design_id=null + # nodes that exist in the DB but never render in the UI until a container + # restart reconciles them. Fall back to the first design, matching bulk-approve. + if data.get("design_id") is None: + first_design = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar() + data["design_id"] = first_design.id if first_design else None + node = Node(**data) db.add(node) await db.commit() await db.refresh(node) diff --git a/backend/app/schemas/edges.py b/backend/app/schemas/edges.py index 05db453..85964d8 100644 --- a/backend/app/schemas/edges.py +++ b/backend/app/schemas/edges.py @@ -51,6 +51,7 @@ class EdgeUpdate(BaseModel): class EdgeResponse(EdgeBase): id: str + design_id: str | None = None created_at: datetime model_config = {"from_attributes": True} diff --git a/backend/tests/test_edges.py b/backend/tests/test_edges.py index 3665650..e9a560d 100644 --- a/backend/tests/test_edges.py +++ b/backend/tests/test_edges.py @@ -97,6 +97,37 @@ async def test_create_edge_requires_auth(client: AsyncClient, two_nodes): assert res.status_code == 401 +async def test_create_edge_without_design_id_falls_back_to_first_design(client: AsyncClient, headers: dict, two_nodes): + # Regression for #225: MCP create_edge sent no design_id, so edges were + # persisted with design_id=null and never rendered until a restart. + src, tgt = two_nodes + design = await client.post("/api/v1/designs", json={"name": "Primary"}, headers=headers) + design_id = design.json()["id"] + + res = await client.post( + "/api/v1/edges", + json={"source": src, "target": tgt, "type": "ethernet"}, + headers=headers, + ) + assert res.status_code == 201 + assert res.json()["design_id"] == design_id + + +async def test_create_edge_respects_explicit_design_id(client: AsyncClient, headers: dict, two_nodes): + src, tgt = two_nodes + await client.post("/api/v1/designs", json={"name": "First"}, headers=headers) + second = await client.post("/api/v1/designs", json={"name": "Second"}, headers=headers) + second_id = second.json()["id"] + + res = await client.post( + "/api/v1/edges", + json={"source": src, "target": tgt, "type": "ethernet", "design_id": second_id}, + headers=headers, + ) + assert res.status_code == 201 + assert res.json()["design_id"] == second_id + + async def test_create_cluster_edge_with_handles(client: AsyncClient, headers: dict, two_nodes): src, tgt = two_nodes res = await client.post( diff --git a/backend/tests/test_nodes.py b/backend/tests/test_nodes.py index 048e5c3..fafe670 100644 --- a/backend/tests/test_nodes.py +++ b/backend/tests/test_nodes.py @@ -67,6 +67,50 @@ async def test_update_node_not_found(client: AsyncClient, headers: dict): assert res.status_code == 404 +async def test_create_node_without_design_id_falls_back_to_first_design(client: AsyncClient, headers: dict): + # Regression for #225: MCP create_node sent no design_id, so nodes were + # persisted with design_id=null and never rendered on the canvas until a + # container restart reconciled them. They must attach to a design on create. + design = await client.post("/api/v1/designs", json={"name": "Primary"}, headers=headers) + design_id = design.json()["id"] + + res = await client.post( + "/api/v1/nodes", + json={"type": "generic", "label": "mcp-node", "ip": "192.168.18.99"}, + headers=headers, + ) + assert res.status_code == 201 + assert res.json()["design_id"] == design_id + + +async def test_create_node_respects_explicit_design_id(client: AsyncClient, headers: dict): + # When a design_id is supplied it must win over the first-design fallback. + first = await client.post("/api/v1/designs", json={"name": "First"}, headers=headers) + second = await client.post("/api/v1/designs", json={"name": "Second"}, headers=headers) + second_id = second.json()["id"] + assert first.json()["id"] != second_id + + res = await client.post( + "/api/v1/nodes", + json={"type": "generic", "label": "n", "design_id": second_id}, + headers=headers, + ) + assert res.status_code == 201 + assert res.json()["design_id"] == second_id + + +async def test_create_node_without_any_design_stays_null(client: AsyncClient, headers: dict): + # No designs exist yet: fallback can't invent one, so design_id stays null + # rather than erroring. + res = await client.post( + "/api/v1/nodes", + json={"type": "generic", "label": "orphan"}, + headers=headers, + ) + assert res.status_code == 201 + assert res.json()["design_id"] is None + + async def test_delete_node_not_found(client: AsyncClient, headers: dict): res = await client.delete("/api/v1/nodes/nonexistent", headers=headers) assert res.status_code == 404 diff --git a/mcp/app/resources.py b/mcp/app/resources.py index e14f1ed..386f416 100644 --- a/mcp/app/resources.py +++ b/mcp/app/resources.py @@ -21,6 +21,10 @@ ROUTES = { async def read_resource(uri: str) -> list[TextContent]: + # The MCP framework hands us a pydantic AnyUrl, not a plain str, so string + # ops like .startswith / dict lookups blow up with + # "'AnyUrl' object has no attribute 'startswith'". Coerce to str first. + uri = str(uri) if uri.startswith("homelable://nodes/") and uri != "homelable://nodes/": node_id = uri.split("/")[-1] data = await backend.get(f"/api/v1/nodes/{node_id}") diff --git a/mcp/tests/test_resources.py b/mcp/tests/test_resources.py index 36f5d7f..5bc0c2d 100644 --- a/mcp/tests/test_resources.py +++ b/mcp/tests/test_resources.py @@ -1,4 +1,5 @@ import pytest +from pydantic import AnyUrl from unittest.mock import AsyncMock, patch from app.resources import read_resource @@ -45,3 +46,24 @@ async def test_read_scan_pending(mock_backend): async def test_read_unknown_uri(mock_backend): with pytest.raises(ValueError, match="Unknown resource URI"): await read_resource("homelable://unknown") + + +@pytest.mark.anyio +async def test_read_resource_accepts_anyurl(mock_backend): + # Regression for #225: the MCP framework calls the handler with a pydantic + # AnyUrl, not a str, which raised "'AnyUrl' object has no attribute + # 'startswith'". The handler must coerce to str and still route correctly. + await read_resource(AnyUrl("homelable://canvas")) + mock_backend.get.assert_called_once_with("/api/v1/canvas") + + +@pytest.mark.anyio +async def test_read_edges_anyurl(mock_backend): + await read_resource(AnyUrl("homelable://edges")) + mock_backend.get.assert_called_once_with("/api/v1/edges") + + +@pytest.mark.anyio +async def test_read_single_node_anyurl(mock_backend): + await read_resource(AnyUrl("homelable://nodes/abc123")) + mock_backend.get.assert_called_once_with("/api/v1/nodes/abc123")