fix(metadata-core,metadata): a form SECTION binds current_user too, so the unbound-root notice stops flagging one

.changeset/form-predicate-root-section-binds-current-user.md

@@ -0,0 +1,59 @@
+---
+"@objectstack/metadata-core": patch
+"@objectstack/metadata": patch
+---
+
+fix(metadata-core,metadata): a form SECTION binds `current_user` too, so the unbound-root notice stops flagging one (#13072)
+
+Second correction to the unbound-root boot notice, and the same defect as the
+first one a surface later. The notice judged a SECTION-level predicate against
+`record` / `previous` / `parent` / `data`, sourced faithfully from the section
+contract prose — which was stale.
+
+`current_user` and its ADR-0068 alias roots (`user`, `ctx.user`, `os.user`)
+**resolve on a section-level `visibleWhen`**: objectui#6110 threads the host
+shell's predicate scope into `isSectionVisible` where it used to pass
+`undefined`, and objectui#6111 copies the authored `visibleWhen` onto the
+`section-divider` pseudo-field whose predicate the SDUI form renderer evaluates
+with that scope bound. #12914 re-measured the contract text accordingly. Until
+this change, a legacy artifact carrying a legitimate section-level
+`current_user.role == "admin"` predicate was reported at boot as an unbound root
+that faults open — a notice about a predicate that resolves, which is the
+cry-wolf failure the module's own doc forbids and the one that trains operators
+to ignore the channel.
+
+**What changes:** one vocabulary now serves both form-view predicate surfaces —
+`record`, `previous`, `parent`, `data`, `current_user`, `user`, `ctx`, `os`. A
+section predicate rooted at the `current_user` family is silent; a section
+predicate rooted at a bare field identifier is still reported, and the operator
+line still prints the rule per surface for the surfaces the findings implicate.
+
+**Blast radius, stated without inflation:** this is a **notice**, not a refusal
+— no parse change, no gate, no behaviour change, and it only runs inside the
+versioned window `applyArtifactForwardConversions` opens. The cost it removes is
+a false operator signal on legacy artifacts, not a broken runtime.
+
+**Removed export, with its migration:** `FIELD_ONLY_BOUND_PREDICATE_ROOTS` is
+gone from `@objectstack/metadata-core`. The section binding empties it, and an
+exported constant named `FIELD_ONLY_…` holding `[]` asserts a per-surface
+difference that no renderer makes. FROM → TO: read
+`BOUND_FORM_VIEW_PREDICATE_ROOTS` (every root bound on any form-view predicate)
+or `BOUND_FORM_FIELD_PREDICATE_ROOTS` (the field question, the same list today).
+No consumer can be carrying it: the notice has never shipped — the two
+changesets that introduce it are still pending in `.changeset/`, the newest
+published `@objectstack/metadata-core` is 17.2.0, and the commit that added
+`form-predicate-root-policy.ts` is in no release tag. This was the last moment
+at which the removal cost nothing.
+
+**Why the vocabulary is no longer justified by quoting the contract.** Both
+times this list has been wrong, it was wrong by transcribing a correct-looking
+sentence that the renderer had already moved past. The prose is a transcription
+of a renderer and can only lag one, so membership is now stated as the mechanism
+— *a root is bound on a surface iff some renderer threads a scope carrying it
+into that surface's evaluator* — with the threading site named per entry, and
+the module's test reads the LIVE `.describe()` text of
+`FormFieldSchema.visibleWhen` / `FormSectionSchema.visibleWhen` out of
+`@objectstack/spec` instead of copying it into a comment. A comment quoting that
+sentence goes stale in silence, twice now; an assertion that fetches it cannot.
+
+<!-- adr-0087: not-required (unpublished) the removed export FIELD_ONLY_BOUND_PREDICATE_ROOTS was added after 17.2.0 and is in no release tag, so no upgrader can be holding it and there is nothing to migrate from. -->

packages/metadata-core/src/form-predicate-root-policy.test.ts

@@ -12,10 +12,10 @@
  */
 
 import { describe, it, expect } from 'vitest';
+import { FormFieldSchema, FormSectionSchema } from '@objectstack/spec/ui';
 import {
   BOUND_FORM_VIEW_PREDICATE_ROOTS,
   BOUND_FORM_FIELD_PREDICATE_ROOTS,
-  FIELD_ONLY_BOUND_PREDICATE_ROOTS,
   detectUnboundFormViewPredicateRoots,
   unboundRootsInCelSource,
 } from './form-predicate-root-policy.js';
@@ -44,44 +44,126 @@ function definitionWithFieldPredicate(predicate: unknown, object = 'crm_lead'):
   };
 }
 
