diff --git a/canvas/src/components/Canvas.tsx b/canvas/src/components/Canvas.tsx index f099e22e..b55d6808 100644 --- a/canvas/src/components/Canvas.tsx +++ b/canvas/src/components/Canvas.tsx @@ -8,11 +8,13 @@ import { Controls, MiniMap, type Edge, + type Node, BackgroundVariant, } from "@xyflow/react"; import "@xyflow/react/dist/style.css"; import { useCanvasStore } from "@/store/canvas"; +import type { WorkspaceNodeData } from "@/store/canvas"; import { WORKSPACE_KIND } from "@/lib/workspace-kind"; import { stripPlatformRootForMap } from "@/store/canvas-topology"; import { useTheme } from "@/lib/theme-provider"; @@ -86,6 +88,8 @@ function CanvasInner() { const a2aEdges = useCanvasStore((s) => s.a2aEdges); const showA2AEdges = useCanvasStore((s) => s.showA2AEdges); const deletingIds = useCanvasStore((s) => s.deletingIds); + const topologyError = useCanvasStore((s) => s.topologyError); + const setTopologyError = useCanvasStore((s) => s.setTopologyError); // Hide the org-level platform agent (the concierge) from the map graph: it is // the undeletable org ROOT surfaced in the shell (topbar + Home tree), not a // draggable/deletable map node. Its direct children are reparented to @@ -304,7 +308,14 @@ function CanvasInner() { Skip to canvas
- setTopologyError(null)} + workspaces={storeNodes} + /> + ) : ( + + )} {/* Screen-reader live region — announces workspace count on initial load and live status updates from WebSocket events (online, offline, provisioning, etc.). @@ -441,3 +453,52 @@ function CanvasInner() { ); } + +interface MapRenderFallbackProps { + error: string; + onDismiss: () => void; + workspaces: Node[]; +} + +function MapRenderFallback({ error, onDismiss, workspaces }: MapRenderFallbackProps) { + return ( +
+
+

Couldn't render workspace map

+

+ The org-map layout hit an unexpected tree shape and stopped to avoid a silent hang. +

+ {error && ( + + {error} + + )} + +
+ {workspaces.length > 0 && ( +
+

Workspaces in this org

+
    + {workspaces.slice(0, 50).map((n) => ( +
  • + {n.data.name || n.id} +
  • + ))} +
+ {workspaces.length > 50 && ( +

...and {workspaces.length - 50} more

+ )} +
+ )} +
+ ); +} diff --git a/canvas/src/store/__tests__/canvas-topology.test.ts b/canvas/src/store/__tests__/canvas-topology.test.ts index dcf853aa..25925159 100644 --- a/canvas/src/store/__tests__/canvas-topology.test.ts +++ b/canvas/src/store/__tests__/canvas-topology.test.ts @@ -155,6 +155,21 @@ describe("buildNodesAndEdges – parent + child workspaces", () => { }); }); +describe("buildNodesAndEdges – cycle detection", () => { + it("throws a clear error when parent chain contains a cycle", () => { + const workspaces = [ + makeWS({ id: "a", parent_id: "b" }), + makeWS({ id: "b", parent_id: "a" }), + ]; + expect(() => buildNodesAndEdges(workspaces)).toThrow(/cyclic parent chain detected/); + }); + + it("throws a clear error on a self-referencing parent", () => { + const workspaces = [makeWS({ id: "a", parent_id: "a" })]; + expect(() => buildNodesAndEdges(workspaces)).toThrow(/cyclic parent chain detected/); + }); +}); + describe("buildNodesAndEdges – auto-rescue respects live grown parent size", () => { // Regression: child the user dragged into a user-grown area was // false-rescued by every periodic rehydrate (socket health check diff --git a/canvas/src/store/__tests__/canvas.test.ts b/canvas/src/store/__tests__/canvas.test.ts index 941d979e..d5a366bf 100644 --- a/canvas/src/store/__tests__/canvas.test.ts +++ b/canvas/src/store/__tests__/canvas.test.ts @@ -215,6 +215,33 @@ describe("hydrate", () => { // gates on currentTask (e.g. ChatTab thinking indicator). expect(!!node.data.currentTask).toBe(true); }); + + it("catches cyclic parent chains and sets topologyError instead of hanging (issue #2601)", () => { + const cyclic = [ + makeWS({ id: "a", name: "A", parent_id: "b" }), + makeWS({ id: "b", name: "B", parent_id: "a" }), + ]; + + useCanvasStore.getState().hydrate(cyclic); + const { topologyError, nodes, edges } = useCanvasStore.getState(); + + expect(topologyError).toMatch(/cyclic parent chain/i); + // Should fall back to empty/unchanged canvas, not wedge or partial bad nodes. + expect(nodes).toHaveLength(0); + expect(edges).toHaveLength(0); + }); + + it("clears topologyError on the next successful hydrate", () => { + useCanvasStore.getState().hydrate([ + makeWS({ id: "a", parent_id: "b" }), + makeWS({ id: "b", parent_id: "a" }), + ]); + expect(useCanvasStore.getState().topologyError).toMatch(/cyclic parent chain/i); + + useCanvasStore.getState().hydrate([makeWS({ id: "solo", name: "Solo" })]); + expect(useCanvasStore.getState().topologyError).toBeNull(); + expect(useCanvasStore.getState().nodes).toHaveLength(1); + }); }); describe("summarizeWorkspaceCapabilities", () => { @@ -1293,7 +1320,7 @@ describe("hydrate – cyclic parent chain", () => { makeWS({ id: "b", parent_id: "a" }), ]); - expect(useCanvasStore.getState().hydrationError).toContain("cyclic parent chain"); + expect(useCanvasStore.getState().topologyError).toContain("cyclic parent chain"); expect(useCanvasStore.getState().nodes).toEqual(before); }); }); diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index f3c5ee6d..60c00176 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -284,6 +284,10 @@ interface CanvasState { /** Hydration error message — set when initial canvas load fails. Null when no error. */ hydrationError: string | null; setHydrationError: (error: string | null) => void; + /** Topology/layout error message — set when computeAutoLayout/buildNodesAndEdges + * cannot safely render the workspace tree (e.g. cyclic parent chain). Null when no error. */ + topologyError: string | null; + setTopologyError: (error: string | null) => void; // ── A2A topology overlay (issue #744) ───────────────────────────────────── /** Directed delegation edges shown as an overlay on the canvas (separate from topology edges). */ a2aEdges: Edge[]; @@ -383,6 +387,8 @@ export const useCanvasStore = create((set, get) => ({ setWsStatus: (status) => set({ wsStatus: status }), hydrationError: null, setHydrationError: (error) => set({ hydrationError: error }), + topologyError: null, + setTopologyError: (error) => set({ topologyError: error }), // A2A overlay — default on, persisted to localStorage a2aEdges: [], setA2AEdges: (edges) => set({ a2aEdges: edges }), @@ -941,49 +947,50 @@ export const useCanvasStore = create((set, get) => ({ }, hydrate: (workspaces: WorkspaceData[]) => { - // Drop ids tombstoned by a recent removeSubtree (#2069 — stale - // in-flight GET /workspaces). - const live = workspaces.filter((w) => !wasRecentlyDeleted(w.id)); - const layoutOverrides = computeAutoLayout(live); - // Carry the live measured/grown parent sizes from the existing - // store into the rebuild. buildNodesAndEdges runs an auto-rescue - // pass on each child to detach orphans whose stored relative - // position falls outside the parent bbox — without the live - // size, the bbox is the initial grid-derived minimum, which - // false-flags any child the user has dragged into the - // user-grown area. Periodic rehydrate (socket.ts health check, - // 30s) was reasserting the rescue against legitimate user - // placements, causing the "child jumps to weird location, then - // settles" symptom. - const current = get().nodes; - const currentParentSizes = new Map(); - for (const n of current) { - const w = (n.measured?.width ?? n.width) as number | undefined; - const h = (n.measured?.height ?? n.height) as number | undefined; - if (typeof w === "number" && typeof h === "number") { - currentParentSizes.set(n.id, { width: w, height: h }); - } - } try { + // Drop ids tombstoned by a recent removeSubtree (#2069 — stale + // in-flight GET /workspaces). + const live = workspaces.filter((w) => !wasRecentlyDeleted(w.id)); + const layoutOverrides = computeAutoLayout(live); + // Carry the live measured/grown parent sizes from the existing + // store into the rebuild. buildNodesAndEdges runs an auto-rescue + // pass on each child to detach orphans whose stored relative + // position falls outside the parent bbox — without the live + // size, the bbox is the initial grid-derived minimum, which + // false-flags any child the user has dragged into the + // user-grown area. Periodic rehydrate (socket.ts health check, + // 30s) was reasserting the rescue against legitimate user + // placements, causing the "child jumps to weird location, then + // settles" symptom. + const current = get().nodes; + const currentParentSizes = new Map(); + for (const n of current) { + const w = (n.measured?.width ?? n.width) as number | undefined; + const h = (n.measured?.height ?? n.height) as number | undefined; + if (typeof w === "number" && typeof h === "number") { + currentParentSizes.set(n.id, { width: w, height: h }); + } + } const { nodes, edges } = buildNodesAndEdges( live, layoutOverrides, currentParentSizes, ); - set({ nodes, edges }); + set({ nodes, edges, topologyError: null }); + for (const [nodeId, { x, y }] of layoutOverrides) { + api.patch(`/workspaces/${nodeId}`, { x, y }).catch(() => {}); + } } catch (err) { - // Fail closed: cyclic/corrupt topology must not hang or blank the app. - // Surface a retryable error state and keep the previous nodes so the - // user isn't left with an empty canvas. - const message = - err instanceof TopologyCycleError - ? `Workspace map has a cyclic parent chain: ${err.message}. Please reload or contact support.` - : "Failed to render workspace map: corrupt topology. Please reload or contact support."; - set({ hydrationError: message }); - return; - } - for (const [nodeId, { x, y }] of layoutOverrides) { - api.patch(`/workspaces/${nodeId}`, { x, y }).catch(() => {}); + const message = err instanceof Error ? err.message : String(err); + console.error("[canvas] topology render failed:", err); + set({ topologyError: message }); + // Preserve existing nodes if we have them so the user isn't left + // with a blank canvas on a transient layout bug. Empty nodes fall + // back to the EmptyState / Onboarding wizard path. + const currentNodes = get().nodes; + if (currentNodes.length === 0) { + set({ nodes: [], edges: [] }); + } } },