Interaction, content, ECA and the modelers, and deletion that refuses to cascade

Fixes the 19 findings of the audit, one commit each: 19 fixed, 0 withdrawn, 0 deferred. Two of the audit's prescribed fixes were replaced by a different one, and the commit bodies say why.

It also carries a second, larger piece of work, folded into this branch rather than opened as its own MR: three configuration entities stopped cascading their deletion into data, and gained a way to be retired instead. That is the first two sections below.

Four further commits come from auditing this branch itself. Three are de-duplications the fixes above introduced, each landing where the extracted version reads better at every call site; the fourth makes one test fail rather than error when the fix is reverted. The rounds also corrected a dozen things inside the nineteen, which their own commit bodies record. What the rounds found is listed at the end.

Deleting no longer destroys what it did not create

Folded in at the maintainer's request rather than opened as its own issue.

Three configuration entities cascaded into data when deleted:

• A tenant cancelled whatever was still running in the realm, then deleted every process instance scoped to it - with its tokens, variables, incidents, subprocess children and work items - and every workflow scoped to it.
• A status is a declared config dependency of the workflows whose nodes reference it, so deleting a term edited those workflows: core stripped the reference out of their nodes and saved them.
• A workflow cancelled its running instances and then deleted every instance of it whatever its state, with the same cascade underneath.

A run is the record of work that happened, and a completed one is the answer somebody was given; a workflow is a process someone authored. None of that is collateral of removing the thing it hangs off, and an authoring mistake on a list page took it irrecoverably and without saying so. All three refuse instead.

The refusal lives in each entity's own preDelete(), ahead of parent::preDelete(). That is the only place it can live: ConfigEntityBase deletes a config entity's dependents there, before any hook_ENTITY_TYPE_predelete could look at them, so a guard in a hook is never reached. It therefore holds for a programmatic delete() as much as for a form, and it steps aside during a config import or a module uninstall exactly where the parent skips its own dependency handling, because those flows manage their deletions themselves. The default tenant is the one exception: it is refused even to an import, whatever the sync store says, because a site whose engine cannot resolve a tenant is broken either way.

The Delete operation and its confirmation page stay reachable on all three, because being told what to clear is the point - a withheld operation sends an administrator looking for a reason only that page can give. Each page lists what is in the way per kind and with counts, disables the confirm button, re-checks on submit in case the page was open while the last row went, and says what to do instead. The counts come from one reader per subject (TenantUsage, StatusUsage, WorkflowUsage), the same reader the guard refuses on, so a page cannot promise a deletion the guard would then reject; all three render them through one EntityCountLabelsTrait.

Two purge hooks are deleted along with the behaviour they performed. What still deletes in bulk is unchanged and deliberate: retention purging eligible instances, and an instance's own rows following it.

A workflow's referrers are the two the dependency graph cannot answer for: its runs, which name their definition by machine name, and a subprocess parent, which names its child in the calling node's task configuration.

Retiring, so that refusing to delete is not a dead end

A thing that cannot be deleted needs a way to be taken out of use. Both types gain core's own enabled flag, declared in entity_keys:

• A workflow: the engine refuses start() on a disabled one, the runs already in it carry on to the end, and a subprocess node calling a disabled child dead-letters the parent exactly as it does for a child that is not there at all, so an operator gets an incident naming the node. Declaring the key is what turns the flag into a surface: ModelOwnerBase::supportsStatus() is final and answers hasKey('status'), so the Workflows list's own column, its Enable and Disable operations, their routes and the BPMN.io model panel's checkbox all exist because of that line. The Complete Modeler's Model section carries one too.
• A status: a disabled term stops being offered when authoring, while the nodes and runs that already name it keep resolving it, so history reads as it always did and a requester can still filter their own by it. Its form carries the checkbox, and its list a column, so a retired term reads as retired without opening it.

Every surface that newly picks one of these skips the disabled ones while keeping one already stored, marked: the status node editor, the subprocess child select and the webform handler's start-workflow select. That is not decoration - a select whose default value is not among its options is refused as an illegal choice, so leaving it out would block every save of the workflow, or silently rewrite what the node or the form is pointed at. Start is no longer offered on a workflow that starts nothing.

