Prose, dead code and naming sweep from the pre-beta audit

Problem

The mechanical remainder of the full pre-beta audit: docblocks and comments that describe something other than the code beneath them, dead code with no caller and no @api promise, names that do not match what they return, and stale references. Grouped by area so the list is navigable and the diff stays reviewable: it is large and each item is small.

Most are prose, but not all. Where a docblock and the code disagree, the code is sometimes the thing that was wrong, so about a dozen of these change behaviour: an accessor that collapsed two stored states into one, resolutions that reported a success that had not happened, a read gated on the wrong permission. Each of those carries a test.

83 findings, all low.

The findings

Action, ECA and audit trail

• The timeout subscriber leaves six shared ECA bag names polluted, the hazard the eca_event task documents at length. modules/orchestra_eca/src/EventSubscriber/TaskTimedOutSubscriber.php:81-86 against EcaEventTask.php:137-155,178-192
• The ECA kernel test re-implements two helpers of the trait it uses. modules/orchestra_eca/tests/src/Kernel/OrchestraEcaTest.php:36 against :556-581
• attachment_key is documented nowhere, and the schema comment's enumeration of untyped keys omits it. modules/orchestra_action/config/schema/orchestra_action.schema.yml:1-8, README.md:19-26, key at ActionTask.php:211-212,232,281-290
• docs/actions.md documents a settings shape the shipped schema rejects, and a modeler limitation that no longer exists. :40-52 and :82-86
• A test-module action's docblock claims a test that does not exist in either shape. modules/orchestra_action/tests/modules/orchestra_action_test/src/Plugin/Action/RecordSettingAction.php:17-19
• A dead condition, and the one unguarded action instantiation in the submit handler. ActionTask.php:369-375
• The only non-PII completion-provenance key is left in the purgeable bucket, against the module's own stated rule. modules/orchestra_audit_trail/src/EventSubscriber/OrchestraAuditTrailSubscriber.php:43-57, emitter at src/WorkItemManager.php:1377
• An install test's docblock says three orchestra channels; the chain claims and the test assert four. modules/orchestra_audit_trail/tests/src/Kernel/AuditTrailChainInstallTest.php:17-21 against :60-63
• A routing key is schema-typed as translatable multi-line text. modules/orchestra_eca/config/schema/orchestra_eca.schema.yml:8-10
• One action restates the inherited default. modules/orchestra_eca/src/Plugin/Action/StartProcess.php:123-125

Base: entities and events

• WorkItem::getAudiences() collapses the "no re-offer" and "re-offer that notifies no one" distinction its own field docblock declares. src/Entity/WorkItem.php:217-223 against :93-95
• A notification-event accessor is named for a different method's job. src/Event/OrchestraNotificationEvent.php:126-134, references at :30 and :94
• WorkItemEvent documents two of the three verbs it is dispatched with. src/Event/WorkItemEvent.php:13,28-29

Base: forms, hooks and Drush

• Three docblocks place the work item in a submodule; the base module owns it and deletes it. src/Hook/OrchestraWorkItemHooks.php:18, src/Hook/OrchestraTenantHooks.php:33-34, src/Hook/OrchestraWorkflowHooks.php:29
• The menu alter repoints the section link at the page its own Settings child already opens. src/Hook/MenuLinksAlter.php:26-32 with orchestra.links.menu.yml:13-18
• An emptied default timeout outcome writes an empty outcome, and the runtime fallback only covers a missing key. src/Form/TimeoutSettingsForm.php:81-86,144, consumer at src/Plugin/TimeoutAction/Resume.php:75-77

Base: plugins

• A composite condition instantiates children without checking the plugin exists, so a missing one throws mid-advance. src/CompositeConditionBase.php:200-212

Base: services and contracts

• TenantListBuilder renders raw machine names in an untranslated concatenation, and its helper's declared return type contradicts its docblock. src/TenantListBuilder.php:78-92
• ReadAccess's class docblock has its bulleted list broken mid-line, so three values render as one paragraph; the same stray-wrap defect recurs in about ten docblocks. src/ReadAccess.php:19-26
• ContactRecipient::key() depends on array insertion order, contradicting the interface's "stable identity" on a frozen constructor. src/ContactRecipient.php:68-71 against src/RecipientInterface.php:66-73
• Two smaller contract slips in the variable layer. src/VariableResolver.php:160-172, src/VariableStorageSchema.php:8-12

Base: versioning, retention and incidents

• openIncidentToken() promises a token in the error state and never checks the state. src/IncidentStore.php:237-253
• getExecutableHash()'s docblock omits the variables it does hash. src/WorkflowVersionManager.php:874-889 against :805-817
• The migration's raw writes reset the entity cache but do not invalidate cache tags, unlike every other raw write in the engine. src/WorkflowVersionManager.php:621-629,636-645 against src/StateTransitions.php:82-90,121-126

Content, moderation and ECA

