A nav/area item visible predicate that faults is silent in BOTH builds — ExpressionProvider.evaluateVisibility swallows it with no diagnostic

Found while censusing predicate-fault catch sites for #6038 (production diagnostic for a faulting node gate). Filed unassigned; not fixed in that PR — see "Why it was left out" below.

The site

packages/app-shell/src/providers/ExpressionProvider.tsx — evaluateVisibility(expression, evaluator):

try {
  return evaluator.evaluateCondition(expression);
} catch {
  return true; // Default to visible on error
}

This is the gate for nav and area item visible predicates — the ones an app author writes to hide a menu entry by role. Its own docblock records why it matters: before it routed envelopes and bare strings to evaluateCondition, "a constant-false nav visible predicate (e.g. P`'org_admin' in current_user.positions`) still rendered for everyone — the app author had no working way to hide a menu item by role."

What is silent, measured per dialect

Measured on the built @object-ui/core evaluator (packages/core/dist/evaluator/ExpressionEvaluator.js), a faulting predicate with no options:

dialect	console today
bare string (e.g. `nosuchroot.x > 1`)	nothing
`{ dialect: 'cel' }` envelope	one generic line, deduped per source (`evalFieldPredicate`)
`${…}` template	one generic line per evaluation, never deduped

The bare-string dialect is the one objectstack#11254 measured a live gate breaking on. So a nav item whose visible predicate faults renders for everyone — including the role it was written to exclude — and says nothing. Unlike the node gate #6038 fixed, this one was silent in a development build too.

The fix is now a one-liner

#6038 added EvaluationOptions.onFault to @object-ui/core's ExpressionEvaluator — a passback that reports the fault the evaluator has already caught, at the same number of engine calls (no throwOnError double-evaluation), and reportUnresolvableVisibilityPredicate is exported from @object-ui/react so every surface shares ONE reporter and ONE rate limit. @object-ui/app-shell already depends on @object-ui/react, so this site needs the same shape SchemaRenderer and page:tabs now use:

return evaluator.evaluateCondition(expression, {
  onFault: (reason) => reportUnresolvableVisibilityPredicate(<node type>, <id>, 'visible', expression, reason),
});

What needs deciding rather than copying: what to pass as the node type/id, since a nav item is not a schema node — the dedupe key is (type, key, source), so whatever is chosen decides the rate limit's granularity here.

Why it was left out of #6038

The 2026-08-25 ruling is scoped to "a visibleWhen / node-gate predicate" and to "replacing the current __DEV__-only silence". This site is neither: a different key (visible), a different tier (app-shell chrome rather than the schema node tree), and its silence is unconditional rather than __DEV__-gated. Stating the boundary rather than quietly widening the PR's file surface into a third package.

Related: #6038 (the ruling and the seam), #5926 (the parent visibleWhen census), objectstack#11254 (where a silent bare-string breakage was measured).

Generated by Claude Code

Claim: domain:ui execution seat (objectui), session session_011SfZeFWrhGLHmfq61xbz4q, round 26.
Branch: claude/issue-6443-nav-visible-fault

Serial constraints — file face: packages/app-shell/src/providers/ExpressionProvider.tsx (evaluateVisibility, confirmed at :117 on origin/main) and its __tests__.

Non-overlap inside app-shell, checked directory by directory — three cards touch this package and none share a file: #6238 is src/services/ + src/views/metadata-admin/, #6431 (PR #6476, awaiting CI) is src/hooks/useConsoleActionRuntime.tsx + src/views/RecordDetailView.tsx, and this card is src/providers/.

Triage's family note is honoured: #6444 and #6445 are the other two of the #6038-census trio, neither is dispatched, and the trio shares zero files — so this goes out alone with no serial constraint against them.

Worth knowing before writing: onFault does not yet appear in ExpressionProvider.tsx — the seam landed in @object-ui/react under #6038, and wiring it here is the work. Re-derive what that seam actually exposes on the merged ref rather than assuming its shape from the card.

Generated by Claude Code

os-dev-report

{
  "issue": 6443,
  "status": "done",
  "branch": "claude/issue-6443-nav-visible-fault",
  "pr": "https://github.com/objectstack-ai/objectui/pull/6486",
  "premise_still_valid": true,
  "summary": "Wired ExpressionProvider.evaluateVisibility to EvaluationOptions.onFault, reporting through reportUnresolvableVisibilityPredicate from @object-ui/react — the same reporter, message, severity, dedupe Set and rate limit the node gate and page:tabs already use, at the same single engine call. Re-derived the #6038 seam on the merged ref rather than assuming it from the card: the reporter is NOT __DEV__-gated (that gate survives in SchemaRenderer.tsx only to choose between the throwOnError dev probe and the onFault production path — it no longer decides whether a fault is reported), so one wiring covers both builds, which is what this card needed. Also confirmed the deeper cause the card only implies: evaluateCondition is fail-soft and returns true from its OWN catch without throwing, so this site's try/catch never saw a predicate fault at all — the whole class was swallowed one layer down. THE DECISION (what stands in for node type/id): the constant 'app-shell:visible', id omitted. Rationale, measured not preferred: the key is (type, key, source) and id is not in it, so the choice is constant-vs-per-item. Two entries with different broken predicates already differ on source and report either way; two entries sharing one broken predicate (the copy-pasted role gate) are ONE authoring mistake in ONE string and must print once, where a per-item key prints N lines of identical text and reason. So per-item is never better and sometimes much worse. It is also unreachable without widening VisibilityEvaluator in @object-ui/layout, whose whole signature is (expression) to boolean — a cross-package public type change a diagnostics-only card does not get to make; the predicate source is the locator instead, and it is what an author greps their metadata for. Deliberately NOT 'nav:item': this same function is the gate over an object's field visible predicates in RecordFormPage, so a nav label would be false — the residue (a nav item and a field with identical broken text share one dedupe entry) is stated in the code, the changeset and the PR rather than hidden. FAIL-OPEN IS UNCHANGED on every path and dialect: the item still renders for everyone including the excluded role. This is diagnostics only; flipping to fail-closed would be a permission-boundary change and would need a maintainer, not this seat.",
  "tests": "GREEN, at 7ada702b8 (the final commit; nothing landed after the gate runs, worktree clean). Pins: `pnpm exec vitest run` on the new pin plus the pre-existing suite — 'Test Files 2 passed (2)' / 'Tests 16 passed (16)'. Affected-package tests, DECLARED NARROWING: the full app-shell suite (538 files) was cap-killed twice at the platform's 10-minute foreground limit (exit 143; it opens network calls, ECONNREFUSED 127.0.0.1:3000), so the set was narrowed mechanically to the 24 app-shell test files naming any importer of evaluateVisibility (ExpressionProvider / AppSidebar / RecordFormPage / the barrel / the symbol) — 'Test Files 24 passed (24)' / 'Tests 255 passed (255)'. Nothing outside that set can move: the diff changes exactly one runtime file, its only behaviour change is console output (the verdict is evaluateCondition's, passed through, and the catch still returns true), and no vitest setup converts console.warn into a failure (checked in all three setup files), so an unrelated test cannot redden merely because a new warn line exists. LINT WAS NOT NARROWED — the repo-wide run fit: `pnpm exec eslint . --format json` over the file set eslint's own config selected = 3811 files, 0 errors, 11046 pre-existing warnings; the changed source file adds ZERO new warnings (its 10 are pre-existing no-explicit-any on the untouched interface plus the 2 pre-existing react-refresh/only-export-components) and the new test file adds 7 no-explicit-any matching its neighbour's idiom. type-check exit 0 with the script name echoed (objectui spells it 'type-check', hyphenated), and the new pin IS in the program — confirmed with `tsc -p tsconfig.test.json --listFiles`, not assumed. Dependency closure built first (`pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build`, exit 0); the first attempt used `--concurrency=2` after --filter, which pnpm forwarded into tsc ('error TS5023: Unknown compiler option'), corrected rather than worked around. Gate verdict lines quoted from each gate's own output, exit codes captured before any pipe: check:control-bytes '✅ OK (scanned 5352 tracked text file(s); skipped 85 binary)' plus a direct grep -naP over exactly the 4 changed files (clean); check:phantom-deps '✅ Every in-scope import is declared by the package that publishes it.'; check:self-import '✅ No package names itself inside its own src/.'; check:doc-fences '✅ every TypeScript block in 223 document(s) is fenced ts/tsx/typescript…'; check:doc-types '✅ Every documented component type is registered.'; check:doc-snippets 'Semantic phase: 267 of 267 block(s) judged, 0 failed.'; check:readme-exports '✅ OK (… 378 self-imports judged (378 real, 0 wrong-path, 0 fabricated) … 0 unbuilt …)'; check:esm-specifiers, check:node-esm-load, check:entry-guard, check:published-dist, check:pre-install-import-graph, check:vi-mock-specifiers, check:eager-closure, check:docs-route-closure, check:shell-escape-residue, check-doc-links all exit 0; one-authority-per-exported-name (the KNOWN_COLLISIONS shrink-only baseline) 'Tests 11 passed (11)' with NO baseline extended; changeset guard '✅ No changeset declares a major bump.'. TWO GATES ARE RECORDED AS NOT MEASURED ON FIRST RUN, NOT AS RED: check:doc-snippets exited 2 and said so in as many words ('This is I could not run, NOT I ran and found errors'), and check:readme-exports exited 1 listing only 'type entry … is not on disk' — both unbuilt-dist preconditions, both green above after building what they name. REVERSE VERIFICATION, direction predicted BEFORE the run and recorded in the pin's docblock: restoring ExpressionProvider.tsx to the PINNED BASE (f7c52e21c, never the moving origin/main ref) turns red exactly the 8 discriminating cells with reports() going to 0, leaves the 4 controls green, and moves the template cell in a SECOND independent direction, total console lines 1 to 3. Observed exactly: 'Tests 8 failed | 8 passed (16)' — the 8 failures being the 8 discriminating cells, the 8 passes being the 4 controls plus all 4 cells of the pre-existing suite. The second direction was initially NOT observed because the first assertion short-circuited; the cell's assertions were reordered and the ablation re-run, which produced it: 'AssertionError: expected [ …(3) ] to have a length of 1 but got 3'. MUTATION PROVEN ON DISK by grepping the deleted AND the restored text (onFault: report 0, APP_SHELL_VISIBLE_SURFACE 0, reportUnresolvableVisibilityPredicate 0, old bare call 1) — never a diffstat, never an editor's exit code; the script aborts with a distinct exit code if any count is wrong so a no-op ablation cannot be read as a result. RESTORE PROVEN by blob hash (BACK_BLOB == HEAD_BLOB == 1c18358ca452a5193b4539ab6fa29d7ac126f843, empty hash treated as FAILURE) plus an empty `git diff HEAD`, under an EXIT/INT/TERM trap holding absolute paths, with restore written as `git checkout HEAD -- ABS` rather than a bare checkout. No rebuild is involved on this leg and the report says why: vitest resolves every workspace package to src through the repo alias map, so the ablation cannot read a stale dist (the dist half is the typecheck, which was run against a freshly built closure). WHY THE PIN IS NECESSARY, as a reading rather than a claim: the pre-existing suite ExpressionProvider.evaluateVisibility.test.ts PASSES GREEN AGAINST THE DEFECT — all 4 of its cells passed in the ablation run — because its 'fails open (visible) on an unevaluable predicate' case asserts the exact verdict the broken site returns. GHOST-ASSERTION GUARD: the bare-string cell pins the log and the OBSERVABLE OUTCOME together (the item survives the real hasVisibleNavigationItems guard for the excluded role), so it cannot go green on a site that reported a fault by refusing to render. DEGENERATE CONTROLS NAMED: 4 cells pass both ways on purpose — the positive console-capture control (without it every toHaveLength(0) here is vacuous), the degenerate control (reports() must be a filter, not a call counter), the fail-open control (carries no console assertion at all, so it cannot redden for the discriminating reason; it guards a future diagnostic change that also flips fail-open to fail-closed), and the healthy-predicate control (guards a future reporter that fires on a genuine false). RATE LIMIT PINNED IN BOTH DIRECTIONS, not vacuously: same source across the REAL AppSidebar area-election composition (areas.filter over hasVisibleNavigationItems, the #3311 derivation) counts 6 evaluations of one faulting predicate over three areas and two passes and asserts BOTH the count and the single line, so it cannot pass on a site entered once; two different sources produce two lines; and a second evaluator does not buy a second line, which pins the cross-package shared Set. CLAUSE ②, stated in as many words: THE PUBLISHED SURFACE IS NOT WIDENED. The diff of every **/src/index.ts from the pinned base is EMPTY, and — because an empty entry diff does not settle it alone — the whole packages/ diff contains no added or removed line carrying an export declaration (the single 'export' match is a word inside a test comment). No member was added to any type the entry already exports by name: the emitted dist/providers/ExpressionProvider.d.ts still declares exactly ExpressionContextValue, ExpressionProvider, useExpressionContext and evaluateVisibility, the last with a byte-identical signature; the one new module-level binding (APP_SHELL_VISIBLE_SURFACE) is deliberately not exported and does not appear in the declaration face.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #6487: reportUnresolvableVisibilityPredicate's closing advice paragraph names node-tier roots (record, page state) that no app-shell visible predicate binds — the app-shell evaluator's context bag is current_user/user/ctx.user/os.user/app/data/features, so an author whose nav gate faulted is pointed at two roots that cannot exist at that tier while the ones that do go unnamed. Observational, labelled finding + domain:ui, unassigned, no pm:queue. Re-tiering shared copy in @object-ui/react is the third-package widening this card's own text declines. Deduped against the 245 open issues before filing; NOT a duplicate of #6445 (which records the same reporter's FIRST paragraph being wrong for the disabled polarity) or #6444 (the un-deduped evaluate() warning)."
  ]
}

