fix(canvas): stop a self-parented node from freezing the canvas

A node recorded as its own parent (`parent_id = id`) made the child walk in
`translateWaypointsForMovedNodes` recurse forever. The resulting stack overflow
was thrown inside the `onNodesChange` reducer, so the whole state update was
discarded: the node still selected but would never move. It also renders
unparented, sitting wherever its stored coordinates put it rather than inside
the container it appears to belong to.

Guard the walk, then close every path that could write the row:

- `propagate` keeps a `walked` set, matching the cycle guard `orderParentsFirst`
  already carries. Covers longer cycles (a -> b -> a) too, not just self-parent.
- `updateNode` / `addNode` drop a `parent_id` equal to the node's own id. The
  key is removed rather than nulled, so the rest of the edit still lands and an
  existing real parent is left alone.
- `importYaml` resolves parents by label, so a node naming its own label — or a
  duplicate label mapping back to it — imported as its own parent. Warn and skip.
- `node_dedupe` re-pointed every child of a duplicate at the canonical node,
  including the canonical node itself when it had been nested under one of its
  own duplicates. Detach instead, mirroring the self-loop edge deletion below it.
- `NodeSave` and the node PATCH route normalize it away. Dropped rather than
  rejected: a canvas that already carries the bad row must still be able to save,
  and a 422 would cost the user the whole save.

`_repair_self_parent_nodes` clears what is already persisted at startup. The
real parent is not recoverable from the row, so NULL returns the node to the top
level where the user can re-nest it. Idempotent, and never fatal to boot.

A longer cycle is deliberately left alone by the repair — there is no single
right link to cut, and the runtime guard keeps the canvas usable either way.

ha-relevant: yes

backend/app/api/routes/nodes.py

@@ -150,6 +150,11 @@ async def update_node(
     if not node:
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Node not found")
     sent = body.model_dump(exclude_unset=True)
+    # A node can never be its own parent: the canvas' parent walks assume an
+    # acyclic tree and a self-parent row freezes the node on screen (#370).
+    # Dropped rather than rejected so the rest of the edit still lands.
+    if sent.get("parent_id") == node_id:
+        sent.pop("parent_id")
     for field, value in node_columns(sent).items():
         setattr(node, field, value)
     # The user edited the device, not just its drawing: push the facts down.

backend/app/db/database.py

@@ -495,6 +495,7 @@ async def init_db() -> None:
     await _drop_legacy_node_columns()
     await _seed_node_views()
     await _backfill_zone_size()
+    await _repair_self_parent_nodes()
 
 
 
@@ -813,6 +814,47 @@ async def _backfill_zone_size() -> None:
         logger.warning("Backfilling zone width/height failed: %s", exc)
 
 
+async def _repair_self_parent_nodes() -> None:
+    """Detach any node that is recorded as its own parent.
+
+    Several write paths could persist ``parent_id = id`` before the guards
+    added alongside this repair: a YAML import resolving a parent by a label
+    that mapped back to the node, the node dedupe re-pointing a canonical node
+    that had been nested under one of its own duplicates, and any client PATCH
+    or canvas save, neither of which validated it (#370).
+
+    The row is fatal on the canvas: the parent walks assume an acyclic tree, so
+    dragging the node overflowed the stack inside the change reducer and the
+    move was silently dropped — the node selected but would not move. It also
+    renders unparented, so it sits wherever its stored coordinates put it
+    rather than inside the container it appears to belong to.
+
+    Clearing the column is the only safe repair: the real parent is not
+    recoverable from the row, and NULL simply returns the node to the top level
+    where the user can re-nest it. Idempotent — a second run matches nothing.
+
+    Never fatal: a failure here leaves the row as it was, and the runtime cycle
+    guards keep the canvas usable either way.
+    """
+    try:
+        async with engine.begin() as conn:
+            rows = (
+                await conn.exec_driver_sql(
+                    "SELECT id, label FROM nodes WHERE parent_id IS NOT NULL AND parent_id = id"
+                )
+            ).fetchall()
+            if not rows:
+                return
+            await conn.exec_driver_sql(
+                "UPDATE nodes SET parent_id = NULL WHERE parent_id IS NOT NULL AND parent_id = id"
+            )
+            for node_id, label in rows:
+                logger.info("Detached self-parented node %s (%s)", node_id, label)
+            logger.info("Repaired %d node(s) recorded as their own parent", len(rows))
+    except Exception as exc:  # pragma: no cover - defensive, boot must not die
+        logger.warning("Repairing self-parented nodes failed: %s", exc)
+
+
 async def get_db() -> AsyncGenerator[AsyncSession, None]:
     async with AsyncSessionLocal() as session:
         yield session

backend/app/schemas/canvas.py

@@ -1,6 +1,6 @@
 from typing import Any
 
-from pydantic import BaseModel, field_validator
+from pydantic import BaseModel, field_validator, model_validator
 
 from app.schemas.edges import EdgeResponse
 from app.schemas.nodes import NodeResponse
@@ -51,6 +51,22 @@ class NodeSave(BaseModel):
     pos_x: float = 0
     pos_y: float = 0
 
+    @model_validator(mode="after")
+    def drop_self_parent(self) -> "NodeSave":
+        """A node can never be its own parent.
+
+        Every parent walk on the canvas — waypoint propagation, ordering
+        parents before children, collapse — assumes an acyclic tree, and a
+        self-parent row makes the node undraggable once it is reloaded (#370).
+
+        Dropped rather than rejected: a canvas that already carries the bad row
+        must still be able to save, and a 422 here would cost the user the whole
+        save. The startup repair clears what is already persisted.
+        """
+        if self.parent_id is not None and self.parent_id == self.id:
+            self.parent_id = None
+        return self
+
 
 class EdgeSave(BaseModel):
     id: str

backend/app/services/node_dedupe.py

@@ -121,7 +121,11 @@ async def dedupe_nodes_by_device(db: AsyncSession) -> int:
             await db.execute(select(Node).where(Node.parent_id.in_(dup_ids)))
         ).scalars().all()
         for child in children:
