diff --git a/.changeset/approval-dead-run-record-lock.md b/.changeset/approval-dead-run-record-lock.md
new file mode 100644
index 0000000000..a9da2514f6
--- /dev/null
+++ b/.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.
diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx
index b803113df9..c946dd18a9 100644
--- a/content/docs/automation/approvals.mdx
+++ b/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.
+
+**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.
+
+
### Progress and notification deep links
A pending multi-approver request also carries a **server-computed
diff --git a/content/docs/references/kernel/execution-context.mdx b/content/docs/references/kernel/execution-context.mdx
index ebe6e4378f..095159246a 100644
--- a/content/docs/references/kernel/execution-context.mdx
+++ b/content/docs/references/kernel/execution-context.mdx
@@ -66,6 +66,7 @@ const result = ExecutionContext.parse(data);
| **accessible_org_ids** | `string[]` | optional | |
| **rlsMembership** | `Record` | optional | |
| **isSystem** | `boolean` | ✅ | |
+| **flowRunId** | `string` | optional | |
| **skipTriggers** | `boolean` | optional | |
| **skipAutomations** | `boolean` | optional | |
| **seedReplay** | `boolean` | optional | |
diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts
index 01d62a82b3..acf3a8ce45 100644
--- a/packages/objectql/src/engine.ts
+++ b/packages/objectql/src/engine.ts
@@ -778,6 +778,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` —
diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts
index fe4b419aca..c8e638e22b 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.test.ts
+++ b/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;
+ 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,
diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts
index 9aa825114c..3732d5d3f8 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.ts
+++ b/packages/plugins/plugin-approvals/src/approval-service.ts
@@ -73,6 +73,19 @@ export interface ApprovalResumeSurface {
* which has no reject edge to resume down.
*/
cancelRun?(runId: string, reason?: string): Promise;
+ /**
+ * Look up a run's recorded outcome (#3456). Used by the dead-run sweep to ask
+ * "is the run behind this pending request still alive?".
+ *
+ * The contract that makes the sweep safe is the answer for a run that is
+ * merely SUSPENDED (the normal state of a run waiting on an approval): the
+ * engine writes no execution-log entry until a run reaches a terminal state,
+ * so a suspended run resolves to `null`, never to a status. The sweep
+ * therefore acts only on an explicit terminal-failure status and treats
+ * `null` — unknown run, evicted log, no durable store, no automation engine —
+ * as "still alive".
+ */
+ getRun?(runId: string): Promise<{ status?: string } | null>;
}
/**
@@ -101,6 +114,24 @@ export const ESCALATION_JOB_NAME = 'approvals-sla-escalation';
export const ESCALATION_SCAN_INTERVAL_MS = 5 * 60 * 1000;
/** Reserved actor id for machine decisions made by the SLA scanner. */
export const SLA_ACTOR_ID = 'system:sla';
+/** Reserved actor id for requests abandoned because their run died (#3456). */
+export const DEAD_RUN_ACTOR_ID = 'system:dead-run';
+/**
+ * Run statuses that mean "this run will never resume", so a request still
+ * pending on it is orphaned (#3456). A CLOSED set, deliberately: the dead-run
+ * sweep treats every other answer — `paused` (a run waiting on its approval,
+ * the normal case), `running`, an unknown status, or no answer at all — as
+ * alive, so an unrecognised state can never cost someone a live approval.
+ *
+ * `completed` belongs here with the failure states. The approval node only
+ * writes a request row on the path where it also suspends the run, and every
+ * in-band transition (decide / recall / send-back / resubmit) finalises the
+ * request *before* it resumes the run — so a completed run with a still-pending
+ * request means the run was resumed out of band and left the request behind.
+ */
+const TERMINAL_RUN_STATUSES: ReadonlySet = new Set([
+ 'completed', 'failed', 'cancelled', 'timed_out',
+]);
/** Default lifetime of an actionable-link token (ADR-0043). */
export const ACTION_TOKEN_TTL_MS = 72 * 60 * 60 * 1000;
@@ -2195,6 +2226,120 @@ export class ApprovalService implements IApprovalService {
return { scanned: rows.length, escalated };
}
+ // ── Dead-run release (#3456) ──────────────────────────────────
+
+ /**
+ * One dead-run sweep: a pending request whose owning flow run has reached a
+ * TERMINAL state can never be decided — nothing is left to resume — so the
+ * request is finalised as `recalled` and, with `lockRecord`, the record it was
+ * holding is released.
+ *
+ * This is the recovery half of #3456. The prevention half is the record lock's
+ * owning-run exemption (`lifecycle-hooks.ts`), which stops a run from killing
+ * itself on its own lock in the first place; this sweep cleans up the runs that
+ * still die — for any reason, including a process crash, which no in-band
+ * handler can catch because the process that would have run it is gone.
+ *
+ * **Fail-safe by construction.** It acts only on an explicit terminal status
+ * from a closed set. Every other answer — `paused` (the normal state of a run
+ * waiting on its approval), `running`, an unrecognised status, `null` (unknown
+ * run, evicted log, no durable store), a `getRun` that throws, or no automation
+ * engine at all — is read as "still alive" and left strictly alone. The failure
+ * mode is therefore "a dead run's lock survives until an admin recalls it"
+ * (today's behaviour, #3424), never "a live approval is destroyed".
+ *
+ * `recalled` is the finalisation because it is the platform's existing terminal
+ * state for *a live request that ended without a decision*; the audit row names
+ * the real cause and {@link DEAD_RUN_ACTOR_ID} the real actor, so a dead-run
+ * release is never mistaken for a submitter's withdrawal.
+ */
+ async releaseDeadRunRequests(): Promise<{ scanned: number; released: number }> {
+ // No liveness oracle → no basis to declare anything dead.
+ if (typeof this.automation?.getRun !== 'function') return { scanned: 0, released: 0 };
+
+ let rows: any[] = [];
+ try {
+ rows = await this.engine.find('sys_approval_request', {
+ where: { status: 'pending' }, limit: 500, context: SYSTEM_CTX,
+ }) ?? [];
+ } catch (err: any) {
+ this.logger?.warn?.('[approvals] dead-run sweep failed to list requests', {
+ error: err?.message ?? String(err),
+ });
+ return { scanned: 0, released: 0 };
+ }
+
+ let released = 0;
+ for (const raw of rows) {
+ try {
+ const runId = raw?.flow_run_id ? String(raw.flow_run_id) : '';
+ if (!runId) continue; // not node-driven — no run owns it, nothing to check
+
+ let status: string | undefined;
+ try {
+ const run = await this.automation.getRun!(runId);
+ status = typeof run?.status === 'string' ? run.status : undefined;
+ } catch (err: any) {
+ // Unknown liveness is NOT death — leave the request pending.
+ this.logger?.warn?.('[approvals] dead-run sweep could not read run status', {
+ request: raw?.id, run: runId, error: err?.message ?? String(err),
+ });
+ continue;
+ }
+ if (!status || !TERMINAL_RUN_STATUSES.has(status)) continue;
+
+ await this.abandonForDeadRun(raw, runId, status);
+ released++;
+ } catch (err: any) {
+ // One bad row never stops the sweep (mirrors runEscalations).
+ this.logger?.warn?.('[approvals] dead-run release failed for request', {
+ request: raw?.id, error: err?.message ?? String(err),
+ });
+ }
+ }
+ if (released > 0) {
+ this.logger?.info?.('[approvals] dead-run sweep', { scanned: rows.length, released });
+ }
+ return { scanned: rows.length, released };
+ }
+
+ /**
+ * Finalise one pending request whose owning run is terminal. Mirrors the
+ * shape of {@link recall} — audit row first (so a crash mid-release leaves a
+ * trace of the intent), then the status transition, approver-index sync and
+ * the optional status-field mirror. No resume/cancel of the run: it is already
+ * terminal, which is precisely why we are here.
+ */
+ private async abandonForDeadRun(raw: any, runId: string, runStatus: string): Promise {
+ const org = raw.organization_id ?? null;
+ const nodeId: string | null = raw.flow_node_id ?? raw.current_step ?? null;
+ const now = this.clock.now().toISOString();
+
+ await this.engine.insert('sys_approval_action', {
+ id: uid('aact'), request_id: raw.id, organization_id: org,
+ step_name: nodeId, step_index: 0, action: 'recall',
+ actor_id: DEAD_RUN_ACTOR_ID,
+ comment: `owning flow run ${runId} is ${runStatus} — request abandoned and record lock released`,
+ created_at: now,
+ }, { context: SYSTEM_CTX });
+
+ await this.engine.update('sys_approval_request', {
+ id: raw.id, status: 'recalled', pending_approvers: null, completed_at: now, updated_at: now,
+ }, { context: SYSTEM_CTX });
+ await this.syncApproverIndex(raw.id, [], org, now);
+
+ const config = parseJson(
+ raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
+ );
+ if (config.approvalStatusField) {
+ await this.mirrorStatusField(raw.object_name, raw.record_id, config.approvalStatusField, 'recalled');
+ }
+
+ this.logger?.warn?.('[approvals] released a record held by a dead approval run', {
+ request: raw.id, run: runId, runStatus, object: raw.object_name, record: raw.record_id,
+ });
+ }
+
/** Execute the configured escalation action for one overdue request. */
private async escalateRequest(raw: any, esc: any): Promise {
const action: string = esc.action ?? 'notify';
diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts
index 2ca815626e..f83b08bcdd 100644
--- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts
+++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts
@@ -161,12 +161,29 @@ export class ApprovalsServicePlugin implements Plugin {
if (!jobs || typeof jobs.schedule !== 'function' || !this.service) return;
const svc = this.service;
const intervalMs = this.options.escalationScanIntervalMs ?? ESCALATION_SCAN_INTERVAL_MS;
- await jobs.schedule(ESCALATION_JOB_NAME, { type: 'interval', intervalMs }, async () => {
- await svc.runEscalations();
- });
+ // Both sweeps ride this one clock: they walk the same `pending` set, and
+ // the dead-run release (#3456) is reconciliation with the same "catch up
+ // after a restart" requirement — a run killed BY the restart is exactly
+ // the shape no in-band handler can clean up.
+ // Genuinely independent — an escalation failure must not strand locked
+ // records, and vice versa, so neither can short-circuit the other.
+ const sweep = async () => {
+ const results = await Promise.allSettled([
+ svc.runEscalations(),
+ svc.releaseDeadRunRequests(),
+ ]);
+ for (const r of results) {
+ if (r.status === 'rejected') {
+ ctx.logger.warn?.('[approvals] periodic sweep leg failed', {
+ error: (r.reason as any)?.message ?? String(r.reason),
+ });
+ }
+ }
+ };
+ await jobs.schedule(ESCALATION_JOB_NAME, { type: 'interval', intervalMs }, sweep);
this.escalationJobScheduled = true;
- void svc.runEscalations().catch((err: any) => {
- ctx.logger.warn?.('[approvals] boot escalation sweep failed', { error: err?.message });
+ void sweep().catch((err: any) => {
+ ctx.logger.warn?.('[approvals] boot sweep failed', { error: err?.message });
});
ctx.logger.info('ApprovalsServicePlugin: SLA escalation scan scheduled', { intervalMs });
} catch { /* job service not installed */ }
diff --git a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
index caa038916f..2cc86796fd 100644
--- a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
+++ b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
@@ -93,6 +93,22 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg
const pending = await pendingRequestFor(engine, object, id);
if (!pending) return;
+ // The run that OPENED this approval may still write its own target record
+ // (#3456). Without this the lock cannot tell "the run that owns this pending
+ // request" from "an unrelated user edit", so a flow that touches the record
+ // between opening the approval and the decision — or a manual `resume` with
+ // no decision — dies on its own `RECORD_LOCKED` and leaves the record locked
+ // behind it.
+ //
+ // Keyed on run identity, NOT on elevation: a `runAs:'user'` run must stay
+ // RLS-scoped, so widening it to `isSystem` would be the wrong tool. The
+ // automation engine stamps `flowRunId` into the server-built ExecutionContext
+ // (never client-supplied, like `isSystem`) and it grants nothing by itself —
+ // the only write it permits is to the one record this very run already holds
+ // a pending request against.
+ const writerRun = (ctx?.session as any)?.flowRunId;
+ if (writerRun && pending.flow_run_id && String(writerRun) === String(pending.flow_run_id)) return;
+
const config = parseJson(pending.node_config_json, {});
if (config?.lockRecord === false) return;
diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts
index b4857fd92f..a83506fe27 100644
--- a/packages/services/service-automation/src/engine.ts
+++ b/packages/services/service-automation/src/engine.ts
@@ -1543,7 +1543,16 @@ export class AutomationEngine implements IAutomationService {
}
async getRun(runId: string): Promise {
- const inMem = this.executionLogs.find(l => l.id === runId);
+ // LAST entry wins, not the first: a run that pauses and later finishes
+ // records TWO entries under the same run id ('paused', then
+ // 'completed'/'failed'/'cancelled'). Scanning forwards returned the stale
+ // 'paused' one, so every suspend-then-finish run — i.e. every approval,
+ // screen and wait flow — reported itself as still paused forever, both on
+ // the Runs surface and to the approvals dead-run sweep (#3456).
+ let inMem: ExecutionLogEntry | undefined;
+ for (let i = this.executionLogs.length - 1; i >= 0; i--) {
+ if (this.executionLogs[i].id === runId) { inMem = this.executionLogs[i]; break; }
+ }
if (inMem) return inMem;
// Durable fallback: after a restart (or ring-buffer eviction) the run's
// terminal history row still answers "what happened, at which node?".
@@ -1578,8 +1587,17 @@ export class AutomationEngine implements IAutomationService {
* `runAs:'system'` to make scheduled elevation explicit (the build-time lint
* `flow-schedule-runas-unscoped` flags the same shape earlier).
*/
- private async resolveRunContext(flow: FlowParsed, context?: AutomationContext): Promise {
- const runContext: AutomationContext = { ...(context ?? {}), runAs: flow.runAs ?? 'user' };
+ private async resolveRunContext(flow: FlowParsed, context?: AutomationContext, runId?: string): Promise {
+ // `flowRunId` is stamped alongside `runAs` because it shares that field's
+ // lifetime and its single construction point: set once here, copied into
+ // every data node's ObjectQL context by `resolveRunDataContext`, and
+ // persisted with a suspended run so it survives pause/resume — including a
+ // cold resume after a restart (#3456). Provenance, not authorization.
+ const runContext: AutomationContext = {
+ ...(context ?? {}),
+ runAs: flow.runAs ?? 'user',
+ ...(runId ? { flowRunId: runId } : {}),
+ };
// #3356 (follow-up to #1888) — a `runAs:'user'` run must enforce its data
// ops as the TRIGGERING user's real authorization. Most trigger surfaces
@@ -1767,7 +1785,9 @@ export class AutomationEngine implements IAutomationService {
// elevation is scoped to this run and the caller's identity is restored
// when execute() returns). Surfaces the user-less fail-open (see helper)
// and resolves the triggering user's real grants for `runAs:'user'` (#3356).
- const runContext = await this.resolveRunContext(flow, context);
+ // Also stamps `flowRunId` so this run's data writes are attributable to it
+ // (#3456).
+ const runContext = await this.resolveRunContext(flow, context, runId);
try {
// Find the start node
@@ -3116,7 +3136,8 @@ export class AutomationEngine implements IAutomationService {
// ADR-0049 / #1888 — establish the run's effective execution identity
// from flow.runAs (see execute() / resolveRunContext); threaded below.
- const runContext = await this.resolveRunContext(flow, context);
+ // `flowRunId` is stamped here too (#3456).
+ const runContext = await this.resolveRunContext(flow, context, runId);
try {
const startNode = flow.nodes.find(n => n.type === 'start');
diff --git a/packages/services/service-automation/src/run-history.test.ts b/packages/services/service-automation/src/run-history.test.ts
index 43273be924..21a9b4779f 100644
--- a/packages/services/service-automation/src/run-history.test.ts
+++ b/packages/services/service-automation/src/run-history.test.ts
@@ -120,6 +120,72 @@ describe('automation run history (durable observability)', () => {
expect(await engine.getRun('run_nope')).toBeNull();
});
+ // A run that pauses and later finishes records TWO log entries under the
+ // same run id ('paused', then the terminal one). getRun scanned forwards and
+ // returned the stale 'paused' entry, so every suspend-then-finish run — i.e.
+ // every approval / screen / wait flow — reported itself as still paused
+ // forever, on the Runs surface and to the #3456 dead-run sweep alike.
+ describe('getRun after a pause (latest entry wins)', () => {
+ function pausingFlow(name: string, tail: string) {
+ return {
+ name, label: name, type: 'autolaunched',
+ nodes: [
+ { id: 'start', type: 'start', label: 's' },
+ { id: 'hold', type: 'hold', label: 'h' },
+ { id: 'tail', type: tail, label: 't' },
+ { id: 'end', type: 'end', label: 'e' },
+ ],
+ edges: [
+ { id: 'e1', source: 'start', target: 'hold' },
+ { id: 'e2', source: 'hold', target: 'tail' },
+ { id: 'e3', source: 'tail', target: 'end' },
+ ],
+ };
+ }
+
+ const holdExecutor = {
+ type: 'hold',
+ async execute() { return { success: true, suspend: true, correlation: 'held' }; },
+ } as never;
+
+ it('reports the terminal status once a paused run completes', async () => {
+ const engine = new AutomationEngine(silent, new InMemorySuspendedRunStore());
+ engine.registerNodeExecutor(holdExecutor);
+ engine.registerNodeExecutor({ type: 'noop', async execute() { return { success: true }; } } as never);
+ engine.registerFlow('held_ok', pausingFlow('held_ok', 'noop') as never);
+
+ const paused = await engine.execute('held_ok', { event: 'test' } as AutomationContext);
+ expect(paused.status).toBe('paused');
+ const runId = paused.runId!;
+ // Accurate while it really is paused — this is the signal the sweep
+ // reads as "alive", so it must not regress either.
+ expect((await engine.getRun(runId))!.status).toBe('paused');
+
+ expect((await engine.resume(runId)).success).toBe(true);
+ await flush();
+ expect((await engine.getRun(runId))!.status).toBe('completed');
+ });
+
+ it('reports failed once a paused run dies after resuming', async () => {
+ const engine = new AutomationEngine(silent, new InMemorySuspendedRunStore());
+ engine.registerNodeExecutor(holdExecutor);
+ engine.registerNodeExecutor({
+ type: 'boom', async execute() { throw new Error('kaboom'); },
+ } as never);
+ engine.registerFlow('held_bad', pausingFlow('held_bad', 'boom') as never);
+
+ const paused = await engine.execute('held_bad', { event: 'test' } as AutomationContext);
+ const runId = paused.runId!;
+ expect(paused.status).toBe('paused');
+
+ expect((await engine.resume(runId)).success).toBe(false);
+ await flush();
+ // The exact shape #3456's sweep needs: a dead approval run must be
+ // recognisable as dead, not as forever-paused.
+ expect((await engine.getRun(runId))!.status).toBe('failed');
+ });
+ });
+
it('caps terminal history per flow (retention stop-gap, #2585)', async () => {
const store = new InMemorySuspendedRunStore({ maxTerminalRunsPerFlow: 2 });
const engine = new AutomationEngine(silent, store);
diff --git a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts
index 6bc6556c96..ba50477788 100644
--- a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts
+++ b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts
@@ -124,3 +124,46 @@ describe("AutomationServicePlugin bridges the runAs:'user' grant resolver (#3356
await kernel.shutdown();
});
});
+
+// The provenance band #3456 rides on: a run's data writes must be attributable
+// to the run that made them, all the way down to the ObjectQL context — without
+// it the approvals record lock cannot tell the owning run from a stranger.
+describe('a run stamps its own id onto its data ops (#3456)', () => {
+ it.each([
+ ["runAs:'user'", 'user'],
+ ["runAs:'system'", 'system'],
+ ] as const)('%s carries flowRunId matching the run id', async (_label, runAs) => {
+ const { engine: ql, crud } = fakeObjectQl(AUTHZ_TABLES);
+ const kernel = await bootWithObjectQl(ql);
+ const automation = kernel.getService('automation');
+ automation.registerFlow(`stamp_${runAs}`, updateFlow(`stamp_${runAs}`, runAs) as never);
+
+ const res = await automation.execute(`stamp_${runAs}`, { userId: 'u1', params: { noteId: 'n1' } });
+ expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true);
+
+ // `result.runId` is only surfaced for a PAUSED run, so cross-check against
+ // the engine's own run log — the id a later `getRun` would be asked about,
+ // which is exactly the join the dead-run sweep depends on.
+ const [logged] = await automation.listRuns(`stamp_${runAs}`, { limit: 1 });
+ const update = crud.find((c) => c.op === 'update' && c.obj === 'runas_thing');
+ expect(update!.ctx.flowRunId, 'the data op carried no run provenance').toBeTruthy();
+ expect(update!.ctx.flowRunId).toBe(logged.id);
+
+ await kernel.shutdown();
+ });
+
+ it('does not leak the stamp back onto the caller-supplied context', async () => {
+ const { engine: ql } = fakeObjectQl(AUTHZ_TABLES);
+ const kernel = await bootWithObjectQl(ql);
+ const automation = kernel.getService('automation');
+ automation.registerFlow('nomutate', updateFlow('nomutate', 'user') as never);
+
+ // resolveRunContext copies rather than mutates, so a caller reusing one
+ // context object across runs can never inherit a stale run id.
+ const caller: any = { userId: 'u1', params: { noteId: 'n1' } };
+ await automation.execute('nomutate', caller);
+ expect(caller.flowRunId).toBeUndefined();
+
+ await kernel.shutdown();
+ });
+});
diff --git a/packages/services/service-automation/src/runtime-identity.ts b/packages/services/service-automation/src/runtime-identity.ts
index 85bfec6c2c..33bde71881 100644
--- a/packages/services/service-automation/src/runtime-identity.ts
+++ b/packages/services/service-automation/src/runtime-identity.ts
@@ -20,6 +20,13 @@ export interface RunDataContext {
permissions: string[];
/** Acting user's tenant/org id. */
tenantId?: string;
+ /**
+ * The run performing this operation (#3456). Provenance only — it is not part
+ * of the identity the security middleware evaluates, so it neither widens nor
+ * narrows what the run may touch. Hooks use it to recognize a run's writes to
+ * state that run itself opened (the approvals record lock).
+ */
+ flowRunId?: string;
}
/**
@@ -44,9 +51,17 @@ export interface RunDataContext {
* by every data-touching node so the policy can't drift between node types.
*/
export function resolveRunDataContext(context: AutomationContext | undefined): RunDataContext | undefined {
+ const flowRunId = context?.flowRunId;
if (context?.runAs === 'system') {
- return { isSystem: true, positions: [], permissions: [] };
+ return { isSystem: true, positions: [], permissions: [], ...(flowRunId ? { flowRunId } : {}) };
}
+ // NOTE (#3456): the identity-less case below returns `undefined`, so a
+ // schedule-triggered `runAs:'user'` run with no user carries NO context at all
+ // — and therefore no `flowRunId` either, leaving it subject to the approvals
+ // record lock on its own target record. Manufacturing a context here just to
+ // carry the run id would flip that run from the documented unscoped fail-open
+ // (#1888) to baseline-member RLS — a separate, larger behavior change. The
+ // dead-run sweep in plugin-approvals is what recovers this shape.
if (!context?.userId) return undefined;
// `context` is now narrowed to a defined AutomationContext with a userId.
const out: RunDataContext = {
@@ -56,6 +71,7 @@ export function resolveRunDataContext(context: AutomationContext | undefined): R
permissions: Array.isArray(context.permissions) ? context.permissions : [],
};
if (context.tenantId) out.tenantId = context.tenantId;
+ if (flowRunId) out.flowRunId = flowRunId;
return out;
}
diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts
index 6152c5b13a..9a93193ce0 100644
--- a/packages/spec/src/contracts/automation-service.ts
+++ b/packages/spec/src/contracts/automation-service.ts
@@ -64,6 +64,22 @@ export interface AutomationContext {
* Callers do NOT set this — the engine derives it from the flow definition.
*/
runAs?: 'system' | 'user';
+ /**
+ * Id of the run this context belongs to, stamped by the engine at run setup
+ * alongside {@link runAs} and carried into every data node's ObjectQL
+ * `context` (see `resolveRunDataContext` in @objectstack/service-automation).
+ * It is persisted with a suspended run, so it survives a pause/resume round
+ * trip — including a cold resume after a process restart.
+ *
+ * Provenance, not authorization: it grants nothing and no security
+ * middleware keys on it. A hook uses it to recognize writes made BY a given
+ * run — the approvals record lock (#3456) lets the run that opened a pending
+ * approval write its own target record, which it otherwise cannot tell apart
+ * from an unrelated user's edit.
+ *
+ * Callers do NOT set this — the engine derives it, exactly like {@link runAs}.
+ */
+ flowRunId?: string;
/** Additional contextual data */
params?: Record;
}
diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts
index 8c94d9f033..9dcf322745 100644
--- a/packages/spec/src/data/hook.zod.ts
+++ b/packages/spec/src/data/hook.zod.ts
@@ -228,6 +228,7 @@ export const HookContextSchema = lazySchema(() => z.object({
roles: z.array(z.string()).optional(),
accessToken: z.string().optional(),
isSystem: z.boolean().optional().describe('True when the call was made with an elevated system context (engine self-writes)'),
+ flowRunId: z.string().optional().describe('Id of the automation flow run performing this write, when it originates from a flow data node. Provenance only — grants nothing, no security middleware keys on it. Lets a hook tell the run that OWNS some externalized state (e.g. a pending approval) from an unrelated caller (#3456).'),
skipTriggers: z.boolean().optional().describe('True when record-change automation (flow triggers) must be suppressed for this write — e.g. package seed replay. Lifecycle hooks still run.'),
skipAutomations: z.boolean().optional().describe('True when metadata-bound automation hooks must be suppressed for this write — e.g. data import with "run automations" unchecked, or import undo. Implies skipTriggers; code-registered system hooks (audit, security) still run.'),
}).optional().describe('Current session context'),
diff --git a/packages/spec/src/kernel/execution-context.zod.ts b/packages/spec/src/kernel/execution-context.zod.ts
index 655b738a89..580bfd52f5 100644
--- a/packages/spec/src/kernel/execution-context.zod.ts
+++ b/packages/spec/src/kernel/execution-context.zod.ts
@@ -192,6 +192,22 @@ export const ExecutionContextSchema = lazySchema(() => z.object({
/** Whether this is a system-level operation (bypasses permission checks) */
isSystem: z.boolean().default(false),
+ /**
+ * Id of the automation flow RUN performing this operation, when the write
+ * originates from a flow's data node. Server-constructed by the automation
+ * engine at run setup and threaded through `resolveRunDataContext` — never
+ * client-supplied, exactly like {@link isSystem} (this envelope is built from
+ * the authenticated session, not from request input).
+ *
+ * Provenance, NOT authorization: it grants nothing on its own and no security
+ * middleware keys on it. It exists so a hook can tell "the run that owns this
+ * externalized state" from "an unrelated caller" — the approvals record lock
+ * (#3456) uses it to let the run that opened a pending approval still write
+ * its own target record, which `isSystem` cannot express for a
+ * `runAs:'user'` run without elevating it.
+ */
+ flowRunId: z.string().optional(),
+
/**
* Suppress record-change AUTOMATION (autolaunched flow triggers:
* record-after-insert / -update / -delete) for writes made under this