Merge pull request #266 from nicolabottini/feat/mcp-design-targeting

feat: MCP design/canvas targeting + auto-position + auto-edge-handles

backend/app/api/routes/edges.py

@@ -4,11 +4,56 @@ 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 Design, Edge
+from app.db.models import Design, Edge, Node
 from app.schemas.edges import EdgeCreate, EdgeResponse, EdgeUpdate
 
 router = APIRouter()
 
+# ---------------------------------------------------------------------------
+# Auto-handle helpers
+# ---------------------------------------------------------------------------
+
+
+async def _abs_y(db: AsyncSession, node_id: str) -> float | None:
+    """Resolve the approximate absolute canvas Y of a node.
+
+    Walks up the parent chain (up to 8 levels) and accumulates pos_y offsets so
+    that children inside containers are compared correctly against top-level nodes.
+    Returns None when the node is not found.
+    """
+    node = await db.get(Node, node_id)
+    if node is None:
+        return None
+    y = node.pos_y
+    current = node
+    for _ in range(8):
+        if current.parent_id is None:
+            break
+        parent = await db.get(Node, current.parent_id)
+        if parent is None:
+            break
+        y += parent.pos_y
+        current = parent
+    return y
+
+
+async def _auto_handles(
+    db: AsyncSession, source_id: str, target_id: str
+) -> tuple[str, str]:
+    """Return (source_handle, target_handle) that reflect the upstream/downstream
+    relationship between two nodes.
+
+    - Source above target (lower Y value) → downstream flow: exit bottom, enter top
+    - Source below target → upstream flow: exit top, enter bottom
+    - Equal or unknown → default to bottom/top-t (most common topology direction)
+    """
+    src_y = await _abs_y(db, source_id)
+    tgt_y = await _abs_y(db, target_id)
+
+    if src_y is None or tgt_y is None or src_y <= tgt_y:
+        return "bottom", "top-t"
+    return "top", "bottom-t"
+
 
 @router.get("", response_model=list[EdgeResponse])
 async def list_edges(db: AsyncSession = Depends(get_db), _: str = Depends(get_current_user)) -> list[Edge]:
@@ -25,6 +70,18 @@ async def create_edge(body: EdgeCreate, db: AsyncSession = Depends(get_db), _: s
     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
+
+    # Auto-assign source/target handles when the caller omits them.
+    # Compares the canvas Y positions of both nodes so that the edge always exits
+    # the upstream node's bottom and enters the downstream node's top (or vice versa
+    # for reverse flows), matching the UI convention for top-to-bottom topologies.
+    if data.get("source_handle") is None or data.get("target_handle") is None:
+        auto_src, auto_tgt = await _auto_handles(db, data["source"], data["target"])
+        if data.get("source_handle") is None:
+            data["source_handle"] = auto_src
+        if data.get("target_handle") is None:
+            data["target_handle"] = auto_tgt
+
     edge = Edge(**data)
     db.add(edge)
     await db.commit()

backend/app/api/routes/nodes.py

@@ -10,6 +10,49 @@ from app.services.node_dedupe import find_duplicate_node
 
 router = APIRouter()
 
+# ---------------------------------------------------------------------------
+# Auto-positioning helpers
+# ---------------------------------------------------------------------------
+
+# Canvas grid used when placing nodes without explicit coordinates.
+# Each slot is wide/tall enough that a normal node card fits without overlap.
+_SLOT_W = 200.0
+_SLOT_H = 100.0
+_MAX_COLS = 7
+
+
+async def _find_free_position(db: AsyncSession, design_id: str | None) -> tuple[float, float]:
+    """Return (x, y) for a new root-level node that doesn't collide with existing ones.
+
+    Snaps existing root nodes to a virtual grid and returns the first unoccupied
+    cell, scanning left-to-right then top-to-bottom.
+    """
+    result = await db.execute(
+        select(Node.pos_x, Node.pos_y).where(
+            Node.design_id == design_id,
+            Node.parent_id.is_(None),
+        )
+    )
+    positions = list(result.all())
+    if not positions:
+        return 0.0, 0.0
+
+    # Snap each existing node to its true grid cell (negatives kept as-is). The
+    # search below only visits col >= 0 / row >= 0, so nodes parked in negative
+    # space never falsely block — or falsely free — a positive slot.
+    occupied: set[tuple[int, int]] = set()
+    for (px, py) in positions:
+        col = round(px / _SLOT_W)
+        row = round(py / _SLOT_H)
+        occupied.add((col, row))
+
+    for row in range(10_000):
+        for col in range(_MAX_COLS):
+            if (col, row) not in occupied:
+                return col * _SLOT_W, row * _SLOT_H
+
+    return 0.0, 0.0  # unreachable in practice
+
 
 @router.get("", response_model=list[NodeResponse])
 async def list_nodes(db: AsyncSession = Depends(get_db), _: str = Depends(get_current_user)) -> list[Node]:
@@ -38,6 +81,22 @@ async def create_node(body: NodeCreate, db: AsyncSession = Depends(get_db), _: s
         if dup is not None:
             raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=dup)
 
+    # Auto-position: when pos_x / pos_y are omitted (None), find a free canvas
+    # slot so the new node doesn't land on top of an existing one.
+    # Child nodes (parent_id set) use (0, 0) relative to their parent instead.
+    if data["pos_x"] is None or data["pos_y"] is None:
+        if data.get("parent_id") is None:
+            auto_x, auto_y = await _find_free_position(db, data["design_id"])
+            if data["pos_x"] is None:
+                data["pos_x"] = auto_x
+            if data["pos_y"] is None:
+                data["pos_y"] = auto_y
+        else:
+            if data["pos_x"] is None:
+                data["pos_x"] = 0.0
+            if data["pos_y"] is None:
+                data["pos_y"] = 0.0
+
     node = Node(**data)
     db.add(node)
     await db.commit()

backend/app/schemas/nodes.py

@@ -38,6 +38,10 @@ class NodeBase(BaseModel):
 
 
 class NodeCreate(NodeBase):
+    # Override pos_x/pos_y so callers can omit them; None signals "auto-place".
+    # The create_node route resolves None to a free grid slot before persisting.
+    pos_x: float | None = None  # type: ignore[assignment]
+    pos_y: float | None = None  # type: ignore[assignment]
     design_id: str | None = None
     # When a node with the same ip/mac already exists on the target design, the
     # create/approve endpoints reject with 409 so the UI can ask the user. Set

backend/tests/test_edges.py

@@ -220,3 +220,113 @@ async def test_source_and_target_handle_persist_through_update(client: AsyncClie
     assert data["source_handle"] == "cluster-right"
     assert data["target_handle"] == "cluster-left"
     assert data["label"] == "corosync"
+
+
+# ---------------------------------------------------------------------------
+# Auto edge handles (issue #265): omitting source_handle/target_handle lets
+# the backend infer up/downstream sides from the nodes' absolute canvas Y.
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture
+async def stacked_nodes(client: AsyncClient, headers: dict):
+    """Two root nodes at known, distinct Y positions (top at y=0, bottom at y=300)."""
+    top = (
+        await client.post(
+            "/api/v1/nodes",
+            json={"type": "router", "label": "Top", "status": "online", "pos_x": 0, "pos_y": 0},
+            headers=headers,
+        )
+    ).json()
+    bottom = (
+        await client.post(
+            "/api/v1/nodes",
+            json={"type": "switch", "label": "Bottom", "status": "online", "pos_x": 0, "pos_y": 300},
+            headers=headers,
+        )
+    ).json()
+    return top["id"], bottom["id"]
+
+
+async def test_create_edge_auto_handles_source_above_target(client: AsyncClient, headers: dict, stacked_nodes):
+    """Source above target -> downstream flow: exit bottom, enter top-t."""
+    top, bottom = stacked_nodes
+    res = await client.post(
+        "/api/v1/edges", json={"source": top, "target": bottom, "type": "ethernet"}, headers=headers
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert data["source_handle"] == "bottom"
+    assert data["target_handle"] == "top-t"
+
+
+async def test_create_edge_auto_handles_source_below_target(client: AsyncClient, headers: dict, stacked_nodes):
+    """Source below target -> upstream flow: exit top, enter bottom-t."""
+    top, bottom = stacked_nodes
+    res = await client.post(
+        "/api/v1/edges", json={"source": bottom, "target": top, "type": "ethernet"}, headers=headers
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert data["source_handle"] == "top"
+    assert data["target_handle"] == "bottom-t"
+
+
+async def test_create_edge_auto_handles_equal_y_defaults_downstream(client: AsyncClient, headers: dict, two_nodes):
+    """Same Y (auto-placed side by side) falls back to the default bottom/top-t."""
+    src, tgt = two_nodes
+    res = await client.post(
+        "/api/v1/edges", json={"source": src, "target": tgt, "type": "ethernet"}, headers=headers
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert data["source_handle"] == "bottom"
+    assert data["target_handle"] == "top-t"
+
+
+async def test_create_edge_partial_handle_auto_fills_the_other(client: AsyncClient, headers: dict, stacked_nodes):
+    """An explicit source_handle is kept; the omitted target_handle is inferred."""
+    top, bottom = stacked_nodes
+    res = await client.post(
+        "/api/v1/edges",
+        json={"source": top, "target": bottom, "type": "ethernet", "source_handle": "cluster-right"},
+        headers=headers,
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert data["source_handle"] == "cluster-right"
+    assert data["target_handle"] == "top-t"
+
+
+async def test_create_edge_child_node_abs_y_resolved_through_parent(client: AsyncClient, headers: dict):
+    """A child's absolute Y = parent Y + its own pos_y, so a VM low inside a high
+    container still resolves as downstream of a node above the container."""
+    parent = (
+        await client.post(
+            "/api/v1/nodes",
+            json={"type": "proxmox", "label": "PVE", "status": "online", "container_mode": True, "pos_x": 0, "pos_y": 0},
+            headers=headers,
+        )
+    ).json()
+    child = (
+        await client.post(
+            "/api/v1/nodes",
+            json={"type": "vm", "label": "VM", "status": "online", "parent_id": parent["id"], "pos_x": 0, "pos_y": 50},
+            headers=headers,
+        )
+    ).json()
+    below = (
+        await client.post(
+            "/api/v1/nodes",
+            json={"type": "server", "label": "Below", "status": "online", "pos_x": 0, "pos_y": 500},
+            headers=headers,
+        )
+    ).json()
+    # child abs Y = 0 + 50 = 50, below at 500 -> child is upstream.
+    res = await client.post(
+        "/api/v1/edges", json={"source": child["id"], "target": below["id"], "type": "virtual"}, headers=headers
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert data["source_handle"] == "bottom"
+    assert data["target_handle"] == "top-t"

backend/tests/test_nodes.py

@@ -286,3 +286,80 @@ async def test_properties_icon_can_be_null(client: AsyncClient, headers: dict):
     )
     assert create.status_code == 201
     assert create.json()["properties"] == props
+
+
+# ---------------------------------------------------------------------------
+# Auto-positioning (issue #265): omitting pos_x/pos_y snaps the node to the
+# first free 200x100 grid slot instead of stacking everything at (0, 0).
+# ---------------------------------------------------------------------------
+
+
+async def test_create_node_auto_positions_first_at_origin(client: AsyncClient, headers: dict):
+    """First root node with no coordinates lands at (0, 0)."""
+    res = await client.post(
+        "/api/v1/nodes", json={"type": "server", "label": "A", "status": "unknown"}, headers=headers
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert (data["pos_x"], data["pos_y"]) == (0.0, 0.0)
+
+
+async def test_create_node_auto_position_avoids_collision(client: AsyncClient, headers: dict):
+    """A second auto-placed root node takes the next free grid cell, not (0, 0)."""
+    first = await client.post(
+        "/api/v1/nodes", json={"type": "server", "label": "A", "status": "unknown"}, headers=headers
+    )
+    assert (first.json()["pos_x"], first.json()["pos_y"]) == (0.0, 0.0)
+
+    second = await client.post(
+        "/api/v1/nodes", json={"type": "server", "label": "B", "status": "unknown"}, headers=headers
+    )
+    assert second.status_code == 201
+    # Next free cell in row 0 is column 1 -> x = 1 * 200.
+    assert (second.json()["pos_x"], second.json()["pos_y"]) == (200.0, 0.0)
+
+
+async def test_create_node_child_defaults_to_parent_origin(client: AsyncClient, headers: dict):
+    """A child node (parent_id set) with no coordinates defaults to (0, 0) relative to its parent."""
+    parent = (
+        await client.post(
+            "/api/v1/nodes",
+            json={"type": "proxmox", "label": "PVE", "status": "online", "container_mode": True},
+            headers=headers,
+        )
+    ).json()
+    res = await client.post(
+        "/api/v1/nodes",
+        json={"type": "vm", "label": "VM1", "status": "online", "parent_id": parent["id"]},
+        headers=headers,
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert (data["pos_x"], data["pos_y"]) == (0.0, 0.0)
+
+
+async def test_create_node_explicit_position_preserved(client: AsyncClient, headers: dict):
+    """Explicit coordinates are honored, never auto-placed."""
+    res = await client.post(
+        "/api/v1/nodes",
+        json={"type": "server", "label": "A", "status": "unknown", "pos_x": 512.0, "pos_y": 384.0},
+        headers=headers,
+    )
+    assert res.status_code == 201
+    data = res.json()
+    assert (data["pos_x"], data["pos_y"]) == (512.0, 384.0)
+
+
+async def test_create_node_explicit_zero_position_preserved(client: AsyncClient, headers: dict):
+    """pos=0 is an explicit value, not 'omitted' — auto-position must not treat 0 as None."""
+    await client.post(
+        "/api/v1/nodes", json={"type": "server", "label": "A", "status": "unknown"}, headers=headers
+    )
+    # Second node explicitly pinned to (0, 0) even though the cell is taken.
+    res = await client.post(
+        "/api/v1/nodes",
+        json={"type": "server", "label": "B", "status": "unknown", "pos_x": 0, "pos_y": 0},
+        headers=headers,
+    )
+    assert res.status_code == 201
+    assert (res.json()["pos_x"], res.json()["pos_y"]) == (0.0, 0.0)

mcp/app/tools.py

@@ -21,6 +21,10 @@ _NODE_FIELDS = {
     "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."},
+    "pos_x":         {"type": "number", "description": "X position on the canvas. Omit on create to auto-place (root nodes only)."},
+    "pos_y":         {"type": "number", "description": "Y position on the canvas. Omit on create to auto-place (root nodes only). For child nodes this is relative to the parent container."},
+    "width":         {"type": "number", "description": "Width of the node card in pixels. Mainly useful for container nodes."},
+    "height":        {"type": "number", "description": "Height of the node card in pixels. Mainly useful for container nodes."},
     "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."},
@@ -43,11 +47,20 @@ _NODE_FIELDS = {
     },
 }
 
+# Optional design/canvas selector. The backend attaches nodes/edges to the first
+# design when design_id is omitted (see backend nodes.py / edges.py), so these
+# tools stay backward compatible; pass design_id to target a specific canvas.
+# Use list_designs to discover the available IDs.
+_DESIGN_ID_FIELD = {
+    "design_id": {"type": "string", "description": "Target design/canvas ID. Omit to use the default (first) canvas; call list_designs to discover IDs."},
+}
+
 
 def _build_tools() -> list[Tool]:
     create_node_props = {
         "type": {"type": "string", "enum": NODE_TYPES},
         **_NODE_FIELDS,
+        **_DESIGN_ID_FIELD,
     }
     create_node_props["status"] = {**_NODE_FIELDS["status"], "default": "unknown"}
 
@@ -81,6 +94,7 @@ def _build_tools() -> list[Tool]:
                 "target": {"type": "string"},
                 "type":   {"type": "string", "enum": ["ethernet", "wifi", "iot", "vlan", "virtual"], "default": "ethernet"},
                 "label":  {"type": "string"},
+                **_DESIGN_ID_FIELD,
             },
         }),
         Tool(name="delete_edge", description="Delete a network link", inputSchema={
@@ -110,7 +124,7 @@ def _build_tools() -> list[Tool]:
         }),
         Tool(name="get_canvas", description="Get the full canvas: all nodes and edges in the homelab topology", inputSchema={
             "type": "object",
-            "properties": {},
+            "properties": {**_DESIGN_ID_FIELD},
         }),
         Tool(name="list_nodes", description="List all nodes (devices) in the homelab", inputSchema={
             "type": "object",
@@ -120,6 +134,19 @@ def _build_tools() -> list[Tool]:
             "type": "object",
             "properties": {},
         }),
+        Tool(name="list_designs", description="List all designs (canvases) with their IDs and node/group/text counts", inputSchema={
+            "type": "object",
+            "properties": {},
+        }),
+        Tool(name="create_design", description="Create a new design (canvas) and return it, including its id for use as design_id", inputSchema={
+            "type": "object",
+            "required": ["name"],
+            "properties": {
+                "name":        {"type": "string", "description": "Name of the new canvas."},
+                "icon":        {"type": "string", "description": "Icon name for the canvas (default: dashboard)."},
+                "design_type": {"type": "string", "description": "Design type (default: network)."},
+            },
+        }),
     ]
 
 
@@ -192,7 +219,9 @@ async def _dispatch(name: str, args: dict) -> dict:
         return await backend.post(f"/api/v1/scan/pending/{args['id']}/hide", {})
 
     if name == "get_canvas":
-        raw = await backend.get("/api/v1/canvas")
+        design_id = args.get("design_id")
+        path = f"/api/v1/canvas?design_id={design_id}" if design_id else "/api/v1/canvas"
+        raw = await backend.get(path)
         return _slim_canvas(raw)
 
     if name == "list_nodes":
@@ -201,4 +230,10 @@ async def _dispatch(name: str, args: dict) -> dict:
     if name == "list_pending_devices":
         return await backend.get("/api/v1/scan/pending")
 
+    if name == "list_designs":
+        return await backend.get("/api/v1/designs")
+
+    if name == "create_design":
+        return await backend.post("/api/v1/designs", args)
+
     raise ValueError(f"Unknown tool: {name}")

mcp/tests/test_tools.py

@@ -77,6 +77,21 @@ async def test_create_edge(mock_backend):
     mock_backend.post.assert_called_once_with("/api/v1/edges", {"source": "1", "target": "2", "type": "ethernet"})
 
 
+@pytest.mark.anyio
+async def test_create_node_with_design_id(mock_backend):
+    # design_id is forwarded to the backend, which attaches the node to that canvas.
+    args = {"type": "server", "label": "Proxmox", "design_id": "design-2"}
+    await _dispatch("create_node", dict(args))
+    mock_backend.post.assert_called_once_with("/api/v1/nodes", args)
+
+
+@pytest.mark.anyio
+async def test_create_edge_with_design_id(mock_backend):
+    args = {"source": "1", "target": "2", "type": "ethernet", "design_id": "design-2"}
+    await _dispatch("create_edge", dict(args))
+    mock_backend.post.assert_called_once_with("/api/v1/edges", args)
+
+
 @pytest.mark.anyio
 async def test_delete_edge(mock_backend):
     await _dispatch("delete_edge", {"id": "99"})
@@ -162,6 +177,21 @@ async def test_get_canvas_keeps_documentation_fields(mock_backend):
     assert node["properties"] == [{"name": "rack", "value": "A1"}]
 
 
+@pytest.mark.anyio
+async def test_get_canvas_with_design_id(mock_backend):
+    mock_backend.get = AsyncMock(return_value={"nodes": [], "edges": []})
+    await _dispatch("get_canvas", {"design_id": "design-2"})
+    mock_backend.get.assert_called_once_with("/api/v1/canvas?design_id=design-2")
+
+
+@pytest.mark.anyio
+async def test_get_canvas_without_design_id_uses_default(mock_backend):
+    # Backward compatible: no design_id -> unqualified canvas endpoint (first design).
+    mock_backend.get = AsyncMock(return_value={"nodes": [], "edges": []})
+    await _dispatch("get_canvas", {})
+    mock_backend.get.assert_called_once_with("/api/v1/canvas")
+
+
 def _tool_schema(name: str) -> dict:
     tool = next(t for t in TOOLS if t.name == name)
     return tool.inputSchema["properties"]
@@ -182,6 +212,19 @@ def test_update_node_schema_exposes_full_node_fields():
     assert "id" in props
 
 
+def test_design_id_exposed_on_canvas_targeting_tools():
+    # create_node/create_edge/get_canvas can target a specific canvas.
+    assert "design_id" in _tool_schema("create_node")
+    assert "design_id" in _tool_schema("create_edge")
+    assert "design_id" in _tool_schema("get_canvas")
+
+
+def test_update_node_schema_has_no_design_id():
+    # The backend NodeUpdate schema can't move a node between designs, so the
+    # update_node tool must not advertise design_id.
+    assert "design_id" not in _tool_schema("update_node")
+
+
 @pytest.mark.anyio
 async def test_list_nodes(mock_backend):
     mock_backend.get = AsyncMock(return_value=[{"id": "1", "label": "Freebox"}])
@@ -198,6 +241,27 @@ async def test_list_pending_devices(mock_backend):
     assert result == [{"id": "p1", "ip": "192.168.1.50"}]
 
 
+@pytest.mark.anyio
+async def test_list_designs(mock_backend):
+    mock_backend.get = AsyncMock(return_value=[{"id": "d1", "name": "Network Topology", "node_count": 12}])
+    result = await _dispatch("list_designs", {})
+    mock_backend.get.assert_called_once_with("/api/v1/designs")
+    assert result == [{"id": "d1", "name": "Network Topology", "node_count": 12}]
+
+
+@pytest.mark.anyio
+async def test_create_design(mock_backend):
+    mock_backend.post = AsyncMock(return_value={"id": "d2", "name": "Scan Devices"})
+    result = await _dispatch("create_design", {"name": "Scan Devices"})
+    mock_backend.post.assert_called_once_with("/api/v1/designs", {"name": "Scan Devices"})
+    assert result == {"id": "d2", "name": "Scan Devices"}
+
+
+def test_create_design_schema_requires_name():
+    tool = next(t for t in TOOLS if t.name == "create_design")
+    assert tool.inputSchema["required"] == ["name"]
+
+
 @pytest.mark.anyio
 async def test_unknown_tool():
     with pytest.raises(ValueError, match="Unknown tool"):