fix: attach MCP-created nodes/edges to a design (#225)
create_node/create_edge persisted rows with design_id=null when the client omitted it (the MCP write tools), so they existed in the DB but never rendered on the canvas until a container restart reconciled them. Both routes now fall back to the first design, matching bulk-approve. Also fix MCP resource reads (homelable://canvas, homelable://edges): the framework passes a pydantic AnyUrl, not a str, which raised "'AnyUrl' object has no attribute 'startswith'". Coerce to str. EdgeResponse now exposes design_id for symmetry with NodeResponse. ha-relevant: no
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}")
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user