fix(app-shell): bind the declared predicate roots on both field-visibility evaluators

.changeset/6493-bind-field-visibility-evaluators.md

@@ -0,0 +1,49 @@
+---
+'@object-ui/app-shell': minor
+---
+
+⚠️ **Behaviour change: `current_user` and `features` gates on an object field's
+`visible` that have been doing nothing on the record form page and in the
+console's record modal now TAKE EFFECT.** Read this before upgrading if any of
+your object metadata gates a field on the session user or on a deployment flag.
+
+objectui#6010 and objectui#6110 bound the host predicate scope on the form
+renderer and on the console form routes. `evaluateVisibility` was still being
+reached with a THIRD and FOURTH evaluator that neither of those touched:
+`RecordFormPage` and `AppContent` each built a private
+`new ExpressionEvaluator({ user, app, data })` for the field-visibility filter,
+beside — not from — the `ExpressionProvider` each of them mounts. Those bags
+bound `user`, but not the canonical `current_user` nor the ADR-0068 `ctx.user` /
+`os.user` spellings of that same object, and not `features` at all. So one
+authored predicate meant two different things depending on which evaluator
+reached it: `current_user` resolved on a nav item and was unbound on a field.
+Both sites now build their scope with the same `buildExpressionScope` the
+provider uses, which is the only declaration of what an app-shell predicate may
+name.
+
+**Why nobody noticed, and why the fix is felt as a change.** A field `visible`
+predicate fails OPEN: a field on screen is what you get when the predicate says
+TRUE, when the root was never bound so the predicate faulted, *and* when the
+predicate has a typo. Those worlds are indistinguishable, so an app that
+authored a `current_user` gate saw the field render and had no way to tell the
+rule was inert. After this change the predicate is evaluated for real, and
+**fields that have always been visible will disappear for the users the rule
+excludes** — and a `features` gate whose flag is off will hide its field once
+`/api/v1/auth/config` resolves.
+
+`AppContent`'s bag also hand-rolled its user as `{ name, email, role }`, without
+`positions`. It now uses the same `buildExpressionUser` normaliser every other
+console surface publishes, so `'sales' in current_user.positions` — the gate the
+server enforces on write — reaches the same verdict client-side instead of
+faulting open.
+
+**Before upgrading**, audit any `visible` predicate in your object metadata that
+names `current_user` (or `user` / `ctx.user` / `os.user`) or `features`, and
+confirm each says what you actually want evaluated. Measured on the metadata
+shipped in this repo and in the framework at the time of the change: **nothing
+in it authors such a gate**, so no shipped surface changes behaviour today —
+the audit is for your own object metadata, which this cannot see.
+
+**The error path is deliberately unchanged.** A predicate that throws still
+fails open, exactly as objectui#6443 / objectui#6487 left it. This change is
+about which roots are BOUND, not about what happens when evaluation fails.

packages/app-shell/src/console/AppContent.tsx

@@ -22,14 +22,17 @@ import { useMetadata } from '../providers/MetadataProvider.js';
 import { useAdapter } from '../providers/AdapterProvider.js';
 import { usePreviewDrafts } from '../preview/PreviewModeContext.js';
 import { PreviewDraftEmptyState } from '../preview/PreviewDraftEmptyState.js';
-import { ExpressionProvider, evaluateVisibility } from '../providers/ExpressionProvider.js';
+import {
+ ExpressionProvider,
+ createExpressionEvaluator,
+ evaluateVisibility,
+} from '../providers/ExpressionProvider.js';
 import { useTrackRouteAsRecent } from '../hooks/useTrackRouteAsRecent.js';
 import { resolveRecordFormTarget, resolveFormViewLayout, resolveNavigateCreateUrl, resolveNavigateEditUrl, resolvePostCreateTarget } from '../utils/recordFormNavigation.js';
 import { deriveRecordSurface, deriveRecordFlowSurface } from '@object-ui/plugin-view';
 import { RECORD_FORM_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_LINK_PARAM } from '../urlParams.js';
 import { matchAppBySegment } from '../utils/appRoute.js';
 import { resolveHref, type NavTemplateContext } from '@object-ui/layout';