Notes for the PM, outside the JSON:

PR fix(app-shell): report a faulting nav/area/field visible predicate in both builds #6486 is a draft, Fixes #6443 in the body, changeset included, no auto-merge — it is yours to land. mergeable_state reads behind (main moved during the run); the merge queue rebuilds on current main, so no rebase was done and none is needed.
The family note was honoured: ExpressionEvaluator.evaluate warns ONCE PER EVALUATION on a faulting ${…} — a broken prop in a 200-row list is 200 console lines per render #6444 and The disabled / disabledOn node gate has NO fault diagnostic in either build — and its fail-soft polarity greys a control out rather than showing it #6445 were not folded in. The ${…} reading in the PR is ExpressionEvaluator.evaluate warns ONCE PER EVALUATION on a faulting ${…} — a broken prop in a 200-row list is 200 console lines per render #6444's defect being transferred away from at this one site by supplying onFault, not fixed — that card's evaluate() sites still flood for every other caller.
One judgement call worth your eye: this same evaluateVisibility also gates record-form field visible predicates, so the fix gives that surface the diagnostic too. That is coverage the card did not ask for but could not be avoided without splitting the function, and it is why the surface label is tier-level rather than nav:item.

Generated by Claude Code

ACCEPT — PR #6486, head 7ada702b8. Reviewed from the tree: the dev pushed a complete, green PR and has not returned a report. That does not block review — the tree was always the authority — but it is recorded here rather than left unexplained.

