fix(react): a page node whose visibility predicate faults is withheld (objectui#13299)

.changeset/13299-node-visibility-fail-closed.md

@@ -0,0 +1,6 @@
+---
+"@object-ui/react": patch
+"@object-ui/components": patch
+---
+
+A page node whose declared `visibleWhen` / `visible` / `visibleOn` predicate cannot be evaluated (over the CEL bounds, a type error, an unbound root, a missing function) is no longer rendered: page visibility gates now fail closed, as a `page:tabs` item already did. The fault is still reported once, and the console line says the node was withheld. A blank gate still means "no gate", and a predicate that evaluates decides as before. A `page:tabs` badge no longer counts a related list withheld this way. Form field rules keep their fail-open (objectui#13299).

content/docs/guide/metadata-diagnostics.md

@@ -217,26 +217,32 @@ and the engine's reason:
 [ObjectUI] A visibility predicate could not be evaluated - node "record:alert" (id: "a1")
   visibleWhen: "nosuchroot.status == 'draft'"
   Reason: Failed to evaluate expression "nosuchroot.status == 'draft'": nosuchroot is not defined
-The node was treated as its safe default, which on this surface means the
-gate did NOT bite - a predicate that cannot be evaluated reads on screen
-exactly like one that said yes.
+...
 ```
 
+The rest of the line says what the fault did to the node: on `visibleWhen`,
+`visible` and `visibleOn` the node was **withheld**, on `hidden` and `hiddenOn`
+it was removed (see below).
+
 The line is **rate limited to one per distinct predicate source**, so a broken
 predicate rendered down two hundred rows of a list is one line, not two
 hundred — while a second, differently-broken predicate still gets its own.
-The verdict is unchanged in every case: this is a diagnostic about a
-predicate, not a change to what the gate decides. A node gate that fails open
-renders exactly as it always did; the difference is that it now says so.
-
-One gate in this family fails **closed** instead: a `page:tabs` item's
-`visibleWhen` (objectui#13286). A declared item predicate that cannot be
-evaluated — over the CEL engine's AST bound, a type error, an unbound root, a
-missing function — withholds its tab, header and panel, rather than drawing it
-for a viewer the predicate was written to exclude, and the console line says
-the tab was **withheld**. If that tab was the active one, the strip falls back
-to the first visible tab. An item with no `visibleWhen`, or a blank one, is
-still shown.
+
+Page visibility gates fail **closed**: a page node's `visibleWhen` (and its
+`visible` / `visibleOn` / `hidden` / `hiddenOn` siblings, objectui#13299) and a
+`page:tabs` item's `visibleWhen` (objectui#13286). A declared predicate that
+cannot be evaluated — over the CEL engine's AST bound, a type error, an unbound
+root such as `record` on a surface with no row, a missing function — withholds
+what it gates rather than drawing it for a viewer the predicate was written to
+exclude: the node is not rendered, and a tab is not drawn, header and panel. A
+related list withheld this way is not counted in its tab's badge either. The
+console line says the node or tab was **withheld**. If a withheld tab was the
+active one, the strip falls back to the first visible tab. A node or item with
+no gate, or a blank one (`''`, whitespace, an envelope whose `source` is blank),
+declares nothing and is still shown; a predicate that evaluates decides as it
+always did. Form field rules (`visibleWhen` / `readonlyWhen` on a field, and
+the other rules listed above) are not page visibility gates and keep their
+fail-open.
 
 The **app shell's own `visible` gate** joins the same reporter and the same
 rate limit: a `visible` predicate on a navigation item, on an area's

packages/app-shell/src/providers/__tests__/hasCapability-13006.render.test.tsx

@@ -23,10 +23,10 @@
  *     `isPlatformAdmin` is not a capability, and the predicate names the
  *     capability, not a proxy for it.
  *
- * Then: not loaded faults LOUDLY (a diagnostic naming the predicate, and the
- * node is NOT silently hidden — the fail-soft node gate shows it, which is the
- * shipped node policy, not this binding's answer); an empty list answers
- * `false`; `can()` is unchanged.
+ * Then: not loaded faults LOUDLY (a diagnostic naming the predicate; the node
+ * is withheld by the node gate, which fails CLOSED on a fault since
+ * objectui#13299 — the node policy, not this binding's answer); an empty list
+ * answers `false`; `can()` is unchanged.
  *
  * The watch item: the capability list and the permissions map are bound from
  * one answer, so they change together. `packages/react`'s `ActionProvider`
@@ -141,9 +141,9 @@ describe('a page node gated on current_user.hasCapability (objectui#13006)', ()
   it('not loaded: the predicate FAULTS loudly — reported, and not a silent false', () => {
     render(page(PRINCIPALS.owner.user, [gatedNode('gated-not-loaded'), COMPANION]));
     expect(screen.getByTestId('companion')).toBeInTheDocument();
-    // The node gate is fail-soft (the shipped node policy): a fault shows the
-    // node. A silent `false` would have hidden it with nothing said.
-    expect(screen.getByTestId('gated-not-loaded')).toBeInTheDocument();
+    // The node gate fails CLOSED on a fault (objectui#13299): the node is
+    // withheld, and — unlike a silent `false` — the fault is said out loud.
+    expect(screen.queryByTestId('gated-not-loaded')).toBeNull();
     expect(warnings.some((w) => w.includes("current_user.hasCapability('manage_org_users')"))).toBe(true);
   });
 
@@ -153,7 +153,8 @@ describe('a page node gated on current_user.hasCapability (objectui#13006)', ()
         {page(PRINCIPALS.owner.user, [gatedNode('gated-unreported')])}
       </MePermissionsProvider>,
     );
-    expect(screen.getByTestId('gated-unreported')).toBeInTheDocument();
+    // Withheld: the node gate fails closed on a fault (objectui#13299).
+    expect(screen.queryByTestId('gated-unreported')).toBeNull();
     expect(warnings.some((w) => w.includes("current_user.hasCapability('manage_org_users')"))).toBe(true);
   });
 
@@ -246,9 +247,10 @@ describe('the capability list and the permissions map change together (objectui#
         {page(PRINCIPALS.owner.user, [gatedNode('gated-refused')])}
       </MePermissionsProvider>,
     );
-    // Not bound: the call faults (shown by the fail-soft node gate, reported),
-    // rather than answering from a list whose answer's map was refused.
-    expect(screen.getByTestId('gated-refused')).toBeInTheDocument();
+    // Not bound: the call faults (withheld by the fail-closed node gate since
+    // objectui#13299, and reported), rather than answering from a list whose
+    // answer's map was refused.
+    expect(screen.queryByTestId('gated-refused')).toBeNull();
     expect(warnings.some((w) => w.includes("current_user.hasCapability('manage_org_users')"))).toBe(true);
     expect(errors).toHaveBeenCalled();
   });

packages/components/src/__tests__/page-node-visibility-fail-closed-13299.test.tsx

@@ -0,0 +1,159 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#13299 — a page node whose declared `visibleWhen` FAULTS is withheld
+ * by `SchemaRenderer` (fail-closed), so the `page:tabs` count-probe gate that
+ * mirrors it closes too: a badge counts what is drawn.
+ *
+ * Rows:
+ *  (e) the tab badge does not count a related list withheld by a faulting
+ *      gate, on the list itself or on a container above it; an over-budget
+ *      source is one such fault;
+ *  (f) CONTROL — a form FIELD `visibleWhen` that faults is still drawn: form
+ *      field rules keep their documented fail-open.
+ */
+
+import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
+import { render, cleanup, screen } from '@testing-library/react';
+import { ComponentRegistry } from '@object-ui/core';
+import {
+  RecordContextProvider,
+  SchemaRenderer,
+  UNRESOLVABLE_VISIBILITY_PREFIX,
+  __resetVisibilityPredicateWarnings,
+} from '@object-ui/react';
+import { RelatedCountStore } from '../hooks/related-count-store';
+// Module-scope import (not `beforeAll`) — objectui#3010/#3021.
+import '../renderers';
+
+const RECORD_OBJECT = 'zz13299_env';
+const PARENT_ID = 'ENV-13299';
+const WITHHELD_CHILD = 'zz13299_domain';
+const SHOWN_CHILD = 'zz13299_member';
+const TOTALS: Record<string, number> = { [WITHHELD_CHILD]: 3, [SHOWN_CHILD]: 2 };
+
+const makeDS = () => ({
+  find: vi.fn(async (object: string) => ({ data: [], total: TOTALS[object] ?? 0 })),
+  getObjectSchema: vi.fn(async (name: string) => ({ name, fields: {} })),
+});
+
+const cel = (source: string) => ({ dialect: 'cel', source });
+const orChain = (n: number) => Array.from({ length: n }, (_, i) => `${i} == ${i}`).join(' || ');
+
+const relatedList = (objectName: string, visibleWhen?: unknown) => ({
+  type: 'record:related_list',
+  ...(visibleWhen === undefined ? {} : { visibleWhen }),
+  properties: { objectName, relationshipField: 'environment' },
+});
+
+/** Two tabs minimum — the strip hides itself at length 1. */
+const tabsSchema = (overview: Record<string, unknown>) => ({
+  type: 'page:tabs',
+  id: 'tabs-13299',
+  items: [
+    { label: 'Details', value: 'details', children: [{ type: 'element:text', properties: { content: 'DETAILS' } }] },
+    { label: 'Overview', value: 'overview', ...overview },
+  ],
+});
+
+function renderTabs(overview: Record<string, unknown>) {
+  const ds = makeDS();
+  const utils = render(
+    <RecordContextProvider
+      objectName={RECORD_OBJECT}
+      recordId={PARENT_ID}
+      data={{ id: PARENT_ID, plan: 'free' }}
+      dataSource={ds as any}
+    >
+      <SchemaRenderer schema={tabsSchema(overview) as any} />
+    </RecordContextProvider>,
+  );
+  return { ...utils, ds };
+}
+
+const probedObjects = (ds: ReturnType<typeof makeDS>) =>
+  Array.from(new Set(ds.find.mock.calls.map((c) => c[0] as string))).sort();
+
+type WarnSpy = { mock: { calls: unknown[][] } };
+const reports = (warn: WarnSpy): string[] =>
+  warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes(UNRESOLVABLE_VISIBILITY_PREFIX));
+
+beforeEach(() => {
+  RelatedCountStore._reset();
+  __resetVisibilityPredicateWarnings();
+});
+afterEach(() => {
+  cleanup();
+  RelatedCountStore._reset();
+  vi.restoreAllMocks();
+});
+
+describe('objectui#13299 (e) — the tab badge does not count a node withheld by a faulting gate', () => {
+  it.each([
+    ['an unbound root on the list', relatedList(WITHHELD_CHILD, 'nosuchroot13299e.x > 1')],
+    ['an over-budget CEL source on the list', relatedList(WITHHELD_CHILD, cel(orChain(120)))],
+    [
+      'a faulting gate on a container above the list',
+      { type: 'page:card', visibleWhen: cel('record.plan =='), children: [relatedList(WITHHELD_CHILD)] },
+    ],
+  ])('%s', async (_label, withheld) => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const { findByLabelText, queryByLabelText, ds } = renderTabs({
+      children: [withheld, relatedList(SHOWN_CHILD)],
+    });
+
+    expect((await findByLabelText('2 items')).textContent).toBe('2');
+    expect(queryByLabelText('5 items')).toBeNull();
+    expect(probedObjects(ds)).toEqual([SHOWN_CHILD]);
+    // One authored predicate, one line — and it says the node was withheld.
+    const lines = reports(warn);
+    expect(lines).toHaveLength(1);
+    expect(lines[0]).toContain('WITHHELD');
+  });
+
+  it('CONTROL: a gate that evaluates to true is counted', async () => {
+    const { findByLabelText, ds } = renderTabs({
+      children: [relatedList(WITHHELD_CHILD, cel("record.plan == 'free'")), relatedList(SHOWN_CHILD)],
+    });
+
+    expect((await findByLabelText('5 items')).textContent).toBe('5');
+    expect(probedObjects(ds)).toEqual([WITHHELD_CHILD, SHOWN_CHILD].sort());
+  });
+
+  it('CONTROL: a blank gate declares nothing and is counted', async () => {
+    vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const { findByLabelText } = renderTabs({
+      children: [relatedList(WITHHELD_CHILD, ''), relatedList(SHOWN_CHILD)],
+    });
+
+    expect((await findByLabelText('5 items')).textContent).toBe('5');
+  });
+});
+
+describe('objectui#13299 (f) — CONTROL: a form field rule keeps its fail-open', () => {
+  it('a field whose `visibleWhen` faults is still drawn', () => {
+    vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const Form = ComponentRegistry.get('form')!;
+    render(
+      <Form
+        schema={{
+          type: 'form',
+          showSubmit: false,
+          showCancel: false,
+          fields: [
+            { name: 'subject', label: 'Subject', type: 'input' },
+            { name: 'notes', label: 'Notes', type: 'input', visibleWhen: 'nosuchroot13299f.x > 1' },
+          ],
+        }}
+      />,
+    );
+    expect(screen.getByLabelText(/^subject/i)).toBeInTheDocument();
+    expect(screen.getByLabelText(/^notes/i)).toBeInTheDocument();
+  });
+});

packages/components/src/__tests__/page-tabs-count-visible-when-12987.test.tsx

@@ -125,14 +125,15 @@ describe('page:tabs derived count skips a related list its own visibleWhen hides
     expect(probedObjects(ds)).toEqual([SHOWN_CHILD]);
   });
 
-  it('counts a list whose visibleWhen cannot be evaluated, as the body draws it (fail-open)', async () => {
+  it('skips a list whose visibleWhen cannot be evaluated, as the body withholds it (fail-closed, objectui#13299)', async () => {
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
     try {
-      const { findByLabelText } = renderTabs('free', {
+      const { findByLabelText, queryByLabelText } = renderTabs('free', {
         children: [relatedList(HIDDEN_CHILD, 'record.plan ==== )('), relatedList(SHOWN_CHILD)],
       });
 
-      expect((await findByLabelText('5 items')).textContent).toBe('5');
+      expect((await findByLabelText('2 items')).textContent).toBe('2');
+      expect(queryByLabelText('5 items')).toBeNull();
     } finally {
       warn.mockRestore();
     }

packages/components/src/renderers/complex/__tests__/data-table-empty-action-visible-when.test.tsx

@@ -41,9 +41,9 @@
  * ## The fault case is MEASURED, not chosen
  *
  * A predicate naming a genuinely unbound root must behave the way the central
- * gate behaves, not some new way. `evaluateCondition` fails SOFT — an
- * unresolvable predicate answers `true` on every one of its internal paths —
- * so the node stays visible. Pinned here so a future change to the slot cannot
+ * gate behaves, not some new way. The central gate fails CLOSED since
+ * objectui#13299 — a declared predicate that cannot be evaluated withholds its
+ * node — so the node is not drawn. Pinned here so a future change to the slot cannot
  * quietly pick a different answer than the one `SchemaRenderer` gives every
  * other node.
  */
@@ -125,7 +125,7 @@ describe('data-table emptyAction — the central visibleWhen gate applies', () =
     expect(screen.getByTestId('empty-action-cta')).toBeInTheDocument();
   });
 
-  it('renders the node on a faulting predicate — the central gate fails soft', () => {
+  it('withholds the node on a faulting predicate — the central gate fails closed (objectui#13299)', () => {
     renderEmptyTable(
       {
         type: CTA_TYPE,
@@ -134,7 +134,7 @@ describe('data-table emptyAction — the central visibleWhen gate applies', () =
       /* canCreate */ false,
     );
     expectEmptyStateReached();
-    expect(screen.getByTestId('empty-action-cta')).toBeInTheDocument();
+    expect(screen.queryByTestId('empty-action-cta')).toBeNull();
   });
 
   it('honours the bare-string spelling of the same predicate', () => {

packages/components/src/renderers/form/__tests__/predicate-scope-parity-6010.test.tsx

@@ -264,14 +264,19 @@ function fieldVisibleOnSurface(predicate: unknown, scope: Record<string, unknown
 type Surface = {
   label: string;
   show: (predicate: unknown, scope: Record<string, unknown>) => boolean | Promise<boolean>;
+  /**
+   * What a FAULTING predicate answers on this surface. The page node gate
+   * fails CLOSED since objectui#13299; the form rules keep their fail-open.
+   */
+  shownOnFault: boolean;
 };
 
 const SURFACES: Surface[] = [
-  { label: 'page component / app-nav node `visibleWhen`', show: pageSurface },
-  { label: 'per-option `visibleWhen` (select options)', show: optionSurface },
-  { label: 'form SECTION `visibleWhen`', show: sectionSurface },
-  { label: 'form FIELD `visibleWhen`', show: fieldSurface },
-  { label: 'form FIELD `visibleOn` (ADR-0089 D2 alias)', show: fieldVisibleOnSurface },
+  { label: 'page component / app-nav node `visibleWhen`', show: pageSurface, shownOnFault: false },
+  { label: 'per-option `visibleWhen` (select options)', show: optionSurface, shownOnFault: true },
+  { label: 'form SECTION `visibleWhen`', show: sectionSurface, shownOnFault: true },
+  { label: 'form FIELD `visibleWhen`', show: fieldSurface, shownOnFault: true },
+  { label: 'form FIELD `visibleOn` (ADR-0089 D2 alias)', show: fieldVisibleOnSurface, shownOnFault: true },
 ];
 
 afterEach(() => {
@@ -298,20 +303,21 @@ describe('#6010 — `current_user` binds identically on every `visibleWhen` surf
     }
   });
 
-  describe('FAULTED — a genuinely unbound root still fails OPEN (unchanged by #6010)', () => {
+  describe('FAULTED — a genuinely unbound root: the form rules fail OPEN, the page node fails CLOSED (objectui#13299)', () => {
     // Existing behaviour, pinned so the DENIED rows above mean "evaluated and
     // false" rather than "could not be evaluated". Asserted against the DENIED
     // user on purpose: the only difference from the DENIED block is the ROOT
-    // the predicate names, so a fail-CLOSED regression cannot hide behind the
-    // membership.
+    // the predicate names, so a fail-CLOSED regression on a form rule cannot
+    // hide behind the membership. The page node is the exception, by ruling:
+    // it fails CLOSED since objectui#13299.
     for (const surface of SURFACES) {
       it(surface.label, async () => {
         // Fail-open is loud (#5149 / #5454 leg 3) — the warning is another
         // surface's contract, and silencing it here keeps this file about the
         // verdict.
         vi.spyOn(console, 'warn').mockImplementation(() => {});
         vi.spyOn(console, 'error').mockImplementation(() => {});
-        expect(await surface.show(UNBOUND_ROOT, DENIED)).toBe(true);
+        expect(await surface.show(UNBOUND_ROOT, DENIED)).toBe(surface.shownOnFault);
       });
     }
   });