-            child.parent_id = canonical.id
+            # The canonical node may itself have been nested under one of its
+            # own duplicates. Re-pointing it here would make it its own parent,
+            # which freezes it on the canvas (#370) — detach it instead. Mirrors
+            # the self-loop edge deletion below.
+            child.parent_id = None if child.id == canonical.id else canonical.id
 
         all_edges = (
             await db.execute(

backend/tests/test_canvas.py

@@ -706,3 +706,37 @@ async def test_save_canvas_custom_style_overwrite(client: AsyncClient, headers:
     canvas = (await client.get("/api/v1/canvas", headers=headers)).json()
     assert "proxmox" in canvas["custom_style"]["nodes"]
     assert "server" not in canvas["custom_style"]["nodes"]
+
+
+# ── self-parent guard (#370) ─────────────────────────────────────────────────
+
+async def test_save_canvas_drops_a_node_parented_to_itself(client: AsyncClient, headers: dict):
+    # A self-parent row freezes the node on the canvas and survives a reload, so
+    # the save normalizes it away. Dropped rather than rejected: a canvas that
+    # already carries the bad row must still be able to save.
+    n = node_payload(type="lxc", label="pihole")
+    n["parent_id"] = n["id"]
+
+    res = await client.post(
+        "/api/v1/canvas/save", json={"nodes": [n], "edges": [], "viewport": {}}, headers=headers
+    )
+    assert res.status_code == 200
+
+    saved = (await client.get("/api/v1/canvas", headers=headers)).json()["nodes"]
+    assert len(saved) == 1
+    assert saved[0]["parent_id"] is None
+
+
+async def test_save_canvas_keeps_a_real_parent(client: AsyncClient, headers: dict):
+    host = node_payload(type="proxmox", label="pve", container_mode=True)
+    child = node_payload(type="lxc", label="pihole", parent_id=host["id"])
+
+    res = await client.post(
+        "/api/v1/canvas/save",
+        json={"nodes": [host, child], "edges": [], "viewport": {}},
+        headers=headers,
+    )
+    assert res.status_code == 200
+
+    saved = {n["label"]: n for n in (await client.get("/api/v1/canvas", headers=headers)).json()["nodes"]}
+    assert saved["pihole"]["parent_id"] == host["id"]

backend/tests/test_node_dedupe.py

@@ -134,6 +134,30 @@ async def test_repoints_child_parent(db_session):
     assert child.parent_id == keep.id
 
 
+@pytest.mark.asyncio
+async def test_canonical_nested_under_its_own_duplicate_is_detached(db_session):
+    """#370 — re-pointing the canonical node made it its own parent.
+
+    The canonical node had been nested under one of its duplicates, so the
+    blanket re-point wrote `parent_id = id` and froze it on the canvas.
+    """
+    d = await _design(db_session)
+    device = await _device(db_session, ieee="0xFFF")
+    keep = Node(label="Host", type="proxmox", design_id=d.id, device_id=device.id)
+    db_session.add(keep)
+    await db_session.flush()
+    dup = Node(label="Host", type="proxmox", design_id=d.id, device_id=device.id)
+    db_session.add(dup)
+    await db_session.flush()
+    # The oldest node is the one that survives, and it points at the newer twin.
+    keep.parent_id = dup.id
+    await db_session.flush()
+
+    await dedupe_nodes_by_device(db_session)
+    await db_session.refresh(keep)
+    assert keep.parent_id is None
+
+
 @pytest.mark.asyncio
 async def test_idempotent_noop_when_unique(db_session):
     d = await _design(db_session)

backend/tests/test_nodes.py

@@ -403,3 +403,41 @@ async def test_list_nodes_without_label_param_returns_all(client: AsyncClient, h
     res = await client.get("/api/v1/nodes", headers=headers)
     assert res.status_code == 200
     assert len(res.json()) == 3
+
+
+# ── self-parent guard (#370) ─────────────────────────────────────────────────
+
+async def test_update_node_ignores_a_parent_id_pointing_at_itself(client: AsyncClient, headers: dict):
+    create = await client.post(
+        "/api/v1/nodes", json={"type": "lxc", "label": "pihole", "status": "unknown"}, headers=headers
+    )
+    node_id = create.json()["id"]
+
+    res = await client.patch(
+        f"/api/v1/nodes/{node_id}", json={"parent_id": node_id, "label": "renamed"}, headers=headers
+    )
+
+    assert res.status_code == 200
+    assert res.json()["parent_id"] is None
+    # The key is dropped, not the whole edit.
+    assert res.json()["label"] == "renamed"
+
+
+async def test_update_node_self_parent_leaves_a_real_parent_alone(client: AsyncClient, headers: dict):
+    host = await client.post(
+        "/api/v1/nodes",
+        json={"type": "proxmox", "label": "pve", "status": "unknown", "container_mode": True},
+        headers=headers,
+    )
+    host_id = host.json()["id"]
+    child = await client.post(
+        "/api/v1/nodes",
+        json={"type": "lxc", "label": "pihole", "status": "unknown", "parent_id": host_id},
+        headers=headers,
+    )
+    child_id = child.json()["id"]
+
+    res = await client.patch(f"/api/v1/nodes/{child_id}", json={"parent_id": child_id}, headers=headers)
+
+    assert res.status_code == 200
+    assert res.json()["parent_id"] == host_id

backend/tests/test_self_parent_repair.py

@@ -0,0 +1,92 @@
+"""Detaching nodes recorded as their own parent (#370).
+
+A ``parent_id = id`` row is fatal on the canvas: every parent walk assumes an
+acyclic tree, so dragging the node overflowed the stack inside the change
+reducer and the move was silently dropped — the node selected but would not
+move. It also renders unparented, sitting wherever its stored coordinates put
+it rather than inside the container it appears to belong to.
+
+The repair runs at startup because the bad rows are already in users' databases;
+the write-path guards shipped alongside it only stop new ones.
+"""
+import pytest
+from sqlalchemy.ext.asyncio import create_async_engine
+
+import app.db.database as database
+
+pytestmark = pytest.mark.asyncio
+
+_DDL = "CREATE TABLE nodes (id VARCHAR PRIMARY KEY, label VARCHAR, parent_id VARCHAR)"
+
+
+async def _engine(tmp_path, rows):
+    engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'selfparent.db'}")
+    async with engine.begin() as conn:
+        await conn.exec_driver_sql(_DDL)
+        for node_id, parent_id in rows:
+            await conn.exec_driver_sql(
+                "INSERT INTO nodes (id, label, parent_id) VALUES (?, ?, ?)",
+                (node_id, node_id, parent_id),
+            )
+    return engine
+
+
+async def _parents(engine) -> dict[str, str | None]:
+    async with engine.begin() as conn:
+        rows = (await conn.exec_driver_sql("SELECT id, parent_id FROM nodes")).fetchall()
+    return {r[0]: r[1] for r in rows}
+
+
+async def _run(engine, monkeypatch) -> None:
+    monkeypatch.setattr(database, "engine", engine)
+    await database._repair_self_parent_nodes()
+
+
+async def test_detaches_a_self_parented_node(tmp_path, monkeypatch):
+    engine = await _engine(tmp_path, [("pihole", "pihole")])
+
+    await _run(engine, monkeypatch)
+
+    assert (await _parents(engine))["pihole"] is None
+
+
+async def test_leaves_real_parents_alone(tmp_path, monkeypatch):
+    engine = await _engine(
+        tmp_path,
+        [("pve", None), ("vm", "pve"), ("broken", "broken")],
+    )
+
+    await _run(engine, monkeypatch)
+
+    parents = await _parents(engine)
+    assert parents == {"pve": None, "vm": "pve", "broken": None}
+
+
+async def test_is_idempotent(tmp_path, monkeypatch):
+    engine = await _engine(tmp_path, [("a", "a"), ("b", None)])
+
+    await _run(engine, monkeypatch)
+    await _run(engine, monkeypatch)
+
+    assert await _parents(engine) == {"a": None, "b": None}
+
+
+async def test_does_not_touch_a_two_node_cycle(tmp_path, monkeypatch):
+    """Only the exact self-parent case is repairable here.
+
+    A longer cycle has no single right answer for which link to cut, and the
+    runtime cycle guards keep the canvas usable, so the rows are left as they
+    are rather than guessed at.
+    """
+    engine = await _engine(tmp_path, [("a", "b"), ("b", "a")])
+
+    await _run(engine, monkeypatch)
+
+    assert await _parents(engine) == {"a": "b", "b": "a"}
+
+
+async def test_survives_a_missing_table(tmp_path, monkeypatch):
+    """Boot must not die on a repair; the canvas works either way."""
+    engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'empty.db'}")
+
+    await _run(engine, monkeypatch)

frontend/src/stores/__tests__/canvasStore/containers.test.ts

@@ -236,4 +236,47 @@ describe('canvasStore — containers & nesting', () => {
     const childIdx = nodes.findIndex((n) => n.id === 'c1')
     expect(parentIdx).toBeLessThan(childIdx)
   })
+
+  // #370 — nothing used to stop a node being written as its own parent, and
+  // the row is fatal: it freezes the node on the canvas and survives a save.
+  it('updateNode ignores a parent_id pointing at the node itself', () => {
+    const n = { ...makeNode('n1', { container_mode: true }), position: { x: 50, y: 50 } }
+    useCanvasStore.setState({ nodes: [n] })
+
+    useCanvasStore.getState().updateNode('n1', { parent_id: 'n1', label: 'renamed' })
+
+    const after = useCanvasStore.getState().nodes[0]
+    expect(after.data.parent_id).toBeUndefined()
+    expect(after.parentId).toBeUndefined()
+    // The rest of the edit still lands — the key is dropped, not the update.
+    expect(after.data.label).toBe('renamed')
+    expect(after.position).toEqual({ x: 50, y: 50 })
+  })
+
+  it('updateNode self-parent does not detach a node from its real parent', () => {
+    const parent = { ...makeNode('p1', { container_mode: true }), position: { x: 100, y: 100 } }
+    useCanvasStore.setState({ nodes: [parent] })
+    useCanvasStore.getState().addNode({
+      ...makeNode('c1', { parent_id: 'p1' }),
+      position: { x: 140, y: 160 },
+    } as Node<NodeData>)
+
+    useCanvasStore.getState().updateNode('c1', { parent_id: 'c1' })
+
+    const child = useCanvasStore.getState().nodes.find((x) => x.id === 'c1')!
+    expect(child.parentId).toBe('p1')
+    expect(child.data.parent_id).toBe('p1')
+  })
+
+  it('addNode strips a parent_id pointing at the node itself', () => {
+    useCanvasStore.getState().addNode({
+      ...makeNode('n1', { parent_id: 'n1', container_mode: true }),
+      position: { x: 30, y: 30 },
+    } as Node<NodeData>)
+
+    const added = useCanvasStore.getState().nodes[0]
+    expect(added.data.parent_id).toBeUndefined()
+    expect(added.parentId).toBeUndefined()
+    expect(added.position).toEqual({ x: 30, y: 30 })
+  })
 })