What was wrong, and what each fix does

F022 (HIGH) — an eca_event node took an outer scope's answer as its own. execute() snapshotted the async marker and the node's outcome token before dispatch and restored them after, but never cleared them, then read both back out of ECA's request-scoped bag as this node's answer. A model that answers its own task later, or holds a value under this node's outcome-token name, and then starts an Orchestra workflow synchronously, had that value taken as an answer no model gave the node: the token parked with nobody to resume it, or routed on an outer scope's outcome. Both silent. Now cleared after the snapshot and before the dispatch.

F077 — the content task screen relabelled a delete confirmation as "Save". A delete form passes the form alter's ContentEntityFormInterface guard, so a step whose form operation was delete gave the reviewer a screen whose primary button read Save; pressing it destroyed the content, announced "Saved.", and landed on the 404 with the work item still open. Refused at both ends: the interaction's settings form refuses the delete operation, and the page refuses any operation whose declared form class implements ConfirmFormInterface. Because nothing the page builds can destroy, the access gate is now update access alone.

F019 — the plugin-swap AJAX contract was spelled out at four sites. InnerPluginSettingsTrait gains pluginSwapProperties() and all four call it.

F100 — retyping a node stranded the previous type's node-feature data. Preserved extras are now intersected with the config keys of the features the node's new task type has, in the owner, so both modelers are covered.

F101 — a validation error inside a collapsed section left it collapsed. openRows() mapped the error name's third segment to a feature key only, so an error in the task settings, a routing plugin's settings or a flow condition's settings reopened the row and left the section closed. The segment is now matched to the section it names, and the flows branch gains the condition case.

F102 — a workflow's tenant could never be set from either modeler. The Complete Modeler's Model section gains a Tenant scope select and submitForm() writes it. modelConfigFormAlter() is deliberately still a no-op: BPMN.io's panel script binds change listeners only to its own fixed field list and its metadata writer falls through on any other name, so an owner-added field is rendered and discarded. That is now written down in docs/modeling.md, and docs/multi-tenancy.md says where the scope is set.

F106 — the link variant's request_changes notification had no Orchestra Mail wording. The override fell through the channel's neutral default, so the poster got the bare node label as subject and one generic sentence as body. Added, mirroring the Easy Email plain body, including re-appending the capability link that hook_mail_alter would otherwise drop.

F107 — the interaction operation honoured no return target. operation() did not store the incoming orchestra_return, so a webform or off-site payment hand-off lost it; signal() redirected to the pending-actions page unconditionally; and the guarded-outcome refusal dropped the target. All three now match the task twin and orchestra_ui's own OperationController.

F070 — two visitor-page token queries loaded an unbounded token set. findBranchContinuationTokens() and findTerminalInteraction() now resolve the qualifying node ids from the effective definition first and add the same node_id IN narrowing the sibling findChainMessages() already used, returning early when the list is empty.

F071 — the dispatcher resolved the whole capability outcome twice per page view. Once for the body and once for the route's title callback, each built as its own controller instance, so nothing on $this could share the answer. The page carries its own #title now, which core prefers to a title callback ($main_content['#title'] ?? $this->titleResolver->getTitle(...)), so the body is the only thing that resolves and there is no hand-off to manage. The callback's defensive fallback for an invalid token goes with it: assertGrant() refuses those before there is any page to name.

F072 — the interaction "Open" link lacked the translation context its sibling uses. A French operator saw the preview action labelled with the adjective next to the operator action labelled with the verb. Context added, catalog entry replaced with the msgctxt form.

F109 — park_notify was accepted on a node with no external interaction, then sent nothing. The editor now refuses a chosen audience when the node's interaction feature names no plugin, and the message names the setting to fix.

F073 — the message interaction's comments said the output boundary is admin HTML. respond() contradicts that whenever the format exists. Reworded, in all three places that said it.

F074 — two tautological assertNotSame() calls. Dropped; the class docblock now says where replay is actually proved, and the one check that was genuinely missing was added.

F075 — one instance load per attachment. Both directions of the lookup now issue one loadMultiple(); AttachmentInterface::getInstanceId() reads the raw reference so nothing is attached to the field item.

F076 — EntityTaskController's docblock described removed behaviour. Corrected.

F103 — composite sub-condition rows rendered an unnamed fieldset and identical Remove buttons. Each row is now titled by its position and each Remove button carries that in its accessible name.

F104 — label width and height were captured and stored but never restored. Dropped from both maps, their schema mappings, the two array shapes and the three shipped examples that carried them.

F105 — a class docblock pointed at a method that does not exist. Named correctly, as a real @see.

Where the audit's prescribed fix was replaced

• F071: the audit prescribed a memo on CapabilityResolver, and it would have worked. A service instance lives as long as its container, which is one request, so within a page view it would have served the title the same resolution the body used - and the pre-advance outcome is the right one to title a page the body built from it. This branch first replaced it with a one-way memo on the request, which had the same lifetime and was a back channel besides: a domain object in Symfony's parameter bag, plus two helpers and a constant to manage a hand-off. Neither is needed. A duplicate computation over mutable state is better removed than cached, and #title removes it: one resolution, no key, no lifetime to reason about, and 93 lines fewer.
• F077: the audit offered relabelling or refusing. Refusing is what leaves no screen that says Save and deletes, and it also closes the config-import path.

Tests

Every finding with a reachable consequence got a test that fails against the unfixed code, and CI's test-only changes lane proves it independently: run with every production change reverted, it reports 56 clean assertion failures across 25 classes and no errors. The commit bodies carry the exact red text; the ones with teeth worth naming here:

Test	Red without the fix
OrchestraEcaTest::testAnOuterAsyncMarkerDoesNotParkAnUnansweredNode	Array &0 [0 => 'n_emit'] against an expected empty parked-node list
OrchestraEcaTest::testAnOuterOutcomeIsNotRecordedByAnUnansweredNode	0 => 'n_bled' against an expected 'n_clean'
EntityInteractionTest::testTheSettingsFormRefusesTheDeleteOperation	Failed asserting that an array has the key 'form_operation'.
EntityInteractionTest::testTheTaskScreenRefusesConfirmingFormOperations	Failed asserting that exception of type "…NotFoundHttpException" is thrown.
NodeExtrasRoundTripTest::testRetypingShedsThePreviousTypesFeatureData	Failed asserting that an array does not have the key 'assignments'.
OrchestraCmFormTest::testAnErrorInTheTaskSettingsOpensThatSection (+ routing, + condition)	The task settings section holding the refused field is open. Failed asserting that false is true.
OrchestraCmFormTest::testTheModelSectionScopesTheWorkflowToOneTenant	The Model section offers a tenant scope. Failed asserting that an array has the key 'tenant'.
ExampleNotificationWordingTest::testTheChangesRequestedEmailIsWorded	-'Changes requested on your submission' +'Modify submission'
InteractionOperationTest (3 tests)	Failed asserting that 'http://localhost/orchestra/my-actions' ends with "/orchestra/instances"., the missing orchestra_return= on the guarded redirect, and Failed asserting that null is identical to '/orchestra/instances'.
ChainTest::testBranchContinuationDoesNotLoadTheWholeChain	Failed asserting that 8 is equal to 5 or is less than 5.
ChainTest::testTheTerminalScreenDoesNotLoadEveryConsumedToken	Failed asserting that 12 is equal to 5 or is less than 5.
InteractionAccessTest::testOnePageViewResolvesTheOutcomeOnce	The dispatcher route declares no title callback, so nothing resolves the outcome twice. Failed asserting that '...InteractionController::title' is null.
InteractionAccessTest::testThePageBodyAlwaysResolvesAfresh	red against a memo of any shape, on the request or on the resolver. A canary rather than a reproduction: it drives two page views through one container, which a test arranges and production does not
InteractionOperationsTest::testInteractiveParkedTokenGetsOpenLink	-'Operation' +''
ParkNotificationDispatchTest::testTheEditorRefusesAnAudienceWithNoInteraction	Failed asserting that an array has the key 'nodes][n_wait][park_notify'.
AttachmentTest (2 tests)	Failed asserting that 5 is equal to 2 or is less than 2. on each
ConditionTreeBuilderTest::testEverySubConditionRowAndItsRemoveButtonAreNamed	-'Sub-condition 1' +''
LayoutCaptureTest (2 tests)	the stored +'width' => 48.0 +'height' => 14.0, and +2 => 'width' +3 => 'height'

