Skip to content

Move both record:alert gates onto the has()-guarded node visibleWhen, as a CEL envelope (#9167) - #11254

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-9167-visiblewhen-migration
Aug 23, 2026
Merged

Move both record:alert gates onto the has()-guarded node visibleWhen, as a CEL envelope (#9167)#11254
os-zhuang merged 2 commits into
mainfrom
claude/issue-9167-visiblewhen-migration

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#9167

Round 2 of #9167 — the implementation the 2026-08-23 measurement round and the PM ruling below it produced. Steps 2–3 of the card's "What is owed" stay retired: they name properties.visible, whose evaluator has no has() and never will. This is the replacement remedy.

What changed

On both record:alert pages, the predicate moves off properties.visible onto the component-node visibleWhen, has()-guarded, with the visible key deleted in the same edit:

pagegate
examples/app-showcase/src/ui/pages/task-detail.page.tsvisibleWhen: "has(record.status) && record.status == 'in_review'"
packages/platform-objects/src/pages/sys-user.page.tsvisibleWhen: P`has(record.id) && has(record.email_verified) && record.id == ctx.user.id && record.email_verified == false`

Both ruled conditions hold in the landed diff, and both are verifiable on the wire rather than only in the source: across the 34 pages this app serves, the two record:alert nodes carry ['properties','type','visibleWhen'] and noproperties.visible; repo-wide, properties.visible now appears on 0 served nodes.

⚠️ The P on the platform page is not decoration — the first draft of it shipped the exact defect this card exists to prevent

The ruled string was authored literally, as a bare string, on both pages. Measured, that broke the sys_user banner outright — and the wire said why before the browser did:

pageauthoring formvisibleWhen on the wire
showcase_task_detaildefinePage({…}){"dialect":"cel","source":"has(record.status) && …"}
sys_user_detailraw Page object literal"has(record.id) && …"bare

SysUserDetailPage is a raw typed object literal, so nothing runs ExpressionInputSchema's transform over it and whatever is authored reaches the wire verbatim. And the console keeps bare strings on the legacy JS evaluator by design — ExpressionEvaluator.evaluateCondition, at the pin: "Bare strings and ${…} templates stay on the legacy path (back-compat deprecation window); only an explicit { dialect: 'cel' } envelope is rerouted." That evaluator has no has(), the surface is fail-soft, so the gate stopped gating.

Measured on the bare-string draft, in the real console:

runprobeverdictexpected
B2sys_user self, email_verified forced true (15 keys set)VISIBLEhidden ❌
B3another user's record (control: that page's own title, Ada Auditor)VISIBLEhidden ❌

B3 is the decisive one: on someone else's profile record.id == ctx.user.id is plainly false, and the banner showed anyway — the predicate was not being consulted at all.

Isolated to the dialect envelope alone, on one page, one predicate, one rewrite path (metaPatched=1 in both):

runprobeverdict
P1todo task, node visibleWhen rewritten to the bare stringVISIBLE
P2todo task, node visibleWhen rewritten to the envelope, same sourcehidden ✅

P (@objectstack/spec/shared) emits the { dialect: 'cel', source } envelope. After the fix, 8 of 8 served visibleWhen values are envelopes and 0 are bare.

After-state, measured in a real console

Framework built from this branch (Tasks: 71 successful, 71 total), console rebuilt from the repo's pin (✓ @objectstack/console dist ready (49388 KB) from objectui@190fbd01d061, bundle canary + spec-injection checks passed), objectstack dev --ui --seed-admin -p 39167, Playwright 1.62.1 driving chromium-1194 from PLAYWRIGHT_BROWSERS_PATH via an executablePath override (⛔ playwright install not run). Every mutation is a network-response rewrite against the shipped bundle; the authored source is only ever what the server already serves.

Both banners, both polarities, all correct

runpage / rowverdictexpected
A1task in_reviewVISIBLEVISIBLE ✅
A2task todohiddenhidden ✅
B1sys_user self, email_verified=falseVISIBLEVISIBLE ✅
B2sys_user self, email_verified forced truehiddenhidden ✅
B3another user's recordhiddenhidden ✅

The guard actually holds — and is load-bearing

runprobeverdict
G1task in_review, statusstripped from the read (6 keys removed), landed guarded gatehidden ✅
G2same run, gate swapped for the unguarded envelope (5 keys removed)VISIBLE — the fault, fail-soft
G3sys_user self, email_verifiedstripped (2 keys removed), landed guarded gatehidden ✅
G4same run, unguarded envelope (2 keys removed)VISIBLE

G1↔G2 and G3↔G4 are the same page, the same record, the same strip — only the has() guard differs. That is condition (b) measured on the landed gate rather than inherited.

No flash during load

runprobeverdict
I1todo task, landed gate, polled every 150 ms for 8 s (52 samples)never visible — no flash
I2polling control: unmodified in_review task, same pollhidden → VISIBLE at 1458 ms — the poller does see banners

The census the measurement round could not do

The round-1 confidence gap: whether any other page-block renderer also ignores or mis-binds visibleWhen. Read at the pin, then cross-checked in the browser.

Finding: visibleWhen is not a per-block concern at all. It is enforced once, generically, in packages/react/src/SchemaRenderer.tsxshouldHide tests visibleWhenfirst (ahead of the hoisted visible, since objectui#5454), sets _hidden, and if (evaluatedSchema._hidden) return null fires before the registry dispatches to any renderer. A block renderer cannot ignore the gate: it never sees the node. Every path that places an authored page-component node reaches it — RegionContent and FlatContent (renderers/layout/page.tsx), the slot→region synthesizer for record pages (plugin-detail/src/synth/buildDefaultPageSchema.ts, which folds slots.alerts / highlights / details / tabs / discussion into components), and every page:* container's children via renderChildrenSchemaRenderer. toRenderableSchema is a pass-through, so no key is lost on the way.

Cross-checked behaviourally on block types this card does not touch, each with a paired mount:

runblockgateverdict
X1 / X2record:path (showcase task page){cel,'false'} / {cel,'true'}vanishes / present ✅
X3 / X4record:highlights (showcase task page){cel,'false'} / {cel,'true'}vanishes / present ✅
X5b / X6brecord:highlights (platformsys_user page){cel,'false'} / {cel,'true'}vanishes / present ✅

⛔ Nothing was migrated on the strength of this census — a wider migration is a separate card.

What the census did turn up, reported and not acted on:

  1. One authored-node render path bypasses the gate.renderers/complex/data-table.tsx:1974 renders the emptyAction slot by resolving the registry directly — it calls ComponentRegistry.get(node.type) and renders the returned component with the node as its schema prop, never through SchemaRenderer — so a visibleWhen on that node is never evaluated. It is the only such bypass for authored nodes in the tree; the other direct-registry render (action-bar.tsx:300) is the ADR-0089 action face, which gates on visible inside action-button/action-icon by design.
  2. page:tabs item-level visibleWhen is a second evaluator, with a wider binding and no diagnostic.containers.tsx:449 builds its own ExpressionEvaluator that spreads the row flat and binds data to the row, where the node-level gate binds data to the data-source adapter — same key, opposite meanings — and it calls evaluateCondition without the dev throwOnError probe, so a faulting tab predicate is silent where a faulting node predicate warns.
  3. In a production console bundle the node gate's fault is entirely silent.evaluateVisibilityPredicate short-circuits on if (!__DEV__), so the bare-string breakage above produced no console line at all — unlike the properties.visible face, which logs from record-alert.tsx's useCondition path regardless. Round 1's "has" is not a function lines came from that other face.
  4. The declared root list under-states what binds.page.zod.ts:189 names record, current_user and the page-variable root; app-shell/src/providers/ExpressionProvider.tsx puts { current_user, user, ctx: { user }, os: { user }, app, data, features } into the scope — the ADR-0068 aliases. ctx.user.id in the ruled predicate is therefore a first-class binding, matching the sibling resend_verification_email action predicate on the same object, and measured working (B1/B2/B3).

Verification

  • pnpm --filter @objectstack/platform-objects --filter @objectstack/example-showcase run testTest Files 27 passed (27) / Tests 432 passed (432) and Test Files 25 passed (25) / Tests 367 passed (367).
  • Same filter, run typecheckpackages/platform-objects typecheck: Done, examples/app-showcase typecheck: Done.
  • node scripts/pm/dispatch-gates.mjs (no paths passed — it derives the change set itself) named 15 path-matched families plus the convention-triggered pnpm check:i18n. All 16 run, plus pnpm check:nul-bytes; all exit 0, verdict lines quoted in the report on the card.
  • pnpm lint (repo-wide, eslint . --no-inline-config) reports 89 errors, all 89 inside .cache/objectui-190fbd01d061/ — the objectui clone pnpm objectui:build creates locally, gitignored at .gitignore:42 and absent from a CI checkout. Measured rather than argued: pnpm exec eslint . --no-inline-config --ignore-pattern '.cache/**' exits 0 with no output.
  • Gate union run at the final commit, 3d812516a.

Notes for review

  • Both pages ship together. They are one case, identical on every dimension the card measured; the correction "one case, not two" is on the card twice.
  • The showcase page is left on definePage()'s normalization rather than given an explicit P. That transform is the contract, and it is measured working here (its node serves the envelope, A1/A2 correct). The platform page needs P precisely because it is not built that way.
  • Real-user behaviour is unchanged in every polarity measured. The only difference is that a genuine fault becomes loud (CEL names the missing key) instead of silently answering false.

Generated by Claude Code

…onto the has()-guarded node visibleWhen (#9167)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
…e so has() actually runs (#9167)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 2 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e7d2cc67fdef7fee9d2c6d65d7363fe1c78ce6a4packageMentionDocs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 23, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 23, 2026 07:05
@os-zhuang
os-zhuang added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 064d484Aug 23, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-9167-visiblewhen-migration branch August 23, 2026 07:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

2 participants

@os-zhuang@claude