fix(approvals): resolve field/manager approvers against the live record at node entry (#3447) (#3495)

Approval nodes resolved `field` / `manager` approvers against the trigger
snapshot the flow froze into `$record` at submit time, so an earlier step could
not set the field a later step routes on — dynamic routing / dynamic co-sign
were impossible to express. Resolution was already lazy per-node; the defect was
the data source (a frozen snapshot, never re-read), betrayed by graph approvers
resolving live in the same node while field/manager read the snapshot.

Re-read the record's live state by id at node entry under system identity, with
a warn-and-fall-back-to-snapshot path when the record is gone. Also fan a
multi-select user field into one approver slot per user (previously stringified
to one bogus id) and apply OOO delegation per fanned-out user (previously
silently skipped for multi-value fields). payload_json still snapshots the
submitted record — its contract is unchanged.

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: os-zhuang

.changeset/approver-live-record-3447.md

@@ -0,0 +1,7 @@
+---
+"@objectstack/plugin-approvals": minor
+---
+
+Approval nodes now resolve `field` / `manager` approvers against the record's **live** state at node entry, not the trigger snapshot the flow froze at submit time (#3447). An earlier step — or the approver of an earlier step — can now write the field that routes a later step's approvers, enabling dynamic routing / dynamic co-sign (e.g. a lead reviewer picking which departments co-review, then those departments resolving as parallel approvers). Graph approvers (team / position / department / tier) already resolved live; this brings the in-record types into line.
+
+Also fixes two latent defects on the same path: a multi-select user field now fans out into one approver slot per user (previously the array was stringified to a single bogus id), and out-of-office delegation is applied per fanned-out user (previously silently skipped for multi-value fields). When the record can't be re-read (hard-deleted mid-flow, or a backend that can't serve a point read), resolution falls back to the trigger snapshot and warns rather than wedging the flow.

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

@@ -187,6 +187,65 @@ describe('ApprovalService (node era)', () => {
     expect(engine._tables['opportunity'][0].approval_status).toBe('pending');
   });
 
