fix(approvals): the status mirror names the human who caused the transition (#3783) (#3790)

The write that puts the approval status on the business record is what fires
that object's record-change flows — so it is the seam "when the invoice is
approved, do X" runs through. It presented a bare `{ isSystem: true }` context
with no `userId`, at six call sites that each know exactly who acted.

Combined with #3760 (a `runAs:'user'` run with no trigger user may not touch
data) that gap made the most natural approvals automation unwritable in its
obvious form: the cascade inherited no user, its data nodes were refused, and
the only way forward was blanket `runAs:'system'` elevation for a case where a
scoped identity existed at the call site all along.

The mirror now carries the acting user and stays `isSystem` — the record is
normally locked while its approval is live, so only a platform write can land
the status. Elevation and anonymity are separate choices; this write only ever
needed the first.

- Identity comes from the authenticated context, never the request body's
  `actorId`: that field is caller-supplied and only checked against the pending
  approver slate, so promoting it to the identity of an RLS-scoped write would
  turn a mislabelled audit trail into identity spoofing.
- ADR-0043 action links now decide as the approver their single-use token binds,
  so an emailed approval cascades identically to one made in the UI.
- The SLA auto-decision and the dead-run sweep stay user-less on purpose —
  `system:sla` / `system:dead-run` are audit actors, not users. A flow reacting
  to those declares `runAs:'system'`, which is now a decision, not an artefact.
- Attribution only: the request's org is deliberately not carried, because
  `tenantId` on an ExecutionContext is a driver-scoping knob and would have
  silently no-op'd the mirror on a record whose org differs.

Visible change: the mirrored record's `updated_by` now names the acting user —
ObjectQL's audit stamping is gated on `userId` alone and `isSystem` buys no
exemption. That is the attribution this fix is for.

Verified end-to-end: a new integration test drives a real kernel (ObjectQL +
record-change trigger + automation + ApprovalService) and asserts an approval
cascade now runs, with the dead-run sweep's user-less mirror still refused as
the control. Both fail without this change.

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

Author: os-zhuang

.changeset/approval-status-mirror-names-the-actor.md

