feat(spec)+fix(approvals): approver value sources on the wire, author-facing type order, no more silent dead slots (#3508 / #3807)

.changeset/approver-value-sources-and-dead-slot-warning.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/spec": minor
+"@objectstack/plugin-approvals": patch
+---
+
+feat(spec)+fix(approvals): publish approver value data sources, order the type enum for authors, stop silent dead approver slots (#3508 / #3807 follow-ups)
+
+Four follow-ups from browser-verifying the #3508 approver work end to end.
+
+**`APPROVER_VALUE_SOURCES` — the designer stops guessing where candidates live.**
+`xRef.map` only ever named a picker KIND (`'team'`), never where that picker's
+rows come from, so the designer carried its own copy of the data contract — and
+the first copy was wrong: every directory kind was wired to `GET
+/api/v1/meta/:type`, the metadata REGISTRY, which does not hold `sys_user` /
+`sys_team` / `sys_business_unit` / `sys_position` rows. Candidates came back
+empty and the control degraded to free text (#3508). The binding is now
+projected onto the published JSON schema as `xRef.sources` — `{ source: 'data',
+object, valueField }` for the record-backed kinds, the closed enum inline for
+`org_membership_level` — derived from `APPROVER_VALUE_BINDINGS` so the two
+cannot drift, and inheriting its `satisfies` exhaustiveness (a new
+`ApproverType` member that declares no source is a compile error). Presentation
+— which field to show, whether to open a people-picker, what subtitle to use —
+stays a renderer decision.
+
+**`ApproverType` declaration order is now the authoring recommendation.**
+objectui#2834 argued for leading with indirect bindings and shipped that order
+in its own options array — which the Studio inspector never reads: it derives
+the picker from this enum via the published schema, so `user` still came first.
+The intent only takes effect if the enum carries it, so the enum now reads
+`manager, position, department, team, field, expression, org_membership_level,
+user` (deprecated `role` / `queue` still parse and stay out of every picker via
+`xEnumDeprecated`). Binding one specific person is the least portable choice an
+author can make — it breaks when the flow moves to another environment (that id
+does not exist there) and again when that person leaves.
+
+**A graph approver that expands to nobody no longer does it in silence.**
+`queue` already warned (#3508); every OTHER graph type — `team`, `department`,
+`position`, `org_membership_level`, `manager` — fell back to the same
+unactionable `type:value` literal without a word. That silence is what let
+#3807 hide for as long as it did: the request opened with an empty slate and
+the first symptom was a permanently stuck approval (#3424). The fallback stays
+(15.x slots and substring fixtures depend on it); it now logs the type, value
+and organization that produced it. `user` / `field` stay quiet — they take the
+id they were given and never had an "expanded to nobody" state.
+
+**`plugin-sharing`'s identical org scope is pinned by tests.**
+`BusinessUnitGraphService.orgScope` has the same strict `organization_id`
+equality #3807 fixed in approvals. It is unreachable today — every materialized
+`sys_sharing_rule` carries `organization_id = null`, so the filter is skipped —
+and widening an authorization path on a defect that cannot currently fire is
+not a change to make blind. New tests lock both the reachable paths and the
+divergence itself, so if sharing ever adopts the null-org=env-wide reading it
+is a deliberate edit to a named test rather than a silent behaviour change.

content/docs/references/automation/approval.mdx

@@ -7,6 +7,12 @@ description: Approval protocol schemas
 
 Approval Step Approver Type
 
+Declaration order is author-facing: designers derive their picker from this
+
+enum, and it leads with the portable indirect bindings (`manager`,
+
+`position`, `department`, `team`) — a literal `user` id comes last.
+
 <Callout type="info">
 **Source:** `packages/spec/src/automation/approval.zod.ts`
 </Callout>
@@ -54,7 +60,7 @@ const result = ApprovalDecision.parse(data);
 
 | Property | Type | Required | Description |
 | :--- | :--- | :--- | :--- |
-| **type** | `Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue' \| 'expression'>` | ✅ |  |
+| **type** | `Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>` | ✅ |  |
 | **value** | `string` | optional | User id / membership tier / position / team / department / field — per `type`; for `expression`, a CEL expression over `current.*` / `trigger.*` / `vars.*` |
 | **resolveAs** | `Enum<'user' \| 'department' \| 'position' \| 'team'>` | optional | How an `expression` result is expanded into approvers (default 'user') |
 | **group** | `string` | optional | Group label for per_group sign-off (e.g. "legal", "finance") |
@@ -68,7 +74,7 @@ const result = ApprovalDecision.parse(data);
 
 | Property | Type | Required | Description |
 | :--- | :--- | :--- | :--- |
-| **approvers** | `{ type: Enum<'user' \| 'org_membership_level' \| 'role' \| 'position' \| 'team' \| 'department' \| 'manager' \| 'field' \| 'queue' \| 'expression'>; value?: string; resolveAs?: Enum<'user' \| 'department' \| 'position' \| 'team'>; group?: string }[]` | ✅ | Allowed approvers for this node |
+| **approvers** | `{ type: Enum<'manager' \| 'position' \| 'department' \| 'team' \| 'field' \| 'expression' \| 'org_membership_level' \| 'role' \| 'user' \| 'queue'>; value?: string; resolveAs?: Enum<'user' \| 'department' \| 'position' \| 'team'>; group?: string }[]` | ✅ | Allowed approvers for this node |
 | **behavior** | `Enum<'first_response' \| 'unanimous' \| 'quorum' \| 'per_group'>` | ✅ | How to combine multiple approvers |
 | **minApprovals** | `integer` | optional | Approvals required — total (quorum) or per group (per_group). Default 1 |
 | **lockRecord** | `boolean` | ✅ | Lock the record from editing while pending |
@@ -85,16 +91,16 @@ const result = ApprovalDecision.parse(data);
 
 ### Allowed Values
 
-* `user`
-* `org_membership_level`
-* `role`
+* `manager`
 * `position`
-* `team`
 * `department`
-* `manager`
+* `team`
 * `field`
-* `queue`
 * `expression`
+* `org_membership_level`
+* `role`
+* `user`
+* `queue`
 
 
 ---

packages/plugins/plugin-approvals/src/approval-service.test.ts

@@ -2331,6 +2331,66 @@ describe('ApprovalService — queue approver is unresolved (#3508)', () => {
   });
 });
 
+// #3807 follow-up: `queue` was not the only way to end up with a slot nobody
+// can act on — every GRAPH approver type falls back to the same literal when
+// its lookup finds no one, and that fallback used to happen in total silence.
+// A stuck approval was the first symptom; the log said nothing. The literal
+// stays (15.x slots and substring fixtures depend on it) — it just announces
+// itself now.
+describe('ApprovalService — a graph approver that expands to nobody warns (#3807)', () => {
+  const svcWithWarnings = (engine: any) => {
+    const warnings: any[] = [];
+    let n = 0;
+    const svc = new ApprovalService({
+      engine,
+      clock: { now: () => new Date(1757000000000 + (n++) * 1000) },
+      logger: { warn: (msg: any, meta: any) => warnings.push([msg, meta]) },
+    });
+    return { svc, warnings };
+  };
+
+  const approverInput = (type: string, value: string) => ({
+    ...openInput([]),
+    config: { approvers: [{ type, value }], behavior: 'first_response' as const, lockRecord: false },
+  });
+
+  it.each([
+    ['team', 'team_gone'],
+    ['department', 'bu_gone'],
+    ['position', 'nobody_holds_this'],
+    ['org_membership_level', 'member'],
+  ])('%s: the dead literal is logged with its type, value and org', async (type, value) => {
+    const engine = makeFakeEngine();
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(approverInput(type, value), CTX);
+
+    expect(req.pending_approvers).toEqual([`${type}:${value}`]);
+    const hit = warnings.find(([msg]) => String(msg).includes('expanded to nobody'));
+    expect(hit, `no warning for ${type}`).toBeTruthy();
+    expect(String(hit[0])).toContain('#3807');
+    expect(hit[1]).toMatchObject({ type, value, organizationId: 't1' });
+  });
+
+  it('stays quiet when the graph DOES resolve someone', async () => {
+    const engine = makeFakeEngine();
+    engine._tables['sys_team_member'] = [{ id: 'tm1', team_id: 'team_ok', user_id: 'u5' }];
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(approverInput('team', 'team_ok'), CTX);
+
+    expect(req.pending_approvers).toEqual(['u5']);
+    expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]);
+  });
+
+  it('stays quiet for `user` — a literal id was never a lookup that could come back empty', async () => {
+    const engine = makeFakeEngine();
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(approverInput('user', 'u_unknown'), CTX);
+
+    expect(req.pending_approvers).toEqual(['u_unknown']);
+    expect(warnings.filter(([msg]) => String(msg).includes('expanded to nobody'))).toEqual([]);
+  });
+});
+
 // ── File-access delegate (ADR-0104 D3 wave 2) ────────────────────────
 //
 // A decision attachment is OWNED by its `sys_approval_action` row, so the

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -177,6 +177,21 @@ function actingUserId(context: SharingExecutionContext | undefined): string | nu
  */
 const OOO_MAX_CHAIN = 8;
 
+/**
+ * Approver types resolved by QUERYING a graph rather than by taking `value`
+ * literally (#3807). Each can legitimately come back empty — an unstaffed
+ * position, an emptied team, a mis-pointed unit — and the caller then falls
+ * back to a `type:value` literal that no user can act on. They are listed here
+ * so that dead end gets one warning instead of passing in silence.
+ *
+ * `user` / `field` are deliberately absent: they resolve to the id they were
+ * given without a lookup, so there is no "expanded to nobody" state to report.
+ * `business_unit` / `bu` are the accepted dialects of `department`.
+ */
+const GRAPH_APPROVER_TYPES: ReadonlySet<string> = new Set([
+  'team', 'department', 'business_unit', 'bu', 'position', 'org_membership_level', 'manager',
+]);
+
 /** One OOO delegation hop applied while resolving an approver (#1322 M1/M4). */
 interface OooSubstitution {
   /** The approver who was skipped (out of office). */
@@ -757,6 +772,21 @@ export class ApprovalService implements IApprovalService {
         `[approvals] approver type 'queue' is not implemented — the slot resolves to nobody (#3508)`,
         { value: a.value },
       );
+    } else if (GRAPH_APPROVER_TYPES.has(type)) {
+      // #3807 follow-up — every OTHER way to land here is a graph type whose
+      // lookup produced nobody, and the literal below is a slot no user can
+      // ever act on. That silence is what let #3807 hide: a `department`
+      // approver pointing at a seeded (env-wide) unit resolved to
+      // `department:<id>` on every request, the request opened with an empty
+      // slate, and nothing in the logs said so — the first symptom was a
+      // permanently stuck approval (#3424). The fallback itself stays (a
+      // literal keeps 15.x slots and substring fixtures working); it just
+      // stops being invisible.
+      this.logger?.warn?.(
+        `[approvals] approver '${type}:${a.value}' expanded to nobody — the slot routes to no one `
+        + `and the request cannot advance until someone is added or the approver is re-pointed (#3807)`,
+        { type, value: a.value, organizationId: organizationId ?? null },
+      );
     }
     return [`${a.type}:${a.value}`];
   }