+  // ── approver expansion: field (live re-read + fan-out, #3447) ────
+  //
+  // A `field` approver names WHO decides from a record field. It must bind to
+  // the record's LIVE value at node entry — an earlier step (or the approver of
+  // an earlier step) may have written it after submit — not the trigger snapshot
+  // the run froze into `$record`. A multi-select user field fans out into one
+  // slot per user.
+
+  const fieldInput = (extra: Record<string, any> = {}) => ({
+    ...openInput([]),
+    config: {
+      approvers: [{ type: 'field' as const, value: 'approvers_dynamic' }],
+      behavior: 'unanimous' as const,
+      lockRecord: true,
+    },
+    ...extra,
+  });
+
+  it('field approver: resolves against the LIVE record, not the trigger snapshot (#3447)', async () => {
+    // The minimal repro: submitted with the routing field empty; a prior step
+    // wrote the co-reviewers mid-flow. Resolution must see them.
+    engine._tables['opportunity'] = [{ id: 'opp1', amount: 100, approvers_dynamic: ['u2', 'u3'] }];
+    const req = await svc.openNodeRequest(
+      fieldInput({ record: { id: 'opp1', amount: 100, approvers_dynamic: [] } }), CTX,
+    );
+    expect(req.pending_approvers.sort()).toEqual(['u2', 'u3']);
+  });
+
+  it('field approver: the live value WINS over a stale snapshot value (#3447)', async () => {
+    // Snapshot named u1; the record now names u2. u2 decides.
+    engine._tables['opportunity'] = [{ id: 'opp1', approvers_dynamic: ['u2'] }];
+    const req = await svc.openNodeRequest(
+      fieldInput({ record: { id: 'opp1', approvers_dynamic: ['u1'] } }), CTX,
+    );
+    expect(req.pending_approvers).toEqual(['u2']);
+  });
+
+  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'] }];
+    const req = await svc.openNodeRequest(fieldInput({ record: { id: 'opp1' } }), CTX);
+    expect(req.pending_approvers.sort()).toEqual(['u2', 'u3', 'u4']);
+  });
+
+  it('field approver: fans a legacy CSV string field into multiple slots (#3447)', async () => {
+    engine._tables['opportunity'] = [{ id: 'opp1', approvers_dynamic: 'u2,u3' }];
+    const req = await svc.openNodeRequest(fieldInput({ record: { id: 'opp1' } }), CTX);
+    expect(req.pending_approvers.sort()).toEqual(['u2', 'u3']);
+  });
+
+  it('field approver: falls back to the trigger snapshot when the record is gone (#3447)', async () => {
+    // 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.
+    const req = await svc.openNodeRequest(
+      fieldInput({ record: { id: 'opp1', approvers_dynamic: ['u7'] } }), CTX,
+    );
+    expect(req.pending_approvers).toEqual(['u7']);
+  });
+
   // ── approver expansion: position (ADR-0090 D3) ──────────────────
 
   const positionInput = (extra: Record<string, any> = {}) => ({
@@ -1294,6 +1353,20 @@ describe('ApprovalService — out-of-office delegation (#1322)', () => {
     expect(req.pending_approvers).toEqual(['bob']);
   });
 
+  it('type:field (multi-select) — reroutes each out-of-office user independently (#3447)', async () => {
+    // Pre-#3447 the array was stringified to one bogus id ('alice,dave'), which
+    // matched no delegation — OOO silently no-op'd. Fanned out, alice → bob
+    // applies and dave is left untouched.
+    seedDelegation([{ delegator_id: 'alice', delegate_id: 'bob' }]);
+    const input = {
+      ...openInput([]),
+      record: { id: 'opp1', reviewers: ['alice', 'dave'] },
+      config: { approvers: [{ type: 'field', value: 'reviewers' }], behavior: 'unanimous', lockRecord: true },
+    };
+    const req = await svc.openNodeRequest(input as any, CTX);
+    expect(req.pending_approvers.sort()).toEqual(['bob', 'dave']);
+  });
+
   it('type:manager — reroutes when the resolved manager is out of office', async () => {
     engine._tables['sys_user'] = [{ id: 'carol', manager_id: 'alice' }];
     seedDelegation([{ delegator_id: 'alice', delegate_id: 'bob' }]);

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

@@ -443,7 +443,15 @@ export class ApprovalService implements IApprovalService {
       return this.applyOooDelegation(String(a.value), now, organizationId, substitutions);
     }
     if (type === 'field' && record) {
-      return this.applyOooDelegation(String((record as any)[a.value] ?? ''), now, organizationId, substitutions);
+      // #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.
+      const out: string[] = [];
+      for (const id of csvSplit((record as any)[a.value])) {
+        out.push(...await this.applyOooDelegation(id, now, organizationId, substitutions));
+      }
+      return out;
     }
     try {
       if (type === 'team') {
@@ -662,6 +670,45 @@ export class ApprovalService implements IApprovalService {
     }
   }
 
+  /**
+   * Re-read a business record's CURRENT state by id so approver resolution binds
+   * to live data at node entry, not the trigger snapshot the flow froze into
+   * `$record` at submit time (#3447). A `field` / `manager` approver names *who*
+   * decides, and an earlier node — or the approver of an earlier step — may have
+   * written that routing field after submit (e.g. a lead reviewer picking which
+   * departments co-review). Graph approvers (team / position / …) already query
+   * live; this brings the in-record types into line.
+   *
+   * Read under system identity: approver routing is a platform concern and the
+   * record is the flow's own subject, so the submitter's RLS/FLS must not narrow
+   * it. Degrades to `fallback` (the snapshot) when the record can't be re-read —
+   * hard-deleted between submit and node entry, or an object whose backend can't
+   * serve a point read — warning rather than throwing so a transient miss can't
+   * wedge an approval. That "warn but proceed" stance matches the
+   * no-concrete-approver guard (#3424) and the "record is gone" enrichment path
+   * that already falls back to the payload snapshot.
+   */
+  private async loadLiveRecord(object: string, recordId: string, fallback?: any): Promise<any> {
+    try {
+      const rows = await this.engine.find(object, {
+        where: { id: recordId }, limit: 1, context: SYSTEM_CTX,
+      } as any);
+      const live = Array.isArray(rows) ? rows[0] : rows;
+      if (live) return live;
+      this.logger?.warn?.(
+        `[approvals] live record ${object}/${recordId} not found at node entry — `
+        + 'resolving approvers against the trigger snapshot (#3447 fallback).',
+        { object, recordId },
+      );
+    } catch (err: any) {
+      this.logger?.warn?.(
+        `[approvals] live record re-read failed for ${object}/${recordId}: ${err?.message ?? err} — `
+        + 'resolving approvers against the trigger snapshot (#3447 fallback).',
+      );
+    }
+    return fallback ?? {};
+  }
+
   // ── ADR-0019: Approval-as-flow-node ──────────────────────────
   //
   // A flow's Approval node opens a request via `openNodeRequest` (carrying its
@@ -715,8 +762,12 @@ export class ApprovalService implements IApprovalService {
     // per_group finalization is decided against the slate resolved at OPEN time
     // (OOO-substituted), not re-resolved live at each decision.
     const groups: Record<string, string[]> = {};
+    // #3447: resolve approvers against the record's LIVE state at node entry, not
+    // the trigger snapshot carried in `input.record`. This is the whole fix — an
+    // earlier step may have written the field this node routes on.
+    const liveRecord = await this.loadLiveRecord(input.object, input.recordId, input.record);
     const approvers = await this.expandApprovers(
-      { approvers: input.config.approvers }, input.record, ctxOrg, { now: nowDate.getTime(), substitutions, groups },
+      { approvers: input.config.approvers }, liveRecord, ctxOrg, { now: nowDate.getTime(), substitutions, groups },
     );
 
     // #3424: an approval routed to a target with no holders (e.g. an unstaffed