fix(form): harden the spec↔runtime form-field chokepoint, derive SelectOption, complete FormFieldSchema (#3090 PR1)

.changeset/form-field-chokepoint-hardening.md

@@ -0,0 +1,34 @@
+---
+"@object-ui/types": patch
+"@object-ui/plugin-form": patch
+---
+
+fix(form): the spec↔runtime form-field chokepoint stops dropping spec 17 vocabulary, and the validator stops contradicting the renderer — #3090
+
+`normalizeSectionField` — the one translation point between the spec's authored
+form-field shape (`field` = object-field reference) and the runtime shape
+(`name` = data path) — silently dropped four spec keys, worst of all the
+ADR-0089 **canonical** `visibleWhen` spelling while the deprecated `visibleOn`
+worked. Now:
+
+- view-level `visibleWhen` routes into the view-level slot (`visibleOn`) so it
+  ANDs with the object-level rule instead of clobbering it, and the wizard's
+  final-submit gate folds the same slot into its verdict (before, a required
+  field the view itself hides could block submission from off-screen);
+- `dependsOn`, `keyField`, and `disclosure` carry through;
+- a behavioral parity gate walks the spec `FormFieldSchema` key set — a key the
+  spec adds fails as unmapped, a key it retires fails as stale.
+
+`SelectOptionSchema` is now derived from `@objectstack/spec/data` by reference
+(it used to strip `color` — which `@object-ui/fields` renders — plus `default`
+and the per-option `visibleWhen` gate), with pinned divergences (`value`
+widened for UI forms, `visibleWhen` on the #2212 wire contract) and documented
+UI-only extensions (`disabled`, `icon`). `SelectOption` (TS) gains `color` and
+`default`.
+
+`FormFieldSchema` (the runtime vocabulary `objectui validate` enforces) now
+covers every key the `FormField` interface declares — `widget`, `dependsOn`,
+`hidden`, `readonly`, `visibleOn`/`visibleWhen`/`readonlyWhen`/`requiredWhen`,
+`span` — and `type` is optional, matching the interface. A typo'd predicate now
+fails loudly instead of being stripped; spec-shape fields (`{ field: … }`) are
+still rejected, pinning the two-layer boundary.

packages/plugin-form/src/WizardForm.tsx

@@ -17,7 +17,7 @@ import React, { useState, useCallback, useMemo } from 'react';
 import type { FormField, DataSource } from '@object-ui/types';
 import { Button, cn, toast } from '@object-ui/components';
 import { AlertCircle, Check, ChevronLeft, ChevronRight, Loader2 } from 'lucide-react';
-import { resolveFieldRuleState } from '@object-ui/core';
+import { resolveFieldRuleState, evalFieldPredicate } from '@object-ui/core';
 import { createSafeTranslation } from '@object-ui/i18n';
 import { FormSection } from './FormSection';
 import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react';
@@ -352,8 +352,16 @@ export const WizardForm: React.FC<WizardFormProps> = ({
             record,
             { required: !!field.required, readonly: (field as any).readonly === true },
           );
+          // View-level FormField.visibleOn hides the field the same way a
+          // field-level visibleWhen does — fold it into the verdict exactly
+          // like the form renderer (form.tsx) does, or the gate demands a
+          // field the view itself hides (#3090). Since #3090 this slot also
+          // receives the ADR-0089 canonical view-level `visibleWhen` spelling.
+          const viewVisible =
+            (field as any).visibleOn == null ||
+            evalFieldPredicate((field as any).visibleOn, record, true);
           // A hidden or read-only field is not the user's to fill in.
-          if (!state.visible || state.readonly || !state.required) continue;
+          if (!viewVisible || !state.visible || state.readonly || !state.required) continue;
           if (!isEmptyValue(record[name])) continue;
           out.set(index, [...(out.get(index) ?? []), (field as any).label || name]);
         }

packages/plugin-form/src/sectionFields.spec-parity.test.ts

@@ -0,0 +1,138 @@
+/**
+ * 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.
+ */
+
+/**
+ * `normalizeSectionField` ↔ `@objectstack/spec` FormFieldSchema coverage gate
+ * (#3090, objectstack#4115).
+ *
+ * The normalizer is THE chokepoint between the two form-field vocabularies:
+ * the spec's authored shape (`field` = object-field reference, presentation
+ * deltas only) and the runtime shape (`name` = data path, self-contained
+ * widget config). It cannot be a derivation — every key is a *translation*
+ * with a destination of its own — so the gate is behavioral instead:
+ *
+ *   - TABLE below holds one row per spec key: a sample authored value and an
+ *     assertion that the normalized output reflects it at its documented
+ *     destination. Deleting a mapping line in `sectionFields.ts` fails the
+ *     row — the rows are mutation-tested coverage, not a mirror list.
+ *   - The key sets are then compared BOTH ways against the spec's own
+ *     enumeration: a key the spec adds fails as UNMAPPED (decide: map it or
+ *     exempt it with a reason); a key the spec retires fails as STALE.
+ *
+ * Until this gate existed the drift was real: the ADR-0089 canonical
+ * `visibleWhen` spelling, `dependsOn`, `keyField` and `disclosure` were all
+ * silently dropped while the DEPRECATED `visibleOn` spelling worked.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { FormFieldSchema as SpecFormFieldSchema } from '@objectstack/spec/ui';
+import { mapFieldTypeToFormType } from '@object-ui/fields';
+import { normalizeSectionField } from './sectionFields';
+
+const objectSchema = {
+  name: 'crm_account',
+  fields: {
+    industry: {
+      type: 'select',
+      label: 'Industry',
+      options: [{ label: 'Tech', value: 'tech' }],
+    },
+  },
+};
+
+const ctx = {
+  objectSchema,
+  objectName: 'crm_account',
+  fieldLabel: (_obj: string, _name: string, fallback: string) => fallback || _name,
+};
+
+/** Normalize a spec field def against a schema that does NOT define `ghost`,
+ * so every override is observable verbatim (no object-metadata interference). */
+const norm = (def: Record<string, unknown>) =>
+  normalizeSectionField({ field: 'ghost', ...def }, ctx) as any;
+
+/**
+ * One row per spec FormFieldSchema key: sample authored value → where the
+ * runtime FormField must carry it. Non-obvious destinations are the point:
+ * `helpText`→`description`, `readonly`→`disabled`, and BOTH visibility
+ * spellings→the view-level `visibleOn` slot (the runtime `visibleWhen` slot
+ * belongs to the object-level rule and must not be clobbered).
+ */
+const TABLE: Record<string, (f: (def: Record<string, unknown>) => any) => void> = {
+  field: () => {
+    const out = normalizeSectionField({ field: 'industry' }, ctx) as any;
+    expect(out.name).toBe('industry'); // identity key translated to the data path
+    expect(out.type).toBe(mapFieldTypeToFormType('select')); // object schema merged in
+  },
+  type: (f) => expect(f({ type: 'text' }).type).toBe(mapFieldTypeToFormType('text')),
+  options: (f) =>
+    expect(f({ options: [{ label: 'A', value: 'a' }] }).options).toEqual([
+      { label: 'A', value: 'a' },
+    ]),
+  reference: (f) => {
+    const out = f({ reference: 'accounts' });
+    expect(out.reference).toBe('accounts');
+    expect(out.reference_to).toBe('accounts'); // both spellings stamped (#2407)
+  },
+  maxLength: (f) => expect(f({ maxLength: 10 }).maxLength).toBe(10),
+  minLength: (f) => expect(f({ minLength: 2 }).minLength).toBe(2),
+  min: (f) => expect(f({ min: 1 }).min).toBe(1),
+  max: (f) => expect(f({ max: 9 }).max).toBe(9),
+  precision: (f) => expect(f({ precision: 10 }).precision).toBe(10),
+  scale: (f) => expect(f({ scale: 2 }).scale).toBe(2),
+  multiple: (f) => expect(f({ multiple: true }).multiple).toBe(true),
+  label: (f) => expect(f({ label: 'Custom' }).label).toBe('Custom'),
+  placeholder: (f) => expect(f({ placeholder: 'Type…' }).placeholder).toBe('Type…'),
+  helpText: (f) => expect(f({ helpText: 'Hint' }).description).toBe('Hint'),
+  readonly: (f) => expect(f({ readonly: true }).disabled).toBe(true),
+  immutable: (f) => expect(f({ immutable: true }).immutable).toBe(true),
+  required: (f) => expect(f({ required: true }).required).toBe(true),
+  hidden: (f) => expect(f({ hidden: true }).hidden).toBe(true),
+  colSpan: (f) => expect(f({ colSpan: 2 }).colSpan).toBe(2),
+  span: (f) => expect(f({ span: 'full' }).span).toBe('full'),
+  widget: (f) => expect(f({ widget: 'rating' }).widget).toBe('rating'),
+  language: (f) => expect(f({ language: 'sql' }).language).toBe('sql'),
+  fields: (f) =>
+    expect(f({ fields: [{ field: 'inner' }] }).fields).toEqual([{ field: 'inner' }]),
+  keyField: (f) =>
+    expect(f({ keyField: { field: 'name' } }).keyField).toEqual({ field: 'name' }),
+  dependsOn: (f) => expect(f({ dependsOn: 'country' }).dependsOn).toBe('country'),
+  visibleWhen: (f) =>
+    expect(f({ visibleWhen: 'record.a == 1' }).visibleOn).toBe('record.a == 1'),
+  visibleOn: (f) =>
+    expect(f({ visibleOn: 'record.b == 2' }).visibleOn).toBe('record.b == 2'),
+  disclosure: (f) => expect(f({ disclosure: 'popover' }).disclosure).toBe('popover'),
+};
+
+/** Spec keys the normalizer deliberately does not carry, each with a reason.
+ * Empty today — add entries here (never silently) when the spec grows a key
+ * that genuinely has no runtime destination. */
+const EXEMPT: Record<string, string> = {};
+
+describe('normalizeSectionField covers the spec FormFieldSchema key set', () => {
+  // `.strict().transform()` wraps the object in a ZodPipe; `.in` is the
+  // strict object carrying the authoring shape.
+  const specKeys = Object.keys((SpecFormFieldSchema as any).in.shape).sort();
+
+  it('has a behavioral row (or an exemption with a reason) for every spec key', () => {
+    const covered = [...Object.keys(TABLE), ...Object.keys(EXEMPT)].sort();
+    expect(covered).toEqual(specKeys);
+  });
+
+  it('has no stale rows for keys the spec has retired', () => {
+    for (const key of [...Object.keys(TABLE), ...Object.keys(EXEMPT)]) {
+      expect(specKeys, `'${key}' is no longer a spec FormField key`).toContain(key);
+    }
+  });
+
+  for (const [key, assertRow] of Object.entries(TABLE)) {
+    it(`carries spec '${key}' to its runtime destination`, () => {
+      assertRow(norm);
+    });
+  }
+});

packages/plugin-form/src/sectionFields.test.ts

@@ -106,6 +106,71 @@ describe('normalizeSectionField', () => {
     expect((f as any).visibleOn).toEqual(expr);
   });
 
+  // ── Spec 17 late-added / renamed keys (#3090) ─────────────────────────────
+  // ADR-0089 renamed the view-level predicate to `visibleWhen` — which is also
+  // the runtime slot for the OBJECT-level rule. The view predicate must land in
+  // the view-level slot (`visibleOn`) so the renderer ANDs both layers
+  // (form.tsx evaluates the two slots independently) instead of one clobbering
+  // the other. Before the fix the canonical spelling was silently dropped while
+  // the DEPRECATED spelling worked.
+
+  it('routes a view-level `visibleWhen` (canonical spelling) into the view-level slot', () => {
+    const f = normalizeSectionField(
+      { field: 'name', visibleWhen: "record.stage == 'won'" },
+      ctx,
+    ) as any;
+    expect(f.visibleOn).toBe("record.stage == 'won'");
+  });
+
+  it('carries a `{ dialect, source }` view-level visibleWhen expression', () => {
+    const expr = { dialect: 'cel', source: "record.priority == 'urgent'" };
+    const f = normalizeSectionField({ field: 'name', visibleWhen: expr }, ctx) as any;
+    expect(f.visibleOn).toEqual(expr);
+  });
+
+  it('layers the view predicate OVER the object-level rule instead of clobbering it', () => {
+    const rulesCtx = {
+      ...ctx,
+      objectSchema: {
+        ...objectSchema,
+        fields: {
+          ...objectSchema.fields,
+          paid_on: { type: 'date', label: 'Paid on', visibleWhen: "record.status == 'paid'" },
+        },
+      },
+    };
+    const f = normalizeSectionField(
+      { field: 'paid_on', visibleWhen: 'record.amount > 0' },
+      rulesCtx,
+    ) as any;
+    expect(f.visibleWhen).toBe("record.status == 'paid'"); // object-level rule intact
+    expect(f.visibleOn).toBe('record.amount > 0'); // view predicate in the view slot
+  });
+
+  it('prefers the canonical spelling when both visibleWhen and deprecated visibleOn are authored', () => {
+    // `saveMeta` persists verbatim, so served metadata can carry either (or,
+    // after a partial migration, both). Canonical wins.
+    const f = normalizeSectionField(
+      { field: 'name', visibleWhen: "record.a == 1", visibleOn: "record.b == 2" },
+      ctx,
+    ) as any;
+    expect(f.visibleOn).toBe("record.a == 1");
+  });
+
+  it('carries a view-level dependsOn (spec cascading declaration)', () => {
+    const f = normalizeSectionField({ field: 'industry', dependsOn: 'country' }, ctx) as any;
+    expect(f.dependsOn).toBe('country');
+  });
+
+  it('carries keyField and disclosure through for record/composite widgets', () => {
+    const f = normalizeSectionField(
+      { field: 'billing_address', keyField: { field: 'name', immutable: true }, disclosure: 'popover' },
+      ctx,
+    ) as any;
+    expect(f.keyField).toEqual({ field: 'name', immutable: true });
+    expect(f.disclosure).toBe('popover');
+  });
+
   it('copies field-level conditional rules from the object schema (#2212)', () => {
     const rulesSchema = {
       ...objectSchema,

packages/plugin-form/src/sectionFields.ts

@@ -152,8 +152,25 @@ export function normalizeSectionField(
   if (fd.scale != null) base.scale = fd.scale;
   if (fd.language != null) base.language = fd.language;
   if (Array.isArray(fd.fields)) base.fields = fd.fields;
+  // View-level cascading declaration (spec FormField.dependsOn, a bare parent
+  // name). The form renderer gates + recomputes options off the runtime
+  // field's top-level `dependsOn`; without this copy the declaration vanished.
+  if (fd.dependsOn != null) base.dependsOn = fd.dependsOn;
+  // Record/composite widget config (ADR-0007). Pass-through: no widget reads
+  // them from the runtime field yet, but dropping them here would make that
+  // support impossible to ship metadata-first.
+  if (fd.keyField != null) base.keyField = fd.keyField;
+  if (fd.disclosure != null) base.disclosure = fd.disclosure;
 
-  return attachVisibility(base as FormField, fd.visibleOn);
+  // View-level visibility predicate. ADR-0089 renamed it to `visibleWhen` —
+  // which collides with the runtime slot holding the OBJECT-level rule (copied
+  // from the object schema in `fromObjectSchema`). Route the view predicate
+  // into the view-level slot (`visibleOn`) instead: the renderer evaluates the
+  // two slots independently and ANDs them, so layering is preserved and the
+  // object rule is never clobbered. Canonical spelling wins over the
+  // deprecated one when both are authored (`saveMeta` persists verbatim, so
+  // served metadata can carry either).
+  return attachVisibility(base as FormField, fd.visibleWhen ?? fd.visibleOn);
 }
 
 /** Normalize every field def in a section. */

packages/plugin-form/src/wizardSkipValidation.test.tsx

@@ -175,6 +175,36 @@ describe('WizardForm allowSkip — required fields on a skipped step (#2959)', (
     expect(stepIndicator(1)).not.toHaveAttribute('data-error');
   });
 
+  it('does not demand a required field the view itself hides (view-level visibleWhen false)', async () => {
+    // The section field def carries the SPEC vocabulary: `field` + a view-level
+    // `visibleWhen` (ADR-0089 canonical spelling). `normalizeSectionField`
+    // routes that predicate into the view-level slot (`visibleOn`, #3090) and
+    // the renderer hides the field — so the submit gate must fold the same
+    // slot into its verdict, or it demands a field the user cannot see.
+    const dataSource = renderWizard({
+      allowSkip: true,
+      sections: [
+        { name: 's1', label: 'Step 1', fields: ['subject'] },
+        {
+          name: 's2',
+          label: 'Step 2',
+          fields: [{ field: 'owner', visibleWhen: "record.subject == 'urgent'" }],
+        },
+        { name: 's3', label: 'Step 3', fields: ['notes'] },
+      ],
+    });
+
+    await waitFor(() => expect(document.body.querySelector('[data-field="subject"]')).toBeTruthy());
+    fill('subject', 'routine'); // predicate false → owner is view-hidden
+    fireEvent.click(stepIndicator(2));
+    await waitFor(() => expect(document.body.querySelector('[data-field="notes"]')).toBeTruthy());
+    fill('notes', 'S3');
+    fireEvent.click(screen.getByRole('button', { name: /Create/i }));
+
+    await waitFor(() => expect(dataSource.create).toHaveBeenCalledTimes(1));
+    expect(stepIndicator(1)).not.toHaveAttribute('data-error');
+  });
+
   it('still lets the sequential wizard submit (the gate is not a new blocker)', async () => {
     const dataSource = renderWizard();