Summary
Follow-up to #3424. That issue's expected-behavior item 3 — "unlock on terminal failure" — is intentionally not covered by the admin-override PR (#3451), because it concerns the flow-engine write context vs. the record-lock hook, not approver resolution. Tracking it here so it isn't lost.
Context
The record lock is a global beforeUpdate hook keyed on a pending sys_approval_request (packages/plugins/plugin-approvals/src/lifecycle-hooks.ts): while a record has a pending request and lockRecord !== false, a non-system, non-admin edit is rejected with RECORD_LOCKED.
In the normal path this is correct — decideNode finalizes the request (status → approved/rejected) before decide() resumes the run, so any post-approval node (mark_approved, a status write, …) runs when there is no longer a pending request.
The gap is the abnormal path: if the owning run writes the target record while its own request is still pending — e.g. a manual resume without a decision, or a flow that touches the record between opening the approval and the decision — the flow's own write hits RECORD_LOCKED. The lock hook can't distinguish "the run that owns this approval" from "an unrelated user edit", so the run can die with the record left locked.
Options (decide before implementing)
- Exempt the owning run's writes. The automation engine already carries run/flow identity; thread it (or a scoped system flag) so a write from the run that owns the pending request bypasses the lock, instead of relying on the caller being
isSystem.
- Release the lock on terminal run failure. When a run that opened a pending approval reaches a failed terminal state, finalize/cancel the request (or clear the lock) so the record isn't orphaned.
- Both.
Impact
Low-frequency but real: a record can be left locked by a dead approval run. The #3424 admin override is the recovery escape hatch today (an admin can recall/reject to release the lock), so this is robustness hardening, not a data-availability dead-end.
Notes
Summary
Follow-up to #3424. That issue's expected-behavior item 3 — "unlock on terminal failure" — is intentionally not covered by the admin-override PR (#3451), because it concerns the flow-engine write context vs. the record-lock hook, not approver resolution. Tracking it here so it isn't lost.
Context
The record lock is a global
beforeUpdatehook keyed on a pendingsys_approval_request(packages/plugins/plugin-approvals/src/lifecycle-hooks.ts): while a record has a pending request andlockRecord !== false, a non-system, non-admin edit is rejected withRECORD_LOCKED.In the normal path this is correct —
decideNodefinalizes the request (status →approved/rejected) beforedecide()resumes the run, so any post-approval node (mark_approved, a status write, …) runs when there is no longer a pending request.The gap is the abnormal path: if the owning run writes the target record while its own request is still pending — e.g. a manual
resumewithout a decision, or a flow that touches the record between opening the approval and the decision — the flow's own write hitsRECORD_LOCKED. The lock hook can't distinguish "the run that owns this approval" from "an unrelated user edit", so the run can die with the record left locked.Options (decide before implementing)
isSystem.Impact
Low-frequency but real: a record can be left locked by a dead approval run. The #3424 admin override is the recovery escape hatch today (an admin can
recall/rejectto release the lock), so this is robustness hardening, not a data-availability dead-end.Notes
lifecycle-hooks.tsbindApprovalLockHook;approval-service.tsdecideNode/decide.