fix(flow-designer): read approver value sources off the schema instead of mirroring them (framework#3508 follow-up) (#2910)

os-zhuang · web-flow · commit eea43917739b · 2026-07-28T15:49:46.000+08:00
The approver Value picker decided WHERE its candidates live from a local table
hand-mirrored from the spec's `APPROVER_VALUE_BINDINGS`. That mirror is what
made framework#3508 possible: `xRef.map` names a picker KIND and nothing more,
so this package had to choose a data source itself — and chose the metadata
REGISTRY, which lists no directory rows.

The spec now publishes the contract as `xRef.sources` (objectstack#3817).
`json-schema-to-fields` carries it through, validating each entry, and
`recordLookupFor()` prefers it over the local table. Presentation stays local,
and the local table remains the fallback for a server that predates the
annotation.

Also corrects the approver `type` options comment in `flow-node-config.ts`:
that list is the offline fallback, which is why the "indirect bindings lead"
order stated only there never reached the live picker.
diff --git a/.changeset/approver-value-sources-from-schema.md b/.changeset/approver-value-sources-from-schema.md
@@ -0,0 +1,36 @@
+---
+"@object-ui/app-shell": patch
+---
+
+fix(flow-designer): read approver value sources off the schema instead of mirroring them (framework#3508 follow-up)
+
+The approver Value picker decided *where its candidates live* from a local
+table, `KIND_TO_RECORD_LOOKUP`, hand-mirrored from the spec's
+`APPROVER_VALUE_BINDINGS`. That mirror is what made framework#3508 possible:
+`xRef.map` names a picker KIND (`'team'`) and nothing more, so this package had
+to pick a data source itself — and picked the metadata REGISTRY
+(`GET /api/v1/meta/:type`), which lists no `sys_user` / `sys_team` /
+`sys_business_unit` / `sys_position` ROWS. Candidates were always empty and the
+control degraded to a raw-id text box.
+
+The spec now publishes the data contract as `xRef.sources` (one entry per
+approver type: `{ source: 'data', object, valueField }`, the closed enum
+inline, or a non-picker marker). `json-schema-to-fields` carries it through —
+validating each entry, dropping any that could not drive a picker — and
+`recordLookupFor()` prefers it over the local table. A new approver type can no
+longer leave a stale mirror behind here.
+
+What did NOT move: presentation. Which field to display, whether to open the
+people picker, what subtitle to show under a row stay this package's calls, so
+the spec ships the data contract and not the look. The local table remains as
+the fallback for a server that predates the annotation, and a `data` source for
+a kind with no presentation entry still renders a lookup labelled by its
+committed column — better than degrading a resolvable reference to free text.
+
+Also corrects the approver `type` options comment in `flow-node-config.ts`: that
+list is the OFFLINE fallback (`FlowNodeInspector` renders
+`serverFields ?? fieldsForNodeType(...)`, so a real backend's published
+configSchema wins). Its "indirect bindings lead, `user` last" ordering therefore
+never reached the live picker, which followed the spec enum with `user` first —
+the opposite of the intent. The ordering now lives in the spec's `ApproverType`
+enum, and the comment says which list is authoritative.
diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.sources.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.sources.test.tsx
@@ -0,0 +1,105 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * framework #3508 follow-up — the approver Value picker now reads WHERE its
+ * candidates live off the published schema (`xRef.sources`) instead of a local
+ * mirror of the spec's `APPROVER_VALUE_BINDINGS`.
+ *
+ * The mirror is what made #3508 possible in the first place: `xRef.map` named
+ * a picker KIND and nothing more, so this package had to decide the data
+ * source itself — and decided wrong, wiring every directory kind to the
+ * metadata registry, which holds no `sys_user` / `sys_team` rows.
+ *
+ * Load-bearing behaviours:
+ *   1. a published `data` source WINS over the local table (that is what stops
+ *      the mirror from drifting from the engine again);
+ *   2. no published source → the local table still drives the picker, so an
+ *      older server keeps working;
+ *   3. a published NON-data source (enum / auto / unsupported) keeps the kind
+ *      off the record-lookup path entirely;
+ *   4. presentation (display field, people-picker, subtitle) stays local — the
+ *      spec ships the data contract, not the look;
+ *   5. the source is keyed by the DISCRIMINATOR, not the picker kind, so
+ *      `role` and `org_membership_level` stay distinguishable though they
+ *      share one kind.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { recordLookupFor, resolveRefKind, KIND_TO_RECORD_LOOKUP } from './FlowReferenceField';
+
+describe('recordLookupFor — published source vs local mirror (#3508 follow-up)', () => {
+  it('prefers the schema’s object and committed column over the local table', () => {
+    // A server that renamed the directory object: the picker must follow the
+    // SCHEMA, or it queries a table the engine does not resolve against.
+    const lookup = recordLookupFor({
+      kind: 'user',
+      source: { source: 'data', object: 'sys_person', valueField: 'external_id' },
+    });
+    expect(lookup).toMatchObject({ object: 'sys_person', valueField: 'external_id' });
+    // …while presentation still comes from here.
+    expect(lookup?.picker).toBe('search');
+    expect(lookup?.subtitle).toEqual(['email']);
+  });
+
+  it('falls back to the local table when the server publishes no source', () => {
+    expect(recordLookupFor({ kind: 'department' })).toEqual(KIND_TO_RECORD_LOOKUP.department);
+    expect(recordLookupFor({ kind: 'position' })).toEqual(KIND_TO_RECORD_LOOKUP.position);
+  });
+
+  it('keeps a published non-data source off the record path', () => {
+    // These are not pickers over rows: a closed enum, a runtime-resolved
+    // value, and a type the runtime never resolves at all.
+    expect(recordLookupFor({ kind: 'org-membership-level', source: { source: 'enum', values: ['owner'] } })).toBeUndefined();
+    expect(recordLookupFor({ kind: 'manager', source: { source: 'auto' } })).toBeUndefined();
+    expect(recordLookupFor({ kind: 'queue', source: { source: 'unsupported' } })).toBeUndefined();
+  });
+
+  it('renders a lookup for a data source this package has no presentation entry for', () => {
+    // Better a working picker labelled by its committed column than degrading
+    // a perfectly resolvable reference to free text.
+    const lookup = recordLookupFor({
+      kind: 'queue',
+      source: { source: 'data', object: 'sys_queue', valueField: 'id' },
+    });
+    expect(lookup).toEqual({ object: 'sys_queue', valueField: 'id', displayField: 'id' });
+  });
+
+  it('resolves nothing for an unresolved reference', () => {
+    expect(recordLookupFor(undefined)).toBeUndefined();
+  });
+});
+
+describe('resolveRefKind — sources are keyed by the discriminator (#3508 follow-up)', () => {
+  const ref = {
+    kindFrom: 'type',
+    map: { user: 'user', role: 'org-membership-level', org_membership_level: 'org-membership-level' },
+    sources: {
+      user: { source: 'data' as const, object: 'sys_user', valueField: 'id' },
+      role: { source: 'enum' as const, values: ['owner', 'admin', 'member'] },
+      org_membership_level: { source: 'enum' as const, values: ['owner', 'admin', 'member'] },
+    },
+  };
+
+  it('carries the source for the row’s own type', () => {
+    const resolved = resolveRefKind(ref, () => 'user');
+    expect(resolved).toMatchObject({ kind: 'user', source: { source: 'data', object: 'sys_user' } });
+  });
+
+  it('distinguishes two discriminators that share one picker kind', () => {
+    // `role` is the deprecated spelling of `org_membership_level`; both render
+    // the same control, and a per-KIND lookup could not tell them apart.
+    expect(resolveRefKind(ref, () => 'role')?.source).toEqual({ source: 'enum', values: ['owner', 'admin', 'member'] });
+    expect(resolveRefKind(ref, () => 'org_membership_level')?.kind).toBe('org-membership-level');
+  });
+
+  it('leaves the source undefined when the server published none', () => {
+    const bare = { kindFrom: 'type', map: { user: 'user' as const } };
+    expect(resolveRefKind(bare, () => 'user')).toEqual({
+      kind: 'user', objectSource: undefined, connectorSource: undefined, source: undefined,
+    });
+  });
+
+  it('still returns undefined for a discriminator with no mapped kind', () => {
+    expect(resolveRefKind(ref, () => 'nope')).toBeUndefined();
+  });
+});
diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowReferenceField.tsx
@@ -41,7 +41,7 @@ import {
 import { Pencil, Search } from 'lucide-react';
 import { useAdapter } from '@object-ui/react';
 import { LookupField } from '@object-ui/fields';
-import type { FlowReferenceSpec, ReferenceKind } from './flow-node-config';
+import type { FlowReferenceSpec, ReferenceKind, RefValueSource } from './flow-node-config';
 import { useMetadataClient } from '../useMetadata';
 import { useObjectFields } from '../previews/useObjectFields';
 
@@ -127,6 +127,13 @@ export interface ResolvedRef {
   kind: ReferenceKind;
   objectSource?: string;
   connectorSource?: string;
+  /**
+   * The schema's own answer for where this kind's candidates live, when the
+   * server published one (framework#3508 follow-up). Takes precedence over
+   * {@link KIND_TO_RECORD_LOOKUP}, which stays as the fallback for a server
+   * that predates the annotation.
+   */
+  source?: RefValueSource;
 }
 
 /**
@@ -140,15 +147,61 @@ export function resolveRefKind(
   sibling: (key: string) => unknown,
 ): ResolvedRef | undefined {
   if (!ref) return undefined;
-  if (ref.kind) return { kind: ref.kind, objectSource: ref.objectSource, connectorSource: ref.connectorSource };
+  if (ref.kind) {
+    return {
+      kind: ref.kind,
+      objectSource: ref.objectSource,
+      connectorSource: ref.connectorSource,
+      source: ref.sources?.[ref.kind],
+    };
+  }
   if (ref.kindFrom && ref.map) {
     const disc = sibling(ref.kindFrom);
     const k = typeof disc === 'string' ? ref.map[disc] : undefined;
-    if (k) return { kind: k, objectSource: ref.objectSource, connectorSource: ref.connectorSource };
+    // The source is keyed by the DISCRIMINATOR (the approver `type`), not by
+    // the picker kind: `role` and `org_membership_level` share one kind but
+    // are separate enum members upstream.
+    if (k) {
+      return {
+        kind: k,
+        objectSource: ref.objectSource,
+        connectorSource: ref.connectorSource,
+        source: typeof disc === 'string' ? ref.sources?.[disc] : undefined,
+      };
+    }
   }
   return undefined;
 }
 
+/**
+ * The record lookup to render for a resolved reference.
+ *
+ * The schema's `xRef.sources` wins when present — it is the spec's own
+ * `APPROVER_VALUE_BINDINGS`, so it cannot drift from what the engine resolves.
+ * {@link KIND_TO_RECORD_LOOKUP} supplies the fallback (older server) and, in
+ * both paths, the PRESENTATION: which field to show, whether to open the
+ * people picker, what subtitle to put under a row. Those are this package's
+ * calls and deliberately stay out of the spec.
+ *
+ * A `data` source for a kind this package has no presentation entry for still
+ * renders a lookup — display falls back to the committed column, which beats
+ * degrading a resolvable reference to free text.
+ */
+export function recordLookupFor(resolved: ResolvedRef | undefined): RecordLookupBinding | undefined {
+  if (!resolved) return undefined;
+  const local = KIND_TO_RECORD_LOOKUP[resolved.kind];
+  const published = resolved.source;
+  if (published && published.source !== 'data') return undefined;
+  if (!published) return local;
+  return {
+    object: published.object,
+    valueField: published.valueField as RecordLookupBinding['valueField'],
+    displayField: local?.displayField ?? published.valueField,
+    ...(local?.picker ? { picker: local.picker } : {}),
+    ...(local?.subtitle ? { subtitle: local.subtitle } : {}),
+  };
+}
+
 /** Read `node.config[key]` as a non-empty string, else undefined. */
 function configString(node: Record<string, unknown> | null | undefined, key: string): string | undefined {
   const cfg = node?.config;
@@ -518,7 +571,10 @@ export function ReferenceCombobox({ resolved, value, onCommit, onBlur, onSelect,
   // (All hooks above have already run — the branches below only render.)
 
   // Directory-backed kinds → single-select record lookup (framework #3508).
-  const lookup = kind ? KIND_TO_RECORD_LOOKUP[kind] : undefined;
+  // The object + committed column come from the schema when the server
+  // publishes them, so this package no longer decides where the engine's
+  // approvers live — see `recordLookupFor`.
+  const lookup = recordLookupFor(resolved);
   if (lookup) {
     return (
       <RecordLookupCell
diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts
@@ -127,8 +127,41 @@ export interface FlowReferenceSpec {
    */
   kindFrom?: string;
   map?: Record<string, ReferenceKind>;
+  /**
+   * Where each discriminator value's candidates actually live, keyed like
+   * {@link map} — published by the spec as `xRef.sources` (framework #3508
+   * follow-up) and carried through by `json-schema-to-fields`.
+   *
+   * {@link map} only ever named a picker KIND. It never said what backs that
+   * picker, so this package had to keep its own copy of the data contract —
+   * and the first copy pointed every directory kind at the metadata REGISTRY
+   * (`GET /api/v1/meta/:type`), which cannot list `sys_user` / `sys_team` /
+   * `sys_business_unit` / `sys_position` ROWS. Candidates came back empty and
+   * the control silently degraded to free text (framework#3508). Reading the
+   * source off the schema means a new approver type can no longer leave a
+   * stale mirror behind here.
+   *
+   * Absent when the server predates the annotation — consumers keep a local
+   * fallback for that case.
+   */
+  sources?: Record<string, RefValueSource>;
 }
 
+/**
+ * How a polymorphic reference's candidates are sourced, per discriminator
+ * value. Mirrors the spec's `APPROVER_VALUE_SOURCES` projection.
+ *
+ * `data` means the DATA API (`/api/v1/data/:object`) — named in deliberate
+ * contrast to `meta`, the registry this used to query by mistake. The other
+ * variants are not pickers at all: a closed `enum`, an `auto`-resolved value, a
+ * `trigger-field` name, a CEL `expression`, or an `unsupported` type the
+ * runtime never resolves.
+ */
+export type RefValueSource =
+  | { source: 'data'; object: string; valueField: string }
+  | { source: 'enum'; values: string[] }
+  | { source: 'auto' | 'trigger-field' | 'expression' | 'unsupported' };
+
 /** Column descriptor for an `objectList` repeater row. */
 export interface FlowConfigColumn {
   key: string;
@@ -537,14 +570,25 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
           key: 'type',
           label: 'Type',
           kind: 'select',
-          // Mirrors the spec's NON_AUTHORABLE_APPROVER_TYPES (approval.zod.ts):
-          // `role` is the deprecated spelling of `org_membership_level`
-          // (ADR-0090 D3), and `queue` is declared-but-unenforced — the runtime
-          // resolves it to nobody (framework #3508). A stored row of either
-          // still renders (the select surfaces it flagged "(deprecated)"), but
-          // the designer never authors a new one. Indirect bindings lead and
-          // the literal `user` binding comes last: binding a specific person is
-          // the least portable choice (env moves, people leave).
+          // OFFLINE FALLBACK ONLY. `FlowNodeInspector` renders
+          // `serverFields ?? fieldsForNodeType(...)`, so against a real backend
+          // the approval node's fields come from the engine-published
+          // configSchema and this list is never read — it covers the preview
+          // gallery and any stack whose server publishes no schema.
+          //
+          // That is why the ordering below is ALSO carried by the spec's
+          // `ApproverType` enum (framework#3508 follow-up): stating it only
+          // here left the live picker in enum order with `user` first, the
+          // exact opposite of the intent. Indirect bindings lead and the
+          // literal `user` binding comes last — binding a specific person is
+          // the least portable choice (env moves, people leave). Keep the two
+          // in sync; the spec is the source of truth.
+          //
+          // Both paths also drop the spec's NON_AUTHORABLE_APPROVER_TYPES —
+          // `role` (deprecated spelling of `org_membership_level`, ADR-0090 D3)
+          // and `queue` (declared-but-unenforced; the runtime resolves it to
+          // nobody, framework#3508). A stored row of either still renders,
+          // flagged "(deprecated)", but neither is offered for new authoring.
           options: [
             { value: 'manager', label: 'Manager' },
             { value: 'position', label: 'Position' },
diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.test.ts
@@ -155,6 +155,67 @@ describe('jsonSchemaToFlowFields', () => {
     expect(col.ref).toEqual({ kind: 'connector' });
   });
 
+  // framework#3508 follow-up: `map` names a picker KIND but never said what
+  // backs it, so this package had to guess the data source — and guessed the
+  // metadata registry, which lists no directory ROWS. `sources` carries the
+  // answer, and an entry the designer cannot act on is dropped rather than
+  // half-trusted (a `data` source with no committed column would query
+  // nothing, which is the failure the annotation exists to end).
+  it('carries xRef.sources through, dropping entries that could not drive a picker', () => {
+    const fields = jsonSchemaToFlowFields({
+      type: 'object',
+      properties: {
+        rows: {
+          type: 'array',
+          items: {
+            type: 'object',
+            properties: {
+              value: {
+                type: 'string',
+                xRef: {
+                  kindFrom: 'type',
+                  map: { user: 'user', team: 'team', manager: 'manager' },
+                  sources: {
+                    user: { source: 'data', object: 'sys_user', valueField: 'id' },
+                    manager: { source: 'auto' },
+                    org_membership_level: { source: 'enum', values: ['owner', 'admin'] },
+                    // Unusable: a record source with no committed column.
+                    team: { source: 'data', object: 'sys_team' },
+                    // Unusable: not a source shape at all.
+                    bogus: { source: 'telepathy' },
+                  },
+                },
+              },
+            },
+          },
+        },
+      },
+    })!;
+    const col = fields.find((f) => f.id === 'rows')!.columns!.find((c) => c.key === 'value')!;
+    expect(col.ref!.sources).toEqual({
+      user: { source: 'data', object: 'sys_user', valueField: 'id' },
+      manager: { source: 'auto' },
+      org_membership_level: { source: 'enum', values: ['owner', 'admin'] },
+    });
+  });
+
+  it('omits sources entirely for a server that predates the annotation', () => {
+    const fields = jsonSchemaToFlowFields({
+      type: 'object',
+      properties: {
+        rows: {
+          type: 'array',
+          items: {
+            type: 'object',
+            properties: { value: { type: 'string', xRef: { kindFrom: 'type', map: { user: 'user' } } } },
+          },
+        },
+      },
+    })!;
+    const col = fields.find((f) => f.id === 'rows')!.columns!.find((c) => c.key === 'value')!;
+    expect(col.ref).toEqual({ kindFrom: 'type', map: { user: 'user' } });
+  });
+
   it('drops a polymorphic xRef whose map has no known kinds (column stays text)', () => {
     const fields = jsonSchemaToFlowFields({
       type: 'object',
diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.ts b/packages/app-shell/src/views/metadata-admin/inspectors/json-schema-to-fields.ts
