Task surfaces: an unnamed bulk checkbox, bypassed guards, and lists that go stale

Problem/Motivation

The inbox, the bulk actions, the Views handlers and the administrative lists. The accessibility one leads: every checkbox on the bulk instance list, including the one that deletes, has an empty accessible name.

Found by a complete file-by-file audit of the whole module at commit bc205f4f: 1130 tracked files, 1126 read in full, 4 binary assets recorded as skipped. Every linter was already clean at that commit, so none of this is something a tool reports: phpcs Drupal over 1008 files and DrupalPractice over 814 both at zero, PHPStan level 5 with no errors, cspell against the CI configuration over 1061 files with no issues, eslint with no errors, stylelint with no problems, composer validate, and the per-project translation check reporting every string translated in the catalog of the project that ships it.

Findings

High

1. Every checkbox on the process-instances bulk list has an empty accessible name (modules/orchestra_ui/src/Form/InstanceListForm.php line 103, accessibility). The tableselect row options are keyed 'instance', not 'title', and core builds each row checkbox's accessible name only from $element['#options'][$key]['title']['data']['#title'], so every checkbox renders with #title => '' and #title_display => 'invisible'. A screen-reader operator on /admin/structure/orchestra/instances hears a column of unnamed checkboxes and cannot tell which instance they are about to Cancel, Migrate or Delete, on a form whose delete cascades to tokens, variables and work items. Fix: Rename the first column key from 'instance' to 'title' in both $header and $options, and assert $form['instances'][$id]['#title'] in InstanceListDisplayTest.
2. An inert delegation still silences the delegator and mails the ex-delegate (modules/orchestra_delegation/src/DelegationResolver.php line 161, correctness). getUserDelegators() drops a delegator who is blocked or has lost 'process orchestra tasks', but getUserDelegates() and shouldNotifyDelegator() apply no equivalent test to the delegate. After a delegate is blocked, task notifications for the delegator are still suppressed and still sent to the blocked account, so nobody who can act is told about the task. Fix: Filter the delegate ids through the same isActive() and hasPermission() test, record their user:(uid) cache tags, and make shouldNotifyDelegator() ignore delegations whose delegate can no longer act.

Medium

