diff --git a/.changeset/27457-bare-string-predicate-cel.md b/.changeset/27457-bare-string-predicate-cel.md new file mode 100644 index 00000000000..b4d00387b12 --- /dev/null +++ b/.changeset/27457-bare-string-predicate-cel.md @@ -0,0 +1,19 @@ +--- +'@object-ui/core': minor +--- + +fix(core): a bare-string predicate is read as CEL, the way the spec defines it and the server reads it + +Clause-②: no (narrowing) +Metadata-protocol: narrowed + +`@objectstack/spec` defines a bare string in a predicate slot as shorthand for `{ dialect: 'cel', source }`, and the server evaluates it on CEL. The console read the shorthand on its legacy JavaScript expression grammar instead, so a predicate using CEL-only syntax faulted there: a navigation entry with `visible: "'org_owner' in current_user.positions"` was hidden from everyone (the navigation gate withholds an entry whose predicate faults), and a page block with `visibleWhen: "size(page.items) > 0"` never rendered, while the same source written as an envelope worked. + +- `ExpressionEvaluator.evaluateCondition` evaluates a bare string on the canonical `@objectstack/formula` CEL engine, the same route a `{ dialect: 'cel' }` envelope takes, with the same fault policy (`onFault`, `throwOnError`). +- `toPredicateInput` normalizes a bare string into `{ dialect: 'cel', source }`, as the spec's input schema does, instead of wrapping it as a `${…}` template. +- `evalRowPredicate` (list conditional formatting and row-action predicates) keeps only a `${…}` template on the legacy engine; any other bare string is evaluated on CEL. A string with legacy-only syntax still prints the one-time warning that prescribes the CEL spelling. +- Unchanged: a `${…}` template still evaluates on the legacy engine, and so does the `source` of an envelope whose dialect is not `cel`. + +**BREAKING**: a bare-string predicate is read on CEL only, as the spec defines it: a predicate that relied on syntax only the legacy JavaScript grammar reads (for example `===`, `.includes(…)`, `.length`, or a read of a `data.*` key the bound scope does not hold) now faults instead of evaluating, and each surface applies its existing fault policy (a navigation entry or a `visible` / `visibleWhen` gate is withheld, a `disabled` gate disables). The server already reads the shorthand as CEL and rejects those spellings, so the console was the only reader that accepted them; measured in this repository, 0 authored predicates outside tests used them. Write such a predicate in CEL, the one predicate language the spec and the server define (ADR-0032). + + diff --git a/ui/apps/console/src/__tests__/record-block-record-reach.test.tsx b/ui/apps/console/src/__tests__/record-block-record-reach.test.tsx index d03a7b45001..1bde6f11388 100644 --- a/ui/apps/console/src/__tests__/record-block-record-reach.test.tsx +++ b/ui/apps/console/src/__tests__/record-block-record-reach.test.tsx @@ -164,7 +164,7 @@ const PROBE_OBJECT_SCHEMA = { label: 'Probe Action', type: 'url', locations: ['record_header'], - visible: "record.stage === 'qualified'", + visible: "record.stage == 'qualified'", }, ], }; @@ -254,7 +254,7 @@ const SAMPLE_BY_INPUT: Readonly> = { // reach this block" is even askable — an alert with no predicate is // unconditionally visible and identical for every record, which would be the // probe answering its own question with a shrug. - visible: "record.stage === 'qualified'", + visible: "record.stage == 'qualified'", title: 'Probe Alert', body: 'Probe alert body', // `record:related_list.add` — spec-valid minimum, i.e. `picker.object` present. diff --git a/ui/packages/app-shell/src/providers/ExpressionProvider.tsx b/ui/packages/app-shell/src/providers/ExpressionProvider.tsx index c979c31ee65..f9795b8ccb4 100644 --- a/ui/packages/app-shell/src/providers/ExpressionProvider.tsx +++ b/ui/packages/app-shell/src/providers/ExpressionProvider.tsx @@ -551,7 +551,8 @@ function reportAppShellVisibleFault( * - `{ dialect: 'cel', source }` envelopes — the shape the spec's * `ExpressionInputSchema` normalizes every authored `visible` string into, * which is what nav/area items carry after the server serves the app schema - * - bare expression strings (evaluated as one boolean expression) + * - bare expression strings — CEL shorthand by the spec, evaluated on the + * same CEL engine as the envelope (objectstack#27457) * * Everything non-literal is delegated to `evaluator.evaluateCondition`, which * routes CEL envelopes to the canonical `@objectstack/formula` engine. The diff --git a/ui/packages/app-shell/src/providers/__tests__/navigationVisibility.bareCel-27457.test.ts b/ui/packages/app-shell/src/providers/__tests__/navigationVisibility.bareCel-27457.test.ts new file mode 100644 index 00000000000..c280a7d3005 --- /dev/null +++ b/ui/packages/app-shell/src/providers/__tests__/navigationVisibility.bareCel-27457.test.ts @@ -0,0 +1,81 @@ +/** + * 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. + */ + +/** + * objectstack#27457 — a navigation entry's `visible` written as a BARE string + * is CEL, read on the same engine as its `{ dialect: 'cel', source }` envelope. + * + * `@objectstack/spec` declares the nav `visible` slot as + * `EvaluatedExpressionInputSchema`, whose string arm is shorthand for the CEL + * envelope, and its own example is ``'org_admin' in current_user.positions``. + * The app metadata door serves the stored bare string as authored, and the + * console used to read it on the legacy JS grammar, where `in` is not an + * operator: the predicate faulted, and the navigation gate withholds on a fault + * (objectui#12700), so the entry was hidden from everyone, its holder included. + * + * The evaluators are built through the shell's own builder, so the bag is the + * one `ExpressionProvider` publishes. + */ + +import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest'; +import { __resetVisibilityPredicateWarnings } from '@object-ui/react'; +import { + createExpressionEvaluator, + evaluateNavigationVisibility, + evaluateVisibility, +} from '../ExpressionProvider'; + +const OWNER = createExpressionEvaluator({ user: { id: 'u1', positions: ['org_owner'] } }); +const MEMBER = createExpressionEvaluator({ user: { id: 'u2', positions: ['member'] } }); + +const SPEC_SHAPE = "'org_owner' in current_user.positions"; + +let warn: MockInstance; +beforeEach(() => { + __resetVisibilityPredicateWarnings(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => warn.mockRestore()); + +const appShellLines = () => + warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes('app-shell:visible')); + +describe('objectstack#27457 — a bare nav `visible` string is CEL', () => { + it('shows the entry to the position holder and withholds it from the member, with no fault', () => { + // The card's reproduction A. Before the fix BOTH answers were `false`: + // `Unexpected token "i"` on the legacy grammar, withheld, and reported. + expect(evaluateNavigationVisibility(SPEC_SHAPE, OWNER)).toBe(true); + expect(evaluateNavigationVisibility(SPEC_SHAPE, MEMBER)).toBe(false); + expect(appShellLines()).toHaveLength(0); + }); + + it('CONTROL: the envelope spelling reaches the same two verdicts', () => { + const env = { dialect: 'cel', source: SPEC_SHAPE }; + expect(evaluateNavigationVisibility(env, OWNER)).toBe(true); + expect(evaluateNavigationVisibility(env, MEMBER)).toBe(false); + }); + + it('the fail-open gate (`evaluateVisibility`) reads the bare string on CEL too', () => { + // Fail-open answers `true` on a fault, so the MEMBER's `false` is the proof + // that the predicate evaluated rather than faulted. + expect(evaluateVisibility(SPEC_SHAPE, OWNER)).toBe(true); + expect(evaluateVisibility(SPEC_SHAPE, MEMBER)).toBe(false); + expect(appShellLines()).toHaveLength(0); + }); + + it('a legacy-only bare spelling has no JS reading any more: it faults and is withheld, loudly', () => { + // `===` is not CEL, so the holder no longer sees an entry spelled that way; + // the report names the predicate. Its CEL spelling is the paired control. + expect(evaluateNavigationVisibility("current_user.id === 'u1'", OWNER)).toBe(false); + const lines = appShellLines(); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("current_user.id === 'u1'"); + expect(lines[0]).toMatch(/WITHHELD/); + expect(evaluateNavigationVisibility("current_user.id == 'u1'", OWNER)).toBe(true); + }); +}); diff --git a/ui/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx b/ui/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx index f8d664e0b52..b8462e36275 100644 --- a/ui/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx +++ b/ui/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx @@ -604,8 +604,10 @@ describe('DeclaredActionsBar — declared `visible` on a server-declared action // Phase 2 of the objectui#5330 canon: the row is bound as `record.*` only, // so `status` is an unknown variable. This leg opts into `throwOnError`, so // its EXISTING policy is fail-closed: the same verdict on the holding row - // and the failing one, and one console line naming the variable. The - // ungated companion proves the bar rendered. + // and the failing one, and one console line naming the predicate. The + // ungated companion proves the bar rendered. The bare string is CEL + // (objectstack#27457), so the line is the CEL fail-closed probe's, which + // names the predicate rather than the legacy grammar's `is not defined`. const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); try { const gated = { ...APPROVE, visible: 'status == "pending"' }; @@ -620,7 +622,7 @@ describe('DeclaredActionsBar — declared `visible` on a server-declared action .map((c) => String(c[0])) .filter((m) => m.includes('was hidden/disabled: its predicate threw')); expect(reports).toHaveLength(1); - expect(reports[0]).toContain('status is not defined'); + expect(reports[0]).toContain('CEL predicate failed to evaluate: status == "pending"'); expect(reports[0]).toContain('declared action "approval_approve" (visible)'); } finally { warn.mockRestore(); diff --git a/ui/packages/components/src/__tests__/page-header-predicate-dialect.test.tsx b/ui/packages/components/src/__tests__/page-header-predicate-dialect.test.tsx index 692dec3ee37..bf0b4d145e9 100644 --- a/ui/packages/components/src/__tests__/page-header-predicate-dialect.test.tsx +++ b/ui/packages/components/src/__tests__/page-header-predicate-dialect.test.tsx @@ -24,11 +24,13 @@ * Failed to evaluate expression 'record.f_date < today()': * "today" is not a function * - * These tests pin the three families from the issue plus the fallback that - * keeps the switch non-breaking: a legacy-dialect string (`${…}`, `===`, - * `.includes()`) still routes to the old evaluator with its deprecation + * These tests pin the three families from the issue plus the fallback: a + * `${…}` template still routes to the old evaluator with its deprecation * warning, and a predicate that genuinely cannot be evaluated still fails - * CLOSED, once, with the action named. + * CLOSED, once, with the action named. A BARE string with legacy-only syntax + * (`===`, `.includes()`) used to fall back too; it is CEL now, as the spec + * defines the shorthand (objectstack#27457), so it faults and fails closed — + * the deprecation warning still names the CEL spelling. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; @@ -247,7 +249,7 @@ describe('page:header — CEL-only constructs in action predicates (#3521)', () }); }); -describe('page:header — legacy dialect still falls back (#3521)', () => { +describe('page:header — only the `${…}` template falls back to the legacy dialect (#3521, objectstack#27457)', () => { let warn: ReturnType; beforeEach(() => { warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); @@ -271,20 +273,34 @@ describe('page:header — legacy dialect still falls back (#3521)', () => { expect(shown()).toBe(false); }); - it('evaluates a bare `===` predicate on the legacy engine', () => { + it('a bare `===` predicate is CEL (objectstack#27457): it faults and fails closed, and still warns with the CEL spelling', () => { renderHeader({ name: 'zoo_legacy_strict_eq', visible: 'record.f_status === "open"' }); - expect(shown()).toBe(true); + expect(shown()).toBe(false); expect(legacyWarnings().length).toBeGreaterThan(0); + cleanup(); + // CONTROL: the CEL spelling of the same predicate holds on the same record. + renderHeader({ name: 'zoo_cel_eq', visible: 'record.f_status == "open"' }); + expect(shown()).toBe(true); }); - it('evaluates a JS-only method (`.includes()`) on the legacy engine', () => { + it('a bare JS-only method (`.includes()`) is CEL too: no such method, so it faults and fails closed', () => { renderHeader({ name: 'zoo_legacy_includes', visible: 'record.f_textarea.includes("urgent")' }); + expect(shown()).toBe(false); + cleanup(); + // CONTROL: CEL's `contains` reads the same field and holds. + renderHeader({ name: 'zoo_cel_contains', visible: 'record.f_textarea.contains("urgent")' }); expect(shown()).toBe(true); }); - it('routes a legacy `disabled` predicate the same way', () => { - renderHeader({ name: 'zoo_legacy_disabled', disabled: 'record.f_status === "open"' }); + it('a bare legacy `disabled` predicate faults the same way, and the disabled gate fails closed (disabled)', () => { + renderHeader({ name: 'zoo_legacy_disabled', disabled: 'record.f_status === "closed"' }); + // `=== "closed"` is false on this record, so the legacy reading enabled the + // button; the CEL fault disables it. expect(button()).toBeDisabled(); + cleanup(); + // CONTROL: the CEL spelling evaluates to false and leaves the button enabled. + renderHeader({ name: 'zoo_cel_disabled', disabled: 'record.f_status == "closed"' }); + expect(button()).not.toBeDisabled(); }); }); diff --git a/ui/packages/core/src/actions/__tests__/ActionEngine.visibility.test.ts b/ui/packages/core/src/actions/__tests__/ActionEngine.visibility.test.ts index 522322daad2..69d309602bb 100644 --- a/ui/packages/core/src/actions/__tests__/ActionEngine.visibility.test.ts +++ b/ui/packages/core/src/actions/__tests__/ActionEngine.visibility.test.ts @@ -261,14 +261,36 @@ describe('ActionEngine.getActionsForLocation — visibility filter', () => { } }); - it('keeps the bare-string predicate on the legacy JS path (no regression)', () => { - // The same predicate text as a bare string is NOT a CEL envelope, so it - // stays on the legacy `${…}` path — `null < null` is `false` there, so - // the action shows. This is the assertion that makes the case above a - // real engine discriminator rather than a predicate that hides anyway. + it('routes the bare-string spelling to CEL too — it is the envelope\'s shorthand (objectstack#27457)', () => { + // The spec defines a bare string as `{ dialect: 'cel', source }` + // shorthand, so the same text as a bare string reaches the SAME verdict + // as its envelope above: the null comparison faults on CEL and the + // fail-closed gate hides. It used to stay on the legacy `${…}` path, + // where `null < null` is `false` and the action showed. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const engine = new ActionEngine(NULLS); + engine.registerAction( + { name: 'bare_nulls', type: 'api', visible: NULL_PRED } as any, + { locations: ['record_section'] }, + ); + expect(engine.getActionsForLocation('record_section')).toHaveLength(0); + const hits = warn.mock.calls.filter(c => String(c[0]).includes('bare_nulls')); + expect(hits).toHaveLength(1); + expect(String(hits[0][0])).toContain(NULL_PRED); + } finally { + warn.mockRestore(); + } + }); + + it('keeps a `${…}` template on the legacy JS path (the discriminator\'s control)', () => { + // The template spelling is not CEL shorthand and still reads on the + // legacy engine — `null < null` is `false` there, so the action shows. + // This is what makes the two cases above a real engine discriminator + // rather than a predicate that hides anyway. const engine = new ActionEngine(NULLS); engine.registerAction( - { name: 'js_nulls', type: 'api', visible: NULL_PRED } as any, + { name: 'js_nulls', type: 'api', visible: `\${${NULL_PRED}}` } as any, { locations: ['record_section'] }, ); expect(engine.getActionsForLocation('record_section').map(a => a.name)).toEqual(['js_nulls']); diff --git a/ui/packages/core/src/actions/__tests__/ActionRunner.disabledGate.test.ts b/ui/packages/core/src/actions/__tests__/ActionRunner.disabledGate.test.ts index 068d12d1eee..4a040e486f6 100644 --- a/ui/packages/core/src/actions/__tests__/ActionRunner.disabledGate.test.ts +++ b/ui/packages/core/src/actions/__tests__/ActionRunner.disabledGate.test.ts @@ -328,13 +328,14 @@ describe('why the gate cannot delegate "is there a condition?" to the verdict (o it('toPredicateInput collapses the EMPTY shapes and passes the BLANK ones through', () => { // Why the gate is not simply `toPredicateInput(x) !== undefined`: blank // predicate text survives normalization as an evaluable-looking value in both - // its spellings — a wrapped template for the bare string, an intact envelope - // for a blank `source` — so the shared definition names blankness explicitly + // its spellings — a `cel` envelope for the bare string (objectstack#27457: + // it is CEL shorthand), an intact envelope for a blank `source` — so the + // shared definition names blankness explicitly // (objectui#3960; the same blank rule `evalRowPredicate` applies in // `evaluator/listConditional.ts`). expect(toPredicateInput('')).toBeUndefined(); expect(toPredicateInput({ dialect: 'cel', source: '' })).toBeUndefined(); - expect(toPredicateInput(' ')).toBe('${ }'); + expect(toPredicateInput(' ')).toEqual({ dialect: 'cel', source: ' ' }); expect(toPredicateInput({ dialect: 'cel', source: ' ' })).toEqual({ dialect: 'cel', source: ' ' }); }); diff --git a/ui/packages/core/src/evaluator/ExpressionEvaluator.ts b/ui/packages/core/src/evaluator/ExpressionEvaluator.ts index a6aa6172090..89c84368af2 100644 --- a/ui/packages/core/src/evaluator/ExpressionEvaluator.ts +++ b/ui/packages/core/src/evaluator/ExpressionEvaluator.ts @@ -360,9 +360,9 @@ export class ExpressionEvaluator { // engine (the one `fieldRules` / list conditionals already use), NOT the legacy // JS evaluator below. This makes a component / action `visible` / `disabled` // predicate reach the SAME verdict as server enforcement — including CEL-only - // behavior like `record.due_date == today()` (framework#3205). Bare strings and - // `${…}` templates stay on the legacy path (back-compat deprecation window); - // only an explicit `{ dialect: 'cel' }` envelope is rerouted. + // behavior like `record.due_date == today()` (framework#3205). A BARE string + // takes the same route further down (objectstack#27457); only a `${…}` + // template, and an envelope of another dialect, stay on the legacy path. if ( condition && typeof condition === 'object' && (condition as { dialect?: string }).dialect === 'cel' @@ -371,9 +371,14 @@ export class ExpressionEvaluator { return this.evaluateCelCondition((condition as { source: string }).source, options); } - // Unwrap Expression envelope (see `evaluate` for rationale). + // Unwrap Expression envelope (see `evaluate` for rationale). Only an + // envelope of a dialect other than `cel` gets here (a `cel` one with a + // string `source` was routed above); its `source` keeps the legacy reading + // below, which is why the unwrap is remembered. + let unwrappedEnvelope = false; if (condition && typeof condition === 'object' && typeof (condition as any).source === 'string') { condition = (condition as any).source as string; + unwrappedEnvelope = true; } // A BLANK gate — `''`, whitespace-only text, or an envelope whose `source` @@ -413,6 +418,26 @@ export class ExpressionEvaluator { if (trimmed.includes('${')) { return Boolean(this.evaluate(trimmed, options)); } + + // objectstack#27457 — a BARE string is CEL. `@objectstack/spec` defines it + // as shorthand for `{ dialect: 'cel', source }` (the string arm of + // `ExpressionInputSchema` / `EvaluatedExpressionInputSchema`), and the + // server reads it that way, so the console reads it on the same engine + // (ADR-0032, one CEL language) instead of keeping a second grammar for the + // shorthand. Raw authored values reach this method unnormalized — a page + // node's `visibleWhen` from `SchemaRenderer`, a nav entry's `visible` from + // app-shell's `ExpressionProvider` — so the shorthand is resolved HERE, at + // the one evaluator every predicate reaches, and `toPredicateInput` emits + // the envelope for the callers that normalize first. Before, a CEL-only + // predicate (`'org_owner' in current_user.positions`, `size(page.items) > 0`) + // faulted on the legacy grammar and its gate failed closed for everyone. + if (!unwrappedEnvelope) { + return this.evaluateCelCondition(trimmed, options); + } + + // What is left is the `source` of an envelope whose dialect is not `cel` + // (a `template` one, or one naming no dialect at all): neither is a CEL + // predicate by the spec, so neither is moved by the line above. try { return Boolean(this.evaluateExpression(trimmed, { sanitize: options.sanitize !== false })); } catch (error) { diff --git a/ui/packages/core/src/evaluator/__tests__/declaredPredicate.test.ts b/ui/packages/core/src/evaluator/__tests__/declaredPredicate.test.ts index fe71d71dddd..490bfc707ce 100644 --- a/ui/packages/core/src/evaluator/__tests__/declaredPredicate.test.ts +++ b/ui/packages/core/src/evaluator/__tests__/declaredPredicate.test.ts @@ -153,7 +153,9 @@ describe('hasDeclaredPredicate is DERIVED from the normalizer, not re-spelled', // rule already applies on the VALUE side — `evaluateCondition` // (`if (!trimmed) return true`), `evaluateCelCondition` (`if (!source.trim())`) // and `evalRowPredicate` (`listConditional.ts`). - expect(toPredicateInput(' ')).toBe('${ }'); + // A bare string is CEL shorthand, so its blank becomes the blank `cel` + // envelope (objectstack#27457); a dialect-less envelope keeps the wrap. + expect(toPredicateInput(' ')).toEqual({ dialect: 'cel', source: ' ' }); expect(toPredicateInput({ dialect: 'cel', source: ' ' })).toEqual({ dialect: 'cel', source: ' ' }); expect(toPredicateInput({ source: ' ' })).toBe('${ }'); expect(toPredicateInput('')).toBeUndefined(); diff --git a/ui/packages/core/src/evaluator/__tests__/listConditional.test.ts b/ui/packages/core/src/evaluator/__tests__/listConditional.test.ts index 60186c55037..e7d15c4cb62 100644 --- a/ui/packages/core/src/evaluator/__tests__/listConditional.test.ts +++ b/ui/packages/core/src/evaluator/__tests__/listConditional.test.ts @@ -103,8 +103,18 @@ describe('evalRowPredicate', () => { expect(String(warn.mock.calls[0][0])).toContain('legacy expression dialect'); }); - it('routes a bare `===` string to the legacy engine', () => { - expect(evalRowPredicate("record.status === 'active'", { status: 'active' })).toBe(true); + it('reads a bare `===` string on CEL, where it faults — no legacy reading (objectstack#27457)', () => { + // A bare string is CEL shorthand, so legacy-only syntax faults on the CEL + // engine exactly as it does on the server: the verdict tracks the + // caller's fallback on a MATCHING row, which a real verdict would not. + expect(evalRowPredicate("record.status === 'active'", { status: 'active' })).toBe(false); + expect(evalRowPredicate("record.status === 'active'", { status: 'active' }, { fallback: true })).toBe(true); + // The CEL spelling of the same predicate holds on that row. + expect(evalRowPredicate("record.status == 'active'", { status: 'active' })).toBe(true); + // The deprecation warning still prescribes the CEL spelling, once. + const legacy = warn.mock.calls.filter(c => String(c[0]).includes('legacy expression dialect')); + expect(legacy).toHaveLength(1); + expect(String(legacy[0][0])).toContain('use "==" not "==="'); }); it('a legacy `${data.x}` / `${x}` string on a row surface retired with the CEL spellings (objectui#5741)', () => { @@ -186,9 +196,9 @@ describe('evalRowPredicate', () => { }); it('reports the legacy path fault with the legacy engine reason', () => { - // `===` routes to the legacy engine, which reports by throwing; the - // fail-closed report carries that message for the same reason. - evalRowPredicate('record.a === $$$', { a: 1 }, { + // A `${…}` template routes to the legacy engine, which reports by + // throwing; the fail-closed report carries that message for the same reason. + evalRowPredicate('${record.a === $$$}', { a: 1 }, { fallback: false, warnOnError: true, label: 'legacy-fault', @@ -201,6 +211,22 @@ describe('evalRowPredicate', () => { expect(failure).toContain('Reason: [legacy]'); }); + it('reports a bare `===` string as a CEL parse fault — it is CEL shorthand (objectstack#27457)', () => { + // The same text as a bare string no longer reaches the legacy engine: it + // faults on CEL, as on the server, and the report names the CEL reason. + evalRowPredicate('record.a === $$$', { a: 1 }, { + fallback: false, + warnOnError: true, + label: 'bare-fault', + }); + const failure = warn.mock.calls + .map((c) => String(c[0])) + .find((m) => m.includes('failed to evaluate') && m.includes('(bare-fault)')); + expect(failure).toBeDefined(); + expect(failure).toContain('Reason: [parse]'); + expect(failure).not.toContain('[legacy]'); + }); + // The other half of objectui#3792: this function stopped being list-only // long ago — kanban formatting, the bulk selection bar and (since #3521) // `page:header` all report through it. Naming every surface "list" sends a @@ -262,28 +288,30 @@ describe('evalRowPredicate — row wins over host scope (both dialect paths)', ( expect(evalRowPredicate("record.tag == 'ROW'", ROW, { scope: DECOY })).toBe(true); expect(evalRowPredicate("record.tag == 'SCOPE'", ROW, { scope: DECOY })).toBe(false); - // Legacy: `===` routes to the other engine — same subject, same verdicts. - expect(evalRowPredicate("record.tag === 'ROW'", ROW, { scope: DECOY })).toBe(true); - expect(evalRowPredicate("record.tag === 'SCOPE'", ROW, { scope: DECOY })).toBe(false); + // Legacy: a `${…}` template routes to the other engine — same subject, same + // verdicts. (A bare `===` string is CEL shorthand since objectstack#27457, + // so the template is the one spelling left on this path.) + expect(evalRowPredicate("${record.tag === 'ROW'}", ROW, { scope: DECOY })).toBe(true); + expect(evalRowPredicate("${record.tag === 'SCOPE'}", ROW, { scope: DECOY })).toBe(false); }); it('no longer binds `data` or the bare name to the row — on either path (objectui#5741)', () => { // `data` is the HOST's own now: the decoy answers, on both dialect paths. expect(evalRowPredicate("data.tag == 'SCOPE'", ROW, { scope: DECOY })).toBe(true); - expect(evalRowPredicate("data.tag === 'SCOPE'", ROW, { scope: DECOY })).toBe(true); + expect(evalRowPredicate("${data.tag === 'SCOPE'}", ROW, { scope: DECOY })).toBe(true); expect(evalRowPredicate("data.tag == 'ROW'", ROW, { scope: DECOY })).toBe(false); - expect(evalRowPredicate("data.tag === 'ROW'", ROW, { scope: DECOY })).toBe(false); + expect(evalRowPredicate("${data.tag === 'ROW'}", ROW, { scope: DECOY })).toBe(false); // The bare name is bound nowhere: it faults into the fallback on both paths. expect(evalRowPredicate("tag == 'ROW'", ROW, { scope: DECOY })).toBe(false); - expect(evalRowPredicate("tag === 'ROW'", ROW, { scope: DECOY })).toBe(false); + expect(evalRowPredicate("${tag === 'ROW'}", ROW, { scope: DECOY })).toBe(false); expect(evalRowPredicate("tag == 'ROW'", ROW, { scope: DECOY, fallback: true })).toBe(true); - expect(evalRowPredicate("tag === 'ROW'", ROW, { scope: DECOY, fallback: true })).toBe(true); + expect(evalRowPredicate("${tag === 'ROW'}", ROW, { scope: DECOY, fallback: true })).toBe(true); }); it('still resolves host-scope keys the row does not shadow', () => { // The row winning is not the scope being dropped: background stays bound. expect(evalRowPredicate('features.on == true', ROW, { scope: DECOY })).toBe(true); - expect(evalRowPredicate('features.on === true', ROW, { scope: DECOY })).toBe(true); + expect(evalRowPredicate('${features.on === true}', ROW, { scope: DECOY })).toBe(true); }); it('holds on the fail-closed route as well as the fast route', () => { @@ -499,8 +527,8 @@ describe('evalRowPredicate — rowless (objectui#4640)', () => { expect(evalRowPredicate("record.tag == 'HOST'", null, { scope: HOST, rowless: true })).toBe(true); expect(evalRowPredicate("data.tag == 'HOST'", null, { scope: HOST, rowless: true })).toBe(true); // …on the legacy path too — one answer per value, not one per dialect. - expect(evalRowPredicate("record.tag === 'HOST'", null, { scope: HOST, rowless: true })).toBe(true); - expect(evalRowPredicate("data.tag === 'HOST'", null, { scope: HOST, rowless: true })).toBe(true); + expect(evalRowPredicate("${record.tag === 'HOST'}", null, { scope: HOST, rowless: true })).toBe(true); + expect(evalRowPredicate("${data.tag === 'HOST'}", null, { scope: HOST, rowless: true })).toBe(true); }); it('is what stops those predicates from faulting into the fallback', () => { diff --git a/ui/packages/core/src/evaluator/__tests__/predicateInput.test.ts b/ui/packages/core/src/evaluator/__tests__/predicateInput.test.ts index e527b1bb471..e2e056b1138 100644 --- a/ui/packages/core/src/evaluator/__tests__/predicateInput.test.ts +++ b/ui/packages/core/src/evaluator/__tests__/predicateInput.test.ts @@ -57,9 +57,15 @@ const CONTEXT = { user: { role: 'admin' }, record: { status: 'active' } }; const TEMPLATE_TRUE = '${user.role === "admin"}'; const TEMPLATE_FALSE = '${user.role === "guest"}'; -describe('toPredicateInput — a bare expression is wrapped (unchanged)', () => { - it('wraps a bare expression as a `${…}` template', () => { - expect(toPredicateInput('data.age >= 18')).toBe('${data.age >= 18}'); +describe('toPredicateInput — a bare expression is CEL shorthand (objectstack#27457)', () => { + it('normalizes a bare expression into the `cel` envelope, as the spec\'s input schema does', () => { + // `@objectstack/spec`'s `ExpressionInputSchema` transforms a bare string + // into `{ dialect: 'cel', source }`; the console's normalizer emits the + // same value, so the predicate reaches the CEL engine the server uses. + // It used to be wrapped as `${data.age >= 18}`, the legacy JS spelling. + expect(toPredicateInput('data.age >= 18')).toEqual({ dialect: 'cel', source: 'data.age >= 18' }); + expect(toPredicateInput("'org_owner' in current_user.positions")) + .toEqual({ dialect: 'cel', source: "'org_owner' in current_user.positions" }); }); it('wraps a bare `source` out of a non-cel envelope', () => { @@ -79,7 +85,9 @@ describe('toPredicateInput — a bare expression is wrapped (unchanged)', () => expect(toPredicateInput(null)).toBeUndefined(); expect(toPredicateInput(undefined)).toBeUndefined(); expect(toPredicateInput({ dialect: 'cel', source: '' })).toBeUndefined(); - expect(toPredicateInput(' ')).toBe('${ }'); + // A blank bare string is the blank `cel` envelope the evaluator diagnoses + // as "no gate" (ADR-0137 D4), the same value its envelope spelling keeps. + expect(toPredicateInput(' ')).toEqual({ dialect: 'cel', source: ' ' }); // objectui#11358: a present value with no evaluable `source` is no longer // folded into `undefined` (absent). It is kept as a `cel` envelope with no // `source` — declared, and a fault to the evaluator — and normalizing that @@ -101,8 +109,10 @@ describe('toPredicateInput — an already-`${…}` string is already normalized // surface calls it on a value it did not produce, and `${…}` is one of the // shapes it produces, so a non-idempotent normalizer corrupts its own // output the moment two call sites are chained. + // A bare string normalizes to a `cel` envelope, which the envelope branch + // re-emits as an equal (fresh) envelope. const once = toPredicateInput('data.age >= 18'); - expect(toPredicateInput(once)).toBe(once); + expect(toPredicateInput(once)).toEqual(once); expect(toPredicateInput(toPredicateInput(TEMPLATE_TRUE))).toBe(TEMPLATE_TRUE); }); diff --git a/ui/packages/core/src/evaluator/listConditional.ts b/ui/packages/core/src/evaluator/listConditional.ts index 11e00e88f35..411c25caf5f 100644 --- a/ui/packages/core/src/evaluator/listConditional.ts +++ b/ui/packages/core/src/evaluator/listConditional.ts @@ -28,11 +28,14 @@ * * **Dialect routing.** A predicate authored as the `{ dialect: 'cel', source }` * envelope is authoritative CEL and is *always* sent to the CEL engine. A bare - * string is CEL per the spec contract, *unless* it carries legacy-only syntax - * (`${…}` templating, `===`/`!==`, `?.` JS optional-chaining, JS string/array - * methods) — those are routed to the legacy engine for back-compat and emit a - * one-time deprecation warning, because such a string means something different - * (or nothing) to the server's CEL engine. + * string is CEL per the spec contract and is sent there too — including one + * that carries legacy-only syntax (`===`/`!==`, `?.` JS optional-chaining, JS + * string/array methods), which faults on CEL exactly as it does on the server + * (objectstack#27457: a bare string has no legacy reading). Only a `${…}` + * template still reads on the legacy engine. Every string carrying legacy-only + * syntax also emits a one-time deprecation warning that prescribes the CEL + * spelling, because such a string means something different (or nothing) to + * the server's CEL engine. */ import { evalFieldPredicate, type FieldRulePredicate } from './fieldRules.js'; import { ExpressionEvaluator } from './ExpressionEvaluator.js'; @@ -306,10 +309,13 @@ export function evalRowPredicate( ? (pred as { source: string }).source : '(expression)'); - // Legacy-dialect *strings* route to the legacy engine (back-compat). The + // Legacy-dialect *strings* are warned about; only a `${…}` TEMPLATE routes to + // the legacy engine (back-compat). Any other bare string is CEL shorthand + // (objectstack#27457) and continues to the CEL path below, where legacy-only + // syntax faults into the fallback with the engine's own report. The // `{ dialect: 'cel', source }` envelope is never routed here — it is CEL. - if (source !== undefined && isLegacyDialectSource(source)) { - warnLegacyDialect(source); + if (source !== undefined && isLegacyDialectSource(source)) warnLegacyDialect(source); + if (source !== undefined && source.includes('${')) { try { const evaluator = new ExpressionEvaluator(scope); return evaluator.evaluateCondition(source, { throwOnError: true }); diff --git a/ui/packages/core/src/evaluator/predicateInput.ts b/ui/packages/core/src/evaluator/predicateInput.ts index 1ba52d1fa37..d67b8571a13 100644 --- a/ui/packages/core/src/evaluator/predicateInput.ts +++ b/ui/packages/core/src/evaluator/predicateInput.ts @@ -72,8 +72,9 @@ const DECLARED_UNEVALUABLE: { readonly dialect: 'cel'; readonly source?: undefin /** * Wrap a BARE expression as a `${…}` template, and leave a string that already * carries template syntax alone (objectui#3871 — rationale in - * {@link toPredicateInput}'s doc below). One rule, so the two string spellings - * inside the normalizer cannot drift apart. + * {@link toPredicateInput}'s doc below). Used for the `source` of an envelope + * whose dialect is not `cel` only: a bare STRING is CEL shorthand and becomes + * a `cel` envelope instead (objectstack#27457). */ function wrapIfBare(expression: string): string { return expression.includes('${') ? expression : `\${${expression}}`; @@ -86,9 +87,11 @@ function wrapIfBare(expression: string): string { * * Accepts: * - `boolean` → returned as-is (predicate evaluation short-circuits). - * - `string` → a BARE expression is wrapped as `${string}` (legacy DX - * shorthand); a string that is already a `${…}` template is ALREADY in the - * normalized shape and is returned untouched (objectui#3871, below). + * - `string` → a BARE expression becomes `{ dialect: 'cel', source }`, + * exactly as `@objectstack/spec`'s `ExpressionInputSchema` normalizes it + * on the server (objectstack#27457, below); a string that is already a + * `${…}` template is ALREADY in the normalized shape and is returned + * untouched (objectui#3871, below). * - `Expression` envelope `{ dialect, source }` (the normalized form * `@objectstack/spec`'s `ExpressionInputSchema` produces for every * authored predicate, including bare strings) → a `cel` dialect keeps @@ -124,6 +127,20 @@ function wrapIfBare(expression: string): string { * normalization parity table, which is exactly the arrangement the paragraph * above describes the failure mode of). * + * ## Why a bare string is a `cel` envelope, not a `${…}` template (objectstack#27457) + * + * The spec defines a bare string as shorthand for `{ dialect: 'cel', source }` + * — the string arm of `ExpressionInputSchema` transforms it into exactly that, + * and so does every evaluated slot's `EvaluatedExpressionInputSchema` — and the + * server evaluates it on CEL. This normalizer is the console's copy of that + * same input step, so it emits the same envelope (ADR-0032: one CEL language, + * no second grammar for the shorthand). It used to wrap the string as + * `${string}`, which sent a CEL-only predicate (`'org_owner' in + * current_user.positions`, `size(page.items) > 0`) to the legacy JS evaluator, + * where it faulted — and a gate that faults closed was hidden from everyone. + * `evaluateCondition` reads a raw bare string the same way, for the callers + * that hand it the authored value without normalizing first. + * * ## Why an already-`${…}` string is NOT wrapped again (objectui#3871) * * `${…}` is one of the spellings this repo documents for a predicate @@ -179,7 +196,9 @@ function wrapIfBare(expression: string): string { export function toPredicateInput(value: unknown): EvaluatorPredicateInput { if (value === null || value === undefined || value === '') return undefined; if (typeof value === 'boolean') return value; - if (typeof value === 'string') return wrapIfBare(value); + if (typeof value === 'string') { + return value.includes('${') ? value : { dialect: 'cel', source: value }; + } // Declared, but nothing here can be evaluated (objectui#11358): kept as its // own state rather than folded into "absent". See `DECLARED_UNEVALUABLE`. if (isUnevaluablePredicate(value)) return DECLARED_UNEVALUABLE; diff --git a/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.loadingFrameDiagnostic.test.tsx b/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.loadingFrameDiagnostic.test.tsx index 08fae63b857..198453fcf02 100644 --- a/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.loadingFrameDiagnostic.test.tsx +++ b/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.loadingFrameDiagnostic.test.tsx @@ -104,11 +104,17 @@ function schema(visible: unknown) { * locale-map test for the real provider), and coupling this pin to that * unrelated noise would make it fail for a reason that has nothing to do * with objectui#5776. + * + * Two diagnostics count: the legacy engine's `Failed to evaluate expression` + * and the CEL leg's `A conditional predicate failed to evaluate`. A bare + * predicate is CEL (objectstack#27457), so the second is the one this card's + * bare shape reaches now; matching only the first would let the loading-frame + * silence below pass for a leg that never logs that text. */ function evaluationFailureWarnings(spy: { mock: { calls: unknown[][] } }): string[] { return spy.mock.calls .map((call: unknown[]) => String(call[0])) - .filter((m: string) => /Failed to evaluate expression/.test(m)); + .filter((m: string) => /Failed to evaluate expression|predicate failed to evaluate/.test(m)); } describe('record:alert — loading-frame evaluation diagnostic (objectui#5776)', () => { @@ -155,6 +161,10 @@ describe('record:alert — loading-frame evaluation diagnostic (objectui#5776)', const BROKEN = "bogus_unbound_field.status == 'x'"; render(); - expect(evaluationFailureWarnings(warnSpy).length).toBeGreaterThan(0); + const lines = evaluationFailureWarnings(warnSpy); + expect(lines.length).toBeGreaterThan(0); + // The bare predicate is CEL (objectstack#27457): the CEL engine names the + // unbound root, which is the line the author needs. + expect(lines.some((l) => l.includes('Unknown variable: bogus_unbound_field'))).toBe(true); }); }); diff --git a/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.rowBinding.test.tsx b/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.rowBinding.test.tsx index 932feb1770c..fab7589d28d 100644 --- a/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.rowBinding.test.tsx +++ b/ui/packages/plugin-detail/src/renderers/__tests__/record-alert.rowBinding.test.tsx @@ -28,12 +28,13 @@ * every row — the same verdict on both rows, i.e. the gate is not consulted. * This is the pre-#4807 signature by design: it is the ruled cost of the * retirement, and the console line is the notice. - * • legacy `data.*` does not throw: the ambient scope app-shell mounts - * (`providers/ExpressionProvider.tsx`) puts `data: {}` in the bag, and that - * `data` is the HOST's own, left standing, so `data.status` reads an - * undefined VALUE on the wrong object and the comparison is a constant - * `false` — the banner is OFF on every row, silently. Same retirement, - * opposite polarity, and no fault to report because nothing faulted. + * • legacy `data.*` reads the HOST's own `data: {}` (the ambient scope + * app-shell mounts, `providers/ExpressionProvider.tsx`), left standing. + * A bare predicate is CEL (objectstack#27457), and on CEL a key the map + * does not hold is a fault (`No such key: status`), not an undefined + * value — so this fail-soft site answers SHOWN on every row, like the + * shorthand, and the fault is reported. (It used to read on the legacy + * grammar, where the comparison was a silent constant `false`.) * * ── Why groups B and C mount different shapes ────────────────────────────── * @@ -72,7 +73,7 @@ */ import * as React from 'react'; -import { describe, it, expect, afterEach } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import { render, cleanup, type RenderResult } from '@testing-library/react'; import { RecordContextProvider, SchemaRenderer, PredicateScopeProvider } from '@object-ui/react'; import '../../index'; @@ -185,14 +186,23 @@ describe('#4807 group B — only the canon binds on this renderer own gate (obje }); }); - it('legacy `data.*` — retired by objectui#5741: OFF on both rows (the host `data: {}` answers)', () => { - // No throw: the ambient `data: {}` from app-shell is the host's own and is - // left standing, so the comparison is constantly false and the banner never - // appears — on either row, and with nothing faulting to report. - expect(verdicts(alertNode(LEGACY, NODE_GATE_OPEN))).toEqual({ - onMatchingRow: false, - onOtherRow: false, - }); + it('legacy `data.*` — retired by objectui#5741: a CEL fault on the host `data: {}`, SHOWN on both rows and reported', () => { + // The ambient `data: {}` from app-shell is the host's own and is left + // standing. The bare predicate is CEL shorthand (objectstack#27457), and + // CEL refuses a key the map does not hold, so the predicate faults on + // either row: this fail-soft site answers SHOWN, and says why. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect(verdicts(alertNode(LEGACY, NODE_GATE_OPEN))).toEqual({ + onMatchingRow: true, + onOtherRow: true, + }); + const faults = warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes(LEGACY)); + expect(faults.length).toBeGreaterThan(0); + expect(faults[0]).toContain('No such key: status'); + } finally { + warn.mockRestore(); + } }); it('the row wins over an ambient `record`; a host `data` is left standing (objectui#5741)', () => { diff --git a/ui/packages/react/src/SchemaRenderer.tsx b/ui/packages/react/src/SchemaRenderer.tsx index 121459ac802..2d7c8b72a8c 100644 --- a/ui/packages/react/src/SchemaRenderer.tsx +++ b/ui/packages/react/src/SchemaRenderer.tsx @@ -513,7 +513,10 @@ const isCelEnvelope = (value: unknown): boolean => * run BEFORE the hoist, so by the time either consumer sees the key the * envelope is gone. * - * Both consumers then read a bare string and both take the legacy JS path: + * Both consumers then read a bare string and both took the legacy JS path + * (a bare string is CEL shorthand since objectstack#27457, so a stripped `cel` + * envelope would no longer change engines; the guard stays as the spelling + * the spec normalizes to): * * 1. this component's own `shouldHide` chain, off the post-hoist node; and * 2. the RENDERER's `toPredicateInput(…)` → `useCondition(…)` call, one @@ -1359,9 +1362,10 @@ export const SchemaRenderer: ForwardRefExoticComponent< * ## A fault fails CLOSED (objectui#13299) * * `evaluateCondition` answers an unresolvable predicate with `true`, on - * every one of its three internal paths: the CEL envelope fails soft to its - * `true` fallback, a bare expression that throws is caught and returns - * `true`, and a `${…}` template that throws returns its own SOURCE TEXT, + * every one of its three internal paths: the CEL envelope (and a bare + * string, its shorthand since objectstack#27457) fails soft to its `true` + * fallback, a non-`cel` envelope's `source` that throws is caught and + * returns `true`, and a `${…}` template that throws returns its own SOURCE TEXT, * which is a non-empty string and therefore truthy. Passing * `throwOnError: true` changes none of those verdicts — it only converts * "could not evaluate" from a value into a throw. diff --git a/ui/packages/react/src/__tests__/SchemaRenderer.bareCelVisibleWhen-27457.test.tsx b/ui/packages/react/src/__tests__/SchemaRenderer.bareCelVisibleWhen-27457.test.tsx new file mode 100644 index 00000000000..adfc45e7150 --- /dev/null +++ b/ui/packages/react/src/__tests__/SchemaRenderer.bareCelVisibleWhen-27457.test.tsx @@ -0,0 +1,120 @@ +/** + * 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. + */ + +/** + * objectstack#27457 — a page component's `visibleWhen` written as a BARE + * string is CEL, read on the same engine as its `{ dialect: 'cel', source }` + * envelope. + * + * `@objectstack/spec` defines a bare string as shorthand for the CEL envelope + * (the string arm of `EvaluatedExpressionInputSchema`), and the server reads it + * that way. The console used to send the bare spelling to the legacy JS + * evaluator, where CEL-only syntax (`size()`, `in`) faulted — and a node gate + * that faults is withheld (objectui#13299), so the block vanished for everyone + * while its envelope twin rendered. + * + * Every hidden verdict below sits beside a mount of the SAME schema that + * renders, differing only in the value under test, so "hidden" is the gate's + * answer and not "this node never renders". + */ + +import { describe, it, expect, beforeEach, afterEach, vi, type MockInstance } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer } from '../SchemaRenderer'; +import { __resetVisibilityPredicateWarnings, UNRESOLVABLE_VISIBILITY_PREFIX } from '../utils/visibilityDiagnostic'; +import { PredicateScopeProvider } from '../hooks/useExpression'; +import { PageVariablesProvider } from '../hooks/usePageVariables'; + +const NAME = 'probe-27457'; +const TYPE = 'element:probe-27457'; + +const Probe = () =>
; + +const OWNER = { id: 'u1', positions: ['org_owner'] }; +const MEMBER = { id: 'u2', positions: ['member'] }; +const scopeFor = (user: Record) => ({ + current_user: user, + user, + features: {}, +}); + +function mount(visibleWhen: unknown, items: unknown[], user: Record = OWNER) { + return render( + + + + + , + ); +} + +const shown = () => screen.queryByTestId('probe') !== null; + +let warn: MockInstance; +beforeEach(() => { + ComponentRegistry.register(NAME, Probe as never, { namespace: 'element', skipFallback: true } as never); + __resetVisibilityPredicateWarnings(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + ComponentRegistry.unregister?.(NAME, 'element'); + vi.restoreAllMocks(); +}); + +const visibilityLines = () => + warn.mock.calls.map((c) => String(c[0])).filter((l) => l.startsWith(UNRESOLVABLE_VISIBILITY_PREFIX)); + +describe('objectstack#27457 — a bare `visibleWhen` string is CEL', () => { + it('`size(page.items) > 0` (a CEL-only builtin) reaches opposite verdicts on the two polarities', () => { + // The card's reproduction B. Before the fix the non-empty mount was hidden + // too: `"size" is not a function` on the legacy grammar, withheld. + mount('size(page.items) > 0', ['a']); + expect(shown()).toBe(true); + cleanup(); + mount('size(page.items) > 0', []); + expect(shown()).toBe(false); + expect(visibilityLines()).toHaveLength(0); + }); + + it("`'org_owner' in current_user.positions` (CEL `in`) shows the holder and hides the member", () => { + // The spec's own example spelling of a role predicate (`in` over a list). + mount("'org_owner' in current_user.positions", ['a'], OWNER); + expect(shown()).toBe(true); + cleanup(); + mount("'org_owner' in current_user.positions", ['a'], MEMBER); + expect(shown()).toBe(false); + expect(visibilityLines()).toHaveLength(0); + }); + + it('CONTROL: the envelope spelling of the same predicate reaches the same verdicts', () => { + const env = { dialect: 'cel', source: 'size(page.items) > 0' }; + mount(env, ['a']); + expect(shown()).toBe(true); + cleanup(); + mount(env, []); + expect(shown()).toBe(false); + }); + + it('a legacy-only bare spelling is no longer read on the JS grammar: it faults, is withheld and is reported', () => { + // `===` is not CEL. The bare string has one reading now, the server's, so + // this predicate faults on BOTH polarities (withheld, objectui#13299) and + // names itself in the report — where it used to evaluate on the legacy + // engine. Its CEL spelling, `==`, is the paired control. + mount("page.items[0] === 'a'", ['a']); + expect(shown()).toBe(false); + const lines = visibilityLines(); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("page.items[0] === 'a'"); + cleanup(); + mount("page.items[0] == 'a'", ['a']); + expect(shown()).toBe(true); + }); +}); diff --git a/ui/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx b/ui/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx index 61b37c80f9e..84ff4e13ce3 100644 --- a/ui/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx +++ b/ui/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx @@ -551,20 +551,24 @@ describe('#6445 group 3 — nothing else became loud', () => { }); }); - it('SUPERSEDED by objectui#6504: an adapter-only `data.*` `disabled` predicate now reports, on the enablement leg — not this fault one', async () => { - // Was pinned SILENT until the 2026-08-27 ruling (option A) extended - // objectui#5687's diagnostic to this gate. This predicate never FAULTS — - // it evaluates perfectly, against the wrong object — so it must never - // appear on THIS file's `reports()` (the #6445 fault leg); see group 6 - // below for the positive coverage of the new leg. + it('objectstack#27457: a BARE adapter-only `data.*` `disabled` predicate is CEL — it FAULTS, disables, and reports on THIS fault leg', async () => { + // Was pinned to objectui#6504's adapter-only leg while a bare string read + // on the legacy grammar, where `undefined == 'locked'` is a clean `false`. + // A bare string is CEL shorthand now, and CEL refuses a key the bound + // `data` does not hold, so the predicate FAULTS: the gate's fail-soft + // answer disables the control, and this file's fault line names the + // engine's reason. The adapter-only leg (group 6, written as the `${…}` + // template that still evaluates cleanly) does not fire for it. await inDevelopment((mount) => { const warn = spyWarn(); mount([{ id: 'n1', disabled: "data.status == 'locked'" }]); - expect(disabledProp()).toBe('absent'); - expect(reports(warn)).toHaveLength(0); + expect(disabledProp()).toBe('true'); + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("data.status == 'locked'"); + expect(lines[0]).toContain('No such key: status'); expect(visibilityReports(warn)).toHaveLength(0); - // It DOES now report — on its own leg and its own prefix. - expect(adapterOnlyReports(warn)).toHaveLength(1); + expect(adapterOnlyReports(warn)).toHaveLength(0); }); }); }); @@ -574,13 +578,18 @@ describe('#6445 group 3 — nothing else became loud', () => { * constant-predicate diagnostic. A DIFFERENT fault class than groups 0-5 * above: nothing here FAULTS (the predicate evaluates perfectly, against the * wrong object), so none of it belongs on `reports()` / `UNRESOLVABLE_ENABLEMENT_PREFIX`. + * + * Every `data.*` predicate here is written as the `${…}` template: since + * objectstack#27457 a BARE string is CEL, which refuses an unanswered key + * instead of reading `undefined`, so the bare spelling faults and never reaches + * this leg (group 3's last case pins where it goes instead). * -------------------------------------------------------------------------- */ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`', () => { it('THE acceptance criterion: the card\'s repro shape reports, dev-only, and the verdict is untouched (constant-FALSE direction)', async () => { await inDevelopment((mount) => { const warn = spyWarn(); - mount([{ id: 'n1', disabled: "data.status == 'locked'" }]); + mount([{ id: 'n1', disabled: "${data.status == 'locked'}" }]); // Verdict UNCHANGED — `undefined == 'locked'` is `false`, so the control // is NOT disabled here. This diagnostic never moves a verdict. expect(disabledProp()).toBe('absent'); @@ -605,7 +614,7 @@ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`' // EVERY build, with nothing on the console. await inDevelopment((mount) => { const warn = spyWarn(); - mount([{ id: 'n1', disabled: 'data.locked == null' }]); + mount([{ id: 'n1', disabled: '${data.locked == null}' }]); expect(disabledProp()).toBe('true'); expect(adapterOnlyReports(warn)).toHaveLength(1); }); @@ -617,7 +626,7 @@ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`' // sentence for THIS gate is about a constant `true` greying it out. await inDevelopment((mount) => { const warn = spyWarn(); - mount([{ id: 'n1', disabled: 'data.locked == null' }]); + mount([{ id: 'n1', disabled: '${data.locked == null}' }]); const line = adapterOnlyReports(warn)[0]; expect(line).toContain('constant `true`'); expect(line).toContain('DISABLED'); @@ -639,8 +648,8 @@ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`' await inDevelopment((mount) => { const warn = spyWarn(); mount([ - { id: 'nA', visibleWhen: "data.status == 'locked'" }, - { id: 'nB', disabled: "data.status == 'locked'" }, + { id: 'nA', visibleWhen: "${data.status == 'locked'}" }, + { id: 'nB', disabled: "${data.status == 'locked'}" }, ]); const visLine = adapterOnlyVisibilityReports(warn); const enLine = adapterOnlyReports(warn); @@ -682,7 +691,7 @@ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`' it('⛔ C excluded: a PRODUCTION build reaches the SAME verdict and stays silent — this leg is dev-only, exactly as its sibling is', async () => { await inProduction((mount) => { const warn = spyWarn(); - mount([{ id: 'n1', disabled: 'data.locked == null' }]); + mount([{ id: 'n1', disabled: '${data.locked == null}' }]); // Same verdict as the dev case above — this diagnostic never moves one. expect(disabledProp()).toBe('true'); expect(adapterOnlyReports(warn)).toHaveLength(0); @@ -696,8 +705,8 @@ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`' await inDevelopment((mount) => { const warn = spyWarn(); mount([ - { id: 'n1', disabled: "data.status == 'locked'" }, - { id: 'n2', disabled: "data.status == 'locked'" }, + { id: 'n1', disabled: "${data.status == 'locked'}" }, + { id: 'n2', disabled: "${data.status == 'locked'}" }, ]); expect(adapterOnlyReports(warn)).toHaveLength(1); }); @@ -711,8 +720,8 @@ describe('#6504 group 6 — the adapter-only diagnostic, extended to `disabled`' await inDevelopment((mount) => { const warn = spyWarn(); mount([ - { id: 'n1', disabled: "data.status == 'locked'" }, - { id: 'n2', visibleWhen: "data.status == 'locked'" }, + { id: 'n1', disabled: "${data.status == 'locked'}" }, + { id: 'n2', visibleWhen: "${data.status == 'locked'}" }, ]); expect(adapterOnlyReports(warn)).toHaveLength(1); expect(adapterOnlyVisibilityReports(warn)).toHaveLength(1); diff --git a/ui/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx b/ui/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx index 825ff14a3c9..f1d3188a0f8 100644 --- a/ui/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx +++ b/ui/packages/react/src/__tests__/SchemaRenderer.nodeGateDataPredicate.test.tsx @@ -66,6 +66,17 @@ * defined` and objectui#5454's reporter takes it. That leg, and the verdict * move it carries, is pinned in `SchemaRenderer.dataRootUnbound-9308.test.tsx`. * + * ## objectstack#27457 — the BARE spelling left this leg for the fault leg + * + * A bare string is CEL shorthand by the spec, so it is read on the CEL engine + * now, which refuses an unanswered key (`No such key`) instead of reading it + * as `undefined`. A bare `data.*` predicate the bound `data` cannot answer is + * therefore a FAULT — reported by objectui#5454's leg and withheld, exactly as + * its `{ dialect: 'cel' }` envelope twin always was — and never reaches this + * leg. The cases that pin THIS leg's line are written as the `${…}` template, + * the one spelling that still evaluates cleanly against the wrong object; the + * bare cases pin where the bare spelling went. + * * ## objectui#5756 — the KNOWN LIMIT below is now FIXED, not pinned as a gap * * The 2026-08-22 "no interpolation changes" ruling quoted above was **this @@ -199,24 +210,29 @@ afterEach(() => { }); describe('#5687 group 1 — the card\'s reproduction shape is reported', () => { - it('THE acceptance criterion: `properties: { visible: "data.status == \'draft\'" }` warns, and still hides on EVERY row', () => { + it('THE acceptance criterion, as CEL now: `properties: { visible: "data.status == \'draft\'" }` is a FAULT, reported, and hides on EVERY row (objectstack#27457)', () => { const warn = spyWarn(); // The exact shape the card names. `properties.visible` is hoisted onto the - // node by the memo and lands on the `visible` leg of the chain. + // node by the memo and lands on the `visible` leg of the chain. A bare + // string is CEL shorthand, and CEL refuses a key the bound `data` does not + // hold (`No such key`) rather than reading it as `undefined` — so this is + // the objectui#5454 fault leg now, the leg its envelope twin always took, + // and the predicate is withheld (objectui#13299). The verdict the author + // sees is the same hidden block; the line that explains it is the fault one. mount({ properties: { visible: "data.status == 'draft'" } }, DRAFT); - // Verdict UNCHANGED — the ruling refuses a verdict change, and this is the - // constant-false the author would otherwise have shipped. expect(shown()).toBe(false); - const msg = reports(warn)[0]; + const msg = unresolvableReports(warn)[0]; expect(msg).toBeDefined(); expect(msg).toContain(TYPE); expect(msg).toContain('visible'); expect(msg).toContain("data.status == 'draft'"); - // The read that could not be answered, named — not just "something is wrong". - expect(msg).toContain('data.status'); - // The two sentences that make it actionable rather than merely noted. - expect(msg).toContain('CONSTANT'); - expect(msg).toContain('record.status'); + // Named as a CEL fault: the bare string was evaluated on the CEL engine + // and faulted there, not on the legacy grammar (which would have read + // `undefined` and evaluated cleanly). + expect(msg).toContain('Reason: CEL predicate failed to evaluate'); + // …and this leg's adapter-only line does not ALSO fire: it is reached only + // by a predicate that evaluated cleanly, which this one did not. + expect(reports(warn)).toHaveLength(0); }); it('… and hides on the OTHER row too, which is what "on every row" means', () => { @@ -228,7 +244,7 @@ describe('#5687 group 1 — the card\'s reproduction shape is reported', () => { cleanup(); mount({ properties: { visible: "data.status == 'draft'" } }, PUBLISHED); expect(shown()).toBe(false); - expect(reports(warn).length).toBeGreaterThan(0); + expect(unresolvableReports(warn).length).toBeGreaterThan(0); }); it('the paired control: the same node with NO gate renders on both rows', () => { @@ -290,28 +306,47 @@ describe('#5687 group 1 — the card\'s reproduction shape is reported', () => { }); it('a node-level `visibleWhen` written the deprecated way reports on ITS key', () => { + // The `${…}` template is the spelling that still evaluates cleanly against + // the wrong object, so it is the one this leg reports; the bare string is + // CEL and reports on the fault leg, under the same key. const warn = spyWarn(); - mount({ visibleWhen: "data.status == 'draft'" }, DRAFT); + mount({ visibleWhen: "${data.status == 'draft'}" }, DRAFT); expect(shown()).toBe(false); expect(reports(warn)[0]).toContain('visibleWhen'); + cleanup(); + mount({ visibleWhen: "data.status == 'draft'" }, DRAFT); + expect(shown()).toBe(false); + expect(unresolvableReports(warn)[0]).toContain('visibleWhen'); }); it('the NON-negated `hidden` leg reports too, and keeps its own inverted answer', () => { // `hidden` is not negated: a constant-`false` there means the node stays // SHOWN by a gate that was meant to hide it. Equally constant, equally // silent before this change — and the verdict is untouched here as well. + // Written as the `${…}` template, the spelling that still reads `undefined` + // instead of faulting (a bare string is CEL, objectstack#27457). const warn = spyWarn(); - mount({ hidden: "data.status == 'draft'" }, DRAFT); + mount({ hidden: "${data.status == 'draft'}" }, DRAFT); expect(shown()).toBe(true); expect(reports(warn)[0]).toContain('hidden'); }); + it('the bare `hidden` spelling is CEL: the unanswered read FAULTS, and a faulting `hidden` conceals (objectstack#27457)', () => { + const warn = spyWarn(); + mount({ hidden: "data.status == 'draft'" }, DRAFT); + expect(shown()).toBe(false); + const msg = unresolvableReports(warn)[0]; + expect(msg).toContain('hidden'); + expect(msg).toContain('Reason: CEL predicate failed to evaluate'); + expect(reports(warn)).toHaveLength(0); + }); + it('deduped: one line per (node type, key, predicate), not one per render', () => { // Matches objectui#5454's posture exactly — same Set, same key shape. const warn = spyWarn(); - mount({ properties: { visible: "data.status == 'draft'" } }, DRAFT); + mount({ visibleWhen: "${data.status == 'draft'}" }, DRAFT); cleanup(); - mount({ properties: { visible: "data.status == 'draft'" } }, PUBLISHED); + mount({ visibleWhen: "${data.status == 'draft'}" }, PUBLISHED); expect(reports(warn)).toHaveLength(1); }); }); diff --git a/ui/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx b/ui/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx index 1543cf3284e..4cf528dc20f 100644 --- a/ui/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx +++ b/ui/packages/react/src/__tests__/SchemaRenderer.productionPredicateFaultWarning.test.tsx @@ -101,6 +101,8 @@ const ROW = { id: 'r1', status: 'open' }; const FAULT_BARE = 'nosuchroot.x > 1'; const FAULT_BARE_2 = 'anotherbadroot.y == 3'; const FAULT_TEMPLATE = '${nosuchroot.x > 1}'; +/** Group 4's own template source, so no earlier cell's dedupe can reach it. */ +const FAULT_TEMPLATE_PARITY = '${parityroot27457.x > 1}'; const FAULT_CEL = { dialect: 'cel', source: 'record.bad(' }; /** A predicate that resolves cleanly — the silence that makes the noise mean something. */ const HEALTHY = "record.status == 'open'"; @@ -370,15 +372,32 @@ describe('#6038 group 3 — nothing else became loud', () => { // evaluated perfectly against the wrong object — not a fault — and its own // ruling (2026-08-22, option A) scoped it to development. Pinned here so // "production is louder" cannot quietly become "production reports - // everything the dev build reports". + // everything the dev build reports". Written as the `${…}` template: that + // is the spelling that still evaluates cleanly against the wrong object (a + // bare string is CEL since objectstack#27457 — see the next case). await inProduction((mount) => { const warn = spyWarn(); - mount([{ id: 'n1', visibleWhen: "data.status == 'draft'" }]); + mount([{ id: 'n1', visibleWhen: "${data.status == 'draft'}" }]); // Verdict: the constant-false still hides, exactly as documented. expect(shownCount()).toBe(0); expect(allWarnings(warn)).toHaveLength(0); }); }); + + it('the BARE spelling of that predicate is CEL: the unanswered read is a fault, reported in production (objectstack#27457)', async () => { + // CEL refuses a key the bound `data` does not hold, so this is no longer the + // dev-only adapter leg's case but a fault — the production line this card + // made loud, naming the engine's reason — and the node is withheld. + await inProduction((mount) => { + const warn = spyWarn(); + mount([{ id: 'n1', visibleWhen: "data.status == 'draft'" }]); + expect(shownCount()).toBe(0); + const lines = reports(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("data.status == 'draft'"); + expect(lines[0]).toContain('No such key: status'); + }); + }); }); /* -------------------------------------------------------------------------- * @@ -391,6 +410,14 @@ describe('#6038 group 4 — dev and production print the IDENTICAL line', () => // decides HOW the fault is detected, not WHETHER it is reported" a fact // rather than a comment: a production-only message would be a second // diagnostic to keep in sync, and they would drift. + // + // Measured on the `${…}` template, the dialect whose dev (`throwOnError`) + // and production (`onFault`) channels carry the same reason. A bare string + // used to be measured here; it is CEL shorthand since objectstack#27457, + // and the CEL route's fail-closed probe throws a generic sentence naming + // the predicate where production names the engine's reason — the + // `{ dialect: 'cel' }` envelope's long-standing split, recorded in the + // PR's acceptance notes rather than changed by it. const { __resetVisibilityPredicateWarnings } = await import('../utils/visibilityDiagnostic'); const { SchemaRenderer } = await import('../SchemaRenderer'); const { SchemaRendererContext } = await import('../context/SchemaRendererContext'); @@ -408,7 +435,7 @@ describe('#6038 group 4 — dev and production print the IDENTICAL line', () => - + , @@ -416,7 +443,7 @@ describe('#6038 group 4 — dev and production print the IDENTICAL line', () => const devLines = reports(warn); expect(devLines).toHaveLength(1); expect(devLines[0]).toContain(TYPE); - expect(devLines[0]).toContain(FAULT_BARE); + expect(devLines[0]).toContain(FAULT_TEMPLATE_PARITY); // Dev fails closed too (objectui#13299) — the verdict is one behaviour, // not two. expect(shownCount()).toBe(0); @@ -433,7 +460,7 @@ describe('#6038 group 4 — dev and production print the IDENTICAL line', () => let prodLine = ''; await inProduction((mount) => { const prodWarn = spyWarn(); - mount([{ id: 'n1', visibleWhen: FAULT_BARE }]); + mount([{ id: 'n1', visibleWhen: FAULT_TEMPLATE_PARITY }]); const lines = reports(prodWarn); expect(lines).toHaveLength(1); prodLine = lines[0]; diff --git a/ui/packages/react/src/hooks/__tests__/actionPredicate.parity.test.tsx b/ui/packages/react/src/hooks/__tests__/actionPredicate.parity.test.tsx index 1228b405a89..08400c7b0b1 100644 --- a/ui/packages/react/src/hooks/__tests__/actionPredicate.parity.test.tsx +++ b/ui/packages/react/src/hooks/__tests__/actionPredicate.parity.test.tsx @@ -114,10 +114,17 @@ describe('action `visible` — engine path vs renderer path parity (#3314)', () visible: { dialect: 'cel', source: '!(record.a < record.b)' }, expected: false, }, - // Same text, no envelope → legacy JS on BOTH paths → shown on both. + // Same text, no envelope → still CEL on BOTH paths: a bare string is the + // envelope's shorthand (objectstack#27457) → hidden on both. { - what: 'bare string with the same null comparison → shown on both paths', + what: 'bare string with the same null comparison → CEL shorthand, hidden on both paths', visible: '!(record.a < record.b)', + expected: false, + }, + // The legacy JS reading survives only as the `${…}` template → shown on both. + { + what: '`${…}` template with the same null comparison → legacy JS, shown on both paths', + visible: '${!(record.a < record.b)}', expected: true, }, { @@ -191,8 +198,9 @@ describe('action `visible` — engine path vs renderer path parity (#3314)', () * falsy verdict. objectui#3957 moved the engine onto the same definition. * * Why `rendererVerdict` alone cannot pin this: it models only the second question. - * For `visible: ' '` it returns `false` (the normalizer wraps a blank string into - * `'${ }'`, whose verdict is falsy) even though the real renderer SHOWS the + * For `visible: ' '` it used to return `false` (the normalizer wrapped a blank + * string into `'${ }'`, whose verdict is falsy; since objectstack#27457 it is + * the blank `cel` envelope, "no gate") even though the real renderer SHOWS the * action, because the declared gate in front of it never consults that verdict. * `rendererFace` below composes both questions, exactly as `action-button` / * `action-menu` / `DeclaredActionsBar` do. diff --git a/ui/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx b/ui/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx index 35ceea69e6c..d62f2893bee 100644 --- a/ui/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx +++ b/ui/packages/react/src/hooks/__tests__/useCondition.canonSpelling.test.tsx @@ -21,7 +21,9 @@ * policy, nothing new: the bag binds `record` and nothing else, so a bare field * or `data.*` — and a legacy `${data.x}` / `${x}` string, one bag for both * dialects — is an unknown variable. The throwing leg (`throwOnError`) hides and - * reports once; the non-throwing leg fails soft to `true`. Both legs are pinned + * reports once; the non-throwing leg fails soft to `true`. A bare string is CEL + * (objectstack#27457), so its report names the CEL fault and the source rather + * than the legacy engine's `X is not defined`. Both legs are pinned * on BOTH a matching and a non-matching row, because "the same verdict on both" * is what "no longer bound" looks like from outside. */ @@ -69,12 +71,14 @@ describe('[#5741] useCondition tier — the row is bound as `record.*` only', () }); describe.each([ - ['bare shorthand', "status == 'in_review'", 'status'], - ['`data.*`', "data.status == 'in_review'", 'data'], - ['legacy `${data.x}`', '${data.status === "in_review"}', 'data'], - ['legacy `${x}`', '${status === "in_review"}', 'status'], - ])('a %s predicate no longer discriminates', (_what, pred, variable) => { - it('throwing leg (`throwOnError`): hidden on BOTH rows, reported once, naming the variable', () => { + // A bare string is CEL (objectstack#27457): the throw names the CEL fault and the source. + ['bare shorthand', "status == 'in_review'", "CEL predicate failed to evaluate: status == 'in_review'"], + ['`data.*`', "data.status == 'in_review'", "CEL predicate failed to evaluate: data.status == 'in_review'"], + // A `${…}` template is still the legacy engine, which names the variable. + ['legacy `${data.x}`', '${data.status === "in_review"}', 'data is not defined'], + ['legacy `${x}`', '${status === "in_review"}', 'status is not defined'], + ])('a %s predicate no longer discriminates', (_what, pred, reason) => { + it('throwing leg (`throwOnError`): hidden on BOTH rows, reported once, naming the fault', () => { // The label carries the predicate so the hook's warn-once key (label, // source) is unique per case — the registry is module-global. const label = `action "approve" (visible) [${pred}]`; @@ -82,7 +86,7 @@ describe('[#5741] useCondition tier — the row is bound as `record.*` only', () expect(evaluate(pred, other, { throwOnError: true, label })).toBe(false); const reports = messages().filter((m) => m.includes('was hidden/disabled: its predicate threw')); expect(reports).toHaveLength(1); - expect(reports[0]).toContain(`${variable} is not defined`); + expect(reports[0]).toContain(reason); expect(reports[0]).toContain(label); }); @@ -98,23 +102,29 @@ describe('[#5741] useCondition tier — the row is bound as `record.*` only', () }); }); - it("a host's own ambient `data` is left standing: `data.*` reads the HOST object — constant, and silent", () => { + it("a host's own ambient `data` is left standing: `data.*` reads the HOST object — constant on both rows", () => { // app-shell's `ExpressionProvider` mounts `data: {}`; a record page under it - // hands this tier exactly this bag. Not a fault — `data.status` is an - // undefined VALUE on the host's object — so no leg reports it, and the row - // never enters the verdict. Pinned so the constant-false is a measured - // consequence of the ruling, not a surprise. + // hands this tier exactly this bag, and the row never enters the verdict. + // A bare string is CEL (objectstack#27457), and CEL refuses a key the host's + // `data` does not hold rather than reading `undefined`: so this is a FAULT + // now, taking each leg's existing policy — fail-soft `true` on the + // non-throwing leg, hidden and reported once on the throwing leg. Still + // constant across rows, which is what the ruling measured. const wrapper = ({ children }: { children: ReactNode }) => createElement(PredicateScopeProvider, { scope: { data: {} }, children }); const under = (pred: string, record: unknown, options?: Options) => renderHook(() => useCondition(pred as never, usePredicateRecordContext(record), options), { wrapper }) .result.current; - expect(under("data.status == 'in_review'", row)).toBe(false); - expect(under("data.status == 'in_review'", other)).toBe(false); + expect(under("data.status == 'in_review'", row)).toBe(true); + expect(under("data.status == 'in_review'", other)).toBe(true); expect(under("data.status == 'in_review'", row, { throwOnError: true })).toBe(false); + expect(under("data.status == 'in_review'", other, { throwOnError: true })).toBe(false); // …while the row still reaches `record`, which this bag pins over the host. expect(under("record.status == 'in_review'", row)).toBe(true); - expect(messages()).toHaveLength(0); + expect(under("record.status == 'in_review'", other)).toBe(false); + const reports = messages().filter((m) => m.includes('its predicate threw')); + expect(reports).toHaveLength(1); + expect(reports[0]).toContain("CEL predicate failed to evaluate: data.status == 'in_review'"); }); it('binds NOTHING when there is no row (a non-row `useCondition` call)', () => { diff --git a/ui/packages/react/src/hooks/__tests__/useExpression.test.ts b/ui/packages/react/src/hooks/__tests__/useExpression.test.ts index cdd4e2ffa0c..bc911ab15b7 100644 --- a/ui/packages/react/src/hooks/__tests__/useExpression.test.ts +++ b/ui/packages/react/src/hooks/__tests__/useExpression.test.ts @@ -240,8 +240,10 @@ describe('toPredicateInput — CEL envelope preservation (#2661)', () => { .toEqual({ dialect: 'cel', source: 'record.x == 1' }); }); - it('still wraps bare strings and non-cel envelopes as legacy ${…}', () => { - expect(toPredicateInput('data.age >= 18')).toBe('${data.age >= 18}'); + it('normalizes a bare string into the `cel` envelope (objectstack#27457), and still wraps a non-cel envelope as legacy ${…}', () => { + // A bare string is CEL shorthand by the spec, so it reaches the CEL engine + // the server uses; only an envelope of another dialect keeps the wrap. + expect(toPredicateInput('data.age >= 18')).toEqual({ dialect: 'cel', source: 'data.age >= 18' }); expect(toPredicateInput({ dialect: 'template', source: 'data.age >= 18' })).toBe('${data.age >= 18}'); });