diff --git a/.changeset/7727-conditional-formatting-record-scope.md b/.changeset/7727-conditional-formatting-record-scope.md new file mode 100644 index 0000000000..c0ad022264 --- /dev/null +++ b/.changeset/7727-conditional-formatting-record-scope.md @@ -0,0 +1,115 @@ +--- +'@object-ui/app-shell': minor +--- + +Lint conditional-formatting conditions in the `record` scope, stop advertising `data` +(objectui#7727), and align the predicate scope's root vocabulary to the engine's — +`app` is removed, `os` is advertised (objectui#8155). + +**Breaking for authors, deliberately.** A bare field reference in a list/grid/kanban +`conditionalFormatting` condition — `status == 'overdue'` — used to lint clean in +Studio's conditional-formatting editor and now raises a blocking error carrying the +`record.status` fix. + +**Read this before upgrading.** The error is a *blocking* one: it bubbles through +`onBlockingIssuesChange` (objectui#4527), which the inspector aggregates and the host +that owns Save reads. So an already-saved view whose `conditionalFormatting` carries a +legacy bare condition becomes **unsavable in the designer until that condition is +rewritten** — including when you opened the view to change something unrelated. Nothing +is migrated automatically and nothing at runtime changes: those conditions were already +dead (see below), the editor just stops hiding it. Rewrite `status == 'overdue'` as +`record.status == 'overdue'`. + +The editor was teaching a spelling the runtime had already retired. objectui#5741 +(Phase 2 of the objectui#5330 canon, ruled 2026-09-02 and amended 2026-09-05) unbound +the bare shorthand and `data.*` on runtime record surfaces: `evalRowPredicate` binds the +row as `record.*` and nothing else, so `status == 'overdue'` faults with +`Unknown variable: status` and the authored rule never matches. The editor nevertheless +linted it green, because it authored in the `flattened` scope — where any bare +identifier is legal. That is declared-but-unenforced in the direction that costs an +author a silently dead formatting rule. + +On `ConditionalFormattingEditor`: + +- its `CelPredicateField` authors in `scope="record"`, the scope the field conditional + rules `visibleWhen` / `readonlyWhen` / `requiredWhen` already use; +- `ROW_PREDICATE_ROOTS` loses `'data'`, which Phase 2 retired but autocomplete was + still recommending. It is an `export const`, but **not** on this package's + published face: `@object-ui/app-shell`'s `index.ts` has no `export *` lines and + re-exports neither the const nor this editor, and the package `exports` map is + `"."` plus `./styles.css` with no deep subpath — so no consumer outside the + package can import it, and nothing you depend on changes shape; +- the docblock and inline comment that described the old three-way binding are + rewritten to the one binding that survives. + +## The `app` root is removed from the predicate scope (objectui#8155) + +Ruled 2026-09-07. `app` was the mirror of the bug above, one level up: app-shell +*bound* it, this editor *advertised* it, and the engine that lints the very same field +*refused* it — ADR-0068 declares `current_user` with the `user` / `ctx.user` aliases +and nothing named `app`, and `@objectstack/formula`'s `SCOPE_ROOTS` has no `app` +either. So `app.name == 'crm'` raised a blocking error whose suggested remedy, +`record.app`, was nonsense, and there was **no** spelling that both linted clean and +resolved. The ruling is that the engine's `SCOPE_ROOTS` is the contract and this +consumer aligns to it, rather than the engine growing a root to match this consumer. + +`buildExpressionScope` (`providers/ExpressionProvider.tsx`) therefore no longer binds +`app`, and `ROW_PREDICATE_ROOTS` no longer advertises it. + +⚠️ **This is breaking for anyone whose saved metadata spells `app.*`, and that +population cannot be measured from this repository.** In-tree usage is zero — swept +across `packages/`, `apps/`, `examples/` and `content/` with a firing control — but +metadata authored in real deployments lives outside this tree and no sweep here can +see it. Any predicate that reads `app.*` — a conditional-formatting condition, an +action `visible` / `disabled`, a field `visibleWhen` — stops resolving and, because +unresolvable visibility predicates **fail open**, will start reading as "yes" rather +than erroring. That is the accepted cost of the ruling, not an oversight. There is no +replacement root: `app` was never in the protocol. If you need a "current app" value in +a predicate, that is a spec/engine vocabulary widening to be filed (the producer-side +card, objectstack#16420, stays open as the record to reopen). + +`ExpressionProvider` still accepts an `app` prop and still publishes `app` on its React +**context value**, which components read as a plain value (`DashboardView` does). Only +the **expression scope** loses it — those are two different things, and only the second +was ever a CEL root. + +## `os` is now advertised (same ruling, opposite direction) + +`os` was the exact mirror: **bound** by `buildExpressionScope`, **accepted** by the +engine, and merely never offered — the one root an author could legitimately write but +would never be shown. It is also the spec's canonical identity spelling +(`os.user.id`) and the measured in-tree one: authored predicates spell +`record.owner == os.user.id` across `packages/core`, `packages/components` and +`packages/plugin-grid`, including a conditional-formatting `condition`. It joins +`ROW_PREDICATE_ROOTS`. This is additive — nothing that linted clean before stops doing +so. + +**Autocomplete moves with the scope.** Under `scope="record"`, `CelPredicateField` +builds its bare-position catalog with `fields: []`, so typing `sta` at the start of a +condition no longer offers `status`; fields are offered as member completion after +`record.` instead. That is the correct affordance for the new scope — the bare form it +used to complete is now an error — and the member-completion list itself is unchanged: +the engine's `introspectScope` returns byte-identical `fields` for `record` and +`flattened` (measured against `@objectstack/formula@17.2.0`; it echoes the caller's +`fields` hint rather than deriving one per scope). + +The `flattened` default at the shared authoring seam is **untouched**: RLS predicates +and flow conditions are not row surfaces (objectui#5738 stand-down 3) and stay +flattened. + +**What this does NOT close — one half is left open, and it is filed.** + +- **The `data.*` half.** Dropping `'data'` from `ROW_PREDICATE_ROOTS` stops + *recommending* it; it does not stop the lint *accepting* it. + `@objectstack/formula`'s `SCOPE_ROOTS` lists `data`, so `data.status == 'x'` still + lints clean at `scope:'record'` while resolving against the host's ambient `data` + rather than the row — constant-false, silently. Pinned here as a characterization + test, tracked as objectui#8166. This changeset closes the **bare-field** half of the + retirement only. + +⛔ And this editor is **not** the last authoring site still on the flattened default — +`ConditionBuilder` reaches it by passing no `scope` at all, which is why a grep for the +explicit spelling missed it. An action's `visible` / `disabled` guard is a row predicate +by the canon's own words and still lints bare refs clean. Filed as objectui#8167; ⛔ not +fixed here, because three of `ConditionBuilder`'s six callers need a per-surface tier +verdict first. diff --git a/content/docs/core/enhanced-actions.mdx b/content/docs/core/enhanced-actions.mdx index 21f1c1a46a..6b05e78171 100644 --- a/content/docs/core/enhanced-actions.mdx +++ b/content/docs/core/enhanced-actions.mdx @@ -154,8 +154,10 @@ gets its real widget, not a text box (ADR-0059): inherit label, type, options, lookup picker config, `multiple`, `accept`, and `maxSize` from the object's field definition; inline properties override. - `required` blocks submit while the value is empty; `visible` (a CEL - predicate over `features` / `current_user` / `app` / `data`) hides a param - entirely — e.g. gate a param on an opt-in server capability. + predicate over `features` / `current_user` / `data`) hides a param + entirely — e.g. gate a param on an opt-in server capability. There is no + `app` root: objectui#8155 removed it, because neither ADR-0068 nor + `@objectstack/formula`'s `SCOPE_ROOTS` declares one. - Values are passed through to the action exactly as the widget emits them (`number` → number, `date` → `YYYY-MM-DD`, lookup → record id(s), `file` → uploaded file descriptor(s); arrays when `multiple`). diff --git a/content/docs/guide/console-architecture.md b/content/docs/guide/console-architecture.md index 32c4b8f1f7..252e1eebe1 100644 --- a/content/docs/guide/console-architecture.md +++ b/content/docs/guide/console-architecture.md @@ -122,7 +122,7 @@ Navigation items can be conditionally hidden using expressions: } ``` -`ExpressionProvider` (`@object-ui/app-shell`) wraps the layout and provides an `ExpressionEvaluator` that resolves `${}` templates against context variables (`user`, `app`, `data`). +`ExpressionProvider` (`@object-ui/app-shell`) wraps the layout and provides an `ExpressionEvaluator` that resolves `${}` templates against context variables (`current_user` and its `user` / `ctx.user` / `os.user` aliases, `data`, `features`). It publishes `app` on the React context value for components to read, but does **not** bind it as an expression root — objectui#8155 removed that binding, because the engine's `SCOPE_ROOTS` has no `app`. ### 2. Action System diff --git a/content/docs/plugins/plugin-form.mdx b/content/docs/plugins/plugin-form.mdx index d87144b5d0..ccca7aaac8 100644 --- a/content/docs/plugins/plugin-form.mdx +++ b/content/docs/plugins/plugin-form.mdx @@ -193,8 +193,8 @@ table at the end of the next section. A tab may carry a `visibleWhen` predicate — the same slot, vocabulary and engine as the field-level rule (`string | { dialect?, source }`, a CEL predicate over the live record, evaluated by `@objectstack/formula` with the -host predicate scope bound, so it can read `current_user` / `app` / `data` / -`features` exactly as a field rule can). Like every conditional rule in this +host predicate scope bound, so it can read `current_user` / `data` / +`features` exactly as a field rule can — there is no `app` root, objectui#8155). Like every conditional rule in this system it **fails open**: a predicate that cannot be evaluated leaves the tab visible rather than hiding data behind a broken expression. diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index 2481e54b38..67e7cbd9fd 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -656,13 +656,14 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // `positions` — so `'sales' in current_user.positions`, the gate the server // enforces on write, faulted here rather than hiding the field. const expressionEvaluator = useMemo( + // ⛔ No `app`: objectui#8155 removed it from the predicate scope, because + // neither ADR-0068 nor the engine's `SCOPE_ROOTS` declares such a root. () => createExpressionEvaluator({ user: buildExpressionUser(user), - app: activeApp || {}, data: editingRecord || {}, features, }), - [user, activeApp, editingRecord, features], + [user, editingRecord, features], ); // objectui#5619 — `isWorkspaceAdminResolved` belongs in this readiness gate diff --git a/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts b/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts index 6e5faa5b6b..fa87c707fa 100644 --- a/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts +++ b/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts @@ -70,10 +70,19 @@ describe('objectui#6493 — buildExpressionScope binds one user object under all expect(scope.os.user).toBe(user); }); - it('binds app, data and features, and defaults every root to an empty object', () => { + it('binds data and features — and NOT `app` — defaulting every root to an empty object', () => { const scope = buildExpressionScope(); + // ⛔ No `app` (objectui#8155, ruled 2026-09-07). Neither ADR-0068 nor + // `@objectstack/formula`'s SCOPE_ROOTS declares such a root, so binding it + // made this tier the only place it existed: advertised by the + // conditional-formatting editor and refused by the linter judging the very + // same field, with no spelling that did both. + // + // `toStrictEqual` is what makes this a fence rather than a sample — a root + // added BACK reddens here just as loudly as one removed, and `app` + // returning to this bag is the drift the ruling is guarding against. expect(scope).toStrictEqual({ - current_user: {}, user: {}, ctx: { user: {} }, os: { user: {} }, app: {}, data: {}, features: {}, + current_user: {}, user: {}, ctx: { user: {} }, os: { user: {} }, data: {}, features: {}, }); // The identity above holds for the defaults too — the hand-written fallback // in `useExpressionContext` used to mint three separate empty objects. diff --git a/packages/app-shell/src/providers/ExpressionProvider.tsx b/packages/app-shell/src/providers/ExpressionProvider.tsx index c0dc6e2147..3bed17d338 100644 --- a/packages/app-shell/src/providers/ExpressionProvider.tsx +++ b/packages/app-shell/src/providers/ExpressionProvider.tsx @@ -43,7 +43,14 @@ const ExprCtx = createContext(null); /** The inputs an app-shell surface has when it needs a predicate scope. */ export interface ExpressionScopeInput { user?: Record; - app?: Record; + /** + * ⛔ No `app`. It is not an input to the predicate scope, because the scope + * does not bind it — objectui#8155, ruled 2026-09-07. Accepting an argument + * this builder then discards is the declared-but-not-enforced shape the same + * ruling exists to remove, so the parameter is gone rather than ignored. + * `ExpressionProvider` still takes an `app` prop and still publishes it on + * the React context value; that is a different thing from a CEL root. + */ data?: Record; features?: Record; } @@ -77,15 +84,36 @@ export interface ExpressionScopeInput { * spec`'s `page.zod.ts` documents for component `visibleWhen` ("the shipping * renderer additionally mounts `app`, `features`, `os.user` … renderer * behaviour, NOT contract-guaranteed"). It is bound here because it is what - * THIS tier's own diagnostic advice tells an author they may name. + * THIS tier's own diagnostic advice tells an author they may name. That quote + * still names `app`; this tier no longer mounts it — see below. + * + * ## Why there is no `app` root (objectui#8155, ruled 2026-09-07) + * + * There was one, and it was a root the protocol never declared. ADR-0068 + * declares `current_user` with the `user` / `ctx.user` aliases and nothing + * named `app`; `@objectstack/formula`'s `SCOPE_ROOTS` (`cel-engine.ts`) has no + * `app` either. So an authored `app.name == 'crm'` was bound HERE and refused + * by the engine that lints it — an editor advertising a root its own linter + * rejects, with the nonsense remedy `record.app` and no spelling that both + * lints clean and resolves. + * + * The ruling is that the engine's `SCOPE_ROOTS` is the contract and this + * consumer aligns to it, NOT that the engine grows a root to match this + * consumer (option A, objectstack#16420, is explicitly not taken and stays + * open as the record to reopen should a real need for a "current app" root + * ever be measured). ⛔ The other refused route was suppressing the diagnostic + * in `celAuthoring.ts`: that is the lenient-fallback shape AGENTS.md #0.1 + * bans. + * + * Every root below is one the engine accepts, so the three surfaces — what + * this binds, what the editor advertises, what the linter admits — now agree. */ export function buildExpressionScope({ user = {}, - app = {}, data = {}, features = {}, }: ExpressionScopeInput = {}): Record { - return { current_user: user, user, ctx: { user }, os: { user }, app, data, features }; + return { current_user: user, user, ctx: { user }, os: { user }, data, features }; } /** @@ -110,7 +138,9 @@ interface ExpressionProviderProps { export function ExpressionProvider({ children, user = {}, app = {}, data = {}, features = {} }: ExpressionProviderProps) { const value = useMemo(() => { - const evaluator = createExpressionEvaluator({ user, app, data, features }); + const evaluator = createExpressionEvaluator({ user, data, features }); + // `app` is still published on the context value — `DashboardView` reads it + // as a plain value. It is NOT handed to the evaluator: objectui#8155. return { user, app, data, features, evaluator }; }, [user, app, data, features]); @@ -120,8 +150,8 @@ export function ExpressionProvider({ children, user = {}, app = {}, data = {}, f // The SAME bag the evaluator above got — one builder, so the imperative and // the hook-driven halves of this provider cannot drift apart either. const scope = useMemo( - () => buildExpressionScope({ user, app, data, features }), - [user, app, data, features], + () => buildExpressionScope({ user, data, features }), + [user, data, features], ); return ( @@ -142,8 +172,16 @@ export function useExpressionContext(): ExpressionContextValue { // Through the same builder: the hand-written version gave `current_user`, // `ctx.user` and `os.user` three DIFFERENT empty objects, which ADR-0068 D1 // spells as aliases "pointing at the same object". - const fallback = { user: {}, app: {}, data: {}, features: {} }; - return { ...fallback, evaluator: createExpressionEvaluator(fallback) }; + // + // The scope input and the context value are no longer the same object: + // `app` is a readable context FIELD but not a CEL root (objectui#8155), so + // handing this bag straight to the builder would smuggle back the very + // binding the ruling removed. + // Left UNANNOTATED on purpose: annotating it `ExpressionScopeInput` widens + // every member to optional, and the spread below then fails to satisfy + // `ExpressionContextValue`, whose members are required. + const scope = { user: {}, data: {}, features: {} }; + return { ...scope, app: {}, evaluator: createExpressionEvaluator(scope) }; } return ctx; } diff --git a/packages/app-shell/src/views/RecordFormPage.tsx b/packages/app-shell/src/views/RecordFormPage.tsx index 22ff50652b..58263be09e 100644 --- a/packages/app-shell/src/views/RecordFormPage.tsx +++ b/packages/app-shell/src/views/RecordFormPage.tsx @@ -202,15 +202,16 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { // faulted here and failed OPEN while resolving normally on a nav item. const expressionEvaluator = useMemo( () => + // ⛔ No `app`: objectui#8155 removed it from the predicate scope, because + // neither ADR-0068 nor the engine's `SCOPE_ROOTS` declares such a root. createExpressionEvaluator({ // expressionUser already handles the anonymous fallback, so we can // pass it through unconditionally. user: expressionUser, - app: { name: appName }, data: {}, features, }), - [expressionUser, appName, features], + [expressionUser, features], ); // Resolve the field list using the same visibility-aware logic as the diff --git a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.test.tsx b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.test.tsx index 6177c8c833..3ea38151f5 100644 --- a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { describe, it, expect, afterEach } from 'vitest'; -import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { evalRowPredicate } from '@object-ui/core'; import { @@ -12,6 +12,7 @@ import { type ConditionalFormattingRuleDraft, } from './ConditionalFormattingEditor'; import { __setCelFormulaLoader } from './celAuthoring'; +import { buildExpressionScope } from '../../providers/ExpressionProvider.js'; afterEach(() => { cleanup(); @@ -133,15 +134,97 @@ describe('ConditionalFormattingEditor', () => { }); describe('ConditionalFormattingEditor · CEL authoring scope (#2571 follow-up)', () => { - it('lints a BARE field condition clean — row predicates bind fields bare at runtime', async () => { + it('flags a BARE field condition with the record. fix — the row binds only record.*', async () => { render(); - // The real engine must accept the bare form (evalRowPredicate spreads the - // row); flipping this editor to scope="record" would break this test. + // TURNED, deliberately (objectui#7727). This pin used to assert the + // opposite — "the real engine must accept the bare form (evalRowPredicate + // spreads the row)" — and its own comment predicted this edit: "flipping + // this editor to scope=\"record\" would break this test". objectui#5741 + // (Phase 2 of the objectui#5330 canon) retired the bare shorthand on + // runtime record surfaces, so `evalRowPredicate` no longer spreads the row + // and `status == 'overdue'` faults with `Unknown variable: status`. The + // editor must say so at authoring time rather than lint it clean; the + // runtime half of this claim is pinned in the contract suite below. + expect(await screen.findByText(/record\.status/, {}, { timeout: 3000 })).toBeTruthy(); + const ta = document.getElementById('cf-condition-0') as HTMLTextAreaElement; + await waitFor(() => expect(ta.getAttribute('aria-invalid')).toBe('true'), { timeout: 3000 }); + }); + + it('still lints a canonical record. condition clean', async () => { + render(); + // The other half of the narrowing: the scope flip must reject the retired + // spelling WITHOUT rejecting the canonical one. expect(await screen.findByText('perm.cel.valid', {}, { timeout: 3000 })).toBeTruthy(); const ta = document.getElementById('cf-condition-0') as HTMLTextAreaElement; expect(ta.getAttribute('aria-invalid')).not.toBe('true'); }); + it('lints EVERY advertised root clean in the record scope — the aligned state', async () => { + // Before objectui#8155 this could only claim FOUR of five advertised roots: + // `app` was advertised and REFUSED, so a comment saying "every advertised + // root" would have contradicted its own neighbour. The ruling removed `app` + // and added `os`, so what this editor advertises is now a subset of what + // `@objectstack/formula`'s SCOPE_ROOTS accepts, and the honest assertion is + // the total one. + // + // DERIVED from the advertised list rather than retyped: a root added here + // without the engine knowing it reddens on this line, which is the whole + // failure mode objectui#8155 was filed for. + const condition = ROW_PREDICATE_ROOTS.map((r) => + r === 'record' ? "record.status != ''" : `size(${r}) >= 0`, + ).join(' && '); + render(); + expect(await screen.findByText('perm.cel.valid', {}, { timeout: 3000 })).toBeTruthy(); + }); + + it('KNOWN GAP — a `data.*` condition still lints CLEAN although the row is not bound under it', async () => { + // NOT desired behaviour, and it is the half of the retirement this card + // does NOT close. Dropping `'data'` from ROW_PREDICATE_ROOTS stops + // RECOMMENDING it; it does not stop the lint ACCEPTING it, because + // `@objectstack/formula`'s `SCOPE_ROOTS` lists `data` and so the + // record-scope bare-reference check waves it through. `rowPredicateCanon.ts` + // already records exactly this for the server oracle: `data.status` is + // "⚠️ silently accepted" while the runtime faults on it. + // + // The runtime half is pinned in the contract suite below, where the same + // predicate against the same host bag evaluates to FALSE. Green here plus + // false there IS the defect. This test REDDENS when the acceptance is + // fixed, at which point objectui#8166 can be closed. + render(); + expect(await screen.findByText('perm.cel.valid', {}, { timeout: 3000 })).toBeTruthy(); + const ta = document.getElementById('cf-condition-0') as HTMLTextAreaElement; + expect(ta.getAttribute('aria-invalid')).not.toBe('true'); + }); + + it('ALIGNED (objectui#8155) — `app` is neither advertised nor bound, and the lint refuses it', async () => { + // This pin used to assert a CONTRADICTION on purpose: the editor advertised + // `app` while its own linter refused it, offering the nonsense remedy + // `record.app` and no spelling an author could use instead. The 2026-09-07 + // ruling took option B — objectui aligns to the engine's root vocabulary, + // rather than the engine growing a root to match this consumer — so the + // contradiction no longer exists and this pin asserts the ALIGNED state. + // + // ⭐ Deliberately THREE-sided, because the defect was a DISAGREEMENT + // between two producers and a one-sided pin would be half a pin. Each + // producer drifting back ON ITS OWN must redden here: + // - `app` back in ROW_PREDICATE_ROOTS -> the first expect fails + // - `app` back in buildExpressionScope -> the second expect fails + // - the engine growing an `app` root -> the DOM assertion fails + // The closure assertion in the contract suite below catches the pair + // moving TOGETHER; it cannot see either half moving alone, which is + // exactly the state objectui#8155 was filed about. + expect(ROW_PREDICATE_ROOTS).not.toContain('app'); + expect(Object.keys(buildExpressionScope({ user: { id: 'u1' } }))).not.toContain('app'); + + // The third side. The refusal itself is unchanged — what changed is that it + // is now CORRECT: nothing advertises `app`, nothing binds it, and the + // engine does not know it, so an author is never lured into writing it. + render(); + const ta = document.getElementById('cf-condition-0') as HTMLTextAreaElement; + await waitFor(() => expect(ta.getAttribute('aria-invalid')).toBe('true'), { timeout: 3000 }); + expect(screen.getByText(/bare reference/)).toBeTruthy(); + }); + it('still flags an unknown record. with did-you-mean', async () => { render(); expect(await screen.findByText(/did you mean/i, {}, { timeout: 3000 })).toBeTruthy(); @@ -176,36 +259,151 @@ describe('ConditionalFormattingEditor · CEL authoring scope (#2571 follow-up)', }); describe('ROW_PREDICATE_ROOTS ↔ evalRowPredicate runtime contract', () => { - // Shaped like the app-shell global predicate scope (ExpressionProvider, - // #1583/ADR-0068) that hosts pass into the shared row-predicate evaluator. const u = { id: 'u1' }; - const hostScope = { - current_user: u, + /** + * The app-shell global predicate scope that hosts hand to the shared + * row-predicate evaluator — READ FROM ITS PRODUCER, not modelled here. + * + * Why it is read rather than written out (objectui#7727). This block used to + * carry a hand-written literal including `data: {}`, and probed every + * advertised root with `size() >= 0`. For `data` that probe hit the + * HOST's own empty object and never the row, so it was green whether or not + * `data` named the row: a reading that could not fail, and therefore + * indistinguishable from one that passed — the exact trap + * `rowPredicateCanon.ts` documents for `data.*` on a record surface. + * + * Writing the bag out by hand is the same defect one level up: a literal + * cannot disagree with the producer, so it silently absorbs any drift. It had + * already drifted — the literal omitted `os`, which + * `buildExpressionScope` really does bind, and an assertion below therefore + * "proved" `os` unbound. Calling the producer is what makes these readings + * able to fail: if `buildExpressionScope` gains or loses a root, the closure + * assertion says so instead of quietly agreeing with itself. + * + * ⭐ There is no `app` here, and it could not be put back even by accident: + * objectui#8155 removed `app` from `ExpressionScopeInput` as well as from the + * returned bag, so passing one is a COMPILE error (TS2353, "'app' does not + * exist in type 'ExpressionScopeInput'"). That is a third fence on the same + * fact, and the only one that holds without the suite being run. + */ + const fullHostScope = buildExpressionScope({ user: u, - ctx: { user: u }, - app: { name: 'crm' }, data: {}, features: { beta: true }, - }; + }); + /** + * The ONE root the host binds that this editor deliberately does not + * advertise: `data`, retired on row surfaces (objectui#5741). It gets its own + * pin below, against `fullHostScope`, which does carry it. + * + * `os` used to sit here too. objectui#8155 ruled it back onto the advertised + * list in the same patch that removed `app`: it is bound here, ACCEPTED by + * the engine, and the measured in-tree identity spelling + * (`record.owner == os.user.id`), so withholding it was curation with + * nothing behind it. + */ + const CURATED_EXCLUSIONS = ['data']; + /** + * The same bag with those two removed. Probes for the ADVERTISED roots run + * against this one, so no probe can pass off a host binding as a row binding. + */ + const hostScope = Object.fromEntries( + Object.entries(fullHostScope).filter(([k]) => !CURATED_EXCLUSIONS.includes(k)), + ); + const row = { id: 'r1', status: 'overdue' }; + + /** Advertised roots the HOST binds — derived, never typed out. */ + const HOST_BOUND_ROOTS = Object.keys(fullHostScope).filter((k) => !CURATED_EXCLUSIONS.includes(k)); + + it('binds the row as `record`, and it is the ROW rather than a host `record`', () => { + // No host scope at all: only the row can be supplying `record`. + expect(evalRowPredicate("record.status == 'overdue'", row, { fallback: false })).toBe(true); + // And the row still wins over a host scope carrying its own `record` + // (listConditional.ts pins `record` AFTER the spread). + expect( + evalRowPredicate("record.status == 'overdue'", row, { + fallback: false, + scope: { ...hostScope, record: { status: 'paid' } }, + }), + ).toBe(true); + }); - it('every advertised root is bound when a row predicate evaluates', () => { + it('every OTHER advertised root is bound by the HOST — and unbound without it', () => { for (const root of ROW_PREDICATE_ROOTS) { - // `size() >= 0` is true iff the root resolves to a bound map — - // an unbound root faults and falls back to `false`. + if (root === 'record') continue; + expect(HOST_BOUND_ROOTS, `advertised root "${root}" is unaccounted for`).toContain(root); expect( - evalRowPredicate(`size(${root}) >= 0`, { id: 'r1' }, { fallback: false, scope: hostScope }), - `root "${root}" should be bound at runtime`, + evalRowPredicate(`size(${root}) >= 0`, row, { fallback: false, scope: hostScope }), + `root "${root}" should be bound by the host scope`, ).toBe(true); + // The half that makes the line above a reading: drop the host scope and + // the root must go unbound. Without this, a root bound by nothing in + // particular would still pass. + expect( + evalRowPredicate(`size(${root}) >= 0`, row, { fallback: false }), + `root "${root}" must come from the HOST scope, not from thin air`, + ).toBe(false); } + // ...and no member escapes the two assertions above by not being checked. + // Both sides are derived: the left from the editor, the right from + // `buildExpressionScope` minus the curated exclusions. Drift on either + // side — a root added to the host bag, a root added to or dropped from the + // advertised list — reddens here. + expect([...ROW_PREDICATE_ROOTS].sort()).toEqual([...HOST_BOUND_ROOTS, 'record'].sort()); + }); + + it('a BARE field ref no longer names the row — the editor ERROR matches the runtime', () => { + // The runtime half of the flipped authoring pin above (objectui#5741). + expect(evalRowPredicate("record.status == 'overdue'", row, { fallback: false, scope: hostScope })).toBe(true); + expect(evalRowPredicate("status == 'overdue'", row, { fallback: false, scope: hostScope })).toBe(false); + }); + + it('`data` is RETIRED: unadvertised, and an ambient host `data` never names the row', () => { + expect(ROW_PREDICATE_ROOTS).not.toContain('data'); + // A host may still legitimately carry its own ambient `data` — app-shell's + // `buildExpressionScope` does, and this is that bag rather than a model of + // it. That is what made the old probe useless... + const ambient = fullHostScope; + expect(evalRowPredicate('size(data) >= 0', row, { fallback: false, scope: ambient })).toBe(true); + // ...while the ROW is not reachable through it at all. Canonical spelling + // against the same scope, so the two differ only in the spelling. + expect(evalRowPredicate("record.status == 'overdue'", row, { fallback: false, scope: ambient })).toBe(true); + expect(evalRowPredicate("data.status == 'overdue'", row, { fallback: false, scope: ambient })).toBe(false); + // ⚠️ The line above is FALSE at runtime while the authoring pin above + // ("a `data.*` condition still lints CLEAN") is green. That pair is the + // half of the retirement this card does not close — objectui#8166. }); it('the engine-default extras stay unadvertised because they are NOT bound', () => { - for (const root of ['previous', 'input', 'os', 'vars']) { + // `os` is NOT in this list: it is genuinely bound, and since objectui#8155 + // it is advertised too, so asserting it here would be the same hand-model + // artefact as the old `data` probe, in the opposite direction. + for (const root of ['previous', 'input', 'vars']) { expect(ROW_PREDICATE_ROOTS).not.toContain(root); expect( - evalRowPredicate(`size(${root}) >= 0`, { id: 'r1' }, { fallback: false, scope: hostScope }), + evalRowPredicate(`size(${root}) >= 0`, row, { fallback: false, scope: hostScope }), `root "${root}" should NOT be bound at runtime`, ).toBe(false); } }); + + it('`os` is ADVERTISED — bound here, and known to the engine (objectui#8155)', () => { + // The mirror image of the `app` case, settled the other way by the same + // ruling: `app` was advertised-but-refused, `os` was accepted-and-bound but + // never offered — the one root an author could legitimately write and would + // never be shown. + // + // The bag here is `buildExpressionScope`'s own output, NOT a scope with + // `os` handed in by this test: injecting it would only have proved that + // `evalRowPredicate` forwards `scope`, which `size(zzz) >= 0` with `zzz` + // injected proves just as well. Reading the producer is what makes this a + // statement about app-shell. + expect(Object.keys(fullHostScope)).toContain('os'); + expect(ROW_PREDICATE_ROOTS).toContain('os'); + expect(CURATED_EXCLUSIONS).not.toContain('os'); + expect(evalRowPredicate('size(os) >= 0', row, { fallback: false, scope: hostScope })).toBe(true); + // The control that makes the line above a reading: a root the host bag does + // NOT carry is unbound against the very same scope. + expect(evalRowPredicate('size(zzz) >= 0', row, { fallback: false, scope: hostScope })).toBe(false); + }); }); diff --git a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx index 4e744f5691..0f1360fc9f 100644 --- a/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx @@ -35,20 +35,64 @@ import type { CelLintIssue } from './celAuthoring.js'; * * A formatting `condition` is evaluated by `@object-ui/core`'s * `evalRowPredicate` (ADR-0058 — list rows, grid rows, kanban cards), which - * binds the row's fields BARE, under `record.*`, and under `data.*`, plus the - * host shell's global predicate scope (`ExpressionProvider`, #1583/ADR-0068: - * `current_user` / `user` / `ctx` / `app` / `features`). The engine's default - * advertisement adds `previous` / `input` / `os` / `vars`, which are NOT bound - * for row predicates — suggesting those would author a condition that silently - * never matches, so this override pins the truthful catalog (#2571 follow-up). + * binds the row ONE way — as the `record` namespace — plus the host shell's + * global predicate scope (`ExpressionProvider`, #1583/ADR-0068: + * `current_user` / `user` / `ctx` / `os` / `features`). + * + * ## What changed, and why this list lost a member (objectui#7727) + * + * It used to bind the row THREE ways: bare fields, `record.*` and `data.*`. + * Phase 2 of the objectui#5330 canon (objectui#5741, ruled 2026-09-02, amended + * 2026-09-05) RETIRED the other two — see `@object-ui/core`'s + * `evaluator/rowPredicateCanon.ts`. Neither `status` nor `data.status` names + * this row any more; both fault as unknown variables, exactly as they always + * did on the server. + * + * `data` is therefore off this list. The subtlety worth keeping: a host scope + * may legitimately carry its OWN ambient `data` (app-shell's + * `buildExpressionScope` does), so `data.*` still RESOLVES — against the + * host's object rather than the row. That is the constant-false signature + * `rowPredicateCanon.ts` describes, and it is why "does `data` resolve?" is + * not a test of whether `data` names the row. + * + * The engine's default advertisement adds `previous` / `input` / `vars`, none + * of which are bound for row predicates at all. Suggesting an unbound root + * would author a condition that silently never matches, so this override pins + * the truthful catalog (#2571 follow-up). + * + * ## The two roots objectui#8155 settled, in opposite directions + * + * They were mirror images, and the ruling (2026-09-07) is that the engine's + * `SCOPE_ROOTS` is the contract this list aligns to — in BOTH directions. + * + * - ⛔ `app` is GONE. It was advertised here and bound by + * `buildExpressionScope`, but the engine refuses it: ADR-0068 declares no + * such root and `@objectstack/formula`'s `SCOPE_ROOTS` has no `app`, so the + * record-scope lint read `app.name` as a bare field and errored with the + * nonsense remedy `record.app`. This editor was advertising a root its own + * linter rejected. `buildExpressionScope` stopped binding it in the same + * patch, so all three surfaces now agree that `app` does not exist here. + * - ✅ `os` is ADDED. The mirror case: bound by `buildExpressionScope`, + * ACCEPTED by the engine, and merely unadvertised — so it was the one root + * an author could legitimately write but was never offered. It is also the + * root authors actually reach for: `os.user.id` is the identity spelling + * ADR-0068 declares and the `@objectstack/spec` expression docs describe, + * and in-tree authored predicates spell `record.owner == os.user.id` across + * `packages/core`, `packages/components` and `packages/plugin-grid`, + * including a conditional-formatting `condition` in + * `core/src/evaluator/__tests__/listConditional.test.ts`. Withholding a root + * that is bound, accepted AND used was curation with nothing behind it. + * + * `data` is deliberately still absent, and that is NOT the same case: the + * engine accepts it but the row is not reachable through it. That half is + * objectui#8166. */ export const ROW_PREDICATE_ROOTS = [ 'record', 'current_user', 'user', 'features', - 'app', - 'data', + 'os', 'ctx', ]; @@ -326,11 +370,15 @@ export function ConditionalFormattingEditor({ placeholder="record.status == 'overdue'" objectName={objectName} fieldNames={fieldNames} - // Row predicates bind the row's fields BARE at runtime - // (`status == 'overdue'` works — evalRowPredicate spreads the - // row), so lint stays in the flattened scope; only the advertised - // roots change to the runtime-bound set. - scope="flattened" + // Row predicates bind the row as `record.*` and nothing else at + // runtime — objectui#5741 (Phase 2 of the objectui#5330 canon) + // retired the bare shorthand and `data.*`. So this lints in the + // RECORD scope, the same one the field conditional rules + // `visibleWhen` / `readonlyWhen` / `requiredWhen` use: a bare + // `status` is an ERROR carrying the `record.status` fix instead of + // linting clean and authoring a rule that never matches + // (objectui#7727). The advertised roots stay the runtime-bound set. + scope="record" roots={ROW_PREDICATE_ROOTS} onChange={(v) => setRule(i, { condition: v })} onLintChange={(issues) => reportCel(i, issues)}