Assorted correctness and robustness findings from the pre-beta audit

Problem

The correctness findings from the full pre-beta audit that do not belong to one of the systemic families filed separately. Each is small and independent; the fixes are grouped so they can be reviewed commit by commit rather than as one change, and grouped by area below so the list is navigable.

76 findings, all medium. Three are marked Withdrawn below: they turned out not to be defects. One of those must not be implemented as written, because doing so would reopen a hole its controller deliberately closes.

The findings

One line each: what it is, and where. The failing scenario and the fix for each are carried by its own commit.

Action, ECA and audit trail

• A variable-named action target is loaded with no access, tenant or attachment check, and the docblock's stated mitigation does not bound it. modules/orchestra_action/src/Plugin/TaskType/ActionTask.php:193-204, claim at :45-50
• The audit_trail README's "what it records" table is stale in four ways. modules/orchestra_audit_trail/README.md:24-32 and :41-44
• eca_event resolves its own settings from the LIVE workflow at execution time, discarding the pinned node config it was handed, so this one task type escapes version pinning. modules/orchestra_eca/src/Plugin/TaskType/EcaEventTask.php:128 and :253-262, same pattern in OrchestraResume.php:119-134
• The ECA resume action answers any parked token in the tenant, bypassing the node's outcome allow-list and assignment re-check. modules/orchestra_eca/src/Plugin/Action/OrchestraResume.php:79-108

Base: entities and events

• TokenStorageSchema justifies one index with a consumer that does not query tokens. src/TokenStorageSchema.php:14-16 and :44-50
• One instance index is a redundant duplicate of the leftmost prefix of another. src/ProcessInstanceStorageSchema.php:65-77

Base: forms, hooks and Drush

• The instance access hooks' docblock claims they gate the user-facing delete paths; no code path in the project consults entity access on an instance. src/Hook/OrchestraInstanceAccessHooks.php:22-27,33-40
• The workflow Reading form names the site value as what "follow" resolves to, ignoring the tenant override that actually wins. src/Form/WorkflowReadAccessForm.php:50-57
• The purge command's limit is unvalidated: a negative value throws a database exception, zero or a non-numeric value is a silent no-op reported as success. src/Drush/Commands/OrchestraRetentionCommands.php:45-58
• Both Drush commands perform destructive work with no confirmation and no required flag. OrchestraRetentionCommands.php:45, OrchestraVersionCommands.php:51
• The migrate command catches only one exception type, so a mid-batch runtime failure escapes after part of the batch has committed. OrchestraVersionCommands.php:95-103, throwers at src/WorkflowVersionManager.php:559,598,614
• The purge dry-run reports a count silently capped at the purge limit. OrchestraRetentionCommands.php:52-54
• The cron-backlog requirement is skipped on any site whose site-wide mode is not queued, though a per-workflow override and the timeout sweep can both create the backlog. src/Hook/OrchestraRequirements.php:61-67

Base: plugins

• Join and split settings are submitted but never validated, so PluginFormInterface is a dead contract for those two plugin types. src/JoinBase.php:19, src/SplitBase.php:16 against modules/orchestra_cm/src/Form/OrchestraCmForm.php:1425-1491
• The email audience saves addresses its own schema declares valid and its own runtime throws away. src/Plugin/Audience/Email.php:41-55 and :74-82
• The users audience mints candidates for blocked accounts, and a sole blocked candidate auto-claims the task. src/Plugin/Audience/Users.php:68-70 and :85-91
• A username typo in the users audience opens the task to everyone, since there is no validation anywhere in the class. src/Plugin/Audience/Users.php:85-91, same for the two variable variants
• The timeout feature drops empty action settings, producing config that violates its own schema. src/Plugin/NodeFeature/Timeout.php:280-285 and :299-304
• A merging join with a collect key but no target silently writes no tally, and the quorum join's documented routing then never matches. src/Plugin/Join/MergingJoinBase.php:40-51
• A comparison condition with an empty variable name is a flow that always holds. src/Plugin/FlowCondition/Comparison.php:95-109

Base: services and contracts

• The task-type plugin memo is keyed by workflow id rather than by the pinned version, so a version-specific guarded outcome leaks across instances and can bypass a required comment. src/PendingActionsFinder.php:289-300, same key shape at src/WorkItemManager.php:1286
• collect() lists actions from work-item state alone and never checks the token is still parked, though its docblock promises "a parked, non-timer token". src/PendingActionsFinder.php:17-19 against :193-252
• Outcome labels are read with a bare array cast on three surfaces, where the owning trait's own comment says a decode is mandatory; the inbox decodes, so the two disagree. src/PendingActionsFinder.php:218, modules/orchestra_ui/src/Form/UserOperationForm.php:177, modules/orchestra_interaction/src/Form/CommentForm.php:145
• An outcome value containing a space or a dot makes the wrong submit button read as the trigger, so the first outcome is recorded and a required comment is validated against it. src/OutcomeButtonsTrait.php:43-50, src/OutcomeConfig.php:27-33

Base: versioning, retention and incidents

• Withdrawn, unreachable. cancelInstance() and failInstance() both resolve every open incident before they finish - "a terminal instance must not carry one, or an operator action on it would re-activate a token on a dead run" - and every operator action already returns early when no open incident remains. A run cannot end with one still open. src/IncidentManager.php:68-91,103-128,141-176
• migrate() silently skips every unpinned running instance while migrateInstance() migrates them. src/WorkflowVersionManager.php:426-433
• migrate() returns the planned count rather than the migrated count, so the operator is told skipped instances moved. src/WorkflowVersionManager.php:452-453 against the skips at :574-578 and :641-644
• migrate() documents no exceptions although the apply step throws mid-batch, and its own UI caller does not catch it. src/WorkflowVersionManagerInterface.php:173-191, caller at modules/orchestra_ui/src/Form/WorkflowMigrateForm.php:114-123
• A translation added after a version was cut is never captured, for running or new instances. src/WorkflowVersionManager.php:776-779,837-853,887-889

Content, moderation and ECA

• Withdrawn, and must not be implemented. The save-without-deciding redirect passes only the token, and that is deliberate: EntityTaskController::page() reads the attachment key and the form operation from the step's own configuration, because a request that could choose the form operation could open any form the entity type declares, its delete form included. Passing them back would reopen exactly that. modules/orchestra_content/src/Hook/OrchestraContentFormHooks.php:174
• The example ECA model config-depends on a workflow owned by a sibling module it does not depend on, so it silently never installs on its own. modules/orchestra_content_eca/config/optional/eca.eca.orchestra_example_article_review.yml:3-8
• orchestra_content/README.md is stale in three places: it calls an interaction a task type, lists shipped submodules as "planned follow-ups", and omits a hard dependency. :12-15, :20-22, :26-29

Examples, config and CI

• The French label of the approval example describes a design that no longer exists. modules/orchestra_examples/config/install/language/fr/orchestra.orchestra_workflow.example_approval.yml:2
• The shipped cron responder never clears its state key, and the resume action treats "token already consumed" as "the task is still running the event". modules/orchestra_examples/config/install/eca.eca.orchestra_example_cron_responder.yml:16-43
• generate-metrics.php reports success and exits zero even when the write failed. scripts/generate-metrics.php:776-782

Notifications and mail

• The task-timeout notification is the one notification handler with no guard, and a throw rolls back the timer's re-arm inside the sweep's transaction, so the sweep silently backs the deadline off. modules/orchestra_inbox_notification/src/EventSubscriber/TaskNotificationSubscriber.php:121-126
• User tokens never resolve in the Easy Email channel although the README and a docblock both promise they do. modules/orchestra_easy_email/README.md:58-64, modules/orchestra_easy_email/src/EventSubscriber/EasyEmailNotificationSubscriber.php:329-350
• The shipped Easy Email templates have translated subjects and untranslated bodies, which the per-project translations check cannot see because it excludes shipped config. modules/orchestra_easy_email/config/install/ against modules/orchestra_easy_email/translations/fr.po
• README and code promise an author-composed subject and body on the Notify node; no such field exists and nothing sets those parameters. modules/orchestra_mail/README.md:25, modules/orchestra_mail/src/Hook/OrchestraMailHooks.php:53-65

Task, operation and presentation

• The interaction operation is documented, described and translated as "work-item-free"; it mints a work item, which is then closed with neither completer nor outcome. modules/orchestra_interaction_operation/src/Plugin/TaskType/InteractionOperation.php:22, its info.yml, and its French catalog
• The shared card's heading is a span, so no card in any personal list is a heading, while the stylesheet still carries the heading reset that proves it was one. modules/orchestra_presentation/templates/orchestra-card-row.html.twig:44-46, css/cards.css:41-45
• Every card column renders its label and its value as two unassociated spans. modules/orchestra_presentation/templates/orchestra-card-column.html.twig:25-35
• Three shipped card colours fall below the 4.5:1 contrast minimum, including the card's primary call to action at 4.20:1. modules/orchestra_presentation/css/cards.css:146-150,205-215,259-262
• The card-row template documents a mapping variable the preprocess destroys. orchestra-card-row.html.twig:35-36, src/Hook/OrchestraPresentationHooks.php:154-159

VBO and domain

• The documented "a cron or CLI run records no origin" fallback is dead, because the tenant resolver negotiates a domain before the origin is read. modules/orchestra_domain/src/DomainNotificationContext.php:81-87
• The resolver declares the one cache context Domain documents as unable to tell path-prefixed domains apart. modules/orchestra_domain/src/DomainTenantResolver.php:36-39
• Every inbox bulk action returns nothing, so VBO's per-row message is empty and a skipped row is indistinguishable from a done one. modules/orchestra_vbo_inbox/src/Plugin/Action/ClaimTask.php:27-31, CompleteTask.php:52-67, ReassignTask.php:58-62
• orchestra_vbo/README.md describes the inbox companion as unbuilt, though orchestra_vbo_inbox ships it. Its core-bulk-form claim is true and stands: those actions extend core's ActionBase, declare a type and implement core's access signature. :33-34
• orchestra_domain/README.md understates the module: a hard dependency and half the feature set are missing. :21-24

orchestra_cm

• Changing a flow's condition seeds the new plugin with the previous plugin's stored settings, and Save persists them. modules/orchestra_cm/src/Form/OrchestraCmForm.php:638
• A composite's child conditions are never validated. A child naming no plugin is skipped at evaluation, so an all composite holding only such children is vacuously true and the flow is always taken. (An any composite goes the other way and is never taken.) modules/orchestra_cm/src/ConditionTreeBuilder.php:283-293, with only the top-level plugin validated at OrchestraCmForm.php:1483-1489
• Withdrawn, not a defect. Limiting validation to the swapped subtree is what #limit_validation_errors is for on an AJAX rebuild button: the swap handlers read only inside their own scope, raw user input survives the rebuild, and validating the whole form on every swap would block the swap on unrelated required fields, which is the failure the pattern exists to avoid. OrchestraCmForm.php:319,380,400,443

orchestra_delegation

• The duplicate-cover guard queries the stored tenant, not the submitted one, so moving a delegation between tenants bypasses it and leaves two overlapping rows. modules/orchestra_delegation/src/Form/DelegationForm.php:206-217
• The overlap guard counts switched-off rows, so "suspend without losing its dates" locks the pair out permanently. DelegationForm.php:212-229
• The Views action-column max-age assertion passes for a permanent cache, the exact failure it is written to pin. modules/orchestra_delegation/tests/src/Kernel/DelegationViewsTest.php:409-413
• A delegate releasing the delegator's task leaves no actor in the audit trail. src/WorkItemManager.php:818 (The reassignment half of this was fixed earlier in the series and is pinned by testReassignmentRecordsWhoDidIt.)

orchestra_inbox

• The "tenant tasks" view lists pull operations, and ships none of the assignee column it advertises in its own description and menu link. modules/orchestra_inbox/modules/orchestra_inbox_views/config/optional/views.view.orchestra_inbox.yml:17,29-168,237-349
• The completion message shows the machine outcome value where everywhere else shows the label. modules/orchestra_inbox/src/Controller/InboxController.php:370-372
• Reassign and release report success unconditionally, although both silently no-op. modules/orchestra_inbox/src/Form/ReassignForm.php:91-94, InboxController.php:278-279
• Reassign accepts a target who cannot act on the task, stranding it. ReassignForm.php:69-75 with src/WorkItemManager.php:837-846

orchestra_interaction

• A bearer capability link dispatches an assigned step's interactive interaction: surface, content and outcome links all render, and only the resume is refused, while three docblocks claim the bearer dispatcher never dispatches an operator step. modules/orchestra_interaction/src/CapabilityResolver.php:59-110 against :140-168
• The interaction and chain flags on a token go stale after an instance migration, silently dropping a visitor's chain messages. modules/orchestra_interaction/src/Hook/OrchestraInteractionChainHooks.php:127-158, src/InteractionResolver.php:590-601

orchestra_interaction_webform

• A refusal on the FIRST save is completely silent: no bind, no resume, no warning, no log, and an orphaned submission the instance-delete cleanup can never find. OrchestraInteractionHandler.php:263-282,351-447
• refuseUnviewableStep() justifies not gating the existing-submission paths with an entity access hook that does not exist anywhere in the project. OrchestraInteractionHandler.php:313-316
• The source provider is instance-scoped but not webform-scoped, so the run page can show a different step's submission as "your submission". src/WebformSourceProvider.php:44-52
• "Review (editable)" and "the party's earlier answers are kept" silently require webform's non-default update-token setting; with it off the party gets a blank form and a second submission is bound, abandoning the first. src/Plugin/Interaction/WebformInteraction.php:110-117,320-323, README.md:33-35
• The source provider's docblock claims webform access-checked rendering; the view builder checks nothing. src/WebformSourceProvider.php:16-21,48-51
• The examples README names a form that does not exist, and its link-variant instruction produces a broken flow. modules/orchestra_interaction_webform/modules/orchestra_interaction_webform_examples/README.md:10,22,57,58,65

orchestra_ui

• The requester's progress timeline conveys the current step by presentation only, and is not a list. modules/orchestra_ui/templates/orchestra-instance.html.twig:46-56, css/instance-detail.css:38-62
• The delete-confirm form's "nothing selected" escape hatch never fires, because a redirect set in the build phase is a no-op on GET. modules/orchestra_ui/src/Form/InstanceDeleteConfirmForm.php:102-108

orchestra_views

• The README describes a set of dashboards this module does not ship. modules/orchestra_views/README.md:8-20
• Four fields whose query adds no column are still click-sortable, so a sortable header orders by an unknown column. src/Hook/OrchestraViewsHooks.php:73-79,86-92,94-100,241-247 with the four handlers
• Two inverted "current value" filters on one view silently share a single bound value. src/Plugin/views/filter/CurrentValueFilterBase.php:59-67
• The current-user-candidate filter's inverted branch drops every unassigned row through SQL three-valued logic. src/Plugin/views/filter/CurrentUserCandidate.php:127-133
• The tenant boundary's absolute claims are wider than what the code does. src/Plugin/views/filter/CurrentTenant.php:21-26, src/Hook/OrchestraViewsTenantScopeHooks.php:22-27,65-67

Beyond the list

Two of the findings above turned out to be narrower than what they needed, so the merge request goes further than the line describing them.

• The username typo in the users audience is one case of a general rule. An audience that named people and matched none of them returns an empty candidate list, and an empty list is also what a step naming no audience returns, so every reader takes it for everyone. Validating one plugin fixes one plugin: the matcher draws the distinction now, for every audience including those shipped by other modules, and an Everyone audience exists so that a step which really is open says so rather than leaving it to be inferred. A human step naming no audience is refused when the model is saved.
• The duplicate-cover guard read the wrong tenant because the delegation form let an administrator move a cover between tenants. That move is gone rather than guarded. No other runtime entity offers one, and the access handler refuses view, update and delete on a cover in another tenant to everybody, so the option could only ever produce a row its author immediately lost sight of.

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.