fix(plugins)!: business-unit recipients and approvers reach every member past 10,000 rows, and a failed member read refuses instead of answering nobody

.changeset/29606-business-unit-member-reads-complete.md

@@ -0,0 +1,27 @@
+---
+'@objectstack/plugin-sharing': patch
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugins)!: business-unit recipients and approvers reach every member past 10,000 rows, a failed business-unit member read is refused loudly instead of answered with nobody, and a failed `resolveAs` expansion says so
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped and no metadata changes: the business-unit expansions and the primary-business-unit backfill read lists they already read, to completion, and refuse a read that already failed. No author has anything to rewrite. -->
+
+**BREAKING** only where a `sys_business_unit_member` read fails: the business-unit expansions used to answer nobody (an empty list) then, and now refuse.
+
+This extends the family the team, position and role fixes closed to the last membership object, `sys_business_unit_member`. Its reads still read once with a 10,000-row bound and turned any read error into nobody.
+
+- **Every member, nobody else.** In plugin-sharing, `BusinessUnitGraphService.expandUnitMembers` (every `business_unit` sharing recipient) and `BusinessUnitGraphService.expandUsers` (every `unit_and_subordinates` recipient) now read in pages of 10,000 ordered by id until the members are exhausted. In plugin-approvals, the `department` / `business_unit` / `bu` approver and an `expression` approver with `resolveAs: 'department'` do the same. Past 10,000 members, the members past the 10,000th row used to get no grant and no approval slot. They now do.
+- **The primary-business-unit backfill projects every primary membership.** The boot backfill of `sys_user.primary_business_unit_id` used to scan the `is_primary` rows once at the same bound, so users past the 10,000th row were never projected and nothing said so. It now pages to completion. A scan that fails, or a page the driver returned without honouring the id cursor, writes nothing and logs why (a failed scan logged before too, now with the consequence).
+- **Smaller units are unchanged.** A unit or subtree with fewer than 10,000 members issues one read, as before, and answers the same set.
+- **Loud, never short or empty.** A member read that fails, or a page the driver returned without honouring the id cursor, now throws.
+  - Sharing, whole-rule passes: the expansion rejects, so that rule's reconcile or revoke pass fails, its callers log their existing per-rule warning, and the grants it already holds stay. Before, a failed read answered `[]`, so the retired-recipient revoke a business-unit graph write triggers took the rule's grants away from every member.
+  - Approvals: the approver slot falls back to its literal (`department:ID`) and logs `could not be expanded — a directory or read-filter lookup failed` with its `error`. Before, a failed read logged "expanded to nobody", indistinguishable from an unstaffed unit.
+  - Ownership queues: a `business_unit` or `unit_and_subordinates` queue specification whose member read fails still contributes nobody, the posture the queue declares for a failed member-table read, and now logs it once.
+- **A failed `resolveAs` expansion says so.** An `expression` approver whose `resolveAs` value could not be expanded (a `department`, `position` or `team` read failed) kept its `KIND:VALUE` literal slot with no log line. It now logs the same `could not be expanded` warning the static approver types log, with the `error` and the `expression` source.
+- **A missing object is still no members.** A stack that never registers, or never creates, `sys_business_unit_member` keeps answering no members through it, and the backfill has nothing to project. That is judged by `isObjectAbsentError(error, 'sys_business_unit_member')`; any other error stays loud.
+
+**The remedy.** None is needed. A refusal means the membership table could not be read: fix that read, and the next expansion succeeds. A failed read is never cached.

content/docs/automation/approvals.mdx

