fix: auto-layout zero-position nodes, fix new-node x===y stacking

canvas/src/store/__tests__/canvas-topology.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from "vitest";
-import { buildNodesAndEdges, extractSkillNames } from "../canvas-topology";
+import { buildNodesAndEdges, extractSkillNames, computeAutoLayout } from "../canvas-topology";
 import type { WorkspaceData } from "../socket";
 
 // ---------------------------------------------------------------------------
@@ -263,3 +263,97 @@ describe("extractSkillNames – mixed valid/invalid skills", () => {
     expect(extractSkillNames(card)).toEqual(["Coding", "Testing"]);
   });
 });
+
+// ---------------------------------------------------------------------------
+// computeAutoLayout
+// ---------------------------------------------------------------------------
+
+describe("computeAutoLayout – all nodes already positioned", () => {
+  it("returns empty map when all nodes have non-zero positions", () => {
+    const workspaces = [
+      makeWS({ id: "a", x: 100, y: 200 }),
+      makeWS({ id: "b", x: 400, y: 200 }),
+    ];
+    const overrides = computeAutoLayout(workspaces);
+    expect(overrides.size).toBe(0);
+  });
+});
+
+describe("computeAutoLayout – empty workspace list", () => {
+  it("returns empty map", () => {
+    const overrides = computeAutoLayout([]);
+    expect(overrides.size).toBe(0);
+  });
+});
+
+describe("computeAutoLayout – single zero-position root node", () => {
+  it("assigns a position to the zero node", () => {
+    const workspaces = [makeWS({ id: "ws-1", x: 0, y: 0 })];
+    const overrides = computeAutoLayout(workspaces);
+    expect(overrides.has("ws-1")).toBe(true);
+    const pos = overrides.get("ws-1")!;
+    expect(typeof pos.x).toBe("number");
+    expect(typeof pos.y).toBe("number");
+  });
+});
+
+describe("computeAutoLayout – multiple zero-position root nodes", () => {
+  it("spreads siblings horizontally (distinct x values)", () => {
+    const workspaces = [
+      makeWS({ id: "a", x: 0, y: 0 }),
+      makeWS({ id: "b", x: 0, y: 0 }),
+      makeWS({ id: "c", x: 0, y: 0 }),
+    ];
+    const overrides = computeAutoLayout(workspaces);
+    const positions = ["a", "b", "c"].map((id) => overrides.get(id)!);
+    const xs = positions.map((p) => p.x);
+    // All x values should be unique (nodes spread horizontally)
+    const uniqueXs = new Set(xs);
+    expect(uniqueXs.size).toBe(3);
+    // All at depth 0 → y should be 0
+    for (const p of positions) {
+      expect(p.y).toBe(0);
+    }
+  });
+});
+
+describe("computeAutoLayout – parent with zero-position children", () => {
+  it("places child at greater y than parent", () => {
+    const workspaces = [
+      makeWS({ id: "parent", x: 0, y: 0 }),
+      makeWS({ id: "child", parent_id: "parent", x: 0, y: 0 }),
+    ];
+    const overrides = computeAutoLayout(workspaces);
+    const parentPos = overrides.get("parent")!;
+    const childPos = overrides.get("child")!;
+    expect(childPos.y).toBeGreaterThan(parentPos.y);
+  });
+});
+
+describe("computeAutoLayout – anchored node not overridden", () => {
+  it("does not include already-positioned node in overrides", () => {
+    const workspaces = [
+      makeWS({ id: "anchored", x: 500, y: 300 }),
+      makeWS({ id: "zero", x: 0, y: 0 }),
+    ];
+    const overrides = computeAutoLayout(workspaces);
+    expect(overrides.has("anchored")).toBe(false);
+    expect(overrides.has("zero")).toBe(true);
+  });
+});
+
+describe("buildNodesAndEdges – layoutOverrides applied", () => {
+  it("uses override position instead of ws.x/ws.y for zero-position nodes", () => {
+    const workspaces = [makeWS({ id: "ws-1", x: 0, y: 0 })];
+    const overrides = new Map([["ws-1", { x: 150, y: 250 }]]);
+    const { nodes } = buildNodesAndEdges(workspaces, overrides);
+    expect(nodes[0].position).toEqual({ x: 150, y: 250 });
+  });
+
+  it("leaves non-overridden node at its own position", () => {
+    const workspaces = [makeWS({ id: "ws-2", x: 100, y: 200 })];
+    const overrides = new Map<string, { x: number; y: number }>();
+    const { nodes } = buildNodesAndEdges(workspaces, overrides);
+    expect(nodes[0].position).toEqual({ x: 100, y: 200 });
+  });
+});