• Both ECA actions re-declare what the ECA action base already provides. modules/orchestra_content_eca/src/Plugin/Action/LoadAttachedEntity.php:46-51,66,143-145, StartProcessForEntity.php:134-136
• The attachment manager's one-per-key comment is a broken sentence, losing the rule it states. modules/orchestra_content/src/AttachmentManager.php:54-57
• A test property carries two consecutive docblocks; the first is orphaned and its explanation is lost. modules/orchestra_content_moderation/tests/src/Kernel/ModerationTransitionTest.php:37-52
• Three test method names still name methods that were renamed. modules/orchestra_content/tests/src/Kernel/AttachmentTest.php:98,119,277

Engine core

• A teardown loads every token an instance ever had, rather than filtering on the live states. src/WorkflowExecutor.php:1963

Examples, config and CI

• Two examples-test docblocks contradict their own bodies. modules/orchestra_examples/tests/src/Kernel/ExamplesTest.php:67,346-354
• The metrics generator double-counts a method that is both prefixed and attributed. scripts/generate-metrics.php:302-306, prose at :753-755
• Ten of the eleven shipped examples carry no diagram layout. Only the quorum example has a layout block
• Without modeler_api, four base-module routes have no tab and nothing linking to them. orchestra.links.task.yml:1-19, orchestra.routing.yml:9-39
• The Tugboat guide page and its French translation describe different sites, and both mis-describe one example. .tugboat/setup.php:408-425 against :454-472
• Two entries on the deliberately-untranslated list are interface strings, permanently exempting them from the check. translations/untranslated.txt:22-28,52,55

Notifications and mail

• A sanitize-false argument does not stop the escaping the comment says it stops. modules/orchestra_easy_email/src/EventSubscriber/EasyEmailNotificationSubscriber.php:231-247
• Two modules each re-implement the resolver's token-node accessor. modules/orchestra_notification/src/Plugin/TaskType/Notify.php:150-159, modules/orchestra_interaction_notification/src/EventSubscriber/ParkNotificationSubscriber.php:161-170
• A node-feature docblock claims only the config key and the node differ from its sibling. modules/orchestra_notification/src/Plugin/NodeFeature/NotifyAudience.php:20-23
• A test docblock names a class that does not exist. modules/orchestra_interaction_notification/tests/src/Kernel/ParkNotificationDispatchTest.php:24
• The attachments accessor reads wrong at both call sites. src/Event/OrchestraNotificationEvent.php:132, callers in orchestra_mail and orchestra_easy_email
• The Easy Email Cc and Bcc path, both channels' Bcc-only fallback, and the reassigned branch are exercised by no test. EasyEmailNotificationSubscriber.php:115-142, NotificationMailSubscriber.php:126-134, TaskNotificationSubscriber.php:95-100,232-239
• A send cancelled by a mail alter hook is logged as a failure. modules/orchestra_mail/src/EventSubscriber/NotificationMailSubscriber.php:159
• The Notify node accepts any notification type; the identical field on its sibling validates it. Notify.php:135-137 against ParkNotification.php:116-126

Task, operation and presentation

• The completion-variables method is one behaviour duplicated byte for byte, and its own twin already lives in the shared trait. InteractionTask.php:143-150, InteractionOperation.php:127-134
• The task gateway mints a token-less URL that can only render "no longer available", where its twin fails loudly. AssignmentGateway.php:53-55 against OperationGateway.php:53-60
• A test docblock documents a method that does not exist. modules/orchestra_interaction_task/tests/src/Kernel/InteractionTaskTest.php:589-590
• Three assertions in the shared-markup test cannot fail, because the class they look for is a prefix of one that is always present. modules/orchestra_presentation/tests/src/Kernel/SharedMarkupTest.php:100,186,188
• One msgid is an orphan in the interaction task's own catalog. modules/orchestra_interaction_task/translations/fr.po:14-15

VBO and domain

• A README sends the reader to the wrong module for the inbox dashboard. modules/orchestra_vbo_inbox/README.md:7
• An entity builder service-locates a manager the class already injects. modules/orchestra_domain/src/Hook/OrchestraDomainTenantHooks.php:97
• The tenant-predelete docblock names the wrong consequence. Same file, :131-135
• The work-item manager interface does not document the exception its own bulk-action caller depends on. src/WorkItemManagerInterface.php:320-339 against CompleteTask.php:64-66

orchestra_cm

• A form test exercises the flow clone against a flow-condition plugin id that does not exist. modules/orchestra_cm/tests/src/Kernel/OrchestraCmFormTest.php:565,571
• The row-opening helper opens only the node or flow row, so a validation error inside a collapsed sub-section stays hidden, contradicting its own docblock. OrchestraCmForm.php:778-797
• A base class's open-by-default setting and its justifying comment are unconditionally overwritten by the host form. OrchestraCmForm.php:426-431 overwriting src/PluginSequenceFeatureBase.php:109-112

orchestra_delegation