@@ -456,11 +456,13 @@ What an entry that resolves to nobody leaves on the slate depends on its kind:
   [Acting on requests in the console](#acting-on-requests-in-the-console)).
   Beside entries that do resolve, a literal is a slot like any other, so a
   `behavior` that needs every slot (`unanimous`) waits on it too. A `team`,
-  `position` or `org_membership_level` entry whose holder read failed
-  (`sys_team_member`, `sys_user_position` or `sys_member`) keeps its literal
-  the same way. It logs `could not be expanded — a directory or read-filter
-  lookup failed` with the `error`, instead of `expanded to nobody`, and never
-  routes to the holders that happened to read.
+  `position`, `department` or `org_membership_level` entry whose holder read
+  failed (`sys_team_member`, `sys_user_position`, `sys_business_unit_member`
+  or `sys_member`) keeps its literal the same way. It logs `could not be
+  expanded — a directory or read-filter lookup failed` with the `error`,
+  instead of `expanded to nobody`, and never routes to the holders that
+  happened to read. An `expression` value whose `resolveAs` read failed keeps
+  its literal and logs the same line, with the `expression` source.
 - **A slate left empty, or holding only literals,** goes to the node's
   `onEmptyApprovers` policy, whose four values are listed under
   [Approving across organizations](#cross-org-approvers). Under the default,

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

@@ -1121,7 +1121,7 @@ const MEMBER_SCREEN_READ_LIMIT = 50000;
 /**
  * [#29362, #29503] Rows per page of the directory holder reads
  * ({@link ApprovalService.readHoldersToCompletion}: the membership tier, the
- * position assignments, the team members) — the bound each read always
+ * position assignments, the team members, the business-unit members) — the bound each read always
  * carried, so a group held by fewer people issues the one read it always did,
  * and one held by more is read in further pages rather than cut.
  */
@@ -2695,12 +2695,7 @@ export class ApprovalService implements IApprovalService {
         // this is, and carry the error, so an operator does not go staffing a
         // position whose holders simply could not be read. The try above also
         // wraps the cross-org read filter (`bounded`), so the line names both.
-        this.logger?.warn?.(
-          `[approvals] approver '${type}:${a.value}' could not be expanded — a directory or read-filter lookup failed, so the `
-          + `slot falls back to its literal rather than route to part of its people. Fix the read (see error); `
-          + `a slate left naming no one is decided by the node's onEmptyApprovers.`,
-          { type, value: a.value, organizationId: organizationId ?? null, error: directoryFailure },
-        );
+        this.warnExpansionFailed(type, a.value, organizationId, directoryFailure);
       } else {
         this.logger?.warn?.(
           `[approvals] approver '${type}:${a.value}' expanded to nobody — the slot names no one, so no one is `
@@ -2717,6 +2712,32 @@ export class ApprovalService implements IApprovalService {
     return [`${type}:${a.value}`];
   }
 
+  /**
+   * [#29362, #29606] The one warning an approver slot logs when its expansion
+   * FAILED — a directory or read-filter lookup threw — rather than came back
+   * empty: the slot falls back to its literal, and the line carries the error.
+   * Shared by the static graph types ({@link resolveApproverSpec}) and an
+   * `expression` approver's `resolveAs` values ({@link resolveExpressionApprovers},
+   * which passes its source as `expression`), so the two cannot drift.
+   */
+  private warnExpansionFailed(
+    type: string,
+    value: unknown,
+    organizationId: string | null | undefined,
+    error: string,
+    expression?: string,
+  ): void {
+    this.logger?.warn?.(
+      `[approvals] approver '${type}:${value}' could not be expanded — a directory or read-filter lookup failed, so the `
+      + `slot falls back to its literal rather than route to part of its people. Fix the read (see error); `
+      + `a slate left naming no one is decided by the node's onEmptyApprovers.`,
+      {
+        type, value, organizationId: organizationId ?? null, error,
+        ...(expression === undefined ? {} : { expression }),
+      },
+    );
+  }
+
   /**
    * Resolve an `expression` approver (#3447 P2): evaluate its CEL source at
    * node entry against the three explicit roots — `current` (live record),
@@ -2859,6 +2880,11 @@ export class ApprovalService implements IApprovalService {
         }
       } catch (err: any) {
         if (String(err?.message ?? '').startsWith('VALIDATION_FAILED')) throw err;
+        // [#29606] The literal slot below is the same fallback the static graph
+        // types take, and it says why exactly as they do: a failed read is not
+        // an unstaffed value, so an operator is pointed at the read rather than
+        // at staffing it.
+        this.warnExpansionFailed(resolveAs, key, organizationId, err instanceof Error ? err.message : String(err), source);
         users = [];
       }
       if (crossOrg && users.length) {
@@ -3210,20 +3236,22 @@ export class ApprovalService implements IApprovalService {
         if (kid && !seen.has(kid)) { seen.add(kid); queue.push(kid); }
       }
     }
-    let rows: any[] = [];
-    try {
-      rows = await this.engine.find('sys_business_unit_member', {
-        // #14946: tenant-screened — {@link businessUnitMemberScope} is STRICT
-        // on purpose and is not {@link businessUnitOrgScope}. The units above
-        // proved their tenancy; these rows have not, and the
-        // shared seeded unit id is exactly where other tenants' rows sit.
-        where: this.businessUnitMemberScope({ business_unit_id: { $in: Array.from(seen) } }, organizationId),
-        fields: ['user_id'],
-        limit: 10000,
-        context: SYSTEM_CTX,
-      } as any);
-    } catch { rows = []; }
-    return Array.from(new Set((rows ?? []).map((r: any) => String(r.user_id ?? '')).filter(Boolean)));
+    // [#29606] Every member of the subtree, read to completion — or a refusal
+    // that {@link resolveApproverSpec}'s directory catch reports as a failed
+    // read, never "expanded to nobody". It read once at a 10,000-row bound and
+    // answered `[]` on any failure: past the bound the slate silently lost the
+    // rest, and a failed read logged as an unstaffed unit. A stack with no
+    // `sys_business_unit_member` has no members.
+    return this.readHoldersToCompletion(
+      'sys_business_unit_member',
+      // #14946: tenant-screened — {@link businessUnitMemberScope} is STRICT
+      // on purpose and is not {@link businessUnitOrgScope}. The units above
+      // proved their tenancy; these rows have not, and the
+      // shared seeded unit id is exactly where other tenants' rows sit.
+      this.businessUnitMemberScope({ business_unit_id: { $in: Array.from(seen) } }, organizationId),
+      `the members of business unit '${businessUnitId}' and its descendant units`
+        + `${organizationId ? ` in organization '${organizationId}'` : ''}`,
+    );
   }
 
   /**
@@ -3445,8 +3473,8 @@ export class ApprovalService implements IApprovalService {
   /**
    * [#29362, #29503] The `user_id`s of every `object` row matching `where`,
    * read to COMPLETION — or a refusal, never a shortened or an invented empty
-   * slate. The one read behind the membership tier, the position assignments
-   * and the team members.
+   * slate. The one read behind the membership tier, the position assignments,
+   * the team members and the business-unit members (#29606).
    *
    * Pages of {@link HOLDER_PAGE_ROWS} through the platform's one keyset walk
    * (`keysetWalk`, `@objectstack/types`), ordered by `id`, so a group held past