From 930dd02b7f25116023c17baa335742c9b714f82b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 16:40:34 +0000 Subject: [PATCH] feat(core): B3 cascading-option guardrail, role-gated demo, ADR + browser e2e (#1583) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complete the objectui side of B3 (dynamic field options / cascading selects): a build-time guardrail for per-option `visibleWhen` predicates, a role-gated showcase example, ADR-0058, and a real-browser cascade e2e. The client feature itself shipped in #2284/#2215 (v12.0.0); this fills the remaining objectui checklist. - @object-ui/core: `lintOptionPredicates()` statically validates option `visibleWhen` predicates — CEL syntax (via @objectstack/formula `validateExpression`), unknown `record.` references, and a literal compared against an enum sibling outside its value domain (`record.country == 'chna'`, the #2284 AI-typo case). Conservative: only flags what it can statically prove; role/context predicates (`current_user.*`) and open-domain fields are left alone (no false positives). - examples/schema-catalog: role-gated select demo (`current_user.roles`), wired live into the Select field docs page; a new catalog test runs the guardrail over every shipped example. - e2e/cascading-options.spec.ts: drives the shipped country -> province example on the docs site (no backend), asserting the offered set re-filters live and a stale child value clears. Auto-skips when the docs site isn't served, like docs-smoke. - docs/adr/0058: the B3 decision record (per-option `visibleWhen` + `dependsOn` over `optionsWhen`/`validFor`; dual-side contract; server-enforcement contract). Server-side option-value enforcement and the live-backend e2e remain framework-side (tracked in #1583). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UFBvVjdHzB29XabjYxe1bN --- .changeset/b3-option-predicate-guardrail.md | 25 +++ content/docs/fields/select.mdx | 10 +- docs/adr/0058-cascading-select-options.md | 153 ++++++++++++++ e2e/cascading-options.spec.ts | 92 ++++++++ examples/schema-catalog/src/index.ts | 12 +- .../fields-select/role-gated-options.json | 29 +++ .../test/option-predicates.test.tsx | 55 +++++ .../evaluator/__tests__/optionLint.test.ts | 159 ++++++++++++++ packages/core/src/evaluator/index.ts | 1 + packages/core/src/evaluator/optionLint.ts | 200 ++++++++++++++++++ 10 files changed, 732 insertions(+), 4 deletions(-) create mode 100644 .changeset/b3-option-predicate-guardrail.md create mode 100644 docs/adr/0058-cascading-select-options.md create mode 100644 e2e/cascading-options.spec.ts create mode 100644 examples/schema-catalog/src/schemas/fields-select/role-gated-options.json create mode 100644 examples/schema-catalog/test/option-predicates.test.tsx create mode 100644 packages/core/src/evaluator/__tests__/optionLint.test.ts create mode 100644 packages/core/src/evaluator/optionLint.ts diff --git a/.changeset/b3-option-predicate-guardrail.md b/.changeset/b3-option-predicate-guardrail.md new file mode 100644 index 000000000..4e2db7d7a --- /dev/null +++ b/.changeset/b3-option-predicate-guardrail.md @@ -0,0 +1,25 @@ +--- +"@object-ui/core": minor +--- + +feat(core): build-time guardrail for cascading select option predicates (#1583) + +`@object-ui/core` now exports `lintOptionPredicates(fields)` — a static, +conservative validator for the per-option `visibleWhen` CEL predicates that +drive cascading / role-gated `select` options (#2284). An option predicate fails +*closed* — a wrong one makes its option silently never appear — so this catches +the class of bug runtime fail-open can't surface: + +- `syntax` — invalid CEL, delegated to `@objectstack/formula`'s + `validateExpression` (no schema hint, so a legitimate `current_user.roles` + reference is never mistaken for an error); +- `unknown-field` — a `record.` reference to a field the form never + declares (a sibling typo); +- `option-literal-not-in-domain` — a literal compared against an *enum* sibling + that is outside its declared option values, e.g. `record.country == 'chna'` + when `country` is `cn`/`us` (the AI-authoring typo #2284 called out). + +It only flags what it can statically prove — non-`record.` roots +(`current_user.*`), open-domain fields, and unrecognized shapes are left alone, +so there are no false positives. The schema catalog runs it over every shipped +example. Design recorded in ADR-0058. diff --git a/content/docs/fields/select.mdx b/content/docs/fields/select.mdx index 0b5c5e582..c4911cfa3 100644 --- a/content/docs/fields/select.mdx +++ b/content/docs/fields/select.mdx @@ -63,10 +63,14 @@ cascade-clear propagate down the chain. ### Role-gated option + + ```json -{ "name": "tier", "type": "select", "options": [ - { "label": "Standard", "value": "standard" }, - { "label": "Admin only", "value": "admin_only", "visibleWhen": "'admin' in current_user.roles" } +{ "name": "visibility", "type": "select", "options": [ + { "label": "Private (only me)", "value": "private" }, + { "label": "My team", "value": "team" }, + { "label": "Whole organization", "value": "org" }, + { "label": "Public — external", "value": "public", "visibleWhen": "'admin' in current_user.roles" } ]} ``` diff --git a/docs/adr/0058-cascading-select-options.md b/docs/adr/0058-cascading-select-options.md new file mode 100644 index 000000000..cd01e75d2 --- /dev/null +++ b/docs/adr/0058-cascading-select-options.md @@ -0,0 +1,153 @@ +# ADR-0058: Cascading & role-gated select options (`option.visibleWhen` + `dependsOn`) + +**Status**: Accepted — implemented (2026-07-15) +**Author**: ObjectUI renderer team +**Consumers**: `@object-ui/core`, `@object-ui/fields` (`SelectField`), `@object-ui/components` (form renderer), `@object-ui/plugin-form`, `@objectstack/spec`, `@objectstack/objectql`, every app whose forms need a select/lookup whose choices depend on another field or on who is editing +**Companion to**: ADR-0036 (field-level conditional rules) — this applies the same dual-side, one-dialect philosophy to *option* sets. Supersedes the `optionsWhen` framing in issue #1583. + +--- + +## TL;DR + +A select's **available options frequently depend on the rest of the record or on +the actor**: pick a Country, then State lists only that country's states; a +"visibility" field offers *Public* only to admins. These are not widget concerns +— they are data-model rules, authored once on the option and honored everywhere +the object is edited. + +We express them by reusing the two primitives ADR-0036 and the dependent-lookup +work (#2215) already established, **not** a new mechanism: + +| Knob | Meaning | Enforced on | +| ----------------------- | ------------------------------------------------------------- | ------------------- | +| `option.visibleWhen` | the option is offered when the CEL predicate is TRUE | client (UX) + server (for authorization) | +| `field.dependsOn` | which sibling field(s) the option list reacts to (gate/recompute) | client | + +A picklist cascade is therefore `dependsOn` (declares the dependency edge) + per-option +`visibleWhen` (the condition) — structurally identical to a lookup cascade. + +## Decision: per-option `visibleWhen`, not a field-level `optionsWhen` or `validFor` + +Issue #1583 floated a field-level `optionsWhen` predicate; #2284 settled the +authoring shape as **per-option `visibleWhen` + `dependsOn`**, explicitly +rejecting Salesforce-style `validFor` / `controllingField` / dependency matrices. +Rationale: + +1. **Minimal, unified vocabulary.** `dependsOn` is already how a lookup declares + its cascade; `visibleWhen` is already the field-level CEL predicate (ADR-0036). + Reusing both introduces no new category and no bespoke matrix format. +2. **`visibleWhen` is strictly more expressive than a value cascade.** It sees + `current_user` (roles/tenant) and `now()`, so the *same* knob covers cascades + (`record.country == 'cn'`) **and** role/context gating + (`'admin' in current_user.roles`). `validFor` can only express "varies with + another field's value" — it cannot gate by role. `current_user` is already + bound on every predicate surface (server formula, RLS, client UI gates), so a + role-referencing option predicate needs **zero new binding**. +3. **AI-authoring friendly.** Schemas are increasingly model-generated and + human-reviewed. `visibleWhen: "record.country == 'cn'"` reads as prose and is + a primitive the model has seen countless times; a `validFor` matrix's main + value (a visual editor) does not exist under this authoring model. +4. **`dependsOn` ⟂ `visibleWhen`.** `dependsOn` only declares which sibling + fields drive the list (gating UX + recompute); `visibleWhen` is the predicate. + An option may carry `visibleWhen` with **no** `dependsOn` (a pure role gate). + The two are never coupled. + +## Why CEL, and the same engine on both ends + +As in ADR-0036, the point of a dual-side rule is that the **client UX and the +persisted server verdict agree** for a given record. Both ends evaluate the +option predicate with the canonical `@objectstack/formula` `ExpressionEngine` +(CEL via `@marcbachmann/cel-js`) — the same dialect, stdlib, and null/missing +semantics the field-level rules use — via the shared `evalFieldPredicate` path. +No parallel client DSL, so no drift. + +## Client implementation (objectui) + +- **`@object-ui/core` — `evaluator/optionRules.ts`** exposes four zero-React helpers, + all reusing `evalFieldPredicate`: + - `resolveVisibleOptions(options, record, scope?)` — keep options whose + `visibleWhen` is TRUE against the live record (+ `{ current_user }` scope); + predicate-less options are always kept; a faulting predicate **fails open** + (option kept), matching field-visibility posture. + - `resolveDependsOnFields(dependsOn)` — normalize the spec `dependsOn` + (`string | Array`) to sibling field names. + - `isOptionGroupGated(dependsOn, record)` — TRUE while any `dependsOn` field is + empty; the list is withheld rather than shown unfiltered. + - `isValueStillOffered(value, visibleOptions)` — cascade-clear decision + (scalar and multi-select), so a parent change drops a now-invalid child value. +- **The form renderer** (`@object-ui/components`, `renderers/form/form.tsx`) + watches the live record (`ruleRecord`, every declared field seeded to `null` + first — the missing-key gotcha from ADR-0036), narrows each dependent select's + options, gates the control with a `Select {parent} first` hint while a + `dependsOn` field is empty, and **cascade-clears** a stale value in a reactive + effect (no "China + California" pair ever submits). +- **`SelectField`** (`@object-ui/fields`) applies the identical resolution + standalone via `dependentValues` (the channel dependent lookups use) + the + global `usePredicateScope()` (so `current_user` resolves). Filtered-out options + are absent from the DOM (`data-testid="select-option-"` only for offered + values); a gated field renders `select-empty-`. +- **`ObjectForm`** (`@object-ui/plugin-form`) carries `options` (and thus their + `visibleWhen`) and `dependsOn` through from object metadata onto the generated + `FormField`s. + +## Static options vs. query-backed lookups + +Two paths, one declaration style: + +- **Small, stable dictionaries** (category → subcategory, a handful of statuses, + role gates) → static `options` + `option.visibleWhen`. This ADR. +- **Large / changing / shared data** (countries, org trees, catalogs, + account → contact) → `lookup` + `dependsOn`, with the dependency folded into + the query `$filter` (#2215). The choice is a data-modeling decision, documented + in `skills/objectui/guides/schema-expressions.md`; the authoring surface + (`dependsOn`) is the same either way. + +## Build-time guardrail + +An option `visibleWhen` fails **closed**: a wrong predicate makes its option +silently never appear, which runtime fail-open cannot surface. The headline case +(#2284) is an AI-authored literal typo — `record.country == 'chna'` when +`country`'s options are `cn`/`us`, so the option is unreachable although the +expression parses and the field exists. + +`@object-ui/core`'s `lintOptionPredicates(fields)` (`evaluator/optionLint.ts`) +closes this at authoring/CI time with three conservative checks: `syntax` (via +`@objectstack/formula`'s `validateExpression`, no schema hint so a legitimate +`current_user.roles` reference is never flagged), `unknown-field` (a `record.` +naming no declared sibling), and `option-literal-not-in-domain` (a literal +compared against an *enum* sibling that is outside its declared value set). It +only flags what it can statically prove — non-`record.` roots, open-domain +fields, and unrecognized shapes are left alone, so there are no false positives. +The schema catalog runs it over every shipped example +(`examples/schema-catalog/test/option-predicates.test.tsx`). + +## Server enforcement (framework) — the authorization half + +Hiding an option client-side is **UX, not a security boundary**: a caller can +still submit a hidden value. When an option is gated for **authorization**, the +server must also reject writes of that value. The contract (framework +`@objectstack/objectql`, alongside ADR-0036's `requiredWhen`/`readonlyWhen`): + +- On write, evaluate the submitted option value's `visibleWhen` over the merged + record (`{ ...previous, ...patch }`) plus the actor; reject with a + `{ field, code }` violation when FALSE — mirroring how `stripReadonlyWhenFields` + already walks conditional fields (`needsPriorRecord`). +- A predicate that fails to evaluate is **fail-open** and logged (a broken rule + must never block a legitimate write), matching the client and ADR-0036. +- Pure cascades / UX-only gating do not require this; it is specifically the + authorization path. + +This ADR records the objectui (client + guardrail) work as shipped; the +server-side option-value enforcement and its live e2e are the framework-side +remainder tracked in #1583. + +## Consequences + +- Authors express dependent and role-gated choices once, on the option, in the + same CEL they already use for field rules and formulas — no widget wiring, no + new matrix format. +- Client and server cannot drift: identical engine and dialect. +- `option.visibleWhen` is UX by default; for authorization it must be paired with + server enforcement — never rely on client hiding for a security guarantee. +- A wrong option predicate is caught before it ships by `lintOptionPredicates`, + not discovered as a silently-missing choice in production. diff --git a/e2e/cascading-options.spec.ts b/e2e/cascading-options.spec.ts new file mode 100644 index 000000000..f68d5d0f7 --- /dev/null +++ b/e2e/cascading-options.spec.ts @@ -0,0 +1,92 @@ +import { test, expect, type Page } from '@playwright/test'; + +/** + * B3 browser e2e: cascading / dependent `select` options (#1583). + * + * Drives the shipped `fields-select/cascading-options` example + * (`country` → `province`) rendered live on the docs site — the same form + * renderer and the same `@objectstack/formula` per-option `visibleWhen` + * filtering an app uses, with **no backend**. This is the piece JSDOM component + * tests can't reach: opening a real dropdown and asserting the *offered set* + * changes as the controlling field changes, plus the cascade-clear of a + * now-invalid child value. + * + * The docs site (Next.js) must be served separately — e.g. + * `pnpm --filter @object-ui/site dev` — and pointed at via `DOCS_BASE_URL` + * (default `http://localhost:3000`). If it's unreachable the suite auto-skips, + * mirroring `docs-smoke.spec.ts`. The equivalent live-backend e2e (a cascading + * pair on a real showcase object, with the server rejecting an out-of-set + * value) is tracked framework-side in #1583. + */ + +const DOCS_BASE = process.env.DOCS_BASE_URL || 'http://localhost:3000'; +const SELECT_PAGE = `${DOCS_BASE}/docs/fields/select`; + +let docsAvailable = false; +test.beforeAll(async () => { + try { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), 5_000); + const response = await fetch(`${DOCS_BASE}/docs`, { signal: controller.signal }); + clearTimeout(timer); + docsAvailable = response.ok; + } catch { + docsAvailable = false; + } +}); + +/** The Radix Select trigger for a form field, scoped by its `field:` wrapper. */ +const trigger = (page: Page, field: string) => + page.locator(`[data-testid="field:${field}"] [role="combobox"]`); + +/** Open `field`'s dropdown, return the offered option labels (sorted), then close it. */ +async function offeredOptions(page: Page, field: string): Promise { + await trigger(page, field).click(); + const listbox = page.getByRole('listbox'); + await expect(listbox).toBeVisible(); + const labels = (await page.getByRole('option').allInnerTexts()).map((s) => s.trim()); + await page.keyboard.press('Escape'); + await expect(listbox).toBeHidden(); + return labels.sort(); +} + +/** Open `field`'s dropdown and pick the option named `name`. */ +async function pick(page: Page, field: string, name: RegExp): Promise { + await trigger(page, field).click(); + const listbox = page.getByRole('listbox'); + await expect(listbox).toBeVisible(); + await page.getByRole('option', { name }).click(); + await expect(listbox).toBeHidden(); +} + +test('province options re-filter live as country changes, and a stale value clears', async ({ page }) => { + test.skip(!docsAvailable, 'Docs site is not reachable (set DOCS_BASE_URL)'); + + await page.goto(SELECT_PAGE, { waitUntil: 'domcontentloaded', timeout: 30_000 }); + + const country = trigger(page, 'country'); + await country.scrollIntoViewIfNeeded(); + await expect(country).toBeVisible(); + // Let the client-side renderer hydrate before driving the Radix control — a + // pre-hydration click is dropped and the dropdown never opens. + await expect(country).toBeEnabled(); + await page.waitForTimeout(1500); + + // --- Gated: while country is unset the dependent province control is withheld. --- + await expect(trigger(page, 'province')).toHaveCount(0); + + // --- country = China → only Chinese provinces are offered. --- + await pick(page, 'country', /china/i); + await expect(trigger(page, 'province')).toBeVisible(); + expect(await offeredOptions(page, 'province')).toEqual(['Guangdong', 'Zhejiang']); + + // Choose one so we can prove the cascade-clear on the next parent change. + await pick(page, 'province', /zhejiang/i); + await expect(trigger(page, 'province')).toContainText(/zhejiang/i); + + // --- country = United States → the offered set flips and the stale value clears. --- + await pick(page, 'country', /united states/i); + expect(await offeredOptions(page, 'province')).toEqual(['California', 'Texas']); + // 'Zhejiang' is no longer offered under country=us, so the widget dropped it. + await expect(trigger(page, 'province')).not.toContainText(/zhejiang/i); +}); diff --git a/examples/schema-catalog/src/index.ts b/examples/schema-catalog/src/index.ts index 97e90911d..8eaf8ccec 100644 --- a/examples/schema-catalog/src/index.ts +++ b/examples/schema-catalog/src/index.ts @@ -346,6 +346,7 @@ import fields_select_basic_select from './schemas/fields-select/basic-select.jso import fields_select_cascading_options from './schemas/fields-select/cascading-options.json' with { type: 'json' }; import fields_select_colored_options from './schemas/fields-select/colored-options.json' with { type: 'json' }; import fields_select_multi_select from './schemas/fields-select/multi-select.json' with { type: 'json' }; +import fields_select_role_gated_options from './schemas/fields-select/role-gated-options.json' with { type: 'json' }; import fields_summary_average_of_field_values from './schemas/fields-summary/average-of-field-values.json' with { type: 'json' }; import fields_summary_count_of_related_records from './schemas/fields-summary/count-of-related-records.json' with { type: 'json' }; import fields_summary_sum_of_field_values from './schemas/fields-summary/sum-of-field-values.json' with { type: 'json' }; @@ -3526,7 +3527,7 @@ const REGISTRY: Record = { id: 'fields-select/cascading-options', meta: { title: "Cascading Options", - description: "Dependent select — province options narrow to the chosen country via per-option visibleWhen + dependsOn", + description: "", category: 'fields-select', }, schema: fields_select_cascading_options, @@ -3549,6 +3550,15 @@ const REGISTRY: Record = { }, schema: fields_select_multi_select, }, + 'fields-select/role-gated-options': { + id: 'fields-select/role-gated-options', + meta: { + title: "Role Gated Options", + description: "", + category: 'fields-select', + }, + schema: fields_select_role_gated_options, + }, 'fields-summary/average-of-field-values': { id: 'fields-summary/average-of-field-values', meta: { diff --git a/examples/schema-catalog/src/schemas/fields-select/role-gated-options.json b/examples/schema-catalog/src/schemas/fields-select/role-gated-options.json new file mode 100644 index 000000000..a5a31bf9c --- /dev/null +++ b/examples/schema-catalog/src/schemas/fields-select/role-gated-options.json @@ -0,0 +1,29 @@ +{ + "type": "form", + "showSubmit": false, + "showCancel": false, + "fields": [ + { + "name": "subject", + "label": "Subject", + "type": "text", + "placeholder": "Short summary..." + }, + { + "name": "visibility", + "label": "Visibility", + "type": "select", + "placeholder": "Select visibility...", + "options": [ + { "label": "Private (only me)", "value": "private" }, + { "label": "My team", "value": "team" }, + { "label": "Whole organization", "value": "org" }, + { + "label": "Public — external", + "value": "public", + "visibleWhen": "'admin' in current_user.roles" + } + ] + } + ] +} diff --git a/examples/schema-catalog/test/option-predicates.test.tsx b/examples/schema-catalog/test/option-predicates.test.tsx new file mode 100644 index 000000000..38bc41a15 --- /dev/null +++ b/examples/schema-catalog/test/option-predicates.test.tsx @@ -0,0 +1,55 @@ +/** + * Catalog-wide guardrail for cascading `select` option predicates (#1583). + * + * Every shipped example is a reference authors copy-paste (and AI few-shot + * retrieves), so a wrong option `visibleWhen` here silently teaches the mistake. + * We walk every `fields` array in every example and run the canonical + * `lintOptionPredicates` guardrail (`@object-ui/core`): a per-option predicate + * that references an unknown sibling, compares against a literal outside the + * controlling enum's value domain (`record.country == 'chna'`), or doesn't parse + * as CEL fails the catalog. Fields without option predicates contribute nothing. + */ +import { describe, it, expect } from 'vitest'; +import { lintOptionPredicates, type LintFieldLike, type OptionLintIssue } from '@object-ui/core'; +import { allExamples } from '../src/index.js'; + +/** Collect every `fields: [...]` array anywhere in a schema tree. */ +function collectFieldGroups(node: unknown, out: LintFieldLike[][] = []): LintFieldLike[][] { + if (Array.isArray(node)) { + for (const item of node) collectFieldGroups(item, out); + return out; + } + if (node && typeof node === 'object') { + const rec = node as Record; + if (Array.isArray(rec.fields)) out.push(rec.fields as LintFieldLike[]); + for (const value of Object.values(rec)) collectFieldGroups(value, out); + } + return out; +} + +function lintExample(schema: unknown): OptionLintIssue[] { + return collectFieldGroups(schema).flatMap((fields) => lintOptionPredicates(fields)); +} + +describe('schema-catalog — option predicate guardrail (#1583)', () => { + it.each(allExamples().map((e) => [e.id, e] as const))( + '%s has no faulty option predicates', + (_id, example) => { + const issues = lintExample(example.schema); + expect(issues, JSON.stringify(issues, null, 2)).toEqual([]); + }, + ); + + it('actually exercises the cascading-options example (guardrail is not vacuous)', () => { + // The canonical cascade must be present AND clean — proves the linter runs on + // real per-option `visibleWhen` content, not just field-less schemas. + const cascade = allExamples().find((e) => e.id === 'fields-select/cascading-options'); + expect(cascade, 'fields-select/cascading-options example is missing').toBeTruthy(); + const groups = collectFieldGroups(cascade!.schema); + const hasOptionPredicate = groups.some((fields) => + fields.some((f) => f.options?.some((o) => o?.visibleWhen != null)), + ); + expect(hasOptionPredicate, 'expected the cascade example to carry option visibleWhen').toBe(true); + expect(lintExample(cascade!.schema)).toEqual([]); + }); +}); diff --git a/packages/core/src/evaluator/__tests__/optionLint.test.ts b/packages/core/src/evaluator/__tests__/optionLint.test.ts new file mode 100644 index 000000000..2360899cb --- /dev/null +++ b/packages/core/src/evaluator/__tests__/optionLint.test.ts @@ -0,0 +1,159 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Build-time guardrail for cascading option predicates (#1583, #2284). + */ +import { describe, it, expect } from 'vitest'; +import { lintOptionPredicates, type LintFieldLike } from '../optionLint'; + +/** country (cn/us) → province, the canonical cascade. */ +function cascade(provinceOptions: Array<{ value: string; visibleWhen?: string }>): LintFieldLike[] { + return [ + { + name: 'country', + type: 'select', + options: [ + { label: 'China', value: 'cn' }, + { label: 'United States', value: 'us' }, + ], + }, + { + name: 'province', + type: 'select', + dependsOn: 'country', + options: provinceOptions.map((o) => ({ label: o.value, value: o.value, visibleWhen: o.visibleWhen })), + }, + ]; +} + +describe('lintOptionPredicates — clean predicates', () => { + it('passes a correct country → province cascade', () => { + const issues = lintOptionPredicates( + cascade([ + { value: 'zj', visibleWhen: "record.country == 'cn'" }, + { value: 'ca', visibleWhen: "record.country == 'us'" }, + { value: 'other' }, // predicate-less → always offered, nothing to check + ]), + ); + expect(issues).toEqual([]); + }); + + it('passes an `in [...]` membership predicate whose literals are all in domain', () => { + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "record.country in ['cn', 'us']" }]), + ); + expect(issues).toEqual([]); + }); + + it('leaves role/context predicates (current_user.*) untouched', () => { + const issues = lintOptionPredicates([ + { + name: 'tier', + type: 'select', + options: [ + { label: 'Standard', value: 'standard' }, + { label: 'Admin only', value: 'admin_only', visibleWhen: "'admin' in current_user.roles" }, + ], + }, + ]); + expect(issues).toEqual([]); + }); + + it('skips comparisons against a non-enum sibling (open domain)', () => { + // `region` is free text — its domain is unknowable, so any literal is allowed. + const issues = lintOptionPredicates([ + { name: 'region', type: 'text' }, + { + name: 'city', + type: 'select', + dependsOn: 'region', + options: [{ label: 'SF', value: 'sf', visibleWhen: "record.region == 'anything-goes'" }], + }, + ]); + expect(issues).toEqual([]); + }); + + it('returns [] for empty / missing field lists', () => { + expect(lintOptionPredicates([])).toEqual([]); + expect(lintOptionPredicates(null)).toEqual([]); + expect(lintOptionPredicates(undefined)).toEqual([]); + }); +}); + +describe('lintOptionPredicates — the #2284 literal typo', () => { + it('flags a literal outside the controlling enum domain (country == \'chna\')', () => { + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "record.country == 'chna'" }]), + ); + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ + field: 'province', + option: 'zj', + code: 'option-literal-not-in-domain', + }); + expect(issues[0].message).toContain("'chna'"); + expect(issues[0].message).toContain("'cn'"); + }); + + it('flags the mirrored operand order (\'chna\' == record.country)', () => { + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "'chna' == record.country" }]), + ); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('option-literal-not-in-domain'); + }); + + it('flags a bad literal inside an `in [...]` list but not the good ones', () => { + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "record.country in ['cn', 'chna']" }]), + ); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain("'chna'"); + }); + + it('flags `!=` against an out-of-domain literal too', () => { + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "record.country != 'chna'" }]), + ); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('option-literal-not-in-domain'); + }); +}); + +describe('lintOptionPredicates — unknown field & syntax', () => { + it('flags a misspelled sibling field reference (record.contry)', () => { + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "record.contry == 'cn'" }]), + ); + // `contry` is unknown; the literal check is skipped (no such enum domain). + expect(issues.some((i) => i.code === 'unknown-field')).toBe(true); + expect(issues.every((i) => i.code !== 'option-literal-not-in-domain')).toBe(true); + }); + + it('flags a CEL syntax error and stops deeper checks for that predicate', () => { + // The classic `{ref}` brace mistake — `{…}` parses as a CEL map literal. + const issues = lintOptionPredicates( + cascade([{ value: 'zj', visibleWhen: "{record.country} == 'cn'" }]), + ); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('syntax'); + }); + + it('reports one issue per faulty option, keyed by field + option value', () => { + const issues = lintOptionPredicates( + cascade([ + { value: 'zj', visibleWhen: "record.country == 'chna'" }, + { value: 'ca', visibleWhen: "record.country == 'usa'" }, + { value: 'ok', visibleWhen: "record.country == 'cn'" }, + ]), + ); + expect(issues).toHaveLength(2); + expect(issues.map((i) => i.option).sort()).toEqual(['ca', 'zj']); + }); +}); diff --git a/packages/core/src/evaluator/index.ts b/packages/core/src/evaluator/index.ts index a93ff612d..5586399bc 100644 --- a/packages/core/src/evaluator/index.ts +++ b/packages/core/src/evaluator/index.ts @@ -10,6 +10,7 @@ export * from './ExpressionContext.js'; export * from './ExpressionEvaluator.js'; export * from './fieldRules.js'; export * from './optionRules.js'; +export * from './optionLint.js'; export * from './ExpressionCache.js'; export * from './FormulaFunctions.js'; export * from './SafeExpressionParser.js'; diff --git a/packages/core/src/evaluator/optionLint.ts b/packages/core/src/evaluator/optionLint.ts new file mode 100644 index 000000000..58e3babbf --- /dev/null +++ b/packages/core/src/evaluator/optionLint.ts @@ -0,0 +1,200 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Build-time / authoring guardrail for cascading `select` options (#1583, #2284). + * + * A per-option `visibleWhen` predicate (see {@link ./optionRules.js}) fails + * *closed*: a broken or wrong predicate makes its option silently never show. + * That is invisible at runtime — the offered set just quietly lacks a value — + * so the class of bug this catches is exactly the one runtime fail-open can't + * surface. The motivating case (#2284) is an AI-authored literal typo: + * + * { value: 'zj', visibleWhen: "record.country == 'chna'" } // 'chna' ≠ 'cn' + * + * `country`'s own options are `cn` / `us`, so `record.country` can never equal + * `'chna'` and Zhejiang is unreachable. The reference *parses*, the field + * *exists* — only the compared literal is outside the controlling field's value + * domain, which no syntax/existence check knows about. This linter closes that + * hole by cross-checking each option predicate against its sibling fields. + * + * Three checks, in order of confidence: + * 1. `syntax` — the predicate is valid CEL. Delegated to the canonical + * `@objectstack/formula` `validateExpression` (no schema + * hint → pure parse validation, so a legitimate + * `current_user.roles` reference is never flagged). + * 2. `unknown-field` — a `record.` reference names a field the form + * doesn't declare (a sibling typo, `record.contry`). + * 3. `option-literal-not-in-domain` — a `record. == ''` (or + * `!=` / `in [...]`) compares against a value outside that + * *enum* sibling's declared option set (the `'chna'` case). + * + * Deliberately **conservative**: it only flags what it can statically prove and + * never guesses. Anything it can't resolve — a non-`record.` root + * (`current_user.*`), a reference to a non-enum field (free text / number / + * lookup, whose domain is open), a comparison it doesn't recognise — is left + * alone. False positives would train authors to ignore it, so there are none by + * construction; it is a safety net, not a schema validator. + */ +import { validateExpression } from '@objectstack/formula'; +import type { FieldRulePredicate } from './fieldRules.js'; +import type { DependsOnInput, OptionLike } from './optionRules.js'; + +/** Field types whose value set is a closed, statically-known enum. */ +const OPTION_FIELD_TYPES = new Set(['select', 'radio', 'multiselect']); + +/** A lint finding on one option's `visibleWhen` predicate. */ +export interface OptionLintIssue { + /** Name of the `select`/`radio` field that owns the option (`''` if unnamed). */ + field: string; + /** `value` of the option whose predicate is faulty. */ + option: string; + /** Machine-readable finding kind. */ + code: 'syntax' | 'unknown-field' | 'option-literal-not-in-domain'; + /** Human-readable, self-correcting explanation. */ + message: string; +} + +/** + * Minimal field shape the linter reads — structurally satisfied by + * `FormFieldConfig`, `SelectFieldMetadata`, and a bare object-schema field. + * Only `name`, the type (`type` or `widget`), `options`, and `dependsOn` are + * consulted. + */ +export interface LintFieldLike { + name?: string; + type?: string; + widget?: string; + dependsOn?: DependsOnInput; + options?: readonly OptionLike[] | null; +} + +/** The literal source of a predicate (`string` or `{ source }` envelope). */ +function predicateSource(pred: FieldRulePredicate): string { + return typeof pred === 'string' ? pred : (pred?.source ?? ''); +} + +/** Resolved value type of a field, or `select`/`radio`/`multiselect` for enums. */ +function fieldType(f: LintFieldLike): string { + return (f.widget || f.type || '').toString(); +} + +/** All `record.` field references in a predicate source. */ +function recordRefs(source: string): string[] { + const refs: string[] = []; + const re = /record\.([A-Za-z_$][\w$]*)/g; + let m: RegExpExecArray | null; + while ((m = re.exec(source)) !== null) refs.push(m[1]); + return refs; +} + +/** + * Every `(field, literal)` pair where the predicate compares a `record.` + * to a string literal via `==`, `!=`, or `in [...]` — the forms whose literal + * can be domain-checked. Recognises both operand orders for equality. Anything + * else (ranges, function calls, arithmetic) yields nothing and is skipped. + */ +function literalComparisons(source: string): Array<{ field: string; literal: string }> { + const out: Array<{ field: string; literal: string }> = []; + + // record.field == 'lit' / record.field != 'lit' + const eqRight = /record\.([A-Za-z_$][\w$]*)\s*[=!]=\s*'([^']*)'/g; + // 'lit' == record.field / 'lit' != record.field + const eqLeft = /'([^']*)'\s*[=!]=\s*record\.([A-Za-z_$][\w$]*)/g; + // record.field in ['a', 'b', ...] + const inList = /record\.([A-Za-z_$][\w$]*)\s+in\s+\[([^\]]*)\]/g; + + let m: RegExpExecArray | null; + while ((m = eqRight.exec(source)) !== null) out.push({ field: m[1], literal: m[2] }); + while ((m = eqLeft.exec(source)) !== null) out.push({ field: m[2], literal: m[1] }); + while ((m = inList.exec(source)) !== null) { + const field = m[1]; + const listRe = /'([^']*)'/g; + let lm: RegExpExecArray | null; + while ((lm = listRe.exec(m[2])) !== null) out.push({ field, literal: lm[1] }); + } + return out; +} + +/** + * Statically lint the per-option `visibleWhen` predicates of a field list + * (a form's `fields`, or an object's fields) and return every provable issue. + * An empty array means nothing was disprovable — not that every predicate is + * semantically correct (see the conservative-by-design note above). + * + * @param fields Sibling fields sharing one record scope. Enum fields + * (`select`/`radio`/`multiselect`) contribute their option + * `value`s as the value domain for cross-checks. + */ +export function lintOptionPredicates(fields: readonly LintFieldLike[] | null | undefined): OptionLintIssue[] { + if (!fields || fields.length === 0) return []; + + const knownFields = new Set(); + // Value domain of each *enum* field; fields with an open domain are absent. + const domainByField = new Map>(); + for (const f of fields) { + if (!f?.name) continue; + knownFields.add(f.name); + if (OPTION_FIELD_TYPES.has(fieldType(f)) && f.options && f.options.length > 0) { + domainByField.set(f.name, new Set(f.options.map((o) => o.value))); + } + } + + const issues: OptionLintIssue[] = []; + for (const f of fields) { + if (!OPTION_FIELD_TYPES.has(fieldType(f)) || !f.options) continue; + const field = f.name ?? ''; + for (const opt of f.options) { + if (opt?.visibleWhen == null) continue; + const source = predicateSource(opt.visibleWhen); + if (!source.trim()) continue; + const option = opt.value; + + // 1. Syntax — canonical CEL parse (no schema hint = no scope false-positives). + const parsed = validateExpression('predicate', source); + if (!parsed.ok) { + issues.push({ + field, + option, + code: 'syntax', + message: parsed.errors[0]?.message ?? `invalid CEL predicate: ${source}`, + }); + // Extraction below trusts a parseable source; skip it for broken CEL. + continue; + } + + // 2. Unknown sibling field — a `record.` the form never declares. + for (const ref of recordRefs(source)) { + if (!knownFields.has(ref)) { + issues.push({ + field, + option, + code: 'unknown-field', + message: `option '${option}' visibleWhen references record.${ref}, which is not a field of this form`, + }); + } + } + + // 3. Literal outside the controlling enum's value domain (the #2284 case). + for (const { field: ref, literal } of literalComparisons(source)) { + const domain = domainByField.get(ref); + if (domain && !domain.has(literal)) { + issues.push({ + field, + option, + code: 'option-literal-not-in-domain', + message: + `option '${option}' visibleWhen compares record.${ref} to '${literal}', ` + + `which is not one of ${ref}'s option values (${[...domain].map((v) => `'${v}'`).join(', ')})`, + }); + } + } + } + } + return issues; +}