fix(canvas): cascade delete locally so children disappear without WS

Deleting a parent on a wedged WS used to leave the child cards on
the canvas as orphaned roots until the user manually refreshed.

Why: Canvas.tsx and DetailsTab.tsx both called `removeNode(parentId)`
after `DELETE /workspaces/:id?confirm=true` returned 200. `removeNode`
deliberately re-parents children rather than cascading — it relies on
the per-descendant WORKSPACE_REMOVED WS events the platform emits as
part of the cascade to drop each child individually. When the WS is
unhealthy those events never arrive, so the local store keeps the
children alive (now re-parented to root since their actual parent is
gone).

Fix: new `removeSubtree(rootId)` action on the canvas store mirrors
the server-side cascade — drops the root + every descendant + every
incident edge in one atomic set(). Both delete call sites now use it.
The WS events still arrive when WS is healthy and become idempotent
no-ops because the nodes are already gone.

Why a new action instead of changing removeNode: removeNode's
re-parenting behavior is correct for non-cascading flows (drag-out,
manual node detach in the future). Adding a sibling action keeps
both call shapes available rather than forcing every caller to opt
out of cascade.

6 new unit tests cover root cascade, mid-level cascade, leaf
no-op-cascade, selection clearing across the subtree, selection
preservation outside the subtree, and edge cleanup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang 2026-04-24 20:51:09 -07:00
parent f8c900909e
commit 21db85d691
4 changed files with 126 additions and 10 deletions

View File

@ -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);

View File

@ -36,7 +36,7 @@ export function DetailsTab({ workspaceId, data }: Props) {
const [restartError, setRestartError] = useState<string | null>(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<HTMLButtonElement>(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");

View File

@ -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", () => {

View File

@ -132,6 +132,16 @@ interface CanvasState {
updateNodeData: (id: string, data: Partial<WorkspaceNodeData>) => void;
restartWorkspace: (id: string) => Promise<void>;
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<void>;
isDescendant: (ancestorId: string, nodeId: string) => boolean;
@ -789,6 +799,43 @@ export const useCanvasStore = create<CanvasState>((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<string, string[]>();
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<string>([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