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

Twenty audit findings on the task surfaces: the process-instances bulk list, the two bulk actions, the inbox page and the Views columns and filters behind the shipped task lists, plus the delegation resolver, the workflow version list and two tests that could not see what they were written to catch.

One commit per finding, in the order the issue lists them, with the red failure text in each body.

Fixed

Accessibility. Every checkbox on /admin/structure/orchestra/instances rendered with an empty accessible name: core's tableselect builds it only from the row option keyed title, and the first column was keyed instance. A screen-reader operator heard a column of unnamed checkboxes on the form whose Delete cascades to tokens, variables and work items.

Bypassed guards.

• The bulk Complete task action handed the configured outcome straight to WorkItemManager::complete(), so a guarded outcome — one the node marked as requiring input collected on the task's own form — was recorded with that input never collected. Every other doorway refuses it; this one now does too.
• WorkItemManager::reassign() screened only for a blocked account, so a bulk reassign over an unrestricted autocomplete could hand fifty tasks to a colleague without process orchestra tasks: VBO reported "Reassigned." for every row while the tasks left every inbox and every pool. The permission test now sits beside the existing isActive() one in the manager, so the single-task form, the bulk action and an escalation timer all reach through it.
• A delegation whose delegate is blocked or has lost the task permission still silenced the delegator and still mailed the ex-delegate: the one person who could act was the one person not told. Both sides of a delegation now go through one getUserIdsThatCanAct().

Lists that go stale or lie.

• The inbox page declared no user_list and no config:orchestra_workflow_list, so a renamed operator, node or outcome kept its old wording on every operator's /orchestra/tasks until some unrelated work item was saved.
• A filtered instance list that matched nothing reported "No instances yet", and sent the operator to a "Workflows tab" no route or local task provides.
• The shipped tenant task view filtered on tenant and state but never on mode = task, so pull-mode operations were listed as tenant tasks.
• The five current X boolean views filters inherited core BooleanOperator's FALSE default, which query() reads as the inverted "show everybody else's rows" branch. A site builder who dragged Current tenant onto a view and clicked Apply without touching the preselected radio saved the inverted filter.

Other correctness and standards.

• The Action link views column was click-sortable although query() adds nothing, so ticking Sortable produced ORDER BY unknown.
• The linked-username columns built an anchor with no view access check on the account, so an operator without access user profiles read a link onto a 403 on every row, and declared no cache context to vary by. They now return core's #theme => 'username' shape.
• OperationController rendered an authored node label through #markup, so Approve <urgent> requests reached the operator as Approve requests. Both cells use #plain_text.

Performance. The workflow version list had no pager and ran a COUNT per row and a loadRevision() per row; it now pages, reads the page's snapshots in one loadMultipleRevisions(), and counts their running instances in one grouped query, so the page costs the same whatever it lists. The inbox loads every account it names in one pass, delegated holders included. The on-behalf-of column, which two views filters read, gained the index its parallel completer lens already had.

Duplication. An abstract StateLabelFieldBase for the three state-label columns; the inbox page warms its rows through TaskActions::prefetch(); the delegation redirect rule is one trait; the incident test drains through its own helper.

Documentation. Three statements said the work item lives in orchestra_inbox; it is the base module's, and both isset() guards resting on that were dead. The instance-list display test called the engine state filter a "status" filter.

Tests. The contrast test's completeness guard only fired on rules declaring color, but every status palette class declares a background alone and inherits its text color, so the next tone would have shipped unmeasured — as would any rule using the background-color longhand, which the cascade walk never read.

Withdrawn or deferred

None.

Tests run red to green

Every one below was run against the unfixed file, then against the fix, on the impacted class only. Functional tests were written but left to CI.

