fix(ui-core): read a bare-string predicate as CEL shorthand, as the spec and server do

.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).
+
+<!-- adr-0087: not-required (no-migration-prescription) the narrowed spellings were never valid under the protocol: the spec defines a bare string as CEL and the server already evaluated it on CEL and rejected them, so no stored metadata the server accepted changes meaning and there is nothing for objectstack migrate meta to rewrite -->

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<Record<string, unknown>> = {
   // 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.

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

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<typeof console.warn>;
+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);
+  });
+});

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();

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<typeof vi.spyOn>;
   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();
   });
 });
 

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']);

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: '   ' });
   });
 

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) {