1. The bulk Complete task action bypasses the guarded-outcome guard every other completion doorway enforces (modules/orchestra_vbo_inbox/src/Plugin/Action/CompleteTask.php line 62, correctness). CompleteTask::execute() passes the configured outcome straight to WorkItemManager::complete() without checking getGuardedOutcomes(), so an outcome the node marked as requiring UI input is recorded with that mandatory input never collected. A manager selects comment tasks in a view and runs Complete task with outcome rejected; every task is recorded as rejected with no comment, and the later step reading the node's comment_variable finds nothing. Fix: Skip a task whose getGuardedOutcomes() contains the configured outcome, with a per-row message in the style of the existing ones.
2. The bulk Reassign task action can strand tasks on an account that cannot process them (modules/orchestra_vbo_inbox/src/Plugin/Action/ReassignTask.php line 65, correctness). The bulk reassign takes any account from an unrestricted entity_autocomplete and never checks 'process orchestra tasks' on the target, although the single-task ReassignForm refuses exactly that case and WorkItemManager::reassign() screens only for a blocked account. A manager bulk-reassigns fifty tasks to a colleague lacking the permission; VBO reports Reassigned for every row, the tasks vanish from every inbox and nobody is told. Fix: Move the permission check into WorkItemManager::reassign() so both doorways share it, and fix the test that currently pins the wrong behaviour.
3. The Action link views column is click-sortable although it contributes no field to the query (modules/orchestra_inbox/modules/orchestra_inbox_views/src/Plugin/views/field/TaskActionLink.php line 101, correctness). TaskActionLink::query() is deliberately empty so field_alias stays at FieldPluginBase's default 'unknown', yet neither the handler nor its views data turns click-sorting off, unlike its twin TaskOperations. A site builder who adds Action link to a table view and ticks Sortable gets ORDER BY unknown and the page dies with Unknown column 'unknown' in 'order clause'. Fix: Override clickSortable() to return FALSE the way TaskOperations does.
4. The inbox page declares no cache tag for the accounts it names or the workflow config its labels come from (modules/orchestra_inbox/src/Controller/InboxController.php line 244, cacheability). buildInbox()'s #cache.tags carry only the work-item list tags plus the viewer's and the matcher's, while its rows print other users' display names and labels resolved live from the workflow config. After an operator is renamed or a workflow node or outcome label is renamed, every operator's /orchestra/tasks keeps showing the old wording until some unrelated work item is saved. Fix: Merge user_list and config:orchestra_workflow_list into the build's #cache.tags and extend the presentation test to pin them.
5. The shipped tenant task view lists pull operations alongside inbox tasks (modules/orchestra_inbox/modules/orchestra_inbox_views/config/optional/views.view.orchestra_inbox.yml line 261, correctness). The tenant overview filters on tenant and on state, but never on mode = task, so work items in operation mode are listed as tenant tasks. A manager on the tenant inbox dashboard sees pull-operation work items mixed into a list whose own description promises all open user tasks, rows no inbox surfaces and whose Assignee column answers a question never asked of them. Fix: Add the same mode filter my_tasks carries, re-export cache_metadata, and add the mirror test case.
6. The five 'current X' boolean views filters default to their inverted half (modules/orchestra_views/src/Plugin/views/filter/CurrentValueFilterBase.php line 64, correctness). Neither CurrentValueFilterBase (so CurrentTenant, CurrentUserInitiator, CurrentUserCompleter, CurrentUserOnBehalfOf) nor CurrentUserCandidate overrides defineOptions(), so they inherit core BooleanOperator's $options['value']['default'] = FALSE, and query() reads that empty value as the inverted 'show everybody else's rows' branch. A site builder who drags Current tenant onto a view and clicks Apply without touching the preselected radio saves the inverted filter: a cross-tenant administrator's dashboard lists every tenant except the one they are acting in, and a tenant-scoped operator's lists nothing at all, while the filter's title, its help and its docblock all promise the opposite. Fix: Add defineOptions() to CurrentValueFilterBase and CurrentUserCandidate setting $options['value']['default'] = '1'.
7. The linked-username views fields offer a link the viewer may not follow (modules/orchestra_views/src/Plugin/views/field/LinkedUserFieldBase.php line 118, standards). render() builds $user->toLink()->toRenderable() unconditionally with no access('view') check on the account, unlike core's own username rendering and unlike every other link this module builds. An operator with administer orchestra but not access user profiles reads an anchor on every row of the Assignee or Initiator column that lands on a 403, and because the handler declares no cache contexts the output does not even vary by permission. Fix: Return core's shape, a username theme element with the account, which access-checks, truncates and carries its own cacheability.
8. The workflow version list has no pager and runs a COUNT query per row (modules/orchestra_ui/src/Controller/WorkflowVersionController.php line 98, performance). buildVersionListPage() iterates every version a workflow has ever cut, with no range and no pager, and calls countRunningInstances($vid) once per row. An administrator opening the versions page on a workflow edited repeatedly under auto versioning waits on N loadRevision() calls plus N COUNT queries, growing without bound, on a page that has no second page. Fix: Page the version list the way the trace does, and resolve the running-instance counts for the shown page in one grouped query.
9. A filtered instance list that matches nothing reports 'No instances yet' (modules/orchestra_ui/src/Form/InstanceListForm.php line 228, correctness). The tableselect's #empty is a single unconditional string although the same build applies the GET filter's workflow, state, initiator and tenant conditions to the query above it. An operator who filters by state completed and matches nothing is told the tenant has no instances at all, when it may have thousands, and is sent to a Workflows tab that does not exist under that name. Fix: Pick between a filtered and an unfiltered message as MyInstancesController already does, and name the surface that exists.
10. Three files say the work item lives in orchestra_inbox, and it does not (modules/orchestra_views/src/Hook/OrchestraViewsHooks.php line 36, documentation). entityTypeAlter(), viewsDataAlter() and the README state that the work-item wiring is conditional on orchestra_inbox and that the on-behalf-of lens is conditional on a delegation base field, but orchestra_work_item and its on_behalf_of field are declared unconditionally by the base module, so both isset() guards are dead. A site builder installs orchestra_inbox believing it is what puts work-item columns and the actionable-by-current-user filter into Views, when orchestra_views alone provides them. Fix: Correct all three statements and drop the two dead guards.
11. The contrast test's completeness guard only catches new rules that declare color, so a new tag tone ships unmeasured (modules/orchestra_presentation/tests/src/Kernel/PresentationAssetsTest.php line 129, tests). The guard that is supposed to make the measured-selector list exhaustive only asserts a rule is listed when the rule declares color, but every status palette class declares background alone and inherits its text color, so a new background-only tone escapes both the guard and the measurement - as would any rule using the background-color longhand, which the rule reader never reads. A maintainer adds the next status tone the way the stylesheet's own header invites, the suite stays green, and a citizen reads a status tag whose inherited text color fails AA on that background - the exact failure the test's docblock says it exists to catch. Fix: Extend the guard to any rule declaring color or background or background-color, and read both background spellings in the cascade walk.

Low

1. An author-supplied label renders through the admin XSS filter in the same function that escapes it four lines above (modules/orchestra_ui/src/Controller/OperationController.php line 141, standards). processCell() returns a link element with the label as #title (escaped) in the happy path and #markup in the fallback, and buildMyActionsPage() renders the node label the same way; core's convention for an untrusted plain string is #plain_text. An instance or node label containing angle brackets renders as text when the row links and has the bracketed part silently stripped when it does not, so the same label reads differently in two cells of the same table. Fix: Use #plain_text at both sites.
2. The on-behalf-of work-item filter has no index behind it (src/WorkItemStorageSchema.php line 29, performance). viewsDataAlter() registers two filters on orchestra_work_item.on_behalf_of, a column WorkItemStorageSchema indexes nothing on, while the parallel completer lens got a dedicated completer_completed index for exactly this reason. A site builder who adds the offered 'Handled on the current user's behalf' list gets a full scan of the work-item table, the table whose own schema class says it grows without deletion. Fix: Add an on_behalf_of plus completed index, mirroring the completer one.
3. Three state-label views field handlers repeat the same component-render scaffolding (modules/orchestra_views/src/Plugin/views/field/WorkItemState.php line 34, duplication). InstanceState, WorkItemState and InstanceStatus each carry the same body: renderer injection and create(), a no-op query(), clickSortable() FALSE, one boolean option, a checkbox, a preRender() attaching the presentation library, and a render() choosing between a sanitized label and a rendered component. A maintainer changing how these columns render the shared component has to edit three near-identical files, and WorkItemState has already drifted from InstanceState. Fix: Extract an abstract StateLabelFieldBase holding the shared half, leaving the subclasses the entity check, the label, the theme hook and the classes.
4. The inbox controller re-implements TaskActions::prefetch() inline (modules/orchestra_inbox/src/Controller/InboxController.php line 141, duplication). buildInbox() builds the token id list, loads the tokens, derives the instance ids and loads the instances by hand, the exact query shape TaskActions::prefetch() implements and that both Views handlers call from preRender(). A maintainer adding a third entity to warm edits prefetch(), both Views columns follow, and the inbox page - the primary surface - keeps the old shape and pays a load per row again. Fix: Call the shared prefetch(), keeping the separate assignee preload.
5. The post-action redirect rule is written twice, verbatim (modules/orchestra_delegation/src/Form/DelegationForm.php line 319, duplication). The rule deciding whether saving or revoking lands on the personal or the tenant-wide list exists as an identical expression in two classes, and the delete form's comment acknowledges the copy. A maintainer changing the rule in one form leaves the other on the old behaviour, the exact drift the existing UI test was written to catch. Fix: Extract a small trait holding the one rule, since the two forms extend different core bases.
6. The incident functional test inlines the drain loop its own helper already provides (modules/orchestra_ui/tests/src/Functional/OrchestraUiIncidentTest.php line 76, duplication). testResolveIncidentFromInstancePage() writes out the claim/process/release loop verbatim while drainToIncident() in the same class is the identical loop. A maintainer changing how the fixture drains to an incident fixes one copy and the other test keeps the old behaviour silently. Fix: Call the existing helper and delete the inlined loop.
7. The instance-list display test calls the engine state filter a 'status' filter (modules/orchestra_ui/tests/src/Kernel/InstanceListDisplayTest.php line 25, documentation). The class and method docblocks and two inline comments name a status filter and a status query parameter, but the form reads state; the module's status is a different, workflow-authored field with its own filter. The class docblock also omits the Initiator filter the class itself asserts. A maintainer reading this test to learn what the instances filter offers goes looking for a status filter and finds a State select. Fix: Replace status with state throughout and add initiator to the list of filters the docblock names.

Proposed resolution

One merge request for this issue, one commit per finding, each commit naming the finding it closes. Every defect gets a test that fails against the unfixed code, so the ground is closed and the next audit pass has to look somewhere new.

Remaining tasks

• Fix each finding above, with its test.
• Run every linter the pipeline runs, and the impacted test classes, before pushing.
• Play the next-major lane by hand, since its composer job is manual and a skipped lane reads as green.

User interface changes

Named per finding above; no new screens.

API changes

None.

AI-Generated: Yes (Claude Code was used to run this audit and to draft this issue summary. I reviewed the findings against the source myself before posting; the code and tests will follow on the merge request.)