Uh oh!
There was an error while loading. Please reload this page.
test(config): state the unit project's real isolate: false invariant, and enforce it - #7309
Merged
Merged
Conversation
…ant, and enforce it `vitest.config.mts` justified the `unit` project's `isolate: false` with "node-env pure logic with no ComponentRegistry or DOM state to leak across files". The premise was false in both directions: the project holds files whose import closure registers into the `ComponentRegistry` singleton AND files that assert a key is ABSENT from it, and under a shared module graph each is visible to the other. Measured on eb33a8d: the project's 810 files import 600 distinct specifiers whose closures register 502 keys into the singleton. No registered key collides with an asserted-absent key today, so the defect is latent — but the outcome of a collision is order-dependent, so it would arrive as a failure in a file that did nothing wrong, in some shards and not others. The comment now states the constraint that actually has to hold and names where it is enforced. The enforcement is a new gate that derives both populations from the config's own `include`/`domTsTests` on every run, EXECUTES the import closures in a fresh module graph to learn what they register (the live field path registers from data, so the writers' keys appear in no `register(...)` call site anywhere), and fails naming both files and the key. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
…es import A file that registers what it then asserts absent is hermetic, not a collision. Bisecting the whole union let the reader's own closure answer for the key, which would have reported a false red (and, in the ablation, hidden a true one behind the reader's own import of the same module). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
… gate `tsconfig.scripts.json` has no path mapping into the workspace packages, so a static specifier failed `type-check:scripts` (TS2307). Every other import in the file already goes through a computed id. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
This was referenced Sep 2, 2026
yinlianghui
marked this pull request as ready for review
September 2, 2026 05:43
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 2, 2026
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 freeto 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.
Fixes#7134
The
unitproject'sisolate: falsewas justified by a premise that is false: "node-env pure logic with no ComponentRegistry or DOM state to leak across files". This PR replaces that justification with the constraint that actually has to hold, and ships the gate that enforces it.Both populations, re-derived on this branch
Derived from
vitest.config.mtsitself — the project's ownincludelist minus its owndomTsTests— so the population is the project, not a hand-copied guess. It reconciles exactly with what Vitest collects: 811 files, againstTest Files 811 passed (811)from the full run below.WRITERS — measured by EXECUTION, not by grep. Fresh module graph (
vi.resetModules()), import@object-ui/core, snapshotComponentRegistry.getAllTypes(), import the project's import specifiers, diff. Overec0a7b846the population's 551 distinct resolved modules register 505 keys into the shared singleton, across 28 namespaces (ui133, bare 215,field47,view14,record13,element10, ...).Grep cannot answer this half, and that is why the gate executes. The live field path registers from data —
registerAllFields()walks a map — sofield:multiselectexists at runtime and appears in noregister('field:multiselect')call site anywhere in the repo. A static reader would report "nothing registers it" and the gate would be green for the empty reason.The one thing execution cannot see is a registration made in a test BODY (it happens when the test runs, not when it is imported). Those are read off the TypeScript AST: 1 file,
packages/runner/src/plugin-integration.test.ts(test-kanban-manual,test-bar-chart-manual). The AST matters here — a raw-text scan reported 12 such files, 11 of them registrations written inside fixture template literals, which are source to a regex and not registrations.READERS — files asserting a key ABSENT. Also off the AST: an
expect(...)whose argument reads the singleton, followed by a matcher that asserts the subject is not there (toBeUndefined/toBeFalsy/toBeNull, and.not.toBeDefined/.not.toBeTruthy), with template keys resolved through local string consts.packages/fields/src/__tests__/capability-multiselect-retired.test.ts:81toBeUndefinedfield:capability-multiselectpackages/fields/src/__tests__/capability-multiselect-retired.test.ts:82toBeUndefinedcapability-multiselectpackages/app-shell/src/views/metadata-admin/previews/__tests__/exclusion-reason-truthfulness.test.ts:222toBeFalsyComponentRegistry.get(type), a key set derived at runtime fromPALETTE_EXCLUSIONS2 readers, 2 statically resolved keys, 1 unresolvable site. The unresolvable one is reported and PINNED rather than dropped: a new one fails the gate, so a place it goes blind is a decision instead of a silent shrink.
The seat's grep found 7 candidate files. Five are not readers of this singleton:
app-generator.test.tsmentions it only in a comment,component-deprecation-declaration.test.tsandreport-bare-key-ownership.test.tsassert over a LOCALRegistryinstance,plugin-editor/index.test.ts's absence matchers are aboutreadOnlydefaults, andtimeline-bare-key-ownership.test.tsasserts a meta FLAG (getMeta(...)?.skipFallback), not key absence.Latent, not live — confirmed. Neither
field:capability-multiselectnorcapability-multiselectis among the 505 registered keys, and neither is written by the one in-body writer. The absence assertion is also non-vacuous under the shared graph: the live path registers 47field:*keys, includingfield:multiselectand thefield:ownertombstone, with the retired one absent.One correction to the card
The card reads the hazard as a silent green ("if some other file registers that key first, this one goes green while proving nothing"). Measured,
toBeUndefined()goes red when the key is registered — the ablation below shows exactly that. The defect is not a direction, it is ORDER DEPENDENCE: which of the two files the worker ran first decides the outcome, and neither outcome is information about the code under test. The invariant, and the fix, are unchanged.The gate: chosen shape, and the one rejected
Chosen (A): a collision gate.
scripts/__tests__/unit-registry-absence-collision.test.tscomputes both populations on every run and fails when a key asserted absent by one file is registered by another, naming both files and the key. It satisfies the dispatch's criterion (i) literally: it goes red on a planted collision.Rejected (B): an isolation-proof pattern plus a style gate (every absence assertion resets the module graph and re-imports only its own subject; a gate fails when one does not). Rejected for three measured reasons, not for taste:
exclusion-reason-truthfulness.test.ts) deliberately imports six renderer leaves instead of the package barrel because the barrel costs 6105 ms against 553 ms (its own measurement, finding(app-shell):exclusion-reason-truthfulness's import set excludes app-shell, so a false "no renderer" on a shell singleton passes green #7117). Making its dynamic-key absence loop hermetic means re-importing ten specifiers after each reset.And (A) turned out to be nearly free, which is the measurement that decided it. Its cold standalone cost is ~43 s, but the modules it loads are overwhelmingly the ones the project it measures already loads into the same worker under
isolate: false. Full-project wall clock: 173.52 s before, 182.16 s after — +8.6 s, about +5% (shared-box seconds; four sibling agents build in this container, so this is an upper-ish bound).Moving registry-touching files out of the shared-graph project — the dispatch's fallback — was not needed and is not done: the gate is non-vacuous, so the 3.2x is kept on every file.
Why the gate does not pollute the project it measures
It runs inside the very project it is about, so its own imports would otherwise be the single largest registry write in it.
vi.resetModules()before the measurement gives it a private module graph — a fresh@object-ui/core, therefore a fresh singleton, not the one its worker's other files hold — and this is asserted, not assumed (startedEmpty). A second reset afterwards drops that graph so files running later re-import their own. Evidence it works: the full project is green with the gate in it, all 811 files.Attribution runs only on the red path, and only over the specifiers some other file imports — a file that registers what it then asserts absent is hermetic, not a collision, and reporting one would be a false red.
The corrected comment
vitest.config.mts, theunitproject. The perf measurement is kept (it is still true); the false premise is replaced by the invariant, the order-dependence is named, and the justification points at where the constraint is enforced:isolateitself is untouched. The gate asserts that this comment still names it, so the justification and its enforcement cannot drift apart silently.Ablation
Every mutation is proven ON DISK by marker count and
git hash-objectbefore the run; every restore is proven by blob hash equal to the HEAD blob, marker count back to 0, and an emptygit diff HEAD— never by an exit code. Each leg carries atrap ... EXIT INT TERMrestoring by ABSOLUTE path.'capability-multiselect'added toRETIRED_FIELD_TYPESinpackages/core/src/utils/retired-field-types.ts— a DATA-driven registration, invisible to any static readerbb80173btoe9d313de"field:capability-multiselect", asserted absent bycapability-multiselect-retired.test.ts, registered bypackages/app-shell/src/__tests__/spec-symbol-parity.test.ts(viapackages/app-shell/src/views/ScreenView.tsx)ComponentRegistry.register('capability-multiselect', ...)in the body ofpackages/runner/src/plugin-integration.test.ts3273700btob70843a5"capability-multiselect", asserted absent bycapability-multiselect-retired.test.ts, registered bypackages/runner/src/plugin-integration.test.tscollectFilesforced to return[]inscripts/unit-registry-collision.mjs0dbb587btoed0c4f98a population COLLAPSED - this run proves nothing:with all five census counts at 0Restores: A1
bb80173b= HEAD blob; A23273700b= HEAD blob; A30dbb587b= HEAD blob,git diff HEAD0 bytes,git status --porcelainempty.A1 is the leg that proves the EXECUTION half is load-bearing: the planted key is registered from a frozen data table, so a grep-based gate would have stayed green on it. A3 is the gate's own control — an empty population must FAIL, not pass — and the file also carries the pure-function form of it (
checkFloors({})reportsNOT MEASURED, never zero).The gate additionally carries fixture controls for its readers: a registration written inside a template literal is NOT counted as a registration,
.not.toBeDefined()counts as absence while plain.toBeDefined()does not, a dynamic registration key is reported rather than dropped, andregister(type, C, { namespace: n })yields bothn:typeand the bare fallback (onlyn:typeunderskipFallback), matchingRegistry.register.One honest property of the red message: attribution bisects for a specifier whose closure registers the key, so in A1 it named
ScreenView.tsxrather than the more obvious@object-ui/fields. Both statements are true; the message names a file and a module an author can act on, which is what it is for.Verification
Full unit project, from the repo root, both runs under the shared verify lock (
os-verify-lock, VERDICTcommand-exit 0for each):The controlled pair — same tree, with and without this gate:
eb33a8d4c(no gate)Test Files 810 passed (810)/Tests 12610 passed | 9 skipped (12619)e41a20cc8(gate in)Test Files 811 passed (811)/Tests 12625 passed | 9 skipped (12634)The delta is +1 file and +15 tests, which is exactly this gate, and +8.6 s of wall clock.
And the final head, after merging
origin/mainat9bf0abfec— not a controlled comparison, since the sibling work merged in brings its own tests:6044abb9fTest Files 811 passed (811)/Tests 12652 passed | 9 skipped (12661)Gates, re-run on the final head
6044abb9f; every exit code captured by redirect-then-$?, never through a pipe:pnpm exec vitest run --project unit --maxWorkers=2(under the lock, at6044abb9f)command-exit 0—Test Files 811 passed (811)pnpm exec vitest run --project unit ... unit-registry-absence-collision.test.tsTest Files 1 passed (1)/Tests 15 passed (15)pnpm type-check:scriptspnpm check:control-bytespnpm check:entry-guardnode scripts/check-changeset-presence.mjseslint --no-inline-config --format jsonon the changed filestype-check:scriptswas red on the first attempt (TS2307: Cannot find module '@object-ui/core'—tsconfig.scripts.jsonhas no path mapping into the workspace packages) and is fixed in the gate rather than in the tsconfig:@object-ui/coreis imported by computed specifier, like every other import in that file. Changing the scripts tsconfig to type one line would have put everyscripts/file on a different module resolution than the one CI type-checks them with.Declared narrowing. The repo-wide
pnpm lintis CI's run; the lint here is narrowed to the diff, and the narrowing is measured rather than asserted: (1) the population comes from ESLint's own configuration, which reportsvitest.config.mtsas "File ignored because no matching configuration was supplied" — it is outside the configured population, not skipped by me; (2) the count comes from--format json: 3 paths requested, 2 linted, 0 errors, 1 warning (that ignore notice); (3)eslint.config.jscontains 0 occurrences ofprojectService/parserOptions/project:— type-aware linting is not enabled, so this diff cannot move the verdict on any untouched file.Changeset: none owed, and the gate says so in its own words (quoted above). No package's published source or contract changed — the diff is one config comment plus two new files under
scripts/. Noskip-changesetlabel: in this repo that label is inert.Serial constraint (#7291 / #7183)
origin/mainis merged (9bf0abfec, no conflict; the final head is6044abb9f). PR #7291 appends adistproject and two constants to the same file and had not landed within the dispatch's 20-minute poll budget (14 polls ofgit ls-remote origin refs/heads/mainfrom 05:03Z to 05:22Z; main moved once, to #7294, which is docs-only). It is still an open DRAFT,mergeable_state: behind. Its hunks and this one are disjoint (its constants sit abovedefineConfig, its project appends to the end ofprojects; this change is inside theunitproject block), and that was verified rather than assumed:git merge-tree --write-treeagainst its head857c8afc5produced a clean tree, exit 0, and this gate's config reader parses that merged config correctly (isolateFalse: true, the same fourincludeglobs, the same 18domTsTests, and both changes present). No hunk of another PR was resolved by hand.https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
Generated by Claude Code