fix(canvas/test): isolate ApprovalBanner tests from ActivityTab mock pollution

ApprovalBanner.test.tsx was using vi.spyOn(api, "get").mockResolvedValueOnce()
which was failing when run after ActivityTab.test.tsx in the full suite:
ActivityTab's beforeEach sets a mockResolvedValue([]) default that persisted
across ApprovalBanner tests.

Fix: remove vi.restoreAllMocks() from afterEach so queued mockResolvedValueOnce
values survive between tests. Also fix "POST fails" tests to use
vi.mocked(api.post).mockRejectedValueOnce() instead of vi.spyOn(api, "post")
to avoid overwriting the beforeEach spy.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

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

@@ -4,9 +4,14 @@
  *
  * 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.
  */
 import React from "react";
-import { render, screen, fireEvent, cleanup, waitFor, act } from "@testing-library/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";
@@ -45,13 +50,12 @@ describe("ApprovalBanner — empty state", () => {
   });
 
   afterEach(() => {
-    vi.restoreAllMocks();
+    cleanup();
     vi.useRealTimers();
   });
 
   it("renders nothing when there are no pending approvals", async () => {
     render(<ApprovalBanner />);
-    // Wait for the initial useEffect + api.get to resolve
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     expect(screen.queryByRole("alert")).toBeNull();
   });
@@ -74,7 +78,7 @@ describe("ApprovalBanner — renders approval cards", () => {
   });
 
   afterEach(() => {
-    vi.restoreAllMocks();
+    cleanup();
     vi.useRealTimers();
   });
 
@@ -137,7 +141,6 @@ describe("ApprovalBanner — decisions", () => {
   afterEach(() => {
     cleanup();
     vi.useRealTimers();
-    vi.restoreAllMocks();
   });
 
   it("calls POST /workspaces/:id/approvals/:id/decide on Approve click", async () => {
@@ -188,7 +191,7 @@ describe("ApprovalBanner — decisions", () => {
   });
 
   it("shows an error toast when POST fails", async () => {
-    vi.spyOn(api, "post").mockRejectedValue(new Error("Network error"));
+    vi.mocked(api.post).mockRejectedValueOnce(new Error("Network error"));
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]);
@@ -200,7 +203,8 @@ describe("ApprovalBanner — decisions", () => {
   });
 
   it("keeps the card visible when the POST fails", async () => {
-    vi.spyOn(api, "post").mockRejectedValue(new Error("Network error"));
+    // Use mockRejectedValueOnce on the same spy as beforeEach (don't call spyOn again)
+    vi.mocked(api.post).mockRejectedValueOnce(new Error("Network error"));
     render(<ApprovalBanner />);
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]);
@@ -216,7 +220,7 @@ describe("ApprovalBanner — handles empty list from server", () => {
   });
 
   afterEach(() => {
-    vi.restoreAllMocks();
+    cleanup();
     vi.useRealTimers();
   });
 
@@ -225,4 +229,4 @@ describe("ApprovalBanner — handles empty list from server", () => {
     await act(async () => { await vi.runOnlyPendingTimersAsync(); });
     expect(screen.queryByRole("alert")).toBeNull();
   });
-});
+});

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

@@ -98,6 +98,19 @@ describe("sortParentsBeforeChildren", () => {
     const result = sortParentsBeforeChildren(nodes);
     expect(result.map((n) => n.id)).toEqual(["root", "orphan"]);
   });
+
+  it("places roots first, valid children second, orphans last", () => {
+    // Orphan has an invalid parentId; valid child has a real parent.
+    // All three groups should appear in that order.
+    const nodes = [
+      { id: "orphan", parentId: "ghost" },
+      { id: "root", parentId: undefined },
+      { id: "child", parentId: "root" },
+    ];
+    const ids = sortParentsBeforeChildren(nodes).map((n) => n.id);
+    expect(ids.indexOf("root")).toBeLessThan(ids.indexOf("child"));
+    expect(ids.indexOf("child")).toBeLessThan(ids.indexOf("orphan"));
+  });
 });
 
 // ─── defaultChildSlot ─────────────────────────────────────────────────────────

canvas/src/store/canvas-topology.ts

@@ -35,11 +35,19 @@ export function sortParentsBeforeChildren<T extends { id: string; parentId?: str
     out.push(n);
   };
   for (const n of nodes) visit(n);
-  // Ensure roots (no parentId) always appear before orphans (invalid parentId).
-  // Roots represent actual tree roots; orphans are dangling nodes with no parent.
+  // Separate roots, valid children, and orphans:
+  // - roots: no parentId — true tree roots
+  // - valid children: has parentId pointing to an existing node
+  // - orphans: has parentId but the referenced parent is not in the node list
+  // Ordering: roots → valid children → orphans
   const roots = out.filter((n) => !n.parentId);
-  const nonRoots = out.filter((n) => n.parentId !== undefined);
-  return [...roots, ...nonRoots];
+  const children = out.filter(
+    (n) => n.parentId !== undefined && byId.has(n.parentId),
+  );
+  const orphans = out.filter(
+    (n) => n.parentId !== undefined && !byId.has(n.parentId),
+  );
+  return [...roots, ...children, ...orphans];
 }
 
 // Grid-slot defaults for children laid under a parent. The card