fix(app-shell): report a faulting nav/area/field visible predicate in both builds

.changeset/6443-nav-visible-fault-diagnostic.md

@@ -0,0 +1,41 @@
+---
+'@object-ui/app-shell': patch
+---
+
+A nav / area / field `visible` predicate that FAULTS now says so, in both builds, once per
+distinct predicate source (objectui#6443). Observability only — no verdict moves.
+
+`ExpressionProvider.evaluateVisibility` is the gate behind a navigation item's `visible`,
+an area's derived visibility, and the field list `RecordFormPage` renders. It is fail-open:
+a predicate that cannot be evaluated returns `true`, so a menu entry whose role gate has
+stopped working renders **for everyone — including the role it was written to exclude** and
+looks exactly like an entry the author meant to show.
+
+That fault was swallowed one layer down. `evaluateCondition` is fail-soft: it answers an
+unevaluable predicate with `true` from its own `catch` and does not throw, so this site's
+`try/catch` never saw a predicate fault at all. Measured per dialect at this site before the
+fix — the bare-string dialect, the one a live gate was measured breaking on, printed
+**nothing at all**:
+
+| dialect | console at this site, before | after |
+|---|---|---|
+| bare string | nothing | one named line |
+| `{ dialect: 'cel' }` envelope | one generic line | one named line (the generic one is *replaced*, not added to) |
+| `${…}` template | one generic line **per evaluation** | one named line, deduped |
+
+The fix wires `EvaluationOptions.onFault` (the seam objectui#6038 landed) to
+`reportUnresolvableVisibilityPredicate`, exported from `@object-ui/react` — the same
+reporter, message, severity, dedupe `Set` and rate limit the node gate and `page:tabs`
+already use, so one authored predicate is entitled to one line rather than one line per
+package. It costs no extra engine call: the evaluator hands back the reason at the point it
+already knows the predicate faulted, with no `throwOnError` double evaluation.
+
+A nav item is not a schema node, so the reporter's `type` slot — which, with the gate key
+and the predicate source, is the dedupe key — is the constant `app-shell:visible`. The rate
+limit is therefore **one line per distinct authored predicate source**, not one per menu
+entry: a broken role gate copy-pasted across eight entries is one authoring mistake, in one
+string, fixed in one edit.
+
+**Fail-open is unchanged.** The item still renders for everyone on a fault. Flipping that to
+fail-closed is a permission-boundary change, not a diagnostic, and is not this change's to
+make; the change makes the silence stop and nothing else.

content/docs/guide/metadata-diagnostics.md

@@ -192,6 +192,27 @@ 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.
 
+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
+navigation, or on an object field rendered by the record form page. This one
+was silent in **both** builds before — including the bare-string dialect,
+which printed nothing at all — so a menu entry whose role gate had stopped
+working rendered for everyone, silently, with nothing to grep for. It now
+reports under the surface label `app-shell:visible`:
+
+```text
+[ObjectUI] A visibility predicate could not be evaluated - node "app-shell:visible"
+ visible: "'org_admin' in current_user.postions"
+ Reason: ...
+```
+
+The dedupe key is the predicate **source**, not the menu entry — one broken
+role gate copy-pasted across eight entries is one authoring mistake and prints
+one line, while a second, differently-broken predicate still gets its own.
+Fail-open is unchanged here too: the item still renders for everyone,
+including the role the predicate was written to exclude. That is what the line
+exists to tell you.
+
 ### 4. Governance overview page
 
 `/apps/<app>/metadata/_diagnostics` — a single sortable table of every

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

@@ -16,7 +16,7 @@
 
 import React, { createContext, useContext, useMemo } from 'react';
 import { ExpressionEvaluator } from '@object-ui/core';
-import { PredicateScopeProvider } from '@object-ui/react';
+import { PredicateScopeProvider, reportUnresolvableVisibilityPredicate } from '@object-ui/react';
 
 export interface ExpressionContextValue {
  /** Current authenticated user */
@@ -93,6 +93,62 @@ export function useExpressionContext(): ExpressionContextValue {
  return ctx;
 }
 
+/**
+ * The `type` slot of the shared visibility-fault reporter, for THIS surface
+ * (objectui#6443).
+ *
+ * ## Why the slot needed a decision at all
+ *
+ * `reportUnresolvableVisibilityPredicate` (@object-ui/react, objectui#6038)
+ * dedupes on `(type, key, predicate source)` — `id` rides along in the printed
+ * line but is deliberately NOT in the key. So whatever goes in `type` sets this
+ * site's RATE LIMIT, and a nav item is not a schema node: there is no
+ * `schema.type` to hand over.
+ *
+ * ## Why a CONSTANT, and not the item's identity
+ *
+ * The choice is between a constant (one line per distinct authored predicate
+ * SOURCE) and something per-item (one line per menu entry). Measured against
+ * the three cases that can actually occur:
+ *
+ * - two entries, two different broken predicates -> BOTH keys already differ
+ * on `source`, so both report either way. No difference.
+ * - two entries sharing ONE broken predicate (the copy-pasted role gate, the
+ * common shape) -> a constant reports ONCE, which is correct: it is one
+ * authoring mistake, in one string, fixed in one edit. A per-item key would
+ * report once per entry and add no information — same text, same reason.
+ * - one entry, evaluated many times -> both dedupe. This site is re-entered
+ * more than a node gate is: `hasVisibleNavigationItems` re-runs every item
+ * predicate to DERIVE area visibility (objectui#3311) before
+ * `NavigationRenderer` runs them again to render, and both re-run on every
+ * sidebar re-render.
+ *
+ * A per-item key is therefore never better and sometimes much worse. It is also
+ * unreachable without widening `VisibilityEvaluator` (@object-ui/layout), whose
+ * whole signature is `(expression) => boolean` — a cross-package public type
+ * change this diagnostics-only card does not get to make. The predicate SOURCE
+ * is the locator instead, and it is in both the key and the printed line: it is
+ * the string an author greps their metadata for, and it is unique to the bug in
+ * a way an item id is not.
+ *
+ * ## Why this spelling
+ *
+ * Namespaced with a colon, like `page:tabs` (the other non-node surface wired to
+ * this reporter). It names the app-shell `visible` GATE, not a component key —
+ * `app-shell` is deliberately NOT a registry key (objectui#4841), and the colon
+ * keeps this label out of that bare namespace so a diagnostic can never be read
+ * as claiming one.
+ *
+ * It is deliberately NOT `nav:item`, even though this card is written about nav
+ * and area items: `evaluateVisibility` is also the gate `RecordFormPage` runs
+ * over an object's field `visible` predicates, and labelling those "nav" would
+ * be false. The consequence is stated rather than hidden — a nav item and a
+ * form field carrying the IDENTICAL broken predicate text share one dedupe
+ * entry and produce one line. That is still one typo in one string; the line
+ * names the string, which is what both sites are grepped by.
+ */
+const APP_SHELL_VISIBLE_SURFACE = 'app-shell:visible';
+
 /**
  * Evaluate a visibility expression.
  * Supports:
@@ -122,9 +178,38 @@ export function evaluateVisibility(
  if (expression === true || expression === 'true') return true;
  if (expression === false || expression === 'false') return false;
 
+ // objectui#6443 — the fault is REPORTED now, and the verdict is untouched.
+ //
+ // `evaluateCondition` is fail-soft: it answers an unevaluable predicate with
+ // `true` from its OWN catch and does not throw, so the `catch` below never
+ // saw a predicate fault and this site swallowed every one of them in
+ // silence — in production AND in development, which is what made it worse
+ // than the node gate objectui#6038 fixed. `onFault` is the only channel that
+ // reaches that swallowed fault, and it costs nothing: no `throwOnError`
+ // second evaluation, same single engine call as before.
+ //
+ // FAIL-OPEN IS UNCHANGED, deliberately. A predicate that cannot be evaluated
+ // still returns `true`, so the nav item, area or field still renders for
+ // everyone — including the role the predicate was written to exclude. That is
+ // the shipped permission-boundary semantics; flipping it to fail-closed is a
+ // behaviour change, not a diagnostic, and it is not this card's to make. This
+ // card makes the silence stop, nothing else.
+ const report = (reason: unknown): void =>
+ reportUnresolvableVisibilityPredicate(
+ APP_SHELL_VISIBLE_SURFACE,
+ undefined,
+ 'visible',
+ expression,
+ reason,
+ );
+
  try {
- return evaluator.evaluateCondition(expression);
- } catch {
+ return evaluator.evaluateCondition(expression, { onFault: report });
+ } catch (err) {
+ // Defensive, and now loud too: `evaluateCondition` handles its own faults,
+ // so reaching here means the evaluator itself threw. That path returned the
+ // same fail-open `true` in silence; it no longer does.
+ report(err);
  return true; // Default to visible on error
  }
 }