Uh oh!
There was an error while loading. Please reload this page.
test(app-shell): generalize the #3291 DOM-leak canary sweep beyond packages/fields (#4425 phase 1) - #4441
Merged
Merged
Conversation
…ckages/fields (#4425) Phase 1 of #4425: a MEASUREMENT gate over the registry-reachable SDUI widgets of plugin-charts / plugin-calendar / plugin-chatbot / plugin-dashboard. No widget contract and no widget source changes; the leaks it finds are recorded in an in-gate ledger with exact attribute sets and owning issues. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
`@object-ui/plugin-charts` exports only `.`, so the deep subpath resolved for Vite's alias but not for tsc (TS2882). Widening that package's exports would be a public-surface change this measurement-only PR must not make; each chart target's readiness selector already covers the lazy boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
commented
Aug 12, 2026
CollaboratorAuthor
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 07:39
Uh oh!
There was an error while loading. Please reload this page.
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
2 participants
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.
Part of #4425
Phase 1 as ruled in the claim comment: measure before converging. The #3291 canary sweep, generalized beyond
packages/fieldsto the registry-reachable SDUI widgets of the four packages the card names. Zero widget contract changes, zero widget source changes — leaks are RECORDED, not fixed.New suite:
packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx, 39 cases over 23 measured targets. Plus one changeset with empty frontmatter (test-only, declared as releasing nothing).The enumeration, measured from the registry
Not taken from the card's candidate list. Each plugin barrel was imported in isolation and the registry diffed before/after, then attributed by canonical namespace — because
plugin-calendar's barrel transitively registersplugin-detailand all 46 field widgets, which a naive diff over-attributes by 100+ entries.5 of 23 targets leak. The two clean packages are clean for opposite reasons:
plugin-chartsnever spreads the node onto its container, whileplugin-calendaris clean only because its one spreading target is swept with a canary withheld — authoringeventscrashes it outright (#4433).Enumeration parity is asserted per package against the registry itself, so a widget added to any of these plugins cannot slip past unswept — the guarantee
FORM_FIELD_TYPESgives the fields gate.The reading phase 2 was waiting for
The dashboard rows are the interesting ones. Read them against what is not in them:
Every one of the seven keys
schemaHostProps.tsenumerates (#4357 / PR #4428) is absent on both components —schema,bind,events,props,ariaLabel,ariaDescribedByand the injecteddataSourceadapter are all correctly stripped. The deny-list does exactly what it claims. Raw post-fix DOM from the sweep:What remains is the open tail, which is
toDomProps' argument measured rather than predicted.colorvariantis #4425's own example;labelis a new one —MetricCardPropsspells its headingtitle, so authoring the key its siblingMetricWidgettakes puts the heading on the DOM as an attribute instead of rendering it. A deny-list bounded by enumeration cannot be finished; a whitelist bounded by declaration can.Option 3 ("record the divergence") happens in the gate's docblock either way, as the ruling said it would. Option 1 stays with the maintainer.
Four traps that made the first measurement a lie
Each was hit, and each is now an assertion rather than a comment:
class="animate-pulse …"—ChartRenderer's Suspense fallback — and reported nine clean chart targets while no chart had mounted. Every target now has to reach a readiness selector proving its real markup exists.chatbot-floatingmounts throughcreatePortalinto#floating-chatbot-portal; a container-scoped scan saw zero elements and called it clean. The sweep scansdocument.body, andafterEachremoves the portal node RTL does not own.SchemaErrorBoundaryrenders tidy, attribute-clean markup. Three calendar targets "passed" that way. Every target asserts the boundary is absent before scanning.object-calendarthrowsuseSchemaContext must be used within a SchemaRendererProvideron a bare renderer, and an unconfigured one renders a placeholder. Renders go through the provider with the minimum schema each widget needs.Non-vacuity, and the reverse verification
Direction predicted before each run. Four mutations, each restored with
git checkout(nevergit stash— the stack is shared across worktrees):metric-cardledger row removedview:dashboard's rowpie-chartdropped from TARGETSThe
metric-cardprediction was wrong, and the extra red is correct. I implemented "remove the row" as a rename to a non-swept key, so the ledger shape assertion also fired withzz-removed-for-rv:metric-card: ledgered but not a swept target. Two assertions caught one mutation; the prediction was off because of how the mutation was written, not because the gate misbehaved. Recording it rather than quietly reporting 1/1.The mechanism case is the one the ruling asked for explicitly — a fixture component with a deliberate leak, rendered through the same harness, going red when it stops leaking:
Its twin — a fixture that filters its spread and must stay clean — is what keeps the judge from being merely noisy.
The ledger, and why it cannot rot
Assertion is exact set equality, so a row is a two-way ratchet: a new leaked attribute fails the gate, and fixing a leak also fails it until the row is deleted in the same change. That is the expiry — no date to forget, no allow-list to rot; the tree itself expires the row. A separate case rejects a row with no reason, no owning issue, or no matching swept target.
Ledger contents:
plugin-chatbot:chatbotand:chatbot-enhanced(14 each, #4431),plugin-dashboard:metric(7) and:metric-card(9) (both #4425, phase 2 decides the shape),view:dashboard(13, #4432).The withheld canary is a recorded defect, not an exemption
calendar-viewis swept withouteventsbecause authoring it — ordinary SDUI action metadata, legal on any node — crashes the component: the renderer computes aCalendarEvent[], passes it asevents, then spreads{...props}after it, so the SDUI object overwrites the array andCalendarViewthrowsevents is not iterable. Verified in isolation:eventsalone crashes it, the same node without it renders the real calendar. A crashing render produces attribute-clean error-boundary DOM and would have read as a clean pass. The omission carries its own pin, so fixing the crash turns that case red and both halves come out together.Filed from the measurement
chatbotandchatbot-enhancedspread the whole SDUI node onto the host element — 14 non-DOM attributes reach the DOM #4431 — plugin-chatbot:chatbot/chatbot-enhancedspread the whole node, 14 attributes including the injected adapter and a duplicatedarialabelnext to the correctaria-labelDashboardRenderer's widget grid spreads the whole SDUI node onto its container — 13 non-DOM attributes reach the DOM #4432 — plugin-dashboard:DashboardRenderer's widget grid, 13 attributes (same package asMetricWidget/MetricCardspread...propsonto the DOM, emitting aschema="[object Object]"attribute on every KPI card #4357, different element, not in that card's scope)eventson acalendar-viewnode crashes the component — the SDUI action object overwrites the computed CalendarEvent array #4433 — plugin-calendar: theeventscrash abovefindinglabel, observation-class: the judge now exists in two copiesmetric/metric-cardare deliberately not filed separately — they are #4425's own subject, so a new issue would be a twin.Home justification (#4409 dependency-direction method)
None of the four target packages can host this:
plugin-dashboarddeclares onlyplugin-charts, and nothing declaresplugin-calendarorplugin-chatbot, so hosting it in any one of them inverts the direction for the other three — the inversion #4409 refused when it kept the map gate out of@object-ui/i18n. Four workspace entries declare all four; three are apps/examples.packages/app-shelldeclares all four asdevDependencies(the correct field for a test-only import, and whatcheck-phantom-dependencieshonours for a__tests__/file) and already hostsspec-symbol-parity.test.tsand #4409's own map gate. One suite covered all four — the "one suite per package" fallback the card allowed for was not needed.Two deviations worth flagging
React.lazyboundary into module scope.@object-ui/plugin-chartsexports only., so@object-ui/plugin-charts/ChartImplresolves for Vite's alias but not for tsc (TS2882 on both passes), and widening that package'sexportsis a public-surface change this PR must not make. Each chart target's readiness selector covers it instead, with a deliberately generous 10s budget. Three consecutive runs: 39/39 each.packages/fieldsis out of scope here. Recorded as finding: the DOM-leak "is this attribute HTML-defined" judge now exists in two copies, in two test files that cannot import each other #4434; the calibration fixtures keep this copy honest meanwhile.Verification
Control-byte self-scan beyond the gate over both touched files: clean.
origin/mainmoved 4 commits mid-flight and was merged in; it carried #4426'sdomPassthroughPins.tsandMetricWidget.domPassthrough.test.tsx. That change declares the passthrough types without moving the runtime filter, and the gate stayed 39/39 with the ledger matching exactly — which, under exact set equality, is positive evidence that the leak surface did not move rather than an absence of evidence. Build, both type-check passes and every gate were re-run after the merge.Generated by Claude Code