fix(approvals): a dead approval run no longer leaves the record RECORD_LOCKED (#3456) (#3703)

Closes #3456 (#3424 follow-up, expected-behavior item 3). Implements both
options from the issue.

Prevention: the automation engine stamps `flowRunId` onto the run context at
setup, alongside `runAs`; it rides into every data node's ObjectQL context and
into `ctx.session`, and the lock hook exempts a write whose `flowRunId` matches
the pending request's `flow_run_id`. Keyed on run identity rather than
elevation, so a `runAs:'user'` run stays RLS-scoped while it writes. Pure
provenance — server-constructed like `isSystem`, read by no security
middleware.

Recovery: a sweep on the existing approvals clock finalizes a pending request
whose owning run is terminal as `recalled`, releasing the lock, audited under
the reserved actor `system:dead-run`. Covers the process-crash case no in-band
handler can. Fail-safe by construction: it acts only on an explicit terminal
status from a closed set, so `paused`, `running`, an unknown status, an
unreadable run and a missing automation engine are all read as alive.

Also fixes `AutomationEngine.getRun`, which returned the first log entry for a
run id; a run that pauses then finishes records two entries under one id, so
every approval/screen/wait flow reported itself as `paused` forever.

Residual, deliberate: a `runAs:'user'` run with no trigger user (a schedule)
passes no ObjectQL context, so it carries no `flowRunId` and the sweep is what
recovers that shape.

Author: os-zhuang

.changeset/approval-dead-run-record-lock.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/service-automation": patch
+"@objectstack/plugin-approvals": patch
+"@objectstack/objectql": patch
+"@objectstack/spec": patch
+---
+
+fix(approvals): a dead approval run no longer leaves the record RECORD_LOCKED (#3456)
+
+The record lock is keyed on a **pending** `sys_approval_request`, and it could
+not tell *the run that owns that request* from *an unrelated user editing the
+record*. So a flow that touched its own target record while its own approval was
+still pending — a manual `resume` with no decision, or a node that writes the
+record between opening the approval and the decision — died on its own
+`RECORD_LOCKED`, and the record stayed locked behind the dead run. Recovery
+existed (#3424 lets an admin `recall`/`reject` to release it) but nothing made it
+self-healing.
+
+Both halves are now closed.
+
+**Prevention — the owning run may write its own record.** The automation engine
+stamps `flowRunId` onto the run context at setup, alongside `runAs`, and it
+travels with every data node's ObjectQL context into `ctx.session`. The lock hook
+exempts a write whose `flowRunId` matches the pending request's `flow_run_id`.
+It is keyed on run identity rather than elevation on purpose: a `runAs:'user'`
+run stays fully RLS-scoped while it writes. `flowRunId` is pure provenance —
+server-constructed like `isSystem`, never client-supplied, evaluated by no
+security middleware, and the only write it permits is to the one record its own
+run already holds a pending request against.
+
+**Recovery — a sweep releases records held by runs that died anyway.** A pending
+request whose owning run has reached a terminal state (`completed`, `failed`,
+`cancelled`, `timed_out`) can never be decided, so it is finalised as `recalled`
+— releasing the lock — and audited under the reserved actor `system:dead-run`
+with the run and its status in the comment, so it is never mistaken for a
+submitter's withdrawal. It runs on the existing approvals sweep clock, which also
+covers the case no in-band handler can: a run killed by a process crash.
+
+The sweep is fail-safe by construction. It acts only on an explicit terminal
+status from a closed set; `paused` (the normal state of a live approval),
+`running`, an unrecognised status, an unknown run, a `getRun` that throws, and a
+deployment with no automation engine are all read as "still alive". The failure
+mode is "a dead run's lock survives until an admin recalls it" — today's
+behaviour — never "a live approval is destroyed".
+
+Also fixes `AutomationEngine.getRun`, which returned the **first** log entry for
+a run id rather than the latest. A run that pauses and later finishes records two
+entries under one id, so every suspend-then-finish run — every approval, screen
+and wait flow — reported itself as `paused` forever, both on the Runs
+observability surface and to this sweep.
+
+Residual, deliberately not changed here: a `runAs:'user'` run with no trigger
+user (a schedule) passes no ObjectQL context at all, so it carries no
+`flowRunId` and is still subject to the lock. Manufacturing a context just to
+carry the run id would flip that run from its documented unscoped fail-open
+(#1888) to baseline-member RLS — a separate, larger change. The sweep is what
+recovers that shape.

content/docs/automation/approvals.mdx

@@ -210,6 +210,12 @@ The node writes a `sys_approval_request` row: `status: 'pending'`,
 record is **locked** against edits while pending (`lockRecord`, default `true`),
 and the flow run parks until a decision arrives.
 
+The lock applies to everyone *except the run that opened the request*. A flow may
+still write its own target record while its own approval is pending, so it can
+never deadlock against itself. The exemption is keyed on run identity rather than
+elevation, so a `runAs:'user'` run stays row-level-security scoped while it
+writes — it does not become a system write.
+
 Only `approvers` is required on the node; everything else has a default
 (`behavior: 'first_response'`, `lockRecord: true`, `maxRevisions: 3`).
 
@@ -383,6 +389,20 @@ and is audited under the admin's own id. Prefer a guaranteed-staffed fallback
 approver so the set is never empty in the first place.
 </Callout>
 
+<Callout type="info">
+**A dead run releases its own lock.** If the flow run that opened an approval
+reaches a terminal state without a decision — it failed, was cancelled, timed
+out, or the process hosting it crashed — nothing is left to decide the request,
+so a periodic sweep finalizes it as `recalled` and releases the record. The
+audit row records the actor `system:dead-run` and names the run and its status,
+so it reads distinctly from a submitter's own recall.
+
+The sweep only ever acts on a run it can positively confirm is terminal: a
+paused run (the normal state of a live approval), an unknown run, or an
+unreachable automation engine all count as *alive* and are left untouched. It
+frees orphaned records; it never cancels a live approval.
+</Callout>
+
 ### Progress and notification deep links
 
 A pending multi-approver request also carries a **server-computed

content/docs/references/kernel/execution-context.mdx

@@ -66,6 +66,7 @@ const result = ExecutionContext.parse(data);
 | **accessible_org_ids** | `string[]` | optional |  |
 | **rlsMembership** | `Record<string, string[]>` | optional |  |
 | **isSystem** | `boolean` | ✅ |  |
+| **flowRunId** | `string` | optional |  |
 | **skipTriggers** | `boolean` | optional |  |
 | **skipAutomations** | `boolean` | optional |  |
 | **seedReplay** | `boolean` | optional |  |

packages/objectql/src/engine.ts

@@ -783,6 +783,12 @@ export class ObjectQL implements IDataEngine {
       // Propagate system-elevated flag so hooks can distinguish engine
       // self-writes (e.g. approval status mirror) from genuine user writes.
       ...((execCtx as any).isSystem ? { isSystem: true } : {}),
+      // Propagate the owning flow run so a hook can recognize writes made BY a
+      // run it already knows about — the approvals record lock lets the run that
+      // opened a pending approval write its own target record (#3456). Pure
+      // provenance: it grants nothing, and unlike `isSystem` it does not widen
+      // the write's authorization, so a `runAs:'user'` run stays RLS-scoped.
+      ...((execCtx as any).flowRunId ? { flowRunId: String((execCtx as any).flowRunId) } : {}),
       // Propagate the automation-suppression flag so the record-change trigger
       // can skip flow dispatch for seed/bulk writes (ADR: seed loads end-state
       // data, not user events). `skipAutomations` implies `skipTriggers` —

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

@@ -1561,12 +1561,178 @@ describe('record-lock hook (node era)', () => {
     ).resolves.toBeUndefined();
   });
 
+  // ── #3456 prevention half: the lock must not kill the run that owns it ──
+
+  it('allows the OWNING run to write its own target record', async () => {
+    await expect(
+      engine.fire('beforeUpdate', {
+        object: 'opportunity',
+        input: { id: 'opp1', data: { amount: 200 } },
+        // Neither elevated nor admin — the exemption rides on run identity
+        // alone, so a `runAs:'user'` run stays RLS-scoped while it writes.
+        session: { isSystem: false, positions: [], userId: 'u1', flowRunId: 'run_1' },
+      }),
+    ).resolves.toBeUndefined();
+  });
+
+  it('still blocks a DIFFERENT run writing the locked record', async () => {
+    await expect(
+      engine.fire('beforeUpdate', {
+        object: 'opportunity',
+        input: { id: 'opp1', data: { amount: 200 } },
+        session: { isSystem: false, positions: [], userId: 'u1', flowRunId: 'run_other' },
+      }),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('does not exempt anyone when the pending request carries no run id', async () => {
+    // A request with no owning run has nothing to match against — a stray
+    // `flowRunId` must not become a skeleton key.
+    engine._tables['sys_approval_request'][0].flow_run_id = null;
+    await expect(
+      engine.fire('beforeUpdate', {
+        object: 'opportunity',
+        input: { id: 'opp1', data: { amount: 200 } },
+        session: { isSystem: false, positions: [], userId: 'u1', flowRunId: 'run_1' },
+      }),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
   it('unbindAllHooks removes the lock hook', () => {
     expect(unbindAllHooks(engine as any)).toBe(1);
     expect(engine._hooks['beforeUpdate']).toHaveLength(0);
   });
 });
 
+// ── #3456 recovery half: release records held by a dead approval run ──
+//
+// The prevention half above stops a run from dying on its own lock. This sweep
+// covers the runs that die anyway — including a process crash, which no in-band
+// handler can clean up because the process that would run it is gone.
+//
+// The load-bearing property is what it must NOT do: a run merely *paused* on its
+// approval is the normal state of every live request, so anything short of an
+// explicit terminal status has to be read as "alive".
+describe('ApprovalService — dead-run release (#3456)', () => {
+  let engine: ReturnType<typeof makeFakeEngine>;
+  let svc: ApprovalService;
+  let n = 0;
+  const baseTime = new Date('2026-01-15T10:00:00Z').getTime();
+
+  /** Attach an automation surface whose `getRun` answers with `status`. */
+  const withRunStatus = (status: string | null) =>
+    svc.attachAutomation({ getRun: async () => (status == null ? null : { status }) } as any);
+
+  const requestRow = () => engine._tables['sys_approval_request'][0];
+
+  beforeEach(async () => {
+    engine = makeFakeEngine();
+    n = 0;
+    svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } });
+    bindApprovalLockHook(engine as any);
+    await svc.openNodeRequest(openInput(['u9'], {}, { approvalStatusField: 'approval_status' }), CTX);
+    engine._tables['opportunity'] = [{ id: 'opp1', amount: 100 }];
+  });
+
+  it('releases a pending request whose owning run failed', async () => {
+    withRunStatus('failed');
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 1, released: 1 });
+    expect(requestRow().status).toBe('recalled');
+    expect(requestRow().pending_approvers).toBeNull();
+    expect(requestRow().completed_at).toBeTruthy();
+  });
+
+  it('audits the release as a dead-run abandonment, not a submitter recall', async () => {
+    withRunStatus('failed');
+    await svc.releaseDeadRunRequests();
+    const action = engine._tables['sys_approval_action'].find((a: any) => a.actor_id === 'system:dead-run');
+    expect(action).toBeTruthy();
+    expect(action.action).toBe('recall');
+    expect(action.comment).toMatch(/run_1/);
+    expect(action.comment).toMatch(/failed/);
+  });
+
+  it('actually unlocks the record — a plain user edit succeeds afterwards', async () => {
+    // The end-to-end point of the whole sweep.
+    const edit = () => engine.fire('beforeUpdate', {
+      object: 'opportunity',
+      input: { id: 'opp1', data: { amount: 200 } },
+      session: { isSystem: false, positions: [], userId: 'u1' },
+    });
+    await expect(edit()).rejects.toThrow(/RECORD_LOCKED/);   // held by the dead run
+    withRunStatus('failed');
+    await svc.releaseDeadRunRequests();
+    await expect(edit()).resolves.toBeUndefined();            // released
+  });
+
+  it('mirrors the configured status field on release', async () => {
+    withRunStatus('failed');
+    await svc.releaseDeadRunRequests();
+    expect(engine._tables['opportunity'][0].approval_status).toBe('recalled');
+  });
+
+  it('leaves a PAUSED run alone — that is a live approval', async () => {
+    withRunStatus('paused');
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 1, released: 0 });
+    expect(requestRow().status).toBe('pending');
+  });
+
+  it.each([
+    ['an unknown run (null)', null],
+    ['an unrecognised status', 'reticulating_splines'],
+    ['a still-running run', 'running'],
+  ])('leaves the request pending for %s', async (_label, status) => {
+    withRunStatus(status as any);
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 1, released: 0 });
+    expect(requestRow().status).toBe('pending');
+  });
+
+  it('leaves the request pending when getRun throws', async () => {
+    svc.attachAutomation({ getRun: async () => { throw new Error('engine unreachable'); } } as any);
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 1, released: 0 });
+    expect(requestRow().status).toBe('pending');
+  });
+
+  it('is a no-op with no automation engine attached', async () => {
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 0, released: 0 });
+    expect(requestRow().status).toBe('pending');
+  });
+
+  it('is a no-op when the surface has no getRun (older engine)', async () => {
+    svc.attachAutomation({ resume: async () => undefined } as any);
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 0, released: 0 });
+    expect(requestRow().status).toBe('pending');
+  });
+
+  it('skips a request with no owning run', async () => {
+    requestRow().flow_run_id = null;
+    withRunStatus('failed');
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 1, released: 0 });
+    expect(requestRow().status).toBe('pending');
+  });
+
+  it.each(['completed', 'cancelled', 'timed_out'])(
+    'releases on the other terminal status %s', async (status) => {
+      // A terminal run can never decide its request, whatever ended it — a
+      // `completed` one means someone resumed the run out of band.
+      withRunStatus(status);
+      expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 1, released: 1 });
+      expect(requestRow().status).toBe('recalled');
+    },
+  );
+
+  it('one unreadable request does not stop the sweep', async () => {
+    await svc.openNodeRequest(
+      { ...openInput(['u9']), recordId: 'opp2', runId: 'run_2' } as any, CTX,
+    );
+    let call = 0;
+    svc.attachAutomation({
+      getRun: async () => { call++; if (call === 1) throw new Error('boom'); return { status: 'failed' }; },
+    } as any);
+    expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 2, released: 1 });
+  });
+});
+
 // ── Out-of-office auto-skip (#1322 M1/M4) ─────────────────────────────
 //
 // When a resolved individual approver has declared an active OOO delegation,