The page title has one assertion of a different kind. Seven tests read ['#title'] off the array the dispatcher returns, and all seven would pass if core ignored #title entirely, so a functional assertion goes through the real render pipeline where a test already lands on the dispatcher over HTTP. It does not fail against the unfixed code and is not meant to: the visitor sees the same title either way, which is the point of the refactor. It fails if the mechanism stops working. Kernel is the wrong level for it - renderResponse() there returns the main content alone, with no html template and so no title element - and it cannot run locally for this module, since the only docroot a web server serves here is a shared checkout this work must not touch.

Four findings have no red-to-green proof, and each commit body says so rather than implying one:

• F019 is a de-duplication with no defect to reproduce, and a test written against the base cannot call a helper the base does not have. It gets the contract test the finding says is missing — NodeFeaturePluginSwapTest asserts all six properties on every swapping select — proven to have teeth by dropping #executes_submit_callback from the helper.
• F073, F076 and F105 are comment and docblock corrections. F073 gets coverage of the branch that had none (testRespondFiltersAsAdminHtmlWithNoFormat), so both statements the comments make are checkable.
• F074 is a test-quality finding: the proof runs the other way. Removing the instance from the token's signature payload leaves both dropped assertions passing while only testTamperedTokenIsRejected() catches it.

Performance

Strictly fewer queries. Measured on the fixtures: the branch-continuation fetch drops from 8 placeholders to the entity query's own conditions, the terminal fetch from 12 to the same, the dispatcher page view from five token queries to three, and both attachment lookups from five queries to one. Nothing was added to a hot path: the two new form-time reads are of values already in the form state, the tenant select is one config-entity load per build of an authoring form, and the park_notify guard runs only in the editor.

What auditing this branch found

Five rounds, each over the whole diff. Rounds 1 to 3 found 15 defects between them and round 4 found none. Round 5 came from reading the test-only changes lane's output per class rather than in aggregate, which showed one test erroring where it should have failed: with the tenant select reverted away entirely it read ['#options'] off a missing element, and an error in that lane proves nothing. Its sibling already guarded against that; proving it red locally against only the options-building lines is what hid it. Both tenant tests now report the same clean failure.

Rounds over the folded-in work

Five more rounds over the whole branch after the deletion work landed; the fifth found nothing. What the first four found, each fixed with a test that fails without it unless noted:

• The status node editor asked the vocabulary twice - once for the enabled terms and again for the whole visible set - and called a term it could not find missing, when re-scoping a workflow leaves nodes naming terms that are still perfectly there. It reads unavailable now, and asks once, which left getEnabledInTenant() with no caller.
• A subprocess node calling a disabled child surfaced the engine's own LogicException from underneath, instead of the node's existing contract for a child it cannot run. Worse, launchChild() documents that it does not throw - the re-launch path calls it after re-parking a parent - and a child disabled since that parent parked broke the promise.
• Start was still offered on a workflow the engine refuses to start, so the only way to find out was the exception page.
• The subprocess child select and the webform handler's start-workflow select still offered disabled workflows, each guaranteeing a failure the author could not see coming.
• A guard on the Enabled checkbox that cannot fire, explained by a submission not carrying the field. The form builder fills an element's value from the element itself, so a submit handler never sees it absent. Measured, then removed; the finding it was written for turned out to be unreachable and was reverted with it.
• StatusForm::copyFormValuesToEntity() did nothing: config casts a value to the schema's type on save, so the flag is a boolean either way. Removed, and the stored type is now asserted.
• No coverage at all for the status forms, or for either Enabled checkbox. Added; writing the first of them is what showed the copy above was inert.
• Both entity_keys comments explained the flag by isEnabled(), which config entities do not have, and by the wrong reason. What declaring the key actually does is now written down and pinned by a test.
• The claim that the Complete Modeler is the only place to disable a workflow, repeated in a comment, a test docblock and the documentation. The flag is the Modeler API's own, so its model panel and its list operations reach it too.
• The Statuses list gave no sign which terms were retired, which is most of the point of retiring rather than deleting.
• The documentation named a Disable operation for statuses. That is the Workflows list's; a term is retired by unchecking Enabled on its own form.

One measurement was withdrawn rather than shipped: a query budget written to pin the double read counted zero, because a config entity query is served through the config factory and issues no database query at all. The de-duplication stands on its own; a test asserting a number that cannot be measured does not.

Corrections inside the nineteen, all with a test that fails without them:

• TenantContextInterface::getTenantOptions() already existed and orders by weight then label. The hand-rolled loop it replaced used storage order, so this one select ordered tenants differently from every other on the site and ignored the weight that exists to order them.
• A workflow scoped to a tenant that no longer exists could not be saved at all: a select whose default is not among its options is refused as an illegal choice, and drush config:delete on a tenant leaves the workflow behind, which docs/multi-tenancy.md warns about. Verified against the real form ("The submitted value ghost in the Tenant scope element is not allowed."). The stored scope is now offered marked (missing).
• Batching the entity-to-runs lookup also deduped a run bound under two keys. The contract is a list of runs, so that is a fix, but only the performance half was written down.
• The grouped target load is a two-level [$type][$id] lookup, and every single-type fixture passes whether it is nested or flat. Flattened, a run binding an order and a customer whose ids coincide returns one of them under both keys.
• docs/multi-tenancy.md promises the scope survives a save from the other editor. It does, but nothing pinned it, and losing it would be silent.
• One query budget sat at exactly its threshold. All five were measured; that one now has a condition of headroom and still catches the defect.
• Three commit bodies named test methods renamed for phpcs, and one claimed a behaviour-neutrality the sequence row had not earned.

That lane's result, read per class rather than by de-duplicated message: 25 classes, 56 clean assertion failures, zero errors. Two of the newer tests correctly pass under it, because they are mutation-proven guards rather than red-to-green proofs - reverting the diff neither makes resetGraph() clear the tenant nor flattens the grouped target lookup - and their commit bodies say so.

The three de-duplication commits:

• The hand-off return block was verbatim in both identity doorways, down to the comment; it moves to the trait that already owns the call whose response it inspects. The task doorway had no coverage of it at all, so dropping the call there would have been silent.
• hasChosenPlugin() moves to PluginSequenceFeatureBase, removing a private helper from one feature and a mid-method foreach { return; } from the other.
• readNodeInteraction() makes the query narrowing and the row filter provably one predicate. Worse than ordinary duplication: the two ends meet, so a condition added to one silently drops a continuation entry. It also removes a redundant definition resolution on the finished page.

Noticed and not fixed

In ConditionTreeBuilder, the Add sub-condition buttons have the same accessibility ambiguity as the Remove buttons at depth: one per composite, with nothing naming which. Out of this finding's scope.

Three more, all pre-existing and none of them touched here:

• A workflow declares no config dependency on the workflows its subprocess nodes name, though it declares one on its tenant and on every status its nodes reference. A config import can therefore order a parent before the child it calls. The deletion guard covers the delete case by walking the definitions; the import ordering is a separate question.
• The webform handler's start path guards nothing: a workflow that cannot start - deleted, and now also disabled - throws after the submission has been saved, so the submitter meets an error page with their answers stored and no run begun. Reachable today by deleting a referenced workflow, and unchanged by this branch, which only made the select stop offering one more way in.
• Deleting a tenant clears the Orchestra binding on any domain pointing at it, silently. That is a pointer rather than data, and leaving it dangling is worse - a tenant machine name is reusable - which the hook doing it explains. It is not counted as something the realm owns, so the confirmation page does not mention it.

composer.json and phpstan.neon each carry one word the project's own cspell run flags (Yoyaku, breakingly); CI's own dictionary accepts both, and neither is touched here.