fix(plugins)!: team and position recipients reach every holder past 10,000 rows, and a failed holder read refuses instead of answering nobody

.changeset/29503-recipient-reads-complete.md

@@ -0,0 +1,27 @@
+---
+'@objectstack/types': patch
+'@objectstack/plugin-sharing': patch
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugins)!: team and position recipients reach every holder past 10,000 rows in sharing and approvals, and a team, position or role holder read that fails is refused loudly instead of answered with nobody
+
+Clause-②: yes (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 team and position expansions read lists they already read, to completion, and the holder checks refuse a read that already failed. The one new export (isObjectAbsentError in @objectstack/types) is an addition. No author has anything to rewrite. -->
+
+**BREAKING** only where a `sys_team_member`, `sys_user_position` or `sys_member` read fails: the team and position expansions and the user-scoped holder checks used to answer nobody (an empty list, or `false`) then, and now refuse.
+
+This completes the family the role and membership-tier fix started. The team and position reads still read once with a 10,000-row bound and turned any read error into nobody.
+
+- **Every holder, nobody else.** In plugin-sharing, `TeamGraphService.expandUsers` (`sys_team_member`, every `team` sharing recipient) and the assignment limb of `PositionGraphService.expandPositionUsers` (`sys_user_position`, every `position` sharing recipient) now read in pages of 10,000 ordered by id until the holders are exhausted. In plugin-approvals, the `team` approver and the assignment limb of the `position` approver (and every caller of the position expansion: escalation targets, reassign checks, `resolveAs` expressions) do the same. Past 10,000 holders, the holders past the 10,000th row used to get no grant and no approval slot. They now do.
+- **Smaller groups are unchanged.** A team or position held by fewer than 10,000 people issues one read, as before, and answers the same set.
+- **Loud, never short or empty.** A 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 team read answered `[]`, and a failed assignment read answered the `sys_member.role` holders alone, so the retired-recipient revoke took grants away from people who still held the recipient.
+  - Sharing, per-user recompute (a write to `sys_team_member` or `sys_user_position`): the holder checks `teamHasMember`, `holdsRole` and `PositionGraphService.holdsPosition` now refuse a failed read instead of answering `false`. The recompute logs `recipient membership re-evaluation failed` for that rule and user, and the user's grants stay. Before, an outage revoked that user's grants.
+  - Approvals: the approver slot falls back to its literal (`team:ID` or `position:NAME`) and logs `could not be expanded — a directory or read-filter lookup failed` with its `error`. Before, a failed team read logged "expanded to nobody", and a failed assignment read routed a `position` approver to its membership-tier holders alone, which looked like a complete slate.
+- **Ownership queues: a position specification now contributes nobody on a `sys_user_position` outage.** A queue team specification whose `sys_team_member` read fails still contributes nobody, as before. A queue position specification whose `sys_user_position` read fails now contributes nobody too, on the expansion and on the queue's holder check alike, where before it contributed that position's `sys_member.role` transition-source holders. That is the posture the queue declares for a failed member-table read, reached here for the first time. Both now log once. Whether a queue should refuse instead is left to a ruling.
+- **A missing object is still no holders.** A stack that never registers, or never creates, `sys_team_member`, `sys_user_position` or `sys_member` keeps answering no holders through it. That is judged by the new `isObjectAbsentError(error, readObject)` in `@objectstack/types`: the shared missing-table predicate for that object, or the engine's `OBJECT_NOT_FOUND` naming that object. Any other error, including an absence of a different object, 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

@@ -455,8 +455,9 @@ What an entry that resolves to nobody leaves on the slate depends on its kind:
   decide it (the admin-override callout under
   [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 `position` or
-  `org_membership_level` entry whose `sys_member` read failed keeps its literal
+  `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.

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

@@ -21,7 +21,7 @@ import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formul
 // writer-local re-derivation here was rejected by name (Option B): it would be
 // a third answer to a question the codebase already answered two ways.
 import { createRecordOrganizationResolver, type RecordOrganizationResolver } from '@objectstack/metadata-core';
-import { isMissingTableError, keysetWalk, strandedDecisionFailure } from '@objectstack/types';
+import { isObjectAbsentError, keysetWalk, strandedDecisionFailure } from '@objectstack/types';
 // [#15981 / #16166] Every built-in identity NAME is deliberately absent here:
 // both arms of `isOverrideActor` read an ADR-0095 capability rung, never a name.
 // `BUILTIN_IDENTITY_PLATFORM_ADMIN` went with the platform arm's name read and
@@ -1119,29 +1119,13 @@ const OOO_MAX_CHAIN = 8;
 const MEMBER_SCREEN_READ_LIMIT = 50000;
 
 /**
- * [#29362] Rows per page of the membership-tier read
- * ({@link ApprovalService.expandMembershipTierUsers}) — the bound the read
- * always carried, so a tier held by fewer people issues the one read it always
- * did, and a tier held by more is read in further pages rather than cut.
+ * [#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
+ * 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.
  */
-const MEMBER_TIER_PAGE_ROWS = 10000;
-
-/**
- * [#29362] Did a `sys_member` read fail because this stack has no membership
- * object at all — the one failure whose empty answer is the truth? Two
- * spellings of the same absence, each attributed to `sys_member` so a failure
- * about another object is not evidence about this one: the driver's
- * missing-table refusal (the shared `isMissingTableError`, the table
- * registered but never created), and the engine's `OBJECT_NOT_FOUND` (a
- * composition that never registers the object, refused before any driver is
- * asked — the reading `sys-metadata-repository.ts` gives its own history
- * object). Anything else is an outage and stays loud.
- */
-function membershipObjectAbsent(err: unknown): boolean {
-  if (isMissingTableError(err, 'sys_member')) return true;
-  const refused = err as { code?: unknown; object?: unknown } | null | undefined;
-  return refused?.code === 'OBJECT_NOT_FOUND' && refused.object === 'sys_member';
-}
+const HOLDER_PAGE_ROWS = 10000;
 
 /**
  * Approver types resolved by QUERYING a graph rather than by taking `value`
@@ -2917,16 +2901,12 @@ export class ApprovalService implements IApprovalService {
   private async expandTeamUsers(teamId: string, organizationId?: string | null): Promise<string[]> {
     if (!teamId) return [];
     if (await this.teamIsProvablyOutsideOrg(teamId, organizationId)) return [];
-    let rows: any[] = [];
-    try {
-      rows = await this.engine.find('sys_team_member', {
-        where: { team_id: teamId },
-        fields: ['user_id'],
-        limit: 10000,
-        context: SYSTEM_CTX,
-      } as any);
-    } catch { rows = []; }
-    const users = Array.from(new Set((rows ?? []).map((r: any) => String(r.user_id ?? '')).filter(Boolean)));
+    // [#29503] Every member, read to completion — or a refusal that
+    // {@link resolveApproverSpec}'s directory catch reports as a failed read,
+    // never "expanded to nobody". A stack with no `sys_team_member` has none.
+    const users = await this.readHoldersToCompletion(
+      'sys_team_member', { team_id: teamId }, `the members of team '${teamId}'`,
+    );
     // #10547: the TEAM proved its tenancy above; its members have not proved
     // theirs, and `sys_team_member` holds no fact that could. One read.
     return await this.dropMembersProvablyOutsideOrg(teamId, users, organizationId);
@@ -3303,18 +3283,16 @@ export class ApprovalService implements IApprovalService {
    */
   private async expandPositionUsers(positionName: string, organizationId?: string | null): Promise<string[]> {
     if (!positionName) return [];
-    const users = new Set<string>();
-    const filter: any = { position: positionName };
+    const filter: Record<string, unknown> = { position: positionName };
     if (organizationId) filter.organization_id = organizationId;
-    try {
-      const rows = await this.engine.find('sys_user_position', {
-        filter, fields: ['user_id'], limit: 10000, context: SYSTEM_CTX,
-      } as any);
-      for (const r of (rows ?? []) as any[]) {
-        const uid = String(r.user_id ?? '');
-        if (uid) users.add(uid);
-      }
-    } catch { /* table may not exist on minimal stacks — union source below still applies */ }
+    // [#29503] Every assignment, read to completion — or a refusal, never the
+    // tier limb below alone (a shrunk slate that looks complete). A stack with
+    // no `sys_user_position` has no assignment holders; the union source below
+    // still applies.
+    const users = new Set<string>(await this.readHoldersToCompletion(
+      'sys_user_position', filter,
+      `the assignments of position '${positionName}'${organizationId ? ` in organization '${organizationId}'` : ''}`,
+    ));
     // ADR-0057 D4 transition source: pre-migration stacks still carry the
     // position name in better-auth's `sys_member.role` column, so the same
     // lookup serves a position name here and a membership tier for
@@ -3402,7 +3380,7 @@ export class ApprovalService implements IApprovalService {
    * change here changes position routing too.
    *
    * [#29362] Read to COMPLETION, or refused — never a shortened or an invented
-   * empty slate. Pages of {@link MEMBER_TIER_PAGE_ROWS} through the platform's
+   * empty slate. Pages of {@link HOLDER_PAGE_ROWS} through the platform's
    * one keyset walk (`keysetWalk`, `@objectstack/types`), ordered by `id`, so a
    * tier held past the bound routes to every holder rather than the first
    * 10,000 rows. A read that fails, or a walk that cannot vouch for completion
@@ -3414,17 +3392,40 @@ export class ApprovalService implements IApprovalService {
    * nobody", indistinguishable from a tier no one holds. A membership object
    * that does not exist is the one failure that IS an answer (a stack that never
    * materialises `sys_member` has no tier holders) and keeps answering `[]`;
-   * see {@link membershipObjectAbsent}.
+   * see {@link readHoldersToCompletion}.
    */
   private async expandMembershipTierUsers(tier: string, organizationId?: string | null): Promise<string[]> {
     if (!tier) return [];
     const filter: Record<string, unknown> = { role: tier };
     if (organizationId) filter.organization_id = organizationId;
-    const scope = `the holders of membership tier '${tier}'${organizationId ? ` in organization '${organizationId}'` : ''}`;
+    return this.readHoldersToCompletion(
+      'sys_member', filter,
+      `the holders of membership tier '${tier}'${organizationId ? ` in organization '${organizationId}'` : ''}`,
+    );
+  }
+
+  /**
+   * [#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.
+   *
+   * Pages of {@link HOLDER_PAGE_ROWS} through the platform's one keyset walk
+   * (`keysetWalk`, `@objectstack/types`), ordered by `id`, so a group held past
+   * the bound routes to every holder rather than the first 10,000 rows. A read
+   * that fails, or a walk that cannot vouch for completion (a row without an
+   * `id`, a page that did not honour the cursor), THROWS:
+   * {@link resolveApproverSpec}'s directory catch then lets the slot fall back
+   * to its literal and says the read failed. An object that does not exist on
+   * this stack is the one failure that IS an answer (no holders through it) and
+   * answers `[]` — `isObjectAbsentError` (`@objectstack/types`) decides that,
+   * attributed to `object`, so an absence of another object is still loud.
+   */
+  private async readHoldersToCompletion(object: string, where: Record<string, unknown>, scope: string): Promise<string[]> {
     const users = new Set<string>();
     const walk = keysetWalk<Record<string, unknown>>(
-      (page) => this.engine.find('sys_member', { ...page, fields: ['id', 'user_id'], context: SYSTEM_CTX }),
-      { where: filter, pageSize: MEMBER_TIER_PAGE_ROWS },
+      (page) => this.engine.find(object, { ...page, fields: ['id', 'user_id'], context: SYSTEM_CTX }),
+      { where, pageSize: HOLDER_PAGE_ROWS },
     );
     try {
       for await (const page of walk.pages()) {
@@ -3434,7 +3435,7 @@ export class ApprovalService implements IApprovalService {
         }
       }
     } catch (err) {
-      if (membershipObjectAbsent(err)) return [];
+      if (isObjectAbsentError(err, object)) return [];
       throw Object.assign(new Error(
         `[approvals] ${scope} could not be read: ${err instanceof Error ? err.message : String(err)}. `
         + 'Routing to a shortened slate would leave holders unasked, so the expansion refuses instead.',