Fix order-dependent pointer-support flake: registry leaks across test files - #709
Merged
Conversation
…n 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The flake
Night-8 private-editor CI (run 32580694134) failed three tests in
packages/editor/src/components/tools/shared/pointer-support-cap.test.ts:discovers a plugin-declared top surface without a kind-name listnever elects the node the active interaction is placing or movinguses a plugin-declared top as the wall construction surfaceall with
TypeError: undefined is not an object (evaluating 'definition.capabilities.surfaces')atpointer-support-cap.ts:163. The same tree passed 752/752 locally twice (forced).Root cause: order-dependent test pollution
nodeRegistryis a module singleton and bun runs a package's test files sequentially in one process.floorplan-registry-layer.test.tsregisters capabilities-less fixture definitions (cabinet,cabinet-module,linked-floorplan-test) after anodeRegistry._reset()and never restores. Bun's test-file order differs between platforms (macOS local vs CI Linux) — when the floorplan file runs beforepointer-support-cap.test.ts, the leaked entries hit the top-surface enumeration:and only the three tests that pass
includeNodeTopSurfaces: truecrash — exactly the CI signature.Reproduced deterministically on unmodified
mainwithbun test --randomize --seed=1on the two-file pair: same three tests, same TypeError, same line.The fix (both sides + a gate)
nodeRegistry._snapshot()(captures definitions + plugin bookkeeping, returns a restore fn). Applied in:floorplan-registry-layer.test.ts— both mutation sites now snapshot/restorewall-drafting.test.ts— the mid-test_reset()no longer strips the registry for later filesdefinition.capabilities?.surfaces?.top.capabilitiesis typed required, but the resolver enumerates every registered kind and a plugin bundle can bypass the type at runtime; a minimal definition must read as "no top surface", not crash the election.resolvePointerSupportSurface: no throw, and the platform's declared top is still elected. Verified failing against the unguarded code, passing with the chain.Bonus: seed-22 randomized verification surfaced a second latent flake of the same class (
apply-alignment.test.tsinheriting a leaked building fixture through the scene/viewer store singletons — confirmed pre-existing on unmodifiedmain). Pinned its empty-scene context explicitly.Verification
bun test --randomizex10 fresh seeds: 753 pass / 0 fail eachbunx turbo run test --filter=@pascal-app/editor --force: 753/753 (752 + new gate test)bunx turbo run test: 13/13 tasks greencheck-types(editor + core) and Biome on changed files: clean🤖 Generated with Claude Code
Note
Low Risk
Mostly test isolation plus optional chaining on a registry enumeration path. Production behavior only changes for malformed/minimal definitions that previously threw.
Overview
Stops order-dependent CI flakes from tests mutating the process-wide
nodeRegistry(and scene/viewer stores) without restoring them. Bun runs a package’s tests in one process, so a leaked capabilities-less fixture crashed laterpointer-support-capfiles when file order differed on Linux.Adds test-only
nodeRegistry._snapshot()to capture definitions plus plugin bookkeeping and restore them. Floorplan registry and wall-drafting tests now snapshot/restore around_reset()and throwaway registrations.resolvePointerSupportSurfacenow uses optional chaining oncapabilities, so a kind without that field is skipped instead of throwing. A gate test registers a capless definition and still elects the platform top. Alignment tests also pin an empty scene/selection so a leaked building fixture cannot break world-axis alignment.Reviewed by Cursor Bugbot for commit 14eb77a. Bugbot is set up for automated code reviews on this repo. Configure here.