packages/plugins/plugin-sharing/src/business-unit-graph.test.ts

@@ -0,0 +1,162 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * BusinessUnitGraphService — org scoping of the unit tree.
+ *
+ * These pin the ORG-SCOPE behaviour specifically, because it is the exact
+ * shape that broke approvals in #3807: `orgScope()` AND-composes a strict
+ * `organization_id = <rule org>` equality, so a unit written with no
+ * organization at all (a seeded / file-layer / bootstrap row — a seed cannot
+ * know the org id the runtime mints at boot) matches nothing, the seed check
+ * fails, and the expansion returns zero members. In approvals that produced a
+ * dead `department:<id>` approver slot; here it would produce a sharing rule
+ * that silently grants nobody.
+ *
+ * It is NOT reachable today: every materialized `sys_sharing_rule` row carries
+ * `organization_id = null` (verified on a live showcase stack), so
+ * `expandRecipient` passes `null` and `orgScope` is skipped entirely. The
+ * moment rules start carrying an org — a multi-tenant deployment — a BU
+ * subtree rule against a seeded unit stops granting, and the symptom is
+ * "the right people cannot see the record", which is far quieter than a stuck
+ * approval.
+ *
+ * So this file locks BOTH sides down:
+ *   - the reachable paths (null-org rule) keep working, and
+ *   - the divergence from approvals is written down as an executable fact
+ *     rather than a comment, so flipping it is a deliberate edit to a named
+ *     test and never a silent behaviour change.
+ *
+ * If the platform decides null-org means "env-wide, visible to every org" for
+ * sharing too — the way `plugin-approvals` and `sys_metadata` already read it —
+ * the test named `[divergence]` below is the one to flip, and `orgScope` grows
+ * the same `$or: [{ organization_id }, { organization_id: null }]` predicate.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { BusinessUnitGraphService } from './business-unit-graph.js';
+
+interface UnitRow {
+  id: string;
+  parent_business_unit_id?: string | null;
+  organization_id?: string | null;
+  active?: boolean;
+}
+interface MemberRow { business_unit_id: string; user_id: string }
+
+/**
+ * Minimal engine over `sys_business_unit` + `sys_business_unit_member`.
+ * Mirrors the real filter surface the service uses: plain equality, `$ne`,
+ * `$in`, and `$or` (so a widened predicate would actually be exercised here
+ * rather than silently ignored by the stub).
+ */
+function makeEngine(units: UnitRow[], members: MemberRow[]) {
+  const matches = (row: any, filter: any): boolean => {
+    if (!filter || typeof filter !== 'object') return true;
+    for (const [k, v] of Object.entries(filter)) {
+      if (k === '$or') {
+        if (!(v as any[]).some((sub) => matches(row, sub))) return false;
+        continue;
+      }
+      const rv = row[k];
+      if (v && typeof v === 'object' && '$ne' in (v as any)) {
+        if (rv === (v as any).$ne) return false;
+        continue;
+      }
+      if (v && typeof v === 'object' && '$in' in (v as any)) {
+        if (!(v as any).$in.includes(rv)) return false;
+        continue;
+      }
+      // `organization_id: null` must also match a row that simply omits the
+      // column — that is what a NULL column reads back as.
+      if (v === null) {
+        if (rv != null) return false;
+        continue;
+      }
+      if (rv !== v) return false;
+    }
+    return true;
+  };
+
+  return {
+    async find(object: string, options: any) {
+      const filter = options?.where ?? options?.filter ?? {};
+      if (object === 'sys_business_unit') return units.filter((u) => matches(u, filter));
+      if (object === 'sys_business_unit_member') return members.filter((m) => matches(m, filter));
+      return [];
+    },
+  } as any;
+}
+
+/** A seeded org tree: rows written before any organization existed. */
+const SEEDED_UNITS: UnitRow[] = [
+  { id: 'bu_root', organization_id: null, active: true },
+  { id: 'bu_child', parent_business_unit_id: 'bu_root', organization_id: null, active: true },
+];
+const SEEDED_MEMBERS: MemberRow[] = [
+  { business_unit_id: 'bu_root', user_id: 'u_root' },
+  { business_unit_id: 'bu_child', user_id: 'u_child' },
+];
+
+describe('BusinessUnitGraphService — subtree expansion', () => {
+  it('expands the unit and every descendant', async () => {
+    const g = new BusinessUnitGraphService({ engine: makeEngine(SEEDED_UNITS, SEEDED_MEMBERS) });
+    expect((await g.expandUsers('bu_root')).sort()).toEqual(['u_child', 'u_root']);
+  });
+
+  it('an inactive unit contributes nobody and stops the descent', async () => {
+    const units: UnitRow[] = [
+      { id: 'bu_root', organization_id: null, active: false },
+      { id: 'bu_child', parent_business_unit_id: 'bu_root', organization_id: null, active: true },
+    ];
+    const g = new BusinessUnitGraphService({ engine: makeEngine(units, SEEDED_MEMBERS) });
+    expect(await g.expandUsers('bu_root')).toEqual([]);
+  });
+});
+
+describe('BusinessUnitGraphService — org scoping (#3807)', () => {
+  it('an org-less rule (today’s materialized shape) expands seeded units fine', async () => {
+    // `expandRecipient` passes `rule.organization_id ?? null`, and every
+    // materialized sharing rule carries null — so `orgScope` adds nothing and
+    // the seeded tree resolves. This is the only path that runs in practice.
+    const g = new BusinessUnitGraphService({
+      engine: makeEngine(SEEDED_UNITS, SEEDED_MEMBERS),
+      organizationId: null,
+    });
+    expect((await g.expandUsers('bu_root')).sort()).toEqual(['u_child', 'u_root']);
+  });
+
+  it('an org-scoped rule expands units belonging to that org', async () => {
+    const units: UnitRow[] = [
+      { id: 'bu_root', organization_id: 'org_a', active: true },
+      { id: 'bu_child', parent_business_unit_id: 'bu_root', organization_id: 'org_a', active: true },
+    ];
+    const g = new BusinessUnitGraphService({
+      engine: makeEngine(units, SEEDED_MEMBERS),
+      organizationId: 'org_a',
+    });
+    expect((await g.expandUsers('bu_root')).sort()).toEqual(['u_child', 'u_root']);
+  });
+
+  it('an org-scoped rule never reaches another org’s unit', async () => {
+    const units: UnitRow[] = [{ id: 'bu_root', organization_id: 'org_b', active: true }];
+    const g = new BusinessUnitGraphService({
+      engine: makeEngine(units, SEEDED_MEMBERS),
+      organizationId: 'org_a',
+    });
+    expect(await g.expandUsers('bu_root')).toEqual([]);
+  });
+
+  it('[divergence] an org-scoped rule does NOT see an env-wide (null-org) unit — approvals does (#3807)', async () => {
+    // Same inputs that #3807 fixed on the approvals side. Sharing still reads
+    // a null-org unit as "belongs to no org, therefore not mine" and grants
+    // nobody. Unreachable today (rules are null-org), deliberate until the
+    // platform rules on null-org semantics for AUTHORIZATION paths — widening
+    // who can SEE a record is not a change to make on a defect that cannot
+    // currently fire.
+    const g = new BusinessUnitGraphService({
+      engine: makeEngine(SEEDED_UNITS, SEEDED_MEMBERS),
+      organizationId: 'org_a',
+    });
+    expect(await g.expandUsers('bu_root')).toEqual([]);
+  });
+});

packages/spec/api-surface.json

@@ -1982,6 +1982,7 @@
     "APPROVAL_NODE_TYPE (const)",
     "APPROVER_EXPRESSION_ROOTS (const)",
     "APPROVER_VALUE_BINDINGS (const)",
+    "APPROVER_VALUE_SOURCES (const)",
     "ActionCategory (type)",
     "ActionCategorySchema (const)",
     "ActionDescriptor (type)",