Skip to content

Approval: a dead approval run can leave the record RECORD_LOCKED (#3424 follow-up, expected-behavior 3) #3456

Description

@os-zhuang

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)

  1. 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.
  2. 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.
  3. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions