fix: #3620619 Engine, inbox and admin surfaces: an audience that matched nobody no longer opens a step to everyone, a run's own version decides what it offers, and every action says what it did

Implements the findings of #3620619 that fall outside the clusters fixed in their own merge requests. !435 (merged) (content and notifications), !436 (merged) (interaction and webform) and !437 (merged) (presentation, UI and views) have all merged; this branch is rebased on them and holds what none of them covered.

Scope grew past the original "src/ plus orchestra_action and orchestra_eca" as those merge requests landed and the remaining findings turned out to live wherever they live: src/, and orchestra_action, orchestra_audit_trail, orchestra_cm, orchestra_delegation, orchestra_domain, orchestra_eca, orchestra_inbox, orchestra_inbox_views, orchestra_ui, orchestra_vbo, orchestra_vbo_inbox and orchestra_views.

Every behavior change was seen to fail against deliberately broken production code, with the break named. Where that was not possible, the commit says so.

Assignment and the unresolved sentinel

An empty candidate list means "everyone" to isAccountCandidate(), which is the documented reading of a node naming no audience. An audience that named two reviewers and resolved neither produced the same empty list, so a username typo, a since-blocked account or a stale id handed the step to every viewer on the site. The two audiences whose resolution can collapse a non-empty configuration mint AssignmentMatcher::UNRESOLVED instead; the matcher drops the sentinel whenever another audience found somebody, so it only ever appears alone. Creating such a task logs it by node and instance: no leak, and no silence either.

Alongside: users loaded blocked accounts, so a blocked sole candidate auto-claimed to nobody; and neither users nor email validated its configuration, so a mistyped username and an unparseable address were both accepted and then dropped at run time.

Version pinning

Four places read live where the pinned answer was the point. The ECA task and its resume action took node config from the live workflow. The task-type plugin memo was keyed by workflow and node but not version, so two runs of one workflow on different versions shared a plugin and one of them was offered the other's guarded outcomes — the same key shape, and the same leak, in WorkItemManager, where it decides the stored mode.

And a translation made after a version was cut never reached the runs. That fix took two goes: refreshing inside resolveVersion() looked right and never ran, because a Config Translation save writes a language override, fires no workflow update hook and unpins nothing. A subscriber on the override save and delete is the event that actually happens.

Versions and retention

migrate() reported the planned count rather than the committed one, silently skipped every instance pinning no version (the selector's <> never matches NULL) while migrateInstance() moved the same instance, documented none of the exceptions it raises, and let a mid-batch RuntimeException escape its UI caller as a white screen after part of the batch had committed. The purge command's --limit was unvalidated and its dry run reported a capped scan as a total.

What the operator is told

collect() promised "a parked, non-timer token" and listed from work-item state alone, so a step whose token errored behind an incident sat in "my actions" for ever. Release and reassign reported success unconditionally though both silently no-op. Reassign accepted a target who cannot process tasks, stranding the task. The completion message named the machine outcome value where every other surface shows the label. Every inbox bulk action returned nothing, so Views Bulk Operations showed a blank message per row — worst on the branch that deliberately does nothing, where a skipped row read exactly like a completed one. The Reading form named the site's read-access scope while the tenant's is what a following workflow inherits. The tenant task list promised "every task and its assignee" and shipped no assignee column.

An outcome value may contain a space or a dot, and the submit button was named after it: PHP rewrites those in a POST key, core then matches no button and falls back to the first, so the operator picked one outcome and the run recorded another. UserTaskForm had its own copy of the loop, and its own copy of the bug.

Elsewhere

A flow condition swap seeded the new plugin with the previous one's stored settings, and Save wrote them as its own. A composite's child conditions were never validated, and a child left unchosen is skipped at evaluation, so an all composite holding only such children is vacuously true — a guard that silently became a flow always taken. The delegation cover guard counted switched-off rows, so suspending a cover locked the pair out permanently. The same form also let an administrator move an existing cover to another tenant, which nothing else in Orchestra allows: of the seven entities carrying the shared tenant field the other six take it from context and never ask, and moving a cover re-points who is covered where without saying so. A cover now keeps the tenant it was declared in, and the guard reads that. A cron or CLI run recorded the default domain as its origin, because Domain's negotiation ends in an unconditional fallback and only the match type says so. The advance-backlog requirement was gated on the site-wide execution mode, which excluded exactly the sites that needed it.

Every key inside a timeout action's settings mapping was declared required, while the feature's own submit handler drops a setting the author left empty, so a spawn action given a variable and no value wrote config that violated its own schema and threw on save. An index that another one's leading column already served is gone, and two dead getStorage() calls with it.

Deliberately not changed

Four findings turned out not to be defects, and are left alone rather than "fixed":

• The save-without-deciding redirect passes only the token. EntityTaskController reads the attachment key and form operation from the step's configuration on purpose — a request that could choose the form operation could open any form the entity type declares, its delete form included.
• The node-scoped plugin-swap selects limit validation to themselves. That is what #limit_validation_errors is for on an AJAX rebuild: the handlers read only inside their own scope, raw input survives the rebuild, and validating the whole form on every swap would block the swap on unrelated required fields.
• orchestra_vbo's README claim that its actions also work with core's bulk-actions field is true: they extend core's ActionBase, declare a type and implement core's access signature. Only its "planned companion" line was stale.
• An operator action on an incident cannot reach a run that has ended. cancelInstance() and failInstance() both resolve every open incident before they finish, with a comment naming that exact hazard, and every operator action returns early when no open incident remains. A guard for it was written, found unreachable while auditing, and withdrawn.

Two more are real but not settled here:

• The Drush commands still take no confirmation before destructive work. DrushStyle::confirm() reaches Drush::affirmative(), which throws No input service available without a Drush runtime, so it cannot be called from commands that are unit-tested by direct invocation, as these deliberately are. It needs a Drush test harness, which is its own piece of work.
• A variable-named action target is loaded by id with no access, tenant or attachment check. There is no per-entity tenant primitive to bound it with, so restricting it is a design decision. The docblock and the setting's help text now say what actually bounds it, where an administrator configuring it will read them.

One thing to know before merging

A workflow can already hold an empty composite condition today - that is the defect - and refusing it applies on save, not only to the edit that introduced it. So a model that already stores all with no sub-conditions cannot be saved again until that flow is fixed, even if the author opened it to change something else. The message names the flow and says why, and the alternative is leaving a guard that takes every flow it is on, so this is the intended trade rather than an oversight.

@api changes

Five, all on interfaces marked @api. Nothing outside orchestra implements any of them, and the only downstream callers - yoyaku's order subscriber and its end-to-end test - use getDefinition() and complete(), neither of which moved. Checked against a current checkout rather than from memory.

• WorkItemManagerInterface::release() takes an optional $actor and returns bool instead of void.
• WorkItemManagerInterface::reassign() returns bool instead of void.
• WorkflowVersionManagerInterface::refreshCapturedTranslations() is new.
• WorkflowVersionInterface::getCapturedTranslations() and setCapturedTranslations() are new.
• RetentionManagerInterface::purge() returns a third key, capped.

Reading the red test-only changes lane

It is red on purpose. The job reverts the source changes and runs the tests, so a failure naming a new assertion is the proof that assertion bites. Two shapes appear there that are not proofs: an Error: Undefined constant is a revert artifact, because the constant is one of the source changes; and the index test's assertion that the redundant index is gone necessarily fails when the schema change is reverted.

This merge request was prepared with the assistance of an AI agent (Claude). The analysis and the wording were reviewed by me before posting, and accountability for the content is mine.