diff --git a/.changeset/6110-predicate-scope-unbound-evaluators.md b/.changeset/6110-predicate-scope-unbound-evaluators.md new file mode 100644 index 0000000000..4be26dce28 --- /dev/null +++ b/.changeset/6110-predicate-scope-unbound-evaluators.md @@ -0,0 +1,59 @@ +--- +'@object-ui/app-shell': minor +'@object-ui/console': minor +'@object-ui/plugin-form': minor +--- + +⚠️ **Behaviour change: `current_user` predicates that have been doing nothing on +the console form routes and in the wizard's submit gate now TAKE EFFECT.** Read +this before upgrading if any of your form metadata gates on the session user. + +objectui#6010 bound the host predicate scope on the five authored-predicate call +sites in the components form renderer, so `current_user` (plus the ADR-0068 +`user` / `ctx.user` / `os.user` aliases) resolves on `visibleWhen` / `visibleOn` +there. Two other authored-predicate evaluators were still passing `undefined` +for that argument, so the same authored text meant two different things +depending on which surface opened the form (objectui#6110): + +- **`apps/console`'s form renderer**, on the authed internal route + `/forms/:name`. The internal route is a runtime record surface by ADR-0089 + D1's own words (*"runtime record surfaces bind `record` + `current_user`"*), + and its `visibleWhen` metadata is the same `*.view.ts` FormView the + object-view chain renders — so a role gate authored once behaved differently + depending on which route opened the form. +- **`WizardForm`'s submit-time required re-check** (`missingRequiredByStep`), + the gate that re-checks the whole declared field set at final submit because + `allowSkip` can jump past a step. Its docstring promises *"the same verdict + from all three rather than a second, divergent dialect"*, and since #6010 it + was the divergent one. + +**Why nobody noticed, and why the fix is felt as a change.** `visibleWhen` fails +OPEN: a field on screen is what you get when the predicate resolves TRUE, when +the scope was never bound so the predicate faulted, *and* when the predicate is +broken. Those worlds were indistinguishable, so an app that authored a +`current_user` gate saw the field render and had no way to tell the rule was +inert. After this change the predicate is evaluated for real, and fields and +sections that have always been visible will disappear for the users the rule +excludes. `requiredWhen` fails the other way (CLOSED), so a `current_user` +requiredWhen that has been silently not applying will now start holding submits. + +In the wizard the change is a fix in the user's favour as well: a required field +the wizard HID from this user was still counted as visible by the submit gate, +so the submit was refused on a control the submitter could neither see nor fill +in. + +**Before upgrading**, audit any `visibleWhen` / `visibleOn` / `requiredWhen` in +your form-view and object metadata that names `current_user`, and confirm each +predicate says what you actually want evaluated against `record` + +`current_user`. + +**The public anonymous form `/f/:slug` is deliberately unchanged.** It is +mounted outside `ProtectedRoute` so an anonymous visitor can submit it, there is +no authenticated principal, and no provider is mounted above it — so its scope +is empty and a `current_user` predicate authored on a public form still faults +and still fails open, exactly as before. Nothing new is declared to say so: the +two routes are told apart by which component mounts them. + +`@object-ui/app-shell` exports `buildExpressionUser`, the `ExpressionProvider` +user normalisation, so every console surface that mounts the provider publishes +the same `current_user` shape rather than re-deriving it. diff --git a/apps/console/src/__tests__/internalFormShell.test.tsx b/apps/console/src/__tests__/internalFormShell.test.tsx index b85e8e7a08..3b2fc69be9 100644 --- a/apps/console/src/__tests__/internalFormShell.test.tsx +++ b/apps/console/src/__tests__/internalFormShell.test.tsx @@ -81,6 +81,15 @@ vi.mock('@object-ui/app-shell', () => ({ // Consumed by InternalFormRoute to resolve the created-record target. useMetadata: () => ({ apps: [{ name: 'showcase_app', _packageId: 'ai.objectstack.showcase' }] }), useNavigationContext: () => ({ currentAppName: 'showcase_app' }), + // Consumed by InternalFormRoute to publish the session principal as the form + // route's predicate scope (objectui#6110). Stubbed, like every other entry in + // this factory: this file pins the #4109 SHELL NESTING and authors no + // `current_user` predicate, so a real provider would only slow it down. The + // BINDING itself is pinned in `components/FormPage.predicateScope.test.tsx`, + // whose `hop1SessionPrincipal` case mounts the real `InternalFormRoute` with + // the real provider and goes red the moment this mount is removed. + ExpressionProvider: passthrough, + buildExpressionUser: (user: unknown) => user, })); vi.mock('@object-ui/auth', () => ({ diff --git a/apps/console/src/components/FormPage.predicateScope.test.tsx b/apps/console/src/components/FormPage.predicateScope.test.tsx new file mode 100644 index 0000000000..c1a0abca17 --- /dev/null +++ b/apps/console/src/components/FormPage.predicateScope.test.tsx @@ -0,0 +1,369 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#6110 — the console form renderer binds the HOST PREDICATE SCOPE, so + * `current_user` resolves on its authored predicates the way it already does on + * the sibling renderer (`packages/components/src/renderers/form/form.tsx`, + * fixed by #6010). + * + * ## The defect, and why it needed TWO hops repaired, not the card's one + * + * The card named two evaluator call sites in this file passing `undefined` for + * `resolveFieldRuleState`/`evalFieldPredicate`'s `scope` parameter. Binding + * them alone would have shipped INERT, because nothing in this app published a + * scope for them to read: + * + * hop 1 — `usePredicateScope()` returns `{}` unless an `ExpressionProvider` + * is mounted above. The console mounted one in exactly two places + * (`AppContent`, for the `/apps/:appName/*` subtree, and app-shell's + * `RecordFormPage`) and NEITHER is above `/forms/:name`, which is + * mounted from `App.tsx` through `InternalFormRoute` → + * `DefaultHomeLayout`. So the authed form route had no principal in + * scope at all. `InternalFormRoute` now mounts the provider, over the + * SAME `buildExpressionUser` normalisation `AppContent` uses. + * hop 2 — the evaluator call sites in `FormPage.tsx`, which is what the card + * enumerated. There are THREE, not the two it named: the field + * predicate, the section predicate, and `resolveRowState`'s + * `resolveFieldRuleState` call carrying the OBJECT-level rules. + * + * `hop1SessionPrincipal` below is the case that would have caught a hop-2-only + * fix: it mounts the real `InternalFormRoute` and lets the route itself supply + * the principal, instead of handing one to `FormPage` directly. + * + * ## ⚠️ Why every case here asserts HIDDEN (or REQUIRED), never merely SHOWN + * + * `visibleWhen` is evaluated with `fallback: true` — it fails OPEN. A field on + * screen is therefore the outcome of THREE different worlds: the predicate + * resolved true, the scope was never bound so the predicate faulted, and the + * predicate is broken. An assertion that a field IS shown separates none of + * them and is green on unfixed code. The deliverable is the DENIED block: a + * `current_user` predicate that is FALSE for the bound principal, asserted + * ABSENT. + * + * `requiredWhen` fails the other way (`fallback: false`), which is why the + * `requiredWhen` case asserts the marker is PRESENT: unbound, that predicate + * faults to "not required" and the asterisk is missing. The two directions + * together are what make this file a measurement of the BINDING rather than of + * either fallback. + * + * ## The anonymous `/f/:slug` route — the fork clause, answered by structure + * + * Both routes share one call site, and the card left open what an anonymous + * form binds. It needed no new contract surface to answer: the two routes are + * distinguished by WHICH COMPONENT MOUNTS THEM. `/forms/:name` renders inside + * `InternalFormRoute`, which has an authenticated session and now publishes it; + * `/f/:slug` is mounted bare from `App.tsx`, deliberately outside + * `ProtectedRoute`, so `usePredicateScope()` there returns `{}` — there is no + * principal to bind, and binding an empty scope is exactly that statement. The + * `publicRouteHasNoPrincipal` case pins it: on the public route the same + * authored text still faults open, unchanged by this card. + */ + +import { describe, expect, it, vi, afterEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { PredicateScopeProvider } from '@object-ui/react'; +import { FormPage } from './FormPage'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +/** + * The host scope an `ExpressionProvider` publishes, transcribed from + * `packages/app-shell/src/providers/ExpressionProvider.tsx` — the canonical + * `current_user` plus the ADR-0068 `user` / `ctx.user` / `os.user` aliases. + * Same spelling as #6010's parity pin, on purpose: one authored text, one + * verdict, every surface. + */ +function hostScope(positions: string[]) { + const user = { id: 'u1', name: 'Kim', role: 'user', isPlatformAdmin: false, positions }; + return { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} }; +} + +/** The SAME principal shape, one admitted by `GATE` and one refused by it. */ +const DENIED = hostScope(['sales']); +const ALLOWED = hostScope(['sales_manager']); + +/** THE authored predicate. One text, asked of every surface below. */ +const GATE = "'sales_manager' in current_user.positions"; + +/** + * A root that genuinely does not exist in any scope — the FAULT control. Its + * only difference from `GATE` is the root it names, so a fail-CLOSED + * regression cannot hide behind the membership test. + */ +const UNBOUND_ROOT = "'sales_manager' in no_such_root.positions"; + +const BASE_SCHEMA = { + name: 'showcase_task', + label: 'Task', + fields: { + title: { type: 'text', label: 'Title' }, + priority: { type: 'text', label: 'Priority', defaultValue: 'low' }, + notes: { type: 'text', label: 'Notes' }, + } as Record>, +}; + +/** `BASE_SCHEMA` with OBJECT-level rule keys merged onto named fields. */ +function withRules(rules: Record>) { + const fields: Record> = {}; + for (const [name, def] of Object.entries(BASE_SCHEMA.fields)) { + fields[name] = { ...def, ...(rules[name] ?? {}) }; + } + return { ...BASE_SCHEMA, fields }; +} + +function viewEnvelope(sections: unknown[]) { + return { + name: 'showcase_task.edit', + object: 'showcase_task', + viewKind: 'form', + label: 'Task', + config: { type: 'simple', sections }, + }; +} + +function stubFetch(routes: Array<{ method?: string; match: string; body?: unknown }>) { + return vi.fn(async (url: string, init?: RequestInit) => { + const method = (init?.method ?? 'GET').toUpperCase(); + const route = routes.find( + (r) => (r.method ?? 'GET').toUpperCase() === method && String(url).includes(r.match), + ); + if (!route) throw new Error(`unstubbed fetch: ${method} ${url}`); + return { + ok: true, + status: 200, + statusText: 'OK', + json: async () => route.body, + text: async () => (route.body === undefined ? '' : JSON.stringify(route.body)), + } as unknown as Response; + }); +} + +/** + * The INTERNAL route, with a scope handed straight to `FormPage` — this is the + * hop-2 harness. It deliberately does NOT go through `InternalFormRoute`, so a + * fix that binds the call sites but never publishes a scope still passes here + * and is caught by `hop1SessionPrincipal` instead. + */ +function renderInternalWithScope( + scope: Record, + sections: unknown[], + objectSchema: unknown = BASE_SCHEMA, +) { + vi.stubGlobal( + 'fetch', + stubFetch([ + { match: '/meta/view/', body: viewEnvelope(sections) }, + { match: '/meta/object/', body: objectSchema }, + ]), + ); + return render( + + + + } /> + + + , + ); +} + +/** The PUBLIC route (`/f/:slug`) — mounted bare, exactly as `App.tsx` does. */ +function renderPublic(sections: unknown[], objectSchema: unknown = BASE_SCHEMA) { + vi.stubGlobal( + 'fetch', + stubFetch([ + { + match: '/forms/task-intake', + body: { + slug: 'task-intake', + object: 'showcase_task', + label: 'Task intake', + form: { type: 'simple', sections }, + objectSchema, + }, + }, + ]), + ); + return render( + + + } /> + + , + ); +} + +/** + * The un-gated sibling control. Every case waits on it: a missing `Priority` + * would mean the form never rendered, and then an absent gated field is an + * INABILITY rather than a verdict. + */ +async function awaitForm() { + await waitFor(() => expect(screen.getByLabelText('Priority')).toBeInTheDocument()); +} + +/** The `label` element, so the REQUIRED MARKER can be read off the output. */ +function labelFor(container: HTMLElement, name: string): HTMLElement { + const el = container.querySelector(`label[for="f_${name}"]`); + if (!el) throw new Error(`no label rendered for field '${name}'`); + return el as HTMLElement; +} + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +// ─── DENIED — the deliverable ──────────────────────────────────────────── + +describe('#6110 DENIED — a `current_user` predicate false for the bound principal HIDES', () => { + it('the VIEW-level field `visibleWhen` (FormPage.tsx isFieldVisible)', async () => { + renderInternalWithScope(DENIED, [ + { label: 'Basics', fields: ['priority'] }, + { label: 'Pay', fields: [{ field: 'notes', label: 'Notes', visibleWhen: GATE }] }, + ]); + await awaitForm(); + expect(screen.queryByLabelText('Notes')).not.toBeInTheDocument(); + }); + + it('the SECTION `visibleWhen` (FormPage.tsx isSectionVisible)', async () => { + renderInternalWithScope(DENIED, [ + { label: 'Basics', fields: ['priority'] }, + { label: 'Compensation', visibleWhen: GATE, fields: ['notes'] }, + ]); + await awaitForm(); + // Heading AND fields — a section rule takes the whole `
`. + expect(screen.queryByText('Compensation')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Notes')).not.toBeInTheDocument(); + }); + + it('the OBJECT-level `rules.visibleWhen` — the THIRD site, not named on the card', async () => { + // `resolveRowState` → `resolveFieldRuleState`. The card enumerated two + // evaluators in this file; this is the one it missed. + renderInternalWithScope( + DENIED, + [{ label: 'Basics', fields: ['priority', 'notes'] }], + withRules({ notes: { visibleWhen: GATE } }), + ); + await awaitForm(); + expect(screen.queryByLabelText('Notes')).not.toBeInTheDocument(); + }); +}); + +describe('#6110 DENIED (other direction) — `requiredWhen` faults CLOSED when unbound', () => { + it('marks the field REQUIRED when a `current_user` requiredWhen is TRUE for the principal', async () => { + // The direction that cannot be reached by a fail-open accident: with no + // scope this predicate faults to `false` and the asterisk is absent. + const { container } = renderInternalWithScope( + ALLOWED, + [{ label: 'Basics', fields: ['priority', 'notes'] }], + withRules({ notes: { requiredWhen: GATE } }), + ); + await awaitForm(); + expect(labelFor(container, 'notes').textContent).toContain('*'); + }); +}); + +// ─── Controls — green on a revert, and stated as such ──────────────────── + +describe('#6110 controls — green before AND after the fix, by construction', () => { + it('ALLOWED: the SAME predicate text, a principal it admits ⇒ still shown', async () => { + renderInternalWithScope(ALLOWED, [ + { label: 'Basics', fields: ['priority'] }, + { label: 'Compensation', visibleWhen: GATE, fields: ['notes'] }, + ]); + await awaitForm(); + expect(screen.getByText('Compensation')).toBeInTheDocument(); + expect(screen.getByLabelText('Notes')).toBeInTheDocument(); + }); + + it('FAULTED: a genuinely unbound ROOT still fails OPEN (unchanged by this card)', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + renderInternalWithScope(DENIED, [ + { label: 'Basics', fields: ['priority'] }, + { label: 'Compensation', visibleWhen: UNBOUND_ROOT, fields: ['notes'] }, + ]); + await awaitForm(); + // Pinned so the DENIED block above means "evaluated and false" rather than + // "could not be evaluated at all". + expect(screen.getByText('Compensation')).toBeInTheDocument(); + }); + + it('publicRouteHasNoPrincipal: `/f/:slug` binds an EMPTY scope, so `current_user` still faults open', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + renderPublic([ + { label: 'Basics', fields: ['priority'] }, + { label: 'Compensation', visibleWhen: GATE, fields: ['notes'] }, + ]); + await awaitForm(); + // The fork clause's answer, pinned as behaviour rather than as prose: the + // anonymous route is mounted outside `ProtectedRoute` with no provider + // above it, so there is no principal to bind and nothing changes here. + expect(screen.getByText('Compensation')).toBeInTheDocument(); + }); +}); + +// ─── HOP 1 — the route itself must PUBLISH the principal ───────────────── + +/** + * `InternalFormRoute`'s own dependencies, stubbed to the minimum the route + * needs — and `ExpressionProvider` / `buildExpressionUser` deliberately left + * REAL, because they are the thing under test. `DefaultHomeLayout` is a + * pass-through: the chrome is #4109's subject, not this card's, and stubbing it + * cannot hide the provider, which the route mounts itself. + */ +vi.mock('@object-ui/app-shell', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + useMetadata: () => ({ apps: [] }), + useNavigationContext: () => ({ currentAppName: undefined }), + DefaultHomeLayout: ({ children }: { children?: unknown }) => children as never, + }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + // The shape `AuthProvider` yields; `buildExpressionUser` normalises it into + // the scope, which is why `positions` reaches `current_user` at all. + user: { id: 'u1', name: 'Kim', email: 'kim@example.com', positions: ['sales'] }, + }), +})); + +describe('#6110 hop 1 — `/forms/:name` publishes the SESSION principal', () => { + it('hop1SessionPrincipal: the route alone binds `current_user`, with no scope handed in', async () => { + // ⚠️ THE anti-inert case. Every other DENIED case above hands `FormPage` a + // scope directly and stays green even if nothing in the app ever publishes + // one — which was the real state of this route before this card: the + // console mounts `ExpressionProvider` only in `AppContent` (the + // `/apps/:appName/*` subtree) and in app-shell's `RecordFormPage`, and + // NEITHER is above `/forms/:name`. Binding the evaluator call sites without + // this hop would have shipped a fix that reads `{}` forever. + const { InternalFormRoute } = await import('./InternalFormRoute'); + vi.stubGlobal( + 'fetch', + stubFetch([ + { + match: '/meta/view/', + body: viewEnvelope([ + { label: 'Basics', fields: ['priority'] }, + { label: 'Compensation', visibleWhen: GATE, fields: ['notes'] }, + ]), + }, + { match: '/meta/object/', body: BASE_SCHEMA }, + ]), + ); + render( + + + } /> + + , + ); + await awaitForm(); + expect(screen.queryByText('Compensation')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Notes')).not.toBeInTheDocument(); + }); +}); diff --git a/apps/console/src/components/FormPage.tsx b/apps/console/src/components/FormPage.tsx index b2b447d6d4..3cddad63a8 100644 --- a/apps/console/src/components/FormPage.tsx +++ b/apps/console/src/components/FormPage.tsx @@ -114,6 +114,7 @@ import { resolveFieldRuleState, type FieldRulePredicate, } from '@object-ui/core'; +import { usePredicateScope } from '@object-ui/react'; import type { FormFieldSpec, FormSectionSpec, FormViewSpec } from '@object-ui/app-shell'; import { resolveSubmitRedirect } from './submitRedirect'; @@ -605,9 +606,10 @@ export function isFieldVisible( field: RenderableField, values: Record, previous?: Record | null, + predicateScope?: Record, ): boolean { if (field.hidden) return false; - return evalFieldPredicate(field.visibleWhen, values, true, previous ?? undefined, undefined, { + return evalFieldPredicate(field.visibleWhen, values, true, previous ?? undefined, predicateScope, { // Named for the canonical key even when the deprecated alias supplied the // predicate: ADR-0089 is what this predicate is CALLED, and the warning // prints the source text verbatim, which is what finds a hand-written @@ -647,8 +649,9 @@ export function isSectionVisible( section: RenderableSection, values: Record, previous?: Record | null, + predicateScope?: Record, ): boolean { - return evalFieldPredicate(section.visibleWhen, values, true, previous ?? undefined, undefined, { + return evalFieldPredicate(section.visibleWhen, values, true, previous ?? undefined, predicateScope, { // Sections have no `name`, so the locator is the heading an author can // actually find in their own metadata; an unlabelled section says so // rather than printing `undefined`. @@ -704,6 +707,7 @@ export function resolveRowState( values: Record, previous: Record | null | undefined, isCreateForm: boolean, + predicateScope?: Record, ): { visible: boolean; readonly: boolean; required: boolean } { const ruleState = resolveFieldRuleState( field.rules ?? {}, @@ -720,10 +724,10 @@ export function resolveRowState( ), }, previous ?? undefined, - undefined, + predicateScope, `field '${field.name}'`, ); - const viewVisible = isFieldVisible(field, values, previous); + const viewVisible = isFieldVisible(field, values, previous, predicateScope); return { visible: ruleState.visible && viewVisible, readonly: ruleState.readonly, @@ -1536,6 +1540,32 @@ export function FormPage({ mode, recordPath }: FormPageProps) { * never reads `?recordId=` at all. */ const isCreateForm = target.kind !== 'edit'; + /** + * The host shell's global predicate scope — `current_user` plus the ADR-0068 + * `user` / `ctx.user` / `os.user` aliases, `app`, `data`, `features` — read + * from the `PredicateScopeProvider` an `ExpressionProvider` mounts, and + * threaded into all three evaluators below (objectui#6110). Same binding the + * sibling renderer took in #6010, so one authored `visibleWhen` means one + * thing on both form chains. + * + * ## The two routes are told apart by WHO MOUNTS THEM, not by a new key + * + * `/forms/:name` renders inside `InternalFormRoute`, which has an + * authenticated session and publishes it through `ExpressionProvider`. + * `/f/:slug` is mounted bare in `App.tsx`, deliberately outside + * `ProtectedRoute`, because an anonymous visitor must be able to submit it — + * so no provider sits above it and this returns `{}`. That is not a gap left + * unfilled: an anonymous form HAS no principal, and binding an empty scope is + * precisely that statement. A `current_user` predicate authored on a public + * form therefore still faults and still fails OPEN, exactly as before this + * change; nothing new is declared to say so. + * + * `features` is likewise empty here rather than fetched. `ExpressionProvider` + * already documents `{}` as the pre-load state whose predicates default to + * visible, and wiring a deployment-config fetch into this route would be a + * different card. + */ + const predicateScope = usePredicateScope(); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); @@ -1816,7 +1846,7 @@ export function FormPage({ mode, recordPath }: FormPageProps) { // would renumber every later section the moment one hid, remounting // their inputs and taking the caret with them. `null` keeps each // surviving section on the key it already had. - if (!isSectionVisible(sec, values, loaded.record)) return null; + if (!isSectionVisible(sec, values, loaded.record, predicateScope)) return null; return (
{sec.label && ( @@ -1838,7 +1868,7 @@ export function FormPage({ mode, recordPath }: FormPageProps) { // things that depend on it below — the row's presence, the // required marker, and the control's own attributes. Computing // it three times would be three chances to disagree. - const state = resolveRowState(f, values, loaded.record, isCreateForm); + const state = resolveRowState(f, values, loaded.record, isCreateForm, predicateScope); if (!state.visible) return null; return (
buildExpressionUser(user), [user]); + return ( - - - + + + + + ); } diff --git a/packages/app-shell/src/index.ts b/packages/app-shell/src/index.ts index e353c85436..8e95fa3829 100644 --- a/packages/app-shell/src/index.ts +++ b/packages/app-shell/src/index.ts @@ -12,6 +12,16 @@ export { AppShell } from './components/AppShell.js'; export { AdapterProvider, useAdapter } from './providers/AdapterProvider.js'; export { MetadataProvider, useMetadata, useMetadataItem } from './providers/MetadataProvider.js'; export { ExpressionProvider, useExpressionContext, evaluateVisibility } from './providers/ExpressionProvider.js'; +/** + * The `user` normalisation `ExpressionProvider` is fed with — exported so every + * console surface that mounts the provider publishes the SAME `current_user` + * shape (objectui#6110). It supplies the defaults a predicate needs in order to + * evaluate to FALSE rather than FAULT: an absent `positions` makes + * `'x' in current_user.positions` an unbound-key fault, which fails OPEN, so a + * second mount site re-deriving this by hand would reintroduce exactly the + * asymmetry #6010's parity pin exists to refuse. + */ +export { buildExpressionUser } from './console/AppContent.js'; // Hooks export { useObjectActions } from './hooks/useObjectActions.js'; diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index 9d6d1b0370..4817f4c1d8 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -20,7 +20,7 @@ import { AlertCircle, Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-r import { resolveFieldRuleState, evalFieldPredicate, isMissingForRequired, isServerOwnedValue } from '@object-ui/core'; import { createSafeTranslation } from '@object-ui/i18n'; import { FormSectionContainer } from './FormSection'; -import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react'; +import { SchemaRenderer, useSafeFieldLabel, usePredicateScope } from '@object-ui/react'; import { buildSectionFields as buildSectionFieldsShared } from './sectionFields'; import { seedCreateValues, omitServerResolvedDefaults, isCreateFormMode } from './schemaDefaults'; import { usePermissions } from '@object-ui/permissions'; @@ -405,6 +405,24 @@ export const WizardForm: React.FC = ({ // form and gated as an edit one. const isCreateWizard = isCreateFormMode(schema); + /** + * The host shell's global predicate scope (`ExpressionProvider` → + * `PredicateScopeProvider`) — `current_user` plus the ADR-0068 `user` / + * `ctx.user` / `os.user` aliases, `app`, `data`, `features`. Empty `{}` when + * no provider is mounted, which is the same "nothing to bind" the renderer + * sees (objectui#6110). + * + * Read HERE, at the component's top level, because the only consumer is a + * `useCallback` — a hook cannot be called inside one, and the gate below is + * invoked from a submit handler where no React context is readable at all. + * It joins that callback's dependency list for the same reason it joins the + * renderer's (#6010): a scope change — the host switching organisations, so + * `current_user.positions` changes — can flip a `visibleWhen` exactly as a + * keystroke can, and a memoized gate holding the old scope would answer the + * final submit with a principal the user no longer is. + */ + const predicateScope = usePredicateScope(); + // Current section fields const currentSectionFields = useMemo(() => { if (currentStep >= 0 && currentStep < totalSteps) { @@ -458,7 +476,11 @@ export const WizardForm: React.FC = ({ serverOwnedValue: isServerOwnedValue(field, isCreateWizard), }, undefined, - undefined, + // The host predicate scope, so `current_user` resolves here exactly + // as it does in the form renderer that DREW this field (#6010). + // Without it this gate faults on every `current_user` predicate and + // fails OPEN, demanding a field the wizard itself hid (#6110). + predicateScope, `field '${name}'`, ); // View-level FormField.visibleOn hides the field the same way a @@ -468,7 +490,7 @@ export const WizardForm: React.FC = ({ // receives the ADR-0089 canonical view-level `visibleWhen` spelling. const viewVisible = (field as any).visibleOn == null || - evalFieldPredicate((field as any).visibleOn, record, true, undefined, undefined, { + evalFieldPredicate((field as any).visibleOn, record, true, undefined, predicateScope, { context: `visibleOn of field '${name}'`, }); // A hidden or read-only field is not the user's to fill in. @@ -479,7 +501,7 @@ export const WizardForm: React.FC = ({ }); return out; }, - [schema.sections, buildSectionFields, isCreateWizard], + [schema.sections, buildSectionFields, isCreateWizard, predicateScope], ); // Handle step data collection (merge partial data into formData) diff --git a/packages/plugin-form/src/wizardPredicateScope.test.tsx b/packages/plugin-form/src/wizardPredicateScope.test.tsx new file mode 100644 index 0000000000..083b1e66e1 --- /dev/null +++ b/packages/plugin-form/src/wizardPredicateScope.test.tsx @@ -0,0 +1,225 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6110 — `WizardForm`'s SUBMIT-TIME required re-check binds the host + * predicate scope, so `current_user` resolves there the way it already does in + * the form renderer (`packages/components/src/renderers/form/form.tsx`, #6010). + * + * ## What was broken, and why it is worse than a render-path gap + * + * `missingRequiredByStep` is not a render path. It re-checks the WHOLE declared + * field set at final submit, because `allowSkip` lets the user jump past a step + * whose fields were therefore never mounted, never registered and never + * validated. Its own docstring states the contract it is built to keep: + * + * > "the same one the form renderer and the server's rule-validator use, so a + * > conditionally required/hidden field gets the same verdict from all three + * > rather than a second, divergent dialect." + * + * Since #6010 the form renderer's verdict binds `current_user` and this gate's + * did not — so the wizard would HIDE a field from a user (predicate false, with + * the scope bound) and then, at submit, count that same field as visible + * (predicate faults open, with no scope) and refuse the submit on a control the + * submitter can neither see nor fill in. A dead end, with the wizard navigating + * to a step that shows nothing missing. + * + * ## ⚠️ Both fallback directions are exercised, on purpose + * + * `visibleWhen` / `visibleOn` fail OPEN (`fallback: true`), so an unbound scope + * makes a field MORE visible and blocks the submit. `requiredWhen` fails CLOSED + * (`fallback: false`), so an unbound scope makes a field LESS required and lets + * an invalid create through. Asserting only one direction would leave the pin + * satisfiable by whichever fallback happened to line up; asserting both is what + * makes this a measurement of the BINDING rather than of a default. + * + * The observable in every case is whether `dataSource.create` was called — + * i.e. whether the gate let the submit through — because that IS the harm the + * card names, and it is not reachable by an "is it on screen" assertion: the + * blocking field is on a step the user skipped and never renders either way. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { registerAllFields } from '@object-ui/fields'; +import { PredicateScopeProvider } from '@object-ui/react'; +import { WizardForm } from './WizardForm'; + +registerAllFields(); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** + * The host scope an `ExpressionProvider` publishes, transcribed — canonical + * `current_user` plus the ADR-0068 `user` / `ctx.user` / `os.user` aliases. + * Identical spelling to #6010's parity pin and to the console half of this + * card: one authored text, one verdict, every surface. + */ +function hostScope(positions: string[]) { + const user = { id: 'u1', name: 'Kim', positions }; + return { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} }; +} + +const DENIED = hostScope(['sales']); +const ALLOWED = hostScope(['sales_manager']); + +/** THE authored predicate — one text, asked of every surface. */ +const GATE = "'sales_manager' in current_user.positions"; + +/** A root bound in no scope at all — the FAULT control. */ +const UNBOUND_ROOT = "'sales_manager' in no_such_root.positions"; + +/** Object schema with per-case rule keys merged onto `owner`. */ +const makeDataSource = (ownerRules: Record = {}): any => ({ + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'case', + fields: { + subject: { type: 'text', label: 'Subject' }, + // Parked on the MIDDLE step — the one the user skips past, so it never + // mounts and only the submit-time gate can have an opinion about it. + owner: { type: 'text', label: 'Owner', ...ownerRules }, + notes: { type: 'text', label: 'Notes' }, + }, + }), + create: vi.fn().mockResolvedValue({ id: 'case-1' }), + update: vi.fn(), + findOne: vi.fn(), +}); + +const fill = (name: string, value: string) => { + const input = document.body.querySelector(`[data-field="${name}"] input`); + if (!input) throw new Error(`field not rendered: ${name}`); + fireEvent.change(input, { target: { value } }); +}; + +const stepIndicator = (index: number) => + document.body.querySelectorAll('nav[aria-label="Progress"] button')[index]; + +/** + * Render a 3-step wizard with `allowSkip`, jump past step 2, fill step 3 and + * press Create. Returns the dataSource so the caller can read whether the + * submit-time gate let it through. + * + * `ownerField` is step 2's single entry: a bare string (rules come from the + * OBJECT schema) or a spec object (whose `visibleWhen` is routed to the + * VIEW-level `visibleOn` slot by `normalizeSectionField`). + */ +async function skipPastOwnerAndSubmit( + scope: Record, + ownerField: string | Record, + ownerRules: Record = {}, +) { + const dataSource = makeDataSource(ownerRules); + render( + + + , + ); + + await waitFor(() => expect(document.body.querySelector('[data-field="subject"]')).toBeTruthy()); + fill('subject', 'S1'); + // Straight to the last step — step 2 is never mounted, so react-hook-form has + // no rule registered for `owner` and only `missingRequiredByStep` can speak. + fireEvent.click(stepIndicator(2)); + await waitFor(() => expect(document.body.querySelector('[data-field="notes"]')).toBeTruthy()); + fill('notes', 'S3'); + expect(document.body.querySelector('[data-field="owner"]')).toBeNull(); + + fireEvent.click(screen.getByRole('button', { name: /Create/i })); + return dataSource; +} + +/** The gate let the submit through. */ +async function expectSubmitted(dataSource: any) { + await waitFor(() => expect(dataSource.create).toHaveBeenCalled()); +} + +/** + * The gate REFUSED — measured by the wizard landing back on the step holding + * the field, which is the refusal's own observable, and then by `create` + * never having been called. + */ +async function expectBlocked(dataSource: any) { + await waitFor(() => expect(document.body.querySelector('[data-field="owner"]')).toBeTruthy()); + expect(dataSource.create).not.toHaveBeenCalled(); +} + +// ─── The deliverable: a field the wizard HIDES must not block the submit ── + +describe('#6110 — the submit-time gate binds `current_user` (fail-OPEN direction)', () => { + it('OBJECT-level `visibleWhen` false for the principal ⇒ the hidden required field does NOT block', async () => { + // resolveFieldRuleState, WizardForm.tsx — the card's first site. + const ds = await skipPastOwnerAndSubmit(DENIED, 'owner', { + required: true, + visibleWhen: GATE, + }); + await expectSubmitted(ds); + }); + + it('VIEW-level `visibleOn` false for the principal ⇒ the hidden required field does NOT block', async () => { + // evalFieldPredicate, WizardForm.tsx — the card's second site. Authored as + // the canonical `visibleWhen` on the SECTION FIELD, which + // `normalizeSectionField` routes into the view-level `visibleOn` slot. + const ds = await skipPastOwnerAndSubmit( + DENIED, + { field: 'owner', visibleWhen: GATE }, + { required: true }, + ); + await expectSubmitted(ds); + }); +}); + +describe('#6110 — the submit-time gate binds `current_user` (fail-CLOSED direction)', () => { + it('OBJECT-level `requiredWhen` true for the principal ⇒ the empty field DOES block', async () => { + // The direction no fail-open accident can reach: unbound, `requiredWhen` + // faults to `false`, the gate says nothing, and an invalid create is sent. + const ds = await skipPastOwnerAndSubmit(ALLOWED, 'owner', { requiredWhen: GATE }); + await expectBlocked(ds); + }); +}); + +// ─── Controls — green before AND after the fix, by construction ─────────── + +describe('#6110 controls — stated plainly: these pass on a revert', () => { + it('ALLOWED: the SAME predicate text, a principal it admits ⇒ still blocks', async () => { + const ds = await skipPastOwnerAndSubmit(ALLOWED, 'owner', { + required: true, + visibleWhen: GATE, + }); + await expectBlocked(ds); + }); + + it('FAULTED: a genuinely unbound ROOT still fails OPEN, so it still blocks', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + // Pinned so the cases above mean "evaluated and false" rather than "could + // not be evaluated at all" — the only difference from `GATE` is the ROOT. + const ds = await skipPastOwnerAndSubmit(DENIED, 'owner', { + required: true, + visibleWhen: UNBOUND_ROOT, + }); + await expectBlocked(ds); + }); +});