Test	Red against the unfixed code
InstanceListDisplayTest::testBulkCheckboxHasAccessibleName	The row checkbox names the instance it selects. -'Update Instance 1' +''
InstanceListDisplayTest::testEmptyMessageDistinguishesFilteredFromEmpty	A filter that matches nothing says so, rather than claiming there are none. -'No instances match this filter.' +'No instances yet. Start one from the Workflows tab.'
InboxActionsTest::testCompleteTaskGuardedOutcomeRefused	The guarded outcome is refused with a per-row message. -'Not completed: "rejected" must be chosen from the opened task…' +'Completed.'
TaskReassignTest::testReassignRefusesAnAccountThatCannotProcessTasks	The task did not move onto an account that cannot process tasks. Failed asserting that true is false.
InboxActionsTest::testReassignRefusesAnAccountThatCannotProcessTasks	The row says the task did not move. -'Not reassigned: it is already finished, or that account cannot hold it.' +'Reassigned.'
MyTasksViewTest::testActionColumnsAreNotClickSortable	The action_link column offers no click sort. Failed asserting that true is false.
WorkItemPresentationTest::testInboxDeclaresTheAccountsAndConfigItReads	Renaming an operator changes the Assignee cell. Failed asserting that an array contains 'user_list'.
TenantTasksViewTest::testTheListLeavesOperationsOut	Only the task is listed. Failed asserting that actual size 2 matches expected size 1.
ViewsDataTest::testEveryCurrentValueFilterDefaultsToEnabled	The orchestra_instance.tenant_current filter defaults to the half its title and help promise. Failed asserting that false is identical to '1'.
TenantTasksViewTest::testTheAssigneeLinksOnlyWhereTheViewerMayFollowIt	But not as a link the viewer cannot follow. Failed asserting that '<a href="/user/1" hreflang="en">carol</a>' does not contain "<a".
WorkflowVersionListTest::testVersionListCountsRunningInstancesInOneQuery	One grouped query answered every row, whatever the number of versions. Failed asserting that actual size 3 matches expected size 1.
WorkflowVersionListTest::testVersionListIsPaged	A limit returns one page, newest first. Failed asserting that two arrays are identical.
PendingActionsFinderTest::testMyActionsRowsPrintAnAuthoredLabelWhole	The Step cell escapes the whole authored label rather than stripping it. -'Approve &lt;urgent&gt; requests' +'Approve requests'
DelegationResolverTest::testDelegateWhoCannotActStandsInForNobody	A blocked delegate is not named a recipient of the delegator's work. -[] +[0 => 3]
DelegationRecipientsTest::testCoverFromAnInertDelegateChangesNothing	The blocked stand-in is dropped and the delegator is told again. -[0 => 2] +[0 => 3]
WorkItemIndexTest::testInboxColumnsAreIndexed	Missing work item index: orchestra_work_item__on_behalf_of_completed. Failed asserting that false is true.
PresentationAssetsTest::testEveryColoredTextMeetsTheContrastMinimum	Two runs, cards.css restored after each. With a background-only tone added: .orchestra-status-tag.is-pending decides how text reads, so a case in COLORED_TEXT names it. (the old guard: OK (1 test, 150 assertions)). With the warning tone rewritten as background-color: A warning status reads #1a1a1a on #8a6008, which is 3.11:1. WCAG 2.1 AA asks 4.5:1 of text. (the old test passes, having measured against the page).

Four findings are behaviour-preserving, so no red against the old code exists. Each gained a test that pins the property the change exists for, and each of those was shown not to be vacuous:

• StateStatusViewsTest::testEveryStateLabelColumnCarriesTheSharedShape — with the three subclasses reverted to their own copies: orchestra_instance.state is a state-label column. Failed asserting that an instance of class InstanceState is an instance of class StateLabelFieldBase.
• WorkItemPresentationTest::testInboxWarmsItsRowsInOnePassEach — with the prefetch() call removed: Four rows cost the same token and instance queries as one. Failed asserting that 8 is identical to 2.
• ViewsDataTest no longer enables orchestra_inbox at all, so its work-item base-table, relationship and lens assertions now run against orchestra_views alone.
• DelegationUiTest::testDirectlyReachedFormLandsOnTheOwningList drives the extracted redirect rule through both forms; it is Functional, so it runs here.

