diff --git a/canvas/src/components/Canvas.tsx b/canvas/src/components/Canvas.tsx index 19c6dc41..0d793df5 100644 --- a/canvas/src/components/Canvas.tsx +++ b/canvas/src/components/Canvas.tsx @@ -183,7 +183,7 @@ function CanvasInner() { // outside-click handler. const pendingDelete = useCanvasStore((s) => s.pendingDelete); const setPendingDelete = useCanvasStore((s) => s.setPendingDelete); - const removeNode = useCanvasStore((s) => s.removeNode); + const removeSubtree = useCanvasStore((s) => s.removeSubtree); const confirmDelete = useCallback(async () => { if (!pendingDelete) return; const { id } = pendingDelete; @@ -207,12 +207,13 @@ function CanvasInner() { state.beginDelete(subtree); try { await api.del(`/workspaces/${id}?confirm=true`); - removeNode(id); - // Server-side cascade will emit WORKSPACE_REMOVED per node; - // handleCanvasEvent drops each from the store. Clear the - // deleting set in one shot once the DELETE resolves so any - // node that lags the WS (or is preserved locally, e.g. an - // external workspace) doesn't stay dimmed forever. + // Mirror the server-side cascade locally — drop the parent AND + // every descendant in one atomic update. The per-descendant + // WORKSPACE_REMOVED WS events still arrive (and are no-ops + // because the nodes are already gone), but we no longer depend + // on them: a wedged WS used to leave orphan child cards on the + // canvas until the user refreshed the page. + removeSubtree(id); state.endDelete(subtree); } catch (e) { // Network or server error — restore the subtree to normal @@ -220,7 +221,7 @@ function CanvasInner() { state.endDelete(subtree); showToast(e instanceof Error ? e.message : "Delete failed", "error"); } - }, [pendingDelete, setPendingDelete, removeNode]); + }, [pendingDelete, setPendingDelete, removeSubtree]); const onPaneClick = useCallback(() => { selectNode(null); diff --git a/canvas/src/components/tabs/DetailsTab.tsx b/canvas/src/components/tabs/DetailsTab.tsx index 8bd0d69a..211b1637 100644 --- a/canvas/src/components/tabs/DetailsTab.tsx +++ b/canvas/src/components/tabs/DetailsTab.tsx @@ -36,7 +36,7 @@ export function DetailsTab({ workspaceId, data }: Props) { const [restartError, setRestartError] = useState(null); const [consoleOpen, setConsoleOpen] = useState(false); const updateNodeData = useCanvasStore((s) => s.updateNodeData); - const removeNode = useCanvasStore((s) => s.removeNode); + const removeSubtree = useCanvasStore((s) => s.removeSubtree); const selectNode = useCanvasStore((s) => s.selectNode); // Ref for the "Delete Workspace" trigger — Cancel returns focus here const deleteButtonRef = useRef(null); @@ -94,7 +94,11 @@ export function DetailsTab({ workspaceId, data }: Props) { setDeleteError(null); try { await api.del(`/workspaces/${workspaceId}?confirm=true`); - removeNode(workspaceId); + // Mirror the server-side cascade — drop the row + every + // descendant locally so the canvas reflects the deletion + // immediately, even when the WS is dead and the per-descendant + // WORKSPACE_REMOVED events never arrive. + removeSubtree(workspaceId); selectNode(null); } catch (e) { setDeleteError(e instanceof Error ? e.message : "Failed to delete"); diff --git a/canvas/src/store/__tests__/canvas.test.ts b/canvas/src/store/__tests__/canvas.test.ts index df0460f6..790833f0 100644 --- a/canvas/src/store/__tests__/canvas.test.ts +++ b/canvas/src/store/__tests__/canvas.test.ts @@ -484,6 +484,70 @@ describe("removeNode", () => { }); }); +// ---------- removeSubtree ---------- + +describe("removeSubtree", () => { + beforeEach(() => { + useCanvasStore.getState().hydrate([ + makeWS({ id: "root" }), + makeWS({ id: "mid", parent_id: "root" }), + makeWS({ id: "leaf", parent_id: "mid" }), + makeWS({ id: "sibling", parent_id: "root" }), + makeWS({ id: "unrelated" }), // separate root + ]); + }); + + it("removes the root and every descendant in one shot", () => { + useCanvasStore.getState().removeSubtree("root"); + const ids = useCanvasStore + .getState() + .nodes.map((n) => n.id) + .sort(); + expect(ids).toEqual(["unrelated"]); + }); + + it("removes a mid-level node and its descendants but leaves siblings + ancestors", () => { + useCanvasStore.getState().removeSubtree("mid"); + const ids = useCanvasStore + .getState() + .nodes.map((n) => n.id) + .sort(); + expect(ids).toEqual(["root", "sibling", "unrelated"]); + }); + + it("removing a leaf is a no-op cascade (just drops the leaf)", () => { + useCanvasStore.getState().removeSubtree("leaf"); + const ids = useCanvasStore + .getState() + .nodes.map((n) => n.id) + .sort(); + expect(ids).toEqual(["mid", "root", "sibling", "unrelated"]); + }); + + it("clears selection when the selected node is anywhere in the removed subtree", () => { + useCanvasStore.getState().selectNode("leaf"); + useCanvasStore.getState().removeSubtree("root"); + expect(useCanvasStore.getState().selectedNodeId).toBeNull(); + }); + + it("preserves selection when the selected node is outside the removed subtree", () => { + useCanvasStore.getState().selectNode("unrelated"); + useCanvasStore.getState().removeSubtree("root"); + expect(useCanvasStore.getState().selectedNodeId).toBe("unrelated"); + }); + + it("drops edges incident to any removed node", () => { + // The hydrate-built edges connect parent → child. After removing + // `root`, no edge involving root/mid/leaf/sibling should remain. + useCanvasStore.getState().removeSubtree("root"); + const remaining = useCanvasStore.getState().edges; + for (const e of remaining) { + expect(["root", "mid", "leaf", "sibling"]).not.toContain(e.source); + expect(["root", "mid", "leaf", "sibling"]).not.toContain(e.target); + } + }); +}); + // ---------- isDescendant ---------- describe("isDescendant", () => { diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index 2971de39..f9565e47 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -132,6 +132,16 @@ interface CanvasState { updateNodeData: (id: string, data: Partial) => void; restartWorkspace: (id: string) => Promise; removeNode: (id: string) => void; + /** Remove a node AND every descendant in one atomic update. Mirrors + * the server-side cascade — `DELETE /workspaces/:id?confirm=true` + * drops the row plus every descendant in one transaction. The + * caller (Canvas / DetailsTab delete handlers) used to call + * `removeNode(rootId)` and rely on per-descendant WORKSPACE_REMOVED + * WS events to clear the rest. When the WS is unhealthy those + * events never arrive and the children orphan to the root until a + * manual page refresh — `removeSubtree` makes the cascade + * WS-independent. */ + removeSubtree: (rootId: string) => void; setDragOverNode: (id: string | null) => void; nestNode: (draggedId: string, targetId: string | null) => Promise; isDescendant: (ancestorId: string, nodeId: string) => boolean; @@ -789,6 +799,43 @@ export const useCanvasStore = create((set, get) => ({ }); }, + removeSubtree: (rootId) => { + const { nodes, edges, selectedNodeId } = get(); + // Build a parentId → childIds index once so the descent is O(N), + // not O(N · depth). The store typically holds <500 nodes; even + // doing a linear scan per parent would be fine, but the index + // keeps the cost predictable as orgs grow. + const childrenByParent = new Map(); + for (const n of nodes) { + const p = n.data.parentId ?? null; + if (p === null) continue; + const arr = childrenByParent.get(p); + if (arr) arr.push(n.id); + else childrenByParent.set(p, [n.id]); + } + const removed = new Set([rootId]); + const stack = [rootId]; + while (stack.length) { + const cur = stack.pop()!; + const kids = childrenByParent.get(cur); + if (!kids) continue; + for (const k of kids) { + if (!removed.has(k)) { + removed.add(k); + stack.push(k); + } + } + } + set({ + nodes: nodes.filter((n) => !removed.has(n.id)), + edges: edges.filter((e) => !removed.has(e.source) && !removed.has(e.target)), + selectedNodeId: + selectedNodeId !== null && removed.has(selectedNodeId) + ? null + : selectedNodeId, + }); + }, + hydrate: (workspaces: WorkspaceData[]) => { const layoutOverrides = computeAutoLayout(workspaces); // Carry the live measured/grown parent sizes from the existing