• The page-limit constant's docblock names one listing; the pager is applied to all three. DelegationsController.php:33-35,266
• The "when does this verdict next change" rule is implemented twice. DelegationResolver.php:220-234, DelegationsController.php:392-413
• The entity label returns a pre-escaped string, which the revoke confirmation escapes again. src/Entity/Delegation.php:129-145
• The access handler's stated read rule is contradicted by the "delegated to me" tab, and view access is consulted nowhere in the application. DelegationAccessControlHandler.php:17-24 against DelegationsController.php:254-266

orchestra_inbox

• A stylesheet names a library that does not exist. modules/orchestra_inbox/css/inbox.css:5-7
• The controller reimplements two methods of the service it already injects. InboxController.php:129-146,351-355
• Two tests do not measure what their docblocks claim. MyTasksViewTest.php:533-593,729-769

orchestra_interaction

• Config schema marks three keys required that the code treats as optional, under a fully validatable parent. config/schema/orchestra_interaction.schema.yml:6-16,34-55
• The README's shipped-plugins row is one plugin short. README.md:22
• The operations hook reads the chain field by string literal, bypassing the constant every other reader uses. src/Hook/OrchestraInteractionOperationsHooks.php:56
• Two public methods with no caller and no api promise. src/InteractionToken.php:170, src/InteractionResolver.php:367
• Chain messages are rendered with an instance-scoped context even for a branch-scoped visitor. src/Controller/InteractionController.php:272
• Three docblocks name a method that does not exist, and two parameter or return types are wrong. InteractionController.php:83,90,98,300, InteractionAccessTest.php:578
• The message interaction discards the text format's bubbleable metadata. MessageInteraction.php:251-256

orchestra_interaction_webform

• Renaming or deleting the binding element degrades silently, and on a start-and-resume form into starting a new run per resubmit. OrchestraInteractionHandler.php:358-366,535-541,562-568
• A trait's docblock describes sharing between handlers that no longer exist. src/SubmissionValuesTrait.php:20-21
• Two shipped-config inconsistencies in the examples module. The two example webforms and the request-validation workflow against its own serialized diagram
• Three comments name methods that do not exist. OrchestraInteractionHandler.php:638,705,707
• An instance-submission locator promises newest first; the multiple-load does not preserve the sort. src/InstanceSubmissionLocator.php:31-47

orchestra_ui

• State and status are swapped in the instances-list comments, where both words name different real filters, and one comment omits a field that exists. src/Form/InstanceFilterForm.php:15 with the fields at :96-126, and src/Form/InstanceListForm.php:154-155
• A return-embedding helper re-implements the shared one and diverges from it in two ways. src/PendingActionLinksTrait.php:85-88
• Four unreachable or dead fragments.
• Four access-check-disabling calls with no written reason. src/Controller/MyInstancesController.php:101, src/Form/InstanceListForm.php:109, src/Controller/OrchestraUiController.php:179,294
• A lost claim race on the resume form discards the operator's variable edits under a success message. src/Form/IncidentResumeForm.php:97-117
• The justification for a zero max-age on the trace names a mechanism that does invalidate tags, in three places. OrchestraUiController.php:409-418, InstancePageCacheTest.php:18-24,68-70
• A guard test says the engine throws on a non-parked token; it cannot. SignalGuardTest.php:19-23,66
• Three docblocks name APIs that do not exist: a pending-actions method, two test method names plus four references to a per-token accessor, and a library in a module that has no libraries file. InstanceListForm.php:541-542, PendingActionsFinderTest.php:396,447-478, css/instance-detail.css:5-6

orchestra_views

• A vacuous assertion in the trace test. tests/src/Kernel/ViewsHistoryTest.php:113

Outcome

Worked in the merge request, grouped by area. Where a docblock and the code disagreed, the code was sometimes the thing that was wrong, so about a dozen findings changed behaviour rather than prose; each of those carries a test, and the test-only job runs them against the unfixed code so a test that pins nothing shows up as a pass.

Seven of the findings above did not reproduce against 1.x. Six had been fixed by work merged since the audit was written: the composite condition's missing plugin check, the moderation test's double docblock, the shared-markup test's prefix assertions, the interaction operations hook's string literal, the inbox views test docblocks, and the migration's cache-tag invalidation at the site that finding names. The orchestra_ui "four unreachable or dead fragments" entry was not located either, by sweeps for unused private members, for unreachable statements, or for always-true conditions; that module had moved by 795 insertions and 117 deletions in the meantime.

Found here, not fixed here

Two defects this work uncovered are engine changes rather than part of a prose sweep, and want their own issue:

• Six raw token writes never invalidate their cache tags: three in SubprocessCoordinator, two in TimeoutSweeper, one in WorkflowExecutor. This is the family of the cache-tag finding above, which named only the migration.
• EcaEventTask's token-bag snapshot cannot restore. It snapshots through getTokenData(), 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.

Remaining tasks

• Review the merge request.

This issue summary was drafted 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.