Performance

Query count is equal or lower everywhere. The version list goes from N+1 to 2 for a page of N versions. Every guard added is gated so it costs nothing where it cannot apply: the guarded-outcome check reads only when an outcome was entered and then resolves the node plugin complete() resolves next; the reassign permission test reads the account the manager already loaded; shouldNotifyDelegator() tests accounts getUserDelegates() resolves for the same delegator anyway, and loads nothing at all when there is no delegation. The two inbox cache tags are static strings.

Linters

• phpcs --standard=Drupal --warning-severity=1: 1010 files, exit 0.
• phpcs --standard=DrupalPractice --warning-severity=1: 816 files, exit 0.
• phpstan analyse -c phpstan.neon at level 5, after clear-result-cache: [OK] No errors.
• cspell with core's en-US config over the 51 changed and added files, minus .cspell-project-words.txt: nothing left.
• scripts/check-translations.php: exit 0. fr.po follows in orchestra_ui and orchestra_vbo_inbox; the new reassign log message joins its sibling in translations/untranslated.txt.

Kernel classes were run on SQLite and the schema- and query-sensitive ones again on MySQL, which is what CI runs.

Audit rounds

Asked to audit until a round found nothing, and not to push until it was clean. Seven rounds, by hand. What they found, in commit order after 0fed2661:

1. The StateLabelFieldBase extraction stated the stored option key twice per subclass — once in the constant the base reads, once as a literal in the options form. A change to one would have left the checkbox editing a key nothing reads, so the component would simply never appear.
2. Four sentences this branch had made untrue: two in DelegationResolver that still said the cache tags are the delegators' only, one in LinkedUserFieldBase that contradicted the trait beside it without saying so, and one in WorkItemStorageSchema claiming a sort that does not ship for the on-behalf-of lens. Verified against the shipped views rather than assumed.
3. Finding F034 named "N loadRevision() calls plus N COUNT queries"; the first pass fixed only the COUNTs, and its body claimed a query count the diff did not deliver. getVersions() now batches the revision loads.
4. Both exhaustiveness guards were lists written by hand, so the defect each exists to catch could ship again at a new site. They now bind to the set of plugins the module ships.
5. Two in buildInbox(): the holder label reads a second account on a task a stand-in took under cover, and the preload collected only assignees, so a page of delegated work paid a user load per row; and the cache tag this branch added for those names was user_list, which every profile save on the site invalidates. Both answered by one set of uids.
6. The widened contrast guard exempted pseudo-elements as a class, true of cards.css today by inspection rather than by construction — the same weakness the widening removed. The exemption now reads the rule.

Rounds 6 and 7 found no defect. What they swept, and why each was clean: all five callers of WorkItemManager::complete() (the two the finding did not name are the guarded surface itself, so offering a guarded outcome there is the point); both callers of claim() (always the acting user, both gated); every views field and filter handler in the module (all in the two modules the guards now cover exhaustively); every #markup in the module (one consistency change, the rest are t() or self-composed); the other two surfaces printing another account's name (DelegationsController already keys on its parties, InstanceReadController is uncacheable); and the third account the inbox can name — the delegator behind a pooled row — which is tagged through the matcher, whose cacheability merges the delegation resolver's.

Considered and deliberately not changed

• An out-of-range ?page= shows the "nothing yet" message. On any paged list here, a hand-set or stale page number ranges past the end and the empty text reads as though the tenant had nothing. Core's own list builders behave identically, and reaching it needs a typed URL, so the message is left alone rather than given a third branch. Named here because it is the same sentence finding F035 was about, reached another way.
• countRunningInstances() was replaced rather than kept beside its plural. Pre-1.0, and one rule at one site.

Correction to an earlier commit body

357e8909 says "Nineteen lines become one call" for the inbox prefetch; the removed code is fourteen lines plus its comment. The behaviour it describes is right and the query count is unchanged, but the number is wrong and the commit is already pushed, so it is corrected here rather than rewritten.