-import { ExpressionEvaluator } from '@object-ui/core';
 
 // Components (eagerly loaded — always needed)
 import { ConsoleLayout } from '../layout/ConsoleLayout.js';
@@ -668,13 +671,25 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
  navigate(`/apps/${newAppName}`);
  };
 
+ // Evaluator for the ModalForm's field-visibility gates below, over the SAME
+ // bag the `ExpressionProvider` this component mounts publishes — one builder,
+ // called with the same inputs (objectui#6493). This one sits ABOVE that
+ // provider in its own tree, so it cannot read it back through the hook; what
+ // it can do is stop hand-writing a second, narrower bag.
+ //
+ // Two roots the private bag dropped, both of which fail OPEN when named:
+ // `current_user` (and the `ctx.user` / `os.user` spellings of the same
+ // object) and `features`. Its `user` was hand-rolled too, without
+ // `positions` — so `'sales' in current_user.positions`, the gate the server
+ // enforces on write, faulted here rather than hiding the field.
  const expressionEvaluator = useMemo(
- () => new ExpressionEvaluator({
- user: user ? { name: user.name, email: user.email, role: user.role ?? 'user' } : {},
+ () => createExpressionEvaluator({
+ user: buildExpressionUser(user),
  app: activeApp || {},
  data: editingRecord || {},
+ features,
  }),
- [user, activeApp, editingRecord],
+ [user, activeApp, editingRecord, features],
  );
 
  // objectui#5619 — `isWorkspaceAdminResolved` belongs in this readiness gate

packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts

@@ -0,0 +1,154 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#6493 — the app-shell predicate scope is built ONCE, and the two
+ * imperative evaluators bind it too.
+ *
+ * `evaluateVisibility` was reached with three different evaluators. Only
+ * `ExpressionProvider`'s carried the full bag; `RecordFormPage` and
+ * `AppContent` each built a private `new ExpressionEvaluator({ user, app,
+ * data })` for the SAME kind of gate — an object field's `visible` — and those
+ * bags named neither `current_user` (nor its `ctx.user` / `os.user` spellings)
+ * nor `features`.
+ *
+ * ## Why the old shape could not be caught by rendering alone
+ *
+ * A CEL predicate over an unbound root does not throw here: `evaluateCelCondition`
+ * fails SOFT to `true` when the caller has not asked for `throwOnError`, and
+ * `evaluateVisibility` is such a caller. So the field rendered — exactly as it
+ * would for a predicate that legitimately said yes, and exactly as it would for
+ * a predicate with a typo. The three worlds are indistinguishable on screen,
+ * which is why the fail-open direction is asserted below as its own case rather
+ * than assumed: `OLD_BAG` reproduces the pre-fix bag literally, and every
+ * assertion against it is the RED this change turns green.
+ *
+ * ADR-0068 D1 is the rule being conformed to — one user object under four
+ * spellings, so "a predicate `'org_admin' in current_user.roles` evaluates
+ * identically in a formula, an RLS policy, and a client `visible` gate".
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync } from 'node:fs';
+import { ExpressionEvaluator } from '@object-ui/core';
+import {
+ buildExpressionScope,
+ createExpressionEvaluator,
+ evaluateVisibility,
+} from './ExpressionProvider';
+
+/**
+ * The served shape. `ExpressionInputSchema` normalises every authored `visible`
+ * string into a `{ dialect, source }` envelope, so this — not a bare string —
+ * is what actually reaches the evaluator once the server has served the schema.
+ */
+const POSITION_GATE = { dialect: 'cel', source: "'sales_manager' in current_user.positions" };
+const CTX_ALIAS_GATE = { dialect: 'cel', source: "'sales_manager' in ctx.user.positions" };
+const OS_ALIAS_GATE = { dialect: 'cel', source: "'sales_manager' in os.user.positions" };
+const FEATURE_GATE = { dialect: 'cel', source: 'features.multiOrgEnabled == true' };
+
+const MANAGER = { name: 'Ada', email: 'ada@example.com', role: 'user', positions: ['sales_manager'] };
+const CLERK = { name: 'Bo', email: 'bo@example.com', role: 'user', positions: ['sales_clerk'] };
+
+/** The bag both ad-hoc sites hand-wrote before this change, reproduced verbatim. */
+const oldBag = (user: Record<string, unknown>) =>
+ new ExpressionEvaluator({ user, app: { name: 'crm' }, data: {} });
+
+describe('objectui#6493 — buildExpressionScope binds one user object under all four spellings', () => {
+ it('current_user / user / ctx.user / os.user are the SAME object, not four copies', () => {
+ const user = { name: 'Ada', positions: ['sales_manager'] };
+ const scope = buildExpressionScope({ user });
+
+ expect(scope.current_user).toBe(user);
+ expect(scope.user).toBe(user);
+ expect(scope.ctx.user).toBe(user);
+ expect(scope.os.user).toBe(user);
+ });
+
+ it('binds app, data and features, and defaults every root to an empty object', () => {
+ const scope = buildExpressionScope();
+ expect(scope).toStrictEqual({
+ current_user: {}, user: {}, ctx: { user: {} }, os: { user: {} }, app: {}, data: {}, features: {},
+ });
+ // The identity above holds for the defaults too — the hand-written fallback
+ // in `useExpressionContext` used to mint three separate empty objects.
+ expect(scope.current_user).toBe(scope.user);
+ expect(scope.ctx.user).toBe(scope.user);
+ expect(scope.os.user).toBe(scope.user);
+ });
+});
+
+describe('objectui#6493 — a current_user gate BITES through the shared scope', () => {
+ it('hides the field from a user the rule excludes', () => {
+ expect(evaluateVisibility(POSITION_GATE, createExpressionEvaluator({ user: CLERK }))).toBe(false);
+ });
+
+ it('shows the field to a user the rule admits', () => {
+ expect(evaluateVisibility(POSITION_GATE, createExpressionEvaluator({ user: MANAGER }))).toBe(true);
+ });
+
+ it('reaches the same verdict through the ctx.user and os.user spellings', () => {
+ const clerk = createExpressionEvaluator({ user: CLERK });
+ const manager = createExpressionEvaluator({ user: MANAGER });
+
+ expect(evaluateVisibility(CTX_ALIAS_GATE, clerk)).toBe(false);
+ expect(evaluateVisibility(OS_ALIAS_GATE, clerk)).toBe(false);
+ expect(evaluateVisibility(CTX_ALIAS_GATE, manager)).toBe(true);
+ expect(evaluateVisibility(OS_ALIAS_GATE, manager)).toBe(true);
+ });
+
+ it('binds features, so a deployment flag can hide a field', () => {
+ expect(evaluateVisibility(FEATURE_GATE, createExpressionEvaluator({ features: { multiOrgEnabled: false } }))).toBe(false);
+ expect(evaluateVisibility(FEATURE_GATE, createExpressionEvaluator({ features: { multiOrgEnabled: true } }))).toBe(true);
+ });
+});
+
+describe('objectui#6493 — the bag this change replaced failed OPEN on every one of those roots', () => {
+ it('showed the excluded user the field: an unbound current_user faults, and a fault reads as YES', () => {
+ // The whole defect in one line. Same predicate, same user, same
+ // `evaluateVisibility` — and the opposite answer from the one above.
+ expect(evaluateVisibility(POSITION_GATE, oldBag(CLERK))).toBe(true);
+ });
+
+ it('did the same for the ctx.user / os.user spellings', () => {
+ expect(evaluateVisibility(CTX_ALIAS_GATE, oldBag(CLERK))).toBe(true);
+ expect(evaluateVisibility(OS_ALIAS_GATE, oldBag(CLERK))).toBe(true);
+ });
+
+ it('did the same for a features flag that was off', () => {
+ expect(evaluateVisibility(FEATURE_GATE, oldBag(CLERK))).toBe(true);
+ });
+
+ it('bound `user` all along — which is what made the divergence invisible', () => {
+ // A gate authored against the back-compat spelling worked before AND after.
+ // An author who tested with `user.positions` had no way to discover that the
+ // canonical spelling was inert on this surface.
+ const USER_ALIAS_GATE = { dialect: 'cel', source: "'sales_manager' in user.positions" };
+ expect(evaluateVisibility(USER_ALIAS_GATE, oldBag(CLERK))).toBe(false);
+ expect(evaluateVisibility(USER_ALIAS_GATE, createExpressionEvaluator({ user: CLERK }))).toBe(false);
+ });
+});
+
+describe('objectui#6493 — neither call site hand-writes a predicate bag any more', () => {
+ // A source guard, because the defect was a COPY of the bag rather than a
+ // wrong value in it: nothing about a second `new ExpressionEvaluator({...})`
+ // is visible in a render, and the copy that drifted read as reasonable code
+ // for as long as it existed. The fence is the producer-side repair.
+ const sources = {
+ 'views/RecordFormPage.tsx': new URL('../views/RecordFormPage.tsx', import.meta.url),
+ 'console/AppContent.tsx': new URL('../console/AppContent.tsx', import.meta.url),
+ };
+
+ for (const [label, url] of Object.entries(sources)) {
+ it(`${label} builds its evaluator through createExpressionEvaluator`, () => {
+ const src = readFileSync(url, 'utf8');
+ expect(src).not.toMatch(/new\s+ExpressionEvaluator\s*\(/);
+ expect(src).toMatch(/createExpressionEvaluator\s*\(/);
+ });
+ }
+});

packages/app-shell/src/providers/ExpressionProvider.tsx

@@ -40,6 +40,66 @@ export interface ExpressionContextValue {
 
 const ExprCtx = createContext<ExpressionContextValue | null>(null);
 
+/** The inputs an app-shell surface has when it needs a predicate scope. */
+export interface ExpressionScopeInput {
+ user?: Record<string, any>;
+ app?: Record<string, any>;
+ data?: Record<string, any>;
+ features?: Record<string, any>;
+}
+
+/**
+ * The ONE predicate scope this tier binds — the single declaration of what an
+ * app-shell expression can name.
+ *
+ * ADR-0068 D1: expose the SAME user object under the canonical `current_user`
+ * plus the back-compat `user` alias, the server-RLS-parity `ctx.user` alias,
+ * and the server-CEL-parity `os.user` alias (the spec's canonical identity
+ * scope — `{{os.user.id}}` per @objectstack/spec expression docs), so a
+ * predicate authored against any one form evaluates identically on client,
+ * server-formula, and server-RLS (#2358 trap 1). D1 names a client `visible`
+ * gate as one of the three surfaces that must agree.
+ *
+ * ## Why this is a function and not three literals
+ *
+ * It was three literals, and they drifted (objectui#6493). `ExpressionProvider`
+ * built the full bag, while `RecordFormPage` and `AppContent` each built a
+ * private `new ExpressionEvaluator({ user, app, data })` for the SAME kind of
+ * gate — an object field's `visible` — beside the provider they never read.
+ * Those bags bound `user` but not the other three spellings of the same object,
+ * and not `features` at all, so ONE authored predicate meant two things
+ * depending on which evaluator reached it: `current_user` resolved on a nav
+ * item and FAULTED on a field, and a fault fails OPEN (`evaluateVisibility`
+ * below), which is indistinguishable on screen from a gate that said yes.
+ * A copy of the bag is how that recurs; a call is not.
+ *
+ * `features` is renderer-tier, not contract — the same posture `@objectstack/
+ * spec`'s `page.zod.ts` documents for component `visibleWhen` ("the shipping
+ * renderer additionally mounts `app`, `features`, `os.user` … renderer
+ * behaviour, NOT contract-guaranteed"). It is bound here because it is what
+ * THIS tier's own diagnostic advice tells an author they may name.
+ */
+export function buildExpressionScope({
+ user = {},
+ app = {},
+ data = {},
+ features = {},
+}: ExpressionScopeInput = {}): Record<string, any> {
+ return { current_user: user, user, ctx: { user }, os: { user }, app, data, features };
+}
+
+/**
+ * An `ExpressionEvaluator` over {@link buildExpressionScope}.
+ *
+ * Every app-shell site that needs an evaluator imperatively (i.e. one it cannot
+ * take from `useExpressionContext()`, because it builds the field list ABOVE
+ * the provider it mounts) calls this instead of `new ExpressionEvaluator(...)`
+ * with a hand-written bag.
+ */
+export function createExpressionEvaluator(input: ExpressionScopeInput = {}): ExpressionEvaluator {
+ return new ExpressionEvaluator(buildExpressionScope(input));
+}
+
 interface ExpressionProviderProps {
  children: React.ReactNode;
  user?: Record<string, any>;
@@ -50,24 +110,17 @@ interface ExpressionProviderProps {
 
 export function ExpressionProvider({ children, user = {}, app = {}, data = {}, features = {} }: ExpressionProviderProps) {
  const value = useMemo(() => {
- // ADR-0068: expose the SAME user object under the canonical `current_user`
- // plus the back-compat `user` alias, the server-RLS-parity `ctx.user`
- // alias, and the server-CEL-parity `os.user` alias (the spec's canonical
- // identity scope — `{{os.user.id}}` per @objectstack/spec expression docs),
- // so a predicate authored against any one form evaluates identically on
- // client, server-formula, and server-RLS (#2358 trap 1).
- const context = { current_user: user, user, ctx: { user }, os: { user }, app, data, features };
- const evaluator = new ExpressionEvaluator(context);
+ const evaluator = createExpressionEvaluator({ user, app, data, features });
  return { user, app, data, features, evaluator };
  }, [user, app, data, features]);
 
  // Also feed the predicate scope used by useCondition/useExpression in
  // @object-ui/react so action visibility predicates (e.g. on toolbar
  // buttons) can see deployment-level flags like features.multiOrgEnabled.
- // Mirror the canonical `current_user`/`user`/`ctx.user`/`os.user` aliases
- // here too.
+ // The SAME bag the evaluator above got — one builder, so the imperative and
+ // the hook-driven halves of this provider cannot drift apart either.
  const scope = useMemo(
- () => ({ current_user: user, user, ctx: { user }, os: { user }, app, data, features }),
+ () => buildExpressionScope({ user, app, data, features }),
  [user, app, data, features],
  );
 
@@ -85,10 +138,12 @@ export function ExpressionProvider({ children, user = {}, app = {}, data = {}, f
 export function useExpressionContext(): ExpressionContextValue {
  const ctx = useContext(ExprCtx);
  if (!ctx) {
- // Return a safe default so components can be used outside the provider
+ // Return a safe default so components can be used outside the provider.
+ // Through the same builder: the hand-written version gave `current_user`,
+ // `ctx.user` and `os.user` three DIFFERENT empty objects, which ADR-0068 D1
+ // spells as aliases "pointing at the same object".
  const fallback = { user: {}, app: {}, data: {}, features: {} };
- const evalContext = { current_user: {}, ctx: { user: {} }, os: { user: {} }, ...fallback };
- return { ...fallback, evaluator: new ExpressionEvaluator(evalContext) };
+ return { ...fallback, evaluator: createExpressionEvaluator(fallback) };
  }
  return ctx;
 }