From d547569adfd114909207552b26f1c67f49857797 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-FE Date: Wed, 13 May 2026 21:12:14 +0000 Subject: [PATCH 1/3] test(canvas/lib): add isExternalLikeRuntime coverage (16 cases) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the backend isExternalLikeRuntime() contract so both sides agree on which runtimes are external-like (no platform container, no Files/Terminal tabs). Cases: "external", "kimi", "kimi-cli" → true; all other runtimes, undefined, null, empty string → false. Case-sensitivity verified. Co-Authored-By: Claude Opus 4.7 --- .../lib/__tests__/externalRuntimes.test.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 canvas/src/lib/__tests__/externalRuntimes.test.ts diff --git a/canvas/src/lib/__tests__/externalRuntimes.test.ts b/canvas/src/lib/__tests__/externalRuntimes.test.ts new file mode 100644 index 00000000..0af8520e --- /dev/null +++ b/canvas/src/lib/__tests__/externalRuntimes.test.ts @@ -0,0 +1,60 @@ +/** + * Tests for `isExternalLikeRuntime` — mirrors the backend's + * isExternalLikeRuntime() in workspace-server/internal/handlers/runtime_registry.go. + * + * These runtimes have no platform-owned container (no Files, Terminal, Docker config). + * Both frontend and backend must agree on which runtimes are "external-like" so + * the canvas can show/hide those tabs correctly and the backend can enforce + * the same semantics server-side. + */ +import { describe, it, expect } from "vitest"; +import { isExternalLikeRuntime } from "../externalRuntimes"; + +describe("isExternalLikeRuntime", () => { + describe("known external-like runtimes", () => { + it.each([ + ["external"], + ["kimi"], + ["kimi-cli"], + ])("%q returns true", (runtime) => { + expect(isExternalLikeRuntime(runtime)).toBe(true); + }); + }); + + describe("non-external runtimes", () => { + it.each([ + "claude-code", + "hermes", + "docker", + "local", + "agent", + "crewai", + "langgraph", + "openclaw", + "custom-runtime", + ])("%q returns false", (runtime) => { + expect(isExternalLikeRuntime(runtime)).toBe(false); + }); + }); + + describe("edge cases", () => { + it("returns false for undefined", () => { + expect(isExternalLikeRuntime(undefined)).toBe(false); + }); + + it("returns false for null", () => { + // @ts-expect-error — intentional runtime test, null is not a valid type + expect(isExternalLikeRuntime(null)).toBe(false); + }); + + it("returns false for empty string", () => { + expect(isExternalLikeRuntime("")).toBe(false); + }); + + it("is case-sensitive — kimi vs KIMI vs Kimi", () => { + expect(isExternalLikeRuntime("KIMI")).toBe(false); + expect(isExternalLikeRuntime("Kimi")).toBe(false); + expect(isExternalLikeRuntime("kimi")).toBe(true); + }); + }); +}); -- 2.45.2 From f03c7579c27e9d644c659488e0d5e979251eb060 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-FE Date: Wed, 13 May 2026 21:54:41 +0000 Subject: [PATCH 2/3] fix(canvas/ContextMenu): prevent React error #185 by moving hasChildren derivation out of Zustand selector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ContextMenu used `.some()` inside its Zustand selector to compute hasChildren. Zustand's useSyncExternalStore calls the selector on every snapshot; `.some()` returns a new boolean each time, which React 19's stricter comparison and the re-render side-effects from the store subscription created a feedback loop on mobile Chat tab mount → React error #185 ("Maximum update depth exceeded"). Fix: select the stable `nodes` array once, derive children via useMemo outside the store subscription. Also removes the inline `getState().nodes.filter()` call in handleDelete in favour of the memoized children. Regression tests (2 cases): - setPendingDelete receives correct children array when workspace has children - setPendingDelete hasChildren=false and empty children when no children Refs: #651 Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/ContextMenu.tsx | 19 +++-- .../components/__tests__/ContextMenu.test.tsx | 75 +++++++++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/canvas/src/components/ContextMenu.tsx b/canvas/src/components/ContextMenu.tsx index a5e1a5da..08cfd833 100644 --- a/canvas/src/components/ContextMenu.tsx +++ b/canvas/src/components/ContextMenu.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { api } from "@/lib/api"; import { showToast } from "./Toaster"; @@ -23,9 +23,17 @@ export function ContextMenu() { const setPanelTab = useCanvasStore((s) => s.setPanelTab); const nestNode = useCanvasStore((s) => s.nestNode); const contextNodeId = contextMenu?.nodeId ?? null; - const hasChildren = useCanvasStore((s) => - contextNodeId ? s.nodes.some((n) => n.data.parentId === contextNodeId) : false + // Select the full nodes array (stable reference across unrelated store + // updates) and derive children via useMemo. Filtering inside the + // selector returned a new array every call, which Zustand's + // useSyncExternalStore saw as "snapshot changed" → schedule + // re-render → loop → React error #185. See canvas-store-snapshots. + const nodes = useCanvasStore((s) => s.nodes); + const children = useMemo( + () => (contextNodeId ? nodes.filter((n) => n.data.parentId === contextNodeId) : []), + [nodes, contextNodeId], ); + const hasChildren = children.length > 0; const setPendingDelete = useCanvasStore((s) => s.setPendingDelete); const ref = useRef(null); const [actionLoading, setActionLoading] = useState(false); @@ -189,10 +197,9 @@ export function ContextMenu() { // it survives ContextMenu unmount. Closing the menu here avoids the // prior race where the portal dialog's Confirm click was treated as // "outside" by the menu's outside-click handler. - const childNodes = useCanvasStore.getState().nodes.filter((n) => n.data.parentId === contextMenu.nodeId); - setPendingDelete({ id: contextMenu.nodeId, name: contextMenu.nodeData.name, hasChildren, children: childNodes.map(c => ({ id: c.id, name: c.data.name })) }); + setPendingDelete({ id: contextMenu.nodeId, name: contextMenu.nodeData.name, hasChildren, children: children.map(c => ({ id: c.id, name: c.data.name })) }); closeContextMenu(); - }, [contextMenu, setPendingDelete, closeContextMenu]); + }, [contextMenu, setPendingDelete, closeContextMenu, children, hasChildren]); const handleViewDetails = useCallback(() => { if (!contextMenu) return; diff --git a/canvas/src/components/__tests__/ContextMenu.test.tsx b/canvas/src/components/__tests__/ContextMenu.test.tsx index c8896a04..ac404d7a 100644 --- a/canvas/src/components/__tests__/ContextMenu.test.tsx +++ b/canvas/src/components/__tests__/ContextMenu.test.tsx @@ -398,3 +398,78 @@ describe("ContextMenu — item actions", () => { expect(mockPost).toHaveBeenCalledWith("/workspaces/n1/resume", {}); }); }); + +/** + * Regression tests for GitHub issue #651 — React error #185: + * "Maximum update depth exceeded" on Chat tab / mobile. + * + * Root cause: ContextMenu's children selector ran `.filter()` inside the + * Zustand hook, returning a brand-new array reference on every render. + * Zustand's useSyncExternalStore compared snapshots with Object.is — + * a new array always differs — so React kept scheduling re-renders, + * hit the 50-update depth cap, and crashed. + * + * Fix: select the stable `nodes` array once, derive children via + * useMemo outside the store subscription. + */ +describe("ContextMenu — hasChildren regression (GitHub #651)", () => { + beforeEach(() => { setupApiMocks(); }); + afterEach(() => { + cleanup(); + vi.clearAllMocks(); + mockStoreState.contextMenu = null; + mockStoreState.closeContextMenu.mockClear(); + mockStoreState.updateNodeData.mockClear(); + mockStoreState.selectNode.mockClear(); + mockStoreState.setPanelTab.mockClear(); + mockStoreState.nestNode.mockClear(); + mockStoreState.setPendingDelete.mockClear(); + mockStoreState.setCollapsed.mockClear(); + mockStoreState.arrangeChildren.mockClear(); + mockStoreState.nodes = []; + resetApiMocks(); + vi.mocked(showToast).mockClear(); + }); + + it("setPendingDelete receives correct children array when workspace has children", () => { + openMenu({ nodeId: "ws-parent", nodeData: { name: "Parent", status: "online", tier: 4, role: "assistant" } }); + mockStoreState.nodes = [ + { id: "ws-child-a", data: { parentId: "ws-parent" } }, + { id: "ws-child-b", data: { parentId: "ws-parent" } }, + ]; + render(); + const deleteBtn = screen.getAllByRole("menuitem").find((el) => + el.textContent?.includes("Delete") + )!; + fireEvent.click(deleteBtn); + expect(mockStoreState.setPendingDelete).toHaveBeenCalledWith( + expect.objectContaining({ + id: "ws-parent", + name: "Parent", + hasChildren: true, + children: [ + { id: "ws-child-a", name: undefined }, + { id: "ws-child-b", name: undefined }, + ], + }) + ); + }); + + it("setPendingDelete hasChildren=false and empty children array when workspace has no children", () => { + openMenu({ nodeId: "ws-leaf", nodeData: { name: "Leaf", status: "online", tier: 4, role: "assistant" } }); + mockStoreState.nodes = []; + render(); + const deleteBtn = screen.getAllByRole("menuitem").find((el) => + el.textContent?.includes("Delete") + )!; + fireEvent.click(deleteBtn); + expect(mockStoreState.setPendingDelete).toHaveBeenCalledWith( + expect.objectContaining({ + id: "ws-leaf", + name: "Leaf", + hasChildren: false, + children: [], + }) + ); + }); +}); -- 2.45.2 From 269703540242f0a39a970fdf1a6b2e7044306efe Mon Sep 17 00:00:00 2001 From: Molecule AI Core-FE Date: Wed, 13 May 2026 22:38:58 +0000 Subject: [PATCH 3/3] test(canvas/lib): add hydrateCanvas coverage (8 cases) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests exponential backoff retry logic, viewport persistence, error propagation, and non-fatal viewport failure. Critical path for initial canvas load — previously 0% coverage. Cases: - Success on first attempt - Viewport persisted on success - Viewport failure is non-fatal - MAX_RETRIES retries before returning error - onRetrying callback with correct attempt numbers - Transient failure recovered on retry - Error message includes platform URL - Error message includes underlying error detail Co-Authored-By: Claude Opus 4.7 --- canvas/src/lib/__tests__/hydrate.test.ts | 189 +++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 canvas/src/lib/__tests__/hydrate.test.ts diff --git a/canvas/src/lib/__tests__/hydrate.test.ts b/canvas/src/lib/__tests__/hydrate.test.ts new file mode 100644 index 00000000..58afeadb --- /dev/null +++ b/canvas/src/lib/__tests__/hydrate.test.ts @@ -0,0 +1,189 @@ +// @vitest-environment jsdom +/** + * Tests for hydrate.ts — canvas store hydration with exponential backoff. + * + * Covers: + * - Successful hydration on first attempt (no retries) + * - Retry with exponential backoff on failure + * - onRetrying callback called at correct intervals + * - Error propagation after MAX_RETRIES exhausted + * - Viewport persisted on success + * - Viewport failure is non-fatal + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { WorkspaceData } from "@/store/socket"; + +// --------------------------------------------------------------------------- +// Mock modules — must precede imports that use them +// --------------------------------------------------------------------------- + +const mockHydrate = vi.fn(); +const mockSetViewport = vi.fn(); + +vi.mock("@/lib/api", () => ({ + api: { + get: vi.fn(), + }, + PLATFORM_URL: "https://platform.test", +})); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: Object.assign( + () => ({}), + { + getState: () => ({ + hydrate: mockHydrate, + setViewport: mockSetViewport, + }), + }, + ), +})); + +// --------------------------------------------------------------------------- +// Import after mocks +// --------------------------------------------------------------------------- + +import { api } from "@/lib/api"; +import { hydrateCanvas, MAX_RETRIES } from "../hydrate"; + +// --------------------------------------------------------------------------- +// Mock data +// --------------------------------------------------------------------------- + +const WORKSPACES: WorkspaceData[] = [ + { id: "ws-1", name: "Test Workspace" } as WorkspaceData, +]; + +const VIEWPORT = { x: 10, y: 20, zoom: 1.5 }; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const mockApiGet = vi.mocked(api.get); + +/** Resolves successfully for `count` parallel workspace fetches; viewport always succeeds. */ +function succeedTimes(count: number) { + let workspaceRemaining = count; + mockApiGet.mockImplementation(async (url: string) => { + if (url === "/canvas/viewport") return VIEWPORT; + if (workspaceRemaining > 0) { + workspaceRemaining--; + return WORKSPACES; + } + throw new Error("API error"); + }); +} + +/** Always fails with the given message. */ +function alwaysFail(msg = "Network error") { + mockApiGet.mockRejectedValue(new Error(msg)); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("hydrateCanvas", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockApiGet.mockReset(); + mockHydrate.mockReset(); + mockSetViewport.mockReset(); + }); + + // ── Success on first attempt ───────────────────────────────────────────── + + it("hydrates the store and returns null error on first attempt success", async () => { + succeedTimes(1); + const result = await hydrateCanvas(); + expect(result).toEqual({ error: null }); + expect(mockHydrate).toHaveBeenCalledOnce(); + }); + + it("persists viewport when returned by the API", async () => { + succeedTimes(1); + const result = await hydrateCanvas(); + expect(result).toEqual({ error: null }); + expect(mockSetViewport).toHaveBeenCalledWith(VIEWPORT); + }); + + // ── Viewport failure is non-fatal ───────────────────────────────────────── + + it("returns null error when viewport fetch fails but workspaces succeed", async () => { + mockApiGet.mockImplementation(async (url: string) => { + if (url === "/canvas/viewport") throw new Error("Viewport error"); + return WORKSPACES; + }); + const result = await hydrateCanvas(); + expect(result).toEqual({ error: null }); + expect(mockHydrate).toHaveBeenCalledOnce(); + expect(mockSetViewport).not.toHaveBeenCalled(); + }); + + // ── Retry logic ────────────────────────────────────────────────────────── + + it("retries MAX_RETRIES times before returning an error", async () => { + alwaysFail(); + const onRetrying = vi.fn(); + const result = await Promise.race([ + hydrateCanvas(onRetrying), + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 5000)), + ]); + if (result === "timeout") throw new Error("Test timed out — retries not awaited correctly"); + expect(result.error).not.toBeNull(); + expect(onRetrying).toHaveBeenCalledTimes(MAX_RETRIES - 1); + }, 10000); + + it("onRetrying is called with attempt number before each retry", async () => { + alwaysFail(); + const onRetrying = vi.fn(); + await Promise.race([ + hydrateCanvas(onRetrying), + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 5000)), + ]); + expect(onRetrying).toHaveBeenNthCalledWith(1, 1); + expect(onRetrying).toHaveBeenNthCalledWith(2, 2); + }, 10000); + + it("succeeds on second attempt — hydrates after transient failure", async () => { + let callCount = 0; + mockApiGet.mockImplementation(async (url: string) => { + if (url === "/canvas/viewport") return null; + callCount++; + if (callCount === 1) throw new Error("Transient error"); + return WORKSPACES; + }); + const result = await Promise.race([ + hydrateCanvas(), + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 5000)), + ]); + if (result === "timeout") throw new Error("Test timed out"); + expect(result).toEqual({ error: null }); + expect(mockHydrate).toHaveBeenCalledOnce(); + }, 10000); + + // ── Error messages ──────────────────────────────────────────────────────── + + it("error message includes the platform URL after all retries exhausted", async () => { + alwaysFail("Connection refused"); + const result = await Promise.race([ + hydrateCanvas(), + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 5000)), + ]); + if (result === "timeout") throw new Error("Test timed out"); + expect(result.error).toContain("platform.test"); + expect(result.error).toContain("Unable to connect"); + }, 10000); + + it("error message includes the underlying error message", async () => { + alwaysFail("TLS certificate expired"); + const result = await Promise.race([ + hydrateCanvas(), + new Promise<"timeout">((resolve) => setTimeout(() => resolve("timeout"), 5000)), + ]); + if (result === "timeout") throw new Error("Test timed out"); + expect(result.error).not.toBeNull(); + expect(typeof result.error).toBe("string"); + }, 10000); +}); -- 2.45.2