+/** The same artifact with the predicate on the SECTION rather than on a field. */
+function definitionWithSectionPredicate(predicate: unknown, object = 'crm_lead'): unknown {
+  return {
+    manifest: { id: 'app.test', engines: { protocol: '^17.0.0-rc.1' } },
+    views: [
+      {
+        form: {
+          type: 'simple',
+          data: { object },
+          sections: [
+            { name: 'main', visibleWhen: predicate, fields: [{ field: 'name' }] },
+          ],
+        },
+      },
+    ],
+  };
+}
+
 const CEL = (source: string) => ({ dialect: 'cel', source });
 
-describe('the bound vocabulary comes from the contract, not from this module', () => {
-  it('the shared base — and therefore the SECTION vocabulary — is record / previous / parent / data', () => {
-    // `packages/spec/src/ui/view.zod.ts`, `FormSectionSchema.visibleWhen`:
-    // "Root: `record` (+ `previous`, `parent`) in runtime forms, or `data` in
-    // metadata forms. No `current_user` at section level — it is unbound here
-    // and the predicate would fault open."
-    expect([...BOUND_FORM_VIEW_PREDICATE_ROOTS]).toEqual(['record', 'previous', 'parent', 'data']);
-    expect(BOUND_FORM_VIEW_PREDICATE_ROOTS).not.toContain('current_user');
-  });
+/**
+ * The `current_user` family as ROOT identifiers — `current_user` plus the
+ * ADR-0068 D1 aliases, whose two-segment spellings (`ctx.user`, `os.user`) put
+ * `ctx` and `os` in root position.
+ *
+ * Spelled out here rather than imported: this list used to BE an export
+ * (`FIELD_ONLY_BOUND_PREDICATE_ROOTS`), and asserting the module against its
+ * own constant would have made the cases below agree with any value it took.
+ */
+const CURRENT_USER_FAMILY_ROOTS = ['current_user', 'user', 'ctx', 'os'] as const;
+
+/**
+ * The `visibleWhen` contract sentence for one form-view surface, read LIVE out
+ * of `@objectstack/spec` instead of copied into a comment in this file.
+ *
+ * `FormFieldSchema` / `FormSectionSchema` are `strictObject(...).transform(...)`
+ * pipes (ADR-0089 D3a), so the authored `.describe()` text hangs off the pipe's
+ * INPUT shape, reached through zod 4's public `.in`. The guard is the point of
+ * the helper: if that accessor path ever moves, this file must go RED rather
+ * than hand every assertion below an `undefined` that quietly matches nothing.
+ */
+function visibleWhenContract(schema: unknown, label: string): string {
+  const shape = (schema as { in?: { shape?: Record<string, unknown> } }).in?.shape;
+  const slot = shape?.visibleWhen as { description?: unknown } | undefined;
+  const prose = slot?.description;
+  expect(
+    typeof prose === 'string' && prose.length > 0,
+    `${label}.visibleWhen description unreadable — the accessor this pin depends `
+      + 'on has moved. Fix the accessor; do not delete the pin.',
+  ).toBe(true);
+  return prose as string;
+}
 
-  it('the FIELD vocabulary adds the current_user family (objectui#6010, re-measured by #12930)', () => {
-    // `FormFieldSchema.visibleWhen`: "`current_user` (and the ADR-0068 aliases
-    // `user` / `ctx.user` / `os.user`) resolves here since objectui#6010".
-    // This is the correction: the first version of this policy judged a field
-    // by the section vocabulary and false-flagged a legitimate predicate.
-    expect([...BOUND_FORM_FIELD_PREDICATE_ROOTS]).toEqual([
+describe('the bound vocabulary is checked against the LIVE contract, not a copy of it', () => {
+  /**
+   * ⚠️ This block used to CARRY the section contract sentence as a comment:
+   *
+   *     "Root: `record` (+ `previous`, `parent`) in runtime forms, or `data` in
+   *      metadata forms. No `current_user` at section level — it is unbound
+   *      here and the predicate would fault open."
+   *
+   * #12914 replaced that sentence — objectui#6110 threads the host scope into
+   * `isSectionVisible`, objectui#6111 evaluates the section predicate on the
+   * `section-divider` pseudo-field with that scope bound — and the copy above
+   * went stale HERE in total silence, because no gate reads a comment. It is
+   * kept as history, the record of how this file failed, and replaced as a
+   * MECHANISM by the reads below: the sentence is now fetched from the schema
+   * at run time, so the next re-measurement of it fails this file instead of
+   * outliving it.
+   */
+  it('both surfaces bind the current_user family, and the live contract still says so', () => {
+    for (const [label, schema] of [
+      ['FormFieldSchema', FormFieldSchema],
+      ['FormSectionSchema', FormSectionSchema],
+    ] as const) {
+      const prose = visibleWhenContract(schema, label);
+      expect(prose, label).toMatch(/`current_user`[\s\S]{0,160}resolves here/);
+      for (const alias of ['`user`', '`ctx.user`', '`os.user`']) {
+        expect(prose, `${label} / ${alias}`).toContain(alias);
+      }
+    }
+
+    expect([...BOUND_FORM_VIEW_PREDICATE_ROOTS]).toEqual([
       'record', 'previous', 'parent', 'data',
       'current_user', 'user', 'ctx', 'os',
     ]);
-    // The field vocabulary is a strict superset — the base can never drift out
-    // from under it.
-    for (const root of BOUND_FORM_VIEW_PREDICATE_ROOTS) {
-      expect(BOUND_FORM_FIELD_PREDICATE_ROOTS, root).toContain(root);
+    for (const root of CURRENT_USER_FAMILY_ROOTS) {
+      expect(BOUND_FORM_VIEW_PREDICATE_ROOTS, root).toContain(root);
     }
   });
 
-  it('judges the SAME predicate differently per surface — the whole point of the split', () => {
+  it('the FIELD and SECTION vocabularies are ONE list, not two that happen to match', () => {
+    // By identity, not by value: while these were two constants they could
+    // drift apart silently, which is precisely what happened to the section
+    // half. Nothing can now update one surface and leave the other behind.
+    expect(BOUND_FORM_FIELD_PREDICATE_ROOTS).toBe(BOUND_FORM_VIEW_PREDICATE_ROOTS);
+  });
+
+  it('judges the SAME predicate identically on both surfaces — the split is empty', () => {
+    // ⚠️ INVERTED IN PLACE. Was "judges the SAME predicate differently per
+    // surface — the whole point of the split", expecting `['current_user']`
+    // from the section vocabulary. The section binds the root since
+    // objectui#6110 + #6111 (contract landed by #12914), so the section answer
+    // is now `[]` too, and a finding there would be a boot notice about a
+    // predicate that resolves.
     const source = 'current_user.id == record.owner';
     expect(unboundRootsInCelSource(source, BOUND_FORM_FIELD_PREDICATE_ROOTS)).toEqual([]);
-    expect(unboundRootsInCelSource(source, BOUND_FORM_VIEW_PREDICATE_ROOTS)).toEqual(['current_user']);
+    expect(unboundRootsInCelSource(source, BOUND_FORM_VIEW_PREDICATE_ROOTS)).toEqual([]);
   });
 
-  it('defaults to the stricter (section) vocabulary, so a forgetful caller fails loudly', () => {
-    // A missed detection is silent; a false positive is findable. The default
-    // is chosen to fail in the findable direction — the traversal never uses it.
-    expect(unboundRootsInCelSource('current_user.id == record.owner')).toEqual(['current_user']);
+  it('defaults to the whole vocabulary — the verdict the traversal itself gives', () => {
+    // ⚠️ INVERTED IN PLACE. Was "defaults to the stricter (section)
+    // vocabulary, so a forgetful caller fails loudly", expecting
+    // `['current_user']`. There is no stricter vocabulary left to default to,
+    // and narrowing one purely to preserve that property would manufacture the
+    // false positive this module exists to avoid.
+    expect(unboundRootsInCelSource('current_user.id == record.owner')).toEqual([]);
+    // Non-vacuity: the default still judges — a genuinely unbound root is
+    // still reported without the caller naming a vocabulary.
+    expect(unboundRootsInCelSource('status == "unqualified"')).toEqual(['status']);
   });
 });
 
@@ -171,22 +253,33 @@ describe('detectUnboundFormViewPredicateRoots — traversal', () => {
     ]);
   });
 
-  it('stays SILENT on a field predicate rooted at the current_user family', () => {
-    // The regression this patch exists for: each of these resolves at field
-    // level (objectui#6010), so flagging one is crying wolf on a legitimate,
-    // correctly-authored predicate.
-    for (const root of FIELD_ONLY_BOUND_PREDICATE_ROOTS) {
+  it('stays SILENT on a current_user-family predicate on EITHER surface', () => {
+    // Both regressions in one loop. Each of these resolves at FIELD level
+    // (objectui#6010) and at SECTION level (objectui#6110 + #6111), so
+    // flagging one is crying wolf on a legitimate, correctly-authored
+    // predicate — the failure the module doc forbids, once per surface.
+    for (const root of CURRENT_USER_FAMILY_ROOTS) {
       const source = root === 'ctx' || root === 'os'
         ? `${root}.user.role == "admin"`
         : `${root}.role == "admin"`;
       expect(
         detectUnboundFormViewPredicateRoots(definitionWithFieldPredicate(CEL(source))),
-        source,
+        `field / ${source}`,
+      ).toEqual([]);
+      expect(
+        detectUnboundFormViewPredicateRoots(definitionWithSectionPredicate(CEL(source))),
+        `section / ${source}`,
       ).toEqual([]);
     }
   });
 
-  it('still FLAGS the same root at SECTION level, where the contract says it is unbound', () => {
+  it('says NOTHING about the same root at SECTION level either — it binds there now', () => {
+    // ⚠️ INVERTED IN PLACE. This case asserted exactly ONE finding — the
+    // section slot — "where the contract says it is unbound", while the
+    // identical field predicate stayed silent. #12914 replaced that contract
+    // sentence, so the two slots now answer alike and the artifact below is
+    // healthy on both. A finding here would be a boot notice about a predicate
+    // that resolves, which the module doc names as worse than no notice.
     const findings = detectUnboundFormViewPredicateRoots({
       views: [
         {
@@ -202,14 +295,28 @@ describe('detectUnboundFormViewPredicateRoots — traversal', () => {
         },
       ],
     });
-    // Exactly one: the section slot. The identical field predicate is silent.
-    expect(findings).toHaveLength(1);
-    expect(findings[0]!.surface).toBe('section');
-    expect(findings[0]!.root).toBe('current_user');
-    expect(findings[0]!.path).toBe('views[0].form.sections[0].visibleWhen');
+    expect(findings).toEqual([]);
+  });
+
+  it('still flags a genuinely unbound SECTION root — the silence above is not blanket', () => {
+    // The control the inversion above needs: the section arm of the traversal
+    // still reports, so "no finding" there is a verdict about `current_user`
+    // and not a section scan that stopped running.
+    const findings = detectUnboundFormViewPredicateRoots(
+      definitionWithSectionPredicate(CEL('stage == "closed"')),
+    );
+    expect(findings).toEqual([
+      {
+        path: 'views[0].form.sections[0].visibleWhen',
+        view: 'crm_lead',
+        root: 'stage',
+        source: 'stage == "closed"',
+        surface: 'section',
+      },
+    ]);
   });
 
-  it('tags every finding with the surface that decided its vocabulary', () => {
+  it('tags every finding with the slot it sits in', () => {
     const findings = detectUnboundFormViewPredicateRoots({
       views: [
         {