2026-06-15 06:00:57 +00:00
6 changed files with 256 additions and 16 deletions
@@ -23,6 +23,7 @@ const monoFont = JetBrains_Mono({
 import { AuthGate } from "@/components/AuthGate";
 import { CookieConsent } from "@/components/CookieConsent";
 import { PurchaseSuccessModal } from "@/components/PurchaseSuccessModal";
+import { ErrorBoundary } from "@/components/ErrorBoundary";
 import { ThemeProvider } from "@/lib/theme-provider";
 import {
  THEME_COOKIE,
@@ -245,11 +246,12 @@ export default async function RootLayout({
      </head>
      <body className={`bg-surface text-ink ${interFont.variable} ${monoFont.variable}`}>
        <ThemeProvider initialTheme={theme}>
-          {/* AuthGate is a client component; it checks the session on mount
-              and bounces anonymous users to the control plane's login page
-              when running on a tenant subdomain. Non-SaaS hosts (localhost,
-              vercel preview URL, apex) pass through unchanged. */}
-          <AuthGate>{children}</AuthGate>
+          {/* ErrorBoundary is a client component; it catches render crashes
+              anywhere inside AuthGate / children so a single failing view
+              degrades to a reloadable fallback instead of a blank white screen. */}
+          <ErrorBoundary>
+            <AuthGate>{children}</AuthGate>
+          </ErrorBoundary>
          <CookieConsent />
          {/* Demo Mock #1: post-purchase success toast. Mounted at the
              layout level so it persists across page state transitions
@@ -1,9 +1,9 @@
+// @vitest-environment jsdom
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";

 // ---------------------------------------------------------------------------
-// We test ErrorBoundary in a pure-unit style by instantiating the class
-// directly, avoiding the need for a full React DOM renderer (which the
-// project's vitest environment = "node" does not provide).
+// We test ErrorBoundary both in pure-unit style (class instantiation) and
+// with React DOM rendering to verify it catches child render throws.
 // ---------------------------------------------------------------------------

 // Mock fetch globally so transitive imports of api.ts don't blow up
@@ -12,6 +12,7 @@ globalThis.fetch = vi.fn(() =>
 );

 import React from "react";
+import { render, screen, cleanup, fireEvent } from "@testing-library/react";
 import { ErrorBoundary } from "../ErrorBoundary";

 // ---------------------------------------------------------------------------
@@ -23,6 +24,13 @@ function createInstance(props: { children: React.ReactNode } = { children: null
  return instance;
 }

+function Thrower({ shouldThrow }: { shouldThrow: boolean }) {
+  if (shouldThrow) {
+    throw new Error("boom");
+  }
+  return <div data-testid="child">healthy child</div>;
+}
+
 // ---------------------------------------------------------------------------
 // Tests
 // ---------------------------------------------------------------------------
@@ -36,6 +44,7 @@ describe("ErrorBoundary", () => {

  afterEach(() => {
    consoleErrorSpy.mockRestore();
+    cleanup();
  });

  // ---- static getDerivedStateFromError -----------------------------------
@@ -124,4 +133,34 @@ describe("ErrorBoundary", () => {
    expect(json).toContain("Unknown error");
    expect(json).toContain("Something went wrong");
  });
+
+  // ---- DOM integration: catches child render throws ---------------------
+
+  it("renders a fallback when a child throws during render", () => {
+    render(
+      <ErrorBoundary>
+        <Thrower shouldThrow={true} />
+      </ErrorBoundary>,
+    );
+    expect(screen.getByRole("alert")).toBeDefined();
+    expect(screen.queryByText("Something went wrong")).not.toBeNull();
+    expect(screen.queryByText("boom")).not.toBeNull();
+  });
+
+  it("reload button calls window.location.reload", () => {
+    const reload = vi.fn();
+    Object.defineProperty(window, "location", {
+      value: { reload },
+      writable: true,
+    });
+
+    render(
+      <ErrorBoundary>
+        <Thrower shouldThrow={true} />
+      </ErrorBoundary>,
+    );
+
+    fireEvent.click(screen.getByRole("button", { name: /reload/i }));
+    expect(reload).toHaveBeenCalled();
+  });
 });
@@ -1,5 +1,11 @@
 import { describe, it, expect } from "vitest";
-import { buildNodesAndEdges, extractSkillNames, computeAutoLayout } from "../canvas-topology";
+import {
+  buildNodesAndEdges,
+  extractSkillNames,
+  computeAutoLayout,
+  sortParentsBeforeChildren,
+  TopologyCycleError,
+} from "../canvas-topology";
 import type { WorkspaceData } from "../socket";

 // ---------------------------------------------------------------------------
@@ -487,3 +493,38 @@ describe("buildNodesAndEdges – child rescue heuristic", () => {
    expect(child.position).toEqual({ x: 100, y: 50 });
  });
 });
+
+// ---------------------------------------------------------------------------
+// Cycle guards
+// ---------------------------------------------------------------------------
+
+describe("sortParentsBeforeChildren – cyclic parent chain", () => {
+  it("bails and returns the original array instead of hanging", () => {
+    const nodes = [
+      { id: "a", parentId: "b" },
+      { id: "b", parentId: "a" },
+    ];
+    const result = sortParentsBeforeChildren(nodes);
+    expect(result).toBe(nodes);
+    expect(result).toHaveLength(2);
+  });
+
+  it("still returns a topologically sorted array for acyclic input", () => {
+    const nodes = [
+      { id: "child", parentId: "parent" },
+      { id: "parent" },
+    ];
+    const result = sortParentsBeforeChildren(nodes);
+    expect(result.map((n) => n.id)).toEqual(["parent", "child"]);
+  });
+});
+
+describe("buildNodesAndEdges – cyclic parent chain", () => {
+  it("throws TopologyCycleError instead of hanging", () => {
+    const workspaces = [
+      makeWS({ id: "a", parent_id: "b" }),
+      makeWS({ id: "b", parent_id: "a" }),
+    ];
+    expect(() => buildNodesAndEdges(workspaces)).toThrow(TopologyCycleError);
+  });
+});
@@ -6,8 +6,10 @@ global.fetch = vi.fn(() =>
 );

 import { useCanvasStore, summarizeWorkspaceCapabilities } from "../canvas";
+import type { WorkspaceNodeData } from "../canvas";
 import { __resetTombstonesForTest } from "../deleteTombstones";
 import type { WorkspaceData, WSMessage } from "../socket";
+import type { Node } from "@xyflow/react";

 // Helper to build a WorkspaceData object with sensible defaults
 function makeWS(overrides: Partial<WorkspaceData> & { id: string }): WorkspaceData {
@@ -33,6 +35,36 @@ function makeWS(overrides: Partial<WorkspaceData> & { id: string }): WorkspaceDa
  };
 }

+function makeNode(
+  overrides: Partial<WorkspaceNodeData> & { id: string; parentId?: string | null },
+): Node<WorkspaceNodeData> {
+  const { id, parentId, ...dataOverrides } = overrides;
+  const ws = makeWS({ id });
+  return {
+    id,
+    type: "workspaceNode",
+    position: { x: 0, y: 0 },
+    data: {
+      name: ws.name,
+      status: ws.status,
+      tier: ws.tier,
+      agentCard: ws.agent_card,
+      activeTasks: ws.active_tasks,
+      collapsed: ws.collapsed,
+      role: ws.role,
+      lastErrorRate: ws.last_error_rate,
+      lastSampleError: ws.last_sample_error,
+      url: ws.url,
+      parentId: parentId ?? ws.parent_id,
+      currentTask: ws.current_task,
+      runtime: ws.runtime,
+      needsRestart: false,
+      budgetLimit: ws.budget_limit,
+      ...dataOverrides,
+    },
+  } as Node<WorkspaceNodeData>;
+}
+
 function makeMsg(overrides: Partial<WSMessage> & { event: string; workspace_id: string }): WSMessage {
  return {
    timestamp: new Date().toISOString(),
@@ -1245,3 +1277,60 @@ describe("moveNode", () => {
    });
  });
 });
+
+// ---------- cycle guards (#2601 follow-up) ----------
+
+describe("hydrate – cyclic parent chain", () => {
+  it("sets hydrationError and preserves existing nodes instead of hanging", () => {
+    useCanvasStore.getState().hydrate([
+      makeWS({ id: "existing", name: "Existing" }),
+    ]);
+    const before = useCanvasStore.getState().nodes;
+    expect(before).toHaveLength(1);
+
+    useCanvasStore.getState().hydrate([
+      makeWS({ id: "a", parent_id: "b" }),
+      makeWS({ id: "b", parent_id: "a" }),
+    ]);
+
+    expect(useCanvasStore.getState().hydrationError).toContain("cyclic parent chain");
+    expect(useCanvasStore.getState().nodes).toEqual(before);
+  });
+});
+
+describe("isDescendant – cyclic parent chain", () => {
+  it("returns false instead of hanging when the ancestor is not reachable", () => {
+    useCanvasStore.setState({
+      nodes: [
+        makeNode({ id: "a", parentId: null }),
+        makeNode({ id: "b", parentId: "c" }),
+        makeNode({ id: "c", parentId: "b" }),
+      ],
+    });
+    expect(useCanvasStore.getState().isDescendant("a", "b")).toBe(false);
+  });
+});
+
+describe("arrangeChildren – cyclic parent chain", () => {
+  it("does not hang on a cycle", () => {
+    useCanvasStore.setState({
+      nodes: [
+        makeNode({ id: "a", parentId: "b" }),
+        makeNode({ id: "b", parentId: "a" }),
+      ],
+    });
+    expect(() => useCanvasStore.getState().arrangeChildren("a")).not.toThrow();
+  });
+});
+
+describe("nestNode – cyclic parent chain", () => {
+  it("does not hang when current data contains a cycle", async () => {
+    useCanvasStore.setState({
+      nodes: [
+        makeNode({ id: "a", parentId: "b" }),
+        makeNode({ id: "b", parentId: "a" }),
+      ],
+    });
+    await expect(useCanvasStore.getState().nestNode("a", null)).resolves.toBeUndefined();
+  });
+});
@@ -3,6 +3,15 @@ import type { WorkspaceData } from "./socket";
 import type { WorkspaceNodeData } from "./canvas";
 import { WORKSPACE_KIND } from "@/lib/workspace-kind";

+/** Thrown when workspace parent chains contain a cycle. Catching this at
+ *  load time lets the UI surface a degraded state instead of hanging. */
+export class TopologyCycleError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "TopologyCycleError";
+  }
+}
+
 const H_SPACING = 320;
 const V_SPACING = 200;

@@ -25,17 +34,38 @@ export function sortParentsBeforeChildren<T extends { id: string; parentId?: str
 ): T[] {
  const byId = new Map(nodes.map((n) => [n.id, n]));
  const visited = new Set<string>();
+  const visiting = new Set<string>();
  const out: T[] = [];
+
  const visit = (n: T) => {
    if (visited.has(n.id)) return;
+    if (visiting.has(n.id)) {
+      throw new TopologyCycleError(
+        `cyclic parent chain detected at workspace ${n.id}`,
+      );
+    }
+    visiting.add(n.id);
    if (n.parentId) {
      const parent = byId.get(n.parentId);
      if (parent && !visited.has(parent.id)) visit(parent);
    }
+    visiting.delete(n.id);
    visited.add(n.id);
    out.push(n);
  };
-  for (const n of nodes) visit(n);
+
+  try {
+    for (const n of nodes) visit(n);
+  } catch (err) {
+    if (err instanceof TopologyCycleError) {
+      // Fail-closed: a corrupt cycle is not recoverable by re-ordering.
+      // Return the input unchanged so callers (e.g. drag re-parent) don't
+      // hang, and let the next full hydrate surface the topology error.
+      return nodes;
+    }
+    throw err;
+  }
+
  // Separate roots, valid children, and orphans:
  // - roots: no parentId — true tree roots
  // - valid children: has parentId pointing to an existing node
@@ -323,12 +353,20 @@ export function buildNodesAndEdges(
  // after their parent regardless of the order the API returned them.
  const byId = new Map(workspaces.map((w) => [w.id, w]));
  const visited = new Set<string>();
+  const visiting = new Set<string>();
  const sorted: WorkspaceData[] = [];
  function visit(ws: WorkspaceData) {
    if (visited.has(ws.id)) return;
+    if (visiting.has(ws.id)) {
+      throw new TopologyCycleError(
+        `cyclic parent chain detected at workspace ${ws.id}`,
+      );
+    }
+    visiting.add(ws.id);
    if (ws.parent_id && byId.has(ws.parent_id) && !visited.has(ws.parent_id)) {
      visit(byId.get(ws.parent_id)!);
    }
+    visiting.delete(ws.id);
    visited.add(ws.id);
    sorted.push(ws);
  }
@@ -20,6 +20,7 @@ import {
  CHILD_DEFAULT_WIDTH,
  PARENT_BOTTOM_PADDING,
  PARENT_SIDE_PADDING,
+  TopologyCycleError,
 } from "./canvas-topology";

 /**
@@ -488,7 +489,10 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
    const absOf = (id: string | null | undefined): { x: number; y: number } => {
      let sum = { x: 0, y: 0 };
      let cursor: string | null | undefined = id;
+      const seen = new Set<string>();
      while (cursor) {
+        if (seen.has(cursor)) break;
+        seen.add(cursor);
        const n = byId.get(cursor);
        if (!n) break;
        sum = { x: sum.x + n.position.x, y: sum.y + n.position.y };
@@ -499,7 +503,10 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
    const depthOf = (id: string | null | undefined): number => {
      let d = 0;
      let cursor: string | null | undefined = id;
+      const seen = new Set<string>();
      while (cursor) {
+        if (seen.has(cursor)) break;
+        seen.add(cursor);
        const n = byId.get(cursor);
        if (!n) break;
        cursor = n.data.parentId;
@@ -693,9 +700,12 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({

  isDescendant: (ancestorId, nodeId) => {
    const { nodes } = get();
+    const seen = new Set<string>();
    let current = nodes.find((n) => n.id === nodeId);
    while (current?.data.parentId) {
      if (current.data.parentId === ancestorId) return true;
+      if (seen.has(current.data.parentId)) return false;
+      seen.add(current.data.parentId);
      current = nodes.find((n) => n.id === current?.data.parentId);
    }
    return false;
@@ -716,7 +726,10 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
    const absOf = (id: string | null): { x: number; y: number } => {
      let sum = { x: 0, y: 0 };
      let cursor: string | null = id;
+      const seen = new Set<string>();
      while (cursor) {
+        if (seen.has(cursor)) break;
+        seen.add(cursor);
        const n = nodes.find((nn) => nn.id === cursor);
        if (!n) break;
        sum = { x: sum.x + n.position.x, y: sum.y + n.position.y };
@@ -745,7 +758,10 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
    const depthOf = (id: string | null | undefined): number => {
      let d = 0;
      let cursor: string | null | undefined = id;
+      const seen = new Set<string>();
      while (cursor) {
+        if (seen.has(cursor)) break;
+        seen.add(cursor);
        const n = nodes.find((nn) => nn.id === cursor);
        if (!n) break;
        cursor = n.data.parentId;
@@ -948,12 +964,24 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
        currentParentSizes.set(n.id, { width: w, height: h });
      }
    }
-    const { nodes, edges } = buildNodesAndEdges(
-      live,
-      layoutOverrides,
-      currentParentSizes,
-    );
-    set({ nodes, edges });
+    try {
+      const { nodes, edges } = buildNodesAndEdges(
+        live,
+        layoutOverrides,
+        currentParentSizes,
+      );
+      set({ nodes, edges });
+    } 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(() => {});
    }
@@ -1077,7 +1105,10 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
    const absOf = (id: string | null | undefined): { x: number; y: number } => {
      let sum = { x: 0, y: 0 };
      let cursor: string | null | undefined = id;
+      const seen = new Set<string>();
      while (cursor) {
+        if (seen.has(cursor)) break;
+        seen.add(cursor);
        const n = nodes.find((nn) => nn.id === cursor);
        if (!n) break;
        sum = { x: sum.x + n.position.x, y: sum.y + n.position.y };