The 1.0.0-alpha8 delta-audit and its fixes (#3608117: Fix 1.0.0-alpha8 delta-audit findings: concurrency recovery races and webform branch-scoping) closed a series of concurrency and recovery defects, but the audits kept finding the SAME class of problem in different places. That recurrence points to structural debt in the engine's state-transition and recovery layer, not just individual bugs. This issue tracks paying that debt down deliberately. It is a refactor: the behavioral fixes themselves belong to #3608117: Fix 1.0.0-alpha8 delta-audit findings: concurrency recovery races and webform branch-scoping, and the core execution model (the token and lock discipline, the exactly-once conditional-claim on signal and advance, the capability-token security model) was repeatedly verified sound and is not in question here.

The recurring theme

State changes are made ad hoc at each call site. Some correctly use a conditional write ("UPDATE ... WHERE status = expected"), which current-reads even under MySQL REPEATABLE READ; others use "load entity, set a field, save()", which trusts a plain re-read to observe a concurrent commit, and the snapshot defeats that. Nearly every audit finding was one more site using the second pattern, or a comment asserting that a plain in-transaction SELECT "reads the committed status" (it does not). The fix each time was to convert that one site to a conditional write. That is a missing shared discipline, not a set of unrelated bugs.

Proposed work

  • Introduce a single guarded state-transition primitive for tokens and instances (a "transition from expected to new, atomically" helper) and route every status flip through it, so the correct conditional-write pattern is the only pattern. The current claimTerminalStatus is this idea applied to three sites; generalize it and remove the remaining ad-hoc load-then-save flips.
  • Codify the isolation invariant: inside an open transaction, never trust a re-read to see another transaction's commit; guard the write instead. Document it where the transaction helpers live, correct the misleading "reads the committed status directly" comments, and consider a lightweight check (a test or a review checklist) so the pattern does not regress.
  • Consolidate the recovery sweeps. reconcileStuckInstances, recoverStalledInstances, and the timeout sweep each re-derive "find running instances of a given stuck shape, excluding incident-halted ones, in a bounded ordered batch" and each independently meet the same head-of-line and exclusion questions. Unify them behind one parameterized stuck-instance finder that owns the batching, ordering, and open-incident exclusion once.

Out of scope

The behavioral bug fixes are handled in #3608117: Fix 1.0.0-alpha8 delta-audit findings: concurrency recovery races and webform branch-scoping and should land first. The Webform interaction handler's coupling to the webform save lifecycle (the binding token living in submission data, start versus resume threaded across preSave, postSave and confirmForm) is a related fragility surfaced by the same audit, but it lives in orchestra_interaction_webform and is better tracked as its own follow-up rather than folded in here.

Acceptance

Every token and instance status transition goes through the shared primitive; no load-then-save status flip remains; the recovery sweeps share one finder; the isolation invariant is documented; and the full test suite still passes.

Issue fork orchestra-3608146

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Status: Active » Needs review

  • mably committed ab7e0e3c on 1.x
    task: #3608146 Refactor the state-transition and recovery layer: one...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.