From a43b4f934d545c6406ea582d637b267082bf8988 Mon Sep 17 00:00:00 2001 From: Julien Brissonneau Date: Sat, 22 Aug 2026 23:33:22 -0400 Subject: [PATCH 1/4] registry: add test-only _snapshot() for leak-free registry mutation in tests nodeRegistry is a module singleton and bun runs a package's test files sequentially in one process, so a test that registers a throwaway kind (or _reset()s) without restoring leaks that state into every later test file. File order varies by platform (macOS vs CI Linux), which turns such leaks into order-dependent flakes. _snapshot() captures defs + plugin bookkeeping and returns a restore function for afterEach/finally. Co-Authored-By: Claude Fable 5 --- packages/core/src/registry/registry.test.ts | 29 +++++++++++++++++++++ packages/core/src/registry/registry.ts | 26 ++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/packages/core/src/registry/registry.test.ts b/packages/core/src/registry/registry.test.ts index 1b53ca093..91a1db63c 100644 --- a/packages/core/src/registry/registry.test.ts +++ b/packages/core/src/registry/registry.test.ts @@ -105,6 +105,35 @@ describe('nodeRegistry', () => { registerNode(b) expect(nodeRegistry.schemas()).toEqual([a.schema, b.schema]) }) + + test('_snapshot() restores definitions and plugin bookkeeping', async () => { + const kept = makeDefinition('kept') + registerNode(kept) + await loadPlugin({ + id: 'test:kept-plugin', + apiVersion: 1, + nodes: [makeDefinition('kept-plugin-kind')], + } as Plugin) + + const restore = nodeRegistry._snapshot() + + // Mutate every kind of registry state a test can leak: a throwaway + // definition, a full reset, and a plugin load with its kind bookkeeping. + registerNode(makeDefinition('leaked')) + nodeRegistry._reset() + await loadPlugin({ + id: 'test:leaked-plugin', + apiVersion: 1, + nodes: [makeDefinition('leaked-plugin-kind')], + } as Plugin) + + restore() + + expect(Array.from(nodeRegistry.entries(), ([k]) => k)).toEqual(['kept', 'kept-plugin-kind']) + expect(nodeRegistry.get('kept')).toBe(kept) + expect(getNodePluginId('kept-plugin-kind')).toBe('test:kept-plugin') + expect(getNodePluginId('leaked-plugin-kind')).toBeUndefined() + }) }) describe('isPresettable', () => { diff --git a/packages/core/src/registry/registry.ts b/packages/core/src/registry/registry.ts index 7cd86f02d..1d6e35ed4 100644 --- a/packages/core/src/registry/registry.ts +++ b/packages/core/src/registry/registry.ts @@ -131,11 +131,37 @@ class NodeRegistryImpl implements NodeRegistry { inspectorExtensionsByKind.clear() notifyRegistryChanged() } + + // Test-only — captures the registry (definitions + plugin bookkeeping) and + // returns a restore function. The registry is a module singleton and bun + // runs a package's test files sequentially in ONE process, so a test that + // registers a throwaway kind (or `_reset()`s) without restoring leaks that + // state into every later test FILE — and file order varies by platform + // (macOS vs CI Linux), which turns the leak into an order-dependent flake. + // Wrap registry mutations in `const restore = nodeRegistry._snapshot()` + // + `restore()` in `afterEach`/`finally`. + _snapshot(): () => void { + const defs = new Map(this.defs) + const pluginIds = new Map(pluginIdsByKind) + const extensions = new Map( + Array.from(inspectorExtensionsByKind, ([kind, list]) => [kind, [...list]] as const), + ) + return () => { + this.defs.clear() + for (const [kind, def] of defs) this.defs.set(kind, def) + pluginIdsByKind.clear() + for (const [kind, id] of pluginIds) pluginIdsByKind.set(kind, id) + inspectorExtensionsByKind.clear() + for (const [kind, list] of extensions) inspectorExtensionsByKind.set(kind, [...list]) + notifyRegistryChanged() + } + } } export const nodeRegistry: NodeRegistry & { _register: (def: AnyNodeDefinition) => void _reset: () => void + _snapshot: () => () => void } = new NodeRegistryImpl() export function registerNode(def: AnyNodeDefinition): void { From 950cc3650e600f4d68c205ae1b51e6db39ac0715 Mon Sep 17 00:00:00 2001 From: Julien Brissonneau Date: Sat, 22 Aug 2026 23:37:14 -0400 Subject: [PATCH 2/4] editor tests: stop leaking registry mutations across test files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit floorplan-registry-layer.test.ts registered capabilities-less fixture definitions (cabinet, cabinet-module, linked-floorplan-test) into the shared nodeRegistry singleton without cleanup. When bun's file order put it before pointer-support-cap.test.ts (order differs macOS vs CI Linux), the leaked entries crashed the top-surface enumeration at pointer-support-cap.ts:163 with 'definition.capabilities is undefined' — the night-8 CI flake (run 32580694134). Reproduced locally with bun test --randomize (seeds 1/2/4/6 on the two-file pair). wall-drafting.test.ts also _reset() the registry mid-test, stripping it for later files — same pollution class, contained the same way. Both now snapshot/restore via the new nodeRegistry._snapshot(). Co-Authored-By: Claude Fable 5 --- .../floorplan-registry-layer.test.ts | 20 ++++++++++++++++++- .../tools/wall/wall-drafting.test.ts | 15 +++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts b/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts index 5ef69dc98..e416ad0df 100644 --- a/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts +++ b/packages/editor/src/components/editor-2d/renderers/floorplan-registry-layer.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, mock, test } from 'bun:test' +import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test' import type { AnyNode, AnyNodeId, @@ -447,12 +447,20 @@ describe('floorplan annotation overlay routing', () => { }) describe('computeAffectedSiblingIds', () => { + // The cabinet fixture definitions have no `capabilities` — leaking them + // past this describe crashes any later test FILE that enumerates the + // registry (night-8 CI: pointer-support-cap.test.ts, run 32580694134). + let restoreRegistry: () => void + beforeEach(() => { + restoreRegistry = nodeRegistry._snapshot() nodeRegistry._reset() registerCabinetFloorplanDefinition('cabinet') registerCabinetFloorplanDefinition('cabinet-module') }) + afterEach(() => restoreRegistry()) + test('propagates cabinet live overrides through the cabinet family', () => { const run = cabinetRun('cabinet_run', ['cabinet-module_main', 'cabinet-module_corner']) const module = cabinetModule('cabinet-module_main', run.id) @@ -545,6 +553,16 @@ describe('collectFloorplanDependencyNodes', () => { }) describe('collectFloorplanLinkedLevelNodes', () => { + // Same containment as computeAffectedSiblingIds above: the fixture + // definition has no `capabilities`, so it must not outlive this describe. + let restoreRegistry: () => void + + beforeEach(() => { + restoreRegistry = nodeRegistry._snapshot() + }) + + afterEach(() => restoreRegistry()) + test('projects a node onto a linked destination level with its real children', () => { nodeRegistry._reset() registerNode({ diff --git a/packages/editor/src/components/tools/wall/wall-drafting.test.ts b/packages/editor/src/components/tools/wall/wall-drafting.test.ts index eaf7b662d..718d537cd 100644 --- a/packages/editor/src/components/tools/wall/wall-drafting.test.ts +++ b/packages/editor/src/components/tools/wall/wall-drafting.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, test } from 'bun:test' +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' import { type AnyNode, type AnyNodeId, @@ -143,6 +143,15 @@ function levelWalls(): WallNode[] { } describe('createWallOnCurrentLevel', () => { + // Set by tests that mutate the process-wide node registry; restored here + // so the mutation can't leak into later test files (order-dependent flakes). + let restoreRegistry: (() => void) | undefined + + afterEach(() => { + restoreRegistry?.() + restoreRegistry = undefined + }) + beforeEach(() => { useViewer.setState({ selection: { @@ -314,6 +323,10 @@ describe('createWallOnCurrentLevel', () => { }) test('pins an existing construction source before a generated room slab can lift it', () => { + // The reset + throwaway `block` registration is scoped to this test — + // the registry is a process-wide singleton, so leaking it would leave + // later test FILES with a stripped registry (order-dependent flakes). + restoreRegistry = nodeRegistry._snapshot() nodeRegistry._reset() spatialGridManager.clear() registerNode({ From 8de7a40f4d57996f75bf06a1a0e9d6ce077eb44c Mon Sep 17 00:00:00 2001 From: Julien Brissonneau Date: Sat, 22 Aug 2026 23:38:35 -0400 Subject: [PATCH 3/4] pointer-support: tolerate registry definitions without capabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The top-surface election enumerates every registered kind. capabilities is typed required on NodeDefinition, but a plugin bundle (or a leaked test fixture) can ship a minimal definition without it at runtime — one such entry crashed the resolver with a TypeError at pointer-support-cap line 163 in the night-8 CI run (32580694134). Defensive optional chain + a gate test that registers a capabilities-less definition and asserts the election neither throws nor mis-elects (verified failing against the unguarded code, passing with the chain). Co-Authored-By: Claude Fable 5 --- .../tools/shared/pointer-support-cap.test.ts | 35 +++++++++++++++++++ .../tools/shared/pointer-support-cap.ts | 7 +++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/components/tools/shared/pointer-support-cap.test.ts b/packages/editor/src/components/tools/shared/pointer-support-cap.test.ts index c8c508509..5b2fea075 100644 --- a/packages/editor/src/components/tools/shared/pointer-support-cap.test.ts +++ b/packages/editor/src/components/tools/shared/pointer-support-cap.test.ts @@ -241,4 +241,39 @@ describe('resolvePointerSupportSurface node tops', () => { wallSupport.baseSegments.every((segment) => Math.abs(segment.elevation - 2) < 1e-6), ).toBe(true) }) + + test('tolerates a registered definition without capabilities', () => { + // Gates the pollution class behind the night-8 CI flake (run + // 32580694134): `capabilities` is typed required, but a minimal plugin + // definition (or a leaked test fixture) can ship without it at runtime. + // The resolver enumerates every registered kind, so one capabilities-less + // entry must read as "no top surface" — not crash the election. + const restoreRegistry = nodeRegistry._snapshot() + try { + registerNode({ + kind: 'plugin-minimal-capless', + schemaVersion: 1, + schema: z.object({}), + category: 'structure', + defaults: () => ({}), + // No `capabilities` — deliberately. + } as unknown as AnyNodeDefinition) + addPluginPlatform() + + const camera = new PerspectiveCamera() + camera.position.set(0, 5, 0) + camera.updateMatrixWorld(true) + + const support = resolvePointerSupportSurface(camera, [0, 0, 0], { + includeNodeTopSurfaces: true, + }) + + // No throw, and the election still works: the capabilities-less kind is + // skipped while the platform's declared top is found as usual. + expect(support?.sourceNodeId).toBe(PLATFORM_ID) + expect(support?.elevation).toBeCloseTo(2) + } finally { + restoreRegistry() + } + }) }) diff --git a/packages/editor/src/components/tools/shared/pointer-support-cap.ts b/packages/editor/src/components/tools/shared/pointer-support-cap.ts index 3af19b7b2..9b10855b1 100644 --- a/packages/editor/src/components/tools/shared/pointer-support-cap.ts +++ b/packages/editor/src/components/tools/shared/pointer-support-cap.ts @@ -158,9 +158,14 @@ export function resolvePointerSupportSurface( // lifts anything placed inside a finished room. Only the tools that build ON // a surface (wall / column / fence / stair / block) mean that, and they say // so. Everything else places against the floor the pointer indicates. + // + // `capabilities` is typed required on NodeDefinition, but this enumerates + // EVERY registered kind — including plugin bundles that bypass the type at + // runtime. A minimal definition without `capabilities` must read as "no top + // surface", not crash the resolver (night-8 CI, run 32580694134). const nodeTopSurfaceKinds = options?.includeNodeTopSurfaces ? Array.from(nodeRegistry.entries()) - .filter(([, definition]) => definition.capabilities.surfaces?.top !== undefined) + .filter(([, definition]) => definition.capabilities?.surfaces?.top !== undefined) .map(([kind]) => kind) : [] if (nodeTopSurfaceKinds.some((kind) => (sceneRegistry.byType[kind]?.size ?? 0) > 0)) { From 14eb77a2f27ee18151a6557fc044da32361ba1ad Mon Sep 17 00:00:00 2001 From: Julien Brissonneau Date: Sat, 22 Aug 2026 23:41:14 -0400 Subject: [PATCH 4/4] editor tests: pin empty-scene context in apply-alignment tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaced by the randomized-order verification for the registry-leak fix (bun test --randomize --seed=22): the test assumes no active building, but the scene/viewer store singletons can carry a selected building fixture leaked by an earlier test file — getActiveBuildingPose then crashes on the fixture's missing rotation array. Same order-dependent pollution class as the night-8 registry flake, different singleton. Co-Authored-By: Claude Fable 5 --- .../src/lib/floorplan/apply-alignment.test.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/lib/floorplan/apply-alignment.test.ts b/packages/editor/src/lib/floorplan/apply-alignment.test.ts index d9355b4b2..d10036a5c 100644 --- a/packages/editor/src/lib/floorplan/apply-alignment.test.ts +++ b/packages/editor/src/lib/floorplan/apply-alignment.test.ts @@ -1,8 +1,23 @@ -import { afterEach, describe, expect, test } from 'bun:test' +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { useScene } from '@pascal-app/core' +import { useViewer } from '@pascal-app/viewer' import useAlignmentGuides from '../../store/use-alignment-guides' import { applyFloorplanAlignment } from './apply-alignment' describe('applyFloorplanAlignment', () => { + beforeEach(() => { + // These tests assume no active building (alignment runs on world axes). + // The scene/viewer stores are process-wide singletons, so an earlier test + // FILE can leak a selected building fixture into them — under bun's + // platform-dependent file order that turned into an order-dependent + // failure (getActiveBuildingPose reading a fixture building without a + // rotation array). Pin the empty-scene context explicitly. + useScene.setState({ nodes: {} } as never) + useViewer.setState({ + selection: { buildingId: null, levelId: null, zoneId: null, selectedIds: [] }, + } as never) + }) + afterEach(() => { useAlignmentGuides.getState().clear() })