Follow-up from #3608540: Webform resume fallback can resume a sibling branch parked on the same node. That issue fixed finding L1 (the webform resume fallback could resume a sibling branch on a shared step) by making OrchestraInteractionHandler::resumeTarget() branch-aware. The deeper observation: resumeTarget is not webform-specific. The base orchestra_interaction module already resolves the same thing, more robustly, and the webform handler duplicates it.
The duplication
Two implementations resolve "a (possibly consumed) branch-scoped handle to the branch's live step":
- Base: CapabilityResolver::consumedBranchOutcome() uses InteractionResolver::branchContinuation(), which resolves from the consumed token's own descendants so a sibling branch is never included. It has been sibling-safe by design.
- Webform: OrchestraInteractionHandler::resumeTarget() reimplements it as parkedTokenById(), then currentChain() plus parkedTokenInChain(), then a parkedTokenAtNode() node fallback. That node fallback was the L1 defect (it resumed the lowest-id token on the node, which could be a sibling), now guarded in #3608540: Webform resume fallback can resume a sibling branch parked on the same node.
So the webform side is the weaker copy of a mechanism the base module already owns, and L1 was a defect present only in the copy.
Proposal
Resolve the webform resume target through the base InteractionResolver (its branchContinuation(), or a small shared method built on it) and retire resumeTarget's duplicated exact/chain/node logic. Both channels then share one implementation, so branch-scoped resume correctness is structural rather than maintained per channel.
The two callers want different results: CapabilityResolver builds a dispatch outcome for display, while the webform handler needs a token to resume. So the shared piece is the token selection (branchContinuation), which the webform handler would consume to pick the token, not the whole outcome method.
To verify
- Whether branchContinuation() already covers the plain single-branch loop re-entry (a non-interactive step, where the re-parked token is the consumed token's own descendant). If so, the node fallback (parkedTokenAtNode) can be dropped entirely; if not, keep a guarded fallback.
- That the existing webform resume tests (loop re-entry, branch-scoped resume, bound re-edit after expiry, and the L1 sibling test from #3608540: Webform resume fallback can resume a sibling branch parked on the same node) still pass unchanged.
No behavior change intended: this consolidates onto one resolver, keeping the #3608540: Webform resume fallback can resume a sibling branch parked on the same node correctness while removing the duplicate mechanism.
Issue fork orchestra-3608543
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 commentedOpened MR !292 against 1.x.
The webform handler's resumeTarget() is moved into the base resolver as InteractionResolver::liveBranchToken(), and the handler now calls it, so the branch resume-target resolution lives in one layer beside the capability doorway's branchContinuation() rather than being reimplemented per channel. The moved logic is identical to what was shipped in #3608540: Webform resume fallback can resume a sibling branch parked on the same node, so behavior is unchanged and the existing webform resume tests still pass; a base-level InteractionResolverTest case now covers liveBranchToken() (exact, consumed, sibling-safe, unknown) where the logic lives. The handler drops about 50 lines. Local run: InteractionResolverTest (5) and OrchestraInteractionHandlerTest (15) green, phpcs and cspell clean. Setting to Needs review pending CI.
Comment #5
mably commented