From bd2a7ec50d085387622e12a2b1034315eabc4b9b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 04:27:22 +0000 Subject: [PATCH 1/2] feat(app-shell): declare a precise `sideEffects` array, with the gate that keeps it honest `@object-ui/app-shell` declared no `sideEffects` field, so every bundler had to assume every module in it does something on import and nothing in the package was shakeable. `"sideEffects": false` is not the answer and is closed by measurement: it drops three live SDUI widget registrations to zero chunks on a green build. The package now declares the precise ARRAY -- its entry forms plus the ten modules that register at load time, in both source and published spellings. An incomplete array fails silently inside a CONSUMER's bundle, so the array never ships alone (maintainer ruling, 2026-08-29): - `scripts/check-side-effects-array.mjs` re-derives the enumeration from the module bodies and fails on a missing registrar, a stale name, a registrar no chain of covered modules reaches, or a top-level effect it does not recognise. - `scripts/check-sdui-registration-pins.mjs` weighs the built console for every registration the array promises, with the key set derived from the array itself. `MAX_EAGER_CLOSURE_GZIP_BYTES` is re-baselined downward, 3,345,000 -> 3,300,000 over a measured 3,254,004, in the same commit -- the array took 56,668 gzipped bytes out of the closure, more than the old ceiling's whole headroom. Two entries leave `DECLARED_LAZY_VIEWS_STILL_EAGER`: `RecordFormPage` and `ReportView` are no longer eager, because the co-tenant modules that anchored their chunks became shakeable. The build named both, which is what a recorded win looks like there. Fixes #6683 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6683-app-shell-side-effects-array.md | 35 + .github/workflows/ci.yml | 21 + .github/workflows/performance-budget.yml | 34 + content/docs/guide/ci-cd-pipeline.md | 2 +- package.json | 2 + packages/app-shell/package.json | 25 + .../check-eager-closure-budget.test.ts | 21 +- .../check-sdui-registration-pins.test.ts | 162 ++++ .../check-side-effects-array.test.ts | 344 +++++++++ ...de-effects-declaration-consistency.test.ts | 118 ++- scripts/check-eager-closure-budget.mjs | 55 +- scripts/check-sdui-registration-pins.mjs | 260 +++++++ scripts/check-side-effects-array.mjs | 726 ++++++++++++++++++ scripts/vite-declared-lazy-views.ts | 66 +- 14 files changed, 1811 insertions(+), 60 deletions(-) create mode 100644 .changeset/6683-app-shell-side-effects-array.md create mode 100644 scripts/__tests__/check-sdui-registration-pins.test.ts create mode 100644 scripts/__tests__/check-side-effects-array.test.ts create mode 100644 scripts/check-sdui-registration-pins.mjs create mode 100644 scripts/check-side-effects-array.mjs diff --git a/.changeset/6683-app-shell-side-effects-array.md b/.changeset/6683-app-shell-side-effects-array.md new file mode 100644 index 0000000000..ee2ac3e333 --- /dev/null +++ b/.changeset/6683-app-shell-side-effects-array.md @@ -0,0 +1,35 @@ +--- +'@object-ui/app-shell': minor +--- + +`@object-ui/app-shell` now publishes a precise `sideEffects` ARRAY. + +**What this means for a consumer.** Until now the package declared no +`sideEffects` field at all, which every bundler reads as "assume every module in +this package does something when it is imported" — so nothing in the package +could be tree-shaken, and importing one named export from the barrel pulled in +the barrel's whole reachable graph. The package now names exactly the modules +that DO something on import: its entry forms (including `./styles.css`, which a +bundler must never drop) and the ten modules that register SDUI widgets, admin +components and metadata resources at load time. Everything else is now +shakeable, so a consumer's bundler may drop the parts of `@object-ui/app-shell` +their app does not use. + +⚠️ **If your build depends on a module of this package being evaluated for its +side effects without importing anything from it, and that module is not one of +the ten named**, it may now be dropped from your bundle. Import the value you +need by name, or call the registration explicitly. Measured on this repo's own +console: 56,668 gzipped bytes left the eager closure and every SDUI registration +stayed present. + +`"sideEffects": false` was NOT adopted and remains disproven by measurement: it +drops three live SDUI widget registrations (`mcp:connect-agent`, +`cloud:onboarding-next`, `cloud:ai-model-status`) to zero chunks on a green +build with no warning anywhere. + +Two gates ship with the array, because an INCOMPLETE array fails silently inside +a consumer's bundle and would otherwise have no witness: +`scripts/check-side-effects-array.mjs` re-derives the enumeration from the module +bodies and fails when the array and the derivation disagree in either direction, +and `scripts/check-sdui-registration-pins.mjs` weighs the built console for every +registration the array promises to keep. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b433f304ad..a37ea94566 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,6 +263,27 @@ jobs: if: steps.relevant.outputs.should_run == 'true' run: pnpm check:self-import + # `sideEffects` is a PUBLISHED CONTRACT, and an ARRAY form of it fails in + # the one direction nothing can witness: an INCOMPLETE array drops a + # registration inside a CONSUMER's bundle, with no error, no warning and + # exit 0 — the same failure mode as `"sideEffects": false`, only quieter. + # `@object-ui/app-shell` declares one because both simpler answers are + # measurably wrong for it (objectui#6683; `false` drops three live SDUI + # widget registrations, measured in objectui#6535). This gate re-derives + # the enumeration from the module bodies and fails when the array and the + # derivation disagree in EITHER direction — a missing registrar, or a + # stale name whose module no longer registers anything. + # + # Placed here rather than with the build steps for the same reason as the + # two above: it parses sources with `typescript` and reads package + # manifests, so it needs the install and nothing built. The artifact half + # of the same contract — do the registrations survive a real bundler — + # cannot run here at all and lives in `performance-budget.yml`, which + # builds the console. + - name: Verify every `sideEffects` array names exactly its registering modules + if: steps.relevant.outputs.should_run == 'true' + run: pnpm check:side-effects-array + # Node's ESM resolver does not extension-search relative specifiers, so an # extensionless `./SchemaRenderer` in a published `dist/` is unloadable # under plain Node — `@object-ui/react`'s entry died with diff --git a/.github/workflows/performance-budget.yml b/.github/workflows/performance-budget.yml index 6b4f608a91..994f2e0b06 100644 --- a/.github/workflows/performance-budget.yml +++ b/.github/workflows/performance-budget.yml @@ -41,6 +41,14 @@ on: - 'scripts/check-eager-closure-budget.mjs' - 'scripts/render-budget-comment.mjs' - 'scripts/invoked-as.mjs' + # The SDUI registration pins (objectui#6683) and their two dependencies. + # Same rule as the three above — this job runs them, so an edit to any of + # them must be able to turn this gate red on its OWN PR rather than on + # somebody else's next `packages/**` change. + - 'scripts/check-sdui-registration-pins.mjs' + - 'scripts/check-side-effects-array.mjs' + - 'scripts/component-registrations.mjs' + - 'scripts/js-comment-mask.mjs' pull_request: branches: [main, develop] paths: @@ -81,6 +89,14 @@ on: - 'scripts/check-eager-closure-budget.mjs' - 'scripts/render-budget-comment.mjs' - 'scripts/invoked-as.mjs' + # The SDUI registration pins (objectui#6683) and their two dependencies. + # Same rule as the three above — this job runs them, so an edit to any of + # them must be able to turn this gate red on its OWN PR rather than on + # somebody else's next `packages/**` change. + - 'scripts/check-sdui-registration-pins.mjs' + - 'scripts/check-side-effects-array.mjs' + - 'scripts/component-registrations.mjs' + - 'scripts/js-comment-mask.mjs' concurrency: group: bundle-analysis-${{ github.event.pull_request.number || github.ref }} @@ -330,6 +346,24 @@ jobs: echo "✅ Budget OK: entry chunk and eager closure are both within budget" echo "budget_status=pass" >> "$GITHUB_OUTPUT" + # The ARTIFACT half of the `sideEffects` contract (objectui#6683). The + # static gate in `ci.yml` proves the array agrees with the module bodies; + # it cannot prove that a real bundler reading that array still EMITS the + # registrations, and that is the question the hazard turns on — + # `"sideEffects": false` is statically coherent and drops three live SDUI + # widget registrations to 0 chunks on a green build (objectui#6535). + # + # The key set is derived from the array itself, so this step needs no + # list of its own. It runs after the budget step and never instead of it: + # a dropped registration and a size regression are different verdicts, and + # a size win bought by deleting a feature must not read as a size win. + # + # Not `if: always()` — with no `dist/` the checker exits 2 and says so, + # which is the honest verdict for a run that measured nothing, but there + # is no reason to spend it on a build that already failed. + - name: Pin the SDUI registrations the `sideEffects` array promises + run: pnpm check:sdui-registration-pins + - name: Generate package size report id: size-report # NOT `always()`. `always()` also fires on a cancelled run, where diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index ddead2537a..f4cfc739f7 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -205,7 +205,7 @@ it green — which is how two of `type-check`'s gates came to be missing from th | Job key | Appears as | What it runs | When | |---|---|---|---| | `changeset-check` | Changeset Fixed Group Check | `scripts/check-changeset-fixed.mjs` — every workspace package must be in the changeset `fixed` group or explicitly ignored. It checks group *membership*; it does **not** check whether the PR added a changeset. | Every run | -| `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm type-check:scripts`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The two locale gates sit in the middle because both parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | +| `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:side-effects-array`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm type-check:scripts`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:side-effects-array` runs next, sources only and no build: it fails when a package's `sideEffects` ARRAY and its module bodies disagree in either direction — a module that registers something at load time and is not named (a bundler drops it, and the registration is gone from a *consumer's* app with no error, no warning and exit 0), or a name whose module no longer registers anything. `@object-ui/app-shell` declares such an array because both simpler answers are measurably wrong for it: omitting the field makes the whole package unshakeable, and `"sideEffects": false` silently drops three live SDUI widget registrations to zero chunks ([#6535](https://github.com/objectstack-ai/objectui/issues/6535), [#6683](https://github.com/objectstack-ai/objectui/issues/6683)). The enumeration is re-derived from the module bodies on every run rather than listed, so there is no second copy to rot. The artifact half of the same contract — do those registrations survive a real bundler — cannot run in this job at all: it needs a built console, so it lives in the SDUI registration pin step of `performance-budget.yml`. `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The two locale gates sit in the middle because both parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | | `test` | Test (shard N/4) | `pnpm test --shard=N/4` across a 4-runner matrix with `fail-fast: false`, so every shard reports its own failures. No coverage instrumentation — v8 adds 40–100% overhead. | Pull requests and merge-queue builds (everything but `push`); steps short-circuit on a PR that changed only ignored paths | | `test-coverage` | Test (coverage shard N/4) | `pnpm test:coverage --reporter=blob --shard=N/4` across a 4-runner matrix with `fail-fast: false`. Each shard writes `.vitest-reports/blob-N-4.json` — raw coverage and test results in one file — and uploads it as an artifact even when the shard is red, which is what makes a failing coverage run diagnosable at all (vitest deletes `coverage/` on a red run unless `coverage.reportOnFailure` is set, [#5402](https://github.com/objectstack-ai/objectui/issues/5402)). The configured coverage thresholds are neutralised on the shard legs, because a quarter of the suite judged against a whole-suite threshold is not a defect signal; they are enforced once, on the merged report, by the job below ([#5403](https://github.com/objectstack-ai/objectui/issues/5403)). | **Push only** | | `coverage-report` | Test (coverage) | Downloads the four blob reports, refuses to continue unless all four arrived, merges them with `pnpm test:coverage --merge-reports` into one complete report — which is where the configured coverage thresholds are enforced, over the whole merged map, the shard legs having overridden them to zero — and publishes that report as the `coverage-report` artifact (kept 7 days, the same as the blobs it is derived from). Its last step runs on every path and states the outcome: the job is **red, with an error annotation**, whenever the gate did not run for the commit — before [#5403](https://github.com/objectstack-ai/objectui/issues/5403) the final step carried the implicit `success()` and was silently skipped by 311 of 373 coverage jobs, which is how four days of a 100%-failing coverage job went unnoticed. A breach of the thresholds is reported *separately* from a lane that never delivered, because the two call for opposite actions. ⛔ It never merges a report from fewer than four shards: a wrong coverage number is worse than a missing one. The Codecov upload this job used to carry was retired by [#5436](https://github.com/objectstack-ai/objectui/issues/5436) — `CODECOV_TOKEN` was never set, so it failed on every push; the trend dashboard and PR coverage comments are gone with it, the gate is not. | **Push only** | diff --git a/package.json b/package.json index 90157792f4..c7be49433a 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,8 @@ "check:doc-snippets": "node scripts/check-doc-snippet-types.mjs", "check:doc-fences": "node scripts/check-doc-fence-languages.mjs", "check:eager-closure": "node scripts/check-eager-closure-budget.mjs", + "check:side-effects-array": "node scripts/check-side-effects-array.mjs", + "check:sdui-registration-pins": "node scripts/check-sdui-registration-pins.mjs", "check:docs-route-closure": "node scripts/check-docs-route-eager-closure.mjs", "check:entry-guard": "node scripts/check-entry-guard.mjs", "check:upstream-port-parity": "node scripts/check-upstream-port-parity.mjs", diff --git a/packages/app-shell/package.json b/packages/app-shell/package.json index 37335039eb..5bde8d0427 100644 --- a/packages/app-shell/package.json +++ b/packages/app-shell/package.json @@ -2,6 +2,31 @@ "name": "@object-ui/app-shell", "version": "17.6.0", "type": "module", + "sideEffects": [ + "./dist/index.js", + "./dist/console/cloud-connection/CloudConnectionPanel.js", + "./dist/console/connect/ConnectAgentWidget.js", + "./dist/console/diagnostics/CloudAiModelStatus.js", + "./dist/console/home/CloudOnboardingNext.js", + "./dist/console/marketplace/InstalledListWidget.js", + "./dist/services/builtinComponents.js", + "./dist/views/metadata-admin/index.js", + "./dist/views/record-approvals-renderer.js", + "./dist/views/record-attachments-renderer.js", + "./dist/views/studio-design/studio-canvas-preview.js", + "./src/index.ts", + "./src/console/cloud-connection/CloudConnectionPanel.tsx", + "./src/console/connect/ConnectAgentWidget.tsx", + "./src/console/diagnostics/CloudAiModelStatus.tsx", + "./src/console/home/CloudOnboardingNext.tsx", + "./src/console/marketplace/InstalledListWidget.tsx", + "./src/services/builtinComponents.tsx", + "./src/views/metadata-admin/index.ts", + "./src/views/record-approvals-renderer.tsx", + "./src/views/record-attachments-renderer.tsx", + "./src/views/studio-design/studio-canvas-preview.tsx", + "./src/styles.css" + ], "license": "MIT", "description": "Minimal application shell for ObjectUI - framework-agnostic rendering engine", "homepage": "https://www.objectui.org/docs/layout/app-shell", diff --git a/scripts/__tests__/check-eager-closure-budget.test.ts b/scripts/__tests__/check-eager-closure-budget.test.ts index 9a07b5fc23..b3ac408d64 100644 --- a/scripts/__tests__/check-eager-closure-budget.test.ts +++ b/scripts/__tests__/check-eager-closure-budget.test.ts @@ -427,9 +427,19 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { /** The ceiling this file shipped before objectui#5924 re-baselined it. */ const CEILING_BEFORE_5924 = 4_086_000; + /** + * The payload objectui#5924 measured, pinned as its own constant. + * + * It used to read `BASELINE.gzipBytes`, which made a HISTORICAL incident + * reproduction track today's measurement: objectui#6683 moved the baseline + * and the recorded "8.63x" became arithmetic about a moment that never + * happened. A reproduction of a past reading has to carry that reading. + */ + const BASELINE_AT_5924 = 3_299_898; + it('reds on the drift objectui#5924 recorded: 8.6x the regression above the live payload', () => { const result = evaluateHeadroomSensitivity({ - report: sensitivityReport(BASELINE.gzipBytes), + report: sensitivityReport(BASELINE_AT_5924), budgetBytes: CEILING_BEFORE_5924, }); expect(result.status).toBe('error'); @@ -459,8 +469,11 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { ...Object.keys(PER_CHUNK_GZIP_CEILINGS), ]); // A passing run still prints every measurement, so a reader watching a - // ceiling drift upward sees it coming rather than the day it reds. - expect(result.message).toContain('3222.6'); + // ceiling drift upward sees it coming rather than the day it reds. The + // literal is `BASELINE.gzipBytes` rendered, re-taken when objectui#6683 + // re-baselined it downward — a rendering derived in the test would agree + // with the renderer by construction and pin nothing. + expect(result.message).toContain('3177.7'); }); it('is exactly one regression wide, from either side of the line', () => { @@ -635,7 +648,7 @@ describe('main', () => { expect(code).toBe(0); expect(outputs.closure_status).toBe('pass'); expect(outputs.closure_chunks).toBe('5'); - expect(outputs.closure_gzip_kb).toBe('3222.6'); + expect(outputs.closure_gzip_kb).toBe('3177.7'); }); it('exits 1 — a verdict about the BUNDLE — when over budget', () => { diff --git a/scripts/__tests__/check-sdui-registration-pins.test.ts b/scripts/__tests__/check-sdui-registration-pins.test.ts new file mode 100644 index 0000000000..1f7d77f26c --- /dev/null +++ b/scripts/__tests__/check-sdui-registration-pins.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { + EXIT_DROPPED, + EXIT_NO_MEASUREMENT, + EXIT_OK, + NEGATIVE_CONTROL_KEY, + RULED_CONTROLS, + countChunksCarrying, + derivePinnedKeys, + main, +} from '../check-sdui-registration-pins.mjs'; + +/** + * objectui#6683. This gate weighs the BUILT console for the registrations a + * `sideEffects` array promises to keep. `"sideEffects": false` is statically + * coherent and drops three of them to 0 chunks (measured, objectui#6535), so + * the artifact question is not answerable from the source — and a gate about an + * artifact is the easiest kind to make vacuous: point it at a missing `dist/`, + * or at a matcher that matches everything, and it reports success forever. + * + * Both of those are tested here as FAILURES, alongside the ordinary red. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +/** A fixture root: one array package with one registrar, plus a fake console dist. */ +function fixture(chunks: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'objectui-6683-pins-')); + fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n"); + const pkg = path.join(dir, 'packages/pkg'); + fs.mkdirSync(path.join(pkg, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(pkg, 'package.json'), + JSON.stringify({ + name: '@fixture/pkg', + main: './dist/index.js', + sideEffects: ['./dist/index.js', './src/index.ts'], + }), + ); + fs.writeFileSync( + path.join(pkg, 'src/index.ts'), + "import { ComponentRegistry } from 'somewhere';\nComponentRegistry.register('fixture:widget', 1);\n", + ); + const assets = path.join(dir, 'apps/console/dist/assets'); + fs.mkdirSync(assets, { recursive: true }); + for (const [name, code] of Object.entries(chunks)) fs.writeFileSync(path.join(assets, name), code); + return dir; +} + +describe('countChunksCarrying', () => { + const read = (f: string) => f; + + it('counts each of the three quote spellings a minifier may pick', () => { + expect(countChunksCarrying(["r.register('a:b',x)"], 'a:b', read)).toBe(1); + expect(countChunksCarrying(['r.register("a:b",x)'], 'a:b', read)).toBe(1); + expect(countChunksCarrying(['r.register(`a:b`,x)'], 'a:b', read)).toBe(1); + }); + + it('does NOT count a bare substring', () => { + // `attachments` is a real registry key AND a word that occurs all over a + // bundle. A matcher that counted those would report every key present + // whatever the bundler did — the vacuity this gate cannot afford. + expect(countChunksCarrying(['const attachmentsPanel = 1;'], 'attachments', read)).toBe(0); + expect(countChunksCarrying(["x('attachments')"], 'attachments', read)).toBe(1); + }); + + it('counts chunks, not occurrences', () => { + expect(countChunksCarrying(["'a:b' 'a:b' 'a:b'"], 'a:b', read)).toBe(1); + expect(countChunksCarrying(["'a:b'", "'a:b'"], 'a:b', read)).toBe(2); + }); +}); + +describe('the fixture console', () => { + it('passes when the registration is in a chunk', () => { + const dir = fixture({ 'index-abc.js': "R.register('fixture:widget',()=>1)" }); + try { + expect(main([], dir, ['fixture:widget'])).toBe(EXIT_OK); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('fails when the same build no longer carries it', () => { + // The partner of the case above: identical package, identical derivation, + // one chunk's content changed. This is the shape `"sideEffects": false` + // produces — a green build with the registration simply absent. + const dir = fixture({ 'index-abc.js': 'console.log(1)' }); + try { + expect(main([], dir, ['fixture:widget'])).toBe(EXIT_DROPPED); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('exits 2 — never 0 — when there is no build to weigh', () => { + const dir = fixture({}); + fs.rmSync(path.join(dir, 'apps/console/dist'), { recursive: true, force: true }); + try { + expect(main([], dir, ['fixture:widget'])).toBe(EXIT_NO_MEASUREMENT); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('exits 2 when a pinned control is no longer in the DERIVED set', () => { + // The floor's own discrimination. `RULED_CONTROLS` is checked against the + // derivation, so a `sideEffects` array that stopped naming the registering + // module must not read as "0 keys, all present". Same fixture, a control + // that nothing registers. + const dir = fixture({ 'index-abc.js': "R.register('fixture:widget',1)" }); + try { + expect(main([], dir, ['fixture:widget', 'never:registered'])).toBe(EXIT_NO_MEASUREMENT); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('exits 2 when a chunk carries the negative control, because then the matcher cannot miss', () => { + const dir = fixture({ + 'index-abc.js': `R.register('fixture:widget',1);const s='${NEGATIVE_CONTROL_KEY}';`, + }); + try { + expect(main([], dir, ['fixture:widget'])).toBe(EXIT_NO_MEASUREMENT); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('the real workspace', () => { + it('derives the keys from the arrays, and the ruled controls are among them', () => { + const { keys, sources, unreadable, modulesRead } = derivePinnedKeys(repoRoot); + expect(unreadable).toEqual([]); + expect(modulesRead).toBeGreaterThan(0); + expect(keys.length, 'an empty key set makes every assertion in this gate vacuous').toBeGreaterThan(0); + for (const control of RULED_CONTROLS) { + expect(keys, `${control} is one of the three registrations the 2026-08-29 ruling pins`).toContain(control); + } + // The derivation must point at the module it read the key from, or a drop + // would be reported without saying which array entry promised it. + expect(sources.get('mcp:connect-agent')).toBe( + 'packages/app-shell/src/console/connect/ConnectAgentWidget.tsx', + ); + }); + + it('keeps RULED_CONTROLS a floor rather than the population', () => { + // If the two ever coincide, the "derived" set has quietly become the hand + // list the ruling forbids. + const { keys } = derivePinnedKeys(repoRoot); + expect(keys.length).toBeGreaterThan(RULED_CONTROLS.length); + }); + + it('nothing registers the negative control', () => { + const { keys } = derivePinnedKeys(repoRoot); + expect(keys).not.toContain(NEGATIVE_CONTROL_KEY); + }); +}); diff --git a/scripts/__tests__/check-side-effects-array.test.ts b/scripts/__tests__/check-side-effects-array.test.ts new file mode 100644 index 0000000000..d43ecea925 --- /dev/null +++ b/scripts/__tests__/check-side-effects-array.test.ts @@ -0,0 +1,344 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import ts from 'typescript'; +import { fileURLToPath } from 'node:url'; + +// Types are INFERRED from the .mjs source by `tsconfig.scripts.json` +// (`allowJs`), so no `@ts-expect-error` and no hand-written `.d.mts`. +import { + EXIT_DISAGREES, + EXIT_NO_MEASUREMENT, + EXIT_OK, + classifyEffect, + evaluate, + evaluatePackage, + main, + readArrayPackages, +} from '../check-side-effects-array.mjs'; + +/** + * objectui#6683. The gate under test exists because an INCOMPLETE `sideEffects` + * array fails silently inside a CONSUMER's bundle — no error, no warning, exit + * 0. A gate against a silent failure is worth exactly as much as its ability to + * go red, so this file's first duty is DISCRIMINATION: every assertion below + * has a partner that makes the same fixture fail. + * + * The fixtures are synthetic workspaces rather than the repo, for the reason + * `check-eager-closure-budget.test.ts` gives about its own: a gate whose only + * test is "it passes on today's tree" is green because the tree is currently + * correct, and stays green when the gate stops looking. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +interface Files { + [relativePath: string]: string; +} + +/** A throwaway workspace on disk. The gate reads files, so the fixture is files. */ +function workspace(files: Files): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'objectui-6683-')); + fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n"); + for (const [rel, content] of Object.entries(files)) { + const file = path.join(dir, rel); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); + } + return dir; +} + +const manifest = (sideEffects: string[]): string => + JSON.stringify( + { + name: '@fixture/pkg', + type: 'module', + main: './dist/index.js', + exports: { '.': { types: './dist/index.d.ts', import: './dist/index.js' } }, + sideEffects, + }, + null, + 2, + ); + +/** barrel -> a registrar (bare import) and a pure module (named import). */ +const SOURCES: Files = { + 'packages/pkg/src/index.ts': "import './registrar.js';\nexport { pure } from './pure.js';\n", + 'packages/pkg/src/registrar.ts': "import { Registry } from 'somewhere';\nRegistry.register('fixture:key', 1);\n", + 'packages/pkg/src/pure.ts': 'export const pure = 1;\n', +}; + +const HONEST_ARRAY = ['./dist/index.js', './dist/registrar.js', './src/index.ts', './src/registrar.ts']; + +function run(files: Files): ReturnType { + const dir = workspace(files); + try { + const packages = readArrayPackages(dir); + expect(packages, 'the fixture workspace must expose exactly one array package').toHaveLength(1); + return evaluatePackage(packages[0], dir); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +describe('the honest array', () => { + it('passes, and names the registrar it derived', () => { + const verdict = run({ ...SOURCES, 'packages/pkg/package.json': manifest(HONEST_ARRAY) }); + expect(verdict.problems).toEqual([]); + expect(verdict.missing).toEqual([]); + expect(verdict.stale).toEqual([]); + expect(verdict.registrars).toEqual(['src/registrar.ts']); + expect(verdict.ok).toBe(true); + }); + + it('is not passing because the walk found nothing', () => { + // The anti-vacuity partner of the case above. A walk that collapsed to the + // barrel would derive an empty registrar set, and an empty set agrees with + // any array at all. + const verdict = run({ ...SOURCES, 'packages/pkg/package.json': manifest(HONEST_ARRAY) }); + expect(verdict.modulesWalked).toBe(3); + expect(verdict.registrars.length).toBeGreaterThan(0); + }); +}); + +describe('MISSING — the silent drop this gate exists to make loud', () => { + it('fails when the array omits a registering module', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/package.json': manifest(HONEST_ARRAY.filter((e) => !e.includes('registrar'))), + }); + expect(verdict.ok).toBe(false); + expect(verdict.missing).toEqual(['dist/registrar.js', 'src/registrar.ts']); + expect(verdict.stale).toEqual([]); + }); + + it('fails when only the SOURCE spelling is named and the published one is not', () => { + // The half a consumer pays for. In-repo bundlers resolve the alias to + // `src/`; everyone who installs the package resolves `exports` to `dist/`. + const verdict = run({ + ...SOURCES, + 'packages/pkg/package.json': manifest(HONEST_ARRAY.filter((e) => e !== './dist/registrar.js')), + }); + expect(verdict.missing).toEqual(['dist/registrar.js']); + }); + + it('fails when an entry form itself is missing', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/package.json': manifest(HONEST_ARRAY.filter((e) => e !== './src/index.ts')), + }); + expect(verdict.missing).toEqual(['src/index.ts']); + }); +}); + +describe('STALE — a name whose module no longer registers anything', () => { + it('fails when the array names a pure module', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/package.json': manifest([...HONEST_ARRAY, './src/pure.ts']), + }); + expect(verdict.ok).toBe(false); + expect(verdict.stale).toEqual(['src/pure.ts']); + expect(verdict.missing).toEqual([]); + }); + + it('fails when the array names a path that does not exist at all', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/package.json': manifest([...HONEST_ARRAY, './src/gone.ts']), + }); + expect(verdict.stale).toEqual(['src/gone.ts']); + }); +}); + +describe('the gauge — exit 2 territory, never a pass', () => { + it('refuses a top-level side effect it does not recognise', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/src/pure.ts': "export const pure = 1;\nsomeGlobal.installed = true;\n", + 'packages/pkg/package.json': manifest(HONEST_ARRAY), + }); + expect(verdict.gauge).toBe(true); + expect(verdict.problems.join('\n')).toContain('does not recognise'); + // ...and it did NOT quietly decide the module was pure, which is the whole + // point: an unrecognised effect must not collapse into "not a registration". + expect(verdict.stale).toEqual([]); + }); + + it('refuses a glob, which would make the comparison vacuous on whatever it covers', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/package.json': manifest([...HONEST_ARRAY, './src/**/*.ts']), + }); + expect(verdict.gauge).toBe(true); + expect(verdict.problems.join('\n')).toContain('is a glob'); + }); + + it('refuses an unresolved relative specifier rather than shrinking the walk', () => { + const verdict = run({ + ...SOURCES, + 'packages/pkg/src/index.ts': "import './registrar.js';\nimport './not-here.js';\n", + 'packages/pkg/package.json': manifest(HONEST_ARRAY), + }); + expect(verdict.gauge).toBe(true); + expect(verdict.problems.join('\n')).toContain('unresolved relative specifier'); + }); + + it('refuses a spelling map that does not round-trip on the barrel', () => { + const dir = workspace({ + ...SOURCES, + 'packages/pkg/package.json': JSON.stringify({ + name: '@fixture/pkg', + main: './build/index.js', + sideEffects: ['./build/index.js', './src/index.ts'], + }), + }); + try { + const verdict = evaluatePackage(readArrayPackages(dir)[0], dir); + // `src/index.ts` -> `build/index.js` round-trips, so this one is FINE; + // the failure below is the real asymmetry. Keeping both in one test is + // deliberate: a map test that only ever sees `src`/`dist` proves nothing + // about the derivation being a derivation. + expect(verdict.problems.join('\n')).not.toContain('round-trip'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + + const broken = workspace({ + ...SOURCES, + 'packages/pkg/src/index.ts': "import './registrar.js';\n", + 'packages/pkg/package.json': JSON.stringify({ + name: '@fixture/pkg', + // A published barrel two levels deep: `src/index.ts` cannot produce it. + main: './dist/esm/index.js', + sideEffects: ['./dist/esm/index.js', './src/index.ts'], + }), + }); + try { + const verdict = evaluatePackage(readArrayPackages(broken)[0], broken); + expect(verdict.gauge).toBe(true); + expect(verdict.problems.join('\n')).toContain('round-trip'); + } finally { + fs.rmSync(broken, { recursive: true, force: true }); + } + }); + + it('main() exits 2 when no package declares an array', () => { + const dir = workspace({ 'packages/pkg/package.json': JSON.stringify({ name: '@fixture/pkg' }) }); + try { + expect(main([], dir)).toBe(EXIT_NO_MEASUREMENT); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('reachability — naming a module is not enough to retain it', () => { + it('fails when a registrar is reachable only through a shakeable module', () => { + // `barrel -> pure.ts -> registrar.ts`. Every name is in the array, and the + // registration is still lost: `pure.ts` is shakeable, so when its exports go + // unused a bundler drops it and takes the registrar's ONLY edge with it. + const verdict = run({ + 'packages/pkg/src/index.ts': "export { pure } from './pure.js';\n", + 'packages/pkg/src/pure.ts': "import './registrar.js';\nexport const pure = 1;\n", + 'packages/pkg/src/registrar.ts': "import { Registry } from 'somewhere';\nRegistry.register('fixture:key', 1);\n", + 'packages/pkg/package.json': manifest(HONEST_ARRAY), + }); + expect(verdict.ok).toBe(false); + expect(verdict.problems.join('\n')).toContain('no chain of `sideEffects`-covered modules reaches it'); + }); + + it('passes when the same registrar is reached from the barrel directly', () => { + // The partner. Identical shape apart from where the edge starts, so the + // assertion above is about REACHABILITY and not about the fixture. + const verdict = run({ ...SOURCES, 'packages/pkg/package.json': manifest(HONEST_ARRAY) }); + expect(verdict.problems).toEqual([]); + }); +}); + +describe('classifyEffect', () => { + const statements = (source: string) => + ts.createSourceFile('probe.ts', source, ts.ScriptTarget.Latest, true).statements; + const kindOf = (source: string, locals: string[] = []) => + [...statements(source)].map((stmt) => classifyEffect(stmt, new Set(locals))).filter((k) => k !== null); + + it('reads any top-level call as a registration, whatever it is called', () => { + // Deliberately NOT a name test. `Registry.add(...)` and `register(...)` are + // indistinguishable to a bundler, and a name test is an UNDER-reading — + // which here is the silent drop. + expect(kindOf("register('x');")).toEqual(['registration']); + expect(kindOf("Registry.add('x');")).toEqual(['registration']); + expect(kindOf('new Thing();')).toEqual(['registration']); + expect(kindOf('try { register(); } catch {}')).toEqual(['registration']); + }); + + it('reads a write to this module’s own binding as module-local', () => { + expect(kindOf("Banner.displayName = 'Banner';", ['Banner'])).toEqual(['local-binding-write']); + }); + + it('...but a write to something it did NOT declare is unknown, not local', () => { + // The asymmetry that keeps the carve-out honest: `window.x = 1` is + // observable from outside and must not ride the displayName exemption. + expect(kindOf("window.installed = true;")).toEqual(['unknown']); + expect(kindOf("Banner.displayName = 'Banner';")).toEqual(['unknown']); + }); + + it('reads a bare import as propagation, not as an effect of the importer', () => { + expect(kindOf("import './x.js';")).toEqual(['side-effect-only-import']); + expect(kindOf("import { a } from './x.js';")).toEqual([]); + expect(kindOf("export { a } from './x.js';")).toEqual([]); + }); + + it('reads declarations and directive prologues as nothing', () => { + expect(kindOf("'use client';\nconst a = 1;\nfunction f() { register(); }\nexport const b = f;")).toEqual([]); + }); +}); + +describe('the real workspace', () => { + it('agrees with every array this repo actually declares', () => { + const { packages, results } = evaluate(repoRoot); + expect(packages.length, 'this gate is a set comparison; over nothing it is green for nothing').toBeGreaterThanOrEqual(2); + for (const r of results) { + expect(r.problems, `${r.name}: ${r.problems.join('\n')}`).toEqual([]); + expect(r.missing, `${r.name} is missing ${r.missing.join(', ')}`).toEqual([]); + expect(r.stale, `${r.name} has stale entries ${r.stale.join(', ')}`).toEqual([]); + } + expect(main([], repoRoot)).toBe(EXIT_OK); + }); + + it('finds the three registrations the 2026-08-29 ruling names as controls', () => { + // A FLOOR on the derivation, not a copy of it: the enumeration is derived + // from the module bodies, and these three are the modules whose registrations + // `"sideEffects": false` was measured to drop to 0 chunks (objectui#6535). + // If the walk stops seeing them, the array agrees with an empty enumeration. + const { results } = evaluate(repoRoot); + const appShell = results.find((r) => r.name === '@object-ui/app-shell'); + expect(appShell, '@object-ui/app-shell must still declare a `sideEffects` array').toBeDefined(); + expect(appShell!.registrars).toContain('src/console/connect/ConnectAgentWidget.tsx'); + expect(appShell!.registrars).toContain('src/console/home/CloudOnboardingNext.tsx'); + expect(appShell!.registrars).toContain('src/console/diagnostics/CloudAiModelStatus.tsx'); + }); + + it('would go RED if one of those controls left the array', () => { + // The discrimination proof against the REAL manifest: same package, same + // module bodies, one entry removed. A gate that passes both before and + // after proves nothing. + const packages = readArrayPackages(repoRoot); + const appShell = packages.find((p) => p.name === '@object-ui/app-shell')!; + const wrong = { + ...appShell, + declared: (appShell.declared as string[]).filter((e: string) => e !== 'src/console/connect/ConnectAgentWidget.tsx'), + }; + const verdict = evaluatePackage(wrong, repoRoot); + expect(verdict.ok).toBe(false); + expect(verdict.missing).toEqual(['src/console/connect/ConnectAgentWidget.tsx']); + expect(main([], repoRoot)).toBe(EXIT_OK); // ...and the real one still passes + }); + + it('publishes distinct exit codes for a wrong array and a broken gauge', () => { + expect(EXIT_OK).toBe(0); + expect(EXIT_DISAGREES).toBe(1); + expect(EXIT_NO_MEASUREMENT).toBe(2); + }); +}); diff --git a/scripts/__tests__/side-effects-declaration-consistency.test.ts b/scripts/__tests__/side-effects-declaration-consistency.test.ts index 402d282cf5..10293dc491 100644 --- a/scripts/__tests__/side-effects-declaration-consistency.test.ts +++ b/scripts/__tests__/side-effects-declaration-consistency.test.ts @@ -624,6 +624,63 @@ const trackedPaths = gitTrackedPaths(); const isSourceEntry = (pkg: DeclaringPackage, entry: string): boolean => trackedPaths.has(path.posix.join(pkg.dir, entry)); +/** + * Whether an entry form (or an array entry) is a module a parser can read. + * + * `@object-ui/app-shell` exports `./styles.css`, and a stylesheet IS a + * `sideEffects` surface — webpack's classic silent bug is a dropped CSS import + * — so it must be COVERED by the array. What it is not is something the + * TypeScript parser below can scan for a load-time side effect, and what a + * bundler cannot be handed is a JS marker written into a `.css` file. Both + * narrowings below are keyed on this predicate rather than on a filename, so a + * second non-JS export form is covered the day it is added. + */ +const isParseableModule = (entry: string): boolean => /\.(ts|tsx|mts|js|jsx|mjs)$/.test(entry); + +/** + * Array entries that name a module of the package which is not an ENTRY FORM. + * + * objectui#3943 was written when the only array in the workspace named entry + * forms and nothing else (`@object-ui/layout`: its barrel, in three spellings). + * objectui#6683 added the general case: `@object-ui/app-shell` names ten DEEP + * modules that register SDUI widgets at load time, because those are exactly + * the modules a bundler must not drop. + * + * So the phantom direction below can no longer be "not an entry form ⇒ made + * up". It asks the weaker, still-worth-asking question — does this path name + * anything real? — and the STRONGER question, is the set exactly right, belongs + * to `scripts/check-side-effects-array.mjs`, which derives the enumeration from + * the module bodies and fails on a missing OR a stale name. Two guards, two + * derivations, on purpose. + */ +function packageModuleForms(pkg: DeclaringPackage): Set { + const forms = new Set(); + const graph = entryGraphs.get(pkg.name); + if (!graph) return forms; + + // The published spelling of a source module, derived from the barrel pair + // this package actually declares (`src/index.ts` <-> `dist/index.js`), never + // assumed. A package whose build does not preserve the tree simply yields no + // published forms here, and its `dist/*` entries fall to the entry-form half. + const sourceBarrel = resolvableEntryPaths(pkg).find((e) => isSourceEntry(pkg, e) && /(^|\/)index\./.test(e)); + const publishedBarrel = resolvableEntryPaths(pkg).find((e) => !isSourceEntry(pkg, e) && /(^|\/)index\.[cm]?js$/.test(e)); + + for (const abs of graph.modules) { + const rel = path.relative(path.join(repoRoot, pkg.dir), abs).split(path.sep).join('/'); + if (rel.startsWith('..')) continue; + forms.add(rel); + if (sourceBarrel && publishedBarrel) { + const srcRoot = sourceBarrel.split('/')[0]; + const distRoot = publishedBarrel.split('/')[0]; + const publishedExt = path.extname(publishedBarrel); + if (rel.startsWith(`${srcRoot}/`)) { + forms.add(`${distRoot}/${rel.slice(srcRoot.length + 1).replace(/\.(tsx|ts|mts|jsx|js|mjs)$/, publishedExt)}`); + } + } + } + return forms; +} + /* -------------------------------------------------------------------------- */ /* The static scan: top-level statements that reach OUT of the module. */ /* -------------------------------------------------------------------------- */ @@ -951,9 +1008,19 @@ interface ProbeCase { entry: string; } -/** Every entry form of every ARRAY package: the array's whole content is the claim. */ +/** + * Every entry form of every ARRAY package: the array's whole content is the claim. + * + * JS-shaped forms only. `mirrorPackage` probes a form by writing a JS marker + * module AT that path and asking a bundler whether it survives — which is a + * measurement for `dist/index.js` and a category error for + * `@object-ui/app-shell`'s `./src/styles.css`, where Vite's CSS pipeline emits + * an asset and no chunk for the marker to be in. The narrowing is keyed on the + * extension, not on a filename, and it removes nothing from the STATIC halves + * above: `src/styles.css` is still required to be covered by the array there. + */ const arrayEntryProbes: ProbeCase[] = arrayPackages.flatMap((pkg) => - (entryForms.get(pkg.name) ?? []).map((entry) => ({ pkg, name: pkg.name, entry })), + (entryForms.get(pkg.name) ?? []).filter(isParseableModule).map((entry) => ({ pkg, name: pkg.name, entry })), ); /** @@ -979,17 +1046,19 @@ describe('`sideEffects` declarations match load-time behaviour (objectui#3943)', it('discovers the declaring packages (guard cannot pass by finding nothing)', () => { // The hazard here is a SILENT one, so a guard that inspected an empty list // would reproduce it exactly: green output, nothing checked. These floors - // sit at the census measured on main@97da1b0d6 — 8 packages declare the - // field, 5 of them `false` and 1 an array — so a lost workspace root, a - // broken glob parser or a renamed field is a failure and not a quiet pass. - expect(declaringPackages.length).toBeGreaterThanOrEqual(6); + // sit at the census re-measured for objectui#6683 — 9 packages declare the + // field, 5 of them `false` and 2 an array (`components`/`fields` declare + // `true`) — so a lost workspace root, a broken glob parser or a renamed + // field is a failure and not a quiet pass. + expect(declaringPackages.length).toBeGreaterThanOrEqual(7); expect(falsePackages.length).toBeGreaterThanOrEqual(5); - expect(arrayPackages.length).toBeGreaterThanOrEqual(1); + expect(arrayPackages.length).toBeGreaterThanOrEqual(2); // The census by name. `components`/`fields` declare `true` and are excluded // on purpose: `true` is the conservative claim and can never lose a // registration, so it is not falsifiable in the direction that hurts. expect(declaringPackages.map((p) => p.name)).toEqual([ + '@object-ui/app-shell', '@object-ui/core', '@object-ui/i18n', '@object-ui/layout', @@ -1232,18 +1301,27 @@ describe('`sideEffects` declarations match load-time behaviour (objectui#3943)', // list look maintained when it is not. const phantom = arrayPackages.flatMap((pkg) => { const forms = entryForms.get(pkg.name) ?? []; + const modules = packageModuleForms(pkg); return (pkg.declared as string[]) - .filter((entry) => !forms.includes(entry)) - .map((entry) => `${pkg.name}: "${entry}" is not a module form of this package`); + .filter((entry) => !forms.includes(entry) && !modules.has(entry)) + .map((entry) => `${pkg.name}: "${entry}" is neither an entry form nor a module of this package`); }); expect( phantom, [ - '`sideEffects` names a path that is not a resolvable module form of its package.', - 'If a NEW module genuinely has load-time side effects, teach resolvableEntryPaths() where it comes', - 'from (an `exports` subpath, a build output, a bundler alias) so the derivation stays the', - 'authority. Otherwise delete it.', + '`sideEffects` names a path that is neither a resolvable entry form nor a module in the package’s', + 'entry graph — it points at nothing at all.', + '', + 'A DEEP module is legitimate here (objectui#6683: `@object-ui/app-shell` names the ten modules that', + 'register SDUI widgets at load time, because those are exactly the ones a bundler must not drop).', + 'What is not legitimate is a path no derivation can find. If a NEW module genuinely has load-time', + 'side effects, make sure the barrel really reaches it; if a new ENTRY form appeared, teach', + 'resolvableEntryPaths() where it comes from (an `exports` subpath, a build output, a bundler alias)', + 'so the derivation stays the authority. Otherwise delete it.', + '', + 'Whether the set is EXACTLY right — no missing registrar, no stale name — is the separate question', + '`scripts/check-side-effects-array.mjs` answers by re-deriving it from the module bodies.', '', ...phantom, ].join('\n'), @@ -1262,6 +1340,11 @@ describe('`sideEffects` declarations match load-time behaviour (objectui#3943)', const phantomClaims = arrayPackages.flatMap((pkg) => (pkg.declared as string[]) .filter((entry) => isSourceEntry(pkg, entry)) + // A `.css` entry form is a real `sideEffects` surface — a dropped + // stylesheet import is webpack's classic silent bug — but its effect IS + // the stylesheet, and running a TypeScript parser over it would report + // "no top-level side effect" about a file that has no top level. + .filter((entry) => isParseableModule(entry)) .filter((entry) => scanModule(path.join(repoRoot, pkg.dir, entry)).effects.length === 0) .map((entry) => `${pkg.name}: "${entry}" is declared in \`sideEffects\` but has no top-level side effect`), ); @@ -1284,9 +1367,16 @@ describe('`sideEffects` declarations match load-time behaviour (objectui#3943)', // `src/index.ts` is the one source entry any array declares today, and its // `try { registerLayout(); } catch {}` is the effect being detected. const sourceEntries = arrayPackages.flatMap((pkg) => - (pkg.declared as string[]).filter((entry) => isSourceEntry(pkg, entry)).map((entry) => `${pkg.name}/${entry}`), + (pkg.declared as string[]) + .filter((entry) => isSourceEntry(pkg, entry) && isParseableModule(entry)) + .map((entry) => `${pkg.name}/${entry}`), ); expect(sourceEntries).toContain('@object-ui/layout/src/index.ts'); + // objectui#6683's specimen: a DEEP module, not an entry form. Its presence + // is what makes the loop above cover the general case rather than one + // barrel, and it is one of the three registrations the 2026-08-29 ruling + // names as controls. + expect(sourceEntries).toContain('@object-ui/app-shell/src/console/connect/ConnectAgentWidget.tsx'); }); it('no array package declares a wildcard entry (this guard resolves literal paths)', () => { diff --git a/scripts/check-eager-closure-budget.mjs b/scripts/check-eager-closure-budget.mjs index b6025aa56f..899772bd99 100644 --- a/scripts/check-eager-closure-budget.mjs +++ b/scripts/check-eager-closure-budget.mjs @@ -48,9 +48,9 @@ * * - It must PASS on today's `main`. A gate that lands red is a gate someone * disables, and this one replaced a gate nobody could fail. Headroom above - * the current 3,299,898 bytes: 45,102 (1.37%). + * the current 3,254,004 bytes: 45,996 (1.41%). * - The headroom must stay SMALLER than the regression the gate exists to - * catch. objectui#5266 was 89 KiB = 91,136 bytes; 45,102 < 91,136, so this + * catch. objectui#5266 was 89 KiB = 91,136 bytes; 45,996 < 91,136, so this * ceiling would have failed on that change. Widening the headroom past ~89 * KiB would leave the gate green through a repeat of its own motivating * incident. @@ -128,6 +128,25 @@ * triage disposition 3) rather than silently, which is what the "Raising it" * note below asks of a re-baseline in either direction. * + * objectui#6683 lowered it again, to 3,300,000 over 3,254,004 — and this one was + * EARNED rather than drifted into. `@object-ui/app-shell` now publishes a + * precise `sideEffects` array (guarded by + * `scripts/check-side-effects-array.mjs`), which made 56,668 gzipped bytes of + * the barrel's closure shakeable. That is LARGER than the 45,102 bytes of + * headroom the 3,345,000 ceiling carried, so the ceiling had to move with it or + * the aggregate gauge would sit at 1.00x its own sensitivity — the blind band + * reopening the same day it was measured shut. The two numbers move in ONE + * commit for the reason the paragraph above gives. + * + * ⚠️ Read the direction correctly: the closure did NOT fall by the 242.6 KB the + * objectui#6683 card projected. That figure was measured for + * `"sideEffects": false`, which is closed by measurement because it also DROPS + * three live SDUI widget registrations. The precise array keeps them, and + * keeping them keeps their import closure eager; 56,668 bytes is what the + * correct declaration actually buys. The gap is not a defect in the array — it + * is the price of the correctness the ruling required, and the difference is + * recorded here rather than smoothed over. + * * ⛔ The floor is unchanged and applies to a LOWERING too: never put a ceiling * below a measured figure to express an aspiration. That is not a tighter * ratchet, it is a gate that lands red on `main`, which is how a budget gets @@ -191,14 +210,24 @@ import { isEntrypoint } from './invoked-as.mjs'; /** * Ceiling for the console eager closure, in gzipped bytes. See the header for - * how this number was chosen; measured 3,299,898 on `48e53814e`. - * - * Re-baselined DOWNWARD by objectui#5924 from 4,086,000 (derived from the - * 4,005,911 reading on `4c1623c0c`, which the payload had since fallen 706,013 - * bytes below). Headroom is now 45,102 bytes — 0.49x - * {@link REGRESSION_THIS_GATE_MUST_CATCH_BYTES}, where it had drifted to 8.6x. + * how this number was chosen; measured 3,254,004 on `SHA_PLACEHOLDER`. + * + * Re-baselined DOWNWARD twice, each time toward a measurement the payload had + * already fallen to: + * + * - objectui#5924, from 4,086,000 (derived from the 4,005,911 reading on + * `4c1623c0c`) to 3,345,000 over 3,299,898 on `48e53814e`. + * - objectui#6683, to 3,300,000 over 3,254,004. `@object-ui/app-shell` now + * declares a precise `sideEffects` ARRAY, which took 56,668 gzipped bytes + * out of the closure — more than the 45,102 of headroom the previous + * ceiling had, so leaving it in place would have parked the aggregate gauge + * at 1.00x its own sensitivity and, on the next byte of shrink, tripped the + * exit-2 verdict about the gauge. Lowering it in the SAME change is the + * tightening the maintainer ruling of 2026-08-29 asked for. + * + * Headroom is 45,996 bytes — 0.50x {@link REGRESSION_THIS_GATE_MUST_CATCH_BYTES}. */ -export const MAX_EAGER_CLOSURE_GZIP_BYTES = 3_345_000; +export const MAX_EAGER_CLOSURE_GZIP_BYTES = 3_300_000; /** * The measurement the ceiling above was derived from. Exported so the two @@ -209,10 +238,10 @@ export const MAX_EAGER_CLOSURE_GZIP_BYTES = 3_345_000; */ export const BASELINE = Object.freeze({ /** `emitEagerClosureReport`'s `eagerGzipBytes` on this commit. */ - gzipBytes: 3_299_898, - chunks: 52, - totalChunks: 508, - commit: '48e53814e', + gzipBytes: 3_254_004, + chunks: 48, + totalChunks: 513, + commit: 'SHA_PLACEHOLDER', }); /** diff --git a/scripts/check-sdui-registration-pins.mjs b/scripts/check-sdui-registration-pins.mjs new file mode 100644 index 0000000000..75c08a5558 --- /dev/null +++ b/scripts/check-sdui-registration-pins.mjs @@ -0,0 +1,260 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * check-sdui-registration-pins -- the registrations a `sideEffects` array + * PROMISES to keep must still be in the built console. + * + * node scripts/check-sdui-registration-pins.mjs # the verdict + * node scripts/check-sdui-registration-pins.mjs --list # the derived key set + * + * Run it after `pnpm --filter @object-ui/console build`; it weighs + * `apps/console/dist/assets/*.js`. + * + * ## Why a second gate, on the ARTIFACT (objectui#6683) + * + * `scripts/check-side-effects-array.mjs` is a static gate: it proves the array + * agrees with the module bodies. It cannot prove the one thing the hazard turns + * on -- that a real bundler, reading that array, still emits the registrations. + * That question has a wrong answer that is invisible from inside the source: + * `"sideEffects": false` on `@object-ui/app-shell` is statically coherent and + * DROPS three live SDUI widget registrations. Measured in objectui#6535 / + * PR #6682: `mcp:connect-agent`, `cloud:onboarding-next` and + * `cloud:ai-model-status` all fall to **0 chunks**, on a green build, with no + * warning anywhere -- the barrel reaches them through bare side-effect imports, + * and `false` licenses the bundler to skip exactly those. + * + * The maintainer ruling of 2026-08-29 names those three as the controls this + * work must pin present at >= 1 chunk each. This file is that pin, and it is + * wider than the three on purpose (see below). + * + * ## The key set is DERIVED from the array, not listed here + * + * The ruling requires the enumeration to be re-derived mechanically rather than + * copied. So the population is: every module the `sideEffects` array names, read + * with `scripts/component-registrations.mjs` -- this tree's ONE answer to "which + * component keys does this source register?". Change the array and the pinned + * key set changes with it; there is no second list to keep honest. + * + * `RULED_CONTROLS` below is not that list. It is an ANTI-VACUITY FLOOR: three + * keys the ruling names, asserted to be IN the derived set, so a derivation that + * quietly stopped seeing registrations cannot report "0 keys, all present". + * A floor is checked against the derivation; a list would replace it. + * + * ## Every zero here is loud, in both directions + * + * - a pinned key in 0 chunks -> the array dropped a registration (exit 1) + * - no dist to read -> exit 2. A gate that reports success over a + * tree it never measured is the failure mode + * of every budget gate in this repo. + * - 0 keys derived -> exit 2, for the same reason: "all present" + * over an empty set is green for nothing. + * - the matcher cannot MISS -> exit 2. A sentinel key that must be absent + * is searched for on every run, because a + * matcher that matches everything reports + * every key present and can never fail. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { findComponentRegistrations } from './component-registrations.mjs'; +import { readArrayPackages } from './check-side-effects-array.mjs'; +import { isEntrypoint } from './invoked-as.mjs'; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +const DEFAULT_DIST = 'apps/console/dist/assets'; + +export const EXIT_OK = 0; +export const EXIT_DROPPED = 1; +export const EXIT_NO_MEASUREMENT = 2; + +/** + * The three registrations the 2026-08-29 ruling names as this work's controls, + * each measured at 0 chunks under `"sideEffects": false`. + * + * ⚠️ This is a FLOOR on the derived set, never the set itself. It answers "is + * the derivation still seeing the registrations the ruling cared about?", and + * it is the only hand-written key list in this gate for that reason. + */ +export const RULED_CONTROLS = Object.freeze([ + 'cloud:ai-model-status', + 'cloud:onboarding-next', + 'mcp:connect-agent', +]); + +/** + * A key that must be found in ZERO chunks. Its absence is what proves the + * matcher below can return zero at all — without it, a matcher that reported + * every key present would pass every assertion in this file forever. + */ +export const NEGATIVE_CONTROL_KEY = 'objectui:6683-registration-pin-negative-control'; + +/** + * Every component key registered by a module some package's `sideEffects` array + * names. Derived; see the header. + * + * @param {string} [root] + * @returns {{keys: string[], sources: Map, unreadable: string[], modulesRead: number}} + */ +export function derivePinnedKeys(root = REPO_ROOT) { + const keys = []; + const sources = new Map(); + const unreadable = []; + let modulesRead = 0; + + for (const pkg of readArrayPackages(root)) { + for (const entry of pkg.declared) { + if (!/\.(ts|tsx|mts|js|jsx|mjs)$/.test(entry)) continue; + const abs = path.join(root, pkg.dir, entry); + if (!fs.existsSync(abs)) continue; // a `dist/*` spelling in an unbuilt tree + modulesRead += 1; + const rel = `${pkg.dir}/${entry}`; + const scan = findComponentRegistrations(fs.readFileSync(abs, 'utf8')); + for (const call of scan.unreadable) { + unreadable.push(`${rel}:${call.line}: ${call.text}`); + } + for (const key of scan.keys) { + if (!sources.has(key)) { + keys.push(key); + sources.set(key, rel); + } + } + } + } + + return { keys: keys.sort(), sources, unreadable, modulesRead }; +} + +/** The emitted JS chunks of a console build, absolute paths. */ +export function readChunks(distDir) { + if (!fs.existsSync(distDir)) return undefined; + return fs + .readdirSync(distDir) + .filter((f) => f.endsWith('.js')) + .map((f) => path.join(distDir, f)); +} + +/** + * How many chunks carry `key` as a QUOTED string literal. + * + * Quoted rather than a bare substring: a registry key like `attachments` occurs + * inside unrelated identifiers and prose all over a bundle, and a matcher that + * counted those would report every key present whatever the bundler did. + * Minified output uses whichever quote rolldown picked, so all three are tried. + */ +export function countChunksCarrying(chunkFiles, key, read = (f) => fs.readFileSync(f, 'utf8')) { + const needles = [`'${key}'`, `"${key}"`, `\`${key}\``]; + let found = 0; + for (const file of chunkFiles) { + const code = read(file); + if (needles.some((n) => code.includes(n))) found += 1; + } + return found; +} + +/** + * @param {string[]} [argv] + * @param {string} [root] + * @param {readonly string[]} [controls] the anti-vacuity FLOOR on the derived key + * set. Defaults to {@link RULED_CONTROLS}; a fixture workspace passes its own, + * so the floor stays under test rather than being switched off to test around it. + */ +export function main(argv = process.argv.slice(2), root = REPO_ROOT, controls = RULED_CONTROLS) { + const { keys, sources, unreadable, modulesRead } = derivePinnedKeys(root); + + if (unreadable.length > 0) { + console.error( + '❌ A module named in a `sideEffects` array registers a key this reader cannot read:\n' + + unreadable.map((u) => ` ${u}`).join('\n') + + '\n\n Returning the readable keys and dropping this one is the objectui#4894 failure: the dropped key\n' + + ' would simply never be asserted, and this gate would go green without ever weighing it. Write the\n' + + ' key as a plain string literal, or teach scripts/component-registrations.mjs to resolve this form.', + ); + return EXIT_NO_MEASUREMENT; + } + + if (keys.length === 0) { + console.error( + `❌ No registration keys were derived from any \`sideEffects\` array (${modulesRead} module(s) read).\n` + + ' "Every pinned key is present" over an empty set is green for an empty reason, which is the exact\n' + + ' shape this gate exists to reject.', + ); + return EXIT_NO_MEASUREMENT; + } + + const missingControls = controls.filter((k) => !sources.has(k)); + if (missingControls.length > 0) { + console.error( + `❌ The derivation no longer sees ${missingControls.length} of the controls this gate pins:\n` + + missingControls.map((k) => ` ${k}`).join('\n') + + '\n\n These are a FLOOR on the derived set, not the set itself. Either a `sideEffects` array stopped\n' + + ' naming the module that registers the key — which is the silent drop, and must be fixed rather\n' + + ' than re-pinned — or the reader has stopped seeing it.', + ); + return EXIT_NO_MEASUREMENT; + } + + if (argv.includes('--list')) { + console.log(`${keys.length} key(s) derived from ${modulesRead} module(s) named by a \`sideEffects\` array:`); + for (const key of keys) console.log(` ${key.padEnd(28)} ${sources.get(key)}`); + return EXIT_OK; + } + + const distDir = path.join(root, DEFAULT_DIST); + const chunks = readChunks(distDir); + if (chunks === undefined || chunks.length === 0) { + console.error( + `❌ No console build to weigh at ${DEFAULT_DIST}.\n` + + ' This is exit 2, not a pass: the registrations this gate pins are dropped by a WRONG ARRAY at\n' + + ' BUNDLE time, so a run with nothing to read has measured nothing. Build the console first:\n' + + ' pnpm --filter @object-ui/console build', + ); + return EXIT_NO_MEASUREMENT; + } + + // The matcher must be able to return zero, or every assertion below is + // unfalsifiable. + const sentinel = countChunksCarrying(chunks, NEGATIVE_CONTROL_KEY); + if (sentinel !== 0) { + console.error( + `❌ The negative control ${NEGATIVE_CONTROL_KEY} was found in ${sentinel} chunk(s).\n` + + ' Nothing registers it, so a matcher that finds it finds everything — and a gate whose matcher\n' + + ' cannot miss reports every key present whatever the bundler did.', + ); + return EXIT_NO_MEASUREMENT; + } + + const dropped = []; + const table = []; + for (const key of keys) { + const count = countChunksCarrying(chunks, key); + table.push({ key, count, source: sources.get(key) }); + if (count === 0) dropped.push({ key, source: sources.get(key) }); + } + + for (const row of table) { + console.log(` ${row.count === 0 ? '❌' : '✅'} ${row.key.padEnd(28)} ${String(row.count).padStart(2)} chunk(s) ${row.source}`); + } + + if (dropped.length > 0) { + console.error( + `\n❌ ${dropped.length} SDUI registration(s) named by a \`sideEffects\` array are in ZERO chunks of the\n` + + ` built console. The array told the bundler those modules were droppable and it dropped them —\n` + + ` silently, on a green build, exactly as \`"sideEffects": false\` does (objectui#6535/#6683).\n` + + dropped.map((d) => ` - ${d.key} (registered by ${d.source})`).join('\n'), + ); + return EXIT_DROPPED; + } + + console.log( + `✅ All ${keys.length} registration(s) a \`sideEffects\` array promises are present in the built console ` + + `(${chunks.length} chunks weighed; the ${controls.length} ruled control(s) are in the derived set).`, + ); + return EXIT_OK; +} + +if (isEntrypoint(import.meta.url)) { + process.exit(main()); +} diff --git a/scripts/check-side-effects-array.mjs b/scripts/check-side-effects-array.mjs new file mode 100644 index 0000000000..b2361db12e --- /dev/null +++ b/scripts/check-side-effects-array.mjs @@ -0,0 +1,726 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * check-side-effects-array -- a `sideEffects` ARRAY must name EXACTLY the + * modules that register something at load time. + * + * node scripts/check-side-effects-array.mjs # the verdict + * node scripts/check-side-effects-array.mjs --list # the enumeration, per package + * + * ## Why this gate exists (objectui#6683) + * + * `sideEffects` is a PUBLISHED CONTRACT: every consumer's bundler reads it and + * takes it at its word. `@object-ui/app-shell` declares it as an ARRAY because + * the two simpler answers are both wrong for this package, and both wrongnesses + * were MEASURED rather than argued: + * + * - omitting the field -> "assume every module does something on import", + * so nothing in the package is shakeable. Measured + * on the objectui#6683 branch: 3,310,672 bytes + * gzipped in the console's eager closure. + * - `sideEffects: false` -> the bundler drops every module whose exports go + * unused, including three live SDUI widget + * registrations (`mcp:connect-agent`, + * `cloud:onboarding-next`, `cloud:ai-model-status`) + * that the barrel pulls in through BARE side-effect + * imports. They fall to 0 chunks. Closed by + * measurement in objectui#6535 / PR #6682. + * + * The array is the honest third answer. Its failure mode is the reason this + * gate is not optional and ships in the SAME change as the array: an array that + * is INCOMPLETE fails **silently, in someone else's bundle**. No error, no + * warning, exit 0 -- the bundler believes it is executing a declaration rather + * than discovering a defect. That is the same failure mode as `false`, only + * harder to see, and the maintainer ruling of 2026-08-29 refuses a bare array + * for exactly that reason. + * + * ## The rule, stated once + * + * the array names EXACTLY: entry forms + * + every module in the package's entry graph that + * performs a top-level REGISTRATION, + * in BOTH its source and its published spelling + * + * Both directions are checked, because a `sideEffects` array can be wrong in + * two ways and only one of them is loud: + * + * - MISSING -- a registering module the array does not name. A bundler drops + * it and the registration is gone from a consumer's app. Silent. + * - STALE -- a name whose module no longer registers anything. It costs + * every consumer bytes, and it reads to the next author as "this module + * registers something", which is how a REAL entry gets deleted as noise. + * + * ## The enumeration is DERIVED, never listed + * + * The ruling is explicit that the implementing change re-derives the set + * mechanically and never reuses a hand-copied list, so there is no list of + * module paths anywhere in this file or in the array's neighbourhood -- the + * array in `package.json` is the CLAIM and this file is the DERIVATION, and the + * gate is the comparison of the two. A hand-copied enumeration would be a + * second source of truth free to rot exactly as quietly as the array it was + * meant to protect. + * + * ## What counts as a registration, and why an UNKNOWN effect is an ERROR + * + * {@link classifyEffect} sorts every top-level side effect a module's body + * performs into exactly three buckets, and refuses anything it does not + * recognise: + * + * - `registration` -- a top-level CALL or `new`. Not "a call whose name looks + * like `register`": a name test is an under-reading, and an under-reading + * here is precisely the silent drop. `ComponentRegistry.register(...)`, + * `registerAppComponent(...)` and a hypothetical `Registry.add(...)` are + * indistinguishable to a bundler and are treated alike here. + * - `local-binding-write` -- `X.displayName = 'X'` where `X` is declared in + * THIS module and the right-hand side calls nothing. It is provably + * module-local: a bundler that drops the module drops its target too, so + * nothing outside can observe the difference. This is the carve-out that + * keeps three pure React components (and, through them, the route views + * they anchor) shakeable. + * - `side-effect-only-import` -- `import './x.js';`. A PROPAGATION edge, not + * an effect of its own. It is handled by {@link checkReachability} below + * rather than by making its importer unshakeable. + * + * Anything else is `unknown` and fails the gate with exit 2. That asymmetry is + * the whole design: a new spelling of a load-time effect must make this gate + * LOUD, never make it quietly decide the module is pure. "I did not recognise + * that" and "that is not a registration" must not be the same answer. + * + * ## Reachability -- naming a module is not enough + * + * A module named in the array is retained only when a RETAINED module still + * imports it. So the array's promise only holds if every registering module is + * reachable from an entry form through modules that are THEMSELVES covered. + * A chain `barrel -> pure-helper -> registrar` breaks: the pure helper is + * shakeable, so when its exports go unused it is dropped and takes the + * registrar's edge with it -- the registrar is named, retained by nobody, and + * gone. {@link checkReachability} rejects that shape. + * + * ## Scope, and why this is not the same gate as the consistency pin + * + * `scripts/__tests__/side-effects-declaration-consistency.test.ts` + * (objectui#3943) asks whether a declaration AGREES WITH module bodies across + * the whole workspace, and proves with a real bundler that the field is honoured + * at all. It is the wider gate and it stays the authority on that question. + * + * This file asks the narrower one the ruling names: for an ARRAY, is the array + * the exact enumeration? The two are deliberately independent -- they derive the + * population by different routes (that one folds in the workspace ALIAS tables; + * this one derives the source barrel and the published spelling from the + * manifest) -- because two guards that share a derivation fail together. + * + * Exit codes follow this tree's convention that a broken gauge must be LOUDER + * than a reading over the line: + * + * 0 -- every array agrees with its enumeration + * 1 -- an array disagrees (missing / stale / unreachable) + * 2 -- no trustworthy enumeration (unknown effect, unresolved specifier, + * no package found, a spelling map that does not round-trip) + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; +import { isEntrypoint } from './invoked-as.mjs'; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +/** Extensions a bundler tries, in Vite's own order. `.tsx` is why this matters. */ +const RESOLVE_EXTENSIONS = ['.mjs', '.js', '.mts', '.ts', '.jsx', '.tsx']; + +/** Files whose bodies this gate can parse. A `.css` entry form is not one. */ +const MODULE_FILE_RE = /\.(ts|tsx|mts|js|jsx|mjs)$/; + +export const EXIT_OK = 0; +export const EXIT_DISAGREES = 1; +export const EXIT_NO_MEASUREMENT = 2; + +/** `"./dist/index.js"` and `"dist/index.js"` name one file; compare on this. */ +export const normalize = (p) => (p.startsWith('./') ? p.slice(2) : p); + +/* -------------------------------------------------------------------------- */ +/* The workspace. */ +/* -------------------------------------------------------------------------- */ + +/** + * The `packages:` globs from `pnpm-workspace.yaml`, read rather than hardcoded + * so a new workspace root is covered the day it is added. Understands only the + * two shapes the file uses; anything else THROWS rather than being skipped, + * because a guard that silently stops looking at part of the workspace goes on + * reporting success over a shrinking surface. + * + * @param {string} [root] + * @returns {string[]} + */ +export function workspaceGlobs(root = REPO_ROOT) { + const yaml = fs.readFileSync(path.join(root, 'pnpm-workspace.yaml'), 'utf8'); + const lines = yaml.split('\n'); + const start = lines.findIndex((l) => /^packages:\s*$/.test(l)); + if (start === -1) throw new Error('pnpm-workspace.yaml no longer declares a top-level `packages:` key.'); + + const globs = []; + for (const line of lines.slice(start + 1)) { + if (/^\s*(#.*)?$/.test(line)) continue; + if (!/^\s/.test(line)) break; + const match = line.match(/^\s*-\s*['"]?([^'"#\s]+)['"]?\s*(#.*)?$/); + if (!match) { + throw new Error( + `Unparsed entry in pnpm-workspace.yaml \`packages:\`: ${JSON.stringify(line)} — teach this guard the new syntax.`, + ); + } + globs.push(match[1]); + } + return globs; +} + +/** @param {string} [root] @returns {string[]} absolute package directories. */ +export function workspacePackageDirs(root = REPO_ROOT) { + const dirs = []; + for (const glob of workspaceGlobs(root)) { + if (glob.endsWith('/*')) { + const parent = path.join(root, glob.slice(0, -2)); + if (!fs.existsSync(parent)) continue; + for (const d of fs.readdirSync(parent)) { + const full = path.join(parent, d); + if (fs.statSync(full).isDirectory()) dirs.push(full); + } + } else if (!glob.includes('*')) { + dirs.push(path.join(root, glob)); + } else { + throw new Error(`Unsupported workspace glob ${JSON.stringify(glob)} — teach this guard how to expand it.`); + } + } + return dirs; +} + +/** + * Every workspace package whose `sideEffects` is an ARRAY. + * + * `false`, `true` and an omitted field are all out of scope here by design: + * this gate is about the content of an array. The `false` direction is the + * objectui#3943 consistency pin's, and `true`/omitted are the conservative + * claim, which can never lose a registration. + * + * @param {string} [root] + */ +export function readArrayPackages(root = REPO_ROOT) { + const found = []; + for (const dir of workspacePackageDirs(root)) { + const pkgPath = path.join(dir, 'package.json'); + if (!fs.existsSync(pkgPath)) continue; + const manifest = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + if (!manifest.name || !Array.isArray(manifest.sideEffects)) continue; + if (manifest.sideEffects.some((e) => typeof e !== 'string')) { + throw new Error(`${manifest.name} declares a non-string \`sideEffects\` entry — teach this guard that shape.`); + } + found.push({ + name: manifest.name, + dir: path.relative(root, dir).split(path.sep).join('/'), + manifest, + declared: manifest.sideEffects.map(normalize), + }); + } + return found.sort((a, b) => a.name.localeCompare(b.name)); +} + +/* -------------------------------------------------------------------------- */ +/* The static scan. */ +/* -------------------------------------------------------------------------- */ + +/** Statement kinds that RUN when the module is evaluated (rather than declaring). */ +function isExecutedStatement(stmt) { + return ( + ts.isIfStatement(stmt) || + ts.isForStatement(stmt) || + ts.isForOfStatement(stmt) || + ts.isForInStatement(stmt) || + ts.isWhileStatement(stmt) || + ts.isDoStatement(stmt) || + // `try { registerLayout(); } catch {}` — objectui#3899's actual shape. + ts.isTryStatement(stmt) || + ts.isSwitchStatement(stmt) || + ts.isBlock(stmt) || + ts.isLabeledStatement(stmt) || + ts.isThrowStatement(stmt) + ); +} + +/** Whether `node` contains a call or a construction anywhere inside it. */ +function containsCall(node) { + let hit = false; + const walk = (n) => { + if (hit) return; + if (ts.isCallExpression(n) || ts.isNewExpression(n)) hit = true; + else ts.forEachChild(n, walk); + }; + walk(node); + return hit; +} + +/** Every identifier declared at the top level of this source file. */ +function moduleScopeBindings(source) { + const names = new Set(); + for (const stmt of source.statements) { + if (ts.isVariableStatement(stmt)) { + for (const d of stmt.declarationList.declarations) { + if (ts.isIdentifier(d.name)) names.add(d.name.text); + } + } else if ( + (ts.isFunctionDeclaration(stmt) || ts.isClassDeclaration(stmt)) && + stmt.name && + ts.isIdentifier(stmt.name) + ) { + names.add(stmt.name.text); + } + } + return names; +} + +/** + * The kind of one top-level side effect: `registration`, `local-binding-write`, + * `side-effect-only-import`, or `unknown`. + * + * `unknown` is the load-bearing return. See the header: a spelling this gate + * does not recognise must be LOUD, not quietly filed as pure. + * + * @param {import('typescript').Statement} stmt + * @param {Set} localBindings identifiers declared at module scope. + * @returns {'registration' | 'local-binding-write' | 'side-effect-only-import' | 'unknown' | null} + * `null` means the statement performs no top-level side effect at all. + */ +export function classifyEffect(stmt, localBindings) { + if (ts.isImportDeclaration(stmt)) { + return stmt.importClause ? null : 'side-effect-only-import'; + } + if (ts.isExportDeclaration(stmt)) return null; + + if (ts.isExpressionStatement(stmt)) { + // A bare string is a directive prologue (`'use client'`). + if (ts.isStringLiteral(stmt.expression)) return null; + if (containsCall(stmt)) return 'registration'; + + const expr = stmt.expression; + if ( + ts.isBinaryExpression(expr) && + expr.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isPropertyAccessExpression(expr.left) + ) { + // `X.displayName = 'X'` where `X` is this module's own binding: nothing + // outside can observe it once the module is dropped, and the right-hand + // side is already known call-free (`containsCall` above returned false). + const target = expr.left.expression; + if (ts.isIdentifier(target) && localBindings.has(target.text)) return 'local-binding-write'; + return 'unknown'; + } + if (ts.isBinaryExpression(expr) || ts.isElementAccessExpression(expr) || ts.isPropertyAccessExpression(expr)) { + return 'unknown'; + } + // Everything left is an expression evaluated for nothing: `1;`, `x;`. + return null; + } + + if (isExecutedStatement(stmt)) { + if (containsCall(stmt)) return 'registration'; + return 'unknown'; + } + + return null; +} + +/** + * One module's top-level effects and its relative import edges. + * + * @param {string} absFile + * @param {string} [root] + */ +export function scanModule(absFile, root = REPO_ROOT) { + const source = ts.createSourceFile( + absFile, + fs.readFileSync(absFile, 'utf8'), + ts.ScriptTarget.Latest, + /* setParentNodes */ true, + ); + const rel = path.relative(root, absFile).split(path.sep).join('/'); + const at = (n) => source.getLineAndCharacterOfPosition(n.getStart(source)).line + 1; + const firstLine = (n) => n.getText(source).split('\n')[0].trim().slice(0, 120); + const localBindings = moduleScopeBindings(source); + + const effects = []; + const edges = []; + + for (const stmt of source.statements) { + if (ts.isImportDeclaration(stmt) || ts.isExportDeclaration(stmt)) { + const specifier = stmt.moduleSpecifier; + if (specifier && ts.isStringLiteral(specifier) && specifier.text.startsWith('.')) { + edges.push({ specifier: specifier.text, bare: ts.isImportDeclaration(stmt) && !stmt.importClause }); + } + } + const kind = classifyEffect(stmt, localBindings); + if (kind !== null) effects.push({ file: rel, line: at(stmt), kind, text: firstLine(stmt) }); + } + + return { effects, edges }; +} + +/** + * Resolve a relative specifier the way this repo's TypeScript sources spell + * them: ESM-style, with a `.js` extension naming the `.ts` file next to it. + * Getting this wrong does not fail loudly on its own — it silently truncates + * the reachable set — so the caller REPORTS an unresolved specifier instead of + * skipping it. + */ +export function resolveRelative(fromFile, specifier) { + const rewritten = specifier.replace(/\.js$/, '.ts').replace(/\.jsx$/, '.tsx').replace(/\.mjs$/, '.mts'); + for (const candidate of [rewritten, specifier]) { + const abs = path.resolve(path.dirname(fromFile), candidate); + if (fs.existsSync(abs) && fs.statSync(abs).isFile()) return abs; + } + const base = path.resolve(path.dirname(fromFile), specifier.replace(/\.(js|jsx|mjs)$/, '')); + for (const ext of RESOLVE_EXTENSIONS) { + if (fs.existsSync(base + ext)) return base + ext; + } + if (fs.existsSync(base) && fs.statSync(base).isDirectory()) { + for (const ext of RESOLVE_EXTENSIONS) { + const index = path.join(base, `index${ext}`); + if (fs.existsSync(index)) return index; + } + } + return undefined; +} + +/** + * The barrel plus every module reachable from it by relative import: the set a + * bundler may shake, and therefore the set the declaration is a promise about. + * + * Bare package specifiers stop the walk — another package's manifest is that + * package's problem. + * + * @param {string} entryFile absolute path to the source barrel. + * @param {string} [root] + */ +export function walkEntryGraph(entryFile, root = REPO_ROOT) { + const seen = new Set(); + /** @type {Map} */ + const scans = new Map(); + /** @type {Map} */ + const importedBy = new Map(); + const unresolved = []; + const stack = [entryFile]; + + while (stack.length > 0) { + const file = stack.pop(); + if (seen.has(file)) continue; + seen.add(file); + if (!MODULE_FILE_RE.test(file) || /\.d\.ts$/.test(file)) continue; + + const scan = scanModule(file, root); + scans.set(file, scan); + for (const edge of scan.edges) { + const resolved = resolveRelative(file, edge.specifier); + if (!resolved) { + unresolved.push(`${path.relative(root, file)} -> ${edge.specifier}`); + continue; + } + const list = importedBy.get(resolved) ?? []; + list.push({ from: file, bare: edge.bare }); + importedBy.set(resolved, list); + stack.push(resolved); + } + } + + return { modules: [...seen], scans, importedBy, unresolved }; +} + +/* -------------------------------------------------------------------------- */ +/* Entry forms and the source <-> published spelling map. */ +/* -------------------------------------------------------------------------- */ + +/** + * Every module path a bundler can resolve the PACKAGE to, package-relative, + * derived from the manifest. + * + * `types` is skipped: type declarations are erased and are never a bundling + * surface. `*` patterns are skipped because they name no single file — and a + * package that grows one while declaring an array is reported by + * {@link evaluatePackage} rather than silently dropped. + */ +export function manifestEntryForms(manifest) { + const found = new Set(); + for (const field of [manifest.main, manifest.module]) { + if (typeof field === 'string') found.add(normalize(field)); + } + const walk = (node) => { + if (node === null || node === undefined) return; + if (typeof node === 'string') { + if (node.startsWith('./')) found.add(normalize(node)); + return; + } + if (typeof node !== 'object') return; + for (const [key, value] of Object.entries(node)) { + if (key === 'types') continue; + walk(value); + } + }; + walk(manifest.exports); + return [...found].sort(); +} + +/** + * The map between a package's SOURCE spelling and its PUBLISHED spelling, and + * the source barrel both are anchored on. + * + * Derived, not configured: the published barrel comes from the manifest, the + * source barrel is found on disk beside it, and the transform is whatever turns + * one into the other (`src/` -> `dist/`, `.ts`/`.tsx` -> `.js`). The derivation + * is then required to ROUND-TRIP on the barrel itself, which is the anti-vacuity + * check: a map that cannot reproduce the one pair it was derived from would + * quietly mis-spell every other module. + */ +export function deriveSpellingMap(pkg, root = REPO_ROOT) { + const forms = manifestEntryForms(pkg.manifest); + const publishedBarrel = forms.find((f) => /(^|\/)index\.(js|mjs|cjs)$/.test(f)); + if (!publishedBarrel) { + return { error: `${pkg.name}: no published barrel (an \`index.js\`-shaped entry) in main/module/exports` }; + } + + const pkgAbs = path.join(root, pkg.dir); + let sourceBarrel; + for (const ext of RESOLVE_EXTENSIONS) { + const candidate = `src/index${ext}`; + if (fs.existsSync(path.join(pkgAbs, candidate))) { + sourceBarrel = candidate; + break; + } + } + if (!sourceBarrel) { + return { error: `${pkg.name}: no source barrel at src/index.* — this gate reads module bodies, so it cannot proceed` }; + } + + const srcRoot = sourceBarrel.split('/')[0]; + const distRoot = publishedBarrel.split('/')[0]; + const publishedExt = path.extname(publishedBarrel); + + /** `src/a/b.tsx` -> `dist/a/b.js`, by the transform derived above. */ + const toPublished = (sourceRel) => + `${distRoot}/${sourceRel.slice(srcRoot.length + 1).replace(/\.(tsx|ts|mts|jsx|js|mjs)$/, publishedExt)}`; + + if (toPublished(sourceBarrel) !== publishedBarrel) { + return { + error: + `${pkg.name}: the source/published spelling map does not round-trip on the barrel — ` + + `${sourceBarrel} maps to ${toPublished(sourceBarrel)} but the manifest publishes ${publishedBarrel}`, + }; + } + + return { forms, sourceBarrel, publishedBarrel, srcRoot, distRoot, toPublished }; +} + +/* -------------------------------------------------------------------------- */ +/* The verdict. */ +/* -------------------------------------------------------------------------- */ + +/** + * A registering module is only retained when a RETAINED module still imports + * it. This walks back from each registrar to an entry form through COVERED + * modules only, so a `barrel -> pure-helper -> registrar` chain — where the + * shakeable helper takes the registrar's only edge with it — is a failure and + * not a green tick. + */ +export function checkReachability(graph, registrars, sourceBarrelAbs, root = REPO_ROOT) { + const covered = new Set([sourceBarrelAbs, ...registrars]); + const reachable = new Set([sourceBarrelAbs]); + let grew = true; + while (grew) { + grew = false; + for (const file of covered) { + if (reachable.has(file)) continue; + const importers = graph.importedBy.get(file) ?? []; + if (importers.some((i) => reachable.has(i.from))) { + reachable.add(file); + grew = true; + } + } + } + return registrars + .filter((r) => !reachable.has(r)) + .map((r) => path.relative(root, r).split(path.sep).join('/')); +} + +/** + * The whole verdict for one array-declaring package. + * + * @returns {{name: string, ok: boolean, gauge: boolean, expected: string[], declared: string[], + * missing: string[], stale: string[], registrars: string[], problems: string[], + * modulesWalked: number}} + */ +export function evaluatePackage(pkg, root = REPO_ROOT) { + const problems = []; + const map = deriveSpellingMap(pkg, root); + if (map.error) { + return { + name: pkg.name, ok: false, gauge: true, expected: [], declared: pkg.declared, + missing: [], stale: [], registrars: [], problems: [map.error], modulesWalked: 0, + }; + } + + const pkgAbs = path.join(root, pkg.dir); + const sourceBarrelAbs = path.join(pkgAbs, map.sourceBarrel); + const graph = walkEntryGraph(sourceBarrelAbs, root); + + for (const u of graph.unresolved) { + problems.push( + `${pkg.name}: unresolved relative specifier ${u} — an unwalked edge silently SHRINKS the enumeration, ` + + `so it is reported rather than skipped`, + ); + } + + const registrars = []; + for (const [file, scan] of graph.scans) { + for (const effect of scan.effects) { + if (effect.kind === 'unknown') { + problems.push( + `${pkg.name}: ${effect.file}:${effect.line} performs a top-level side effect this gate does not ` + + `recognise (${effect.text}). Teach \`classifyEffect\` what it is — an unrecognised effect must ` + + `never be read as "not a registration", which is the silent drop this gate exists to prevent.`, + ); + } + } + if (scan.effects.some((e) => e.kind === 'registration')) registrars.push(file); + } + registrars.sort(); + + // A registering module needs BOTH spellings: consumers resolve the published + // one, in-repo bundler aliases resolve the source one, and a bundler reads the + // same manifest for both. + const expected = new Set(map.forms); + expected.add(map.sourceBarrel); + for (const abs of registrars) { + const rel = path.relative(pkgAbs, abs).split(path.sep).join('/'); + expected.add(rel); + expected.add(map.toPublished(rel)); + } + + const declared = new Set(pkg.declared); + const missing = [...expected].filter((e) => !declared.has(e)).sort(); + const stale = [...declared].filter((d) => !expected.has(d)).sort(); + + for (const entry of pkg.declared) { + if (entry.includes('*')) { + problems.push( + `${pkg.name}: "${entry}" is a glob. This gate compares literal paths, so a pattern would make the ` + + `comparison vacuous on whatever it covers — spell the modules out.`, + ); + } + } + + const unreachable = checkReachability(graph, registrars, sourceBarrelAbs, root); + const reachabilityProblems = unreachable.map( + (m) => + `${pkg.name}: ${m} registers at load time, but no chain of \`sideEffects\`-covered modules reaches it ` + + `from the barrel. Naming it is not enough — every module on the path to it is shakeable and will take ` + + `its only edge with it.`, + ); + + if (graph.modules.length < 2) { + problems.push( + `${pkg.name}: the entry graph walked ${graph.modules.length} module(s) from ${map.sourceBarrel} — ` + + `an enumeration over an empty graph agrees with any array at all`, + ); + } + + return { + name: pkg.name, + ok: missing.length === 0 && stale.length === 0 && problems.length === 0 && reachabilityProblems.length === 0, + gauge: problems.length > 0, + expected: [...expected].sort(), + declared: [...declared].sort(), + missing, + stale, + registrars: registrars.map((r) => path.relative(pkgAbs, r).split(path.sep).join('/')), + problems: [...problems, ...reachabilityProblems], + modulesWalked: graph.modules.length, + }; +} + +/** @param {string} [root] */ +export function evaluate(root = REPO_ROOT) { + const packages = readArrayPackages(root); + return { packages, results: packages.map((p) => evaluatePackage(p, root)) }; +} + +/* -------------------------------------------------------------------------- */ + +export function main(argv = process.argv.slice(2), root = REPO_ROOT) { + const { packages, results } = evaluate(root); + + // Anti-vacuity, first and loudest. Every assertion below is a set difference, + // and every set difference passes trivially over nothing. + if (packages.length === 0) { + console.error( + '❌ No workspace package declares `sideEffects` as an array.\n' + + ' This gate is a set comparison, and a set comparison over an empty population is green for an\n' + + ' empty reason. Either the field was removed (then this gate has to be retired deliberately, not\n' + + ' left passing) or the workspace walk has stopped seeing the packages.', + ); + return EXIT_NO_MEASUREMENT; + } + + if (argv.includes('--list')) { + for (const r of results) { + console.log(`\n${r.name} — ${r.registrars.length} module(s) with a top-level registration, ${r.modulesWalked} walked`); + for (const m of r.registrars) console.log(` ${m}`); + } + return EXIT_OK; + } + + let gauge = false; + let disagrees = false; + + for (const r of results) { + if (r.problems.length > 0) { + gauge = true; + for (const p of r.problems) console.error(`❌ ${p}`); + continue; + } + if (r.missing.length > 0 || r.stale.length > 0) { + disagrees = true; + console.error(`❌ ${r.name}: \`sideEffects\` disagrees with the derived enumeration.`); + for (const m of r.missing) { + console.error( + ` MISSING "./${m}" — this module registers at load time (or is an entry form) and the array does ` + + `not name it. A bundler will drop it from a consumer's app, silently.`, + ); + } + for (const s of r.stale) { + console.error( + ` STALE "./${s}" — the array names it, but nothing in it registers at load time any more. It ` + + `costs every consumer bytes and it reads to the next author as a live registration.`, + ); + } + console.error( + ` The enumeration is DERIVED here, never listed: fix the array, or fix the module — whichever half\n` + + ` is currently false. Run \`node scripts/check-side-effects-array.mjs --list\` to see the set.`, + ); + continue; + } + console.log( + `✅ ${r.name}: \`sideEffects\` names exactly the ${r.registrars.length} module(s) that register at load ` + + `time, plus its entry forms (${r.declared.length} entries, ${r.modulesWalked} modules walked).`, + ); + } + + if (gauge) { + console.error( + '\nExit 2 — this is a verdict about the GAUGE, not about the array. Nothing above says the declaration\n' + + 'is wrong; it says the enumeration could not be trusted, which must never be reported as a pass.', + ); + return EXIT_NO_MEASUREMENT; + } + return disagrees ? EXIT_DISAGREES : EXIT_OK; +} + +if (isEntrypoint(import.meta.url)) { + process.exit(main()); +} diff --git a/scripts/vite-declared-lazy-views.ts b/scripts/vite-declared-lazy-views.ts index 69de27ca76..e5786212b8 100644 --- a/scripts/vite-declared-lazy-views.ts +++ b/scripts/vite-declared-lazy-views.ts @@ -55,14 +55,24 @@ import type { Plugin, Rollup } from 'vite'; * * The fix is to tell the CONSOLE's build what is true of these specific * modules: they are pure React components, so `moduleSideEffects: false`. That - * is deliberately narrower than adding `"sideEffects"` to - * `packages/app-shell/package.json`, which would be the general fix and is NOT - * done here — measured on this branch, `"sideEffects": false` on that package - * silently DROPS three real SDUI widget registrations from the bundle - * (`mcp:connect-agent`, `cloud:onboarding-next`, `cloud:ai-model-status`, all - * registered by bare side-effect imports in the barrel), and an incomplete - * `sideEffects` ARRAY would do the same to third-party embedders with nothing - * to catch it. That is a published-contract decision, not this card's repair. + * was deliberately narrower than adding `"sideEffects"` to + * `packages/app-shell/package.json`, which is the general fix and was reserved + * as a published-contract decision rather than taken in-lane. + * + * ⚠️ That decision has since been taken (objectui#6683, maintainer ruling of + * 2026-08-29): the package now declares a precise `sideEffects` ARRAY, guarded + * by `scripts/check-side-effects-array.mjs`. `"sideEffects": false` stays + * disproven and is NOT what shipped — measured, it silently DROPS three real + * SDUI widget registrations (`mcp:connect-agent`, `cloud:onboarding-next`, + * `cloud:ai-model-status`, all registered through bare side-effect imports in + * the barrel). The ARRAY names those modules, and + * `scripts/check-sdui-registration-pins.mjs` weighs the built console for them. + * + * This plugin is NOT made redundant by that. It is scoped to AppContent's + * declared-lazy route views, which are pure components with no registration of + * their own, so the package array does not — and must not — name them; the + * console-local declaration is what keeps them shakeable, and the ledger below + * is what keeps the agreement honest in both directions. * * ## Defect 2 — edges the barrel has nothing to do with (three views) * @@ -128,37 +138,37 @@ export const EAGER_WALK_CONTROL = 'packages/app-shell/src/views/ObjectView.tsx'; * (`scripts/__tests__/vite-declared-lazy-views.test.ts` checks that, and that * every entry still names a file that exists). * - * All three stand for reasons that are NOT the barrel re-export this card - * removed, and none of the three is fixable by an import spelling. Measured on - * `ece68882`, from the emitted chunks' own module lists: + * One entry stands, and it stands for a reason that is NOT the barrel + * re-export objectui#6535 removed: * * - `RecordDetailView` — a real static edge. * `packages/app-shell/src/views/ObjectView.tsx` imports it by name, and * `ObjectView` sits in AppContent's own "eagerly loaded — always needed" * block. Splitting it would mean giving `ObjectView` a lazy boundary. * - * - `RecordFormPage` — CHUNK CO-TENANCY, not an import of the view at all. - * Rolldown emits it in a chunk it shares with - * `packages/app-shell/src/providers/expressionUser.ts` (objectui#6515's leaf - * module), which `AppContent` imports statically and the barrel re-exports - * to the console's `InternalFormRoute`. The co-tenant is eager, so the whole - * chunk is — the view's bytes ride along. + * ## Two entries were REMOVED here, and that removal is a recorded win + * + * `RecordFormPage` and `ReportView` were pinned for a third reason, the one no + * `grep` over the source shows: CHUNK CO-TENANCY (objectui#6680). Rolldown + * emitted each of them in a chunk shared with a module that IS eagerly used — + * `providers/expressionUser.ts` for the first, `views/RuntimeDraftBar.tsx` for + * the second — so the whole chunk was eager and the view's bytes rode along + * with no import edge to the view itself. * - * - `ReportView` — the same shape. Its chunk also carries - * `views/ReportConfigPanel.tsx` and `views/RuntimeDraftBar.tsx`, and - * `views/ViewConfigPanel.tsx` (a barrel export the console uses) imports - * `RuntimeDraftBar` statically. + * objectui#6683 declared `"sideEffects"` on `@object-ui/app-shell` as a precise + * ARRAY, which makes those co-tenant modules shakeable in their own right; the + * chunks they anchored stopped being eager and both views fell out of the + * closure. The `missing` half of this ledger fired on that build and named both + * views by path, which is what a recorded win looks like here — the lines are + * deleted because the build said so, not because the walk was assumed healthy. * - * The last two are a different defect from the one objectui#6535 names, they - * are not repaired by anything in the console's import graph, and the obvious - * lever — an `advancedChunks` group that isolates the shared leaves — is a - * chunking-policy change that needs its own measurement. Filed separately; - * pinned here so the bytes are recorded rather than implied. + * ⚠️ Deleting a line is a MEASUREMENT, never a repair. Counter-probe 1 below + * (every declared view must be found in SOME chunk) is what separates "the view + * became lazy" from "the matcher stopped matching", and it ran green on the + * same build. */ export const DECLARED_LAZY_VIEWS_STILL_EAGER: readonly string[] = Object.freeze([ 'packages/app-shell/src/views/RecordDetailView.tsx', - 'packages/app-shell/src/views/RecordFormPage.tsx', - 'packages/app-shell/src/views/ReportView.tsx', ]); /** From f905b0753c39b9fcd18dbb2dbf998a5d820e5c81 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 04:31:51 +0000 Subject: [PATCH 2/2] chore(budget): record the commit the re-baselined eager-closure measurement was taken on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The constant is not a build input — the console build's turbo `inputs` cover `scripts/vite-*.ts`, not `scripts/check-*.mjs` — so this commit's tree builds identically to the one it names, and the figure was re-measured on it. Part of #6683 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- scripts/check-eager-closure-budget.mjs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/scripts/check-eager-closure-budget.mjs b/scripts/check-eager-closure-budget.mjs index 899772bd99..f90aa70ca9 100644 --- a/scripts/check-eager-closure-budget.mjs +++ b/scripts/check-eager-closure-budget.mjs @@ -210,7 +210,7 @@ import { isEntrypoint } from './invoked-as.mjs'; /** * Ceiling for the console eager closure, in gzipped bytes. See the header for - * how this number was chosen; measured 3,254,004 on `SHA_PLACEHOLDER`. + * how this number was chosen; measured 3,254,004 on `bd2a7ec50`. * * Re-baselined DOWNWARD twice, each time toward a measurement the payload had * already fallen to: @@ -237,11 +237,20 @@ export const MAX_EAGER_CLOSURE_GZIP_BYTES = 3_300_000; * test instead of quietly becoming decorative. */ export const BASELINE = Object.freeze({ - /** `emitEagerClosureReport`'s `eagerGzipBytes` on this commit. */ + /** + * `emitEagerClosureReport`'s `eagerGzipBytes` on this commit. + * + * `bd2a7ec50` is the commit that carries the array and the gates; this + * constant was written one commit later, and the two trees differ ONLY by + * this recorded identifier. That is safe to state rather than hope: the + * console build's turbo `inputs` cover `scripts/vite-*.ts`, not + * `scripts/check-*.mjs`, so nothing in this file reaches the bundler. The + * figure was re-measured on the later commit and came back identical. + */ gzipBytes: 3_254_004, chunks: 48, totalChunks: 513, - commit: 'SHA_PLACEHOLDER', + commit: 'bd2a7ec50', }); /**