Skip to content

Commit 1fab654

Browse files
committed
fix(react): stop advertising app as a bound expression-scope root
objectui#8164 removed `app` from `buildExpressionScope` (the objectui#8155 option-B ruling) on three faces inside app-shell, but never asked which OTHER surfaces state the same fact. The post-merge tier audit found the residue. The blocking one: `@object-ui/react`'s `SCOPE_TIER_ADVICE['app-shell']` — the paragraph printed in production when a predicate cannot be evaluated — still read "plus `app` and `features`". It is the line an author sees at exactly the moment a stale `app.*` predicate faults, so it answered "why did this not resolve?" by naming the root that is the reason, and its own byte-pin (`expect(msg).toContain('`app`')`) held the false sentence in place. Swept as a class, not as those two coordinates: every place in the tree that stated `app` was a bound expression-scope root — diagnostic copy, ambient-scope docblocks in react / core / components / plugin-detail / plugin-form / app-shell / console, a README, and fourteen test fixtures that transcribed the old bag — is corrected. The two app-shell fixtures now call `buildExpressionScope` instead of transcribing it, so that pair cannot drift again. Also corrects two false sentences in the release-bound changeset: objectstack#16420 was closed `not_planned` 2026-09-07 (not left open), and the consequence of a stale `app.*` predicate is not uniformly "fails open" — it is per surface, and the seven directions are now measured and listed. Nothing unrelated to `app` moved: no export line added or removed anywhere in the diff, no barrel or package manifest touched, and the `app` prop and React context field on `ExpressionProvider` (never CEL roots) are untouched. Part of #8155 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01611D6ZaRaMmwTNQmSbk8MH
1 parent 8a388ee commit 1fab654

38 files changed

Lines changed: 272 additions & 74 deletions

‎.changeset/6487-visibility-advice-per-tier.md‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ prose that described it:
2929
byte-for-byte what it was.
3030
- **`'app-shell'`** — the chrome gate `ExpressionProvider.evaluateVisibility`
3131
runs, wired onto this reporter by objectui#6443. Its evaluator is built from
32-
`{ current_user, user, ctx: { user }, os: { user }, app, data, features }`, so
33-
the line now names `current_user` with its three ADR-0068 alias spellings,
34-
`app`, and `features` — the deployment-flag root that provider documents for
35-
exactly this kind of predicate — and states outright that `record` and
36-
`page.<var>` do not exist there.
32+
`{ current_user, user, ctx: { user }, os: { user }, data, features }`, so
33+
the line now names `current_user` with its three ADR-0068 alias spellings and
34+
`features` — the deployment-flag root that provider documents for exactly
35+
this kind of predicate — and states outright that `record` and `page.<var>`
36+
do not exist there. (That bag and that paragraph also carried an `app` root
37+
when this entry was first written; objectui#8155 removed it from both before
38+
either shipped, so the released message names five roots, not six.)
3739

3840
**Why not generalise the copy instead.** Dropping the concrete root names would
3941
have made one paragraph true everywhere at the cost of making it useful nowhere:

‎.changeset/7727-conditional-formatting-record-scope.md‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,34 @@ consumer aligns to it, rather than the engine growing a root to match this consu
6060
population cannot be measured from this repository.** In-tree usage is zero — swept
6161
across `packages/`, `apps/`, `examples/` and `content/` with a firing control — but
6262
metadata authored in real deployments lives outside this tree and no sweep here can
63-
see it. Any predicate that reads `app.*` — a conditional-formatting condition, an
64-
action `visible` / `disabled`, a field `visibleWhen` — stops resolving and, because
65-
unresolvable visibility predicates **fail open**, will start reading as "yes" rather
66-
than erroring. That is the accepted cost of the ruling, not an oversight. There is no
67-
replacement root: `app` was never in the protocol. If you need a "current app" value in
68-
a predicate, that is a spec/engine vocabulary widening to be filed (the producer-side
69-
card, objectstack#16420, stays open as the record to reopen).
63+
see it. Any predicate that reads `app.*` stops resolving. There is no replacement
64+
root: `app` was never in the protocol. If you need a "current app" value in a
65+
predicate, that is a spec/engine vocabulary widening to be filed fresh — the
66+
producer-side card objectstack#16420 was closed `not_planned` by the same ruling
67+
(2026-09-07), so there is no open record waiting for it.
68+
69+
**What a stale `app.*` predicate does now depends on the surface — the direction is
70+
NOT uniform, and two of them fail the safe way.** Measured per surface on the merged
71+
head, each with a resolvable control predicate firing in the same run:
72+
73+
| Surface | Entry point | A stale `app.*` predicate now |
74+
|---|---|---|
75+
| Conditional-formatting `condition` | `resolveConditionalFormatting` → `evalRowPredicate` (`fallback: false`) | **fails CLOSED** — the rule silently stops matching, no style is applied |
76+
| Row/header action `visible` / `disabled` | `evalRowPredicate` (`fallback: false`) | **fails CLOSED** — the action is hidden / left enabled |
77+
| Action `visible` on `action-button` / `action-menu` / `action-bar` | `useCondition(…, { throwOnError: true })` | **fails CLOSED** — hidden, with a one-time console warning |
78+
| Action `visible` on `action-icon` / `action-group` | `useCondition` (default) | **fails OPEN** — the action is shown |
79+
| Field `visibleWhen` (form field rules) | `resolveFieldRuleState` → `evalFieldPredicate` (fallback `true`) | **fails OPEN** — the field is shown |
80+
| Field `visibleWhen` (app-shell object field) and nav / area `visible` | `isObjectFieldVisible` / `evaluateVisibility` | **fails OPEN** — shown, with a console diagnostic |
81+
| Field `readonlyWhen` / `requiredWhen` | `resolveFieldRuleState` (fallback `false`) | **fails CLOSED** — not readonly, not required |
82+
83+
So the cost is not one shape: on the fail-OPEN surfaces a gate that used to hide
84+
something starts showing it, and on the fail-CLOSED surfaces a rule that used to fire
85+
silently stops. Both are accepted costs of the ruling, not oversights — but they need
86+
opposite checks after upgrading, which is why they are listed apart rather than
87+
summarised. Every faulting predicate warns on the console; the app-shell diagnostic
88+
names the roots this tier really binds, and objectui#8155's follow-up removed `app`
89+
from that list so it no longer sends an author back to the root that is the reason
90+
(`packages/react/src/utils/visibilityDiagnostic.ts`).
7091

7192
`ExpressionProvider` still accepts an `app` prop and still publishes `app` on its React
7293
**context value**, which components read as a plain value (`DashboardView` does). Only
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
'@object-ui/react': patch
3+
---
4+
5+
The published visibility diagnostic no longer tells an author that `app` is a bound
6+
expression-scope root (objectui#8155 follow-up).
7+
8+
`@object-ui/react`'s `SCOPE_TIER_ADVICE['app-shell']` — the paragraph
9+
`reportUnresolvableVisibilityPredicate` prints in production when a predicate cannot be
10+
evaluated — read "App-shell predicates bind `current_user` … plus `app` and `features`".
11+
objectui#8155 removed `app` from `buildExpressionScope`, so that sentence was printed at
12+
exactly the moment a saved `app.*` predicate faulted, and it answered "why did my
13+
predicate not resolve?" by naming the root that is the reason. The line now names the
14+
five roots the provider really binds: `current_user`, its `user` / `ctx.user` / `os.user`
15+
aliases, and `features`.
16+
17+
**Swept as a class, not as two coordinates.** Every other place in this tree that stated
18+
`app` was a bound expression-scope root is corrected in the same change — the diagnostic
19+
copy and its byte-pin, the ambient-scope docblocks in `@object-ui/react`
20+
(`SchemaRenderer`, `useExpression`), `@object-ui/core` (`ActionRunner.ParamDef.visible`,
21+
`RowPredicateOptions.scope`), `@object-ui/components` (`form.tsx`, `containers.tsx`),
22+
`@object-ui/plugin-detail`, `@object-ui/plugin-form` (docblock and README),
23+
`@object-ui/app-shell` and the console app, plus fourteen test fixtures that transcribed
24+
the old bag with an `app` key. The fixtures in `@object-ui/app-shell` now call
25+
`buildExpressionScope` instead of transcribing it, so that pair cannot drift again.
26+
27+
Nothing that was true before objectui#8155 changed: the `app` prop on
28+
`ExpressionProvider` and the `app` field on its React context value are untouched (they
29+
were never CEL roots), and no root other than `app` was added to or removed from any
30+
message, bag or fixture.
31+
32+
Two release-bound corrections travel with it. The producer-side card objectstack#16420
33+
was closed `not_planned` on 2026-09-07 by the same ruling that removed the root, not left
34+
open; and the consequence of a stale `app.*` predicate is **not** uniformly "fails open" —
35+
it is per surface, and the changeset that carries the removal now states the seven
36+
measured directions instead of one.

‎apps/console/src/components/FormPage.predicateScope.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
111111
*/
112112
function hostScope(positions: string[]) {
113113
const user = { id: 'u1', name: 'Kim', role: 'user', isPlatformAdmin: false, positions };
114-
return { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} };
114+
return { current_user: user, user, ctx: { user }, os: { user }, data: {}, features: {} };
115115
}
116116

117117
/** The SAME principal shape, one admitted by `GATE` and one refused by it. */

‎apps/console/src/components/FormPage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1677,7 +1677,8 @@ export function FormPage({ mode, recordPath }: FormPageProps) {
16771677
const isCreateForm = target.kind !== 'edit';
16781678
/**
16791679
* The host shell's global predicate scope — `current_user` plus the ADR-0068
1680-
* `user` / `ctx.user` / `os.user` aliases, `app`, `data`, `features` — read
1680+
* `user` / `ctx.user` / `os.user` aliases, `data`, `features` (⛔ no `app`:
1681+
* objectui#8155 unbound that root) — read
16811682
* from the `PredicateScopeProvider` an `ExpressionProvider` mounts, and
16821683
* threaded into all three evaluators below (objectui#6110). Same binding the
16831684
* sibling renderer took in #6010, so one authored `visibleWhen` means one

‎packages/app-shell/src/providers/ExpressionProvider.evaluateVisibility.test.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { describe, it, expect } from 'vitest';
1010
import { ExpressionEvaluator } from '@object-ui/core';
11-
import { evaluateVisibility } from './ExpressionProvider';
11+
import { buildExpressionScope, evaluateVisibility } from './ExpressionProvider';
1212

1313
/**
1414
* Regression: nav/area `visible` predicates arrive from the server as
@@ -20,9 +20,14 @@ import { evaluateVisibility } from './ExpressionProvider';
2020
* unimplementable from app metadata.
2121
*/
2222

23+
/**
24+
* The provider's own bag, from the provider's own builder. It used to be a
25+
* hand-written transcription carrying an `app` key; `buildExpressionScope` has
26+
* not bound one since objectui#8155 (ruled 2026-09-07), and a copy is how a
27+
* fixture keeps asserting a root the shipped builder no longer has.
28+
*/
2329
function makeEvaluator(user: Record<string, unknown>) {
24-
const context = { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} };
25-
return new ExpressionEvaluator(context as any);
30+
return new ExpressionEvaluator(buildExpressionScope({ user }) as any);
2631
}
2732

2833
describe('evaluateVisibility', () => {

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,13 @@ export interface ExpressionScopeInput {
9999
*
100100
* The ruling is that the engine's `SCOPE_ROOTS` is the contract and this
101101
* consumer aligns to it, NOT that the engine grows a root to match this
102-
* consumer (option A, objectstack#16420, is explicitly not taken and stays
103-
* open as the record to reopen should a real need for a "current app" root
104-
* ever be measured). ⛔ The other refused route was suppressing the diagnostic
105-
* in `celAuthoring.ts`: that is the lenient-fallback shape AGENTS.md #0.1
106-
* bans.
102+
* consumer. Option A — widen the engine vocabulary — was the producer-side
103+
* card objectstack#16420, and the same ruling CLOSED it `not_planned`
104+
* (2026-09-07T04:16:22Z). Should a real need for a "current app" root ever be
105+
* measured, it is a fresh spec/engine vocabulary widening; there is no open
106+
* record waiting for it. ⛔ The other refused route was suppressing the
107+
* diagnostic in `celAuthoring.ts`: that is the lenient-fallback shape
108+
* AGENTS.md #0.1 bans.
107109
*
108110
* Every root below is one the engine accepts, so the three surfaces — what
109111
* this binds, what the editor advertises, what the linter admits — now agree.
@@ -310,7 +312,9 @@ export function evaluateVisibility(
310312
// NODE tier's advice, telling an author whose nav predicate faulted to
311313
// check `record` and `page.<var>` — two roots the bag built in
312314
// `ExpressionProvider` above does not contain at all — while the identity
313-
// aliases, `app` and `features` that it DOES contain went unnamed.
315+
// aliases and `features` that it DOES contain went unnamed. (`app` was in
316+
// that list until objectui#8155 removed the root; the advice no longer
317+
// names it either.)
314318
'app-shell',
315319
);
316320

‎packages/app-shell/src/providers/ExpressionProvider.visibleFaultDiagnostic.test.ts‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,24 @@ import {
7272
__resetVisibilityPredicateWarnings,
7373
} from '@object-ui/react';
7474
import { hasVisibleNavigationItems } from '@object-ui/layout';
75-
import { evaluateVisibility } from './ExpressionProvider';
75+
import { buildExpressionScope, evaluateVisibility } from './ExpressionProvider';
7676

7777
/** The label this site puts in the reporter's `type` slot — the dedupe-key decision. */
7878
const SURFACE = 'app-shell:visible';
7979

80+
/**
81+
* The provider's bag, taken from the provider's own builder rather than
82+
* transcribed. It was a hand-written copy carrying an `app` key, and a copy is
83+
* how a fixture goes on asserting a root the builder has stopped binding
84+
* (objectui#8155) — `buildExpressionScope` is the single declaration, so the
85+
* cells below measure the shipped bag instead of a snapshot of it.
86+
*/
87+
function makeScope(user: Record<string, unknown> = { id: 'u1', positions: ['worker'] }) {
88+
return buildExpressionScope({ user });
89+
}
90+
8091
function makeEvaluator(user: Record<string, unknown> = { id: 'u1', positions: ['worker'] }) {
81-
const context = { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} };
82-
return new ExpressionEvaluator(context as any);
92+
return new ExpressionEvaluator(makeScope(user) as any);
8393
}
8494

8595
type WarnSpy = { mock: { calls: unknown[][] } };
@@ -321,8 +331,8 @@ describe('objectui#6443 — the rate limit, measured in both directions', () =>
321331
* The card #6443 made visible: once this site started printing, it printed the
322332
* NODE gate's closing paragraph, telling a nav author to check `record` and
323333
* `page.<var>`. The bag `ExpressionProvider` builds is
324-
* `{ current_user, user, ctx: { user }, os: { user }, app, data, features }` —
325-
* it contains neither.
334+
* `{ current_user, user, ctx: { user }, os: { user }, data, features }` — it
335+
* contains neither.
326336
*
327337
* These cells are the END-TO-END half of the pin: the unit matrix in
328338
* `@object-ui/react` proves the two paragraphs differ, and these prove the
@@ -347,19 +357,73 @@ describe('objectui#6487 — the line carries the APP-SHELL tier`s roots', () =>
347357
it('it names the roots this provider really binds — including `features`', () => {
348358
// `features` is the deployment-flag root this provider's own docblock
349359
// documents for exactly this kind of predicate, and it was unnamed.
350-
// Asserted against the bag `makeEvaluator` builds, which is a copy of the
351-
// provider's: every root named below is a key of it.
360+
// Asserted against the bag `makeEvaluator` builds, which IS the provider's
361+
// (`buildExpressionScope`): every root named below is a key of it.
352362
const warn = spyWarn();
353363
const evaluator = makeEvaluator();
354364

355365
evaluateVisibility('nosuchroot6487shellroots.x > 1', evaluator);
356366
const [line] = reports(warn);
357367
expect(line).toBeDefined();
358-
for (const root of ['`current_user`', '`user`', '`ctx.user`', '`os.user`', '`app`', '`features`']) {
368+
for (const root of ['`current_user`', '`user`', '`ctx.user`', '`os.user`', '`features`']) {
359369
expect(line).toContain(root);
360370
}
361371
});
362372

373+
/* ------------------------------------------------------------------------ *
374+
* objectui#8155 — the COUPLING pin. The advice copy lives in
375+
* `@object-ui/react` and the bag lives here, so neither package can pin the
376+
* pair alone; this is the only seat that reaches both. Modelled on the
377+
* three-sided pin PR #8164 left on `ConditionalFormattingEditor.test.tsx`.
378+
* ------------------------------------------------------------------------ */
379+
380+
it('objectui#8155 — the printed advice does not name `app`, and the bag does not bind it', () => {
381+
// Face 1 (copy, `@object-ui/react`) and face 2 (bag, this package) asserted
382+
// in one run against the REAL line this surface prints. Re-adding `app` to
383+
// the diagnostic string reddens the first expect; re-adding it to
384+
// `buildExpressionScope` reddens the second.
385+
const warn = spyWarn();
386+
387+
evaluateVisibility('nosuchroot8155copy.x > 1', makeEvaluator());
388+
const [line] = reports(warn);
389+
expect(line).toBeDefined();
390+
expect(line).not.toContain('`app`');
391+
expect(Object.prototype.hasOwnProperty.call(makeScope(), 'app')).toBe(false);
392+
});
393+
394+
it('objectui#8155 — every root the advice names is a root the bag really binds', () => {
395+
// Face 3, and the one that makes the pair a fence rather than two lists.
396+
// The defect this card cleans up was precisely an advertised root the bag
397+
// did not bind, so the invariant — not the spelling — is what is pinned:
398+
// re-adding `app` to the COPY alone (leaving the bag aligned to the
399+
// engine) reddens here even though the census cell above is the one that
400+
// names it. The reverse case, a root bound but not advertised, is legal
401+
// and deliberate (`data`), so this direction is asserted and not the other.
402+
const warn = spyWarn();
403+
const scope = makeScope();
404+
405+
evaluateVisibility('nosuchroot8155coupling.x > 1', makeEvaluator());
406+
const [line] = reports(warn);
407+
expect(line).toBeDefined();
408+
409+
// Root names as the advice spells them, mapped to the key an author would
410+
// have to be able to name for the advice to be true. `ctx.user` / `os.user`
411+
// are member paths, so the root is the segment before the dot.
412+
const advertised = ['current_user', 'user', 'ctx.user', 'os.user', 'app', 'features', 'record', 'data']
413+
.filter((root) => (line as string).includes(`\`${root}\``))
414+
.map((root) => root.split('.')[0]);
415+
expect(advertised.length).toBeGreaterThan(0); // the line named SOMETHING
416+
for (const root of advertised) {
417+
// `record` is named only by the sentence declaring it ABSENT at this
418+
// tier, which is the one advertised name that must NOT be a key.
419+
if (root === 'record') {
420+
expect(Object.prototype.hasOwnProperty.call(scope, root)).toBe(false);
421+
continue;
422+
}
423+
expect(Object.prototype.hasOwnProperty.call(scope, root)).toBe(true);
424+
}
425+
});
426+
363427
it('CONTROL: the first paragraph is UNCHANGED — it is true on this fail-open surface too', () => {
364428
// Green both ways, deliberately. Only the LAST paragraph is per-tier; the
365429
// "gate did NOT bite" sentence is true on every surface wired to this

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,11 @@ export interface ConditionSubjectVocabulary {
121121
* bound through `evalFieldPredicate`'s `scope` extra.
122122
*
123123
* NOT included, on purpose: `data`, `os`, `app`, `features`, `input`, `vars`,
124-
* `page`. Those are real roots at some surfaces, but this builder never offers
125-
* them, and over-capturing there fails in the WRONG direction — `data.csv` is
124+
* `page`. All but `app` are real roots at some surface — `app` is a root at
125+
* none since objectui#8155 unbound it, and it stays on this list because the
126+
* list is what this builder does not capture, not what exists. This builder
127+
* never offers any of them, and over-capturing there fails in the WRONG
128+
* direction — `data.csv` is
126129
* a plausible literal, and `data` IS bound, so reading it as a reference would
127130
* produce another silently-false predicate instead of a loud one. Which roots
128131
* a mounting surface actually binds is caller-supplied vocabulary

‎packages/app-shell/src/views/metadata-admin/predicate.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,10 @@ export interface PredicateCtx {
259259
* RECORD; metadata-admin edits source metadata and has no record. Leaving it
260260
* unbound is what keeps `record.status` a loud diagnostic here instead of a
261261
* silent false — `predicate.test.ts` pins that.
262-
* - ⛔ `app` / `features`. Renderer-tier, not ADR-0068 identity, and not named
263-
* by the ruling. Binding them as empty objects would turn `app.x` from a loud
264-
* unresolved-root warning into a silent `undefined`.
262+
* - ⛔ `app` / `features`. `features` is renderer-tier, not ADR-0068 identity,
263+
* and not named by the ruling; `app` is not a root at ANY tier since
264+
* objectui#8155 unbound it. Binding either as an empty object would turn
265+
* `app.x` from a loud unresolved-root warning into a silent `undefined`.
265266
*/
266267
export const IDENTITY_ROOTS = ['current_user', 'user', 'ctx', 'os'] as const;
267268

0 commit comments

Comments
 (0)