fix(plugin-approvals): a field approver id or an out-of-office delegate that names no account adds no phantom slot

.changeset/27752-field-delegate-names-no-account.md

@@ -0,0 +1,13 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): a `field` approver id, or an out-of-office delegate, that names no account no longer opens a slot no one can take
+
+Clause-②: no
+Metadata-protocol: none
+
+#27397 made a `{ type: 'user' }` approver that names no account expand to nobody. Its two siblings now follow it:
+
+- **`type: 'field'`.** Each id the record field holds is a slot only when an account carries it (the same predicate the reassign door asks, with no organization). An id no account carries logs the shared "expanded to nobody" warning, naming the field and the id, and adds no slot, so a slate it leaves empty reaches the node's `onEmptyApprovers`. A failed directory read keeps the slot.
+- **Out-of-office delegation.** A delegation chain that ends on a delegate no account carries routes the slot back to the original approver, with an "expanded to nobody" warning naming both. It never empties the slate, and no `ooo_substitute` action or notification is recorded for it. A failed read keeps the delegate. A removed delegator whose delegate is live still routes to the delegate, as before.

packages/plugins/plugin-approvals/src/admin-exemption-retired.test.ts

@@ -238,6 +238,7 @@ describe('#4839 evidence — a delegation cannot rescue an IN-FLIGHT approval',
   });
 
   it('the delegation DOES apply to a request opened afterwards — it is an open-time rule, nothing more', async () => {
+    engine._tables['sys_user'].push({ id: 'u7' }); // a delegate routes only when an account carries it (#27752)
     seedDelegation('u9', 'u7');
     const req = await svc.openNodeRequest(staffedInput(), SUBMITTER);
     // Guard the guard: the delegation machinery is wired and working in this

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

@@ -372,6 +372,7 @@ describe('ApprovalService (node era)', () => {
 
   it('field approver: fans a multi-select user field into one slot per user (#3447)', async () => {
     engine._tables['opportunity'] = [{ id: 'opp1', approvers_dynamic: ['u2', 'u3', 'u4'] }];
+    staffDirectory(engine, 'u4'); // a `field` id holds a slot only when an account carries it (#27752)
     const req = await svc.openNodeRequest(fieldInput({ record: { id: 'opp1' } }), CTX);
     expect(req.pending_approvers.sort()).toEqual(['u2', 'u3', 'u4']);
   });
@@ -386,6 +387,7 @@ describe('ApprovalService (node era)', () => {
     // No `opportunity` row — hard-deleted between submit and node entry. The
     // snapshot still carries a value, so the request opens against it (warn but
     // proceed) rather than wedging the flow.
+    staffDirectory(engine, 'u7');
     const req = await svc.openNodeRequest(
       fieldInput({ record: { id: 'opp1', approvers_dynamic: ['u7'] } }), CTX,
     );
@@ -3995,6 +3997,154 @@ describe('ApprovalService — a graph approver that expands to nobody warns (#38
   });
 });
 
+// #27752: the two siblings of #27397's `user` treatment. A `field` approver
+// reads its ids off the record, and an out-of-office delegation reroutes an
+// approver to its delegate: either can name an id no account carries, which
+// was a phantom slot no one could take.
+describe('ApprovalService — a `field` id or an out-of-office delegate that names no account (#27752)', () => {
+  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 fieldInput = (held: unknown, configExtra: Record<string, any> = {}) => ({
+    ...openInput([]),
+    record: { id: 'opp1', amount: 100, approver_ids: held },
+    config: {
+      approvers: [{ type: 'field' as const, value: 'approver_ids' }],
+      behavior: 'first_response' as const,
+      lockRecord: false,
+      ...configExtra,
+    },
+  });
+  const userInput = (value: string) => ({
+    ...openInput([]),
+    config: {
+      approvers: [{ type: 'user' as const, value }],
+      behavior: 'first_response' as const,
+      lockRecord: false,
+    },
+  });
+  const delegation = (delegator: string, delegate: string, id = `del_${delegator}`) => ({
+    id, delegator_id: delegator, delegate_id: delegate,
+    valid_from: '2025-01-01T00:00:00Z', valid_until: '2027-01-01T00:00:00Z',
+    reason: 'leave', organization_id: 't1',
+  });
+  const nobody = (warnings: any[]) => warnings.filter(([msg]) => String(msg).includes('expanded to nobody'));
+  const substitutes = (engine: any) => (engine._tables['sys_approval_action'] ?? [])
+    .filter((r: any) => r.action === 'ooo_substitute');
+  const sysUserDown = (engine: any) => {
+    const find = engine.find.bind(engine);
+    engine.find = async (object: string, options?: any) => {
+      if (object === 'sys_user') throw new Error('directory down');
+      return find(object, options);
+    };
+  };
+
+  it('`field`: an id no account carries warns as `user` does, and adds no slot', async () => {
+    const engine = makeFakeEngine();
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(fieldInput(['u2', 'u_gone']) as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u2']);
+    const hits = nobody(warnings);
+    expect(hits).toHaveLength(1);
+    expect(String(hits[0][0])).toContain("approver 'field:approver_ids' id 'u_gone' expanded to nobody");
+    expect(hits[0][1]).toMatchObject({ type: 'field', value: 'approver_ids', id: 'u_gone', organizationId: 't1' });
+  });
+
+  it('`field`: a slate it empties reaches the node\'s onEmptyApprovers', async () => {
+    const engine = makeFakeEngine();
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(fieldInput('u_gone', {
+      onEmptyApprovers: 'fallback',
+      fallbackApprovers: [{ type: 'user', value: 'u_backstop' }],
+    }) as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u_backstop']);
+  });
+
+  it('`field`: ids an account carries stay quiet', async () => {
+    const engine = makeFakeEngine();
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(fieldInput('u2,u3') as any, CTX) as any;
+
+    expect(req.pending_approvers.sort()).toEqual(['u2', 'u3']);
+    expect(nobody(warnings)).toEqual([]);
+  });
+
+  it('`field`: a failed directory read keeps the slot', async () => {
+    const engine = makeFakeEngine();
+    sysUserDown(engine);
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(fieldInput(['u_gone']) as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u_gone']);
+    expect(nobody(warnings)).toEqual([]);
+  });
+
+  it('a delegate no account carries falls back to the original approver, with the warning and no substitution', async () => {
+    const engine = makeFakeEngine();
+    engine._tables['sys_approval_delegation'] = [delegation('u2', 'u_gone')];
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(userInput('u2') as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u2']);
+    const hits = nobody(warnings);
+    expect(hits).toHaveLength(1);
+    expect(String(hits[0][0])).toContain("out-of-office delegate 'u_gone' of 'u2' expanded to nobody");
+    expect(hits[0][1]).toMatchObject({ delegatorId: 'u2', delegateId: 'u_gone', organizationId: 't1' });
+    expect(substitutes(engine)).toEqual([]);
+  });
+
+  it('a dead delegate at the end of a chain falls back to the original approver, not a middle hop', async () => {
+    const engine = makeFakeEngine();
+    engine._tables['sys_approval_delegation'] = [delegation('u2', 'u3'), delegation('u3', 'u_gone')];
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(userInput('u2') as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u2']);
+    expect(substitutes(engine)).toEqual([]);
+  });
+
+  it('a dead delegate on a `field` id falls back to that id', async () => {
+    const engine = makeFakeEngine();
+    engine._tables['sys_approval_delegation'] = [delegation('u3', 'u_gone')];
+    const { svc } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(fieldInput(['u2', 'u3']) as any, CTX) as any;
+
+    expect(req.pending_approvers.sort()).toEqual(['u2', 'u3']);
+  });
+
+  it('a live delegate still routes, with its substitution', async () => {
+    const engine = makeFakeEngine();
+    engine._tables['sys_approval_delegation'] = [delegation('u2', 'u9')];
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(userInput('u2') as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u9']);
+    expect(nobody(warnings)).toEqual([]);
+    expect(substitutes(engine)).toHaveLength(1);
+  });
+
+  it('a failed directory read keeps the delegate', async () => {
+    const engine = makeFakeEngine();
+    engine._tables['sys_approval_delegation'] = [delegation('u2', 'u_gone')];
+    sysUserDown(engine);
+    const { svc, warnings } = svcWithWarnings(engine);
+    const req = await svc.openNodeRequest(userInput('u2') as any, CTX) as any;
+
+    expect(req.pending_approvers).toEqual(['u_gone']);
+    expect(nobody(warnings)).toEqual([]);
+  });
+});
+
 // The empty-slate policy that NAMES people. Before it, an empty
 // `{ type: 'manager' }` rung had three endings and all three were bad: open on
 // a slate nobody can act on and wait for an admin (`admin_rescue`), kill the

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

@@ -2436,7 +2436,8 @@ export class ApprovalService implements IApprovalService {
       // account still resolves, as a banned team or unit member does (neither
       // expansion reads the flag). What is asked is the slot the route ENDS
       // on: an out-of-office hop that carries the request to a delegate has
-      // named someone (a removed account's delegation still routes, #24065).
+      // named someone (a removed account's delegation still routes, #24065;
+      // a delegate no account carries routes back here, #27752).
       // One that resolves to no one falls through to the graph types'
       // "expanded to nobody" warning below and adds no slot, so a slate it
       // leaves empty reaches the node's onEmptyApprovers. A failed read proves
@@ -2447,18 +2448,31 @@ export class ApprovalService implements IApprovalService {
       const value = String(a.value);
       const routed = await this.applyOooDelegation(value, now, organizationId, substitutions);
       if (routed.length !== 1 || routed[0] !== value || value.includes(':')) return routed;
-      let refusal: string | undefined;
-      try { refusal = await this.reassignTargetRefusal(value, null); } catch { refusal = undefined; }
-      if (refusal === undefined) return routed;
+      if (!await this.namesNoAccount(value)) return routed;
     }
     if (type === 'field' && record) {
       // #3447: a record field can name MANY approvers — a multi-select user
       // field arrives as an array (or a legacy CSV string). Fan each out into
       // its own slot and OOO-substitute per person; collapsing to `String(...)`
       // (→ `'u1,u2'`) would mint one bogus approver id and skip every delegate.
+      //
+      // #27752: each id takes the `user` treatment above — the slot is the id
+      // only when an account carries it. One that resolves to no one warns as
+      // `user` does and adds no slot, so a slate it leaves empty reaches the
+      // node's onEmptyApprovers; a failed read keeps the slot.
       const out: string[] = [];
       for (const id of csvSplit((record as any)[a.value])) {
-        out.push(...await this.applyOooDelegation(id, now, organizationId, substitutions));
+        const routed = await this.applyOooDelegation(id, now, organizationId, substitutions);
+        if (routed.length === 1 && routed[0] === id && !id.includes(':') && await this.namesNoAccount(id)) {
+          this.logger?.warn?.(
+            `[approvals] approver 'field:${a.value}' id '${id}' expanded to nobody — the slot names no one, so no `
+            + `one is asked through it. Staff it or re-point the approver; a slate left naming no one is decided `
+            + `by the node's onEmptyApprovers.`,
+            { type, value: a.value, id, organizationId: organizationId ?? null },
+          );
+          continue;
+        }
+        out.push(...routed);
       }
       return out;
     }
@@ -3411,6 +3425,7 @@ export class ApprovalService implements IApprovalService {
   ): Promise<string[]> {
     const start = String(userId ?? '').trim();
     if (!start) return [];
+    const mark = collector?.length ?? 0;
     let current = start;
     const visited = new Set<string>([current]);
     for (let hop = 0; hop < OOO_MAX_CHAIN; hop++) {
@@ -3422,9 +3437,34 @@ export class ApprovalService implements IApprovalService {
       visited.add(to);
       current = to;
     }
+    // #27752: a delegate no account carries is a slot no one can take. The
+    // original approver is a person the route already named, and delegation is
+    // only their convenience, so a broken one routes back to them — never to
+    // no slot — with the "expanded to nobody" warning, and its hops leave the
+    // collector so no substitution is audited or notified. A failed read
+    // proves nothing, so the delegate stays.
+    if (current !== start && !current.includes(':') && await this.namesNoAccount(current)) {
+      this.logger?.warn?.(
+        `[approvals] out-of-office delegate '${current}' of '${start}' expanded to nobody — the delegation `
+        + `names no account, so the slot stays with '${start}'. Point the delegation at a current user, or `
+        + `delete it.`,
+        { delegatorId: start, delegateId: current, organizationId: organizationId ?? null },
+      );
+      if (collector) collector.length = mark;
+      return [start];
+    }
     return [current];
   }
 
+  /**
+   * Whether no account carries `value` — the {@link reassignTargetRefusal}
+   * predicate asked with NO organization, as a `user` approver asks it
+   * (#27397). A failed read proves nothing, so it answers `false`.
+   */
+  private async namesNoAccount(value: string): Promise<boolean> {
+    try { return (await this.reassignTargetRefusal(value, null)) !== undefined; } catch { return false; }
+  }
+
   /**
    * The active OOO delegation for a delegator at `now`, or null. Validity is the
    * shared `isGrantActive` half-open window (ADR-0091 D2), enforced here at

packages/plugins/plugin-approvals/src/ooo-delegation-organization.test.ts

@@ -29,11 +29,11 @@ import { ApprovalService } from './approval-service.js';
 interface Row { [k: string]: any }
 
 /**
- * The accounts this suite routes `user` approver slots to: a `user` approver
- * holds a slot only when an account carries it (#27397), so the double's
- * directory starts with them.
+ * The accounts this suite routes approver slots to: a `user` approver holds a
+ * slot only when an account carries it (#27397), and a delegate only when an
+ * account carries it (#27752), so the double's directory starts with them.
  */
-const ROUTED_USERS = ['alice', 'carol'];
+const ROUTED_USERS = ['alice', 'bob', 'carol'];
 
 function makeEngine(seed: Record<string, Row[]> = {}) {
   const tables: Record<string, Row[]> = { sys_user: ROUTED_USERS.map((id) => ({ id })), ...seed };