canvas/src/store/canvas-events.ts

@@ -88,10 +88,13 @@ export function handleCanvasEvent(
           ),
         });
       } else {
-        // Spread new nodes so they don't stack at (0,0)
-        const offset = nodes.length * 40;
-        const x = offset;
-        const y = offset;
+        // Spread new nodes in a grid so they don't stack
+        const GRID_COL_WIDTH = 320;
+        const GRID_ROW_HEIGHT = 160;
+        const GRID_COLS = 4;
+        const rootIndex = nodes.filter((n) => !n.data.parentId).length;
+        const x = (rootIndex % GRID_COLS) * GRID_COL_WIDTH;
+        const y = Math.floor(rootIndex / GRID_COLS) * GRID_ROW_HEIGHT;
 
         set({
           nodes: [

canvas/src/store/canvas-topology.ts

@@ -2,38 +2,151 @@ import type { Node, Edge } from "@xyflow/react";
 import type { WorkspaceData } from "./socket";
 import type { WorkspaceNodeData } from "./canvas";
 
+const H_SPACING = 320;
+const V_SPACING = 200;
+
+/**
+ * Computes auto-layout positions for workspaces that have no persisted position
+ * (x === 0 AND y === 0). Workspaces with an existing non-zero position are used
+ * as anchors and are never moved.
+ *
+ * Returns a Map of workspace IDs → {x, y} for every workspace that was assigned
+ * a computed position (i.e. only the ones that were at 0,0). Callers should
+ * persist these back to the API so the positions survive reload.
+ */
+export function computeAutoLayout(
+  workspaces: WorkspaceData[]
+): Map<string, { x: number; y: number }> {
+  const overrides = new Map<string, { x: number; y: number }>();
+
+  // Separate anchored (already positioned) from zero-position workspaces
+  const anchored = new Set<string>();
+  for (const ws of workspaces) {
+    if (ws.x !== 0 || ws.y !== 0) {
+      anchored.add(ws.id);
+    }
+  }
+
+  // If every workspace is already positioned, nothing to do
+  const needsLayout = workspaces.filter((ws) => !anchored.has(ws.id));
+  if (needsLayout.length === 0) return overrides;
+
+  // Build parent→children map
+  const children = new Map<string | null, WorkspaceData[]>();
+  for (const ws of workspaces) {
+    const pid = ws.parent_id ?? null;
+    if (!children.has(pid)) children.set(pid, []);
+    children.get(pid)!.push(ws);
+  }
+
+  // Sort children by name for deterministic layout
+  for (const list of children.values()) {
+    list.sort((a, b) => a.name.localeCompare(b.name));
+  }
+
+  // Assigned positions (includes anchors from the original data + computed overrides)
+  const assigned = new Map<string, { x: number; y: number }>();
+  for (const ws of workspaces) {
+    if (anchored.has(ws.id)) {
+      assigned.set(ws.id, { x: ws.x, y: ws.y });
+    }
+  }
+
+  // BFS from root nodes that need layout
+  // Track the next X offset per depth row to spread siblings horizontally
+  const rowNextX = new Map<number, number>();
+
+  // Enqueue root-level nodes that need layout
+  const queue: Array<{ ws: WorkspaceData; depth: number }> = [];
+  const rootsNeedingLayout = (children.get(null) ?? []).filter(
+    (ws) => !anchored.has(ws.id)
+  );
+  for (const ws of rootsNeedingLayout) {
+    queue.push({ ws, depth: 0 });
+  }
+
+  while (queue.length > 0) {
+    const { ws, depth } = queue.shift()!;
+
+    // Skip if already assigned (e.g. anchored)
+    if (assigned.has(ws.id)) {
+      // Still enqueue its unpositioned children
+      const kids = (children.get(ws.id) ?? []).filter(
+        (c) => !anchored.has(c.id) && !assigned.has(c.id)
+      );
+      for (const kid of kids) {
+        queue.push({ ws: kid, depth: depth + 1 });
+      }
+      continue;
+    }
+
+    // Find parent's x as the center hint for this node
+    const parentPos = ws.parent_id ? assigned.get(ws.parent_id) : undefined;
+    const parentX = parentPos?.x ?? 0;
+
+    // Place node at the next available slot in this row
+    const currentRowX = rowNextX.get(depth) ?? (parentX - H_SPACING / 2);
+    const x = Math.max(currentRowX, parentX);
+    const y = depth * V_SPACING;
+
+    assigned.set(ws.id, { x, y });
+    overrides.set(ws.id, { x, y });
+    rowNextX.set(depth, x + H_SPACING);
+
+    // Enqueue children that need layout
+    const kids = (children.get(ws.id) ?? []).filter(
+      (c) => !anchored.has(c.id) && !assigned.has(c.id)
+    );
+    for (const kid of kids) {
+      queue.push({ ws: kid, depth: depth + 1 });
+    }
+  }
+
+  return overrides;
+}
+
 /**
  * Converts raw workspace data from the API into React Flow nodes and edges.
+ * Accepts an optional layoutOverrides map (from computeAutoLayout) to override
+ * positions for workspaces that were at 0,0.
  */
-export function buildNodesAndEdges(workspaces: WorkspaceData[]): {
+export function buildNodesAndEdges(
+  workspaces: WorkspaceData[],
+  layoutOverrides: Map<string, { x: number; y: number }> = new Map()
+): {
   nodes: Node<WorkspaceNodeData>[];
   edges: Edge[];
 } {
   // All workspaces become nodes (children are rendered inside parent via WorkspaceNode)
-  const nodes: Node<WorkspaceNodeData>[] = workspaces.map((ws) => ({
-    id: ws.id,
-    type: "workspaceNode",
-    position: { x: ws.x, y: ws.y },
-    // Don't set React Flow parentId — children render embedded inside the WorkspaceNode component
-    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: ws.parent_id,
-      currentTask: ws.current_task || "",
-      runtime: ws.runtime || "",
-      needsRestart: false,
-    },
-    // Hide child nodes from canvas — they render inside the parent WorkspaceNode
-    hidden: !!ws.parent_id,
-  }));
+  const nodes: Node<WorkspaceNodeData>[] = workspaces.map((ws) => {
+    const override = layoutOverrides.get(ws.id);
+    const x = override?.x ?? ws.x;
+    const y = override?.y ?? ws.y;
+    return {
+      id: ws.id,
+      type: "workspaceNode",
+      position: { x, y },
+      // Don't set React Flow parentId — children render embedded inside the WorkspaceNode component
+      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: ws.parent_id,
+        currentTask: ws.current_task || "",
+        runtime: ws.runtime || "",
+        needsRestart: false,
+      },
+      // Hide child nodes from canvas — they render inside the parent WorkspaceNode
+      hidden: !!ws.parent_id,
+    };
+  });
 
   // No parent→child edges — children are embedded inside the parent node.
   // Only create edges between siblings or cross-team connections if needed in future.

canvas/src/store/canvas.ts

@@ -8,7 +8,7 @@ import {
 import { api } from "@/lib/api";
 import type { WorkspaceData, WSMessage } from "./socket";
 import { handleCanvasEvent } from "./canvas-events";
-import { buildNodesAndEdges } from "./canvas-topology";
+import { buildNodesAndEdges, computeAutoLayout } from "./canvas-topology";
 
 // Re-export extracted types and functions so existing imports from "@/store/canvas" keep working
 export { summarizeWorkspaceCapabilities } from "./canvas-capabilities";
@@ -209,8 +209,12 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
   },
 
   hydrate: (workspaces: WorkspaceData[]) => {
-    const { nodes, edges } = buildNodesAndEdges(workspaces);
+    const layoutOverrides = computeAutoLayout(workspaces);
+    const { nodes, edges } = buildNodesAndEdges(workspaces, layoutOverrides);
     set({ nodes, edges });
+    for (const [nodeId, { x, y }] of layoutOverrides) {
+      api.patch(`/workspaces/${nodeId}`, { x, y }).catch(() => {});
+    }
   },
 
   applyEvent: (msg: WSMessage) => {