Merge pull request 'test(canvas): add EmptyState tests + restore ApprovalBanner test isolation fix' from test/canvas-empty-state-coverage into main

canvas/src/components/__tests__/ApprovalBanner.test.tsx

@@ -5,20 +5,22 @@
  * Covers: renders nothing when no approvals, polls /approvals/pending,
  * shows approval cards, approve/deny decisions, toast notifications.
  *
- * Note: does NOT mock @/lib/api — uses vi.spyOn on the real module.
- * vi.restoreAllMocks() is omitted from afterEach so queued mock values
- * (set up via mockResolvedValueOnce in beforeEach) are preserved for the
- * component's useEffect to consume.
+ * Uses vi.hoisted + vi.mock (file-level) for @/lib/api. vi.resetModules()
+ * in every afterEach undoes the mock so other test files that import the
+ * real api module (e.g. socket.url.test.ts) are unaffected.
  */
 import React from "react";
 import { render, screen, fireEvent, cleanup, act } from "@testing-library/react";
 import { afterEach, describe, expect, it, vi, beforeEach } from "vitest";
 import { ApprovalBanner } from "../ApprovalBanner";
 import { showToast } from "@/components/Toaster";
-import { api } from "@/lib/api";
 
-vi.mock("@/components/Toaster", () => ({
-  showToast: vi.fn(),
+// ─── Hoisted mock refs ─────────────────────────────────────────────────────────
+// vi.hoisted runs in the same hoisting phase as vi.mock factories, so these
+// refs are stable across all tests and available inside the mock factory.
+const { mockApiGet, mockApiPost } = vi.hoisted(() => ({
+  mockApiGet: vi.fn<(args: unknown[]) => Promise<unknown>>(),
+  mockApiPost: vi.fn<(args: unknown[]) => Promise<unknown>>(),
 }));
 
 // ─── Helpers ──────────────────────────────────────────────────────────────────
@@ -41,28 +43,42 @@ const pendingApproval = (id = "a1", workspaceId = "ws-1"): {
   created_at: "2026-05-10T10:00:00Z",
 });
 
-// Shared spy references so individual tests can reset or reject the POST mock
-// without needing to call spyOn again (which would create a duplicate spy).
-let mockGet: ReturnType<typeof vi.spyOn>;
-let mockPost: ReturnType<typeof vi.spyOn>;
+// ─── Static mocks (file-level — no other test needs the real modules) ─────────
 
-// ─── Tests ────────────────────────────────────────────────────────────────────
+vi.mock("@/components/Toaster", () => ({
+  showToast: vi.fn(),
+}));
+
+// vi.resetModules() in afterEach undoes this mock so other files that import
+// the real api module are unaffected.
+vi.mock("@/lib/api", () => ({
+  api: {
+    get: mockApiGet,
+    post: mockApiPost,
+  },
+}));
+
+// ─── Tests ─────────────────────────────────────────────────────────────────────
 
 describe("ApprovalBanner — empty state", () => {
   beforeEach(() => {
     vi.useFakeTimers();
-    vi.spyOn(api, "get").mockResolvedValueOnce([]);
+    mockApiGet.mockReset().mockResolvedValue([]);
+    mockApiPost.mockReset().mockResolvedValue({});
   });
 
   afterEach(() => {
     cleanup();
     vi.useRealTimers();
+    vi.restoreAllMocks();
+    vi.resetModules();
   });
 
   it("renders nothing when there are no pending approvals", async () => {
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     expect(screen.queryByRole("alert")).toBeNull();
+    expect(mockApiGet).toHaveBeenCalled();
   });
 
   it("does not render any approve/deny buttons when list is empty", async () => {
@@ -76,41 +92,40 @@ describe("ApprovalBanner — empty state", () => {
 describe("ApprovalBanner — renders approval cards", () => {
   beforeEach(() => {
     vi.useFakeTimers();
-    mockGet = vi.spyOn(api, "get").mockResolvedValueOnce([
+    mockApiGet.mockReset().mockResolvedValue([
       pendingApproval("a1"),
       pendingApproval("a2", "ws-2"),
     ]);
+    mockApiPost.mockReset().mockResolvedValue({});
   });
 
   afterEach(() => {
     cleanup();
     vi.useRealTimers();
+    vi.restoreAllMocks();
+    vi.resetModules();
   });
 
   it("renders an alert card for each pending approval", async () => {
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
-    const alerts = screen.getAllByRole("alert");
-    expect(alerts).toHaveLength(2);
-    mockGet.mockRestore();
+    expect(screen.getAllByRole("alert")).toHaveLength(2);
   });
 
   it("displays the workspace name and action text", async () => {
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
-    const nameEls = screen.getAllByText(/test workspace needs approval/i);
-    expect(nameEls).toHaveLength(2);
+    expect(screen.getAllByText(/test workspace needs approval/i)).toHaveLength(2);
   });
 
   it("displays the reason when present", async () => {
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
-    const reasons = screen.getAllByText(/requires human approval/i);
-    expect(reasons).toHaveLength(2);
+    expect(screen.getAllByText(/requires human approval/i)).toHaveLength(2);
   });
 
   it("omits the reason div when reason is null", async () => {
-    vi.spyOn(api, "get").mockResolvedValueOnce([{
+    mockApiGet.mockReset().mockResolvedValue([{
       ...pendingApproval("a1"),
       reason: null,
     }]);
@@ -124,7 +139,6 @@ describe("ApprovalBanner — renders approval cards", () => {
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     const approveBtns = screen.getAllByRole("button", { name: /Approve/i });
     const denyBtns = screen.getAllByRole("button", { name: /Deny/i });
-    // 2 cards, each card has 1 Approve + 1 Deny button → 2 of each minimum
     expect(approveBtns.length).toBeGreaterThanOrEqual(2);
     expect(denyBtns.length).toBeGreaterThanOrEqual(2);
   });
@@ -132,21 +146,22 @@ describe("ApprovalBanner — renders approval cards", () => {
   it("has aria-live=assertive on the alert container", async () => {
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
-    const alert = screen.getAllByRole("alert")[0];
-    expect(alert.getAttribute("aria-live")).toBe("assertive");
+    expect(screen.getAllByRole("alert")[0].getAttribute("aria-live")).toBe("assertive");
   });
 });
 
 describe("ApprovalBanner — decisions", () => {
   beforeEach(() => {
     vi.useFakeTimers();
-    mockGet = vi.spyOn(api, "get").mockResolvedValueOnce([pendingApproval("a1")]);
-    mockPost = vi.spyOn(api, "post").mockResolvedValue({});
+    mockApiGet.mockReset().mockResolvedValue([pendingApproval("a1")]);
+    mockApiPost.mockReset().mockResolvedValue({});
   });
 
   afterEach(() => {
     cleanup();
     vi.useRealTimers();
+    vi.restoreAllMocks();
+    vi.resetModules();
   });
 
   it("calls POST /workspaces/:id/approvals/:id/decide on Approve click", async () => {
@@ -154,7 +169,7 @@ describe("ApprovalBanner — decisions", () => {
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]);
     await act(async () => { /* flush */ });
-    expect(vi.mocked(api.post)).toHaveBeenCalledWith(
+    expect(mockApiPost).toHaveBeenCalledWith(
       "/workspaces/ws-1/approvals/a1/decide",
       expect.objectContaining({ decision: "approved" })
     );
@@ -165,7 +180,7 @@ describe("ApprovalBanner — decisions", () => {
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     fireEvent.click(screen.getAllByRole("button", { name: /deny/i })[0]);
     await act(async () => { /* flush */ });
-    expect(vi.mocked(api.post)).toHaveBeenCalledWith(
+    expect(mockApiPost).toHaveBeenCalledWith(
       "/workspaces/ws-1/approvals/a1/decide",
       expect.objectContaining({ decision: "denied" })
     );
@@ -197,7 +212,10 @@ describe("ApprovalBanner — decisions", () => {
   });
 
   it("shows an error toast when POST fails", async () => {
-    mockPost.mockReset().mockRejectedValue(new Error("Network error"));
+    // mockImplementation preserves the vi.fn() wrapper (unlike mockReset() which
+    // strips it and causes the real fetch() to fire — the root cause of the
+    // original flakiness in this file).
+    mockApiPost.mockImplementation(() => Promise.reject(new Error("Network error")));
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]);
@@ -209,9 +227,9 @@ describe("ApprovalBanner — decisions", () => {
   });
 
   it("keeps the card visible when the POST fails", async () => {
-    // Reset the post mock before rejecting so the beforeEach's resolved value
-    // is gone and we get a clean rejection instead of a resolved→rejected queue.
-    mockPost.mockReset().mockRejectedValue(new Error("Network error"));
+    // Same mockImplementation pattern — preserves the wrapper so the component's
+    // catch block runs instead of the real fetch().
+    mockApiPost.mockImplementation(() => Promise.reject(new Error("Network error")));
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]);
@@ -223,12 +241,15 @@ describe("ApprovalBanner — decisions", () => {
 describe("ApprovalBanner — handles empty list from server", () => {
   beforeEach(() => {
     vi.useFakeTimers();
-    vi.spyOn(api, "get").mockResolvedValueOnce([]);
+    mockApiGet.mockReset().mockResolvedValue([]);
+    mockApiPost.mockReset().mockResolvedValue({});
   });
 
   afterEach(() => {
     cleanup();
     vi.useRealTimers();
+    vi.restoreAllMocks();
+    vi.resetModules();
   });
 
   it("shows nothing when the API returns an empty array on first poll", async () => {

canvas/src/components/__tests__/EmptyState.test.tsx

@@ -0,0 +1,370 @@
+// @vitest-environment jsdom
+/**
+ * Tests for EmptyState — the full-canvas welcome card shown on first load.
+ *
+ * Covers:
+ *   - Loading state (GET /templates in flight)
+ *   - Fetch failure → empty template grid (templates = [])
+ *   - Template grid renders with correct content
+ *   - Template button disabled while deploying
+ *   - "Deploying..." label on the button being deployed
+ *   - "Create blank" button POSTs /workspaces
+ *   - "Creating..." label while blank workspace is being created
+ *   - Blank create error shows error banner
+ *   - Error banner has role="alert"
+ *   - All buttons disabled while any deploy is in-flight
+ *   - handleDeployed fires after 500ms delay
+ *
+ * Uses vi.hoisted + vi.mock to fully isolate the api module, matching
+ * the pattern established in ApprovalBanner, MemoryTab, and ScheduleTab tests.
+ */
+import React from "react";
+import { render, screen, fireEvent, cleanup, act } from "@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { EmptyState } from "../EmptyState";
+
+// ─── Hoisted mock refs ─────────────────────────────────────────────────────────
+// vi.hoisted runs in the same hoisting phase as vi.mock factories, so all refs
+// are available both to the factory and to test bodies.
+const { mockApiGet, mockApiPost } = vi.hoisted(() => ({
+  mockApiGet: vi.fn<(args: unknown[]) => Promise<unknown>>(),
+  mockApiPost: vi.fn<(args: unknown[]) => Promise<{ id: string }>>(),
+}));
+
+// Mutable deploy state — object reference is const; properties can be mutated.
+const _deploy = vi.hoisted(() => ({
+  deployFn: vi.fn(),
+  deploying: undefined as string | undefined,
+  error: undefined as string | undefined,
+  modal: null as React.ReactNode,
+}));
+
+const { mockSelectNode, mockSetPanelTab } = vi.hoisted(() => ({
+  mockSelectNode: vi.fn(),
+  mockSetPanelTab: vi.fn(),
+}));
+
+// ─── Mocks ────────────────────────────────────────────────────────────────────
+
+vi.mock("@/lib/api", () => ({
+  api: {
+    get: mockApiGet,
+    post: mockApiPost,
+  },
+}));
+
+vi.mock("@/hooks/useTemplateDeploy", () => ({
+  useTemplateDeploy: () => ({
+    deploy: _deploy.deployFn,
+    deploying: _deploy.deploying,
+    error: _deploy.error,
+    modal: _deploy.modal,
+  }),
+}));
+
+vi.mock("@/store/canvas", () => ({
+  useCanvasStore: Object.assign(
+    vi.fn((selector: (s: { getState: () => { selectNode: typeof mockSelectNode; setPanelTab: typeof mockSetPanelTab } }) => unknown) =>
+      selector({
+        getState: () => ({
+          selectNode: mockSelectNode,
+          setPanelTab: mockSetPanelTab,
+        }),
+      })
+    ),
+    { getState: () => ({ selectNode: mockSelectNode, setPanelTab: mockSetPanelTab }) }
+  ),
+}));
+
+vi.mock("../TemplatePalette", () => ({
+  OrgTemplatesSection: () => null,
+}));
+
+vi.mock("../Spinner", () => ({
+  Spinner: () => <span data-testid="spinner">⟳</span>,
+}));
+
+vi.mock("@/lib/design-tokens", () => ({
+  TIER_CONFIG: {
+    1: { label: "T1", color: "text-ink-mid bg-surface-card border border-line", border: "text-ink-mid border-line" },
+    2: { label: "T2", color: "text-white bg-accent border border-accent-strong", border: "text-accent border-accent" },
+    3: { label: "T3", color: "text-white bg-violet-600 border border-violet-700", border: "text-violet-600 border-violet-500" },
+    4: { label: "T4", color: "text-white bg-warm border border-warm", border: "text-warm border-warm" },
+  },
+}));
+
+// ─── Fixtures ─────────────────────────────────────────────────────────────────
+
+const TEMPLATE = {
+  id: "tpl-1",
+  name: "Claude Code Agent",
+  description: "A general-purpose coding assistant",
+  tier: 2,
+  skill_count: 3,
+  model: "claude-opus-4-5",
+};
+
+function template(overrides: Partial<typeof TEMPLATE> = {}): typeof TEMPLATE {
+  return { ...TEMPLATE, ...overrides };
+}
+
+// ─── Helpers ───────────────────────────────────────────────────────────────────
+
+function renderEmpty() {
+  return render(<EmptyState />);
+}
+
+// Flush React state + microtasks after an act boundary.
+async function flush() {
+  await act(async () => { await Promise.resolve(); });
+}
+
+// Reset deploy state to defaults before each test.
+function resetDeployState() {
+  _deploy.deployFn.mockReset();
+  _deploy.deploying = undefined;
+  _deploy.error = undefined;
+  _deploy.modal = null;
+}
+
+// ─── Tests ─────────────────────────────────────────────────────────────────────
+
+describe("EmptyState — loading", () => {
+  beforeEach(() => {
+    mockApiGet.mockReset().mockImplementation(
+      () => new Promise(() => {}) // never resolves
+    );
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.restoreAllMocks();
+  });
+
+  it("shows loading state while GET /templates is pending", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByTestId("spinner")).toBeTruthy();
+    expect(screen.getByText("Loading templates...")).toBeTruthy();
+  });
+
+  // "create blank" is rendered outside the loading/template-grid conditional,
+  // so it is always visible — adjust expectation accordingly.
+  it("renders 'create blank' button during loading", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByRole("button", { name: "+ Create blank workspace" })).toBeTruthy();
+  });
+
+  it("does not render template buttons while loading", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.queryByText("Claude Code Agent")).toBeNull();
+  });
+});
+
+describe("EmptyState — templates", () => {
+  beforeEach(() => {
+    mockApiGet.mockReset().mockResolvedValue([template()]);
+    resetDeployState();
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.restoreAllMocks();
+  });
+
+  it("renders the welcome heading", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByText("Deploy your first agent")).toBeTruthy();
+  });
+
+  it("renders template buttons with name and description", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByText("Claude Code Agent")).toBeTruthy();
+    expect(screen.getByText("A general-purpose coding assistant")).toBeTruthy();
+  });
+
+  it("renders tier badge and skill count", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByText("T2")).toBeTruthy();
+    // skill_count renders as "3 skills · <model>"
+    expect(screen.getByText(/^3 skills/)).toBeTruthy();
+  });
+
+  it("renders model name when present", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByText(/claude-opus/i)).toBeTruthy();
+  });
+
+  it("calls deploy with the template on click", async () => {
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByText("Claude Code Agent"));
+    expect(_deploy.deployFn).toHaveBeenCalledWith(template());
+  });
+
+  it("shows 'Deploying...' on the button of the template being deployed", async () => {
+    _deploy.deploying = "tpl-1";
+    renderEmpty();
+    await flush();
+    expect(screen.getByText("Deploying...")).toBeTruthy();
+  });
+
+  it("disables the template button of the deploying template", async () => {
+    _deploy.deploying = "tpl-1";
+    renderEmpty();
+    await flush();
+    const btn = screen.getByText("Deploying...").closest("button") as HTMLButtonElement;
+    expect(btn.disabled).toBe(true);
+  });
+
+  it("disables 'create blank' while a template is deploying", async () => {
+    _deploy.deploying = "tpl-1";
+    renderEmpty();
+    await flush();
+    expect(screen.getByRole("button", { name: "+ Create blank workspace" }).disabled).toBe(true);
+  });
+});
+
+describe("EmptyState — fetch failure / empty templates", () => {
+  beforeEach(() => {
+    mockApiGet.mockReset().mockResolvedValue([]);
+    resetDeployState();
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.restoreAllMocks();
+  });
+
+  it("does not render template grid when GET /templates returns []", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.queryByText("Claude Code Agent")).toBeNull();
+  });
+
+  it("renders 'create blank' button when templates list is empty", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.getByRole("button", { name: "+ Create blank workspace" })).toBeTruthy();
+  });
+
+  it("does not render template grid when GET /templates rejects", async () => {
+    mockApiGet.mockReset().mockRejectedValue(new Error("Network failure"));
+    renderEmpty();
+    await flush();
+    expect(screen.queryByText("Claude Code Agent")).toBeNull();
+  });
+});
+
+describe("EmptyState — create blank", () => {
+  beforeEach(() => {
+    mockApiGet.mockReset().mockResolvedValue([template()]);
+    mockApiPost.mockReset().mockResolvedValue({ id: "ws-new" });
+    resetDeployState();
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it("calls POST /workspaces on 'create blank' click", async () => {
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByRole("button", { name: "+ Create blank workspace" }));
+    await act(async () => { await Promise.resolve(); });
+    expect(mockApiPost).toHaveBeenCalledWith(
+      "/workspaces",
+      expect.objectContaining({ name: "My First Agent" })
+    );
+  });
+
+  it("shows 'Creating...' while blank workspace POST is pending", async () => {
+    mockApiPost.mockReset().mockImplementation(
+      () => new Promise(() => {}) // never resolves
+    );
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByRole("button", { name: "+ Create blank workspace" }));
+    await act(async () => { await Promise.resolve(); });
+    expect(screen.getByRole("button", { name: "Creating..." })).toBeTruthy();
+  });
+
+  it("calls selectNode + setPanelTab after 500ms on successful create", async () => {
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByRole("button", { name: "+ Create blank workspace" }));
+    await act(async () => { await Promise.resolve(); }); // flush POST
+    await act(async () => { vi.advanceTimersByTime(500); });
+    expect(mockSelectNode).toHaveBeenCalledWith("ws-new");
+    expect(mockSetPanelTab).toHaveBeenCalledWith("chat");
+  });
+
+  it("disables template buttons while creating blank workspace", async () => {
+    mockApiPost.mockReset().mockImplementation(
+      () => new Promise(() => {}) // never resolves
+    );
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByRole("button", { name: "+ Create blank workspace" }));
+    await act(async () => { await Promise.resolve(); });
+    expect((screen.getByText("Claude Code Agent").closest("button") as HTMLButtonElement).disabled).toBe(true);
+  });
+
+  it("shows error banner when POST /workspaces fails", async () => {
+    mockApiPost.mockReset().mockRejectedValue(new Error("Server error"));
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByRole("button", { name: "+ Create blank workspace" }));
+    await act(async () => { await Promise.resolve(); });
+    expect(screen.getByRole("alert")).toBeTruthy();
+    expect(screen.getByText(/server error/i)).toBeTruthy();
+  });
+
+  it("clears 'Creating...' and shows button again after POST failure", async () => {
+    mockApiPost.mockReset().mockRejectedValue(new Error("Server error"));
+    renderEmpty();
+    await flush();
+    fireEvent.click(screen.getByRole("button", { name: "+ Create blank workspace" }));
+    await act(async () => { await Promise.resolve(); });
+    // After rejection, blankCreating = false → button reverts to default label
+    expect(screen.getByRole("button", { name: "+ Create blank workspace" })).toBeTruthy();
+  });
+});
+
+describe("EmptyState — error banner", () => {
+  beforeEach(() => {
+    mockApiGet.mockReset().mockResolvedValue([template()]);
+    resetDeployState();
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it("has role=alert on the error banner", async () => {
+    _deploy.error = "Template deploy failed";
+    renderEmpty();
+    await flush();
+    const alert = screen.getByRole("alert");
+    expect(alert).toBeTruthy();
+    expect(alert.textContent).toContain("Template deploy failed");
+  });
+
+  it("does not show error banner when no errors", async () => {
+    renderEmpty();
+    await flush();
+    expect(screen.queryByRole("alert")).toBeNull();
+  });
+});