From dee01af2c26dbc2a649d60f1952d5091b163c088 Mon Sep 17 00:00:00 2001
From: Molecule AI Fullstack Engineer
 <fullstack-engineer@agents.moleculesai.app>
Date: Sun, 10 May 2026 12:39:30 +0000
Subject: [PATCH] =?UTF-8?q?fix(canvas):=20sortParentsBeforeChildren=20?=
 =?UTF-8?q?=E2=80=94=20root-before-orphan=20ordering?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Algorithm did a single DFS pass over all nodes in input order, which
placed orphan nodes (parentId → missing node) before roots when the
orphan appeared earlier in the input array.  Tests: sortParentsBeforeChildren
"does not crash when parentId references a missing node" was failing.

Fix: two-pass approach — visit all root nodes first, then remaining
unvisited nodes (orphans).  Preserves existing correct behaviour for
valid parent→child chains.

Also:
- canvas/vitest.config.ts: add clarifying comment that Node environment
  is intentional (socket.url.test.ts runs in Node, DOM tests use the
  per-file // @vitest-environment jsdom directive).
- canvas/src/store/__tests__/socket.url.test.ts: simplify — drop the
  importWsUrl helper (no longer needed since env handling is direct and
  Node's lack of window.globalThis correctly triggers the localhost:8080
  fallback in deriveWsBaseUrl).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 canvas/src/store/__tests__/socket.url.test.ts | 55 ++++++-------------
 canvas/src/store/canvas-topology.ts           | 12 +++-
 canvas/vitest.config.ts                       |  3 +
 3 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/canvas/src/store/__tests__/socket.url.test.ts b/canvas/src/store/__tests__/socket.url.test.ts
index 563005ab..d572f378 100644
--- a/canvas/src/store/__tests__/socket.url.test.ts
+++ b/canvas/src/store/__tests__/socket.url.test.ts
@@ -1,23 +1,5 @@
 import { describe, it, expect, vi, afterEach } from "vitest";
 
-// Helper: reset modules, set env vars, import module, then restore env.
-async function importWsUrl(env: Record<string, string | undefined>) {
-  vi.resetModules();
-  const saved: Record<string, string | undefined> = {};
-  for (const [k, v] of Object.entries(env)) {
-    saved[k] = process.env[k];
-    if (v === undefined) delete process.env[k];
-    else process.env[k] = v;
-  }
-  const mod = await import("@/store/socket");
-  // Restore env
-  for (const [k, v] of Object.entries(saved)) {
-    if (v === undefined) delete process.env[k];
-    else process.env[k] = v;
-  }
-  return mod;
-}
-
 describe("socket WS_URL derivation", () => {
   afterEach(() => {
     vi.resetModules();
@@ -26,34 +8,34 @@ describe("socket WS_URL derivation", () => {
   });
 
   it("falls back to ws://localhost:8080/ws when no env vars are set", async () => {
-    const mod = await importWsUrl({
-      NEXT_PUBLIC_PLATFORM_URL: undefined,
-      NEXT_PUBLIC_WS_URL: undefined,
-    });
+    vi.resetModules();
+    delete process.env.NEXT_PUBLIC_PLATFORM_URL;
+    delete process.env.NEXT_PUBLIC_WS_URL;
+    const mod = await import("@/store/socket");
     expect(mod.WS_URL).toBe("ws://localhost:8080/ws");
   });
 
   it("derives WS_URL from NEXT_PUBLIC_PLATFORM_URL by replacing http→ws and appending /ws", async () => {
-    const mod = await importWsUrl({
-      NEXT_PUBLIC_PLATFORM_URL: "http://api.example.com",
-      NEXT_PUBLIC_WS_URL: undefined,
-    });
+    vi.resetModules();
+    process.env.NEXT_PUBLIC_PLATFORM_URL = "http://api.example.com";
+    delete process.env.NEXT_PUBLIC_WS_URL;
+    const mod = await import("@/store/socket");
     expect(mod.WS_URL).toBe("ws://api.example.com/ws");
   });
 
   it("handles https→wss correctly", async () => {
-    const mod = await importWsUrl({
-      NEXT_PUBLIC_PLATFORM_URL: "https://api.example.com",
-      NEXT_PUBLIC_WS_URL: undefined,
-    });
+    vi.resetModules();
+    process.env.NEXT_PUBLIC_PLATFORM_URL = "https://api.example.com";
+    delete process.env.NEXT_PUBLIC_WS_URL;
+    const mod = await import("@/store/socket");
     expect(mod.WS_URL).toBe("wss://api.example.com/ws");
   });
 
   it("NEXT_PUBLIC_WS_URL takes precedence over derived value", async () => {
-    const mod = await importWsUrl({
-      NEXT_PUBLIC_PLATFORM_URL: "http://api.example.com",
-      NEXT_PUBLIC_WS_URL: "wss://ws.example.com/custom",
-    });
+    vi.resetModules();
+    process.env.NEXT_PUBLIC_PLATFORM_URL = "http://api.example.com";
+    process.env.NEXT_PUBLIC_WS_URL = "wss://ws.example.com/custom";
+    const mod = await import("@/store/socket");
     expect(mod.WS_URL).toBe("wss://ws.example.com/custom");
   });
 
@@ -67,8 +49,7 @@ describe("socket WS_URL derivation", () => {
   it("PLATFORM_URL in api.ts reads from NEXT_PUBLIC_PLATFORM_URL", async () => {
     vi.resetModules();
     process.env.NEXT_PUBLIC_PLATFORM_URL = "http://prod.example.com";
-    const apiMod = await import("@/lib/api");
-    expect(apiMod.PLATFORM_URL).toBe("http://prod.example.com");
-    delete process.env.NEXT_PUBLIC_PLATFORM_URL;
+    const mod = await import("@/lib/api");
+    expect(mod.PLATFORM_URL).toBe("http://prod.example.com");
   });
 });
diff --git a/canvas/src/store/canvas-topology.ts b/canvas/src/store/canvas-topology.ts
index 334dcff7..5c733626 100644
--- a/canvas/src/store/canvas-topology.ts
+++ b/canvas/src/store/canvas-topology.ts
@@ -34,7 +34,17 @@ export function sortParentsBeforeChildren<T extends { id: string; parentId?: str
     visited.add(n.id);
     out.push(n);
   };
-  for (const n of nodes) visit(n);
+  // First visit all root nodes (no parentId or parent not in the set).
+  // This guarantees root nodes appear before orphans (nodes whose
+  // parentId references a node that is not in the input array).
+  for (const n of nodes) {
+    if (!n.parentId) visit(n);
+  }
+  // Then visit any remaining unvisited nodes — these are orphans whose
+  // referenced parent does not exist in the array.
+  for (const n of nodes) {
+    visit(n);
+  }
   return out;
 }
 
diff --git a/canvas/vitest.config.ts b/canvas/vitest.config.ts
index ab402cff..3acd745e 100644
--- a/canvas/vitest.config.ts
+++ b/canvas/vitest.config.ts
@@ -5,6 +5,9 @@ import path from 'path'
 export default defineConfig({
   plugins: [react()],
   test: {
+    // Tests that need a DOM use the per-file // @vitest-environment jsdom
+    // directive.  Node environment is the default so that socket.url.test.ts
+    // (no DOM needed) works without stubbing window.
     environment: 'node',
     exclude: ['e2e/**', 'node_modules/**', '**/dist/**'],
     // Issue #22 / vitest pool investigation:
-- 
2.45.2