frontend/src/stores/__tests__/canvasStore/waypoints.test.ts

@@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach } from 'vitest'
 import { useCanvasStore } from '@/stores/canvasStore'
 import type { NodeChange, Node } from '@xyflow/react'
 import type { NodeData } from '@/types'
-import { makeNode, makeEdge } from '@/test/factories'
+import { makeNode, makeNodeData, makeEdge } from '@/test/factories'
 
 function resetStore() {
   useCanvasStore.setState({
@@ -102,4 +102,40 @@ describe('canvasStore — edge waypoints follow dragged nodes (#279)', () => {
     useCanvasStore.getState().onNodesChange([{ type: 'select', id: 'a', selected: true }])
     expect(useCanvasStore.getState().edges[0].data!.waypoints).toEqual([{ x: 100, y: 100 }])
   })
+
+  // #370 — a corrupt row pointing a node at itself made the child walk recurse
+  // forever. The overflow was thrown inside onNodesChange's reducer, so the
+  // whole state update was discarded: the node selected but never moved.
+  it('moves a node that is recorded as its own parent', () => {
+    useCanvasStore.setState({
+      nodes: [makeNode({ id: 'a', position: { x: 0, y: 0 }, data: makeNodeData({ parent_id: 'a' }) })],
+      edges: [],
+    })
+    useCanvasStore.getState().onNodesChange([posChange('a', 40, 40)])
+    expect(useCanvasStore.getState().nodes[0].position).toEqual({ x: 40, y: 40 })
+  })
+
+  it('translates waypoints once for a self-parented node instead of hanging', () => {
+    useCanvasStore.setState({
+      nodes: [
+        makeNode({ id: 'a', position: { x: 0, y: 0 }, data: makeNodeData({ parent_id: 'a' }) }),
+        makeNode('b', {}),
+      ],
+      edges: [makeEdge('e1', 'a', 'b', { data: { type: 'ethernet', waypoints: [{ x: 100, y: 100 }] } })],
+    })
+    useCanvasStore.getState().onNodesChange([posChange('a', 10, 10)])
+    expect(useCanvasStore.getState().edges[0].data!.waypoints).toEqual([{ x: 110, y: 110 }])
+  })
+
+  it('survives a longer parent cycle (a -> b -> a)', () => {
+    useCanvasStore.setState({
+      nodes: [
+        makeNode({ id: 'a', position: { x: 0, y: 0 }, data: makeNodeData({ parent_id: 'b' }) }),
+        makeNode({ id: 'b', position: { x: 0, y: 0 }, data: makeNodeData({ parent_id: 'a' }) }),
+      ],
+      edges: [],
+    })
+    useCanvasStore.getState().onNodesChange([posChange('a', 25, 25)])
+    expect(useCanvasStore.getState().nodes[0].position).toEqual({ x: 25, y: 25 })
+  })
 })

frontend/src/stores/canvasStore.ts

@@ -125,7 +125,15 @@ function translateWaypointsForMovedNodes(
     arr.push(n.id)
     childrenByParent.set(pid, arr)
   }
+  // A corrupt row can make a node its own parent (or close a longer cycle),
+  // and the walk below would then never terminate — the resulting stack
+  // overflow is thrown inside onNodesChange's reducer, so the whole state
+  // update is dropped and the node stops moving while still selecting (#370).
+  // Mirrors the cycle guard orderParentsFirst already carries.
+  const walked = new Set<string>()
   const propagate = (id: string, d: { dx: number; dy: number }) => {
+    if (walked.has(id)) return
+    walked.add(id)
     for (const childId of childrenByParent.get(id) ?? []) {
       // A directly-dragged child keeps its own delta; don't overwrite it.
       if (!deltaById.has(childId)) deltaById.set(childId, d)
@@ -496,6 +504,10 @@ export const useCanvasStore = create<CanvasState>((rawSet, get) => {
 
   addNode: (node) =>
     set((state) => {
+      // Same rule as updateNode: never let a node parent itself (#370).
+      if (node.data.parent_id === node.id) {
+        node = { ...node, data: { ...node.data, parent_id: undefined }, parentId: undefined, extent: undefined }
+      }
       const parent = node.data.parent_id ? state.nodes.find((n) => n.id === node.data.parent_id) : null
       // A visual group — and a groupRect zone — nests its children just like a
       // container-mode host.
@@ -526,8 +538,17 @@ export const useCanvasStore = create<CanvasState>((rawSet, get) => {
       return { nodes: [...withoutNew, enriched], hasUnsavedChanges: true }
     }),
 
-  updateNode: (id, data) =>
+  updateNode: (id, incoming) =>
     set((state) => {
+      // A node can never be its own parent. Every parent walk (waypoint
+      // propagation, parents-before-children ordering, collapse) assumes an
+      // acyclic tree, and the row survives a save, so the canvas comes back
+      // broken on the next load (#370). Drop the key and apply the rest.
+      let data = incoming
+      if (incoming.parent_id === id) {
+        data = { ...incoming }
+        delete data.parent_id
+      }
       let nodes = state.nodes.map((n) => {
         if (n.id !== id) return n
         const updated: Node<NodeData> = { ...n, data: { ...n.data, ...data } }

frontend/src/utils/__tests__/importYaml.test.ts

@@ -339,6 +339,26 @@ describe('parseYamlToCanvas', () => {
     warnSpy.mockRestore()
   })
 
+  // #370 — parents resolve by label, so a node naming its own label mapped
+  // back to itself and imported as its own parent, which freezes it on canvas.
+  it('warns and skips a parent label that resolves to the node itself', () => {
+    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    const yaml = `
+- nodeType: lxc
+  label: "pihole"
+  parent:
+    label: "pihole"
+    linkType: virtual
+`
+    const { nodes, edges } = parseYamlToCanvas(yaml, empty, emptyEdges)
+    expect(nodes).toHaveLength(1)
+    expect(nodes[0].parentId).toBeUndefined()
+    expect(nodes[0].data.parent_id).toBeUndefined()
+    expect(edges).toHaveLength(0)
+    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('its own parent'))
+    warnSpy.mockRestore()
+  })
+
   // Regression for issue #208.
   it('restores edge connection points from the YAML link', () => {
     const yaml = `

frontend/src/utils/importYaml.ts

@@ -158,6 +158,11 @@ export function parseYamlToCanvas(
       const parentId = labelToId.get(yn.parent.label)
       if (!parentId) {
         console.warn(`[importYaml] parent label not found: "${yn.parent.label}" — skipping relationship`)
+      } else if (parentId === node.id) {
+        // Parents resolve by label, so a node naming itself — or naming a
+        // duplicate label that maps back to it — would nest it inside itself
+        // and freeze it on the canvas (#370).
+        console.warn(`[importYaml] node "${yn.parent.label}" is its own parent — skipping relationship`)
       } else {
         // Set React Flow parentId for nesting
         node.data = { ...node.data, parent_id: parentId }