diff --git a/canvas/src/components/ProvisioningTimeout.tsx b/canvas/src/components/ProvisioningTimeout.tsx index 91cff327..b27ac98e 100644 --- a/canvas/src/components/ProvisioningTimeout.tsx +++ b/canvas/src/components/ProvisioningTimeout.tsx @@ -6,9 +6,27 @@ import { api } from "@/lib/api"; import { showToast } from "./Toaster"; import { ConsoleModal } from "./ConsoleModal"; -/** Default provisioning timeout in milliseconds (2 minutes). */ +/** Base provisioning timeout in milliseconds (2 minutes). Used as the + * floor; the effective threshold scales with the number of workspaces + * concurrently provisioning (see effectiveTimeoutMs below). */ export const DEFAULT_PROVISION_TIMEOUT_MS = 120_000; +/** The server provisions up to `PROVISION_CONCURRENCY` containers at + * once and paces the rest in a queue (`workspaceCreatePacingMs` = + * 2s). Mirrors the Go constants — if those change, bump these. */ +const PROVISION_CONCURRENCY = 3; +const PER_QUEUE_SLOT_EXTRA_MS = 45_000; // ~45s head-room per queued workspace + +/** Scale the base timeout by how many workspaces are provisioning at + * once. A 30-workspace org import has tail items that legitimately + * wait minutes before Docker even starts on them — flagging each as + * "stuck" after 2m creates a wall of 27 yellow banners that buries + * the canvas. */ +function effectiveTimeoutMs(base: number, concurrentCount: number): number { + const overflow = Math.max(0, concurrentCount - PROVISION_CONCURRENCY); + return base + overflow * PER_QUEUE_SLOT_EXTRA_MS; +} + interface TimeoutEntry { workspaceId: string; workspaceName: string; @@ -33,6 +51,10 @@ export function ProvisioningTimeout({ const [retrying, setRetrying] = useState>(new Set()); const [cancelling, setCancelling] = useState>(new Set()); const trackingRef = useRef>(new Map()); + // Workspaces the user explicitly dismissed — don't re-show their + // banner even if they stay in provisioning. Cleared when the + // workspace leaves provisioning (status changes). + const [dismissed, setDismissed] = useState>(new Set()); // Subscribe to provisioning nodes — use shallow compare to avoid infinite re-render // (filter+map creates new array reference on every store update) @@ -71,17 +93,34 @@ export function ProvisioningTimeout({ } } - // Also remove from timedOut list if no longer provisioning + // Also remove from timedOut list if no longer provisioning, and + // clear `dismissed` entries for workspaces that finished so a + // re-provision (e.g. retry) can surface a fresh banner. setTimedOut((prev) => prev.filter((e) => activeIds.has(e.workspaceId))); + setDismissed((prev) => { + let changed = false; + const next = new Set(prev); + for (const id of prev) { + if (!activeIds.has(id)) { + next.delete(id); + changed = true; + } + } + return changed ? next : prev; + }); // Interval to check for timeouts const interval = setInterval(() => { const now = Date.now(); const newTimedOut: TimeoutEntry[] = []; + const effective = effectiveTimeoutMs( + timeoutMs, + parsedProvisioningNodes.length, + ); for (const node of parsedProvisioningNodes) { const startedAt = tracking.get(node.id); - if (startedAt && now - startedAt >= timeoutMs) { + if (startedAt && now - startedAt >= effective) { newTimedOut.push({ workspaceId: node.id, workspaceName: node.name, @@ -104,6 +143,11 @@ export function ProvisioningTimeout({ return () => clearInterval(interval); }, [parsedProvisioningNodes, timeoutMs]); + const handleDismiss = useCallback((workspaceId: string) => { + setDismissed((prev) => new Set(prev).add(workspaceId)); + setTimedOut((prev) => prev.filter((e) => e.workspaceId !== workspaceId)); + }, []); + const RETRY_COOLDOWN_MS = 5_000; const [retryCooldown, setRetryCooldown] = useState>(new Set()); @@ -180,11 +224,16 @@ export function ProvisioningTimeout({ setConsoleFor(workspaceId); }, []); - if (timedOut.length === 0) return null; + const visibleTimedOut = useMemo( + () => timedOut.filter((e) => !dismissed.has(e.workspaceId)), + [timedOut, dismissed], + ); + + if (visibleTimedOut.length === 0) return null; return (
- {timedOut.map((entry) => { + {visibleTimedOut.map((entry) => { const elapsed = Math.round((Date.now() - entry.startedAt) / 1000); const isRetrying = retrying.has(entry.workspaceId); const isCancelling = cancelling.has(entry.workspaceId); @@ -210,8 +259,20 @@ export function ProvisioningTimeout({
-
- Provisioning Timeout +
+
+ Provisioning Timeout +
+
{entry.workspaceName}{" "} diff --git a/canvas/src/components/TemplatePalette.tsx b/canvas/src/components/TemplatePalette.tsx index 3f67bcba..35f75877 100644 --- a/canvas/src/components/TemplatePalette.tsx +++ b/canvas/src/components/TemplatePalette.tsx @@ -3,10 +3,12 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { api } from "@/lib/api"; import { useCanvasStore } from "@/store/canvas"; +import type { WorkspaceData } from "@/store/socket"; import { checkDeploySecrets, type PreflightResult, type ModelSpec } from "@/lib/deploy-preflight"; import { MissingKeysModal } from "./MissingKeysModal"; import { ConfirmDialog } from "./ConfirmDialog"; import { Spinner } from "./Spinner"; +import { showToast } from "./Toaster"; import { TIER_CONFIG } from "@/lib/design-tokens"; interface Template { @@ -40,10 +42,41 @@ export async function fetchOrgTemplates(): Promise { } } -/** Import an org template by directory name. Throws on platform error so the - * caller can surface the message in its error state. */ -export async function importOrgTemplate(dir: string): Promise { - await api.post("/org/import", { dir }); +/** Server response from POST /org/import. The handler returns 207 + * (StatusMultiStatus) with a populated `error` field when only some of + * the workspaces in the tree could be created — the HTTP status alone + * isn't enough to detect a partial failure. */ +interface OrgImportResponse { + org: string; + workspaces: Array<{ id: string; name: string }>; + count: number; + error?: string; +} + +/** Import an org template by directory name. Throws on platform error + * so the caller can surface the message in its error state. Also throws + * on 2xx-with-error-body (StatusMultiStatus) — without this check a + * partial failure (e.g. first workspace INSERT fails, 0 created) + * appears as a green success toast and the user sees no canvas update. + * + * Uses a long timeout because createWorkspaceTree paces sibling DB + * inserts by `workspaceCreatePacingMs` (2s) to avoid overwhelming + * Docker — a 15-workspace tree sleeps ~28s in the handler alone, + * which blows past the default 15s and makes the client report a + * spurious "signal timed out" error even though the server finished + * successfully. 2min covers trees up to ~60 workspaces. */ +const ORG_IMPORT_TIMEOUT_MS = 120_000; + +export async function importOrgTemplate(dir: string): Promise { + const resp = await api.post( + "/org/import", + { dir }, + { timeoutMs: ORG_IMPORT_TIMEOUT_MS }, + ); + if (resp && resp.error) { + throw new Error(`${resp.error} (created ${resp.count ?? 0} workspaces)`); + } + return resp; } /** @@ -81,8 +114,22 @@ export function OrgTemplatesSection() { setError(null); try { await importOrgTemplate(org.dir); + // Refresh canvas inline — the WebSocket may be offline, in which case + // WORKSPACE_PROVISIONING broadcasts never arrive and the user sees + // no change from clicking "Import org". A direct fetch guarantees + // the new workspaces land on canvas regardless of WS state. + try { + const workspaces = await api.get("/workspaces"); + useCanvasStore.getState().hydrate(workspaces); + } catch { + // Rehydrate failure is non-fatal; WS (if alive) or the next + // health-check cycle will eventually pick the new workspaces up. + } + showToast(`Imported "${org.name || org.dir}" (${org.workspaces} workspaces)`, "success"); } catch (e) { - setError(e instanceof Error ? e.message : "Import failed"); + const msg = e instanceof Error ? e.message : "Import failed"; + setError(msg); + showToast(`Import failed: ${msg}`, "error"); } finally { setImporting(null); } diff --git a/canvas/src/components/Toolbar.tsx b/canvas/src/components/Toolbar.tsx index 19cd04d2..f458e049 100644 --- a/canvas/src/components/Toolbar.tsx +++ b/canvas/src/components/Toolbar.tsx @@ -137,7 +137,11 @@ export function Toolbar() { Molecule AI
- {/* Status counts */} + {/* Status pills + workspace total in one segment — previously two + separate border-delimited cells; merged to drop a redundant + divider and keep the count compact. `whitespace-nowrap` prevents + "+ N sub" from wrapping onto a second line when the toolbar + gets tight. */}
{counts.offline > 0 && ( @@ -149,11 +153,8 @@ export function Toolbar() { {counts.failed > 0 && ( )} -
- - {/* Total */} -
- + + {counts.roots} workspace{counts.roots !== 1 ? "s" : ""} {counts.children > 0 && + {counts.children} sub} @@ -200,13 +201,18 @@ export function Toolbar() { )} + {/* Secondary tools below are icon-only (Figma/Linear pattern) — text + label is exposed via title + aria-label for hover/screen-reader + users. The primary Stop All / Restart Pending buttons above keep + their text because they are urgent + conditional. */} + {/* A2A topology overlay toggle */} {/* Audit trail shortcut — switches selected workspace's panel to the Audit tab */} @@ -244,13 +249,13 @@ export function Toolbar() { } }} aria-label="Open audit trail for selected workspace" - title="View audit ledger for the selected workspace" - className="flex items-center gap-1.5 px-2.5 py-1 bg-zinc-800/50 hover:bg-zinc-700/50 border border-zinc-700/40 rounded-lg transition-colors text-zinc-500 hover:text-zinc-300" + title="Audit — view ledger for the selected workspace" + className="flex items-center justify-center w-7 h-7 bg-zinc-800/50 hover:bg-zinc-700/50 border border-zinc-700/40 rounded-lg transition-colors text-zinc-500 hover:text-zinc-300" > {/* Scroll / ledger icon */} - Audit {/* Search shortcut */} {/* Quick help */}
{helpOpen && ( diff --git a/canvas/src/components/WorkspaceNode.tsx b/canvas/src/components/WorkspaceNode.tsx index a5e3192f..61870fee 100644 --- a/canvas/src/components/WorkspaceNode.tsx +++ b/canvas/src/components/WorkspaceNode.tsx @@ -202,9 +202,12 @@ export function WorkspaceNode({ id, data }: NodeProps>) ); })()} - {/* Role */} + {/* Role — clamp to 2 lines. Without this, a verbose role + * description (common on org-template imports) lets the card + * grow arbitrarily tall, which wrecks the grid-slot layout + * because siblings all plan for the same CHILD_DEFAULT_HEIGHT. */} {data.role && ( -
{data.role}
+
{data.role}
)} {/* Skills */} diff --git a/canvas/src/components/__tests__/TemplatePalette.test.ts b/canvas/src/components/__tests__/TemplatePalette.test.ts index 63cffb3f..ff584513 100644 --- a/canvas/src/components/__tests__/TemplatePalette.test.ts +++ b/canvas/src/components/__tests__/TemplatePalette.test.ts @@ -73,6 +73,26 @@ describe("importOrgTemplate", () => { mockFetch.mockRejectedValueOnce(new Error("offline")); await expect(importOrgTemplate("x")).rejects.toThrow("offline"); }); + + it("treats 2xx with `error` field as a failure (StatusMultiStatus partial)", async () => { + // Server returns 207 — `api.post` treats the 2xx as success and + // returns the body. Without the post-check, a partial failure + // (0 workspaces created) would surface as a green "Imported" + // toast and the user would see no canvas change. + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 207, + json: async () => ({ + org: "Data Team", + workspaces: [], + count: 0, + error: 'pq: column "collapsed" of relation "workspaces" does not exist', + }), + }); + await expect(importOrgTemplate("data-team")).rejects.toThrow( + /collapsed.*relation.*workspaces.*created 0 workspaces/, + ); + }); }); describe("module exports", () => { diff --git a/canvas/src/components/canvas/useCanvasViewport.ts b/canvas/src/components/canvas/useCanvasViewport.ts index c7ce9169..8ab916e5 100644 --- a/canvas/src/components/canvas/useCanvasViewport.ts +++ b/canvas/src/components/canvas/useCanvasViewport.ts @@ -25,14 +25,59 @@ export function useCanvasViewport() { const saveViewport = useCanvasStore((s) => s.saveViewport); const saveTimerRef = useRef>(undefined); const panTimerRef = useRef>(undefined); + const autoFitTimerRef = useRef>(undefined); + // Tracks whether any workspace was provisioning on the previous + // render so we can detect the boundary when the last one finishes + // and auto-fit the viewport around the whole tree. + const hadProvisioningRef = useRef(false); useEffect(() => { return () => { clearTimeout(saveTimerRef.current); clearTimeout(panTimerRef.current); + clearTimeout(autoFitTimerRef.current); }; }, []); + // Auto-fit the viewport once all workspaces finish provisioning. Org + // imports land dozens of new nodes off-screen; without a follow-up + // fit, the user has to manually pan + zoom to find what they just + // created. Only fires when TRANSITIONING from some-provisioning to + // zero-provisioning — not on every re-render. + const provisioningCount = useCanvasStore( + (s) => s.nodes.filter((n) => n.data.status === "provisioning").length, + ); + const nodeCount = useCanvasStore((s) => s.nodes.length); + + useEffect(() => { + const hasProvisioning = provisioningCount > 0; + const wasProvisioning = hadProvisioningRef.current; + hadProvisioningRef.current = hasProvisioning; + + if (wasProvisioning && !hasProvisioning && nodeCount > 0) { + clearTimeout(autoFitTimerRef.current); + // 1200ms settle delay: lets React Flow's DOM measurement pass + // resize newly-online parents before we compute bounds. + // Measuring too early gives us the pre-render skeleton bbox and + // fitView zooms to that smaller-than-real rectangle. + autoFitTimerRef.current = setTimeout(() => { + fitView({ + duration: 1200, + padding: 0.25, + // Cap zoom-in: a small tree (2-3 nodes) would otherwise end + // up at the 2x maxZoom, visually implying "something is + // wrong". 0.8 reads like "here's your whole org" even when + // the tree is small. + maxZoom: 0.8, + // Cap zoom-out: fitView would fall back to the component's + // minZoom=0.1 on a sparse/outlier layout, leaving the user + // staring at a postage-stamp canvas. 0.25 is the floor. + minZoom: 0.25, + }); + }, 1200); + } + }, [provisioningCount, nodeCount, fitView]); + // Pan to a newly deployed / targeted workspace. 100ms delay so React // Flow has time to measure a just-rendered node. useEffect(() => { diff --git a/canvas/src/lib/api.ts b/canvas/src/lib/api.ts index 5ee1d8ce..e65d92fd 100644 --- a/canvas/src/lib/api.ts +++ b/canvas/src/lib/api.ts @@ -11,14 +11,22 @@ export const PLATFORM_URL = // 15s is long enough for slow CP queries but short enough that a // hung backend doesn't leave the UI spinning forever. The abort // propagates through AbortController so React components can observe -// the error and render a retry affordance. +// the error and render a retry affordance. Callers that know the +// endpoint is intentionally slow (org import walks a tree of +// workspaces with server-side pacing) can pass `timeoutMs` to +// override. const DEFAULT_TIMEOUT_MS = 15_000; +export interface RequestOptions { + timeoutMs?: number; +} + async function request( method: string, path: string, body?: unknown, retryCount = 0, + options?: RequestOptions, ): Promise { // SaaS cross-origin shape: // - X-Molecule-Org-Slug: derived from window.location.hostname by @@ -37,7 +45,7 @@ async function request( headers, body: body ? JSON.stringify(body) : undefined, credentials: "include", - signal: AbortSignal.timeout(DEFAULT_TIMEOUT_MS), + signal: AbortSignal.timeout(options?.timeoutMs ?? DEFAULT_TIMEOUT_MS), }); // Transient rate-limit recovery. A single IP bucket can momentarily // spike on page load (several panels hydrate simultaneously). Instead @@ -49,7 +57,7 @@ async function request( const retryAfter = retryAfterHeader ? parseInt(retryAfterHeader, 10) : NaN; const delayMs = Number.isFinite(retryAfter) ? Math.min(retryAfter, 20) * 1000 : 2000; await new Promise((resolve) => setTimeout(resolve, delayMs)); - return request(method, path, body, retryCount + 1); + return request(method, path, body, retryCount + 1, options); } if (res.status === 401) { // Session expired or credentials lost. On SaaS (tenant subdomain) @@ -75,9 +83,9 @@ async function request( } export const api = { - get: (path: string) => request("GET", path), - post: (path: string, body?: unknown) => request("POST", path, body), - patch: (path: string, body?: unknown) => request("PATCH", path, body), - put: (path: string, body?: unknown) => request("PUT", path, body), - del: (path: string) => request("DELETE", path), + get: (path: string, options?: RequestOptions) => request("GET", path, undefined, 0, options), + post: (path: string, body?: unknown, options?: RequestOptions) => request("POST", path, body, 0, options), + patch: (path: string, body?: unknown, options?: RequestOptions) => request("PATCH", path, body, 0, options), + put: (path: string, body?: unknown, options?: RequestOptions) => request("PUT", path, body, 0, options), + del: (path: string, options?: RequestOptions) => request("DELETE", path, undefined, 0, options), }; diff --git a/canvas/src/store/__tests__/canvas.test.ts b/canvas/src/store/__tests__/canvas.test.ts index a183d228..df0460f6 100644 --- a/canvas/src/store/__tests__/canvas.test.ts +++ b/canvas/src/store/__tests__/canvas.test.ts @@ -914,6 +914,39 @@ describe("setCollapsed", () => { })); expect(afterHydrate).toEqual(afterCollapse); }); + + it("sizes the expanded parent to fit nested-parent children, not leaf-count", () => { + // Regression: when a collapsed parent contains a child that is + // itself a parent (CTO → Dev Lead → 6 engineers), expanding must + // use each direct child's actual rendered size — not the + // leaf-count formula. Otherwise the container is too small and + // Dev Lead (wide enough for 6 engineers in a grid) overflows. + useCanvasStore.getState().hydrate([ + makeWS({ id: "cto", name: "CTO", collapsed: true }), + makeWS({ id: "devLead", name: "Dev Lead", parent_id: "cto" }), + makeWS({ id: "fe", name: "Frontend", parent_id: "devLead" }), + makeWS({ id: "be", name: "Backend", parent_id: "devLead" }), + makeWS({ id: "mo", name: "Mobile", parent_id: "devLead" }), + makeWS({ id: "do", name: "DevOps", parent_id: "devLead" }), + makeWS({ id: "se", name: "Security", parent_id: "devLead" }), + makeWS({ id: "qa", name: "QA", parent_id: "devLead" }), + ]); + const devLeadNode = useCanvasStore + .getState() + .nodes.find((n) => n.id === "devLead")!; + const devLeadW = devLeadNode.width as number; + + useCanvasStore.getState().setCollapsed("cto", false); + + const ctoAfter = useCanvasStore + .getState() + .nodes.find((n) => n.id === "cto")!; + // CTO's new width must be wide enough to host its Dev Lead child + // plus the parent's own padding. Leaf-count formula would yield + // ~272 (one 240px leaf slot); subtree-aware should be ≥ Dev Lead + // plus side padding. + expect(ctoAfter.width).toBeGreaterThanOrEqual(devLeadW); + }); }); // ---------- bumpZOrder ---------- diff --git a/canvas/src/store/canvas-topology.ts b/canvas/src/store/canvas-topology.ts index 81c26c24..9c1cb25f 100644 --- a/canvas/src/store/canvas-topology.ts +++ b/canvas/src/store/canvas-topology.ts @@ -38,12 +38,26 @@ export function sortParentsBeforeChildren { + const row = Math.floor(i / cols); + out[row] = Math.max(out[row], s.height); + }); + return out; +} + +/** Uniform column width = max of all sibling widths. Keeps the grid + * rectangular (alternative: variable col widths — visually unstable + * when one sibling is much wider than the rest). */ +function colWidthOf(sizes: NodeSize[]): number { + return sizes.reduce((m, s) => Math.max(m, s.width), 0); +} + /** - * Minimum parent size that still fits `childCount` children laid out via - * defaultChildSlot. Never shrinks below the leaf-card min. + * Grid slot for the n-th sibling when siblings have variable sizes + * (e.g., a mix of leaves and nested parents). Uniform column width + + * per-row max height, so bigger nested parents push their row down + * without displacing columns. + */ +export function childSlotInGrid( + index: number, + siblingSizes: NodeSize[], +): { x: number; y: number } { + if (siblingSizes.length === 0) return { x: PARENT_SIDE_PADDING, y: PARENT_HEADER_PADDING }; + const cols = Math.min(GRID_COLS, siblingSizes.length); + const col = index % cols; + const row = Math.floor(index / cols); + const colW = colWidthOf(siblingSizes); + const rowHs = rowHeightsOf(siblingSizes, cols); + const x = PARENT_SIDE_PADDING + col * (colW + CHILD_GUTTER); + let y = PARENT_HEADER_PADDING; + for (let r = 0; r < row; r++) y += rowHs[r] + CHILD_GUTTER; + return { x, y }; +} + +/** + * Minimum parent size that still fits `childCount` uniformly-sized + * children. Leaf-slot variant — kept for back-compat with callers that + * don't have per-child sizes (bumpZOrder, arrangeChildren). */ export function parentMinSize(childCount: number): { width: number; height: number } { if (childCount <= 0) { return { width: 210, height: 120 }; } - const cols = Math.min(2, childCount); - const rows = Math.ceil(childCount / 2); + const cols = Math.min(GRID_COLS, childCount); + const rows = Math.ceil(childCount / cols); const width = PARENT_SIDE_PADDING * 2 + cols * CHILD_DEFAULT_WIDTH + @@ -84,6 +151,30 @@ export function parentMinSize(childCount: number): { width: number; height: numb return { width, height }; } +/** + * Minimum parent size that fits a set of (possibly non-uniform) + * children. Uniform column width, per-row max height — matches the + * geometry produced by `childSlotInGrid`. Used when a parent has + * grandchildren and a leaf-slot-sized grid can't hold the real, + * bigger nested cards. + */ +export function parentMinSizeFromChildren(children: NodeSize[]): NodeSize { + if (children.length === 0) return { width: 210, height: 120 }; + const cols = Math.min(GRID_COLS, children.length); + const rows = Math.ceil(children.length / cols); + const colW = colWidthOf(children); + const rowHs = rowHeightsOf(children, cols); + const totalRowH = rowHs.reduce((a, b) => a + b, 0); + return { + width: PARENT_SIDE_PADDING * 2 + colW * cols + CHILD_GUTTER * (cols - 1), + height: + PARENT_HEADER_PADDING + + totalRowH + + CHILD_GUTTER * (rows - 1) + + PARENT_BOTTOM_PADDING, + }; +} + /** * 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 @@ -236,13 +327,44 @@ export function buildNodesAndEdges( } } + // Index direct children per parent for post-order subtree sizing. + // We walk `sorted` in REVERSE (post-order — children first) so + // subtreeSize[parent] sees its grandchildren-inclusive sizes via the + // already-computed subtreeSize[child]. + const childrenByParent = new Map(); + for (const ws of workspaces) { + if (ws.parent_id && byId.has(ws.parent_id)) { + const arr = childrenByParent.get(ws.parent_id) ?? []; + arr.push(ws); + childrenByParent.set(ws.parent_id, arr); + } + } + const subtreeSize = new Map(); + for (let i = sorted.length - 1; i >= 0; i--) { + const ws = sorted[i]; + const kids = childrenByParent.get(ws.id) ?? []; + if (kids.length === 0 || ws.collapsed) { + subtreeSize.set(ws.id, { width: CHILD_DEFAULT_WIDTH, height: CHILD_DEFAULT_HEIGHT }); + } else { + const kidSizes = kids.map((k) => + subtreeSize.get(k.id) ?? { width: CHILD_DEFAULT_WIDTH, height: CHILD_DEFAULT_HEIGHT }, + ); + subtreeSize.set(ws.id, parentMinSizeFromChildren(kidSizes)); + } + } + // Track each parent's initial size so we can reset children that land // outside those bounds. Parents without children fall back to the leaf - // default; parents with children get the grid-derived minimum. + // default; parents with children get the grid-derived minimum — which + // now accounts for grandchildren via subtreeSize, so a nested parent + // no longer overflows its slot. const parentSize = new Map(); for (const ws of workspaces) { - const n = childCounts.get(ws.id) ?? 0; - parentSize.set(ws.id, n > 0 ? parentMinSize(n) : { width: 260, height: 140 }); + // Reuse subtreeSize — it already accounts for nested grandchildren. + parentSize.set( + ws.id, + subtreeSize.get(ws.id) ?? { width: CHILD_DEFAULT_WIDTH, height: CHILD_DEFAULT_HEIGHT }, + ); } // Running index of children already placed per parent — used to hand @@ -318,14 +440,21 @@ export function buildNodesAndEdges( // legacy huge positive (position.x = 50000): // child.left = 50000 >= parent.right → no overlap → rescued const psize = parentSize.get(ws.parent_id!)!; + const myW = subtreeSize.get(ws.id)?.width ?? CHILD_DEFAULT_WIDTH; + const myH = subtreeSize.get(ws.id)?.height ?? CHILD_DEFAULT_HEIGHT; const overlapsX = - position.x + CHILD_DEFAULT_WIDTH > 0 && position.x < psize.width; + position.x + myW > 0 && position.x < psize.width; const overlapsY = - position.y + CHILD_DEFAULT_HEIGHT > 0 && position.y < psize.height; + position.y + myH > 0 && position.y < psize.height; if (!overlapsX || !overlapsY) { const idx = nextChildIndex.get(ws.parent_id!) ?? 0; nextChildIndex.set(ws.parent_id!, idx + 1); - position = defaultChildSlot(idx); + // Use sibling-size-aware grid so a nested parent doesn't collide + // with a leaf sibling in the next row. + const siblings = (childrenByParent.get(ws.parent_id!) ?? []).map( + (c) => subtreeSize.get(c.id) ?? { width: CHILD_DEFAULT_WIDTH, height: CHILD_DEFAULT_HEIGHT }, + ); + position = childSlotInGrid(idx, siblings); } } const node: Node = { @@ -365,15 +494,26 @@ export function buildNodesAndEdges( if (hiddenById.get(ws.id)) { node.hidden = true; } - // Give parents a measured-ish starting size so NodeResizer has a - // baseline and child positions have somewhere to live. Without this, - // parents start at React Flow's default min size (well under a - // single child) and children render visually outside their parent - // until the next resize measurement settles. - if ((childCounts.get(ws.id) ?? 0) > 0) { + // Seed every node with an explicit starting size so the initial + // grid layout is stable before React Flow has measured the DOM. + // - Parents (has children, not collapsed): sized to fit the + // child grid via parentMinSize so children don't render + // outside the bounds on first paint. + // - Collapsed parents: leaf-sized (header-only card). + // - Leaves: leaf-sized — they land in their grid slot cleanly. + // + // NodeResizer still drives user-initiated growth at runtime; these + // are only the initial values, and React Flow updates them in place + // when the user drags a resize handle. A future hydrate() will + // reset to the default until we persist width/height server-side. + const kids = childCounts.get(ws.id) ?? 0; + if (kids > 0 && !ws.collapsed) { const size = parentSize.get(ws.id)!; node.width = size.width; node.height = size.height; + } else { + node.width = CHILD_DEFAULT_WIDTH; + node.height = CHILD_DEFAULT_HEIGHT; } return node; }); diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index 649647a8..2cec82ea 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -13,6 +13,7 @@ import { buildNodesAndEdges, computeAutoLayout, defaultChildSlot, + parentMinSizeFromChildren, sortParentsBeforeChildren, CHILD_DEFAULT_HEIGHT, CHILD_DEFAULT_WIDTH, @@ -836,15 +837,42 @@ export const useCanvasStore = create((set, get) => ({ stack.push({ id: childId, hidden: hidden || isCollapsed }); } } + // Expanded size must fit the target's ACTUAL children, including + // any nested-parent children that are themselves oversized. Using a + // leaf-count formula (parentMinSize) would undersize the parent + // whenever a child was itself a team — e.g. CTO expanding to show + // Dev Lead (which carries 6 engineers) would render Dev Lead + // clipped. Read each direct child's current width/height from the + // node itself; those already reflect the subtree sizing computed + // in buildNodesAndEdges. + const directChildIds = childrenByParent.get(parentId) ?? []; + const childSizes = directChildIds.map((cid) => { + const cn = nodes.find((n) => n.id === cid); + return { + width: (cn?.width as number | undefined) ?? CHILD_DEFAULT_WIDTH, + height: (cn?.height as number | undefined) ?? CHILD_DEFAULT_HEIGHT, + }; + }); + const expandedSize = parentMinSizeFromChildren(childSizes); + set({ nodes: nodes.map((n) => { const isTarget = n.id === parentId; const nextHidden = hiddenById.get(n.id) ?? false; if (!isTarget && n.hidden === nextHidden) return n; + if (!isTarget) { + return { ...n, hidden: nextHidden }; + } + // Target parent: update collapsed flag + size. Dropping width/ + // height would leave the node at its prior (possibly huge) + // dimensions after a collapse, leaving a gigantic empty card + // with no visible children. return { ...n, hidden: nextHidden, - data: isTarget ? { ...n.data, collapsed } : n.data, + data: { ...n.data, collapsed }, + width: collapsed ? CHILD_DEFAULT_WIDTH : expandedSize.width, + height: collapsed ? CHILD_DEFAULT_HEIGHT : expandedSize.height, }; }), }); diff --git a/workspace-server/internal/handlers/handlers_additional_test.go b/workspace-server/internal/handlers/handlers_additional_test.go index 2d1dc930..cb638a6f 100644 --- a/workspace-server/internal/handlers/handlers_additional_test.go +++ b/workspace-server/internal/handlers/handlers_additional_test.go @@ -191,12 +191,14 @@ func TestWorkspaceUpdate_Collapsed(t *testing.T) { broadcaster := newTestBroadcaster() handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) - // Canvas "collapse team" flip — the handler must run the UPDATE - // to persist the flag, otherwise the UI state resets on reload. + // Canvas "collapse team" flip — the handler must run the UPSERT + // on canvas_layouts to persist the flag, otherwise the UI state + // resets on reload. `collapsed` lives on canvas_layouts, not + // workspaces (see 005_canvas_layouts.sql). mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id"). WithArgs("dddddddd-0005-0000-0000-000000000000"). WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true)) - mock.ExpectExec("UPDATE workspaces SET collapsed"). + mock.ExpectExec("INSERT INTO canvas_layouts .* collapsed"). WithArgs("dddddddd-0005-0000-0000-000000000000", true). WillReturnResult(sqlmock.NewResult(0, 1)) diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index 872b2169..842aa254 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -29,6 +29,115 @@ const workspaceCreatePacingMs = 2000 // fires 39 goroutines that all hit Docker at once, causing timeouts (#1084). const provisionConcurrency = 3 +// Child grid layout constants — kept in sync with canvas-topology.ts on +// the client. Children laid on import use the same 2-column grid so the +// nested view is clean out of the box. Before this, YAML-declared +// canvas coords (absolute, horizontally fanned at y=180) produced an +// overlapping mess under the nested render (see screenshot in PR +// #1981 thread). +const ( + childDefaultWidth = 240.0 + childDefaultHeight = 130.0 + childGutter = 14.0 + parentHeaderPadding = 130.0 + parentSidePadding = 16.0 + childGridColumnCount = 2 +) + +// childSlot computes the child-relative position for the N-th sibling in +// a parent's 2-column grid. Matches defaultChildSlot in +// canvas-topology.ts exactly — change them together. Leaf-sized slots +// only; for variable-size siblings use childSlotInGrid below. +func childSlot(index int) (x, y float64) { + col := index % childGridColumnCount + row := index / childGridColumnCount + x = parentSidePadding + float64(col)*(childDefaultWidth+childGutter) + y = parentHeaderPadding + float64(row)*(childDefaultHeight+childGutter) + return +} + +type nodeSize struct { + width, height float64 +} + +// sizeOfSubtree computes the bounding-box size for a workspace and its +// entire descendant tree as rendered by the canvas grid layout. +// Post-order: leaves return the CHILD_DEFAULT footprint; parents return +// the size that fits all direct children (which may themselves be +// parents with grandchildren). Matches the client's +// `subtreeSize` pass in canvas-topology.ts so the server can lay out +// org imports the same way the canvas will render them. +func sizeOfSubtree(ws OrgWorkspace) nodeSize { + if len(ws.Children) == 0 { + return nodeSize{childDefaultWidth, childDefaultHeight} + } + cols := childGridColumnCount + if len(ws.Children) < cols { + cols = len(ws.Children) + } + rows := (len(ws.Children) + cols - 1) / cols + childSizes := make([]nodeSize, len(ws.Children)) + maxColW := 0.0 + for i, c := range ws.Children { + childSizes[i] = sizeOfSubtree(c) + if childSizes[i].width > maxColW { + maxColW = childSizes[i].width + } + } + rowHeights := make([]float64, rows) + for i, cs := range childSizes { + row := i / cols + if cs.height > rowHeights[row] { + rowHeights[row] = cs.height + } + } + totalRowH := 0.0 + for _, h := range rowHeights { + totalRowH += h + } + return nodeSize{ + width: parentSidePadding*2 + maxColW*float64(cols) + childGutter*float64(cols-1), + height: parentHeaderPadding + totalRowH + childGutter*float64(rows-1) + parentSidePadding, + } +} + +// childSlotInGrid computes the relative position of sibling `index` +// given all siblings' subtree sizes. Uniform column width (= max width +// across siblings), per-row max height, so a nested parent sibling +// pushes its row down without displacing the column grid. Matches the +// TS mirror in canvas-topology.ts. +func childSlotInGrid(index int, siblingSizes []nodeSize) (x, y float64) { + if len(siblingSizes) == 0 { + return parentSidePadding, parentHeaderPadding + } + cols := childGridColumnCount + if len(siblingSizes) < cols { + cols = len(siblingSizes) + } + rows := (len(siblingSizes) + cols - 1) / cols + maxColW := 0.0 + for _, s := range siblingSizes { + if s.width > maxColW { + maxColW = s.width + } + } + rowHeights := make([]float64, rows) + for i, s := range siblingSizes { + row := i / cols + if s.height > rowHeights[row] { + rowHeights[row] = s.height + } + } + col := index % cols + row := index / cols + x = parentSidePadding + float64(col)*(maxColW+childGutter) + y = parentHeaderPadding + for r := 0; r < row; r++ { + y += rowHeights[r] + childGutter + } + return +} + // orgImportScheduleSQL is the upsert executed for every schedule during // org/import. Extracted to a const so TestImport_OrgScheduleSQLShape can // assert its shape without regex-scanning org.go (issue #24 follow-up). @@ -300,9 +409,12 @@ func (h *OrgHandler) Import(c *gin.Context) { // Semaphore limits concurrent Docker provisioning (#1084). provisionSem := make(chan struct{}, provisionConcurrency) - // Recursively create workspaces + // Recursively create workspaces. Root workspaces keep their YAML + // canvas coords; children are positioned by createWorkspaceTree + // using subtree-aware grid slots (children that are themselves + // parents get a bigger slot so they don't overflow into siblings). for _, ws := range tmpl.Workspaces { - if err := h.createWorkspaceTree(ws, nil, tmpl.Defaults, orgBaseDir, &results, provisionSem); err != nil { + if err := h.createWorkspaceTree(ws, nil, ws.Canvas.X, ws.Canvas.Y, tmpl.Defaults, orgBaseDir, &results, provisionSem); err != nil { createErr = err break } diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index bfc3e251..5e50e7ae 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -21,7 +21,14 @@ import ( "github.com/Molecule-AI/molecule-monorepo/platform/internal/scheduler" "github.com/google/uuid" ) -func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defaults OrgDefaults, orgBaseDir string, results *[]map[string]interface{}, provisionSem chan struct{}) error { +// createWorkspaceTree recursively materialises an OrgWorkspace (and its +// descendants) into the workspaces + canvas_layouts tables and kicks off +// Docker provisioning. absX/absY are THIS workspace's absolute canvas +// coordinates — roots inherit them from ws.Canvas, children receive +// parent.abs + childSlotInGrid(index, siblingSizes) computed by the +// caller. Storing already-absolute coords means a child that is itself +// a parent can simply compound the grid without any per-call math. +func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX, absY float64, defaults OrgDefaults, orgBaseDir string, results *[]map[string]interface{}, provisionSem chan struct{}) error { // Apply defaults runtime := ws.Runtime if runtime == "" { @@ -88,26 +95,35 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa ctx := context.Background() - // Org-template imports can drop dozens of nested workspaces onto the - // canvas at once. Letting them render expanded by default sprays - // child cards across the viewport (sibling workspaces spill below - // the parent before the user can orient themselves). Default every - // parent in the imported tree to collapsed — the parent card shows - // only its header + "N sub" badge until the user double-clicks to - // expand it. Leaf workspaces stay expanded (nothing to hide). - initialCollapsed := len(ws.Children) > 0 + // Org-template imports default to expanded so children render + // visually nested inside their parent — matches the user's mental + // model ("all children should be in front of its parent"). The + // topology rescue heuristic lays any children whose YAML coords + // fall outside the computed parent bbox into a tidy 2-column grid + // (see canvas-topology.ts), so imports don't spray the viewport. + initialCollapsed := false _, err := db.DB.ExecContext(ctx, ` - INSERT INTO workspaces (id, name, role, tier, runtime, awareness_namespace, status, parent_id, workspace_dir, workspace_access, collapsed) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) - `, id, ws.Name, role, tier, runtime, awarenessNS, "provisioning", parentID, workspaceDir, workspaceAccess, initialCollapsed) + INSERT INTO workspaces (id, name, role, tier, runtime, awareness_namespace, status, parent_id, workspace_dir, workspace_access) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + `, id, ws.Name, role, tier, runtime, awarenessNS, "provisioning", parentID, workspaceDir, workspaceAccess) if err != nil { log.Printf("Org import: failed to create %s: %v", ws.Name, err) return fmt.Errorf("failed to create %s: %w", ws.Name, err) } - // Canvas layout with coordinates from YAML - if _, err := db.DB.ExecContext(ctx, `INSERT INTO canvas_layouts (workspace_id, x, y) VALUES ($1, $2, $3)`, id, ws.Canvas.X, ws.Canvas.Y); err != nil { + // Canvas layout — absX/absY were computed by the caller using the + // subtree-aware grid (childSlotInGrid) so a nested-parent child + // doesn't clip into its siblings. Raw YAML canvas coords are only + // consulted at the root: many templates predate the nested-parent + // model and author them as a flat horizontal row (y=180, x=100..1220), + // which overlaps chaotically once the cards render inside a parent + // container. + // + // `collapsed` lives on canvas_layouts (005_canvas_layouts.sql), not + // on workspaces; the UI-only flag is intentionally decoupled from + // the workspace row. + if _, err := db.DB.ExecContext(ctx, `INSERT INTO canvas_layouts (workspace_id, x, y, collapsed) VALUES ($1, $2, $3, $4)`, id, absX, absY, initialCollapsed); err != nil { log.Printf("Org import: canvas layout insert failed for %s: %v", ws.Name, err) } @@ -480,11 +496,24 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa // Recurse into children. Brief pacing avoids overwhelming Docker when // creating many containers in sequence; container provisioning runs in // goroutines so the main createWorkspaceTree returns quickly. - for _, child := range ws.Children { - if err := h.createWorkspaceTree(child, &id, defaults, orgBaseDir, results, provisionSem); err != nil { - return err + // Children's abs coords = this.abs + childSlotInGrid(index, siblingSizes), + // with sibling sizes computed by sizeOfSubtree so a nested-parent + // child claims a bigger grid slot than a leaf sibling — no slot + // clipping across mixed leaf / parent siblings. + if len(ws.Children) > 0 { + siblingSizes := make([]nodeSize, len(ws.Children)) + for i, c := range ws.Children { + siblingSizes[i] = sizeOfSubtree(c) + } + for i, child := range ws.Children { + slotX, slotY := childSlotInGrid(i, siblingSizes) + childAbsX := absX + slotX + childAbsY := absY + slotY + if err := h.createWorkspaceTree(child, &id, childAbsX, childAbsY, defaults, orgBaseDir, results, provisionSem); err != nil { + return err + } + time.Sleep(workspaceCreatePacingMs * time.Millisecond) } - time.Sleep(workspaceCreatePacingMs * time.Millisecond) } return nil diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index b991cb2f..1d428183 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -191,8 +191,14 @@ func (h *WorkspaceHandler) Update(c *gin.Context) { if collapsed, ok := body["collapsed"]; ok { // `collapsed` is the canvas UI-only flag that hides descendants // in the tree view (WorkspaceNode renders the parent as header- - // only). Persisting it here so the state survives reload. - if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET collapsed = $2, updated_at = now() WHERE id = $1`, id, collapsed); err != nil { + // only). It lives on canvas_layouts (005_canvas_layouts.sql), + // not workspaces — UPSERT because workspaces created outside the + // canvas flow (e.g. workspace_handler Create before a layout row + // exists) may not have a canvas_layouts row yet. + if _, err := db.DB.ExecContext(ctx, ` + INSERT INTO canvas_layouts (workspace_id, collapsed) VALUES ($1, $2) + ON CONFLICT (workspace_id) DO UPDATE SET collapsed = EXCLUDED.collapsed + `, id, collapsed); err != nil { log.Printf("Update collapsed error for %s: %v", id, err) } }