Skip to content

[Bugs] core, nodes: tests no longer leak registry and store state across files - #1120

Merged
Aymericr merged 4 commits into
mainfrom
cursor/test-isolation-registry-stores-c6fa
Oct 21, 2026
Merged

Aymericr merged 4 commits into
mainfrom
cursor/test-isolation-registry-stores-c6fa

Conversation

@Aymericr

@Aymericr Aymericr commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Fixes the quality job on main, which has been red since #903. It also fixes 22 order-dependent nodes tests.

Root cause on CI (core, 5 verify_scene tests). Bun runs all of a package's test files in one process, in filesystem enumeration order. That order differs between machines and changes when a file is added. verify_scene looks up a kind's schema in the node registry before falling back to core's own schemas. services/drag-session.test.ts and services/alignment-anchors.test.ts register a stub wall kind with schema z.object({ type: z.literal('wall') }) and never restore the registry. So when those files run before agent-operations/operations.test.ts, malformed walls pass validation: start: null, or a pool retyped as wall.

On the last green run, hosting.test.ts happened to run in between, and its beforeEach wiped the registry by accident. #903 changed no logic involved here. It only added a test file, which reordered the runner's file list so nothing cleaned up before operations.test.ts. Running core in CI's exact 336-file order reproduces exactly those 5 failures locally.

Changes

  • core: add a test preload (packages/core/test-setup.ts) that snapshots nodeRegistry before each test and restores it after. Preload hooks wrap each file's own hooks, so registrations made inside a test or its beforeEach (including after a _reset()) end with that test. Stub registrations made at file scope (top level or beforeAll), which a per-test hook can't reach, now restore in afterAll: validate-dwv, system-graph, port-connectivity, stair-surface-snap and procedural-items/mutations.
  • nodes, the 22 failures: wall-batch-system.test.ts set useViewer's selection.selectedIds and previewSelectedIds to Sets. The store holds arrays. It never restored them, so later stair, column and floor-plan tests threw on .includes. The test now uses arrays and restores the viewer state it started with.
  • nodes, leaks found while verifying:
    • The stair paint test now loads the built-in plugin itself instead of relying on an earlier test in its file.
    • cabinet/hosting and shelf-hosting start a mounted move session that writes the placement preview and alignment guides, and never commit or cancel it. They now clear both in afterEach. Leftover guides made a later stair tool mount render DOM labels.
    • Six hosting tests restored document by assignment, which left an own document: undefined property instead of removing it. They now restore by property descriptor, the way they already restore window.
  • nodes guardrail: a test preload (packages/nodes/test-setup.ts) snapshots and restores, around every test, the node registry and the useScene, useLiveNodeOverrides, useLiveTransforms, useViewer, useEditor, usePlacementPreview and useAlignmentGuides stores (the built @pascal-app/* instances).

No timeouts were raised and no tests were skipped.

Not done: a random-order CI step. bun test --randomize still finds a few latent dependencies unrelated to these fixes, so as a required gate it would flake today:

  • a test-to-test dependency inside core/src/lib/structure-kernel.test.ts (seed 12);
  • the core spatialGridManager singleton in nodes frame-pipeline.test.tsx (seed 6).

The per-test reset preloads are the guardrail in this PR.

How to test

  1. bunx turbo run build --filter=@pascal-app/nodes..., so core's and nodes' self-imports resolve to dist.
  2. In packages/core, run bun test ./src/services/drag-session.test.ts ./src/agent-operations/operations.test.ts. On main this reports the same 5 failures as CI; on this branch it reports 0 failures.
  3. In packages/core, run bun test src. Expected: 5414 pass, 0 fail. Under --randomize, seeds 1, 2, 3, 11, 13 and 14 also pass.
  4. In packages/nodes, run bun test. On main: 22 failures. On this branch: 5021 pass, 0 fail. Under --randomize, seeds 1, 2, 3, 4, 5, 7 and 8 also pass.

Screenshots / screen recording

N/A — non-visual change (tests only).

Checklist

  • I've tested this locally with bun dev
  • My code follows the existing code style (run bun check to verify)
  • I've updated relevant documentation (if applicable)
  • This PR targets the main branch
Open in Web Open in Cursor 

Note

Low Risk
Test-only harness and teardown changes; no production runtime behavior is modified.

Reviewed by Cursor Bugbot for commit 7314993. Bugbot is set up for automated code reviews on this repo. Configure here.

verify_scene reads a kind's schema from the node registry before core's
own. drag-session and alignment-anchors tests register a stub 'wall'
(z.object({ type: z.literal('wall') })) and never restore it; when bun
enumerates those files before operations.test.ts, a wall with start: null
validates and five verify_scene tests fail. #903 only added a file, which
reordered the CI runner's file enumeration so nothing reset the registry
in between.

A test preload snapshots the registry before each test and restores it
after, so in-test and beforeEach registrations cannot outlive their test.
File-scope stub registrations (top level, beforeAll) restore in afterAll.
…s plugin

wall-batch-system.test.ts set selection.selectedIds and previewSelectedIds
to Sets (the store holds arrays) and never restored them, so 22 later
stair, column and plan tests failed on .includes. It now uses arrays and
restores the viewer state it found. The stair paint test relied on an
earlier test in its file to load the built-in plugin.
…ment

The mounted move session writes the placement preview and alignment guides
and these tests never commit or cancel it, so a later stair tool mount
rendered guide labels needing a DOM. Restoring document by assignment left
an own document: undefined instead of removing it.
A test preload snapshots the node registry and the scene, live override,
live transform, viewer, editor, placement preview and alignment guide
stores before each test and restores them after.
@Aymericr Aymericr added the bot:bugs label Oct 21, 2026 — with Cursor
@Aymericr
Aymericr marked this pull request as ready for review October 21, 2026 22:26
@Aymericr
Aymericr merged commit f22bacb into main Oct 21, 2026
4 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-21T22:28:53.678037Z 7314993 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

cursor Bot pushed a commit that referenced this pull request Oct 23, 2026
#1120's test-setup.ts restores the node registry around every test, so the per-file afterAll
restores this branch added to eighteen core test files are gone: none of them registers at file
scope. stair-surface-snap's file-level restore goes too (main's): it registers only in tests.
Files that register at file scope keep theirs (mutations' beforeAll; port-connectivity,
system-graph and validate-dwv through a top-level stubDef).

registry-isolation.test.ts now reads each test file's syntax: a registry write at file scope
(top level, a describe body, beforeAll, or a call there to the file's own writing helper) needs
the file's own snapshot; writes in tests, beforeEach and helpers they call are the shared
setup's.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants