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

Commit e3cb476

Browse files
os-teslaclaude
andauthored
fix(app-shell): the field-rule wrong-layer verdict comes from @objectstack/lint, not a second copy of it (#9366)
Fixes #9440 `rowCanonAdvisory` answered *"is this root bound on this surface?"* from objectui's own knowledge. `@objectstack/lint` publishes the same judgement. Two hand-maintained copies of one verdict, agreeing today, with nothing keeping them agreeing. ## Second prose repair — contract review returned `FAIL` again Contract review [`5659211993`](#9366 (comment)) found **12 of the 16 discharged** and four not: the first repair replaced a false universal with a **closed six-root enumeration**, which was false by a wider margin than the sentence it replaced. ⛔ The fix is not a better list. It is the original universal **plus the one documented exception**, with nothing enumerated, so nothing goes stale when the root set moves. **Re-derived here, through the BUILT dist, over the whole candidate population.** `FIELD_RULE_JUDGED_ROOTS` and `FIELD_RULE_AMBIENT_ROOTS` are module-private at 17.4.0 (neither is in `@objectstack/lint`'s export list), so the judged set is reconstructed as `SCOPE_ROOTS` + the ambient root, read from `@objectstack/formula` — a **declared** dependency of this package, not a phantom one. `SCOPE_ROOTS` has 27 members; `FIELD_RULE_BOUND_ROOTS` is `record` / `previous` / `parent`, all three of them in `SCOPE_ROOTS`; so the candidate set `JUDGED \ BOUND` is **25**. Sweeping all 25 through `lintCelPredicate` at `scope: 'record'` with `slot: 'visibleWhen'`, classifying each by verbatim equality with `fieldRuleRootIssue`'s own message: **24 advised · 1 blocked · 0 silent**, and the blocked set is exactly `["app"]`. Controls on the same instrument: `data.status == 1` ⇒ one `warning` (lit); `record.status == 1` / `previous.status == 1` / `parent.status == 1` ⇒ `[]` (dark, and the helper judges none of them); `app.status == 1` ⇒ one **error**, no warning; `zzz.x == 1` ⇒ one error and `fieldRuleRootIssue` returns `null`, so an unjudged name was never a candidate. ⭐ **The universal is now defended by an instrument rather than by prose.** A new sweep in `celAuthoring.fieldRuleVerdict-9318.test.ts` re-derives the candidate population on every run and asserts `silent = []`, `blocked = ['app']`, `advised = candidates − app`; a second test pins *why* `app` is the exception (`SCOPE_ROOTS` does not contain it, the helper judges it anyway). ⚠️ The reconstruction is a declared limit, written into the test: a NEW ambient root added upstream would be invisible to the sweep until that literal moves, and nothing in this repo can see it. **Ablation** — removing the error gate (`if (issues.every((i) => i.severity !== 'error') && hint.scope === 'record')` ⇒ `if (hint.scope === 'record')`, proved on disk: anchor 1→0, mutant 0→1, blob `0120d42aba`→`91c736bc96`) reddens **exactly those two tests** (`Tests 2 failed | 10 passed (12)`) with `expected [] to deeply equal [ 'app' ]` — the exception itself disappearing, which is the predicted direction. Restored by hash equality to the `HEAD` blob `0120d42aba` with `git diff HEAD` empty, under `trap … EXIT INT TERM`. **Carriers, before → after, censused on the STORED artifacts** (the built `dist/views/metadata-admin/celAuthoring.js`, not the source; `perl -0777` occurrence counts): the six-root list **1→0**, `For the five further` **1→0**, `five by hand` **1→0**, `` `current_user` text ends `` **1→0**; the replacements **0→1** each. In the changeset body: the six-root list **1→0** and `the slots the published vocabulary covers` **1→0**, replacements **0→1**. Lit controls unmoved — `ROW_PREDICATE_ROOTS` **3→3** in the dist and **2→2** in the changeset, `METADATA_LAYER_ROOT` **1→1** in the dist; dark controls `strictly BROADER` and `zzzNotPresentZZZ` **0→0** in both. ⚠️ Escaping was checked rather than assumed before those counts were trusted: the emitted comments carry `` `record` `` **unescaped** (10 occurrences, backslash-escaped form 0), so an unescaped needle is the right instrument for this artifact. ⛔ Three statements the review confirmed TRUE are deliberately untouched: `ROW_PREDICATE_ROOTS` is a six-entry local constant and "five roots the field tier does not bind" is correct for it — that is a different claim from the advised set, and it is not repaired into a universal. ## Prose repair — contract review returned `FAIL` (first round) Contract review [`5658789972`](#9366 (comment)) swept 60 claims across this PR's shipped comments, its changeset body and this description, and found **16 false**. ⭐ The bounding is load-bearing: `@object-ui/app-shell` builds with `tsc` (`"build": "tsc && node ../../scripts/check-dist-completeness.mjs"`), and **nothing in its config chain sets `removeComments`** — `packages/app-shell/tsconfig.json` extends `../../tsconfig.json`, which has no `extends` key and no `removeComments` — so tsc's own default (`false`) stands and comments reach the emit. ⚠️ An earlier draft of this line cited `tsconfig.base.json:22` for that. Measured: `tsconfig.base.json` is extended only by `tsconfig.node.json`, `tsconfig.scripts.json`, `tsconfig.react.json` and `examples/byo-backend-console/tsconfig.json` — **never by app-shell**. Right conclusion, wrong instrument; the instrument is corrected here and the conclusion is additionally confirmed by reading the emitted bytes. `files` ships `dist/**` and `CHANGELOG.md`, so the comments under repair are **published artifacts**, not notes. ⛔ **The mechanism is unchanged.** The repair commit moves comments, the changeset body and the test narration only; the red-first block below still reproduces and the pins still pass. Three of the sixteen were load-bearing rather than decorative: - **the stated reason for taking the engine's message** (see *What changed*) was false on both roots it named. Because it was the justification for a behavioural choice, the question underneath it was re-asked rather than the sentence reworded — the swap stands, for measured reasons, and the old one is withdrawn in place; - **the two arguments for keeping the local fallback on uncovered surfaces** (see *Part 3*) — the answer survives, both arguments were false, and the real asymmetry is written in their place; - **the CI section** named two green checks and missed the one real red. Re-read after writing, from the **built** `dist` (⛔ not the source): `packages/app-shell/dist/views/metadata-admin/celAuthoring.js`, `perl -0777` occurrence census, before → after — `strictly WIDER` 1→0, `strictly NARROWER` 1→0, `stop advising` 1→0, `EVERY root the field level leaves unbound` 1→0; the four replacement anchors 0→1 each. Lit control `ROW_PREDICATE_ROOTS` **3 before and 3 after** (same file, same census, varying only the claim); dark control `strictly BROADER` **0 both**, so the zeros are absence and not a broken read. ## Re-measurement — both sides, at implementation time The card instructs that the re-measurement wins over the card. Both sets of line numbers moved; **the exported shape did not**, so the substance survives intact. | reading | card (`9c44eed4` / `d7f0601`) | measured now | |---|---|---| | objectui refs to `fieldRuleRootIssue` / `FIELD_RULE_BOUND_ROOTS` under `packages/` | 0 | **0 at `2e471dc0a`** (`git grep` exit 1; control `rowCanonAdvisory` exit 0, 4 hits) — ⚠️ no longer 0 at this PR's head, see the note under this table | | `rowCanonAdvisory` | `celAuthoring.ts:184-283` | `celAuthoring.ts:271` on objectui `origin/main` `2e471dc0a` | | `FIELD_RULE_BOUND_ROOTS` export | `validate-expressions.ts:688` | **`:688`** on objectstack `origin/main` `2b6a207` | | `fieldRuleRootIssue` export | `validate-expressions.ts:790` | **`:790`** on objectstack `origin/main` `2b6a207` | | what objectui actually consumes | — | `@objectstack/lint@17.4.0`, installed; both symbols on the package root entry | ⚠️ **That zero is scoped to `2e471dc0a`, and is false if read as a statement about this PR's head.** Re-measured on the head tree, `git grep -c FIELD_RULE_BOUND_ROOTS -- packages/` exits **0** and prints four files. Three are this PR's own. The fourth is **not** — `packages/react/src/SchemaRenderer.tsx:844`, a comment that was already on `main` at this PR's merge-base (`dfb585059`, where that grep exits 0 with exactly that one file) and that arrived here through this PR's merge of `main`. Drift in the reading, not a fabrication, and nothing about the seam it was measuring has moved: the four files hold no *call* of either symbol outside `celAuthoring.ts`. One correction to the dispatch note, offered as a reading and not a complaint: the `:680` / `:782` figures are the sibling checkout's local `HEAD` (`86c5052`, an ancestor of `origin/main`), not `origin/main` itself. Against `origin/main` the card's own `:688` / `:790` still hold. Either way the line numbers are informational — what this PR consumes is the published `dist` of `@objectstack/lint@17.4.0`, where the declared shape reads `FIELD_RULE_BOUND_ROOTS: readonly ["record", "previous", "parent"]` and `fieldRuleRootIssue(slot: string, source: string)`. ## Part 3 — the helper has NO slot vocabulary, so coverage is objectui's call, and the fallback stays This is the part the card most wants answered, so it is answered first and explicitly. `fieldRuleRootIssue` judges against `FIELD_RULE_BOUND_ROOTS` — the FIELD-RULE tier, `record` / `previous` / `parent`. The surfaces `rowCanonAdvisory` guards are everything that reaches `lintCelPredicate` at `scope: 'record'`. Measured, they do not share one bound set: | surface | slot | roots objectui binds | helper's verdict applies? | |---|---|---|---| | field inspector `visibleWhen` / `readonlyWhen` / `requiredWhen` | the three `*When` keys | `FIELD_RULE_ROOTS` = `record`, `previous`, `parent` | **yes** — identical set | | draft gate `validateObjectFieldRules` | the same three keys | same | **yes** | | field inspector `formula` expression | `expression`, role `value` | `FORMULA_ROOTS` = `record` | **no** — strictly NARROWER | | conditional-formatting `condition` | `condition` | `ROW_PREDICATE_ROOTS` = `record`, `current_user`, `user`, `features`, `os`, `ctx` | **no** — five roots the field tier does not bind, but **not a superset**: no `previous`, no `parent` | ⚠️ Neither set is *comparable* to the field-rule set — `FORMULA_ROOTS` is a proper subset of it, and `ROW_PREDICATE_ROOTS` is not a superset, since it lacks `previous` and `parent`. The two overlap on `record` alone. And `fieldRuleRootIssue` has **no slot vocabulary of its own**: hand it any string and it judges against `FIELD_RULE_BOUND_ROOTS` and interpolates the name into its message (measured on 17.4.0 — `('expression', 'current_user.x')` and `('condition', 'current_user.x')` each return a finding, not `null`). The helper never declines a surface, so `FIELD_RULE_VERDICT_SLOTS` is objectui's own load-bearing answer, not a formality. What routing the two uncovered surfaces through it would actually cost — measured, and wrong in **opposite** directions: - a **conditional-formatting condition** would be advised on `current_user`, `user`, `features`, `os` and `ctx` — five roots it binds. A false **red**: the author is told to rewrite a predicate that works there; - a **formula** `expression` would get `previous.*` / `parent.*` back **clean**, because the helper reports them bound at the field tier, where a formula binds only `record`. A false **green** — and the message it does print for other roots names `previous` and `parent` as available here, which on this surface is wrong prose as well as a wrong verdict. ⛔ **Neither is a coverage shrink**, and the earlier draft of this section said it was. Measured on the head tree, both surfaces report nothing at all for `previous.*` / `parent.*` today (`lintCelPredicate("previous.status == 'x'", { scope: 'record', role: 'value' })` ⇒ `[]`, `parent` likewise), and `@object-ui/core`'s `METADATA_LAYER_ROOT` is `'data'` — the only root objectui has ever advised on any surface. So nothing that is advised now would stop being advised. The hazard is the wrong-direction verdict above, not a lost one. So those two keep the local instrument, byte-for-byte as it behaves today, and each is pinned as a live control that reddens if a later tidy-up routes them through the helper anyway. Ablation leg B1 below performs that exact tidy-up and shows both controls going red. `PermissionAdvancedFacets` (RLS `USING` / `CHECK`) and `ConditionBuilder` default to `scope: 'flattened'` and were never guarded by this advisory at all. ## What changed `CelSchemaHint.slot` (and the matching `CelPredicateField` prop) names the authored key. On a covered slot the verdict **and** the message are the helper's; everywhere else the local path runs unchanged. Exactly one message ships per finding — never both. **Why the engine's message and not objectui's.** ⚠️ An earlier draft of this section said *"the engine's message refuses that rewrite by name"*, of `current_user` **and** `app`. That is false on both, and it was this PR's stated reason for a behavioural choice, so it is withdrawn rather than reworded. Measured against the installed `@objectstack/lint@17.4.0`: - for `current_user` the engine's message **prescribes** the rewrite it was said to refuse — it **contains** *"To gate on record state, rewrite the predicate against `record`."*, one of three remedies it offers (the others being an option-level `visibleWhen` and field-level security). ⚠️ It does **not end** there, as an earlier draft of this line claimed: measured against 17.4.0, `includes(…)` is **true** and `endsWith(…)` is **false** — every covered slot's text closes on *"it is not a fourth answer."*; - for `app` the engine's message **does** refuse by name (*"⛔ Do NOT write `record.app`"*) — and it is **unreachable from this diff**. `lintCelPredicate('app.theme == "dark"', { slot: 'visibleWhen', scope: 'record' })` returns exactly one issue, a pre-existing **error** (*"bare reference `app` … Write `record.app`."*), and the advisory is gated behind `issues.every((i) => i.severity !== 'error')`, so the helper is never called. **The swap still stands, on reasons that survive measurement.** The verdict widens, and objectui has no message for most of what it now judges. Its local instrument produces a sentence for exactly **one** root — `@object-ui/core`'s `METADATA_LAYER_ROOT`, `data`. For every **other** root a covered slot now reports there is no objectui sentence to keep; keeping "objectui's message" there would mean hand-writing one per root, which is precisely the second hand-maintained copy this card exists to delete — and they would have to be per-root, since the engine's texts for `data`, the user-root family, the platform-wide family and the ambient family are four different remedies, not one sentence with the root substituted. ⛔ That population is deliberately **not** written down here — an earlier draft of this paragraph said "five further roots", which was false — because it moves whenever the platform moves `FIELD_RULE_JUDGED_ROOTS` or `FIELD_RULE_BOUND_ROOTS`. The sweep named above re-derives it on every run instead. The one message genuinely **swapped** is `data`'s, and objectui's tail there — *"Re-root the reference on `record`"* — is the half that does not generalise: right for `data`, wrong for `current_user`, which is not a field of the record. Reading a verdict from one authority and explaining it from another drifts the same way two verdicts do. ⛔ This changes nothing the diff **does**; it replaces the reason given for what it already did. **Declared behaviour change.** Those three editors now advise on **every root the field level leaves unbound — except `app`**. ⛔ No enumeration, and that is the point: any list goes stale the next time the platform moves a root, and two earlier drafts of this line were false in turn — first "every unbound root" with no exception at all, then a closed six-root list that named a fraction of the population and omitted the rest. The exception is a **mechanism**, not a special case: the advisory runs only once the predicate is error-free (`issues.every((i) => i.severity !== 'error')`), and `app` is the single judged root the platform does not **declare** (`FIELD_RULE_JUDGED_ROOTS` is `SCOPE_ROOTS` plus the ambient `app`), so for `app` alone the pre-existing bare-reference error fires first and the advisory never runs — even though the helper judges it and carries a bespoke message for it. A name the helper does not judge at all is stopped by that same error and was never a candidate. Severity stays objectui's own `warning`: every save gate on this tier counts `severity === 'error'`, so no accept set moves and nothing already stored in customer metadata is refused. The `data.status == 'x'` case still reports at `warning`, never `error`. Progressive enhancement is preserved throughout — the `import()` stays dynamic (a static one would pull the lint bundle onto the eager console graph, objectui#5266), the export is feature-detected, and a lint package without it falls through to the local instrument rather than going quiet. ## Not reopened The three fences the card sets are untouched: objectui already warns on wrong-layer `data.*` and still does; the advisory stays `warning` and is never promoted; the bare-shorthand arm stays disabled (`row = null`) on the fallback path, and the helper path has no such arm at all. ## Red-first, verbatim The pin was written and run on the unmodified tree first: ``` ❯ |unit| packages/app-shell/src/views/metadata-admin/celAuthoring.fieldRuleVerdict-9318.test.ts (10 tests | 3 failed) × TRUE POSITIVE — a covered slot takes BOTH the verdict and the message from `fieldRuleRootIssue` × ships ONE message, never both — objectui's sentence is gone from the covered path × takes the WIDER verdict on a covered slot: a root no field rule binds is advised too AssertionError: expected [] to have a length of 1 but got +0 AssertionError: expected [ { severity: 'warning', …(1) } ] to deeply equal [] AssertionError: expected [] to have a length of 1 but got +0 Test Files 1 failed (1) Tests 3 failed | 7 passed (10) ``` ## Ablation — three legs, all falsifiable, no null results Every leg mutates on disk, proves the mutation reached disk **before** any result is read, and restores by hash equality, never by an exit code. All ran under `trap ... EXIT INT TERM`. **Leg A — does the verdict really come from the published constant?** Mutated the installed `@objectstack/lint@17.4.0` `dist/index.js` to drop `parent` from `FIELD_RULE_BOUND_ROOTS`. ``` PRISTINE_SHA=fef9fda4382de0c9e60582fca1b5b594eeebea16e27eee622ebdcc7f71d7c069 BEFORE orig_count=1 mut_count=0 AFTER orig_count=0 mut_count=1 MUT_SHA=c488ed51ac8957cccf926137b892947326015059bb83647629322541acda91f7 MUTATED_VITEST_EXIT=1 RESTORED_SHA=fef9fda4382de0c9e60582fca1b5b594eeebea16e27eee622ebdcc7f71d7c069 RESTORE_PROVEN orig_count=1 mut_count=0 RESTORED_VITEST_EXIT=0 ``` The reading: `parent.status == "paid"` at `visibleWhen` is immediately advised, carrying the engine's own message (*"`visibleWhen` reads `parent`, but a field-level conditional rule binds only `record` …"*). That is the card's required demonstration — a root the platform DOES bind comes back clean **through the new path**, and stops being clean the moment the platform stops binding it. The file is not git-tracked, so the restore proof is sha256 equality against the pristine snapshot plus the anchor counts, and `git status` on the worktree is empty. **Leg B1 — can the part-3 live controls fail?** Replaced the slot condition with an unconditional one, i.e. performed the exact tidy-up the card warns against. ``` HEAD_BLOB=220980448c9ab1f63598067d7d1c70e1db3b87d1 B1 BEFORE anchor=1 mutant=0 B1 AFTER anchor=0 mutant=1 blob=e95e8a8882e749743f697cc915198fbacd5ffc49 B1 MUTATED_VITEST_EXIT=1 × LIVE CONTROL — a formula `expression` still gets objectui's message, not the engine's × LIVE CONTROL — a conditional-formatting condition is NOT advised for the roots it binds × an unknown slot name falls back rather than guessing B1 RESTORED_BLOB=220980448c9ab1f63598067d7d1c70e1db3b87d1 B1 RESTORE_PROVEN: git diff HEAD empty ``` **Leg B2 — is the helper path actually reached?** Emptied the covered-slot list. ``` B2 BEFORE anchor=1 mutant=0 B2 AFTER anchor=0 mutant=1 blob=59fc8a89995ec849e65a81a2849195495b2e0afd B2 MUTATED_VITEST_EXIT=1 × TRUE POSITIVE — a covered slot takes BOTH the verdict and the message × ships ONE message, never both × takes the WIDER verdict on a covered slot B2 RESTORED_BLOB=220980448c9ab1f63598067d7d1c70e1db3b87d1 B2 RESTORE_PROVEN: git diff HEAD empty ``` No leg was a null result. All three could fail and all three did. ## Verification Acceptance grep: `git grep -c fieldRuleRootIssue packages/app-shell/src` exits 0 with **two** lines — `…/celAuthoring.fieldRuleVerdict-9318.test.ts:4` and `…/celAuthoring.ts:9`. Both are non-zero; an earlier draft transcribed only the second, and read `:8` — correct then, stale now, because this repair added one more mention of the symbol to that file's docblock. objectui#8972's five pins are green **and byte-unchanged**: the pin file's blob hash equals its `origin/main` blob (`dbfc2108221cf22955927ac565804abdc3b4a1ed` both sides), and `celAuthoring.test.ts` reports `Tests 39 passed (39)`. Heavy runs went through the shared verify lock on slot `os-dev-objectui-9318`: ``` os-verify-lock: VERDICT batch-last-exit 0 · held 247s # pnpm build (BUILD_EXIT=0) Tasks: 43 successful, 43 total os-verify-lock: VERDICT command-exit 0 · held 1134s · waited 345s type-check=0 Tasks: 81 successful, 81 total app-shell-vitest=0 Test Files 693 passed (693) · Tests 6753 passed | 1 skipped ``` The one skip is pre-existing; no test was skipped, quarantined or loosened by this change. ⚠️ **Those heavy figures are the implementation run, at the tree before the prose repair** — they are not re-derived at the current head and should be read as such. Re-run after the repair commit, through the same shared lock (slot `os-dev-objectui-9440`), all with the lock's own `VERDICT` line as the verdict rather than a bare `$?`: ``` turbo run build --filter=@object-ui/app-shell --concurrency=2 VERDICT command-exit 0 Tasks: 29 successful, 29 total pnpm --filter @object-ui/app-shell run type-check # tsc --noEmit && tsc -p tsconfig.test.json VERDICT command-exit 0 pnpm exec vitest run …/celAuthoring.fieldRuleVerdict-9318.test.ts …/celAuthoring.test.ts --reporter=verbose VERDICT command-exit 0 Test Files 2 passed (2) · Tests 49 passed (49) `apps/console` occurs 0 times in the verbose log, which names both files itself ``` Gates re-run on the repaired tree, each exit code captured before any pipe: `check:control-bytes` 0, `check:changeset-claims` 0, `check:new-line-citations` 0, `check:test-path-roots` 0, `check:shell-escape-residue` 0, `check:comment-mask-corpus` 0, `check-changeset-presence.mjs` 0 (*"5 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)"*), `check-changeset-no-major.mjs` 0. **Red-first still reproduces on the repaired tree.** The four product files reverted to the merge-base, the mutation proved on disk first (`FIELD_RULE_VERDICT_SLOTS` occurrences 5 → 0 in `celAuthoring.ts`) and restored under `trap … EXIT INT TERM` by blob-hash equality against `HEAD`, never by an exit code: ``` MUTATION_ON_DISK anchor 5 -> 0 × TRUE POSITIVE — a covered slot takes BOTH the verdict and the message from `fieldRuleRootIssue` × ships ONE message, never both — objectui's sentence is gone from the covered path × takes the WIDER verdict on a covered slot: a root no field rule binds is advised too Test Files 1 failed (1) Tests 3 failed | 7 passed (10) RESTORE 86a5199… celAuthoring.ts · 0e6247b… CelPredicateField.tsx 39e9975… clientValidation.ts · 448e1e8… ObjectFieldInspector.tsx (all OK) git diff HEAD: EMPTY ``` Gate family, derived by hand from `package.json` plus `.github/workflows/` (this repo has no dispatch-gates deriver), all exit 0: `check:control-bytes`, `check:designer-field-key-parity`, `check:i18n-keys`, `check:phantom-deps`, `check:unreferenced-sources`, `check:test-path-roots`, `check:new-line-citations`, `check:changeset-claims`, `check:eager-closure`, `check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:self-import`, `check-changeset-presence.mjs`, `check-governed-queue-guard.mjs --test` (NOT GOVERNED, 6 paths against 5 surfaces). Lint was run over the whole tree rather than narrowed: `pnpm exec eslint . --format json` over the population eslint's own config selects — **4920 files, 0 errors**, 12879 pre-existing warnings. The five touched files carry 0 errors; their 12 warnings are pre-existing `no-explicit-any` and `set-state-in-effect` sites this change neither adds to nor moves. **Dependent-set membership read, done here rather than inherited.** Three different lists in this repo share the word "exclude" and are NOT the same set: - `.changeset/config.json` `ignore` = `@object-ui/example-*`, `@object-ui/site`, `@object-ui/test-support` — excluded from **version bumping** only; - the **type-check dependent set** of `@object-ui/app-shell` = `@object-ui/console`, `@object-ui/example-byo-backend-console`, `@object-ui/example-console-starter` — measured by reading every workspace manifest for a dependency edge on `@object-ui/app-shell`; - the root build's `--filter=!@object-ui/site`. `@object-ui/site` sits in the first and the third and has **no** dependency edge on `app-shell`, so it is not in the second at all; the genuine overlap between the first two is `@object-ui/example-*`. `pnpm type-check` was run in full, so all three dependents are covered regardless. `@object-ui/app-shell` is in the single `fixed` group of 40, so a changeset is required and one is included (`minor`). ⚠️ `major` is not withheld "by group size", as an earlier draft said: `scripts/check-changeset-no-major.mjs` refuses `major` for **every** package in this repo irrespective of group, because objectui's major is pinned to `@objectstack`'s — and it **is** available, under `OBJECTUI_ALLOW_MAJOR=1`, for the one synchronized release that follows objectstack across ITS major. ## Clause-② — still `no`, verified rather than assumed No exported signature of `@object-ui/app-shell` moves. `CelSchemaHint`, `CelPredicateField`, `celAuthoring`, `rowCanonAdvisory` and `FIELD_RULE_VERDICT_SLOTS` each appear **0** times in the built `packages/app-shell/dist/index.d.ts`, against a lit control (`MetadataResourceRouter`, 1 hit) proving the grep and the file, plus a dark control (`MetadataResourceRouterZZZ`, 0) proving the same grep can return zero honestly. The package's `exports` map has **two** entries — `"."` and `"./styles.css"` — not one, as an earlier draft said; the substance is unchanged, since neither admits an importable subpath for these symbols. Nothing for me to hang, and I have hung nothing. ## CI — named instrument, not a frozen tally ⛔ An earlier draft of this section said `main` carried two red checks, **Doc Snippet Type Check** and **Skill Example Check**, inherited by every PR. Both halves are false. Re-derived from `GET /repos/objectstack-ai/objectui/commits/{sha}/check-runs` (⛔ not `actions/runs?head_sha=`): | sha | what it is | runs | non-success | |---|---|---|---| | `90b0bf7162` | the head that draft described | 36 | **1** — `Bundle Analysis` | | `dfb585059` | this PR's merge-base (from `git merge-base`, not `base.sha`) | 51 | none | | `7ca6ddd4b5` | `main` tip, read 2026-09-14T03:40Z | 59 | none completed | `Doc Snippet Type Check` and `Skill Example Check` read `success` at **all three**. They were never red here, and the paragraph naming them is deleted rather than corrected. **The one real red is `Bundle Analysis`** — job `103733469427` at `90b0bf7162`, the `ui-components` per-chunk headroom: `387.8 KB measured / 389.6 KB ceiling (headroom 1.9 KB = 0.02x the 89.0 KB regression, under the 0.10x floor …)`. The gate states its own reading: > This row's headroom is a standing debt that predates this change, and it moves under traffic > that has nothing to do with the chunk … So this verdict is NOT an accusation that your diff > spent the bytes … ⛔ There is therefore nothing here for this pull request to "fix", and the > two edits that would turn this green are both forbidden: ⛔ never raise the ceiling, and ⛔ > never raise the allowance. Paying the row down is the open decision on the chunk … take it > there, and **say on this pull request that you did**. **Saying it here, as the gate asks:** the `ui-components` row is being paid down on objectui#9251 / PR objectui#9399 (both reachable). It is not this PR's to fix, this diff does not touch that chunk, and ⛔ neither the ceiling nor the allowance is touched here. In the same run the entry chunk (`144.3 KB / 350 KB`), the aggregate closure (`3106.2 / 3134.8 KB`) and ceiling freshness all pass. ⛔ The table above is a reading **at named shas**, not a live tally — this PR has been pushed to since. Re-derive at the current head rather than trusting these counts. ## Acceptance notes Noted while reading, deliberately **not** filed and **not** repaired here: - `clientValidation.validateObjectFieldRules` keeps only `severity === 'error'`, so the advisory it now asks for is discarded the moment it is produced. That is correct for a draft gate and is stated in its own docblock; the slot is wired there anyway so the two `scope: 'record'` callers ask the same authority the same question. An observation, not a defect — nothing is wrong today and no reader is misled. - ⭐ **Upstream, surfaced by this repair and deliberately not acted on here.** For `app` at a covered slot the author is told *"bare reference `app` … **Write `record.app`**"* by `@objectstack/formula`'s bare-reference check, while `@objectstack/lint`'s own message for the same root says *"⛔ **Do NOT write `record.app`**: `app` is not a field on this object, so that spelling only trades this diagnostic for an `unknown field` error on `app`."* The two published diagnostics contradict each other, and the error wins because it fires first. That is upstream of this repo and pre-existing — this diff neither creates nor worsens it, and ⛔ widening objectui's local path to paper over it is exactly the consumer-side workaround this card exists to stop. Recorded here for the seat; no card filed from this repair order. - `FIELD_RULE_SLOT_CONSEQUENCE`'s slot vocabulary is module-private upstream, so a consumer cannot ask `@objectstack/lint` which slots it covers and must state its own answer, as this PR does in `FIELD_RULE_VERDICT_SLOTS`. A drift tripwire pins the bound set so the next platform move arrives as a red test here rather than as silence. Worth an upstream export one day; nothing is broken, so no card. Carrier for that observation: none — no queued PR or seat is touching that file. Session: `https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ` --- _Generated by [Claude Code](https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ)_ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 14a1b92 commit e3cb476

6 files changed

Lines changed: 470 additions & 15 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
'@object-ui/app-shell': minor
3+
---
4+
5+
The wrong-layer root advisory asks the platform for its verdict instead of keeping
6+
a second copy of it (objectui#9318).
7+
8+
`rowCanonAdvisory` answered "is this root bound on this surface?" from objectui's
9+
own knowledge — `@object-ui/core`'s detector hard-codes the single root `data`, and
10+
the docblock justified that from `ROW_PREDICATE_ROOTS` / `FIELD_RULE_ROOTS` /
11+
`FORMULA_ROOTS`. `@objectstack/lint` publishes the same judgement as
12+
`fieldRuleRootIssue` / `FIELD_RULE_BOUND_ROOTS`, pinned upstream. Two hand-maintained
13+
copies of one judgement, agreeing today, with nothing keeping them agreeing: the next
14+
root the platform binds or unbinds moves one and not the other, silently.
15+
16+
A `CelSchemaHint.slot` (and the matching `CelPredicateField` prop) names the authored
17+
key, and on the three slots whose bound set IS the field-rule set — `visibleWhen`,
18+
`readonlyWhen`, `requiredWhen` — the verdict and the message now come from the helper.
19+
That set is objectui's own answer, not the helper's: `fieldRuleRootIssue` has no slot
20+
vocabulary at all — hand it any slot name and it still judges, gating on the ROOT and
21+
interpolating whatever name it was given. Which surfaces it may answer for is therefore
22+
this repo's call. Both are internal to `@object-ui/app-shell`; no package export moves.
23+
24+
**Behaviour change, deliberate and warning-only.** Those three editors now advise on
25+
every root the field level leaves unbound — except `app` — where before only `data`
26+
was reported, and each carries the engine's own per-root diagnostic rather than
27+
objectui's single sentence. `app` is the one exception, and for a structural reason
28+
rather than as a special case: the advisory runs only on an error-free predicate, and
29+
`app` is the single judged root the platform does not declare, so the pre-existing
30+
bare-reference error fires first and the advisory never runs. Severity stays
31+
objectui's own `warning` — every save gate on this tier counts `severity === 'error'`,
32+
so no accept set moves and no predicate already stored in customer metadata is refused.
33+
34+
**Coverage is not shrunk to fit the helper.** Two guarded surfaces bind a set that is
35+
not the field-rule set, and neither is comparable to it — they overlap on `record`
36+
alone. A `formula` field's `expression` binds `FORMULA_ROOTS` (`record`), a proper
37+
subset. A conditional-formatting `condition` binds `ROW_PREDICATE_ROOTS` (`record`,
38+
`current_user`, `user`, `features`, `os`, `ctx`) — five roots the field tier does not
39+
bind, but not a superset of it either, since it lacks `previous` and `parent`. Routed
40+
through the helper, the condition would be told to rewrite five roots that work there,
41+
and the formula would be told `previous` and `parent` are bound when it binds neither.
42+
Those keep the local instrument unchanged, and both are pinned as live controls against
43+
a later tidy-up that routes them through the helper anyway.

‎packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ export interface CelPredicateFieldProps {
8787
scope?: 'record' | 'flattened';
8888
/** Override the scope roots offered by autocomplete (see CelSchemaHint.roots). */
8989
roots?: string[];
90+
/**
91+
* The authored key this editor writes (`visibleWhen`, `readonlyWhen`,
92+
* `requiredWhen`) — see `CelSchemaHint.slot`. Naming it lets the wrong-layer
93+
* advisory take the platform's published per-slot verdict (objectui#9318);
94+
* leaving it unset keeps the local one, which is the right answer for the
95+
* surfaces whose bound roots are not the field-rule set.
96+
*/
97+
slot?: string;
9098
/**
9199
* Engine field role (see CelSchemaHint.role). `'predicate'` (default) for
92100
* boolean conditions; `'value'` for formula expressions — which also turns
@@ -118,6 +126,7 @@ export function CelPredicateField({
118126
clause,
119127
scope,
120128
roots,
129+
slot,
121130
role,
122131
onLintChange,
123132
onInferredTypeChange,
@@ -168,7 +177,7 @@ export function CelPredicateField({
168177
React.useEffect(() => {
169178
let cancelled = false;
170179
const handle = setTimeout(() => {
171-
const hint = { objectName, fields: fieldNames, clause, scope, role };
180+
const hint = { objectName, fields: fieldNames, clause, scope, slot, role };
172181
lintCelPredicate(value, hint).then((res) => {
173182
if (cancelled) return;
174183
setIssues(res);
@@ -188,7 +197,7 @@ export function CelPredicateField({
188197
clearTimeout(handle);
189198
};
190199
// eslint-disable-next-line react-hooks/exhaustive-deps
191-
}, [value, objectName, fieldsKey, clause, scope, role]);
200+
}, [value, objectName, fieldsKey, clause, scope, slot, role]);
192201

193202
/* Report "clean" upward when the field empties (no debounce needed). */
194203
React.useEffect(() => {
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectui#9318 — the wrong-layer verdict comes from `@objectstack/lint`, not
5+
* from a second hand-maintained copy of it.
6+
*
7+
* ## What moved
8+
*
9+
* `rowCanonAdvisory` used to answer "is this root bound on this surface?" from
10+
* objectui's own knowledge: `@object-ui/core`'s `detectNonCanonicalRowSpelling`
11+
* hard-codes ONE root (`data`) and objectui's docblock justified that from
12+
* `ROW_PREDICATE_ROOTS` / `FIELD_RULE_ROOTS` / `FORMULA_ROOTS`. The platform
13+
* publishes the same judgement as `fieldRuleRootIssue` / `FIELD_RULE_BOUND_ROOTS`.
14+
* Two copies of one judgement agree today and nothing keeps them agreeing.
15+
*
16+
* ## What did NOT move, and is pinned here as such
17+
*
18+
* The helper judges against `FIELD_RULE_BOUND_ROOTS` = `record` / `previous` /
19+
* `parent` — whatever slot name it is handed. It has no vocabulary gate of its
20+
* own (measured: an unrecognised slot still returns a finding, not `null`), so
21+
* WHICH surfaces it may answer for is objectui's call, not the platform's. Two
22+
* of the surfaces `rowCanonAdvisory` guards bind a set that is not that one, and
23+
* neither is comparable to it — they overlap on `record` alone:
24+
*
25+
* - the conditional-formatting `condition` binds `ROW_PREDICATE_ROOTS`
26+
* (`record`, `current_user`, `user`, `features`, `os`, `ctx`) — five roots
27+
* the field tier does not bind, but NOT a superset: no `previous`, no
28+
* `parent`;
29+
* - a `formula` field's `expression` binds `FORMULA_ROOTS` (`record`) — a
30+
* proper subset.
31+
*
32+
* So the helper's verdict is adopted exactly on the three slots whose bound set
33+
* IS the field-rule set, and the local fallback is KEPT for the rest. The two
34+
* "uncovered surface" pins below are live controls for the failure this card
35+
* exists to stop, in miniature: routing every surface through the helper to
36+
* make the code tidier would redden them.
37+
*
38+
* Severity stays objectui's own mapping (`warning`, never `error`) on both
39+
* paths — every save gate on this tier counts `severity === 'error'`.
40+
*/
41+
42+
import { describe, it, expect } from 'vitest';
43+
import { fieldRuleRootIssue, FIELD_RULE_BOUND_ROOTS } from '@objectstack/lint';
44+
import { SCOPE_ROOTS } from '@objectstack/formula';
45+
import { lintCelPredicate } from './celAuthoring';
46+
47+
const HINT = { objectName: 'account', fields: ['organization_id', 'owner_id', 'status', 'amount'] };
48+
/** A field conditional rule — a slot the published helper's vocabulary covers. */
49+
const RULE_SLOT_HINT = { ...HINT, scope: 'record' as const, slot: 'visibleWhen' as const };
50+
/** The same tier with no slot named — the local fallback, unchanged. */
51+
const UNSLOTTED_HINT = { ...HINT, scope: 'record' as const };
52+
53+
/** objectui's own advisory sentence; the engine's never contains it. */
54+
const OBJECTUI_MESSAGE_MARKER = /Re-root/;
55+
56+
describe('celAuthoring · the field-rule verdict is the published one (objectui#9318)', () => {
57+
it('TRUE POSITIVE — a covered slot takes BOTH the verdict and the message from `fieldRuleRootIssue`', async () => {
58+
const source = "data.status == 'x'";
59+
const issues = await lintCelPredicate(source, RULE_SLOT_HINT);
60+
const engine = fieldRuleRootIssue('visibleWhen', source);
61+
// The helper has something to say about this source — if it ever stops,
62+
// this pin is measuring nothing and must be re-derived, not relaxed.
63+
expect(engine).not.toBeNull();
64+
const advisory = issues.filter((i) => i.severity === 'warning' && i.message === engine!.message);
65+
// Verbatim equality against the helper's OWN output, evaluated here rather
66+
// than transcribed: this asserts "objectui ships the engine's message" and
67+
// can never drift with upstream wording the way a quoted string would.
68+
expect(advisory).toHaveLength(1);
69+
});
70+
71+
it('ships ONE message, never both — objectui\'s sentence is gone from the covered path', async () => {
72+
const issues = await lintCelPredicate("data.status == 'x'", RULE_SLOT_HINT);
73+
expect(issues.filter((i) => OBJECTUI_MESSAGE_MARKER.test(i.message))).toEqual([]);
74+
expect(issues.filter((i) => i.severity === 'warning')).toHaveLength(1);
75+
});
76+
77+
it('LIVE CONTROL — the ACCEPT SET is not narrowed: the covered path raises no error', async () => {
78+
// Every save gate on this tier counts `severity === 'error'` and nothing
79+
// else, so "zero errors" IS "still accepted". Reddens on exactly one
80+
// change — promoting the advisory to `error`.
81+
const issues = await lintCelPredicate("data.status == 'x'", RULE_SLOT_HINT);
82+
expect(issues.filter((i) => i.severity === 'error')).toEqual([]);
83+
});
84+
85+
it('a root the platform DOES bind comes back clean THROUGH the new path', async () => {
86+
// The ablation target: `previous` and `parent` are clean here because
87+
// `FIELD_RULE_BOUND_ROOTS` says the field level binds them, not because
88+
// objectui's detector only ever looked at `data`.
89+
expect(await lintCelPredicate("previous.status == 'x'", RULE_SLOT_HINT)).toEqual([]);
90+
expect(await lintCelPredicate('parent.status == "paid"', RULE_SLOT_HINT)).toEqual([]);
91+
expect(await lintCelPredicate("record.status == 'x'", RULE_SLOT_HINT)).toEqual([]);
92+
});
93+
94+
it('takes the WIDER verdict on a covered slot: a root no field rule binds is advised too', async () => {
95+
// A declared behaviour change, and the substance of adopting the published
96+
// verdict: `current_user` is in the engine's baseline `SCOPE_ROOTS`, so
97+
// `validateExpression` says nothing about it, while the FIELD level does
98+
// not bind it. objectui's one-root detector could never reach this.
99+
// "WIDER" in the name is this verdict against the ONE-ROOT detector it
100+
// replaces — ⛔ not a set relation between the two surfaces' bound roots,
101+
// which are incomparable (see the header).
102+
const issues = await lintCelPredicate('current_user.isAdmin', RULE_SLOT_HINT);
103+
const advisory = issues.filter((i) => i.severity === 'warning' && /current_user/.test(i.message));
104+
expect(advisory).toHaveLength(1);
105+
expect(issues.filter((i) => i.severity === 'error')).toEqual([]);
106+
});
107+
108+
it('stands down on a parse error and on a non-CEL dialect, like the path it replaces', async () => {
109+
const broken = await lintCelPredicate('data.status ==', RULE_SLOT_HINT);
110+
expect(broken.some((i) => i.severity === 'error')).toBe(true);
111+
expect(broken.filter((i) => i.severity === 'warning' && /\bdata\b/.test(i.message))).toEqual([]);
112+
const legacy = await lintCelPredicate('${data.status}', RULE_SLOT_HINT);
113+
expect(legacy.filter((i) => i.severity === 'warning' && /\bdata\b/.test(i.message))).toEqual([]);
114+
});
115+
});
116+
117+
describe('celAuthoring · the UNCOVERED surfaces keep the local fallback (objectui#9318 part 3)', () => {
118+
it('LIVE CONTROL — a formula `expression` still gets objectui\'s message, not the engine\'s', async () => {
119+
// `FORMULA_ROOTS` is `['record']` — a proper SUBSET of
120+
// `FIELD_RULE_BOUND_ROOTS`. Routing this through the helper would not shrink
121+
// coverage: measured, `previous.*` / `parent.*` report nothing on this
122+
// surface today either. It would report them CLEAN on the helper's own
123+
// authority — asserting they are bound where this surface binds only
124+
// `record`. A false green, which is worse than the silence it replaces.
125+
const issues = await lintCelPredicate('data.amount * 0.2', { ...UNSLOTTED_HINT, role: 'value' as const });
126+
const advisory = issues.filter((i) => i.severity === 'warning' && OBJECTUI_MESSAGE_MARKER.test(i.message));
127+
expect(advisory).toHaveLength(1);
128+
expect(issues.filter((i) => i.severity === 'error')).toEqual([]);
129+
});
130+
131+
it('LIVE CONTROL — a conditional-formatting condition is NOT advised for the roots it binds', async () => {
132+
// `ROW_PREDICATE_ROOTS` carries `current_user`, `user`, `features`, `os`,
133+
// `ctx` on top of `record` — five roots the field tier does not bind. It is
134+
// NOT a superset of `FIELD_RULE_BOUND_ROOTS` though: it lacks `previous` and
135+
// `parent`, so the two sets overlap on `record` alone. This is the pin that
136+
// reddens if someone routes every `scope: 'record'` surface through the
137+
// helper: the author would be told to rewrite five roots that work here.
138+
expect(await lintCelPredicate('current_user.isAdmin', UNSLOTTED_HINT)).toEqual([]);
139+
expect(await lintCelPredicate("os.name == 'x'", UNSLOTTED_HINT)).toEqual([]);
140+
});
141+
142+
it('an unknown slot name falls back rather than guessing', async () => {
143+
const issues = await lintCelPredicate('current_user.isAdmin', { ...UNSLOTTED_HINT, slot: 'someFutureWhen' });
144+
expect(issues).toEqual([]);
145+
});
146+
});
147+
148+
describe('celAuthoring · platform drift tripwire (objectui#9318)', () => {
149+
it('the published bound set is still the one objectui measured its coverage answer against', async () => {
150+
// NOT a second copy of the verdict — the verdict is read from the helper at
151+
// runtime and this assertion is never consulted by product code. It exists
152+
// so that the next root the platform binds or unbinds arrives as a RED test
153+
// in objectui, with the instruction to re-derive which of this repo's
154+
// surfaces the helper's vocabulary still covers (PR objectui#9318 part 3).
155+
expect([...FIELD_RULE_BOUND_ROOTS]).toEqual(['record', 'previous', 'parent']);
156+
});
157+
158+
it('SWEEPS the whole unbound population: every root is advised EXCEPT `app`', async () => {
159+
// The instrument behind the docblock's universal. The published prose says
160+
// "every root the field level leaves unbound is advised, except `app`" and
161+
// ⛔ names no others — because any written-down list goes stale the next
162+
// time the platform moves a root. This sweep re-derives the population on
163+
// every run instead, so the claim cannot rot silently: it reddens when a
164+
// root stops being advised, when a SECOND root starts being blocked, or
165+
// when any root goes silent.
166+
//
167+
// ⚠️ The universe is RECONSTRUCTED, and that is a declared limit, not an
168+
// oversight: `@objectstack/lint` keeps both `FIELD_RULE_JUDGED_ROOTS` and
169+
// `FIELD_RULE_AMBIENT_ROOTS` module-private (neither is in its export list
170+
// at 17.4.0), so the judged set is rebuilt here as `SCOPE_ROOTS` plus the
171+
// one ambient root. A NEW ambient root added upstream would be invisible to
172+
// this sweep until that literal is updated — nothing in this repo can see
173+
// it. The `app` leg below pins the reconstruction itself.
174+
const judged: readonly string[] = [...SCOPE_ROOTS, 'app'];
175+
const bound: readonly string[] = FIELD_RULE_BOUND_ROOTS;
176+
const candidates = judged.filter((r) => !bound.includes(r));
177+
expect(candidates.length).toBeGreaterThan(1); // the sweep is measuring something
178+
179+
const advised: string[] = [];
180+
const blocked: string[] = [];
181+
const silent: string[] = [];
182+
for (const root of candidates) {
183+
const source = `${root}.x == 1`;
184+
const issues = await lintCelPredicate(source, RULE_SLOT_HINT);
185+
const engine = fieldRuleRootIssue('visibleWhen', source);
186+
// Verbatim equality with the helper's own output, as elsewhere in this
187+
// file — never a transcribed string.
188+
if (engine && issues.some((i) => i.severity === 'warning' && i.message === engine.message)) advised.push(root);
189+
else if (issues.some((i) => i.severity === 'error')) blocked.push(root);
190+
else silent.push(root);
191+
}
192+
193+
// The universal, and the single documented exception.
194+
expect(silent).toEqual([]);
195+
expect(blocked).toEqual(['app']);
196+
expect(advised).toEqual(candidates.filter((r) => r !== 'app'));
197+
});
198+
199+
it('WHY `app` is the exception: the helper judges it, the platform does not DECLARE it', async () => {
200+
// The exception is a mechanism, not a special case: the advisory is gated
201+
// behind `issues.every((i) => i.severity !== 'error')`, and a root the
202+
// engine does not declare raises a bare-reference ERROR first. `app` is the
203+
// only judged root in that position — it is AMBIENT (renderer-mounted),
204+
// which is exactly why the engine's own message for it refuses the
205+
// `record.app` rewrite by name.
206+
expect(SCOPE_ROOTS).not.toContain('app');
207+
expect(fieldRuleRootIssue('visibleWhen', 'app.theme == "dark"')).not.toBeNull();
208+
const issues = await lintCelPredicate('app.theme == "dark"', RULE_SLOT_HINT);
209+
expect(issues.some((i) => i.severity === 'error')).toBe(true);
210+
expect(issues.filter((i) => i.severity === 'warning')).toEqual([]);
211+
});
212+
});

0 commit comments

Comments
 (0)