Prose, dead code and naming sweep from the pre-beta audit
Works the 83 findings the issue lists, one commit per area so the diff stays reviewable.
Behaviour changes, each covered by a test
WorkItem::getAudiences()returns NULL for "never re-offered" and[]for "re-offered to an audience that notifies no one". Both read as[]before, so the one caller reached past the accessor into the raw field to tell them apart.- The incident resolutions return
bool. They are guarded claims that write nothing when they lose, and every surface announced success regardless, so an operator's variable corrections were discarded under "Resumed incident 12". openIncidentToken()checks the token is still errored, so a resolution cannot re-queue a live branch or revive a canceled one.- An emptied site-wide timeout outcome falls back to
__timeout__rather than resuming with no outcome at all. - The Easy Email subject and plain body render through
replacePlain(). A'sanitize' => FALSEoption does nothing, because core's token service has no such option, so an apostrophe out of a process variable reached the reader as'. - A send cancelled by a site's own
hook_mail_alter()is no longer logged as a delivery failure. - The pending-action links embed their return target through
OrchestraReturn::embed(), which refuses an external destination; the hand-rolled copy did not. ContactRecipient::key()covers every address in channel order, so the same recipient built the other way round no longer survives de-duplication twice.- The timeout subscriber restores the shared ECA token bag, the hazard the
eca_eventtask documents at length. - The Notify node validates its notification type, as its sibling always has.
- A delegate may read the cover naming them, which their own tab has always listed and the access handler denied.
- An error inside a collapsed feature section opens that section, not just the row around it.
Also
- The ten examples that had no diagram layout have one, so opening an example shows the shape it was drawn as.
- The metrics generator counts test methods rather than the markers naming them; a method both prefixed and marked was counted twice.
- Duplication removed: two modules re-implementing the resolver's node accessor, two task types duplicating their completion-variables method byte for byte, an inbox controller re-deriving an action URL the shared resolver builds, two tests copying a shared trait helper.
- ReadAccess's bulleted list, folded into one paragraph by a stray wrap, is restored, along with the ~100 other docblock paragraphs left with an orphaned line by the same defect.
- Prose corrected where it described something other than the code beneath it, and the four access-check-disabling queries given their reason.
Not reproducible against this branch's tree
Six of the listed findings were already fixed by work merged since the audit was written against baad712f: the composite condition's missing plugin check, the migration's cache-tag invalidation (at the site the finding names, WorkflowVersionManager; see below for the rest of that family), the moderation test's double docblock, the shared-markup test's prefix assertions, the interaction operations hook's string literal, and the inbox views test docblocks. The orchestra_ui "four unreachable or dead fragments" entry did not reproduce either: unused private members, unreachable statements and always-true conditions were each swept over the whole tree, and the only hit is a false positive on a global potx populates. That module moved by 795 insertions and 117 deletions since the audit.
Gate
phpcs (the gitlab_templates ruleset), phpstan level 5 with a cleared result cache, cspell and scripts/check-translations.php all clean; every touched kernel class and the whole unit suite pass locally. The strings this branch adds are translated.
Found while working this, not fixed here
Two things this branch uncovered that want their own issue rather than growing a prose sweep:
- Six raw engine writes never invalidate their cache tags.
SubprocessCoordinator(three),TimeoutSweeper(two) andWorkflowExecutor(one) write a token row directly and do not go throughRawUpdateTrait. This is the family of the audit's own cache-tag finding, which named only the migration; closing it needs a test per site. EcaEventTask's token-bag snapshot cannot restore. It snapshots throughgetTokenData(), and ECA reuses that same value object on the next write to the name, so the snapshot holds whatever the task then writes into it. The timeout subscriber fixed here hit exactly that and now snapshots the rendered string instead; the task still has the original shape.