A focused delta-audit of the code added since 1.0.0-alpha7 (the concurrency and recovery work from #3608046: Fix 1.0.0-alpha7 audit findings: completion/timeout/migration concurrency, join-stall recovery, and re-offer notification and the Webform handler unification from #3608073: Unify the webform start and resume handlers into a single start-or-resume handler) found a set of concurrency, data-integrity and correctness defects, plus a few pre-existing Webform issues in the same area. This issue fixes all of them in one merge request.
Security fundamentals were verified sound and are not affected: capability tokens are unforgeable (HMAC), there is no cross-instance or cross-tenant IDOR, and the Cancel URL is not an open-redirect vector. The findings below are reliability and correctness, not authentication or authorization bypasses.
Concurrency and recovery (WorkflowEngine, TimeoutSweep, WorkflowVersionManager)
The common root cause is a "load entity, set a field, save()" sequence inside an open transaction that trusts a plain re-read to observe a concurrent commit, which MySQL REPEATABLE READ defeats (the snapshot is fixed at the transaction's first read, and acquiring a lock afterward does not refresh it). The fix uses the conditional raw UPDATE pattern the code already uses correctly for token claims.
- Dead-letter window: the dead-lettered token's ERROR status and its incident are committed by two separate writes, so a reconcile sweep running in between sees zero live tokens and zero open incidents and completes a failed instance as COMPLETED. Fix: commit the ERROR status and the incident (or the FAILED status) in one transaction, and treat a STATUS_ERROR token as blocking completion.
- TimeoutSweep resolves nodes from the live workflow config, not the instance's pinned definition, so a pinned instance's timeout behavior can change under it, and a due token whose node was removed or renamed is skipped without clearing its deadline, permanently occupying the head of the deadline-sorted batch and starving every later timeout. Fix: resolve from the pinned definition, and clear (or log-and-clear) an unresolvable token's deadline so it cannot block the batch.
- Instance migration re-reads token status with a plain SELECT inside its transaction and can therefore write a stale row back over a token a concurrent signal() consumed, resurrecting it beside its successors. Fix: rewrite node_id and definition_version with conditional UPDATEs guarded on the expected status.
- armJoinTimeouts, when reached inside an advancing transaction (a self-cancelling cohort branch), re-reads a waiting token under the join lock but on the transaction's stale snapshot, and can write a deadline back onto a token a concurrent join fire already consumed, later double-firing the join. Fix: arm the deadline with a conditional UPDATE guarded on status and a null deadline.
- checkCompletion flips the instance to COMPLETED with an unconditional save, so a reconcile sweep can overwrite a concurrent cancel or fail. Fix: flip with a conditional UPDATE (WHERE status running) and run the completion side-effects only when it changed a row.
- skipIncident and failFromIncident resolve the incident before the compensating transition commits, reopening the same reconcile window. Fix: resolve the incident inside the transaction that re-activates the token or flips the status.
- Stall recovery (recoverStalledInstance) is only reachable from cancel(); the documented cron caller does not exist, so a join that loses its deadline can hang forever. Fix: add the sweep leg that finds a running instance whose live tokens are all waiting with no deadline and recovers it.
- The reconcile batch has no ordering and does not exclude incident-halted instances, which match it on every run; once enough accumulate they starve genuinely stuck instances. Fix: exclude open-incident instances in SQL and order the scan.
- A join fire can leave a surplus waiting sibling carrying the armed deadline, which the sweep later fires as a bogus second cohort. Fix: clear the surplus sibling's deadline on fire.
- Lifecycle and audit events are dispatched inside open transactions on the recovery paths, so a rolled-back recovery can still have emitted an external notification. Fix: dispatch after commit where feasible, or document the transactional-listener contract.
Webform interaction (handler, interaction plugin, element)
- On resume the handler resolves the parked token by node (lowest id) rather than the exact token the branch-scoped handle named, so with two tokens parked on one node (parallel or converging branches) one party's submission resumes and stores data on another branch. This matches the pre-alpha8 behavior; it is fixed here. Fix: resume the exact named token when it is still parked, falling back to node resolution only for a genuine loop re-entry.
- A collect step reopens the instance's latest submission of the webform, not the current branch's, so parallel branches collecting on one webform can hand one party another's submission. Fix: scope the reopen to the branch.
- The binding element stores the live capability token in submission data (a bearer value at rest, reachable through exports while the target step is still parked). Fix: redact the stored value once the submission is bound.
- Minor: a workflow-starting submission that starts inside preSave can orphan a RUNNING instance if the submission save then fails; an expired binding token on a start-capable form is silently swallowed; re-editing the starting submission re-seeds instance variables. Each is tightened.
Tests
Each fix adds or extends coverage for the specific interleaving or topology it addresses, and the full WorkflowEngine, orchestra_interaction_webform and orchestra_webform_example suites pass.
Issue fork orchestra-3608117
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
Comment #3
mably commentedComment #4
mably commentedMR !270 opened against 1.x. Orchestra kernel suite passes locally (core engine: reconcile, join-stall recovery, dead-letter, join, timeout, migration, incident; plus the interaction_webform handler, 10/10). Functional and FunctionalJavascript run in CI (they need the container's webserver and ChromeDriver).
Comment #6
mably commented