From 101d8bdbcc173747465a8b02392162f7218ded3c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 13:54:59 +0000 Subject: [PATCH 1/2] Honour per-option visibleWhen in the metadata-admin renderer (#6247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SelectOptionSchema declares a per-option `visibleWhen` (ADR-0068) and is z.core.$strict, so a *.form.ts carrying one parses clean — but all three metadata-admin controls that consume `fieldSpec.options` mapped the authored list straight to items and never read the key. Accepted, stored, shipped, ignored: ADR-0049's declared-but-unenforced shape, failing in the permissive direction (the option stayed offered). Per the maintainer ruling A2 + B1 + C1: - A2: bind the four ADR-0068 D1 identity spellings alongside `data`, selected out of the host ExpressionProvider's bag rather than copied. `data` stays the draft — the provider's conflicting `data` key is NOT adopted. `record`/`app`/ `features` stay unbound so they keep raising the loud diagnostic, which now reads the bound names off the actual scope instead of claiming "the only name is `data`". No third evaluator: the filter routes through evaluatePredicate. - B1: face decisions keep reading the RAW option list; only the rendered list is filtered, so an emptied set is an empty picker — never the free-text degradation, never a different widget registration. - C1: no pruning; MultiSelectWidget's toggle re-orders against the raw list so a hidden-but-selected value survives. FormFieldSpec.options now derives from the spec's SelectOption with its narrowings named in an Omit, instead of hand-mirroring three of five keys. A pin per consuming control, each asserting an option ABSENT on a false predicate — the positive case cannot distinguish fail-open from a fix. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6247-option-visiblewhen-metadata-admin.md | 77 ++++ .../SchemaForm.optionVisibleWhen.test.tsx | 350 ++++++++++++++++++ .../src/views/metadata-admin/SchemaForm.tsx | 42 ++- .../src/views/metadata-admin/form-spec.ts | 46 ++- .../src/views/metadata-admin/predicate.ts | 141 ++++++- .../src/views/metadata-admin/widgets.tsx | 91 ++++- 6 files changed, 710 insertions(+), 37 deletions(-) create mode 100644 .changeset/6247-option-visiblewhen-metadata-admin.md create mode 100644 packages/app-shell/src/views/metadata-admin/SchemaForm.optionVisibleWhen.test.tsx diff --git a/.changeset/6247-option-visiblewhen-metadata-admin.md b/.changeset/6247-option-visiblewhen-metadata-admin.md new file mode 100644 index 0000000000..7466d7e936 --- /dev/null +++ b/.changeset/6247-option-visiblewhen-metadata-admin.md @@ -0,0 +1,77 @@ +--- +'@object-ui/app-shell': patch +--- + +A per-option `visibleWhen` written into a metadata form now actually withdraws that option +in the metadata-admin renderer (objectui#6247). + +`SelectOptionSchema` declares `visibleWhen` (ADR-0068 / objectui#2284) and the schema is +`z.core.$strict` — an undeclared sibling key is refused with `unrecognized_keys` — so the +key is a real declaration, and a `*.form.ts` carrying a per-option predicate parses clean. +The metadata-admin renderer never read it: all three controls that consume +`fieldSpec.options` mapped the authored list straight to items. Accepted, stored, shipped, +ignored — ADR-0049's declared-but-unenforced shape. + +**This is a behaviour change, and it is the point of the fix.** Metadata that was inert +starts hiding options. An author who wrote a narrowing and got no narrowing now gets one. +Both of the old failure modes pointed the same way — the option stayed offered — so +nothing that relied on the old behaviour was relying on a narrowing being honoured; it was +relying on one being ignored. Zero `*.form.ts` in the tree uses the key today, so no +shipped form changes shape on this release. + +Three consuming controls now filter, and each has a pin asserting an option **absent** on a +false predicate: the builtin Select in `SchemaForm.tsx`, `MultiSelectWidget`, and +`colorPaletteOptions` → `ColorSwatchGroupWidget`. Asserting absence is the whole of the +test design — this evaluator fails OPEN, so "the option is shown" is equally the outcome of +predicate-true, predicate-never-arrived and predicate-faulted, and a positive-only pin +passes against the unfixed renderer. + +Per the maintainer ruling on the card (affirmed three times: 2026-08-25 batch 4; +2026-08-25 upholding A2 over the A1 counter-proposal; 2026-08-27 declining the A0 +reject-the-key alternative): + +- **Scope (A2).** `SchemaForm`'s `evaluatePredicate` ctx now binds the four ADR-0068 D1 + identity spellings — `current_user`, `user`, `ctx.user`, `os.user` — **alongside** `data`, + selected out of the host `ExpressionProvider`'s bag rather than copied, so the alias set + cannot drift from the one builder. `data` stays the **draft**: the provider's bag also + carries a `data` key meaning its own data scope, and adopting that would be + objectui#5926 gap 2's "same key, opposite meanings" one nesting level apart. `record`, + `app` and `features` are deliberately left **unbound** so they keep raising the loud + warn-once diagnostic instead of resolving to a silent `undefined`. That diagnostic now + reads the bound names off the actual scope instead of asserting "the only name is + `data`", which stopped being true the moment identity was bound — a diagnostic that lies + sends the author to un-write a spelling that was correct. No new evaluator: the option + filter routes through the same `evaluatePredicate` the section, field and repeater-row + gates already use. +- **Emptied sets (B1).** The control's **face** keeps reading the **raw** option list — + `resolveFieldFace`'s `hasOptions`, `resolveColorWidgetKey`, and each `options.length > 0` + branch condition — and only the rendered list is filtered. Withdrawing every option + renders an **empty picker**. Filtering the branch condition instead would have degraded + the builtin Select to a free-text `Input` and `MultiSelectWidget` to its comma-tag + editor — "withdraw every option" displayed as "type whatever you like" — and would have + flipped a fully-withdrawn palette from the `color-picker` registration to `color-input`, + making the labelling channel objectui#4871 point 4 fixes in the host predicate-dependent. +- **Stored values (C1).** No pruning. A selected value whose option is now hidden survives + in the stored metadata and the picker shows the placeholder. This includes the quiet path: + `MultiSelectWidget`'s toggle re-orders the selection against the **raw** list, because + ordering against the filtered one would have dropped a hidden-but-selected value on the + next unrelated click. This renderer edits **source metadata**, and objectui#4247's own + reasoning — "missing information is not a reason to destroy stored data" — applies harder + here than it did on the runtime record surface. + +`FormFieldSpec.options` is also no longer hand-written. It derives from the spec's +`SelectOption` with its narrowings named in an `Omit`, per this file's own convention: +`visibleWhen` is re-pointed to the local `VisibilityPredicate` (`dialect` optional, `source` +required — the shape an evaluator here actually takes), and `default` stays dropped **and +now says so**, because nothing on this surface reads it. Two of the spec's five option keys +had been dropped by silence, which is precisely how a legally-authored per-option +`visibleWhen` came to parse clean and render inert. + +**Not fixed here, and it bounds what this change delivers:** this interim evaluator's `in` +operator requires an array **literal** on the right (`role in ['admin','owner']`), so the +ADR's headline spelling `'admin' in current_user.positions` — membership against a *path* — +still falls through to the bare-truthy branch and evaluates TRUE regardless of the user, +silently. That is a pre-existing grammar gap of the whole subset, identical for `data.*` +(`'x' in data.tags` is equally inert) and unchanged by this card; it is filed separately. +The spellings this fix makes genuinely discriminate are the documented subset — +`path == literal`, `path != literal`, `path in [literals]`, `!path`, `path`, `&&`, `||`. diff --git a/packages/app-shell/src/views/metadata-admin/SchemaForm.optionVisibleWhen.test.tsx b/packages/app-shell/src/views/metadata-admin/SchemaForm.optionVisibleWhen.test.tsx new file mode 100644 index 0000000000..6281f532ae --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/SchemaForm.optionVisibleWhen.test.tsx @@ -0,0 +1,350 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { SchemaForm } from './SchemaForm'; +import { WIDGETS, resolveColorWidgetKey } from './widgets'; +import { ExpressionProvider } from '../../providers/ExpressionProvider'; +import { buildPredicateCtx, IDENTITY_ROOTS, resetPredicateWarnings } from './predicate'; + +afterEach(() => { + cleanup(); + resetPredicateWarnings(); +}); + +/** + * objectui#6247 — a per-option `visibleWhen` written into a `*.form.ts` is + * honoured by the metadata-admin renderer. + * + * `SelectOptionSchema` declares `visibleWhen` (ADR-0068 / objectui#2284) and is + * `z.core.$strict`, so the key is a real declaration and a metadata form + * carrying it parses clean. Until this fix the three controls that consume + * `fieldSpec.options` mapped the list straight to items and never read the key: + * accepted, stored, shipped, inert — ADR-0049's declared≠enforced. + * + * ## ⚠️ Why every pin here asserts an option ABSENT + * + * This evaluator fails OPEN by design (objectstack#6936): a parse error, an + * unresolvable root, a predicate that never ran at all — every one of them + * yields `true`. So "the option is shown" is the outcome of predicate-TRUE, + * predicate-never-arrived AND predicate-faulted, and a pin that only checks the + * true case distinguishes none of them; it passes just as happily against the + * unfixed renderer. The load-bearing assertion is therefore always that an + * option gated by a FALSE predicate is **not in the document**, with the + * shown-when-true case kept only as the control. + * + * Maintainer ruling, affirmed three times (2026-08-25 batch 4 / 2026-08-25 + * upholding A2 over the A1 counter-proposal / 2026-08-27 declining A0): + * **A2 + B1 + C1**, which is what the three sections below pin. + */ + +/* ── shared fixtures ─────────────────────────────────────────────────────── */ + +const ADMIN = { id: 'u1', name: 'Ada', positions: ['admin'], role: 'admin' }; +const VIEWER = { id: 'u2', name: 'Bob', positions: ['viewer'], role: 'viewer' }; + +const selectSchema = { + type: 'object', + properties: { + tier: { type: 'string', title: 'Tier' }, + plan: { type: 'string', title: 'Plan' }, + }, +}; + +/** A form spec whose `plan` options carry per-option predicates. */ +const planForm = (options: Array>) => ({ + type: 'default' as const, + sections: [ + { + label: 'Billing', + fields: [{ field: 'tier', label: 'Tier' }, { field: 'plan', label: 'Plan', options }], + }, + ], +}); + +/** Open a Radix Select trigger the way the other suites in this repo do. */ +async function openSelect(name: string) { + const trigger = screen.getByRole('combobox', { name }); + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); + await waitFor(() => expect(screen.queryAllByRole('option').length).toBeGreaterThan(0)); +} + +/* ── control 1: the builtin Select (SchemaForm.tsx) ──────────────────────── */ + +describe('consuming control 1 — builtin Select honours per-option visibleWhen', () => { + it('⭐ withdraws the option whose predicate is FALSE against the draft', async () => { + render( + {}} + />, + ); + await openSelect('Plan'); + // The control: an option with no predicate is always offered. + expect(screen.getByRole('option', { name: 'Free' })).toBeInTheDocument(); + // ⭐ The load-bearing assertion — fail-open makes the positive case blind. + expect(screen.queryByRole('option', { name: 'Enterprise' })).toBeNull(); + }); + + it('offers the same option once the draft satisfies the predicate', async () => { + render( + {}} + />, + ); + await openSelect('Plan'); + expect(screen.getByRole('option', { name: 'Enterprise' })).toBeInTheDocument(); + }); + + it('⭐ FORK A / A2 — an identity gate discriminates under an ExpressionProvider', async () => { + const view = (user: Record) => ( + + {}} + /> + + ); + + // ⭐ The security case the ruling exists for: an admin-only option must NOT + // render for a viewer. Before `current_user` was bound this root was + // unresolvable, and an unresolvable root fails OPEN — so this option + // rendered for everybody. + const { unmount } = render(view(VIEWER)); + await openSelect('Plan'); + expect(screen.queryByRole('option', { name: 'Danger Zone' })).toBeNull(); + unmount(); + cleanup(); + + render(view(ADMIN)); + await openSelect('Plan'); + expect(screen.getByRole('option', { name: 'Danger Zone' })).toBeInTheDocument(); + }); + + it('⭐ FORK B / B1 — withdrawing EVERY option renders an empty picker, not a text box', async () => { + render( + {}} + />, + ); + // The FACE is decided on the RAW list, so the control is still a combobox. + // Were the branch condition to read the filtered list, this field would fall + // through to `string → Input` and render a FREE-TEXT box — "withdraw every + // option" displayed as "type whatever you like". + expect(screen.getByRole('combobox', { name: 'Plan' })).toBeInTheDocument(); + expect(screen.queryByRole('textbox', { name: 'Plan' })).toBeNull(); + fireEvent.keyDown(screen.getByRole('combobox', { name: 'Plan' }), { key: 'ArrowDown' }); + await waitFor(() => expect(screen.queryByRole('option', { name: 'Free' })).toBeNull()); + expect(screen.queryByRole('option', { name: 'Enterprise' })).toBeNull(); + }); +}); + +/* ── control 2: MultiSelectWidget (widgets.tsx) ──────────────────────────── */ + +describe('consuming control 2 — MultiSelectWidget honours per-option visibleWhen', () => { + const MultiSelect = WIDGETS['multiselect']; + const opts = [ + { label: 'Grid', value: 'grid' }, + { label: 'Kanban', value: 'kanban', visibleWhen: "data.mode == 'advanced'" }, + ]; + + it('⭐ withdraws the option whose predicate is FALSE against the draft', () => { + render( + {}} + schema={{ type: 'array', items: { type: 'string' } }} + fieldSpec={{ field: 'viz', options: opts }} + formData={{ mode: 'basic' }} + />, + ); + expect(screen.getByRole('checkbox', { name: 'Grid' })).toBeInTheDocument(); + // ⭐ The load-bearing assertion. + expect(screen.queryByRole('checkbox', { name: 'Kanban' })).toBeNull(); + }); + + it('offers the same option once the draft satisfies the predicate', () => { + render( + {}} + schema={{ type: 'array', items: { type: 'string' } }} + fieldSpec={{ field: 'viz', options: opts }} + formData={{ mode: 'advanced' }} + />, + ); + expect(screen.getByRole('checkbox', { name: 'Kanban' })).toBeInTheDocument(); + }); + + it('⭐ FORK B / B1 — withdrawing every option keeps the group, not the tag editor', () => { + render( + {}} + schema={{ type: 'array', items: { type: 'string' } }} + fieldSpec={{ + field: 'viz', + options: [ + { label: 'Grid', value: 'grid', visibleWhen: "data.mode == 'advanced'" }, + { label: 'Kanban', value: 'kanban', visibleWhen: "data.mode == 'advanced'" }, + ], + }} + formData={{ mode: 'basic' }} + ariaLabelledBy="lbl" + />, + ); + // Still this widget's own group; the comma-tag free-text editor is the + // degradation B1 forbids, and it would be a `textbox`. + expect(screen.getByRole('group')).toBeInTheDocument(); + expect(screen.queryByRole('textbox')).toBeNull(); + expect(screen.queryAllByRole('checkbox')).toHaveLength(0); + }); + + it('⭐ FORK C / C1 — a selected value whose option is hidden SURVIVES a toggle', () => { + const onChange = vi.fn(); + render( + , + ); + // `kanban` is stored but currently withdrawn. Clicking an unrelated option + // must not silently prune it — this renderer edits SOURCE metadata, and + // C1 rules that a hidden-but-selected value survives. + expect(screen.queryByRole('checkbox', { name: 'Kanban' })).toBeNull(); + fireEvent.click(screen.getByRole('checkbox', { name: 'Grid' })); + expect(onChange).toHaveBeenCalledWith(['kanban']); + }); +}); + +/* ── control 3: colorPaletteOptions → ColorSwatchGroupWidget (widgets.tsx) ── */ + +describe('consuming control 3 — colour swatches honour per-option visibleWhen', () => { + const ColorPicker = WIDGETS['color-picker']; + const palette = [ + { label: 'Red', value: 'red' }, + { label: 'Gold', value: 'gold', visibleWhen: "data.theme == 'premium'" }, + ]; + + it('⭐ withdraws the swatch whose predicate is FALSE against the draft', () => { + render( + {}} + schema={{ type: 'string' }} + fieldSpec={{ field: 'color', options: palette }} + formData={{ theme: 'basic' }} + ariaLabelledBy="lbl" + />, + ); + const names = screen.getAllByRole('radio').map((r) => r.getAttribute('aria-label') ?? r.textContent); + expect(names).toContain('Red'); + // ⭐ The load-bearing assertion. + expect(names).not.toContain('Gold'); + }); + + it('offers the same swatch once the draft satisfies the predicate', () => { + render( + {}} + schema={{ type: 'string' }} + fieldSpec={{ field: 'color', options: palette }} + formData={{ theme: 'premium' }} + ariaLabelledBy="lbl" + />, + ); + const names = screen.getAllByRole('radio').map((r) => r.getAttribute('aria-label') ?? r.textContent); + expect(names).toContain('Gold'); + }); + + it('⭐ FORK B / B1 — the widget REGISTRATION never moves with the predicate', () => { + const allHidden = [ + { label: 'Red', value: 'red', visibleWhen: "data.theme == 'premium'" }, + { label: 'Gold', value: 'gold', visibleWhen: "data.theme == 'premium'" }, + ]; + // `resolveColorWidgetKey` is the HOST's choice of registration, made from + // the schema BEFORE the label is written (objectui#4871 point 4). It reads + // the RAW palette, so withdrawing every swatch must NOT flip the field to + // `color-input` — that would move the field's accessible name onto a + // different element mid-render. + expect(resolveColorWidgetKey({ type: 'string' }, { field: 'color', options: allHidden })).toBe( + 'color-picker', + ); + render( + {}} + schema={{ type: 'string' }} + fieldSpec={{ field: 'color', options: allHidden }} + formData={{ theme: 'basic' }} + ariaLabelledBy="lbl" + />, + ); + expect(screen.getByRole('radiogroup')).toBeInTheDocument(); + expect(screen.queryAllByRole('radio')).toHaveLength(0); + }); +}); + +/* ── the scope itself (FORK A / A2) ──────────────────────────────────────── */ + +describe('buildPredicateCtx — what this surface binds, and what it refuses to', () => { + it('binds the four ADR-0068 identity spellings to the SAME object', () => { + const host = { current_user: ADMIN, user: ADMIN, ctx: { user: ADMIN }, os: { user: ADMIN }, data: { NOT: 'the draft' } }; + const ctx = buildPredicateCtx({ tier: 'business' }, host); + for (const root of IDENTITY_ROOTS) expect(ctx).toHaveProperty(root); + expect(ctx.current_user).toBe(ADMIN); + expect(ctx.user).toBe(ADMIN); + expect((ctx.ctx as { user: unknown }).user).toBe(ADMIN); + expect((ctx.os as { user: unknown }).user).toBe(ADMIN); + }); + + it('⛔ `data` is the DRAFT — never the host provider\'s own `data` scope', () => { + // objectui#5926 gap 2 is "same key, opposite meanings" one nesting level + // apart. The A2 ruling binds the identity roots ALONGSIDE `data` and keeps + // `data` = the draft, explicitly excluding the provider's conflicting key. + const ctx = buildPredicateCtx({ tier: 'business' }, { data: { tier: 'WRONG' }, current_user: ADMIN }); + expect(ctx.data).toEqual({ tier: 'business' }); + }); + + it('⛔ leaves un-published roots ABSENT so they stay loudly diagnosed', () => { + // Binding `record` / `app` / `features` to `{}` would convert an + // unresolved-root WARNING into a silent `undefined`-compares-false. The + // sibling runtime surface binds `record` because it edits a record; this + // one edits source metadata and has none. + const ctx = buildPredicateCtx({ tier: 'business' }, { current_user: ADMIN }); + expect(Object.prototype.hasOwnProperty.call(ctx, 'record')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(ctx, 'app')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(ctx, 'features')).toBe(false); + }); + + it('with no provider mounted, only `data` is bound', () => { + const ctx = buildPredicateCtx({ tier: 'business' }, {}); + expect(Object.keys(ctx)).toEqual(['data']); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx b/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx index f88f222b2a..f80c420e6d 100644 --- a/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx +++ b/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx @@ -55,7 +55,8 @@ import { CollapsibleTrigger, CollapsibleContent, } from '@object-ui/components'; -import { evaluatePredicate } from './predicate.js'; +import { usePredicateScope } from '@object-ui/react'; +import { evaluatePredicate, buildPredicateCtx, visibleOptions } from './predicate.js'; import type { FormFieldSpec, FormSectionSpec, FormViewSpec, VisibilityPredicate } from './form-spec.js'; import { WIDGETS, @@ -709,6 +710,11 @@ function SchemaFormBody({ // Live app locale (follows the i18next language, not just the browser) — // hoisted above the no-schema early return so the hook order is stable. const locale = useMetadataLocale(); + // The host shell's predicate scope (`ExpressionProvider` → `usePredicateScope`). + // Hoisted here with `locale` so the hook order is stable across the early + // returns below. `buildPredicateCtx` selects only the ADR-0068 identity roots + // out of it and keeps `data` = the draft (objectui#6247). + const hostScope = usePredicateScope(); // No schema → synthesize one from the value's top-level keys so the // form renderer can still produce a structured, labelled view (with // proper read-only semantics) instead of falling back to a raw JSON @@ -751,7 +757,7 @@ function SchemaFormBody({ .filter((k) => !hiddenFields.includes(k)) .filter((k) => { const visibility = readVisibility(props[k] as any); - return !visibility || evaluatePredicate(visibility, { data: predicateData }); + return !visibility || evaluatePredicate(visibility, buildPredicateCtx(predicateData, hostScope)); }); const v = value ?? {}; @@ -1007,9 +1013,14 @@ function SectionedSchemaForm({ onChange: (key: string, val: unknown) => void; }) { const locale = useMetadataLocale(); + // The host shell's predicate scope (`ExpressionProvider` → `usePredicateScope`). + // Hoisted here with `locale` so the hook order is stable across the early + // returns below. `buildPredicateCtx` selects only the ADR-0068 identity roots + // out of it and keeps `data` = the draft (objectui#6247). + const hostScope = usePredicateScope(); const sections = (form.sections ?? []).filter((s) => { const visibility = readVisibility(s); - return !visibility || evaluatePredicate(visibility, { data: value }); + return !visibility || evaluatePredicate(visibility, buildPredicateCtx(value, hostScope)); }); // Decide whether to render as tabs or stacked sections. @@ -1022,7 +1033,7 @@ function SectionedSchemaForm({ if (f.hidden) return false; if (hiddenFields.includes(f.field)) return false; const visibility = readVisibility(f); - if (visibility && !evaluatePredicate(visibility, { data: value })) { + if (visibility && !evaluatePredicate(visibility, buildPredicateCtx(value, hostScope))) { return false; } return true; @@ -1139,7 +1150,7 @@ function SectionedSchemaForm({ if (f.hidden) return false; if (hiddenFields.includes(f.field)) return false; const visibility = readVisibility(f); - return !visibility || evaluatePredicate(visibility, { data: value }); + return !visibility || evaluatePredicate(visibility, buildPredicateCtx(value, hostScope)); }), ); if (tabSections.length === 0) return null; @@ -1480,6 +1491,11 @@ function FieldControl({ formData?: Record; }) { const locale = useMetadataLocale(); + // The host shell's predicate scope (`ExpressionProvider` → `usePredicateScope`). + // Hoisted here with `locale` so the hook order is stable across the early + // returns below. `buildPredicateCtx` selects only the ADR-0068 identity roots + // out of it and keeps `data` = the draft (objectui#6247). + const hostScope = usePredicateScope(); // WHICH face renders is not decided here — `resolveFieldFace` decides it, and // `FieldRow` already called it with these very inputs to pick the naming // channel (objectui#5039). Calling the same pure function again here, rather @@ -1666,6 +1682,13 @@ function FieldControl({ const options = fieldSpec?.options; const enumValues = (effective?.enum as unknown[] | undefined) ?? undefined; + // ⚠️ The BRANCH condition reads the RAW list, deliberately (objectui#6247, + // Fork B → B1). Testing the FILTERED length here is the trap: a field whose + // every option is withdrawn would fall through this branch, then through + // `enumValues`, and land on the `string → Input` tail — so "withdraw every + // option" would render as a FREE-TEXT box, i.e. the exact opposite of the + // narrowing the author wrote. The face stays a Select; only its CONTENT is + // filtered, so an emptied set renders an empty picker. if (Array.isArray(options) && options.length > 0) { // Render from fieldSpec.options (Data.SelectOption[]) return ( @@ -1678,7 +1701,7 @@ function FieldControl({ - {options.map((opt) => ( + {visibleOptions(options, buildPredicateCtx(formData, hostScope)).map((opt) => ( {opt.label} {opt.color && ( @@ -2247,6 +2270,11 @@ function RecordField({ onChange: (v: unknown) => void; }) { const locale = useMetadataLocale(); + // The host shell's predicate scope (`ExpressionProvider` → `usePredicateScope`). + // Hoisted here with `locale` so the hook order is stable across the early + // returns below. `buildPredicateCtx` selects only the ADR-0068 identity roots + // out of it and keeps `data` = the draft (objectui#6247). + const hostScope = usePredicateScope(); // State hoisted above every early return below (the widget delegation and the // specialized-editor branches) so hook order stays stable across renders. const [openKey, setOpenKey] = React.useState(null); @@ -2449,7 +2477,7 @@ function RecordField({ /> {specs.map((s) => { const visibility = readVisibility(s); - if (visibility && !evaluatePredicate(visibility, { data: row })) return null; + if (visibility && !evaluatePredicate(visibility, buildPredicateCtx(row, hostScope))) return null; const sub = pickSubSchema(schema, 'record', s.field); return ( ; + /** + * The select options this field offers — DERIVED from the spec's own + * `SelectOption`, with the two narrowings this layer needs named in the + * `Omit` below (objectui#6247). + * + * It was a hand-written `{ label; value; color? }` triple: three of the + * spec's five keys, with the other two dropped by silence rather than by a + * named narrowing — the one element type in this file that the header's + * "every key the two layers agree on comes FROM the spec" rule had never + * been applied to. `visibleWhen` was among the dropped pair, so an author + * could legally write a per-option predicate into a `*.form.ts`, have it + * parse clean against `SelectOptionSchema` (`z.core.$strict`, so the schema + * really does declare it), and get a narrowing that this renderer then + * ignored: accepted, stored, shipped, inert — ADR-0049's declared≠enforced, + * and failing in the PERMISSIVE direction (the option stayed offered). + * + * - `visibleWhen` is RE-POINTED, not dropped: the spec's `ExpressionInput` + * makes `dialect` required and `source` optional, while the predicate + * that actually reaches an evaluator here is {@link VisibilityPredicate} + * (`dialect` optional, `source` required) — the same re-pointing, for the + * same reason, that `FormSectionSpec` already makes for its own + * `visibleWhen` / `visibleOn`. One vocabulary per document. + * - `default` stays DROPPED, and now says so. Nothing on this surface reads + * it: none of the three controls that consume `options` seeds a value + * from it, so declaring it here would advertise an authoring key this + * renderer does not honour — the very shape the `visibleWhen` half of + * this comment exists to close. Tracked separately as objectui#6263; + * honouring it is that card's decision, not a free widening here. + */ + options?: Array< + Omit & { visibleWhen?: VisibilityPredicate } + >; reference?: string; /** * Sibling field name(s) whose value this field's widget reads to decide WHAT diff --git a/packages/app-shell/src/views/metadata-admin/predicate.ts b/packages/app-shell/src/views/metadata-admin/predicate.ts index a18d64ad2a..fd74a80834 100644 --- a/packages/app-shell/src/views/metadata-admin/predicate.ts +++ b/packages/app-shell/src/views/metadata-admin/predicate.ts @@ -52,8 +52,9 @@ * ### The boundary this draws — read before widening it * * "Unresolvable" means **the path's root identifier is not a name this scope - * declares** (`type`, `record.status`, `page.selectedId` against a scope whose - * only name is `data`). It does NOT mean "the value came out undefined": + * declares** (`type`, `record.status`, `page.selectedId` against a scope that + * declares `data` plus the identity roots of {@link IDENTITY_ROOTS}). It does + * NOT mean "the value came out undefined": * * - `data.type == 'text'` on a draft that has no `type` yet → the root `data` * resolves, the draft simply has no value there. That is a legitimate @@ -143,9 +144,118 @@ * This diagnostic retires with the file at ROADMAP M9, same as #4049's. */ +/** + * The evaluation scope this surface publishes. + * + * `data` is ALWAYS the current draft. The index signature carries the extra + * roots {@link buildPredicateCtx} binds — today exactly {@link IDENTITY_ROOTS}. + * + * ⚠️ Read {@link buildPredicateCtx} before adding a root here. A root that is + * bound is a root that no longer warns, so widening this bag silently converts + * a loud diagnostic into a silent `undefined`-compares-false. + */ +export interface PredicateCtx { + /** The current draft. Never the host provider's own `data` scope. */ + data: Record; + [root: string]: unknown; +} + +/** + * The identity spellings this surface binds, ADR-0068 D1 (objectui#6247). + * + * ONE user object under four names: the canonical `current_user`, the + * back-compat `user`, the server-RLS-parity `ctx.user` and the server-CEL-parity + * `os.user`. They are not four dialects — they are the four spellings + * `buildExpressionScope` (`providers/ExpressionProvider.tsx`) already publishes + * for the same object, restated here as a SELECTION from that bag rather than a + * copy of it, so the alias set cannot drift from the one builder. + * + * ## What is deliberately NOT here — the whole of Fork A's ruling + * + * - ⛔ `data`. The host provider's bag also carries a `data` key, and it means + * the provider's data scope, NOT this form's draft. Adopting it head-on would + * be objectui#5926 gap 2's "same key, opposite meanings" — one authored + * `data.viewKind` reaching two different objects one nesting level apart. The + * 2026-08-25/27 maintainer ruling (A2, affirmed three times on objectui#6247) + * binds the identity roots ALONGSIDE `data` and keeps `data` = the draft. + * - ⛔ `record`. The sibling runtime surface binds it because it edits a data + * RECORD; metadata-admin edits source metadata and has no record. Leaving it + * unbound is what keeps `record.status` a loud diagnostic here instead of a + * silent false — `predicate.test.ts` pins that. + * - ⛔ `app` / `features`. Renderer-tier, not ADR-0068 identity, and not named + * by the ruling. Binding them as empty objects would turn `app.x` from a loud + * unresolved-root warning into a silent `undefined`. + */ +export const IDENTITY_ROOTS = ['current_user', 'user', 'ctx', 'os'] as const; + +/** + * Build the scope a metadata-admin predicate evaluates against: the draft under + * `data`, plus whichever of {@link IDENTITY_ROOTS} the host shell published. + * + * `hostScope` is `usePredicateScope()` (`@object-ui/react`) — the SAME bag the + * sibling runtime form surface reads, so one authored `visibleWhen` text names + * the same identity object on both. Roots the host did not publish stay ABSENT + * rather than being bound to `{}`: absent is what raises + * {@link UnresolvedPathError} and produces the loud diagnostic, and a gate that + * cannot resolve its identity must say so, not quietly answer "no". + * + * `data` is assigned LAST and unconditionally, so no host key can displace the + * draft even if the provider's bag grows one. + */ +export function buildPredicateCtx( + draft: Record | undefined, + hostScope?: Record, +): PredicateCtx { + const ctx: PredicateCtx = { data: draft ?? {} }; + if (hostScope) { + for (const root of IDENTITY_ROOTS) { + if (Object.prototype.hasOwnProperty.call(hostScope, root)) ctx[root] = hostScope[root]; + } + } + ctx.data = draft ?? {}; + return ctx; +} + +/** + * Is this select option offered? — the ONE reader of + * `SelectOptionSchema.visibleWhen` on this surface (objectui#6247). + * + * Routes through {@link evaluatePredicate}, the same evaluator the section, + * field and repeater-row gates use. Deliberately NOT a local predicate check + * bolted onto each option mapping: objectui#5926 gap 2 is the record of what a + * second dialect costs, and this surface is already the third evaluator in the + * platform — a fourth was ruled out. + * + * No predicate → offered, which is the same "absent means yes" every other gate + * on this surface uses. + */ +export function isOptionVisible( + opt: { visibleWhen?: string | { dialect?: string; source: string } }, + ctx: PredicateCtx, +): boolean { + return evaluatePredicate(opt.visibleWhen, ctx); +} + +/** + * The rendered subset of an authored option list. + * + * ⚠️ This is the CONTENT half only. Per the Fork B ruling (B1) the control's + * FACE — `resolveFieldFace`'s `hasOptions`, `resolveColorWidgetKey`, and each + * `options.length > 0` branch condition — keeps reading the RAW list, so + * withdrawing every option renders an EMPTY PICKER rather than flipping the + * field to a free-text input or to a different widget registration. Call this + * where the list is mapped to items, never where the branch is chosen. + */ +export function visibleOptions( + options: readonly T[], + ctx: PredicateCtx, +): T[] { + return options.filter((o) => isOptionVisible(o, ctx)); +} + export function evaluatePredicate( expr: string | { dialect?: string; source: string } | null | undefined, - ctx: { data: Record }, + ctx: PredicateCtx, ): boolean { if (expr == null) return true; const source = typeof expr === 'string' ? expr : expr.source; @@ -154,7 +264,7 @@ export function evaluatePredicate( return evalExpr(source.trim(), ctx, source); } catch (err) { // Fail-open either way; an unresolvable path additionally gets a name. - if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source); + if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source, ctx); return true; } } @@ -208,18 +318,27 @@ const isDev = (): boolean => (globalThis as { process?: { env?: Record } }).process?.env?.NODE_ENV !== 'production'; -function warnUnresolvedPath(path: string, source: string): void { +function warnUnresolvedPath(path: string, source: string, ctx: PredicateCtx): void { if (!isDev()) return; const memo = `${path}::${source}`; if (warnedUnresolvedPaths.has(memo)) return; warnedUnresolvedPaths.add(memo); const root = path.split('.')[0]; + // The bound names are READ OFF THE ACTUAL SCOPE, never restated as a literal. + // They differ between call sites — the identity roots are present only where a + // host `ExpressionProvider` published them (objectui#6247) — and a hardcoded + // list would be a diagnostic that lies in whichever world it was not written + // for. A wrong "the only name is X" is worse than no diagnostic: it sends the + // author to un-write a spelling that is in fact correct. + const bound = Object.keys(ctx).sort().map((n) => `\`${n}\``).join(', '); console.warn( `[metadata-admin] visibility predicate \`${source}\` references \`${path}\`, but \`${root}\` ` + - 'is not a name in this form\'s evaluation scope — the only name is `data` (the current draft). ' + - 'The predicate was treated as TRUE (fail-open) so the field stays visible instead of ' + - 'disappearing without a trace; check the spelling — draft values are addressed as `data.` ' + - '(e.g. `data.type in [...]`). A predicate served by an older backend is the usual cause ' + + `is not a name in this form's evaluation scope — the names here are ${bound} ` + + '(`data` is the current draft; the identity roots are bound only under a host ' + + '`ExpressionProvider`). The predicate was treated as TRUE (fail-open) so the field stays ' + + 'visible instead of disappearing without a trace; check the spelling — draft values are ' + + 'addressed as `data.` (e.g. `data.type in [...]`) and the signed-in user as ' + + '`current_user.`. A predicate served by an older backend is the usual cause ' + '(objectstack#6254 re-spelled the bare `objectForm` predicates; objectstack#6936).', ); } @@ -309,7 +428,7 @@ function warnUnparseableInSet(raw: string, source: string): void { function evalExpr( expr: string, - ctx: { data: Record }, + ctx: PredicateCtx, // The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in // a sub-expression can name the predicate the author actually wrote — the // same pairing objectstack#6936's warning makes via UnresolvedPathError. @@ -387,7 +506,7 @@ function splitTopLevel(expr: string, op: string): string[] { function resolveValue( path: string, - ctx: { data: Record }, + ctx: PredicateCtx, source: string, ): unknown { // Allow literals on the left side too. diff --git a/packages/app-shell/src/views/metadata-admin/widgets.tsx b/packages/app-shell/src/views/metadata-admin/widgets.tsx index 5b6761a31d..e509a7a1e8 100644 --- a/packages/app-shell/src/views/metadata-admin/widgets.tsx +++ b/packages/app-shell/src/views/metadata-admin/widgets.tsx @@ -47,7 +47,9 @@ import { iconNames } from 'lucide-react/dynamic.mjs'; import { toast } from 'sonner'; import { useObjectTranslation } from '@object-ui/i18n'; import { useMetadataLocale, t, tFormat } from './i18n.js'; -import type { FormFieldSpec } from './form-spec.js'; +import type { FormFieldSpec, VisibilityPredicate } from './form-spec.js'; +import { usePredicateScope } from '@object-ui/react'; +import { buildPredicateCtx, visibleOptions, type PredicateCtx } from './predicate.js'; import { foldFilterGroupToSpecRules, FILTER_FOLD_REFUSAL_KEYS } from '../viewFilterFold.js'; import { ColorVariantPicker } from './color-variant-field.js'; import { ConditionBuilder } from './inspectors/ConditionBuilder.js'; @@ -1122,18 +1124,32 @@ function humanizeOption(v: string): string { * free-text tag input the generic array renderer fell back to — the author * picks from the real allowed values instead of typing (and mistyping) them. */ -function MultiSelectWidget({ value, onChange, readOnly, schema, fieldSpec, ariaLabelledBy }: WidgetProps) { +function MultiSelectWidget({ value, onChange, readOnly, schema, fieldSpec, formData, ariaLabelledBy }: WidgetProps) { + const hostScope = usePredicateScope(); // Prefer explicit form options; else the JSON Schema enum on the items. - const options: Array<{ label: string; value: string }> = React.useMemo(() => { - if (Array.isArray(fieldSpec?.options) && fieldSpec!.options!.length) { - return fieldSpec!.options!.map((o) => ({ label: o.label, value: o.value })); - } - const enumVals: unknown = - schema?.items?.enum ?? schema?.enum ?? []; - return (Array.isArray(enumVals) ? enumVals : []) - .filter((v): v is string => typeof v === 'string') - .map((v) => ({ label: humanizeOption(v), value: v })); - }, [fieldSpec, schema]); + // + // ⚠️ RAW — every per-option `visibleWhen` is carried through untouched + // (objectui#6247). This list answers the two questions that must NOT depend + // on a predicate: whether this widget renders at all (the degradation branch + // below) and what ORDER a selection is stored in (`toggle`). Only + // `renderedOptions` is filtered. + const options: Array<{ label: string; value: string; visibleWhen?: VisibilityPredicate }> = + React.useMemo(() => { + if (Array.isArray(fieldSpec?.options) && fieldSpec!.options!.length) { + return fieldSpec!.options!.map((o) => ({ label: o.label, value: o.value, visibleWhen: o.visibleWhen })); + } + const enumVals: unknown = + schema?.items?.enum ?? schema?.enum ?? []; + return (Array.isArray(enumVals) ? enumVals : []) + .filter((v): v is string => typeof v === 'string') + .map((v) => ({ label: humanizeOption(v), value: v })); + }, [fieldSpec, schema]); + + // The CONTENT half: what the author is offered right now. + const renderedOptions = React.useMemo( + () => visibleOptions(options, buildPredicateCtx(formData, hostScope)), + [options, formData, hostScope], + ); const selected = React.useMemo( () => (Array.isArray(value) ? (value as unknown[]).filter((v): v is string => typeof v === 'string') : []), @@ -1147,10 +1163,21 @@ function MultiSelectWidget({ value, onChange, readOnly, schema, fieldSpec, ariaL const set = new Set(selected); if (set.has(opt)) set.delete(opt); else set.add(opt); + // ⚠️ RAW list, not the rendered one (objectui#6247, Fork C → C1: no + // pruning). Ordering against the filtered list would DROP any already- + // selected value whose option is currently hidden — pruning authored + // metadata through the back door, on the next unrelated click, with no + // author action that says "remove this". A hidden-but-selected value + // survives; it is simply not offered again. const next = options.map((o) => o.value).filter((v) => set.has(v)); onChange(next.length ? next : undefined); } + // ⚠️ RAW length (objectui#6247, Fork B → B1). Testing the FILTERED length + // here would make "withdraw every option" degrade to the free-text tag + // editor — the opposite of the narrowing the author wrote, and a change of + // naming channel that objectui#4871 removed from this file. A field whose + // options are all withdrawn stays this widget and renders an empty group. if (options.length === 0) { // No known option set — degrade to the comma-tag editor so the field // is still editable rather than rendering an empty box. @@ -1170,7 +1197,7 @@ function MultiSelectWidget({ value, onChange, readOnly, schema, fieldSpec, ariaL return (
- {options.map((o) => { + {renderedOptions.map((o) => { const on = selected.includes(o.value); return (