The title says nav/area/field; the card said nav/area. That is not scope creep

evaluateVisibility is one function with three caller families, and fixing it covers all of them by construction:

layout/AppSidebar.tsx:226 — nav
chrome/CommandPalette.tsx:86 — palette items
console/AppContent.tsx:1090 and :1094 — fields, filtering on f.visible

The card named the surfaces its reporter had hit; the fix lands one level down, where all three meet. The honest title is the wider one.

My fail-open fence held, and it was the thing most worth checking

The dispatch warned that a diagnostics card which silently flips fail-open to fail-closed is a permission-boundary change wearing an observability costume, and would need a maintainer rather than this seat. The fault path reads:

report(err);
return true; // Default to visible on error

Unchanged. The in-file comment gets the distinction exactly right — "That path returned the same fail-open true in silence; it no longer does" — meaning it is no longer silent, not that the return moved. A faulting predicate still renders the item; the difference is that someone now finds out.

The #6038 seam was reused, not reinvented

reportUnresolvableVisibilityPredicate is imported from @object-ui/react and both paths route through one report closure — the onFault callback for an evaluator that reports, and catch (err) for an evaluator that throws. The old bare catch {} was the whole defect on the second path, and it is gone.

The dedupe-key decision — the one thing I handed over — is sound. APP_SHELL_VISIBLE_SURFACE = 'app-shell:visible' is a module-local constant, and the expression is passed as its own argument, so #6038's per-predicate-source keying still separates distinct predicates. The surface constant labels them; it does not collapse them. A nav item and a field predicate that both fault still report independently.

Clause-②: no public surface change, and this one needed checking rather than assuming, because evaluateVisibility is exported by name from packages/app-shell/src/index.ts:14. Its signature is byte-identical before and after — (expression: string | boolean | { dialect?: string; source?: string } | undefined, evaluator: ExpressionEvaluator): boolean. No export line is added or removed, and APP_SHELL_VISIBLE_SURFACE is module-local.

Docs went to content/docs/guide/metadata-diagnostics.md; ⛔ content/docs/releases/ is untouched, and a changeset is present.

29 checks, 0 failed, 0 pending. Landing it.

Generated by Claude Code

Navigation Menu

Repository navigation

A nav/area item `visible` predicate that faults is silent in BOTH builds — `ExpressionProvider.evaluateVisibility` swallows it with no diagnostic #6443

The site

What is silent, measured per dialect

The fix is now a one-liner

Why it was left out of #6038

The title says nav/area/field; the card said nav/area. That is not scope creep

My fail-open fence held, and it was the thing most worth checking

The #6038 seam was reused, not reinvented

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

A nav/area item visible predicate that faults is silent in BOTH builds — ExpressionProvider.evaluateVisibility swallows it with no diagnostic #6443

Description

The site

What is silent, measured per dialect

The fix is now a one-liner

Why it was left out of #6038

Activity

os-support-ai commented on Aug 26, 2026

os-support-ai commented on Aug 26, 2026

os-support-ai commented on Aug 26, 2026

The title says nav/area/field; the card said nav/area. That is not scope creep

My fail-open fence held, and it was the thing most worth checking

The #6038 seam was reused, not reinvented

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

A nav/area item `visible` predicate that faults is silent in BOTH builds — `ExpressionProvider.evaluateVisibility` swallows it with no diagnostic #6443