Skip to content

fix(console): evaluate visibleWhen/visibleOn in FormPage - #5624

Merged
os-sales merged 1 commit into
mainfrom
claude/issue-5594-formpage-visible-predicates
Aug 21, 2026
Merged

fix(console): evaluate visibleWhen/visibleOn in FormPage#5624
os-sales merged 1 commit into
mainfrom
claude/issue-5594-formpage-visible-predicates

Conversation

@os-sales

@os-salesos-sales commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Fixes#5594

Summary

apps/console/src/components/FormPage.tsx is a second, independent form renderer
its own buildSections, its own JSX — and it serves both the public /f/:slug route and
the internal /forms/:name route. It read neither spelling of the FormView field
visibility predicate, so a field an author conditioned on record.priority == 'urgent'
rendered unconditionally on both routes. Fail-open and silent: the author saw the field
always, with no diagnostic.

This wires the predicate into this renderer and pins it with a regression test that
lives with this renderer. It does not touch the other one.

Why the gap survived

Issue #2212 recorded exactly this symptom, and PR #2214 landed the repair on a
different chain: ModalFormresolveFormViewLayout@object-ui/plugin-form
sectionFields.ts@object-ui/componentsrenderers/form/form.tsx. FormPage.tsx is
on that chain at no point, and that PR's regression pin lives with the chain it repaired,
so nothing in the suite could see this copy. One contract, two implementations, each only
ever checked against itself.

The ruling applied, verbatim

Per the PM ruling on the card, this inherits #2212's ruling rather than inventing a
second predicate semantics — two form renderers disagreeing about what visibleWhen
means would be a worse defect than one renderer ignoring it.

Helper signature verified in the built dist/*.d.ts the console's tsc actually
reads
, not from memory (packages/core/dist/evaluator/fieldRules.d.ts:63):

exportdeclarefunctionevalFieldPredicate(pred: FieldRulePredicate|undefined|null,record: Record<string,unknown>,fallback: boolean,previous?: Record<string,unknown>,scope?: Record<string,unknown>,diagnostic?: FieldPredicateDiagnostic): boolean;

Reachable from the package root via export * from './evaluator/index.js' in
packages/core/dist/index.d.ts:26.

So, by construction rather than by agreement:

  • EngineevalFieldPredicate (@object-ui/core, evaluator/fieldRules.ts).
  • Wire shapes — bare CEL string and { dialect, source }.
  • Scoperecord.* is the LIVE input values (the predicate re-decides as the user
    types), previous.* is the stored record an edit form started from (type: 'form' action fired from a record opens an empty CREATE form — /forms/:name ignores the ?recordId= ActionRunner forwards #4278). The same
    pair form.tsx binds.
  • Failure — fail-open, loudly: an unevaluable predicate must never HIDE a field,
    because a hidden field is one the submitter can neither fill in nor see is missing.
  • Precedence — canonical-first, visibleWhen ?? visibleOn, matching both sibling
    readers: sectionFields.ts (fd.visibleWhen ?? fd.visibleOn) and app-shell's
    readVisibility. The deprecated ADR-0089 alias stays honoured because it still has
    live producers that never pass through the spec normaliser.

What changed

FileWhat
apps/console/src/components/FormPage.tsxRenderableField carries the predicate (type derived from FormFieldSpec['visibleWhen'], not restated); buildSections resolves it canonical-first; new exported isFieldVisible answers the static flag and the predicate in one verdict; the render filter calls it.
apps/console/src/components/FormPage.visibleWhen.test.tsxNew. The regression pin, next to the renderer it describes.
.changeset/console-formpage-visible-predicates-5594.md@object-ui/console: patch.

Two things deliberately did not change, recorded rather than assumed:

Evidence

The defect probe, and a control at identical scope. A bare grep -c for the two key
names in this file returns 1 on today's main — but that one hit is a prose mention
inside a docblock
, not a read. Scoped to a property read instead:

$ git rev-parse --short HEAD # origin/main
8c87f0583
# PROBE — predicate reads in this renderer
$ grep -cE '\b(f|field|override|sec)\.(visibleWhen|visibleOn)\b' apps/console/src/components/FormPage.tsx
0
# CONTROL — same file, same regex shape, the static flag this renderer DOES read
$ grep -cE '\b(f|field|override|sec)\.(hidden)\b' apps/console/src/components/FormPage.tsx
2

The control returns non-zero at identical scope, so the 0 is a missing read and not a
bad search.

Reverse verification — measured, not predicted.FormPage.tsx reverted to its pre-fix
state with the new pin left in place (mutation confirmed on disk by anchored greps on the
text being changed, in both legs; the script carries a trap … EXIT INT TERM restore, and
the tree was verified clean afterwards). No build leg is involved: the test imports the
subject as a relative source path (./FormPage), not through a package exportsdist.

Tests 11 failed | 1 passed (12)

The one green is the control that has to be green — "a field with NO predicate at all
still renders"
— without which every "the field is absent" assertion would be equally
satisfied by a renderer that draws nothing at all. The two cases named CONTROL are red
here because each must first establish that the field IS hidden; their control halves
(fail-open direction, submitted payload) are unaffected. The full per-test breakdown is in
the test file's docblock.

Gates

All run after the final commit, from the repo root (package-cwd vitest is refused by
the repo's own guard, #3378), at 93187c334 with a clean worktree:

GateResult
npx vitest run apps/console --maxWorkers=2Test Files 66 passed (66) · Tests 722 passed (722)
pnpm --filter @object-ui/console type-checkexit 0 (tsc --noEmit && tsc -b tsconfig.node.json --force, script name echoed — not a zero-match no-op)
pnpm --filter @object-ui/console lintexit 0 · 201 problems (0 errors, 201 warnings), all pre-existing no-explicit-any; the two touched files contribute 0 errors, and the new test file 0 warnings
check:control-bytesOK (scanned 4673 tracked text file(s); skipped 85 binary)
check-changeset-presence2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)
check-changeset-fixed / check-changeset-no-majorAll workspace packages are in the changeset fixed group. / No changeset declares a major bump.
check:phantom-depsEvery in-scope import is declared by the package that publishes it.
check:lint-coverage46/46 packages linted, 0 with outstanding errors
check:type-check-coverage45/46 via type-check, 0 known-broken
check:eager-closureexit 2 — broken gauge, not a failure: No eager-closure report at apps/console/dist/eager-closure.json … This is a broken gauge, not a passing budget. It needs a console vite build to write the report. Noted, not "repaired". The one new import is evalFieldPredicate from @object-ui/core, a package already eager in this bundle (55 files under packages/app-shell/src import it, and App.tsx imports app-shell statically), so no new package enters the eager closure.

No packages/** file is touched, so the "does dist/*.d.ts move?" question does not arise
for this change — the whole diff is apps/console plus one changeset.

Follow-up filed, not fixed here — #5627

FormPage still drops the other conditional-rule surfaces: section-level
visibleWhen/visibleOn, and the object-level field rules
visibleWhen/readonlyWhen/requiredWhen that the sibling chain copies out of the object
schema. Same class, different keys and different user-visible consequences (disabled state
and submit-blocking, not just hiding), and it raises two semantics questions of its own, so
it is filed rather than smuggled in here.


Generated by Claude Code

`apps/console/src/components/FormPage.tsx` is a second, independent form
renderer — it serves both `/f/:slug` (public, anonymous) and `/forms/:name`
(internal) — and it read neither spelling of the FormView field visibility
predicate. A field conditioned on `record.priority == 'urgent'` rendered
unconditionally on both routes: fail-open and silent.
objectui#2212 recorded the same symptom and PR #2214 fixed it on the OTHER
chain (ModalForm -> resolveFormViewLayout -> plugin-form sectionFields.ts ->
components renderers/form/form.tsx), which this file is on at no point, and
that fix's regression pin lives with that chain.
Applies #2212's ruling verbatim rather than inventing a second predicate
semantics: the predicate is routed through the canonical engine,
`evalFieldPredicate` (@object-ui/core, evaluator/fieldRules.ts), with the same
`record.`/`previous.` scope, the same accepted wire shapes (bare CEL string and
`{ dialect, source }`), and the same fail-open-but-loud behaviour. Resolution
is canonical-first — `visibleWhen ?? visibleOn` — matching plugin-form's
`sectionFields.ts` and app-shell's `readVisibility`.
The regression pin lives with THIS renderer, next to the copy it describes: a
pin that cannot see the second copy is how the first gap survived.
Out of scope by triage fence: FormPage is not folded onto the plugin-form
chain. The second-renderer convergence question stays with the #5596 track.
Fixes#5594
Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
@github-actions

Copy link
Copy Markdown
Contributor

✅ Console Performance Budget

MetricValueBudget
Eager closure (gzip, 52 chunks)3785.8 KB3867.2 KB
Main entry chunk (gzip)151.6 KB350 KB
Entry fileindex-CHAzgaqo.js
StatusPASS

The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it.


📦 Bundle Size Report

PackageSizeGzipped
app-shell (index.js)10.04KB3.72KB
app-shell (runtime-config.js)8.91KB2.99KB
app-shell (types.js)0.01KB0.04KB
app-shell (urlParams.js)10.06KB3.86KB
auth (AuthContext.js)0.31KB0.24KB
auth (AuthGuard.js)1.17KB0.53KB
auth (AuthProvider.js)29.34KB7.05KB
auth (AuthShell.js)3.49KB1.40KB
auth (ForgotPasswordForm.js)12.21KB3.45KB
auth (LoginForm.js)18.15KB5.39KB
auth (PreviewBanner.js)0.90KB0.50KB
auth (RegisterForm.js)6.65KB2.22KB
auth (SocialSignInButtons.js)9.61KB3.89KB
auth (UserMenu.js)3.41KB1.23KB
auth (auth-gate-events.js)1.29KB0.66KB
auth (authStyles.js)5.04KB1.72KB
auth (createAuthClient.js)40.21KB10.80KB
auth (createAuthenticatedFetch.js)6.35KB2.43KB
auth (index.js)2.77KB1.22KB
auth (invitation-status.js)1.22KB0.70KB
auth (org-roles.js)6.66KB2.78KB
auth (phone-identifier.js)1.11KB0.66KB
auth (types.js)0.59KB0.35KB
auth (useAuth.js)5.02KB0.89KB
auth (useIsWorkspaceAdmin.js)3.04KB1.45KB
collaboration (CommentThread.js)26.08KB7.56KB
collaboration (LiveCursors.js)3.17KB1.27KB
collaboration (PresenceAvatars.js)6.49KB2.64KB
collaboration (PresenceProvider.js)2.79KB1.13KB
collaboration (index.js)1.68KB0.73KB
collaboration (useCollaborationTranslation.js)6.05KB2.52KB
collaboration (useCommentSearch.js)1.98KB0.88KB
collaboration (useConflictResolution.js)7.75KB1.86KB
collaboration (useMentionNotifications.js)1.81KB0.68KB
collaboration (usePresence.js)6.33KB1.84KB
collaboration (useRealtimeSubscription.js)7.91KB2.01KB
components (index.js)506.99KB113.73KB
core (index.js)4.51KB1.80KB
create-plugin (index.js)10.08KB3.26KB
data-objectstack (index.js)159.80KB44.33KB
fields (index.js)238.85KB60.13KB
i18n (LocalizationContext.js)1.76KB0.96KB
i18n (currency.js)1.22KB0.64KB
i18n (i18n.js)4.28KB1.75KB
i18n (index.js)3.44KB1.39KB
i18n (pickLocalized.js)7.22KB3.08KB
i18n (provider.js)23.13KB7.63KB
i18n (useDisplayLocale.js)2.85KB1.45KB
i18n (useObjectLabel.js)33.40KB8.71KB
i18n (useSafeTranslation.js)7.77KB3.13KB
layout (index.js)38.95KB10.97KB
mobile (MobileProvider.js)0.92KB0.49KB
mobile (ResponsiveContainer.js)0.94KB0.38KB
mobile (breakpoints.js)1.51KB0.70KB
mobile (createOfflineDataSource.js)5.61KB1.75KB
mobile (index.js)1.55KB0.62KB
mobile (offlineQueue.js)3.91KB1.35KB
mobile (pwa.js)0.97KB0.49KB
mobile (serviceWorker.js)1.48KB0.62KB
mobile (serviceWorkerSource.js)3.41KB1.48KB
mobile (useBreakpoint.js)1.54KB0.65KB
mobile (useGesture.js)6.96KB1.98KB
mobile (useOfflineSync.js)1.99KB0.72KB
mobile (usePullToRefresh.js)2.53KB0.85KB
mobile (useResponsive.js)0.72KB0.42KB
mobile (useResponsiveConfig.js)1.37KB0.63KB
mobile (useSpecGesture.js)4.32KB1.64KB
mobile (useTouchTarget.js)1.01KB0.54KB
permissions (MePermissionsProvider.js)9.35KB3.31KB
permissions (PermissionContext.js)0.31KB0.25KB
permissions (PermissionGuard.js)0.89KB0.45KB
permissions (PermissionProvider.js)4.42KB1.42KB
permissions (evaluator.js)5.12KB1.74KB
permissions (index.js)0.93KB0.41KB
permissions (store.js)0.91KB0.42KB
permissions (useFieldPermissions.js)1.28KB0.53KB
permissions (usePermissions.js)1.81KB0.83KB
plugin-ai (index.js)15.75KB3.80KB
plugin-calendar (index.js)46.62KB12.83KB
plugin-charts (index.js)64.72KB18.35KB
plugin-chatbot (index.js)181.41KB43.22KB
plugin-dashboard (index.js)128.36KB32.95KB
plugin-designer (index.js)212.30KB42.80KB
plugin-detail (index.js)242.15KB60.89KB
plugin-editor (index.js)2.46KB1.10KB
plugin-form (index.js)125.07KB30.43KB
plugin-gantt (index.js)164.10KB39.87KB
plugin-grid (index.js)200.79KB54.26KB
plugin-kanban (index.js)52.93KB14.60KB
plugin-list (index.js)111.70KB27.17KB
plugin-map (index.js)20.06KB6.62KB
plugin-markdown (index.js)13.72KB4.69KB
plugin-report (index.js)43.49KB11.93KB
plugin-timeline (index.js)26.68KB7.66KB
plugin-tree (index.js)8.50KB2.88KB
plugin-view (index.js)84.50KB20.68KB
providers (DataSourceProvider.js)0.75KB0.39KB
providers (MetadataProvider.js)1.37KB0.59KB
providers (ThemeProvider.js)1.90KB0.85KB
providers (UploadProvider.js)11.66KB3.50KB
providers (index.js)0.45KB0.23KB
providers (types.js)0.01KB0.04KB
react-runtime (index.js)5.62KB2.34KB
react (LazyPluginLoader.js)3.77KB1.33KB
react (SchemaRenderer.js)43.66KB14.77KB
react (data-invalidation.js)5.05KB2.08KB
react (index.js)1.33KB0.69KB
react (schema-input.js)2.32KB1.24KB
react (spec-input.js)0.20KB0.18KB
sdui-parser (codegen.js)5.41KB2.34KB
sdui-parser (index.js)4.77KB2.16KB
sdui-parser (input-type.js)2.84KB1.40KB
sdui-parser (parse.js)10.76KB3.17KB
sdui-parser (provenance.js)3.66KB1.82KB
sdui-parser (types.js)0.29KB0.24KB
sdui-parser (validate.js)6.92KB2.40KB
types (ai.js)0.20KB0.17KB
types (api-types.js)0.20KB0.18KB
types (app.js)2.87KB0.99KB
types (base.js)0.20KB0.18KB
types (blocks.js)0.20KB0.18KB
types (complex.js)0.20KB0.18KB
types (crud.js)0.20KB0.18KB
types (dashboard-filter-alias.js)6.23KB2.74KB
types (data-display.js)0.20KB0.18KB
types (data-protocol.js)0.20KB0.19KB
types (data.js)0.20KB0.18KB
types (designer.js)1.87KB0.85KB
types (disclosure.js)0.20KB0.18KB
types (error-code.js)1.54KB0.88KB
types (feedback.js)0.20KB0.18KB
types (field-types.js)0.20KB0.18KB
types (form.js)0.20KB0.18KB
types (http-retry.js)4.32KB2.02KB
types (index.js)3.08KB1.53KB
types (layout.js)0.20KB0.18KB
types (managed-by.js)0.19KB0.18KB
types (mobile.js)2.59KB1.31KB
types (navigation.js)0.20KB0.18KB
types (objectql.js)0.20KB0.18KB
types (overlay.js)0.20KB0.18KB
types (permissions.js)0.20KB0.18KB
types (plugin-scope.js)0.20KB0.18KB
types (record-components.js)0.20KB0.19KB
types (record-semantics.js)1.28KB0.67KB
types (registry.js)0.20KB0.18KB
types (reports.js)0.20KB0.18KB
types (spec-report.js)5.05KB1.93KB
types (system-fields.js)3.33KB1.54KB
types (theme.js)0.20KB0.18KB
types (ui-action.js)3.40KB1.71KB
types (views.js)0.20KB0.18KB
types (widget.js)0.20KB0.18KB

Size Limits

  • ✅ Core packages should be < 50KB gzipped
  • ✅ Component packages should be < 100KB gzipped
  • ⚠️ Plugin packages should be < 150KB gzipped

@os-salesClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — domain:ui seat review, round 8

Gate read by name on 93187c334: 19 success + the 3 always-skipped no-ops = 22. Shards finished 19:23:40–19:24:56Z.

The one thing I checked myself rather than take from the report

Does the new export function isFieldVisible widen a published surface? It sits in a released package (@object-ui/console@17.6.0, publishConfig.access: "public"), so the question is real and it is a clause-② question — which matters here because CONTRACT_REVIEW_TIER is exhausted for this seat and a genuine surface widening could not have been landed today.

Read from apps/console/package.json, not inferred:

  • exports has exactly one entry — "."./plugin.js / ./plugin.d.ts.
  • files is ["dist", "plugin.ts", "plugin.js", "plugin.d.ts", "README.md"].

src/ is neither shipped nor reachable through the package's exports. The published surface is the Hono plugin entry plus the pre-built SPA. So this export is intra-app, consumed only by the sibling test. Clause ② is not tripped and the export is the right shape for a testable pure verdict.

That check is the #5542 lesson applied: a reuse or export target needs a reachability answer, not a plausibility one.

What the review confirmed

The engine choice is the substance, and it is right. Routing through evalFieldPredicate rather than re-deriving predicate semantics is what keeps this from being the actual disaster available here — two form renderers that both "support visibleWhen" and disagree about what it means. Verifying the signature out of the built dist/*.d.ts that apps/console's tsc actually resolves (rather than from source or memory) is the correct place to read it, since apps/console has no path alias and resolves through node_modules.

The card's own probe had gone stale, and the dev said so. The card quoted grep -c → 0; on today's main that bare grep returns 1, and the hit is prose inside a docblock #5542 added. Re-scoped to a property read it is still 0, with a same-scope control (hidden) returning 2. The defect is unchanged; the card's evidence was stale. That is the third round-8 card whose cited evidence had drifted — the discipline of re-deriving rather than re-running keeps paying.

The test docblock was corrected to the measured result. The first draft predicted two cases as green-both-sides; the measurement falsified it and the docblock was rewritten to match. Recording the measured outcome over the predicted one is the whole point of the ablation.

The fence, and the answer it invited

The fence forbade the fold and invited a report if the dev became convinced otherwise. It did not, and the reasoning is substantive enough that I am carrying it to #5596 rather than leaving it in a PR body: FormPage owns a loader/submit surface the plugin-form chain has no concept of — anonymous /forms/:slug resolution, ?recordId= / ?recordObject= create-vs-edit-vs-refuse (#4278 / #4292), submitBehavior including the ruled relative redirect (#4190 / objectstack#7496), and the ExpandedViewItem unwrap (#2208) — each with its own landed ruling and pins.

The load-bearing half: the shared piece is now the canonical engine, which is the part that actually has to agree. Closing the predicate gap did not require the fold and does not make it harder. What the fold buys is one renderer instead of two — a scope and sequencing question, not a side effect of a bug fix. That reframes #5596 from "these two disagree" to "these two now agree on semantics and differ on surface", which is a better question to decide.

#5627 — correct not to fold

Three more conditional-rule surfaces this renderer still drops (section-level visibleWhen/visibleOn, and object-level visibleWhen/readonlyWhen/requiredWhen). Rightly refused the bounded-in-place-fix exemption: it widens the honoured surface into disabled state and submit-blocking, and raises two real semantics questions — whether a predicate-hidden section should suppress its fields' values on submit, and whether requiredWhen needs resolveFieldRuleState's serverOwnedValue carve-out on a route serving both create and edit. Neither is mechanical; neither shape is pinned by existing evidence. Labelled finding + domain:ui; grading is triage's.

A PM error to record

My dispatch's report schema conflicts with the standing dev contract. I asked for {files_changed, what_changed, evidence, changeset, tests, gates, scope_fence_respected, open_questions} while the standing contract requires {issue, status, branch, pr, premise_still_valid, summary, tests, open_questions, out_of_scope_findings}. The dev produced the superset and flagged the conflict rather than silently picking a side, which is the right handling — but the conflict is mine to stop creating. Future dispatches in this lane will extend the standing contract rather than restate a competing one.

Also noted: the earlier notes in this PR's report about a GitHub body sanitizer were wrong and the dev corrected them itself, restoring the description to the true literal signature and rebuilding from local source rather than round-tripping through the sanitizing reader — which is exactly right, since a round-trip is what would have double-escaped the quoted gate output.

Not counted as evidence

The bundle bot's PASS, and the check_suite.completed bursts.

Landing now.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants

@os-sales@claude