@@ -0,0 +1,56 @@
+---
+"@objectstack/plugin-approvals": minor
+"@objectstack/service-automation": patch
+---
+
+fix(approvals): the status mirror names the human who caused the transition (#3783)
+
+When an approval moves, the service writes the new status onto the business
+record (`approvalStatusField`). That write is what fires the record-change flows
+bound to that object — so it is the seam "when the invoice is approved, do X"
+runs through. It presented a bare `{ isSystem: true }` context with **no
+`userId`**, at six call sites that each know exactly who acted: a submitter
+submitting, an approver approving, rejecting, sending back, recalling.
+
+Combined with #3760 — which stopped letting a `runAs:'user'` run with no trigger
+user touch data — that identity gap made the most natural approvals automation
+there is unwritable in its obvious form. The cascade inherited no user, so its
+data nodes were refused, and the author's only way forward was to declare
+`runAs: 'system'` and take blanket elevation for a case where a perfectly good
+scoped identity existed at the call site all along.
+
+The mirror now carries the acting user. It stays `isSystem` — the record is
+normally locked while its approval is live, so only a platform write can land the
+status — because elevation and anonymity are separate choices, and this write
+only ever needed the first. Cascades now run as the deciding user with RLS
+enforced.
+
+- **The identity is the authenticated principal, never the request body's
+  `actorId`.** `actorId` arrives from the caller (`body.actorId ?? context.userId`)
+  and is only checked against the pending approver slate, never against the
+  caller. That is tolerable on an audit row; promoting it to the identity of an
+  RLS-scoped write would have turned a mislabelled audit trail into identity
+  spoofing.
+- **Approval-by-email-link is attributed too.** ADR-0043 action links carry no
+  session, so they used to decide as pure system. The single-use hashed token
+  binds exactly one approver and is re-checked against the live slate at
+  redemption — that is an authentication — so the redeemed decision now presents
+  that approver, and an emailed approval cascades identically to one made in the
+  UI.
+- **The two machine-driven transitions stay user-less on purpose**: the SLA
+  escalation's auto-decision and the dead-run sweep. `system:sla` and
+  `system:dead-run` are reserved audit actors, not users, and presenting one as a
+  user would put a non-user in `updated_by` and in every downstream flow's
+  identity. A flow that wants to react to those declares `runAs:'system'` — the
+  honest answer, and now a deliberate one rather than an artefact.
+- **Attribution only — the write is not newly org-scoped.** On an
+  ExecutionContext `tenantId` is a driver-scoping knob, not attribution
+  (ObjectQL turns it into a tenant predicate), so passing the request's org would
+  have silently no-op'd the mirror on a record whose org differs. The automation
+  engine already back-fills a run's `tenantId` from the resolved user's grants.
+
+**Visible change:** the mirrored record's `updated_by` now names the acting user
+instead of retaining its previous value — ObjectQL's audit stamping is gated on
+the write context's `userId` alone, and `isSystem` buys no exemption. That is the
+attribution this fix is for: the approver who set the record to `approved` is now
+its last modifier.

packages/plugins/plugin-approvals/package.json

@@ -26,6 +26,7 @@
   "devDependencies": {
     "@objectstack/objectql": "workspace:*",
     "@objectstack/service-automation": "workspace:*",
+    "@objectstack/trigger-record-change": "workspace:*",
     "@types/node": "^26.1.1",
     "typescript": "^6.0.3",
     "vitest": "^4.1.10"

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

@@ -45,9 +45,13 @@ function makeFakeEngine() {
     return true;
   }
 
+  /** Every `update` the service made, with the context it presented (#3783). */
+  const writes: Array<{ object: string; data: any; context: any }> = [];
+
   return {
     _tables: tables,
     _hooks: hooks,
+    _writes: writes,
     async find(object: string, options?: any) {
       const rows = ensure(object).filter(r => matches(r, options?.filter ?? options?.where));
       if (options?.orderBy?.[0]) {
@@ -73,6 +77,7 @@ function makeFakeEngine() {
     async update(object: string, idOrData: any, _opts?: any) {
       const data = typeof idOrData === 'object' ? idOrData : _opts;
       const id = typeof idOrData === 'object' ? idOrData.id : idOrData;
+      writes.push({ object, data, context: _opts?.context });
       const table = ensure(object);
       const i = table.findIndex(r => r.id === id);
       if (i >= 0) table[i] = { ...table[i], ...data };
@@ -2538,3 +2543,135 @@ describe('in-band transitions finalise before they resume (#3456 invariant)', ()
     expectCleanHandoffs();
   });
 });
+
+/**
+ * #3783 — the status mirror names the human who caused the transition.
+ *
+ * The mirror write lands on the CUSTOMER's object, so it is what fires that
+ * object's record-change flows. It has to stay `isSystem` (the record is locked
+ * while its approval is live), but dropping the actor left every one of those
+ * cascades with no trigger user — which #3760 now refuses outright, forcing
+ * "when the invoice is approved, do X" to declare `runAs:'system'`.
+ *
+ * Each case therefore asserts BOTH halves: the elevation survives (or the lock
+ * hook stops mirroring at all) and the identity is present.
+ */
+describe('status mirror identity (#3783)', () => {
+  let engine: ReturnType<typeof makeFakeEngine>;
+  let svc: ApprovalService;
+  let n = 0;
+  const baseTime = new Date('2026-01-15T10:00:00Z').getTime();
+
+  const REVISE_FLOW = {
+    name: 'deal_approval',
+    edges: [{ id: 'e_rev', source: 'approve_step', target: 'wait_revision', label: 'revise' }],
+  };
+
+  /** The context the service presented on the mirror write, or undefined. */
+  const mirrorContext = () =>
+    engine._writes.filter(w => w.object === 'opportunity').at(-1)?.context as any;
+
+  const open = (configExtra: Record<string, any> = {}, ctx: any = CTX) =>
+    svc.openNodeRequest(openInput(['u9'], {}, { approvalStatusField: 'approval_status', ...configExtra }), ctx);
+
+  beforeEach(() => {
+    engine = makeFakeEngine();
+    n = 0;
+    svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } });
+    svc.attachAutomation({
+      async resume() {},
+      async cancelRun() {},
+      async getFlow() { return REVISE_FLOW; },
+    } as any);
+    engine._tables['opportunity'] = [{ id: 'opp1', amount: 100 }];
+  });
+
+  it('submit: mirrors as the submitter, still elevated', async () => {
+    await open();
+    expect(mirrorContext()).toMatchObject({ isSystem: true, userId: 'u1' });
+  });
+
+  it('decide: mirrors as the deciding user', async () => {
+    const req = await open();
+    const approver = { ...CTX, userId: 'u9' };
+    await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, approver as any);
+    expect(engine._tables['opportunity'][0].approval_status).toBe('approved');
+    expect(mirrorContext()).toMatchObject({ isSystem: true, userId: 'u9' });
+  });
+
+  it('recall: mirrors as the recalling user', async () => {
+    const req = await open();
+    await svc.recall(req.id, { actorId: 'u1' }, CTX);
+    expect(mirrorContext()).toMatchObject({ isSystem: true, userId: 'u1' });
+  });
+
+  it('sendBack: mirrors as the approver who returned it', async () => {
+    const req = await open();
+    const approver = { ...CTX, userId: 'u9' };
+    await svc.sendBack(req.id, { actorId: 'u9', comment: 'redo the totals' }, approver as any);
+    expect(engine._tables['opportunity'][0].approval_status).toBe('returned');
+    expect(mirrorContext()).toMatchObject({ isSystem: true, userId: 'u9' });
+  });
+
+  it('sendBack past the revision budget: the auto-reject mirror names the approver too', async () => {
+    const req = await open({ maxRevisions: 0 });
+    const approver = { ...CTX, userId: 'u9' };
+    const out = await svc.sendBack(req.id, { actorId: 'u9' }, approver as any);
+    expect(out.autoRejected, 'expected the auto-reject branch').toBe(true);
+    expect(engine._tables['opportunity'][0].approval_status).toBe('rejected');
+    expect(mirrorContext()).toMatchObject({ isSystem: true, userId: 'u9' });
+  });
+
+  it('action link: mirrors as the approver the token is bound to', async () => {
+    // ADR-0043 email approval — no session at all, but the single-use hashed
+    // token names exactly one approver, and `resolveActionToken` has just
+    // re-checked they still hold a pending slot. That IS an authenticated act.
+    const req = await open();
+    const { approve } = await svc.issueActionTokens(req.id, 'u9');
+    expect(await svc.redeemActionToken(approve)).toMatchObject({ ok: true });
+    expect(mirrorContext()).toMatchObject({ isSystem: true, userId: 'u9' });
+  });
+
+  it('never takes the identity from the caller-supplied actorId', async () => {
+    // `actorId` arrives in the REST body (`body.actorId ?? context.userId`) and
+    // is only checked against the pending slate, never against the caller. It is
+    // fine on an audit row; making it the identity of an RLS-scoped write would
+    // let any authenticated caller borrow a slot holder's identity.
+    const req = await open();
+    const someoneElse = { ...CTX, userId: 'intruder' };
+    await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, someoneElse as any);
+    expect(mirrorContext()?.userId).toBe('intruder');
+  });
+
+  it('SLA auto-decision: stays user-less — no human did it', async () => {
+    const req = await open({ escalation: { timeoutHours: 1, action: 'auto_approve', notifySubmitter: false } });
+    const raw = engine._tables['sys_approval_request'].find((r: any) => r.id === req.id)!;
+    raw.created_at = new Date(baseTime - 3 * 60 * 60 * 1000).toISOString();
+    await svc.runEscalations();
+    expect(engine._tables['opportunity'][0].approval_status).toBe('approved');
+    // `system:sla` is a reserved audit actor, not a user — it must never be
+    // presented as one. The cascade stays user-less on purpose; a flow that
+    // wants to react to an SLA auto-decision declares runAs:'system'.
+    expect(mirrorContext()?.userId).toBeUndefined();
+    expect(mirrorContext()).toMatchObject({ isSystem: true });
+  });
+
+  it('dead-run sweep: stays user-less — no human did it', async () => {
+    await open();
+    svc.attachAutomation({ getRun: async () => ({ status: 'failed' }) } as any);
+    expect(await svc.releaseDeadRunRequests()).toMatchObject({ released: 1 });
+    expect(engine._tables['opportunity'][0].approval_status).toBe('recalled');
+    expect(mirrorContext()?.userId).toBeUndefined();
+    expect(mirrorContext()).toMatchObject({ isSystem: true });
+  });
+
+  it('carries the actor WITHOUT org-scoping the write', async () => {
+    // `tenantId` on an ExecutionContext is a driver-scoping knob, not
+    // attribution: ObjectQL turns it into a tenant predicate on the update. The
+    // submitter's org (`t1` on CTX) must therefore not ride along, or the mirror
+    // would silently no-op on a record whose org differs from the request's.
+    await open();
+    expect(mirrorContext()).not.toHaveProperty('tenantId');
+    expect(mirrorContext()).not.toHaveProperty('organizationId');
+  });
+});