From dbd6165ff266053336b045a09ff87e5423c9fade Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 19:29:33 +0000 Subject: [PATCH 1/3] fix(app-shell): stop offering `org.id` in ConditionBuilder's subject dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row builder's subject dropdown carries its own default vocabulary, which objectui#9645 did not reach: it narrowed the raw editor's autocomplete only. So `org.id` stayed one click away at every mount that declares no vocabulary. `org` is bound by no host — `buildExpressionScope` publishes the identity roots and no `org` — and `@objectstack/formula`'s `SCOPE_ROOTS` has none either, so the record-scope validator rejects `org.id` outright and names a record field that does not exist as the remedy. This editor was advertising a subject its own linter refuses: the `app` case objectui#8155 settled for `ConditionalFormattingEditor`, reached through a different control. `user.*` is deliberately left in place. The engine accepts it and the browser-side evaluator binds it, so it is a subject that really works at the client-evaluated mounts; a mount whose host does not bind it narrows its own list through the existing `subjects.context` vocabulary. The asymmetry that licensed objectui#9645's narrowing does not transfer here — `roots` feeds suggestions, while this list is the row builder's only subject control. `REFERENCE_ROOTS` deliberately keeps `org`: it answers whether a typed VALUE is a reference, and quoting one would rebuild the silently-false predicate objectui#6293 fixed. Co-authored-by: Claude --- .changeset/lucky-moons-shake.md | 24 +++ .../ConditionBuilder.contextSubjects.test.tsx | 175 ++++++++++++++++++ ...onditionBuilder.subjectVocabulary.test.tsx | 40 ++-- .../inspectors/ConditionBuilder.tsx | 49 ++++- 4 files changed, 273 insertions(+), 15 deletions(-) create mode 100644 .changeset/lucky-moons-shake.md create mode 100644 packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx diff --git a/.changeset/lucky-moons-shake.md b/.changeset/lucky-moons-shake.md new file mode 100644 index 0000000000..b6b27471ac --- /dev/null +++ b/.changeset/lucky-moons-shake.md @@ -0,0 +1,24 @@ +--- +'@object-ui/app-shell': minor +--- + +fix(app-shell): `ConditionBuilder`'s subject dropdown no longer offers `org.id` + +The row builder's subject dropdown kept its own default vocabulary, so `org.id` +stayed one click away at every mount that declares none — including the +server-evaluated ones. No host binds an `org` root (`buildExpressionScope` +publishes the identity roots and no `org`), and `@objectstack/formula`'s +`SCOPE_ROOTS` has none either, so the record-scope validator rejects `org.id` +outright and names a record field that does not exist as the remedy. The editor +was advertising a subject its own linter refuses — the `app` case objectui#8155 +settled for `ConditionalFormattingEditor`, reached here through a different +control. + +`user.*` is deliberately unchanged: the engine accepts it and the browser-side +evaluator binds it, so it is a subject that really works at the +client-evaluated mounts. A mount whose host does not bind it narrows its own +list through the existing `subjects.context` vocabulary. + +A predicate already authored against `org.id` is unaffected — it still opens in +row mode with its subject selectable, and the raw editor still treats a typed +`org.id` as a reference rather than quoting it. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx new file mode 100644 index 0000000000..49196053f9 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx @@ -0,0 +1,175 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `ConditionBuilder`'s SUBJECT DROPDOWN may not offer a root the engine + * refuses — objectui#9855, the second door of objectui#9645. + * + * ## The defect these cases reproduce + * + * objectui#9645 stopped the raw CEL editor's AUTOCOMPLETE from advertising + * roots a mount's host does not bind. The row builder's subject dropdown was + * not part of that ruling and kept its own default list, so `org.id` stayed + * one click away at every mount that declares no vocabulary. + * + * `org` was strictly worse than the roots objectui#9645 withdrew, and the two + * failures are not the same failure: + * + * - an over-advertised root there linted CLEAN and failed at runtime; + * - `org` is not in `@objectstack/formula`'s `SCOPE_ROOTS` at all, so the + * record-scope validator reads `org.id` as a bare field and REJECTS it + * outright, with a remedy naming a record field that does not exist. + * + * That is the `app` case `ROW_PREDICATE_ROOTS` (`ConditionalFormattingEditor`) + * settled under objectui#8155 — an editor advertising a root its own linter + * rejects — reached through a different control. + * + * ## Real engine, on purpose + * + * Like `ConditionBuilder.mountRoots.test.tsx` and + * `ConditionBuilder.mountScope.test.tsx`: the question under test is what the + * ENGINE says about a subject this component OFFERS. A stub would let the + * suite stay green while the dropdown went on offering the unofferable — which + * is precisely how `org.id` survived objectui#6296 and objectui#9645. + * + * ## The pins are DERIVED, not retyped + * + * The default vocabulary is read off the rendered dropdown and each subject's + * root is put to the real validator, so this file never restates the engine's + * answer (AGENTS.md #9). It fails when either side moves: a subject added to + * the default list that the engine refuses, and equally an engine that starts + * refusing one the list still offers. + * + * ## The must-not-break half + * + * `user.*` is deliberately STILL offered, and a case below holds it there. + * objectui#9855 proposed removing it too; the engine ACCEPTS it and + * `buildExpressionScope` binds it for every predicate evaluated in the + * browser, so withdrawing it by default would take a legitimate subject away + * from the client-evaluated mounts. Narrowing it belongs to the + * server-evaluated mounts as a DECLARED vocabulary — see the note on + * `CONTEXT_SUBJECTS`. An over-narrowing reddens here rather than shipping. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +// Module-scope import of the CEL engine, per AGENTS.md's flaky-test rule: the +// lint runs behind a dynamic `import('@objectstack/formula')` inside +// `celAuthoring`, and a cold first load has been measured near a `waitFor`'s +// whole budget. The specifier must match `loadFormula`'s exactly — ESM caches +// by resolved specifier. +import '@objectstack/formula'; + +// objectui#4697 — `ConditionBuilder` calls `useObjectFields(objectName)` +// unconditionally even when `fields` is supplied, so a mount-time fetch would +// otherwise escape to the real network. +const state = vi.hoisted(() => ({ + metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) }, +})); +vi.mock('../useMetadata', () => ({ + useMetadataClient: () => state.metadataClient, +})); + +import { ConditionBuilder } from './ConditionBuilder'; +import { lintCelPredicate } from '../celAuthoring'; + +afterEach(cleanup); + +const FIELDS = [{ name: 'status', label: 'Status' }]; + +/** Open the subject dropdown of a default-vocabulary mount and read it. */ +async function defaultSubjects(): Promise { + const { container } = render( + {}} + objectName="invoice" + fields={FIELDS} + />, + ); + await userEvent.click(container.querySelectorAll('[role="combobox"]')[0] as HTMLElement); + return (await screen.findAllByRole('option')).map((o) => o.textContent ?? ''); +} + +/** The CONTEXT half of the default vocabulary — what is not a catalog field. */ +function contextSubjectsOf(all: string[]): string[] { + return all.filter((s) => !s.startsWith('record.status') && s !== 'placeholder_subject'); +} + +/** Errors only; `warning` is advisory here as everywhere in this component. */ +async function errorsFor(src: string): Promise { + const issues = await lintCelPredicate(src, { scope: 'record' }); + return issues.filter((i) => i.severity === 'error').map((i) => i.message); +} + +/* ── The offered list, read against the engine that judges it ───────────── */ + +describe('the default subject vocabulary ↔ the engine (objectui#9855)', () => { + it('offers only subjects the record scope ACCEPTS — every one lints clean', async () => { + // THE GATE. Derived from what the dropdown actually renders rather than + // from a copy of the constant, so a subject added to the default list that + // the engine refuses reddens here — which is the reading `org.id` needed + // and never got. This is the `app` failure of objectui#8155, one component + // over and one control across. + const subjects = contextSubjectsOf(await defaultSubjects()); + expect(subjects.length, 'the dropdown rendered no context subjects — dead instrument').toBeGreaterThan(0); + for (const subject of subjects) { + expect( + await errorsFor(`${subject} == 'x'`), + `offered subject "${subject}" must be accepted at scope record`, + ).toEqual([]); + } + }); + + it('`org.id` is refused by the engine — the reading the removal rests on', async () => { + // The positive control for the case above, and the reason `org.id` is not + // on the list any more. Asserted as a REJECTION (the envelope's severity), + // not by matching the engine's wording: the remedy text is the engine's to + // change. If `SCOPE_ROOTS` ever grows an `org`, this reddens and the + // removal gets re-triaged instead of silently outliving its reason. + expect(await errorsFor("org.id == 'o1'")).not.toEqual([]); + expect(contextSubjectsOf(await defaultSubjects())).not.toContain('org.id'); + }); + + it('still offers `user.*` — the half that must NOT be narrowed by default', async () => { + // The `os` case of the objectui#8155 ruling, not the `app` case: accepted + // by the engine AND bound by `buildExpressionScope` for every predicate + // evaluated in the browser. Withdrawing it by default would take a working + // subject away from the client-evaluated mounts; a server-evaluated mount + // narrows it by DECLARING a `context` instead. Both legs are asserted so + // this cannot be satisfied by an empty dropdown. + const subjects = contextSubjectsOf(await defaultSubjects()); + expect(subjects).toContain('user.isAdmin'); + expect(subjects).toContain('record.id'); + expect(await errorsFor('user.isAdmin')).toEqual([]); + }); +}); + +/* ── Removing an OFFER may not orphan an authored predicate ─────────────── */ + +describe('a stored `org.id` predicate survives the withdrawal (objectui#9855)', () => { + it('still round-trips into row mode and keeps its subject selectable', async () => { + // The blast-radius half. Withdrawing an offer must not strand metadata an + // author already wrote: the row builder appends an out-of-vocabulary + // subject as its own option, so an existing `org.id` condition opens in + // ROW mode with its subject intact rather than blanking or falling back to + // the raw editor. Asserted through the dropdown, because the subject being + // *displayed* is not the same claim as it being *selectable*. + const { container } = render( + {}} + objectName="invoice" + fields={FIELDS} + />, + ); + expect(container.querySelectorAll('[aria-label="Remove condition"]').length).toBe(1); + await userEvent.click(container.querySelectorAll('[role="combobox"]')[0] as HTMLElement); + expect((await screen.findAllByRole('option')).map((o) => o.textContent ?? '')).toContain('org.id'); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.subjectVocabulary.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.subjectVocabulary.test.tsx index 348c77547b..da52bd17df 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.subjectVocabulary.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.subjectVocabulary.test.tsx @@ -12,13 +12,18 @@ * `includePrevious` (so `previous.FIELD` subjects, and the create-path * idiom `previous == null`, are offerable). * - * 2. DEFAULT-PATH INVARIANCE. Five consumer files mount this component - * (six mount sites — ActionDefaultInspector mounts it twice), all of them - * record-scoped, and all of them in production. They pass no vocabulary, - * so every one of them must keep behaving exactly as it did. The pins - * below assert the `record.` prefix POSITIVELY (the option list, and the - * CEL a fresh row compiles to) rather than snapshotting, so changing the - * default would go red rather than merely re-baseline. + * 2. DEFAULT-PATH INVARIANCE. Most consumer files mount this component + * without declaring a vocabulary, so every one of them must keep behaving + * exactly as it did. The pins below assert the `record.` prefix + * POSITIVELY (the option list, and the CEL a fresh row compiles to) + * rather than snapshotting, so changing the default would go red rather + * than merely re-baseline. + * + * ⚠️ This paragraph used to give the mount-site census as a figure and to + * state that no consumer passes `subjects`. Both had expired by the time + * objectui#9855 read them — the flow trigger declares a `context` today — + * so they are replaced by a pointer rather than a fresh number (AGENTS.md + * #9): `git grep ') { return { container, onCommit }; } -describe('#6296 DEFAULT-PATH INVARIANCE — the five record-scoped consumers are untouched', () => { - // Five files mount this component (six mount sites; ActionDefaultInspector - // mounts it twice) and none of them passes `subjects`. These assert the - // record-scoped default POSITIVELY, so changing it goes red. - it('offers record. for the catalog, plus the record/user/org context, and nothing else', async () => { +describe('#6296 DEFAULT-PATH INVARIANCE — the record-scoped consumers are untouched', () => { + // These assert the record-scoped default POSITIVELY, so changing it goes red. + // + // ⚠️ This block's heading used to carry a census of the mount sites and the + // claim that none of them passes `subjects`. Both had already expired when + // objectui#9855 read them — the flow trigger declares a `context` today — so + // the numbers are gone rather than refreshed (AGENTS.md #9: point at the + // instrument, never write down its answer). `git grep ' for the catalog, plus the record/user context, and nothing else', async () => { const { container } = mountOneRow(); const opts = await subjectOptions(container); expect(opts).toEqual([ @@ -246,7 +257,10 @@ describe('#6296 DEFAULT-PATH INVARIANCE — the five record-scoped consumers are 'user.email', 'user.role', 'user.isAdmin', - 'org.id', + // ⛔ no `org.id`: objectui#9855 withdrew it — no host binds an `org` root + // and the engine refuses the spelling. The verdict behind that removal is + // re-derived from the engine in `ConditionBuilder.contextSubjects.test.tsx`; + // this case is the shape half, and reddens if the offer comes back. // the row's own out-of-vocabulary subject is appended so it is not lost 'placeholder_subject', ]); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx index 07a8aaa7b2..83efca0ecb 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx @@ -50,6 +50,43 @@ const COMPARE_OPS: Array<{ value: Op; label: string }> = [ * The context subjects a record-scoped mount site binds. This is the DEFAULT * vocabulary — a caller that binds something else declares it via * {@link ConditionSubjectVocabulary.context}. + * + * ## `org.id` is gone — the objectui#8155 case, one control over (objectui#9855) + * + * It was the exact mirror of `app` in `ROW_PREDICATE_ROOTS` + * (`ConditionalFormattingEditor`): a root this editor ADVERTISED and its own + * linter REFUSES. `@objectstack/formula`'s `SCOPE_ROOTS` carries no `org`, so + * at `scope: 'record'` the validator reads `org.id` as a bare field and errors + * with the nonsense remedy `record.org` — and no host binds one either: + * `buildExpressionScope` (`ExpressionProvider`) publishes the identity roots + * and no `org`. Offering it cost the author the write, which is worse than the + * autocomplete half objectui#9645 closed: that one at least linted clean. + * `ConditionBuilder.contextSubjects.test.tsx` re-derives the verdict from the + * engine instead of restating it, so an engine that starts accepting `org` + * reddens there rather than leaving this paragraph to rot. + * + * ⚠️ This does NOT touch {@link REFERENCE_ROOTS}, which still carries `org` — + * see the note there. Withdrawing an OFFER is not the same act as re-reading + * an author's own bytes. + * + * ## `user.*` STAYS, and that is a measurement rather than an omission + * + * The card proposed removing both. `user` is the `os` case of that same + * objectui#8155 ruling, not the `app` case: the engine ACCEPTS it, and + * `buildExpressionScope` really does bind it for every predicate evaluated in + * the browser — so at a client-evaluated mount it is a subject an author can + * legitimately pick and a row that really matches. Where it is genuinely + * unbound is the SERVER-evaluated `condition`, and {@link + * RECORD_CONDITION_ROOTS} states in its own words why this component cannot + * tell those two apart from anything a mount passes. ⛔ So it is not narrowed + * by a default here; such a mount declares its own list through {@link + * ConditionSubjectVocabulary.context}, the way the flow trigger already does. + * + * ⚠️ The asymmetry that licensed objectui#9645's narrowing does NOT transfer + * to this list, which is why the two doors are not one edit. `roots` feeds the + * raw editor's SUGGESTIONS, so a mount that loses one loses an offer and keeps + * the spelling; this list is the row builder's ONLY subject control, so + * dropping an entry drops the author's ability to build that row at all. */ const CONTEXT_SUBJECTS = [ { value: 'record.id', label: 'record.id' }, @@ -57,7 +94,6 @@ const CONTEXT_SUBJECTS = [ { value: 'user.email', label: 'user.email' }, { value: 'user.role', label: 'user.role' }, { value: 'user.isAdmin', label: 'user.isAdmin' }, - { value: 'org.id', label: 'org.id' }, ]; const norm = (s: string) => s.replace(/\s+/g, ' ').trim(); @@ -107,10 +143,19 @@ export interface ConditionSubjectVocabulary { * Deliberately the roots this builder's own vocabulary already commits to — * not every root the CEL engine advertises: * - * - `record` / `user` / `org` — this component's own subject vocabulary + * - `record` / `user` — this component's own subject vocabulary * (`record.` from the field catalog, plus {@link CONTEXT_SUBJECTS}). * A value under one of these is the same identifier the subject dropdown * emits one control to its left. + * - `org` — no longer that case, and kept deliberately (objectui#9855). + * {@link CONTEXT_SUBJECTS} no longer offers `org.id`, so the dropdown does + * not emit it any more; this list stays wider because it answers a + * different question. An author who types `org.id` into the VALUE box means + * a reference, and quoting it would rebuild exactly the silently-false + * predicate objectui#6293 fixed — a loud unknown-variable fault is the + * better failure, and the only one a checker can see. That the two + * populations must not be merged is already pinned by + * `ConditionBuilder.mountRoots.test.tsx`. * - `previous` — the prior persisted record, bound by `evalFieldPredicate` * (`@object-ui/core`) and by the server-side hook / validation evaluators. * This is the change-detection idiom the defect was measured on. From 19d97035f005adce83586b287a877701ea2356fe Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 19:37:13 +0000 Subject: [PATCH 2/3] docs(app-shell): drop the expired mount-site census from the vocabulary docblock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `ConditionSubjectVocabulary` docblock gave the number of record-scoped mount sites as a figure. It had already expired, and this branch's own census contradicts it. Replaced with a pointer to the instrument that re-derives it, per AGENTS.md #9 — the same repair this branch already made to the matching claim in `ConditionBuilder.subjectVocabulary.test.tsx`. Co-authored-by: Claude --- .../src/views/metadata-admin/inspectors/ConditionBuilder.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx index 83efca0ecb..1fb16405da 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx @@ -103,7 +103,9 @@ const norm = (s: string) => s.replace(/\s+/g, ' ').trim(); * * The builder used to hardcode one: `record.` + field name, plus `record.id` / * `user.*` / `org.*`. That is right for every RECORD-scoped site — which is - * all five that mount it today — and wrong for a FLATTENED-scoped one such as + * most of the sites that mount it; grep the JSX tag to enumerate them, rather + * than trusting a figure here, which the census this paragraph used to give + * had already outlived (AGENTS.md #9) — and wrong for a FLATTENED-scoped one such as * the flow designer's entry condition, where the trigger record's fields ARE * the top-level evaluation context (bare `status`) and the prior values arrive * as `previous.FIELD`. See `flow-scope.ts`, which already computes exactly From de08d498434235a3c2187a10dc12c01eb9147ce3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 19:50:56 +0000 Subject: [PATCH 3/3] fix(app-shell): the server-evaluated condition mounts declare their own subjects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hook `condition` and an object validation rule's guard are evaluated by objectstack against `{ record, previous }` and nothing else — measured at source: `wrapDeclarativeHook` in `hook-wrappers.ts`, and `checkPredicate` / `checkConditional` in `validation/rule-validator.ts`. Neither binds `user`. So a `user.*` subject picked from the dropdown at those two mounts compiled a row that can never match: the hook wrapper throws on an unevaluable condition, and the rule validator is fail-closed, rejecting every write to the object. Both mounts now declare `context: RECORD_CONDITION_SUBJECTS`, the dropdown's mirror of `RECORD_CONDITION_ROOTS` — same ruling, the other door. Declared per mount, never defaulted, and deliberately NOT derived from `scope === 'record'`: that test does not separate the hosts. The action `visible` / `disabled` mounts declare `scope="record"` and are evaluated in the browser by `useCondition`, whose scope comes from `buildExpressionScope` and really does bind `user` — narrowing them would take away a subject that works. `scope` is a claim about how the CEL is LINTED, not about what the host binds. The `ConditionWidget` mount in `widgets.tsx` is left alone on purpose: it is polymorphic over `CONDITION_SCOPE_BY_METADATA_TYPE`, which carries both `hook` (server, two bindings) and `action` (client, `user` bound), so one `context` there would be right for one type and wrong for another. Co-authored-by: Claude --- .changeset/lucky-moons-shake.md | 38 +++--- .../ConditionBuilder.contextSubjects.test.tsx | 116 +++++++++++++++++- .../inspectors/ConditionBuilder.tsx | 40 ++++++ .../inspectors/HookDefaultInspector.tsx | 11 +- .../studio-design/ObjectValidationsPanel.tsx | 11 +- 5 files changed, 197 insertions(+), 19 deletions(-) diff --git a/.changeset/lucky-moons-shake.md b/.changeset/lucky-moons-shake.md index b6b27471ac..b06dfdc3e4 100644 --- a/.changeset/lucky-moons-shake.md +++ b/.changeset/lucky-moons-shake.md @@ -2,23 +2,31 @@ '@object-ui/app-shell': minor --- -fix(app-shell): `ConditionBuilder`'s subject dropdown no longer offers `org.id` +fix(app-shell): `ConditionBuilder`'s subject dropdown stops offering roots the host does not bind -The row builder's subject dropdown kept its own default vocabulary, so `org.id` -stayed one click away at every mount that declares none — including the -server-evaluated ones. No host binds an `org` root (`buildExpressionScope` -publishes the identity roots and no `org`), and `@objectstack/formula`'s -`SCOPE_ROOTS` has none either, so the record-scope validator rejects `org.id` -outright and names a record field that does not exist as the remedy. The editor -was advertising a subject its own linter refuses — the `app` case objectui#8155 -settled for `ConditionalFormattingEditor`, reached here through a different -control. +Two halves of one ruling, split by what each root actually is. -`user.*` is deliberately unchanged: the engine accepts it and the browser-side -evaluator binds it, so it is a subject that really works at the -client-evaluated mounts. A mount whose host does not bind it narrows its own -list through the existing `subjects.context` vocabulary. +**`org.id` leaves the default vocabulary.** No host binds an `org` root +(`buildExpressionScope` publishes the identity roots and no `org`), and +`@objectstack/formula`'s `SCOPE_ROOTS` has none either, so the record-scope +validator rejects `org.id` outright and names a record field that does not +exist as the remedy. The editor was advertising a subject its own linter +refuses — the `app` case objectui#8155 settled for `ConditionalFormattingEditor`, +reached here through a different control. -A predicate already authored against `org.id` is unaffected — it still opens in +**The server-evaluated mounts now declare their own narrower subject list.** A +hook `condition` and an object validation rule's guard are evaluated against +`{ record, previous }` and nothing else — the hook wrapper throws on an +unevaluable condition, and the rule validator is fail-closed, so a `user.*` +subject there built a row that could never match. Those two mounts declare +`context: RECORD_CONDITION_SUBJECTS`; the dropdown's default is unchanged. + +`user.*` is deliberately still offered everywhere else: the engine accepts it +and the browser-side evaluator binds it, so it is a subject that really works +at the client-evaluated mounts (action `visible` / `disabled`, page blocks). +The narrowing is declared per mount, never derived from `scope === 'record'` — +that test does not separate the hosts. + +A predicate already authored against `org.id` is unaffected: it still opens in row mode with its subject selectable, and the raw editor still treats a typed `org.id` as a reference rather than quoting it. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx index 49196053f9..c7517c0e8e 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx @@ -53,7 +53,7 @@ import '@testing-library/jest-dom/vitest'; import * as React from 'react'; import { describe, it, expect, vi, afterEach } from 'vitest'; -import { render, screen, cleanup } from '@testing-library/react'; +import { render, screen, cleanup, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; // Module-scope import of the CEL engine, per AGENTS.md's flaky-test rule: the @@ -73,7 +73,9 @@ vi.mock('../useMetadata', () => ({ useMetadataClient: () => state.metadataClient, })); -import { ConditionBuilder } from './ConditionBuilder'; +import { ConditionBuilder, RECORD_CONDITION_ROOTS, RECORD_CONDITION_SUBJECTS } from './ConditionBuilder'; +import { HookDefaultInspector } from './HookDefaultInspector'; +import { ObjectValidationsPanel } from '../../studio-design/ObjectValidationsPanel'; import { lintCelPredicate } from '../celAuthoring'; afterEach(cleanup); @@ -173,3 +175,113 @@ describe('a stored `org.id` predicate survives the withdrawal (objectui#9855)', expect((await screen.findAllByRole('option')).map((o) => o.textContent ?? '')).toContain('org.id'); }); }); + +/* ── The server-evaluated mounts DECLARE their narrower vocabulary ──────── */ + +/** + * objectui#9855 option B: the two mounts whose condition is evaluated by a + * SERVER host binding `{ record, previous }` declare their own `context`, + * rather than the component defaulting one for everybody. + * + * ⚠️ Membership of this set is a MEASUREMENT, not a shape. It was read at + * source in objectstack — `wrapDeclarativeHook` (`hook-wrappers.ts`) for the + * hook `condition`, `checkPredicate` / `checkConditional` + * (`validation/rule-validator.ts`) for a validation rule's guard — and both + * bind exactly two names. ⛔ It is NOT `scope === 'record'`: the action + * `visible` / `disabled` mounts declare that scope and are evaluated in the + * browser, where `buildExpressionScope` binds `user`, so they keep `user.*` + * and a case in the block above holds it there. + */ + +/** A `ConditionBuilder`'s own root, found by the toggle only it renders. */ +function builderRoot(): HTMLElement { + const toggle = screen.getAllByRole('button').find((b) => b.textContent?.includes('Expression')); + if (!toggle) throw new Error('no ConditionBuilder in row mode on screen — harness is dead'); + return toggle.parentElement!.parentElement! as HTMLElement; +} + +/** Open the subject dropdown of the first row and read what it offers. */ +async function rowSubjects(): Promise { + const trigger = within(builderRoot()).getAllByRole('combobox')[0] as HTMLElement; + await userEvent.click(trigger); + return (await screen.findAllByRole('option')).map((o) => o.textContent ?? ''); +} + +function HookHarness() { + const [draft, setDraft] = React.useState>({ + name: 'stamp', + label: 'Stamp', + object: 'invoice', + events: ['beforeInsert'], + condition: "record.status == 'open'", + }); + return ( + setDraft((d) => ({ ...d, ...patch }))} + readOnly={false} + locale={'en-US' as never} + /> + ); +} + +function ValidationHarness() { + const [draft, setDraft] = React.useState>({ + name: 'invoice', + fields: { status: { type: 'text' } }, + validations: [ + { + type: 'script', + name: 'rule_a', + label: 'rule_a', + message: 'nope', + severity: 'error', + active: true, + condition: "record.status == 'open'", + }, + ], + }); + return ( + setDraft((d) => ({ ...d, ...patch }))} + onBlockingIssuesChange={() => {}} + /> + ); +} + +describe('server-evaluated mounts declare `context` (objectui#9855 option B)', () => { + for (const [what, Harness] of [ + ['a hook `condition`', HookHarness], + ["an object validation rule's guard", ValidationHarness], + ] as const) { + it(`${what} offers no \`user.*\` subject — its host binds record and previous alone`, async () => { + render(); + const offered = await rowSubjects(); + // Both legs, so an empty or unopened dropdown cannot satisfy this. + expect(offered, 'the subject dropdown did not open — dead instrument').not.toHaveLength(0); + expect(offered, '`record.id` must survive the narrowing').toContain('record.id'); + const unbound = offered.filter((s) => s.startsWith('user.') || s.startsWith('org.')); + expect( + unbound, + `this mount is evaluated by a host binding { record, previous } only, so ${unbound.join(', ')} compiles a row that can never match`, + ).toEqual([]); + }); + } + + it('declares no subject under a root the server hosts do not bind', () => { + // Derived from BOTH constants, so this states the rule rather than a + // snapshot of today's single entry: the dropdown's list and the + // autocomplete's list are the two doors of one ruling and may not drift. + expect(RECORD_CONDITION_SUBJECTS.length, 'an empty list would satisfy every case below').toBeGreaterThan(0); + for (const s of RECORD_CONDITION_SUBJECTS) { + const root = s.value.split('.')[0]; + expect( + RECORD_CONDITION_ROOTS, + `declared subject "${s.value}" names root "${root}", which neither the hook wrapper nor the rule validator binds`, + ).toContain(root); + } + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx index 1fb16405da..6ff81256f1 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx @@ -241,6 +241,46 @@ const REFERENCE_RE = new RegExp( */ export const RECORD_CONDITION_ROOTS = ['record', 'previous']; +/** + * Context subjects for a mount whose condition is evaluated by a SERVER host + * that binds `record` and `previous` alone (objectui#9855). + * + * The dropdown's mirror of {@link RECORD_CONDITION_ROOTS} — same ruling, the + * other door. That list narrows what the raw editor's autocomplete OFFERS at + * such a mount; this one narrows what the row builder's subject dropdown + * offers, which is the control an author actually builds rows with. + * + * ## Why this is DECLARED by the mount, never defaulted + * + * ⛔ It is deliberately not the component's default, and ⛔ it is not derived + * from `scope === 'record'`. That test does not separate the hosts: the action + * `visible` / `disabled` mounts declare `scope="record"` and are evaluated in + * the BROWSER, where `buildExpressionScope` really does bind `user` — so + * defaulting to this list would take a working subject away from them. + * `scope` is a claim about how the CEL is LINTED; it is not a claim about what + * the host binds. `CONDITION_SCOPE_BY_METADATA_TYPE` (`conditionScope.ts`) + * makes the same distinction for the same reason. + * + * ## The two hosts this list is measured against + * + * Both were read at source in objectstack, and both bind exactly two names: + * + * - a hook `condition` — `wrapDeclarativeHook` (`hook-wrappers.ts`) evaluates + * it against `{ record, previous }`, and an unevaluable one throws + * `HookConditionError` rather than resolving false. + * - an object validation rule's guard — `checkPredicate` / `checkConditional` + * (`validation/rule-validator.ts`) evaluate `condition` / `when` against + * `{ record, previous }`, fail-CLOSED. `conditionScope.ts` carries the same + * ruling in its `validation` row. + * + * `previous` is bound at both and is reachable through + * {@link ConditionSubjectVocabulary.includePrevious}, which is a separate knob + * a mount opts into — so it is deliberately not folded in here. + */ +export const RECORD_CONDITION_SUBJECTS: ReadonlyArray<{ value: string; label?: string }> = [ + { value: 'record.id', label: 'record.id' }, +]; + /** * Quote a raw value for CEL unless it is a number / boolean / null — or a * reference (objectui#6293). diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx index 7c26cca5e7..2b64c04082 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx @@ -36,7 +36,7 @@ import { InspectorCheckboxField, } from './_shared.js'; import { useObjectOptions } from '../previews/useObjectOptions.js'; -import { ConditionBuilder } from './ConditionBuilder.js'; +import { ConditionBuilder, RECORD_CONDITION_SUBJECTS } from './ConditionBuilder.js'; import { expressionSource, writeExpressionSource } from './expression-envelope.js'; /* ─────────────── constants ─────────────── */ @@ -321,6 +321,15 @@ export function HookDefaultInspector({ objectName={conditionObject} disabled={readOnly} scope="record" + /* objectui#9855 — the SUBJECT dropdown's half of the same narrowing + the `scope` above buys for the autocomplete. `wrapDeclarativeHook` + evaluates this condition against `{ record, previous }` and + nothing else, so a `user.*` subject compiles a row that can never + match; offering it here was the row-builder door of the trap + objectui#9645 closed in the raw editor. Declared at the mount + rather than defaulted, because `scope="record"` does not imply a + server host — see `RECORD_CONDITION_SUBJECTS`. */ + subjects={{ context: RECORD_CONDITION_SUBJECTS }} onBlockingIssuesChange={reportCel} /> diff --git a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx index 04ccf5849c..ba1008971d 100644 --- a/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectValidationsPanel.tsx @@ -35,7 +35,7 @@ import React from 'react'; import { Plus, Trash2, ShieldAlert, ChevronDown } from 'lucide-react'; import { Popover, PopoverTrigger, PopoverContent } from '@object-ui/components'; -import { ConditionBuilder } from '../metadata-admin/inspectors/ConditionBuilder.js'; +import { ConditionBuilder, RECORD_CONDITION_SUBJECTS } from '../metadata-admin/inspectors/ConditionBuilder.js'; import { expressionSource, writeExpressionSource } from '../metadata-admin/inspectors/expression-envelope.js'; import { readFields } from '../metadata-admin/previews/object-fields-io.js'; import { t, useMetadataLocale } from '../metadata-admin/i18n.js'; @@ -380,6 +380,15 @@ function RuleTypeFields({ fields={fields} disabled={disabled} scope="record" + /* objectui#9855 — the SUBJECT dropdown's half of the narrowing the + `scope` above buys for the autocomplete. objectql's rule validator + evaluates this guard against `{ record, previous }` and nothing else + (`checkPredicate` for `condition`, `checkConditional` for `when`), + and an unevaluable predicate there is fail-CLOSED — so a `user.*` + subject builds a row that rejects every write to the object. + Declared at the mount, not defaulted: `scope="record"` does not + imply a server host — see `RECORD_CONDITION_SUBJECTS`. */ + subjects={{ context: RECORD_CONDITION_SUBJECTS }} onBlockingIssuesChange={onBlockingIssuesChange} />