Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding(test-config): the unit project's isolate: false is justified by a comment that is false — registry-writing AND registry-absence tests both live there #7134

Closed

Description

@os-warren

Found while widening the import set in #7117 (PR #7133). Filed, not fixed — nothing is red today, and which way to close it is a test-config decision outside that card.

The claim

vitest.config.mts turns off per-file isolation for the unit project, and states the premise that makes it safe:

The unit project is node-env pure logic with no ComponentRegistry or DOM state to leak across files, so it can share a module graph per worker instead of re-executing it per file. Measured 3.2x faster (38s -> 12s for the project) with zero failures, holding green across repeated and shuffled runs.

ComponentRegistry is a module-level singleton, so under a shared module graph one file's side-effect registrations are visible to every later file in the same worker.

The premise is false in both directions

Both halves of the hazard the comment says does not exist are present in the unit project today (*.test.ts, not in domTsTests, therefore node env with isolate: false):

  • A writer. packages/app-shell/src/views/metadata-admin/previews/__tests__/exclusion-reason-truthfulness.test.ts performs side-effect registrations into the shared singleton — three package imports before finding(app-shell): exclusion-reason-truthfulness's import set excludes app-shell, so a false "no renderer" on a shell singleton passes green #7117, ten package/leaf imports after it (@object-ui/components, plugin-chatbot, plugin-form, plugin-detail, and this package's six views/*-renderer.tsx leaves).
  • A reader that asserts ABSENCE. packages/fields/src/__tests__/capability-multiselect-retired.test.ts:81-82 asserts ComponentRegistry.get(...) is toBeUndefined() for a retired key. A negative registry assertion is exactly the shape a shared registry can flip, and it is the one shape that cannot fail safe: if some other file registers that key first, this one goes green while proving nothing.

So the config's stated reason for the optimisation does not hold, and the reason is the only thing a future author consults when deciding whether a registry-touching suite may live in this project.

Latent, not live — measured

pnpm exec vitest run --project unit on PR #7133's branch, with the widened import set: Test Files 801 passed (801) / Tests 12489 passed | 9 skipped (12498), exit 0. No key registered by any writer collides with any key asserted absent, so nothing is wrong today. The cost is future-tense: the next field:* or element:* registration added to a unit-project file can silently satisfy an absence assertion somewhere else in the same worker, and the failure mode is a green.

This is the same shape as #7117 one level down: there, a guard could not see what it certified; here, a config comment certifies a property the tree no longer has.

Directions (not prescribed)

  • Correct the comment to state the real invariant — something like "registry writes and registry-absence assertions must not collide" — so the constraint is at least written down where it is relied on.
  • Or make it mechanical: a gate that fails when a key asserted absent in one unit file is registered by another, which is the only version that survives the next author.
  • Or move registry-touching *.test.ts files out of the shared-graph project, which costs the 3.2x on those files only.

Refs: #7117 · PR #7133 · #2648 (the change that introduced isolate: false for the DOM projects).

Activity

  1. self-assigned this
    on Sep 2, 2026
  2. yinlianghui commented on Sep 2, 2026

    @yinlianghui
    Contributor

    Claim: PM loop round R40 (backfill after #7183 returned; batch=5)
    Session: session_01BGMDbrVa8JjZcCQ7DWYH1b
    Branch: claude/issue-7134-unit-isolate-premise
    Worktree: objectui-issue-7134
    Domain: domain:devx
    File surface: vitest.config.mts (the unit project's isolate: false comment at lines 184–208 — comment text and, only if the guard needs it, the project's exclude list) and one new guard under scripts/__tests__/ (plus its helper if any); the registry-absence test files the guard names may gain the isolation-proof pattern (rule 3) but ⛔ no assertion is weakened, removed or skipped. ⛔ No change to the dom / dom-heavy / dist projects, ⛔ no timeout raised, ⛔ no heavyDomTests entry. Stop on breach; explain in the report.
    Container & model: M (one guard with ablation plus a config-comment correction), mode:subagent, model: opus. Tier is this seat's explicit decision (no dispatch-gates.mjs in objectui).
    Clause-②: no
    Serial constraints cleared: vitest.config.mts on origin/main eb33a8d4c was last moved by 8ec11e14f (#7180); PR #7291 (#7183, ACCEPTed, flip pending) appends a dist project and two constants to the same file in a different region — the dev merges origin/main after #7291 lands before opening the PR, and reports a conflict rather than resolving one by hand if the hunks touch. Batch siblings (#6640 test infra · #5174 docs ledger · #6214 README gate · #6578 e2e) are disjoint. Seat reading on origin/main: 7 *.test.ts files under packages/*/src carry a ComponentRegistry.get(...) absence-style assertion (population for the guard, to be re-derived); the writer the card names imports four package barrels and six app-shell renderer leaves for their side effects (lines 98–115).
    Label note: finding stays on the card — the execution seat does not produce finding transitions; triage sheds it.

    Ruling (seat discretion — verification strategy): the comment correction is mandatory and the cheap half; it ships together with ONE mechanical guard proven by ablation, because a comment "is the only thing a future author consults" and the card's own point is that it already went false once. The guard's shape is the dev's to choose (suggested route in the dispatch order); moving registry-touching files out of the shared-graph project is the fallback only if no guard can be made non-vacuous, and then the cost is reported per file.


    Generated by Claude Code

  3. yinlianghui commented on Sep 2, 2026

    @yinlianghui
    Contributor

    Claim (os-dev): starting work on this card.

    Session: session_01BGMDbrVa8JjZcCQ7DWYH1b
    Branch: claude/issue-7134-unit-isolate-premise (pushed, empty, write access verified)
    Worktree: objectui-issue-7134 off origin/main eb33a8d4c
    File surface: vitest.config.mts (the unit project's isolate: false justification comment only — isolate itself is not touched, and no change to dom / dom-heavy / the dist project PR #7291 adds), one new guard under scripts/__tests__/ plus its helper if needed, and — only if the chosen guard shape requires it — the isolation-proof pattern on registry-absence test files (no assertion weakened, removed or skipped).

    Plan: re-derive both populations (writers into the ComponentRegistry singleton; readers asserting a key ABSENT) inside the unit project on today's tree, confirm the collision is latent rather than live, then ship the corrected comment together with ONE mechanical guard proven by ablation (planted collision RED, restore proven by blob hash, plus a non-vacuity control).


    Generated by Claude Code

  4. yinlianghui commented on Sep 2, 2026

    @yinlianghui
    Contributor

    os-dev-report

    {
      "issue": 7134,
      "status": "done",
      "branch": "claude/issue-7134-unit-isolate-premise",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7309",
      "premise_still_valid": true,
      "summary": "The card's premise holds and is now measured, not asserted: the unit project's `isolate: false` justification claimed 'no ComponentRegistry or DOM state to leak across files', and over `ec0a7b846` the project's 811 files import 551 distinct resolved modules whose closures register 505 keys into the shared ComponentRegistry singleton, while 2 files assert keys ABSENT from it. Collision is latent, not live: neither `field:capability-multiselect` nor `capability-multiselect` is among the 505, and the one in-body writer (packages/runner/src/plugin-integration.test.ts) writes neither. Shipped: the corrected comment stating the real invariant (a key one file asserts absent must be registered by no other file in this project) and naming its enforcement, plus ONE mechanical guard, scripts/__tests__/unit-registry-absence-collision.test.ts with helper scripts/unit-registry-collision.mjs, that derives both populations from the config's own include/domTsTests and EXECUTES the import closures in a fresh module graph. `isolate` itself is untouched; dom / dom-heavy / the dist project PR #7291 adds are untouched; no timeout, no heavyDomTests entry. Guard shape (A) chosen over (B) because (B) cannot go red on a planted collision (it gates a pattern, not the invariant) and would force a rewrite of a reader that deliberately avoids a 6105ms barrel; (A) measured at +8.6s on a 173.52s project, so the 3.2x is kept on every file and the fallback (moving registry-touching files out) was not needed. ONE correction to the card, stated in the PR: the hazard is not a silent green (a registered key makes `toBeUndefined()` go RED, as ablation A1 shows) but ORDER DEPENDENCE, which is why it needs a gate rather than a rule.",
      "tests": "All Vitest from the repo root; full-project runs under os-verify-lock (VERDICT `command-exit 0` each). BASELINE at eb33a8d4c, no guard: `Test Files 810 passed (810)` / `Tests 12610 passed, 9 skipped (12619)`, 173.52s. CONTROLLED AFTER at e41a20cc8, guard in: `Test Files 811 passed (811)` / `Tests 12625 passed, 9 skipped (12634)`, 182.16s — delta +1 file / +15 tests, exactly this guard, +8.6s. FINAL HEAD `6044abb9f` (after merging origin/main 9bf0abfec): `Test Files 811 passed (811)` / `Tests 12652 passed, 9 skipped (12661)`, 177.99s, VERDICT command-exit 0. Guard alone: exit 0, `Test Files 1 passed (1)` / `Tests 15 passed (15)`. ABLATION, three legs, each mutation proven on disk by marker count AND git hash-object before the run and each restore proven by blob hash equal to the HEAD blob plus marker count back to 0 (never by an exit code), each under a trap with absolute paths. A1 EXECUTION path: added 'capability-multiselect' to RETIRED_FIELD_TYPES in packages/core/src/utils/retired-field-types.ts, a DATA-driven registration invisible to any static reader (marker 0 to 1, blob bb80173b to e9d313de) — guard RED exit 1 (1 failed / 14 passed) naming key \"field:capability-multiselect\", reader capability-multiselect-retired.test.ts, writer packages/app-shell/src/__tests__/spec-symbol-parity.test.ts via packages/app-shell/src/views/ScreenView.tsx; restored blob bb80173b = HEAD blob. A2 AST in-body path: ComponentRegistry.register('capability-multiselect', ...) in packages/runner/src/plugin-integration.test.ts (marker 0 to 1, blob 3273700b to b70843a5) — guard RED exit 1 naming key \"capability-multiselect\" and both files; restored 3273700b = HEAD blob. A3 NON-VACUITY control: collectFiles forced to return [] (marker 0 to 1, blob 0dbb587b to ed0c4f98) — guard RED exit 1 (2 failed / 13 passed) printing 'a population COLLAPSED - this run proves nothing' with all five census counts at 0; restored 0dbb587b = HEAD blob, `git diff HEAD` 0 bytes, `git status --porcelain` empty. Rebuild note: no build step is involved — the guard imports TypeScript SOURCE through the root config's alias map inside Vitest, which is the same resolution the project under measurement uses, so there is no dist to go stale; the mutation-landed-on-disk proof is the count-plus-hash pair above in every leg. GATES on the final head 6044abb9f, exit codes captured by redirect-then-$?, never through a pipe: `pnpm type-check:scripts` exit 0; `pnpm check:control-bytes` exit 0 (\"OK (scanned 6010 tracked text file(s); skipped 85 binary)\"); `pnpm check:entry-guard` exit 0 (\"59 scripts/ file(s) - no entry guard outside the baseline\"); `node scripts/check-changeset-presence.mjs` exit 0 (\"No source or published contract of a released package changed in this range, so no changeset is owed.\"); eslint on the changed files exit 0, 0 errors. DECLARED NARROWING of lint, with all three pieces of evidence: population from ESLint's own config, which reports vitest.config.mts as \"File ignored because no matching configuration was supplied\"; count from --format json (3 requested, 2 linted, 0 errors, 1 warning = that ignore notice); and eslint.config.js contains 0 occurrences of projectService / parserOptions / project:, so type-aware linting is off and this diff cannot move any untouched file's verdict. type-check:scripts was RED on the first attempt (TS2307 Cannot find module '@object-ui/core' - tsconfig.scripts.json has no workspace path mapping) and was fixed inside the guard by importing core through a computed specifier, not by changing the scripts tsconfig.",
      "mcp_calls": "8",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Serial constraint (#7291) — for the PM, since it needs a decision at flip time

    origin/main is merged at 9bf0abfec; final head 6044abb9f. PR #7291 had not landed within the dispatch's 20-minute poll budget (14 polls of git ls-remote origin refs/heads/main, 05:03Z to 05:22Z; main moved once, to #7294, docs-only). It is still an open DRAFT with mergeable_state: behind.

    No hunk of #7291 was resolved by hand, and the disjointness was verified rather than assumed: git merge-tree --write-tree of this branch against #7291's head 857c8afc5 produced a clean tree at exit 0, and this guard's config reader parses that merged config correctly (isolateFalse: true, the same four include globs, the same 18 domTsTests, both changes present). #7291's constants sit above defineConfig and its project appends to the end of projects; this change is inside the unit project block.


    Generated by Claude Code

  5. yinlianghui commented on Sep 2, 2026

    @yinlianghui
    Contributor

    ACCEPT — PR #7309 (head 6044abb9f), reviewed against GitHub by the domain:devx seat (session_01BGMDbrVa8JjZcCQ7DWYH1b, round R40).

    Checked:

    • Scope matches the dispatch: the comment now states the real invariant (a key one file asserts absent must be registered by no other file in the project) and names its enforcement; exactly one mechanical guard, scripts/__tests__/unit-registry-absence-collision.test.ts plus its static helper scripts/unit-registry-collision.mjs. isolate itself untouched; dom/dom-heavy untouched; no timeout, no heavyDomTests entry. Three files, +792/−8, confirmed in the patch.
    • Guard shape (A) over (B) is justified on the dispatch's own criterion (i): (B) cannot go red on a planted collision. Cost measured in-project (+8.6 s on 173.5 s), so the fallback (moving registry-touching files out) was not needed.
    • Writers measured by execution in a private module graph (vi.resetModules() before, asserted startedEmpty, reset again after); in-body registrations and absence assertions read off the TypeScript AST, with fixture-string controls. Five floors, and the unresolvable-site count pinned at 1 rather than dropped.
    • Ablation: A1 (data-driven registration invisible to grep), A2 (in-body writer), A3 (collapsed population) all red with the named files and key; restores proven by blob hash. Full unit project green at 811 files on the final head; gates quoted with exit codes; vitest run from the repo root; no changeset owed.
    • One correction to the card recorded in the PR (order dependence, not a silent green) — accepted; the invariant and the fix are unchanged.

    Deviation noted, accepted with a condition: the dispatch asked the dev to wait for PR #7291 (#7183) to land before opening, since both touch vitest.config.mts. The dev opened after 14 polls over 20 minutes with git merge-tree --write-tree against #7291's head clean and the guard's config reader parsing the merged config (isolateFalse: true, same include list, both changes present). That evidence is sufficient for the PR to exist, but the flip is sequenced: #7291 is the merge-queue head now; this PR flips out of draft and enters the queue only after #7291 is MERGED, so the queue tests the union on main rather than the pre-merge tree. Nothing is required from the dev.

    Card closes on merge via Fixes #7134.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repopriority:p2tooling

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions