fix(app-shell): unbind the ambient data root on record surfaces, so data.* at record scope faults loudly (#8166) - #8963
Merged
Conversation
) `@objectstack/formula`'s `SCOPE_ROOTS` contains `data`, so at `scope: 'record'` the authoring lint accepts `data.status == 'x'`. objectui#5741 (Phase 2 of the objectui#5330 canon) retired `data.*` on runtime record surfaces — the row is `record.*` and nothing else — but `buildExpressionScope` kept binding an ambient `data`, so a predicate the linter had waved through also RESOLVED at runtime, against that bag rather than against the row. Measured on `origin/main`, one authored `visibleWhen: "data.status == 'x'"` meant three different things: `[runtime] No such key: status` against the `{}` every `ExpressionProvider` mount passes; a SILENT resolution off the record under edit on `AppContent`'s field-list evaluator in edit mode (`data: editingRecord`); and back to the first case in create mode. `buildExpressionScope` no longer accepts or binds `data`, and the two imperative call sites stop passing one. All three legs collapse onto the engine's own verdict, `[type] Unknown variable: data` — the same shape `app` has produced since objectui#8155, and the verdict the server gives the same string. The fault is loud, not fatal: the fallback directions are untouched (`visibleWhen` fails open, `readonlyWhen`/`requiredWhen` fail permissive), so nothing throws mid-render. `record.*` is unaffected, and metadata-admin's own `data` = draft binding is unaffected by construction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…ips (#8166) `ConditionalFormattingEditor.test.tsx` built its host bag by calling `buildExpressionScope({ user, data: {}, features })` and pinned that an ambient `data` RESOLVES while the row is unreachable through it — closing with a comment naming objectui#8166 as "the half of the retirement this card does not close". That is the branch this card deletes, so the fixture is re-judged rather than re-spelled: the `data:` argument is now a compile error (the same third fence objectui#8155 left on `app`), `CURATED_EXCLUSIONS` drops to empty — the host no longer binds a root this editor withholds — and the probe that read TRUE now reads FALSE against the producer's own bag. The list machinery is kept, empty, because its derivation is the fence that catches a NEW unadvertised host root. The editor's own docblock said an ambient `data` means `data.*` "still RESOLVES"; that is now false for this host and is corrected, with the producer half (`SCOPE_ROOTS` still accepts `data` at `scope: 'record'`) stated as the part that did not move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…ost scope (#8166) Three form/predicate tests hand-transcribe the bag `ExpressionProvider` mounts, because importing `@object-ui/app-shell` from `@object-ui/components` would invert the dependency. Each literal carried `data: {}`, which objectui#8166 removed from the producer. None of them would have gone RED: their predicates name `current_user`, `os` and a deliberately unbound root, so an extra root changes no verdict. That is the hazard rather than a reason to leave it — a transcription cannot disagree with its producer, so it absorbs the producer's drift in silence while claiming, in its own docblock, to be "the host scope `ExpressionProvider` mounts". `predicate-scope-parity-6010.test.tsx` also cited `ExpressionProvider.tsx:59,70`; that pair had already drifted, so the reference is re-keyed BY SYMBOL (`buildExpressionScope`) rather than re-counted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
… fix (#8166) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Sep 10, 2026
os-steve
marked this pull request as ready for review
September 10, 2026 12:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8166
Ruling: PM claim comment 5617500595 (2026-09-10) — fix the CONSUMER half: stop binding an ambient
dataon record surfaces, sodata.*at record scope fails loudly instead of silently resolving against the host's bag. The producer half (splitting@objectstack/formula'sSCOPE_ROOTSper scope) is out of this repo's surface and is not touched here.ZONE 2 — the hard precondition, measured BEFORE any edit
The ruling was made conditional on one measurement: "make it fault" must not mean "make it crash". Measured on
origin/mainat72bcd7783, withdataremoved from the scope bag in a scratch harness:evalFieldPredicate("data.status == 'x'", row, true, undefined, scopeWithoutData)[type] Unknown variable: data— oneconsole.warn,onFaultpassback, returns the fallback. No throw.throwOnErrorleg (action:*renderers,DeclaredActionsBar)Error, caught byuseCondition's owntry/catch→ hide + one warningappcontrol (objectui#8155, already unbound)[type] Unknown variable: app, andErrorunderthrowOnErrorThe precondition holds. Unbinding
dataproduces a loud, catchable diagnostic, not an uncaught runtime exception, and it produces exactly the diagnostic class theapproot has produced on this surface since #8155. It is pinned as an assertion, not a memory:recordScopeDataRoot-8166.render.test.tsxrendersRecordFormPagewith adata.*-gated field and asserts the render does not throw.The card's premise is CONFIRMED in mechanism and CORRECTED in detail
The mechanism holds exactly as filed: the authoring lint's accept set contains
data,objectui#5741retireddata.*on runtime record surfaces, andbuildExpressionScopebound an ambientdataanyway — so a predicate the linter waved through also resolved, against the wrong object.The card's headline — "silently constant-false today" — is not what
maindoes, and the correction matters because it moves where the silence is. Measured, one authoredvisibleWhen: "data.status == 'x'"meant three different things:datamainExpressionProvidermount;RecordFormPage's own evaluator{}[runtime] No such key: status— a fault, already warned, fail-open (field SHOWN, not hidden)AppContent's field-list evaluator, EDIT modeeditingRecord{}(editingRecordis null)So it was never constant-false, and it was not uniformly silent. The genuinely silent leg is row 2 — the one mount in the repo whose ambient
datawas not empty — and it is worse than the card's version rather than better: an author testing adata.*predicate on the record-form modal in edit mode would have seen it work, then shipped it dead everywhere else. Same defect, same fix, sharper reason.What changed
providers/ExpressionProvider.tsx—buildExpressionScopeno longer accepts or bindsdata. This is the whole fix; everything else follows from it.console/AppContent.tsx,views/RecordFormPage.tsx— the two imperativecreateExpressionEvaluatorcall sites stop passing adata:argument. Forced, not optional:ExpressionScopeInputno longer declares the member, so leaving them is a type error.AppContent's was the silent leg above.providers/ExpressionProvider.predicateScope.test.ts— thetoStrictEqualfence on the bag. Forced: the fence is what pins the bag this card changes, so no implementation of the ruling leaves it green.react/src/utils/visibilityDiagnostic.ts— docblock only. Itsapp-shelltier paragraph transcribed the bag being edited ({ current_user, user, ctx, os, data, features }) and asserted "datais bound at the app-shell tier"; both are now false. This is the same paragraph Therecord-scope CEL lint refuses theapproot, which app-shell's predicate scope DOES bind —@objectstack/formula'sSCOPE_ROOTShas noapp#8155 corrected forapp. No behaviour, no advice string: the printed advice never nameddata.providers/ExpressionProvider.tsx(binding only) +packages/app-shell/src/**/__tests__/+.changeset/. The four bullets above outside that set are each a mechanical consequence of the one-line binding removal — two type errors, one fence, one docblock the diff falsifies — not scope growth. Nothing else is touched.Acceptance
Two new files, both under
packages/app-shell/src/providers/__tests__/:recordScopeDataRoot-8166.test.ts(10 cases) — the engine-level half, over the exact callform.tsxmakes (evalFieldPredicate(pred, ruleRecord, fallback, previousRecord, usePredicateScope())). Asserts the loud outcome on both channels (built-inconsole.warnandonFault), in both fallback directions; carries theapppositive control in the same run and compares the reason's first line root-for-root; reproduces the pre-fix silent leg ({...scope, data: row}) and shows it resolving in both polarities with zero diagnostics; and pins the regression floor —record.status == 'x'shows,record.status == 'archived'hides, plus a numeric and an identity predicate.recordScopeDataRoot-8166.render.test.tsx(4 cases) — the page-level half. RendersRecordFormPagewith adata.*-gated field: asserts it does not throw, that the published predicate scope has nodata(and still noapp), that the reported line namesUnknown variable: dataalongside theappcontrol's, and that a resolvable identity gate still HIDES its field while ungated fields still show.The floor is the load-bearing half: this must not break working predicates, and a green suite proves nothing on a silent defect without it.
Discriminating power (ablation)
Mutation: restore the ambient
databinding inbuildExpressionScope(the parameter and the returned bag). Run over the two new files plus the two whose fixtures this change re-judges.Proved on disk before any result was read — anchor counts on the exact text, then the blob hash:
RED:
Test Files 4 failed (4) · Tests 10 failed | 41 passed (51), exit 1. The ten:Restore proved BY STATE, not by an exit code:
git checkout HEAD -- TARGET(never a bare checkout, which would take the mutation back out of the index), thengit hash-objectback to8f8400de…= HEAD,git diff HEAD -- TARGETempty, and both anchor counts back to 0.GREEN:
Test Files 4 passed (4) · Tests 51 passed (51), exit 0.apppositive control would stay GREEN under the mutation. It went RED. The reason is not that the control moved: thatitblock carries adatatwin (data.name == 'crm') alongside theappassertion, precisely so the two reasons can be compared root-for-root, and the twin is what fails. Theapphalf of the case passes under both arms. The real asymmetry the ablation demonstrates is the other one — all 41 remaining cases stayed GREEN under the mutation, including everyrecord.*regression-floor case, so the mutation is discriminated by thedataclaims alone and not by collateral breakage.Gates
scripts/pm/dispatch-gates.mjsdoes not exist in this repo. Gate families were derived by hand frompackage.jsonand.github/workflows/, and are listed with their exit codes below.All exit codes captured to disk before being read (
cmd > log 2>&1; EXIT=$?), never through a pipe.pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' buildpnpm --workspace-concurrency=2 --filter @object-ui/app-shell --filter @object-ui/react type-checkpnpm --workspace-concurrency=2 --filter @object-ui/components type-checkpnpm --workspace-concurrency=2 --filter @object-ui/app-shell --filter @object-ui/react lintpnpm --workspace-concurrency=2 --filter @object-ui/components lintpnpm exec vitest run packages/app-shell/674 files, 6523 passed, 1 skippedpnpm exec vitest run packages/react/ packages/components/src/renderers/form/__tests__/ packages/components/src/renderers/action/__tests__/149 files, 1549 passednode scripts/check-control-bytes.mjsnode scripts/check-changeset-presence.mjsnode scripts/check-governed-queue-guard.mjs --test PATHS (13 of them)node scripts/check-doc-example-types.mjs124 blocks, 35 compile, 89 declared)Type-check is not a hollow pass here: each package's
type-checkistsc --noEmit && tsc -p tsconfig.test.json, so the new test files are inside the checked program.check:doc-examplesfirst returned exit 2 —THE GATE COULD NOT RUN, packages unbuilt. That isPREREQUISITE NOT METand is not a pass. It was re-run afterturbo run build $(node scripts/check-doc-snippet-types.mjs --build-filter) --concurrency=2(exit 0) and the 0 above is from that second, real run. Separately: the ledger is keyedpath:line symbol, and none of its 89 rows keys any file in this diff (app-shellhas 0 rows; the 27reactrows are all in other files), so the line-shift hazard could not bite — checked by parsing the ledger, not by reading line numbers.packages/components/suite was not run, by measurement.@object-ui/componentscannot reach this diff: it declares no dependency on@object-ui/app-shell(checkeddependencies/peerDependencies/devDependencies), and no file underpackages/components/imports, requires, or reads from disk anything underapp-shell— the only occurrences are prose in comments and the CHANGELOG. The form/action test directories WERE run because this branch edits three files in them.packages/react/was run in full because this branch edits a file in it. CI runs the farm.@typescript-eslint/no-explicit-anywarnings, the same shape as the sibling harness it is modelled on (expressionUser.mountParity.test.tsx). The repo's warning baseline is large (2955 in app-shell) and the gate is0 errors.Notes for review
SCOPE_ROOTSstill containsdata, so the lint still acceptsdata.status == 'x'atscope: 'record'. This PR does not narrow the accept set — it makes the runtime stop pretending the accept set was right. De-advertising the root was ruled out by the card (it stops autocomplete recommending while the lint still accepts), and filtering the diagnostic incelAuthoring.tswas ruled out too (there is no diagnostic to filter). The changeset says this in the body.visibleWhenstill fails OPEN andreadonlyWhen/requiredWhenstill fail permissive. Whether that composition is right is objectui#8069's open question and is not decided here.datais the DRAFT under edit (ADR-0089 D3,CANONICAL_ROOT_BY_LAYER=runtime: record,metadata: data), built byviews/metadata-admin/predicate.ts, which copies onlyIDENTITY_ROOTSout of this bag and assigns its owndatalast.SchemaRenderernode gates were never affected.SchemaRenderer.tsxbindsdata: dataSourceafter spreading the ambient scope, so the app-shelldatawas already shadowed on every node predicate.relativeandshorton a datetime cell, as the date cell already did (objectui#8853) #8959 as the lit control at 3 changed files. objectui#8187 (same wave,packages/app-shell) is in a different region —views/metadata-admin/previews/DatasetPreview.tsx— and does not intersect.What grew after the first push
Three things surfaced only once the compiler and the suites ran, and each is a consequence of the binding removal rather than new scope. Every one is a separate commit.
views/metadata-admin/ConditionalFormattingEditor.test.tsx— fixture re-judged, not re-spelled. It built its host bag by callingbuildExpressionScope({ user, data: {}, features })and pinned that an ambientdataresolves while the row is unreachable through it, closing with a comment naming this very card as "the half of the retirement this card does not close". That is the branch this card deletes, so the fixture had to be re-judged: thedata:argument is now a compile error,CURATED_EXCLUSIONSdrops to empty (the host no longer binds a root the editor withholds), and the probe that read TRUE now reads FALSE against the producer's own bag. The now-empty list machinery is kept, because its derivation is the fence that catches a future unadvertised host root.ConditionalFormattingEditor.tsxdocblock said an ambientdatameansdata.*"still RESOLVES". False for this host now; corrected, with the producer half (SCOPE_ROOTSstill acceptsdataat record scope) stated as the part that did not move.packages/componentstests hand-transcribe the app-shell bag, each literal carryingdata: {}, each docblock claiming to be "the host scopeExpressionProvidermounts". None would have gone RED — their predicates namecurrent_user,osand a deliberately-unbound root, so an extra root changes no verdict. That silence is the hazard, not a reason to leave them: a transcription cannot disagree with its producer, so it absorbs drift invisibly.predicate-scope-parity-6010.test.tsxalso citedExpressionProvider.tsx:59,70; that pair had already drifted, so it is re-keyed by symbol rather than re-counted.Acceptance notes
ExpressionProviderstill takes adataprop and still publishesdataonExpressionContextValue, and nothing in the repo reads it — unlikeapp, whichDashboardView/PageView/ReportViewread as a plain value. It is kept here only to hold the published type stable (removing a member of an exported context type is a breaking change, and this repo shipsminor). A follow-up that retires the prop and the context member would be the honest end state.🤖 Generated with Claude Code
https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Generated by Claude Code