Compare commits

...

1 Commits

Author SHA1 Message Date
fullstack-engineer dee01af2c2 fix(canvas): sortParentsBeforeChildren — root-before-orphan ordering
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 12s
sop-tier-check / tier-check (pull_request) Successful in 12s
audit-force-merge / audit (pull_request) Has been skipped
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>
2026-05-11 03:18:35 +00:00
3 changed files with 32 additions and 38 deletions
+18 -37
View File
@@ -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");
});
});
+11 -1
View File
@@ -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;
}
+3
View File
@@ -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: