diff --git a/.github/workflows/canary-verify.yml b/.github/workflows/canary-verify.yml index d11890c63..daa6a2060 100644 --- a/.github/workflows/canary-verify.yml +++ b/.github/workflows/canary-verify.yml @@ -48,9 +48,44 @@ jobs: run: echo "sha=${GITHUB_SHA::7}" >> "$GITHUB_OUTPUT" - name: Wait for canary tenants to pick up :staging- - # Tenant auto-updater runs every 5 min. Sleep 6 min to give every - # canary time to pull + restart. Cheaper than polling. - run: sleep 360 + # Poll canary health endpoints every 30s for up to 7 min instead + # of a fixed 6-min sleep. Exits as soon as ALL canaries report the + # new SHA, freeing the self-hosted runner slot sooner (~2-3 min + # typical vs 6 min fixed). Falls back to proceeding after 7 min + # even if not all canaries responded — the smoke suite will catch + # any that didn't update. + env: + CANARY_TENANT_URLS: ${{ secrets.CANARY_TENANT_URLS }} + EXPECTED_SHA: ${{ steps.compute.outputs.sha }} + run: | + if [ -z "$CANARY_TENANT_URLS" ]; then + echo "No canary URLs configured — falling back to 60s wait" + sleep 60 + exit 0 + fi + IFS=',' read -ra URLS <<< "$CANARY_TENANT_URLS" + MAX_WAIT=420 # 7 minutes + INTERVAL=30 + ELAPSED=0 + while [ $ELAPSED -lt $MAX_WAIT ]; do + ALL_READY=true + for url in "${URLS[@]}"; do + HEALTH=$(curl -s --max-time 5 "${url}/health" 2>/dev/null || echo "{}") + SHA=$(echo "$HEALTH" | grep -o "\"sha\":\"[^\"]*\"" | head -1 | cut -d'"' -f4) + if [ "$SHA" != "$EXPECTED_SHA" ]; then + ALL_READY=false + break + fi + done + if $ALL_READY; then + echo "All canaries running staging-${EXPECTED_SHA} after ${ELAPSED}s" + exit 0 + fi + echo "Waiting for canaries... (${ELAPSED}s / ${MAX_WAIT}s)" + sleep $INTERVAL + ELAPSED=$((ELAPSED + INTERVAL)) + done + echo "Timeout after ${MAX_WAIT}s — proceeding anyway (smoke suite will validate)" - name: Run canary smoke suite env: diff --git a/canvas/src/app/__tests__/orgs-page.test.tsx b/canvas/src/app/__tests__/orgs-page.test.tsx new file mode 100644 index 000000000..430aa8f06 --- /dev/null +++ b/canvas/src/app/__tests__/orgs-page.test.tsx @@ -0,0 +1,368 @@ +// @vitest-environment jsdom +/** + * Tests for /orgs — the post-signup landing page (PR #992 feat/canvas-orgs-landing + * plus #994 feat/canvas-post-checkout-redirect). + * + * The page is the only route the control-plane Callback hands a new session to, + * so bugs here strand new users. Covers: + * - Signed-out → redirectToLogin + * - Failed /cp/orgs → error state + retry button + * - Empty org list → EmptyState w/ CreateOrgForm + * - `running` org → Open button links to `{slug}.{appDomain}` + * - `awaiting_payment` org → "Complete payment" → /pricing?org= + * - `failed` org → mailto support link + * - `?checkout=success` param → CheckoutBanner renders + URL is scrubbed + * - Polling: provisioning orgs schedule a 5s refresh (fake timers) + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, waitFor, cleanup } from "@testing-library/react"; + +// ── Hoisted mocks ──────────────────────────────────────────────────────────── +// vi.mock factories are hoisted above imports; any captured references must +// come from vi.hoisted() or the factory would see "undefined before init". + +const { mockFetchSession, mockRedirectToLogin } = vi.hoisted(() => ({ + mockFetchSession: vi.fn(), + mockRedirectToLogin: vi.fn(), +})); + +vi.mock("@/lib/auth", () => ({ + fetchSession: mockFetchSession, + redirectToLogin: mockRedirectToLogin, +})); + +// api module provides PLATFORM_URL; page imports it as a constant +vi.mock("@/lib/api", () => ({ + PLATFORM_URL: "https://cp.test", +})); + +const mockFetch = vi.fn(); +globalThis.fetch = mockFetch as unknown as typeof fetch; + +// Import page AFTER mocks are declared +import OrgsPage from "../../app/orgs/page"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function okJson(body: unknown, status = 200) { + return { + ok: true, + status, + json: () => Promise.resolve(body), + text: () => Promise.resolve(JSON.stringify(body)), + } as unknown as Response; +} + +function notOk(status: number, text = "boom") { + return { + ok: false, + status, + json: () => Promise.reject(new Error("no json")), + text: () => Promise.resolve(text), + } as unknown as Response; +} + +function setLocation(href: string) { + // jsdom allows window.location replacement via pushState rather than assign; + // the component only reads `search` + `hostname` + `pathname`. + const url = new URL(href); + window.history.pushState({}, "", url.pathname + url.search); + Object.defineProperty(window, "location", { + configurable: true, + value: { + ...window.location, + hostname: url.hostname, + search: url.search, + pathname: url.pathname, + }, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + setLocation("https://moleculesai.app/orgs"); +}); + +afterEach(() => { + cleanup(); +}); + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("/orgs — auth guard", () => { + it("redirects to login when session is null", async () => { + mockFetchSession.mockResolvedValueOnce(null); + render(); + await waitFor(() => expect(mockRedirectToLogin).toHaveBeenCalled()); + // Must not attempt to fetch /cp/orgs before auth is established + expect(mockFetch).not.toHaveBeenCalledWith( + expect.stringContaining("/cp/orgs"), + expect.anything() + ); + }); +}); + +describe("/orgs — error state", () => { + it("shows error + Retry button when /cp/orgs fails", async () => { + mockFetchSession.mockResolvedValueOnce({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce(notOk(500, "db down")); + render(); + await waitFor(() => expect(screen.getByText(/Error:/)).toBeTruthy()); + expect(screen.getByRole("button", { name: /retry/i })).toBeTruthy(); + }); +}); + +describe("/orgs — empty list", () => { + it("renders EmptyState with CreateOrgForm when user has zero orgs", async () => { + mockFetchSession.mockResolvedValueOnce({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce(okJson({ orgs: [] })); + render(); + await waitFor(() => expect(screen.getByText(/don't have any organizations/i)).toBeTruthy()); + expect(screen.getByRole("button", { name: /create organization/i })).toBeTruthy(); + }); +}); + +describe("/orgs — CTAs by status", () => { + const session = { userId: "u-1" }; + + it("running → Open link targets {slug}.moleculesai.app", async () => { + mockFetchSession.mockResolvedValueOnce(session); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-1", + slug: "acme", + name: "Acme", + plan: "pro", + status: "running", + created_at: "", + updated_at: "", + }, + ], + }) + ); + render(); + const link = (await screen.findByRole("link", { name: /open/i })) as HTMLAnchorElement; + expect(link.href).toBe("https://acme.moleculesai.app/"); + }); + + it("awaiting_payment → Complete payment link to /pricing?org=", async () => { + mockFetchSession.mockResolvedValueOnce(session); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-2", + slug: "beta-co", + name: "Beta", + plan: "", + status: "awaiting_payment", + created_at: "", + updated_at: "", + }, + ], + }) + ); + render(); + const link = (await screen.findByRole("link", { + name: /complete payment/i, + })) as HTMLAnchorElement; + expect(link.getAttribute("href")).toBe("/pricing?org=beta-co"); + }); + + it("failed → mailto support link", async () => { + mockFetchSession.mockResolvedValueOnce(session); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-3", + slug: "boom", + name: "Boom", + plan: "", + status: "failed", + created_at: "", + updated_at: "", + }, + ], + }) + ); + render(); + const link = (await screen.findByRole("link", { + name: /contact support/i, + })) as HTMLAnchorElement; + expect(link.getAttribute("href")).toBe("mailto:support@moleculesai.app"); + }); +}); + +describe("/orgs — post-checkout banner", () => { + it("renders CheckoutBanner when ?checkout=success and scrubs the URL", async () => { + setLocation("https://moleculesai.app/orgs?checkout=success"); + const replaceState = vi.spyOn(window.history, "replaceState"); + mockFetchSession.mockResolvedValueOnce({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-1", + slug: "acme", + name: "Acme", + plan: "pro", + status: "running", + created_at: "", + updated_at: "", + }, + ], + }) + ); + render(); + expect(await screen.findByText(/Payment confirmed/i)).toBeTruthy(); + // URL must be rewritten to drop the ?checkout flag so reload doesn't re-show the banner + expect(replaceState).toHaveBeenCalled(); + const callArgs = replaceState.mock.calls[0]; + expect(callArgs[2]).toBe("/orgs"); + }); + + it("does NOT render CheckoutBanner without ?checkout=success", async () => { + mockFetchSession.mockResolvedValueOnce({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce(okJson({ orgs: [] })); + render(); + await waitFor(() => + expect(screen.getByText(/don't have any organizations/i)).toBeTruthy() + ); + expect(screen.queryByText(/Payment confirmed/i)).toBeNull(); + }); +}); + +describe("/orgs — fetch includes credentials + timeout signal", () => { + it("/cp/orgs fetch is called with credentials:include and an AbortSignal", async () => { + mockFetchSession.mockResolvedValueOnce({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce(okJson({ orgs: [] })); + render(); + await waitFor(() => expect(mockFetch).toHaveBeenCalled()); + const callArgs = mockFetch.mock.calls.find((c) => + String(c[0]).includes("/cp/orgs") + ); + expect(callArgs).toBeDefined(); + expect(callArgs![1]).toMatchObject({ credentials: "include" }); + expect(callArgs![1].signal).toBeInstanceOf(AbortSignal); + }); +}); + +// ── Polling ────────────────────────────────────────────────────────────────── +// page.tsx line 83-88: if any org is `provisioning` OR `awaiting_payment`, +// schedule a 5s refresh so the user sees the state flip live after Stripe +// Checkout returns. Cleanup must clear the timer on unmount; otherwise a +// fast-nav-away leaves the interval firing against the CP indefinitely. + +describe("/orgs — polling of in-flight orgs", () => { + it("schedules a 5s refetch when at least one org is provisioning", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + mockFetchSession.mockResolvedValue({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-1", + slug: "acme", + name: "Acme", + plan: "pro", + status: "provisioning", + created_at: "", + updated_at: "", + }, + ], + }) + ); + // Second fetch (the poll refresh) returns a running org so we can + // observe the state flip — and to let the test stop re-scheduling. + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-1", + slug: "acme", + name: "Acme", + plan: "pro", + status: "running", + created_at: "", + updated_at: "", + }, + ], + }) + ); + + render(); + // First fetch resolves + await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(1)); + // Advance past the 5s scheduled refresh + await vi.advanceTimersByTimeAsync(5_100); + // Second fetch is the poll refresh + await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2)); + } finally { + vi.useRealTimers(); + } + }); + + it("does NOT schedule a refetch when all orgs are running", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + mockFetchSession.mockResolvedValue({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-1", + slug: "acme", + name: "Acme", + plan: "pro", + status: "running", + created_at: "", + updated_at: "", + }, + ], + }) + ); + render(); + await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(1)); + // Advance well past the 5s poll window — no second fetch must fire + await vi.advanceTimersByTimeAsync(10_000); + expect(mockFetch).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + + it("clears the poll timer on unmount — no fetch after unmount", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + mockFetchSession.mockResolvedValue({ userId: "u-1" }); + mockFetch.mockResolvedValueOnce( + okJson({ + orgs: [ + { + id: "o-1", + slug: "acme", + name: "Acme", + plan: "pro", + status: "awaiting_payment", + created_at: "", + updated_at: "", + }, + ], + }) + ); + const { unmount } = render(); + await vi.waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(1)); + // Tear down BEFORE the 5s timer fires + unmount(); + await vi.advanceTimersByTimeAsync(10_000); + // Fetch count must stay at 1 — the cleanup cleared the timer + expect(mockFetch).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/canvas/src/components/BatchActionBar.tsx b/canvas/src/components/BatchActionBar.tsx new file mode 100644 index 000000000..5421fbe26 --- /dev/null +++ b/canvas/src/components/BatchActionBar.tsx @@ -0,0 +1,124 @@ +"use client"; + +import { useState } from "react"; +import { createPortal } from "react-dom"; +import { useCanvasStore } from "@/store/canvas"; +import { ConfirmDialog } from "./ConfirmDialog"; +import { showToast } from "./Toaster"; + +type BatchAction = "restart" | "pause" | "delete" | null; + +export function BatchActionBar() { + const selectedNodeIds = useCanvasStore((s) => s.selectedNodeIds); + const clearSelection = useCanvasStore((s) => s.clearSelection); + const batchRestart = useCanvasStore((s) => s.batchRestart); + const batchPause = useCanvasStore((s) => s.batchPause); + const batchDelete = useCanvasStore((s) => s.batchDelete); + + const [pending, setPending] = useState(null); + const [busy, setBusy] = useState(false); + + const count = selectedNodeIds.size; + if (count < 2) return null; + + const confirmMessages: Record, string> = { + restart: `Restart ${count} workspace${count !== 1 ? "s" : ""}? Each will briefly go offline while it restarts.`, + pause: `Pause ${count} workspace${count !== 1 ? "s" : ""}? Their containers will be stopped.`, + delete: `Permanently delete ${count} workspace${count !== 1 ? "s" : ""}? This cannot be undone.`, + }; + + const confirmLabels: Record, string> = { + restart: "Restart All", + pause: "Pause All", + delete: "Delete All", + }; + + async function execute() { + if (!pending) return; + setBusy(true); + try { + if (pending === "restart") await batchRestart(); + if (pending === "pause") await batchPause(); + if (pending === "delete") await batchDelete(); + showToast(`${pending.charAt(0).toUpperCase() + pending.slice(1)} applied to ${count} workspace${count !== 1 ? "s" : ""}`, "success"); + clearSelection(); + } catch { + showToast(`Batch ${pending} failed`, "error"); + } finally { + setBusy(false); + setPending(null); + } + } + + const bar = ( +
+ {/* Selection count badge */} + + {count} selected + + + )} @@ -429,7 +442,7 @@ function TeamMemberChip({ {/* Status + active tasks row */}
{data.status !== "online" ? ( - 0 && (
- + {data.activeTasks}
@@ -453,7 +466,7 @@ function TeamMemberChip({
- {data.currentTask} + {data.currentTask}
)} @@ -461,7 +474,7 @@ function TeamMemberChip({ {/* Recursive sub-children rendered inside this card */} {hasSubChildren && depth < MAX_NESTING_DEPTH && (
-
Team
+
Team
= 2 ? "grid grid-cols-2 gap-1" : "space-y-1"}> {subChildren.map((sub) => ( diff --git a/canvas/src/components/__tests__/BatchActionBar.test.tsx b/canvas/src/components/__tests__/BatchActionBar.test.tsx new file mode 100644 index 000000000..55eb786d4 --- /dev/null +++ b/canvas/src/components/__tests__/BatchActionBar.test.tsx @@ -0,0 +1,127 @@ +// @vitest-environment jsdom +/** + * BatchActionBar tests — Phase 20.3 + * + * Covers: + * - Not rendered when fewer than 2 nodes selected + * - Renders with correct count badge when 2+ selected + * - Restart/Pause/Delete buttons exist with correct labels + * - Clear selection button exists + * - ConfirmDialog appears on destructive action click + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; + +afterEach(() => { + cleanup(); +}); + +// ── Mocks ──────────────────────────────────────────────────────────────────── + +vi.mock("@/components/Toaster", () => ({ + showToast: vi.fn(), +})); + +const mockClearSelection = vi.fn(); +const mockBatchRestart = vi.fn(() => Promise.resolve()); +const mockBatchPause = vi.fn(() => Promise.resolve()); +const mockBatchDelete = vi.fn(() => Promise.resolve()); + +let mockSelectedNodeIds = new Set(); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: vi.fn((selector: (s: Record) => unknown) => + selector({ + selectedNodeIds: mockSelectedNodeIds, + clearSelection: mockClearSelection, + batchRestart: mockBatchRestart, + batchPause: mockBatchPause, + batchDelete: mockBatchDelete, + }) + ), +})); + +// Mock ConfirmDialog to just render buttons for testing +vi.mock("@/components/ConfirmDialog", () => ({ + ConfirmDialog: ({ + open, + title, + onConfirm, + onCancel, + }: { + open: boolean; + title: string; + confirmLabel: string; + message: string; + confirmVariant: string; + onConfirm: () => void; + onCancel: () => void; + }) => + open ? ( +
+ {title} + + +
+ ) : null, +})); + +// Import after mocks +import { BatchActionBar } from "../BatchActionBar"; + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("BatchActionBar", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockSelectedNodeIds = new Set(); + }); + + it("does not render when fewer than 2 nodes selected", () => { + mockSelectedNodeIds = new Set(["ws-1"]); + const { container } = render(); + expect(container.innerHTML).toBe(""); + }); + + it("renders count badge when 2+ nodes selected", () => { + mockSelectedNodeIds = new Set(["ws-1", "ws-2", "ws-3"]); + render(); + expect(screen.getByText("3 selected")).toBeTruthy(); + }); + + it("renders Restart All, Pause All, Delete All buttons", () => { + mockSelectedNodeIds = new Set(["ws-1", "ws-2"]); + render(); + expect(screen.getByText("Restart All")).toBeTruthy(); + expect(screen.getByText("Pause All")).toBeTruthy(); + expect(screen.getByText("Delete All")).toBeTruthy(); + }); + + it("renders clear selection button with aria-label", () => { + mockSelectedNodeIds = new Set(["ws-1", "ws-2"]); + render(); + const clearBtn = screen.getByRole("button", { name: "Clear selection" }); + expect(clearBtn).toBeTruthy(); + }); + + it("clicking clear selection calls clearSelection", () => { + mockSelectedNodeIds = new Set(["ws-1", "ws-2"]); + render(); + fireEvent.click(screen.getByRole("button", { name: "Clear selection" })); + expect(mockClearSelection).toHaveBeenCalled(); + }); + + it("clicking Delete All opens ConfirmDialog", () => { + mockSelectedNodeIds = new Set(["ws-1", "ws-2"]); + render(); + fireEvent.click(screen.getByText("Delete All")); + expect(screen.getByTestId("confirm-dialog")).toBeTruthy(); + }); + + it("has role=toolbar with aria-label", () => { + mockSelectedNodeIds = new Set(["ws-1", "ws-2"]); + render(); + const toolbar = screen.getByRole("toolbar"); + expect(toolbar.getAttribute("aria-label")).toBe("Batch workspace actions"); + }); +}); diff --git a/canvas/src/components/__tests__/Canvas.a11y.test.tsx b/canvas/src/components/__tests__/Canvas.a11y.test.tsx index 9e50f8fd5..a8231eb3e 100644 --- a/canvas/src/components/__tests__/Canvas.a11y.test.tsx +++ b/canvas/src/components/__tests__/Canvas.a11y.test.tsx @@ -69,6 +69,9 @@ const mockStoreState = { showA2AEdges: false, setShowA2AEdges: vi.fn(), setPanelTab: vi.fn(), + selectedNodeIds: new Set(), + clearSelection: vi.fn(), + toggleNodeSelection: vi.fn(), }; vi.mock("@/store/canvas", () => ({ @@ -109,6 +112,7 @@ vi.mock("../ProvisioningTimeout", () => ({
), })); +vi.mock("../BatchActionBar", () => ({ BatchActionBar: () => null })); // ── Import the component under test AFTER mocks ─────────────────────────────── import { Canvas } from "../Canvas"; diff --git a/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx b/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx index c529a0e11..da38d8960 100644 --- a/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx +++ b/canvas/src/components/__tests__/Canvas.pan-to-node.test.tsx @@ -79,6 +79,9 @@ const mockStoreState = { showA2AEdges: false, setShowA2AEdges: vi.fn(), setPanelTab: vi.fn(), + selectedNodeIds: new Set(), + clearSelection: vi.fn(), + toggleNodeSelection: vi.fn(), }; vi.mock("@/store/canvas", () => ({ @@ -113,6 +116,8 @@ vi.mock("../settings", () => ({ })); vi.mock("../Toaster", () => ({ Toaster: () => null })); vi.mock("../WorkspaceNode", () => ({ WorkspaceNode: () => null })); +vi.mock("../BatchActionBar", () => ({ BatchActionBar: () => null })); +vi.mock("../ProvisioningTimeout", () => ({ ProvisioningTimeout: () => null })); import { Canvas } from "../Canvas"; diff --git a/canvas/src/components/__tests__/WorkspaceNode.a11y.test.tsx b/canvas/src/components/__tests__/WorkspaceNode.a11y.test.tsx index ec680bf2a..48c5cb35c 100644 --- a/canvas/src/components/__tests__/WorkspaceNode.a11y.test.tsx +++ b/canvas/src/components/__tests__/WorkspaceNode.a11y.test.tsx @@ -125,6 +125,8 @@ const mockStoreState = { nestNode: mockNestNode, restartWorkspace: vi.fn(() => Promise.resolve()), setPanelTab: vi.fn(), + selectedNodeIds: new Set(), + toggleNodeSelection: vi.fn(), }; vi.mock("@/store/canvas", () => ({ diff --git a/canvas/src/components/__tests__/WorkspaceNode.eject.test.tsx b/canvas/src/components/__tests__/WorkspaceNode.eject.test.tsx index 058b7cfc3..691bc2cde 100644 --- a/canvas/src/components/__tests__/WorkspaceNode.eject.test.tsx +++ b/canvas/src/components/__tests__/WorkspaceNode.eject.test.tsx @@ -97,6 +97,8 @@ const mockStoreState = { isDescendant: vi.fn(() => false), restartWorkspace: vi.fn(), setPanelTab: vi.fn(), + selectedNodeIds: new Set(), + toggleNodeSelection: vi.fn(), }; vi.mock("@/store/canvas", () => ({ diff --git a/canvas/src/lib/__tests__/api.test.ts b/canvas/src/lib/__tests__/api.test.ts index 01cb02cbf..09eb0eff1 100644 --- a/canvas/src/lib/__tests__/api.test.ts +++ b/canvas/src/lib/__tests__/api.test.ts @@ -309,3 +309,74 @@ describe("api – PLATFORM_URL default", () => { expect(url).toContain("localhost:8080"); }); }); + +// --------------------------------------------------------------------------- +// 15s timeout via AbortSignal.timeout (regression for #982 / +// fix/canvas-api-fetch-timeout). The signal prevents a hung backend from +// leaving the UI spinning forever. These assertions pin the behaviour so +// a future edit can't drop the signal without breaking tests. +// --------------------------------------------------------------------------- + +describe("api – request timeout signal", () => { + it("GET passes an AbortSignal to fetch", async () => { + mockSuccess({}); + await api.get("/workspaces"); + const [, options] = mockFetch.mock.calls[0]; + expect(options.signal).toBeDefined(); + expect(options.signal).toBeInstanceOf(AbortSignal); + }); + + it("POST passes an AbortSignal to fetch", async () => { + mockSuccess({}); + await api.post("/workspaces", { name: "x" }); + const [, options] = mockFetch.mock.calls[0]; + expect(options.signal).toBeDefined(); + expect(options.signal).toBeInstanceOf(AbortSignal); + }); + + it("PATCH passes an AbortSignal to fetch", async () => { + mockSuccess({}); + await api.patch("/workspaces/ws-1", { x: 0 }); + const [, options] = mockFetch.mock.calls[0]; + expect(options.signal).toBeInstanceOf(AbortSignal); + }); + + it("PUT passes an AbortSignal to fetch", async () => { + mockSuccess({}); + await api.put("/canvas/viewport", { x: 0, y: 0, zoom: 1 }); + const [, options] = mockFetch.mock.calls[0]; + expect(options.signal).toBeInstanceOf(AbortSignal); + }); + + it("DELETE passes an AbortSignal to fetch", async () => { + mockSuccess({}); + await api.del("/workspaces/ws-1"); + const [, options] = mockFetch.mock.calls[0]; + expect(options.signal).toBeInstanceOf(AbortSignal); + }); + + it("AbortError from timeout is propagated to the caller", async () => { + // Simulate the browser firing a TimeoutError when AbortSignal.timeout + // expires — the fetch promise rejects with a DOMException (name=TimeoutError). + const abortErr = + typeof DOMException === "function" + ? new DOMException("signal timed out", "TimeoutError") + : Object.assign(new Error("signal timed out"), { name: "TimeoutError" }); + mockFetch.mockRejectedValueOnce(abortErr); + + await expect(api.get("/slow")).rejects.toMatchObject({ name: "TimeoutError" }); + }); + + it("each request installs its own signal (not a shared module-level controller)", async () => { + mockSuccess({}); + mockSuccess({}); + await api.get("/a"); + await api.get("/b"); + const sigA = mockFetch.mock.calls[0][1].signal; + const sigB = mockFetch.mock.calls[1][1].signal; + // AbortSignal.timeout() returns a fresh signal per call — they must + // not be the same reference, otherwise one slow request could cancel + // a subsequent fast request. + expect(sigA).not.toBe(sigB); + }); +}); diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index 16191423a..c19e10118 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -71,6 +71,13 @@ interface CanvasState { viewport: { x: number; y: number; zoom: number }; setViewport: (v: { x: number; y: number; zoom: number }) => void; saveViewport: (x: number, y: number, zoom: number) => void; + // ── Batch selection (Phase 20.3) ───────────────────────────────────────── + selectedNodeIds: Set; + toggleNodeSelection: (id: string) => void; + clearSelection: () => void; + batchRestart: () => Promise; + batchPause: () => Promise; + batchDelete: () => Promise; /** Agent-pushed messages keyed by workspace ID. ChatTab consumes and clears these. */ agentMessages: Record>; consumeAgentMessages: (workspaceId: string) => Array<{ id: string; content: string; timestamp: string }>; @@ -96,6 +103,38 @@ export const useCanvasStore = create((set, get) => ({ panelTab: "chat", dragOverNodeId: null, contextMenu: null, + // Batch selection + selectedNodeIds: new Set(), + toggleNodeSelection: (id) => { + const prev = get().selectedNodeIds; + const next = new Set(prev); + if (next.has(id)) { + next.delete(id); + } else { + next.add(id); + } + set({ selectedNodeIds: next }); + }, + clearSelection: () => set({ selectedNodeIds: new Set() }), + batchRestart: async () => { + const ids = Array.from(get().selectedNodeIds); + await Promise.allSettled(ids.map((id) => api.post(`/workspaces/${id}/restart`))); + for (const id of ids) { + get().updateNodeData(id, { needsRestart: false }); + } + }, + batchPause: async () => { + const ids = Array.from(get().selectedNodeIds); + await Promise.allSettled(ids.map((id) => api.post(`/workspaces/${id}/pause`))); + }, + batchDelete: async () => { + const ids = Array.from(get().selectedNodeIds); + await Promise.allSettled(ids.map((id) => api.del(`/workspaces/${id}`))); + for (const id of ids) { + get().removeNode(id); + } + set({ selectedNodeIds: new Set() }); + }, wsStatus: "connecting", setWsStatus: (status) => set({ wsStatus: status }), hydrationError: null, diff --git a/workspace-server/internal/bundle/exporter.go b/workspace-server/internal/bundle/exporter.go index 770a16913..412ecc49c 100644 --- a/workspace-server/internal/bundle/exporter.go +++ b/workspace-server/internal/bundle/exporter.go @@ -92,6 +92,9 @@ func Export(ctx context.Context, workspaceID, configsDir string, dockerCli *clie } } } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("export sub-workspaces: %w", err) + } } return b, nil diff --git a/workspace-server/internal/handlers/mcp.go b/workspace-server/internal/handlers/mcp.go index c10976eda..abaa10c23 100644 --- a/workspace-server/internal/handlers/mcp.go +++ b/workspace-server/internal/handlers/mcp.go @@ -723,8 +723,12 @@ func (h *MCPHandler) toolCommitMemory(ctx context.Context, workspaceID string, a } memoryID := uuid.New().String() - // TODO(#838): run _redactSecrets(content) before insert — plain-text API keys - // from tool responses must not land in the memories table. + // SAFE-T1201 (#838): scrub known credential patterns before persistence so + // plain-text API keys pulled in via tool responses can't land in the + // memories table (and leak into shared TEAM scope). Reuses redactSecrets + // already shipped for the HTTP path in PR #881 — this was the MCP-bridge + // sibling the original fix missed. Runs on every write regardless of scope. + content, _ = redactSecrets(workspaceID, content) _, err := h.database.ExecContext(ctx, ` INSERT INTO agent_memories (id, workspace_id, content, scope, namespace) VALUES ($1, $2, $3, $4, $5) diff --git a/workspace-server/internal/handlers/mcp_test.go b/workspace-server/internal/handlers/mcp_test.go index 9f3800486..c91bf98f5 100644 --- a/workspace-server/internal/handlers/mcp_test.go +++ b/workspace-server/internal/handlers/mcp_test.go @@ -433,6 +433,101 @@ func TestMCPHandler_CommitMemory_GlobalScope_Blocked(t *testing.T) { } } +// TestMCPHandler_CommitMemory_SecretInContent_IsRedactedBeforeInsert verifies +// the SAFE-T1201 (#838) fix on the MCP bridge path. PR #881 closed the HTTP +// handler but missed this one — an agent tool-call carrying plain-text +// credentials must have them scrubbed before the INSERT reaches the DB. +// +// The test asserts via the sqlmock `WithArgs` matcher that the content column +// binds the REDACTED form, not the raw input. sqlmock verifies the exact arg +// values, so a regression (removing the redactSecrets call) would fail with +// "argument mismatch" rather than silently persisting the secret. +func TestMCPHandler_CommitMemory_SecretInContent_IsRedactedBeforeInsert(t *testing.T) { + h, mock := newMCPHandler(t) + + // Content with three distinct secret patterns covered by redactSecrets: + // - env-var assignment (ANTHROPIC_API_KEY=) + // - Bearer token + // - sk-… prefixed key + rawContent := "key=ANTHROPIC_API_KEY=sk-ant-xxxxxxxxxxxxxxxx auth=Bearer ghp_yyyyyyyyyyyyy note=sk-proj-zzzzzzzzzzzzzzzzzzzz" + + // Derive what redactSecrets will produce so the sqlmock arg match is + // exact. This keeps the test brittle-on-purpose: if redactSecrets's + // output shape changes, this test must be re-derived, which surfaces + // the change during review. + expected, changed := redactSecrets("ws-1", rawContent) + if !changed { + t.Fatalf("precondition failed — redactSecrets must change the test content; got unchanged %q", expected) + } + if bytes.Contains([]byte(expected), []byte("sk-ant-xxxxxxxxxxxxxxxx")) { + t.Fatalf("precondition failed — redacted content still contains raw secret: %s", expected) + } + + mock.ExpectExec("INSERT INTO agent_memories"). + WithArgs(sqlmock.AnyArg(), "ws-1", expected, "LOCAL", "ws-1"). + WillReturnResult(sqlmock.NewResult(1, 1)) + + w := mcpPost(t, h, "ws-1", map[string]interface{}{ + "jsonrpc": "2.0", + "id": 99, + "method": "tools/call", + "params": map[string]interface{}{ + "name": "commit_memory", + "arguments": map[string]interface{}{ + "content": rawContent, + "scope": "LOCAL", + }, + }, + }) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp mcpResponse + json.Unmarshal(w.Body.Bytes(), &resp) + if resp.Error != nil { + t.Fatalf("unexpected JSON-RPC error: %+v", resp.Error) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock mismatch — content was NOT redacted before insert: %v", err) + } +} + +// TestMCPHandler_CommitMemory_CleanContent_PassesThrough confirms that the +// redactor is a no-op on content with no credentials — a regression where +// redactSecrets corrupted benign content would be a user-visible bug. +func TestMCPHandler_CommitMemory_CleanContent_PassesThrough(t *testing.T) { + h, mock := newMCPHandler(t) + + cleanContent := "the quick brown fox jumps over the lazy dog — no secrets here" + + // Bind the exact string — no wildcards — so that any transformation + // (whitespace, case, truncation) would fail the arg match. + mock.ExpectExec("INSERT INTO agent_memories"). + WithArgs(sqlmock.AnyArg(), "ws-1", cleanContent, "TEAM", "ws-1"). + WillReturnResult(sqlmock.NewResult(1, 1)) + + w := mcpPost(t, h, "ws-1", map[string]interface{}{ + "jsonrpc": "2.0", + "id": 100, + "method": "tools/call", + "params": map[string]interface{}{ + "name": "commit_memory", + "arguments": map[string]interface{}{ + "content": cleanContent, + "scope": "TEAM", + }, + }, + }) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("clean content should pass through unchanged: %v", err) + } +} + // ───────────────────────────────────────────────────────────────────────────── // tools/call — recall_memory // ───────────────────────────────────────────────────────────────────────────── diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index 6167098be..efc5e6abe 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -168,17 +168,31 @@ func (p *CPProvisioner) Stop(ctx context.Context, workspaceID string) error { } // IsRunning checks workspace EC2 instance state via the control plane. +// +// Contract: +// - transport error → (false, error) +// - non-2xx HTTP response → (false, error). Previously swallowed; +// a CP 500 would return (false, nil) and the sweeper couldn't +// distinguish "workspace stopped" from "CP broken". +// - 2xx with state!="running" → (false, nil) +// - 2xx with state=="running" → (true, nil) func (p *CPProvisioner) IsRunning(ctx context.Context, workspaceID string) (bool, error) { url := fmt.Sprintf("%s/cp/workspaces/%s/status?instance_id=%s", p.baseURL, workspaceID, workspaceID) req, _ := http.NewRequestWithContext(ctx, "GET", url, nil) p.authHeaders(req) resp, err := p.httpClient.Do(req) if err != nil { - return false, err + return false, fmt.Errorf("cp provisioner: status: %w", err) } defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + // Don't leak the body — upstream errors may echo headers. + return false, fmt.Errorf("cp provisioner: status: unexpected %d", resp.StatusCode) + } var result struct{ State string `json:"state"` } - json.NewDecoder(resp.Body).Decode(&result) + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return false, fmt.Errorf("cp provisioner: status decode: %w", err) + } return result.State == "running", nil } diff --git a/workspace-server/internal/provisioner/cp_provisioner_test.go b/workspace-server/internal/provisioner/cp_provisioner_test.go index 1fddcbb24..78b5e9d68 100644 --- a/workspace-server/internal/provisioner/cp_provisioner_test.go +++ b/workspace-server/internal/provisioner/cp_provisioner_test.go @@ -338,6 +338,69 @@ func TestIsRunning_TransportErrorReturnsFalse(t *testing.T) { } } +// TestIsRunning_Non2xxSurfacesError — a CP 500/502/etc. must NOT +// be silently treated as "workspace stopped". Previously the handler +// would decode an empty body → State="" → return (false, nil) and +// the sweeper would see the workspace as not-running. Now every +// non-2xx is an error the caller can log + retry. +func TestIsRunning_Non2xxSurfacesError(t *testing.T) { + cases := []struct { + name string + status int + }{ + {"500 internal", 500}, + {"502 bad gateway", 502}, + {"503 unavailable", 503}, + {"401 unauthorized", 401}, + {"404 not found", 404}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.status) + _, _ = io.WriteString(w, `{"state":"running"}`) // liar body — must not be trusted + })) + defer srv.Close() + p := &CPProvisioner{baseURL: srv.URL, orgID: "org-1", httpClient: srv.Client()} + + got, err := p.IsRunning(context.Background(), "ws-1") + if err == nil { + t.Errorf("status %d: expected error, got nil", tc.status) + } + if got { + t.Errorf("status %d: must not report running=true on non-2xx", tc.status) + } + // Error must NOT echo the upstream body — CP 5xx bodies + // can contain echoed headers and we don't want logs to + // leak bearer tokens. + if err != nil && strings.Contains(err.Error(), "running") { + t.Errorf("status %d: error leaked upstream body: %q", tc.status, err.Error()) + } + }) + } +} + +// TestIsRunning_MalformedJSONBodyReturnsError — 200 but invalid JSON +// must surface an error rather than silently returning false. Prevents +// a middleware glitch (HTML error page with 200) from looking like +// "workspace stopped". +func TestIsRunning_MalformedJSONBodyReturnsError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + _, _ = io.WriteString(w, "maintenance mode") + })) + defer srv.Close() + p := &CPProvisioner{baseURL: srv.URL, orgID: "org-1", httpClient: srv.Client()} + + got, err := p.IsRunning(context.Background(), "ws-1") + if err == nil { + t.Errorf("malformed body: expected error, got nil (got=%v)", got) + } + if got { + t.Errorf("malformed body must not report running=true") + } +} + // TestClose_Noop — explicit contract: Close has no side effects and // no error. Exists for the Provisioner interface; compliance guard. func TestClose_Noop(t *testing.T) { diff --git a/workspace/claude_sdk_executor.py b/workspace/claude_sdk_executor.py index 8f8ce7e8e..f702eef5d 100644 --- a/workspace/claude_sdk_executor.py +++ b/workspace/claude_sdk_executor.py @@ -50,6 +50,7 @@ from executor_helpers import ( commit_memory, extract_message_text, get_a2a_instructions, + get_hma_instructions, get_mcp_server_path, get_system_prompt, read_delegation_results, @@ -211,12 +212,12 @@ class ClaudeSDKExecutor(AgentExecutor): return CONFIG_MOUNT def _build_system_prompt(self) -> str | None: - """Compose system prompt from file + A2A delegation instructions.""" + """Compose system prompt from file + A2A + HMA memory instructions.""" base = get_system_prompt(self.config_path, fallback=self.system_prompt) a2a = get_a2a_instructions(mcp=True) - if base and a2a: - return f"{base}\n\n{a2a}" - return base or a2a + hma = get_hma_instructions() + parts = [p for p in (base, a2a, hma) if p] + return "\n\n".join(parts) if parts else None def _prepare_prompt(self, user_input: str) -> str: """Prepend delegation results that arrived while idle.""" diff --git a/workspace/executor_helpers.py b/workspace/executor_helpers.py index 5bc50c90d..0d6e2d855 100644 --- a/workspace/executor_helpers.py +++ b/workspace/executor_helpers.py @@ -290,6 +290,31 @@ def get_a2a_instructions(mcp: bool = True) -> str: return _A2A_INSTRUCTIONS_MCP if mcp else _A2A_INSTRUCTIONS_CLI +_HMA_INSTRUCTIONS = """## Hierarchical Memory (HMA) +You have persistent memory tools that survive across sessions and restarts: + +- **commit_memory(content, scope)**: Save important information. + - LOCAL: private to you only (default) + - TEAM: shared with your parent workspace and siblings (same team) + - GLOBAL: shared with the entire org (only root workspaces can write) + +- **recall_memory(query)**: Search your accessible memories. Returns LOCAL + TEAM + GLOBAL matches. + +**When to use memory:** +- After making a decision or learning something non-obvious → commit_memory("decision X because Y", scope="TEAM") +- Before starting work → recall_memory("what did the team decide about X") +- When you discover org-wide knowledge (repo locations, API patterns, conventions) → commit_memory(fact, scope="GLOBAL") if you are a root workspace, or scope="TEAM" to share with your team +- After completing a task → commit_memory("completed task X, PR #N opened", scope="TEAM") so your lead and teammates know + +**Memory is automatically recalled** at the start of each new session. Use it proactively during work to share context. +""" + + +def get_hma_instructions() -> str: + """Return HMA memory instructions for system-prompt injection.""" + return _HMA_INSTRUCTIONS + + # ======================================================================== # Misc text helpers # ======================================================================== diff --git a/workspace/tests/test_claude_sdk_executor.py b/workspace/tests/test_claude_sdk_executor.py index e3781ad96..ac3b0c3dd 100644 --- a/workspace/tests/test_claude_sdk_executor.py +++ b/workspace/tests/test_claude_sdk_executor.py @@ -479,7 +479,8 @@ def test_build_system_prompt_combines_base_and_a2a_via_fixture(): """Direct test bypassing the execute() path.""" e = _make_executor() with patch("claude_sdk_executor.get_system_prompt", return_value="BASE"), \ - patch("claude_sdk_executor.get_a2a_instructions", return_value="A2A"): + patch("claude_sdk_executor.get_a2a_instructions", return_value="A2A"), \ + patch("claude_sdk_executor.get_hma_instructions", return_value=""): out = e._build_system_prompt() assert out == "BASE\n\nA2A" @@ -487,17 +488,31 @@ def test_build_system_prompt_combines_base_and_a2a_via_fixture(): def test_build_system_prompt_base_only(): e = _make_executor() with patch("claude_sdk_executor.get_system_prompt", return_value="BASE"), \ - patch("claude_sdk_executor.get_a2a_instructions", return_value=""): + patch("claude_sdk_executor.get_a2a_instructions", return_value=""), \ + patch("claude_sdk_executor.get_hma_instructions", return_value=""): assert e._build_system_prompt() == "BASE" def test_build_system_prompt_a2a_only(): e = _make_executor() with patch("claude_sdk_executor.get_system_prompt", return_value=None), \ - patch("claude_sdk_executor.get_a2a_instructions", return_value="A2A"): + patch("claude_sdk_executor.get_a2a_instructions", return_value="A2A"), \ + patch("claude_sdk_executor.get_hma_instructions", return_value=""): assert e._build_system_prompt() == "A2A" +def test_build_system_prompt_includes_hma(): + """HMA instructions are appended when present.""" + e = _make_executor() + with patch("claude_sdk_executor.get_system_prompt", return_value="BASE"), \ + patch("claude_sdk_executor.get_a2a_instructions", return_value="A2A"), \ + patch("claude_sdk_executor.get_hma_instructions", return_value="## Hierarchical Memory"): + out = e._build_system_prompt() + assert "BASE" in out + assert "A2A" in out + assert "## Hierarchical Memory" in out + + def test_prepare_prompt_no_delegation_returns_unchanged(): e = _make_executor() with patch("claude_sdk_executor.read_delegation_results", return_value=""):