Uh oh!
There was an error while loading. Please reload this page.
test(app-shell): widen the DOM-leak sweep to packages/components renderers, with the first run's ledger - #5629
Conversation
…erers The sweep's discovery was four namespace prefixes wide, so the 158 types registered from `packages/components/src/renderers/**` (`ui:`, `element:`, `page:`, `action:`, `protocol-placeholder:`) sat outside the gate entirely. That is why `ui:grid`'s leak had to be found by hand rather than by this file. Widens the target set to all 158, and records what the first run finds as a ledger: 119 of 158 targets leak, in eight measured shapes, every renderer named. No target is skipped, quarantined or allow-listed — the per-target assertion stays exact set equality in both directions, so a fix cannot go green until its row goes. Also records four classes of phantom clean the widening surfaced: 12 targets rendered no element at all (closed overlays, `action:*` with no actions, `ui:icon` with a non-icon `name`) and 4 threw `useSidebar must be used within a SidebarProvider` into an attribute-clean error boundary. All 16 read clean before they were made to render. Renderer fixes are deliberately NOT folded in: the ledger is the record of the tree the gate arrived on, and a change that both widens and fixes leaves no reading to check the fixes against. Part of #5574
…othing Empty frontmatter, the repo's explicit "releases nothing" declaration. Measured rather than asserted: `packages/app-shell/dist` built twice from a cleared `dist/` and a cleared `tsconfig.tsbuildinfo` — once at the origin/main version of the changed test file, once at this branch's — and sha256-compared. 862 emitted files, 431 of them `.d.ts`, every hash equal. Part of #5574
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
commented
Aug 21, 2026
ACCEPT — |
| leg | perturbation | result |
|---|---|---|
| probe | one extra ui: registration into origin/main's sweep, verbatim via git show | green — the pre-widening completeness case never saw it |
| control | the same perturbation into the same sweep, only the namespace differs (plugin-dashboard:) | red, naming plugin-dashboard:zzprobe5574 |
| verification | the same ui: injection against this branch's gate | red, naming the missing target |
The control is at the probe's scope and it can fail — it did. That is the standing rule satisfied properly, and the third leg closes the loop my dispatch only asked half of: it proves the widened gate would now catch what the old one missed, rather than merely that the old one missed it.
Coverage before/after at identical scope, which is what I asked for and got: 0 of 86 → 86 of 86 renderer files, 0 of 158 → 158 of 158 registry types, 0 of 5 → 5 of 5 namespaces.
The changeset measurement carries a trap the lane did not know about
Empty-frontmatter changeset — this repo's explicit "publishes nothing" declaration, and the right form here. check-changeset-presence exited 1 first and 0 with the file, so the requirement was demonstrated rather than assumed. No skip-changeset label was invented or applied; there is none in this repo.
The dist measurement was taken rather than asserted: packages/app-shell/dist built twice, once per leg, sha256-compared — identical, 862 emitted files, 431 of them .d.ts. Mechanism named: tsconfig.json excludes **/*.test.tsx from the build program, so a test-only change cannot move emitted output.
⛔ And the trap, which is new to this lane's notes: the first attempt produced an empty dist/ and would have read as a false "identical". tsc is composite — with tsconfig.tsbuildinfo left in place it skips the emit entirely. Clearing dist/ alone is not enough; the build info has to go too, or you are comparing two empty directories and calling them equal.
That joins the two measurement traps this lane already carries — gzip storing the source filename in its header, and a zero byte-size delta over non-identical content (index.umd.cjs on #5626, differing at char 2083 with equal length). All three have the same shape: a measurement that returns the expected answer for the wrong reason.
The ledger, and the finding inside it
Covered in full on #5574. The two things worth restating on the PR itself:
It is a ledger, not an allowlist, by construction. Exact set equality in both directions — a listed renderer that leaks one more attribute fails, and one that stops leaking also fails until its row is deleted. Nothing is skipped, it.skip-ed or quarantined; all 158 targets render and are scanned every run. The three meta-cases (no renderer in two groups; every readiness override carries a reason and every reason an override; the four card-named renderers ledgered while ui:grid must not be) are what stop it decaying.
The 16 phantom cleans are the real discovery. First pass read 46 clean; 16 of those rendered nothing or threw into an attribute-clean error boundary, and once made to render real markup, 7 leaked. A DOM-attribute gate cannot tell "safe" from "absent" without a rendering-something precondition. Finding that before publishing the ledger is the difference between a census and a comfortable number.
Two details I would have missed and am recording: BARE_SPREAD_ON_SVG preserves camelCase because SVG attribute names are case-sensitive and a lowercased ledger would have dropped those two; and ui:sidebar-trigger leaks schema itself — the node SchemaRenderer injects on every render.
ui:grid clean means #4787 / PR #5573, which this lane landed in round 4, is now pinned by a gate from both sides instead of by hand.
Findings
#5630 and #5631 labelled finding + domain:ui, unassigned. #5631 got a note distinguishing it from #5586 and #5622 — three defects on one resolver, wrong schema key vs wrong name vs wrong resolver table, none a duplicate of the others.
Burn-down: I am not filing eight cards unilaterally. Reasoning and what I will file instead is on #5574.
Not counted as evidence
The bundle bot's PASS; the check_suite.completed bursts.
Landing now.
Generated by Claude Code
Uh oh!
There was an error while loading. Please reload this page.
Part of #5574 — the widening lands here; the per-renderer burn-down it enables does not, and that card stays open to own it. See "Fold or serial" below.
What changed
packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx— its discovery was four namespace prefixes wide, so the 158 registry-reachable types registered frompackages/components/src/renderers/**were outside the gate entirely. That is whyui:grid's leak had to be found by hand during #4011 / PR #4785 rather than by this file.Plus
.changeset/dom-leak-sweep-components-5574.md— empty frontmatter, the repo's explicit "publishes nothing" declaration.No renderer source is touched. No test is skipped, disabled or quarantined.
Evidence — the discovery probe and its control, at one scope
The gate's discovery mechanism is the per-package completeness case:
registeredTypesUnder(prefix)read offComponentRegistry.getAllConfigs(), compared with the enumerated targets. Both probes are the same perturbation (register one extra widget), against the same assertion, differing only in which namespace the widget lands in.Run against the pre-widening gate (
git show origin/main:of the sweep, one registration injected):packages/components/src/renderers/**ui:zzprobe5574plugin-dashboard, a prefix the gate does coverplugin-dashboard:zzprobe5574plugin-dashboard: every widget it registers is sweptfailed, namingplugin-dashboard:zzprobe5574The control is at the same scope as the probe and it can fail — it did. A control one level up (does the sweep run at all? does the file import
@object-ui/components?) could not have failed and would have proved nothing.Third leg, run against this branch's gate with the same
ui:perturbation, then reverted (git checkout HEAD -- <path>, injection confirmed on disk beforehand with a grep andgit diff --stat):So the perturbation the gate used to miss is now caught, by name.
Evidence — discovered counts, before and after, identical scope
Scope: types registered from
packages/components/src/renderers/**, and the source files under that path that register them.ui:,element:,page:,action:,protocol-placeholder:)The five prefixes are exclusively this package's: a workspace-wide grep finds
namespace: 'ui'outsidepackages/componentsonly in apackages/coredocstring and a standalonepackages/sdui-parser/verify.tsscript, neither of which registers into this registry at import time. So nothing foreign is swept into these counts.The ledger — 119 of 158 targets leak
Recorded, not fixed. Grouped by the mechanism that produces each shape, with every renderer named:
BARE_SPREADnameincluded — the renderer forwards its whole prop bag to the host elementBARE_SPREAD_MINUS_NAMEname, so HTML makes that one legitimateBARE_SPREAD_ON_SVGariaLabelsurvives as authoredaction:groupactions, the authored action list itselfaction:menuname-defining variant, plusactionsui:formFormRendererconsumesdataSource, andformdefinesnameui:sidebar-triggerschemaitself, the nodeSchemaRendererinjects on every renderui:grid, i.e. #4787 / PR #5573's fix now pinned by a gateThe four renderers #5574 named as candidates —
ui:flex,ui:stack,ui:container,ui:text— all leak. A dedicated case pins that, and pinsui:grid's absence from the ledger from the other side.Ledger, not allowlist, and the difference is mechanical rather than rhetorical: the per-target assertion is exact set equality in both directions. A listed renderer that starts leaking a fifteenth attribute fails the gate; a renderer that stops leaking also fails it, until its row is deleted in the same change. An allowlist has neither property. Two new cases keep that honest — no renderer may appear in two groups, and every readiness override must carry a recorded reason (and every reason an override).
The finding behind the finding: 16 phantom cleans
A first pass reported 46 clean targets. Sixteen were not clean — they had rendered nothing, and an empty scan reports no leaks:
defaultOpen;action:bar/action:group/action:menureturnnullwith no actions;ui:iconreturnsnullbecause the canary node'snameiscanary_node, which is not a lucide icon.useSidebar must be used within a SidebarProviderand were caught bySchemaErrorBoundary, whose markup is attribute-clean.All 16 now render real markup and are scanned; 7 of them turned out to leak. This is traps 1/3/4 from the file's own docblock, at scale, and it is why this family's readiness is an authored
classNamerather than 158 transcribed selectors — one string, uniform, and it caught all sixteen. The six targets it cannot cover carry their measured reason inREADY_OVERRIDE_REASONS, pinned two-way.Three of those six (
element:repeater,element:definition-list,element:metadata_viewer) are swept in a placeholder branch because the sweep's adapter answers with no records by design. Recorded rather than implied, and filed as a follow-up.Fold or serial — serial, deliberately
Renderer fixes are not folded in, and the answer is owed on #5574 rather than assumed:
toDomPropswhitelist stops atpackages/fields— SDUI widgets elsewhere still close their DOM leak by hand, if at all #4425 phase-2 pattern this file already grades is explicitly "measure, then burn down per package, deleting your own rows".toDomProps, times 119, in the most-shared package in the repo — the worst possible shape for a merge queue several agents land into.Verification
All at
ed1cbb162, the head of this branch, run after the final commit:pnpm exec vitest run packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsxTest Files 1 passed (1)/Tests 200 passed (200)pnpm --filter @object-ui/app-shell type-checktsc --noEmit && tsc -p tsconfig.test.json)pnpm --filter @object-ui/app-shell lintnode scripts/check-changeset-presence.mjs✅ … declares 1 changeset(s)… Every one of them has an EMPTY frontmatternode scripts/check-changeset-no-major.mjs✅ No changeset declares a major bump.node scripts/check-control-bytes.mjs✅ … scanned 4672 tracked text file(s)Vitest was run from the repo root throughout (#3378's guard; never package-cwd). The
packages/app-shellsuite is ~487 files and exceeds this container's foreground cap, so the sweep was run targeted; lint was run over the whole@object-ui/app-shellpackage — a provable superset of the diff at package granularity — reporting 918 files linted via--format json, with the changed file at 0 errors / 0 warnings.eslint.config.jsconfigures noproject/projectService, so linting here is not type-aware and this diff cannot move the verdict on any file it does not touch.Changeset: present, and it declares nothing
Measured, not asserted.
packages/app-shell/distwas built twice — once at theorigin/mainversion of the changed file, once at this branch's — each time from a cleareddist/and a clearedtsconfig.tsbuildinfo, then sha256-compared:packages/app-shell/tsconfig.jsonexcludes**/*.test.tsxfrom the build program, which is the mechanism behind that reading. One trap worth recording: the first attempt produced an emptydist/and would have read as a false "identical" —tsciscomposite, so with the build info left in place it skips the emit entirely. Clearing it is part of the measurement.Generated by Claude Code