Audit of 1.x at c4593aac, the head alpha15 will be cut from, twelve commits past 1.0.0-alpha14, covering performance, security, Drupal standards, documentation and translations. Every gate is clean at that commit: phpcs over 940 files (exit 0), DrupalPractice (one pre-existing warning on a core-controller admin route), PHPStan level 5 (0 errors), 0 entity queries without accessCheck(), no raw SQL, no unserialize(), no |raw in Twig, 0 orphaned .po entries. Everything below is what the gates cannot see.

Configuration schema

1. The node "Action label" stopped being translatable when it was renamed. #3618935 renamed the setting act_label to action_label in the code and left orchestra.node_setting.act_label in config/schema/orchestra.schema.yml. The named type is now dead and the live key falls through to orchestra.node_setting.*, which is type: string; the old entry was type: label, and label is the type core marks translatable: true. So Config Translation no longer offers the label for translation, while the field's own description still tells the author it "is translatable". A strict-config-schema test cannot catch this: the fallback gives the key a valid type. The comment further down the file that names act_label as the worked example is stale with it.

Translations

2. 35 strings are translated into the wrong project's .po. Each of the 34 modules declares its own interface translation project, so Drupal imports a module's file only when that module is installed. Checked per project rather than against the union of the files, 33 source strings have their French filed under a different project. The direction that costs the most: fourteen base-module strings, the config/schema/orchestra.schema.yml labels (Action settings, Audience, Condition, Delivery, Flows, ID, Nodes, Notification type, Notify on arrival, Recipient, Recipients, Retry) plus src/CommentConfigTrait.php's "Outcomes requiring a comment" and "Store comment in variable", exist only in a submodule's file, so a site that installs the kernel without that particular submodule reads them in English. The reverse happens too: seven orchestra_ui status-form strings sit in the root file, "Assignment plugin" and "Settings" are in the root file but sourced in orchestra_inbox, and "Context to display" is in orchestra_interaction_webform's file but sourced in orchestra_interaction.

3. The package: Workflow string from .info.yml is translated in 4 of the 34 files. Left that way on purpose rather than filled in thirty more times: its French is "Workflow", so an entry produces byte-identical output, the same reasoning as the placeholder-only strings. It is recorded in the new translations/untranslated.txt so a later audit does not re-file it.

4. Checked the way the previous audits checked it, against the union of every .po, the module still reports exactly the five deliberate placeholder-only strings and zero orphans. That is why this went unseen: the union hides which file a translation lives in, and the union is not what Drupal imports.

Documentation

5. Five API names in the docs no longer exist. docs/extending.md names actAccess() twice (it is checkActionAccess() since #3619206), gives WorkItemManagerInterface the service id orchestra_inbox.manager (it is orchestra.work_item_manager, and it lives in the base module, not in orchestra_inbox), and describes OrchestraReturn as remember()ing and recall()ing a target (the methods are set(), get() and delete()). docs/external-interaction-internals.md names InteractionController::step() twice (it is routeToStep()). docs/concepts.md resolves the tenant status vocabulary through an orchestra.status_catalogue service that does not exist (it is orchestra.status_repository).

6. The OpenAPI description of the instance list does not mention that it is paged. GET /orchestra-api/instances takes limit and offset and always returns at most one capped page, which is what OrchestraClientInterface::listInstances() documents and what the controller forwards. docs/openapi.yaml documents neither parameter and describes the 200 as "the matching instance summaries", so a remote consumer reading the contract believes it receives every match and silently receives a page. The 400 the endpoint returns for an unknown state is undocumented too.

Comments that describe code that is not there

7. OperationAccessCheck's class docblock says the verdict is "not cached across requests"; the method deliberately builds a fully cacheable result and says so three lines further down. One of the two is wrong about the security-relevant behaviour of the check.

8. InstanceListForm carries an orphaned comment, "Cache each workflow's published version, so the migrate-to-current operation is offered only for an instance behind it", above the status-label map. No such cache exists, which is finding 9.

Performance

9. The operator instance list issues a key-value query per row in manual versioning mode. WorkflowVersionManager::getPublishedVersion() is an uncached keyvalue read and isInstanceCurrent() calls it once per instance, so a 50-row page costs 50 SELECTs. The same form already memoizes getMigrationTarget() per workflow a few lines below, which is the shape the orphaned comment in finding 8 promised.

10. pruneOrphans() runs on every cron and, per workflow-version snapshot, issues that same uncached pointer read plus an allRevisions() query before its healthy-case fast path can skip anything: two queries per workflow per cron, in every versioning mode.

Continuous integration

11. .gitlab-ci.yml states that the module ships no CSS in a linted path. It ships five stylesheets, and the stylelint job's own trace reads "5 sources checked, 0 problems found". The _STYLELINT_EXTRA: '--allow-empty-input' the claim justifies is not needed, and the comment invites the next contributor to believe their CSS is unlinted.

12. The Drupal 12 lane has not run since it last failed. It is manual, so it sits skipped on every pipeline. The last one that did run it, on da8dc0dc, failed both phpstan (next major) and phpunit (next major), and reported success anyway because those jobs are allow_failure. Played on c4593aac now, the lane is green: composer, phpstan, and phpunit at 227 classes, 990 passed, 0 failed, 5 classes fully skipped, all five the documented skipWithoutDrupal12() set. The current-core lane at the same commit is 227 classes, 1013 passed, 0 failed. Nothing to fix in the code; the lane has to be played and read before alpha15 is tagged rather than assumed.

What the fix added beyond the findings

13. A CI job now holds the rule finding 2 had to be found by hand. scripts/check-translations.php extracts with potx, the extractor localize.drupal.org itself runs, and reports two directions per project: a string a project ships with no French of its own, and a catalog entry no file of that project carries. Advisory, at the severity of its peers in the validate stage. Running it found seven misfiled entries this audit had missed (Roles, Users, Created, Review, By variable and Notify this audience under orchestra_inbox, Comment under orchestra_interaction_task, all base-module strings): a hand-written check searches for the text as a substring, and a short entry like Roles occurs inside something else in almost any project. So the relocation is 35 strings over 60 catalog entries in 12 catalogs, not the 33 counted by hand, and the job is what will keep it at zero.

What an existing site has to do about the rename

14. Renaming the setting strands it in stored data, in two places, and pre-1.0 ships no update hook for it. A workflow saved before this carries act_label in its node config, where the code now reads action_label, so the label silently stops showing; and it carries it again inside the diagram modeler_api stores, as <camunda:field name="act_label">. The second is the one that bites twice: bpmn_io's Parser::findFields() reads every field name straight back into node config, so opening such a workflow in the modeler and saving it restores the dead key and loses the label again, against the lossless round-trip rule. Both are a rename away in the workflow entity (move the key in nodes.*.config, and rename the attribute in third_party_settings.modeler_api.data), saved through the entity API. Version snapshots that already contain the old key are deliberately left alone: they are immutable records of what ran and their hash is computed over the definition, so rewriting one would make the hash a lie. Instances still pinned to such a snapshot keep reading it until they finish.

Verified clean

The capability token is sound: HMAC over instance, branch and expiry keyed by the private key and hash salt, compared with hash_equals(), with the expiry inside the signature. The public dispatcher routes assert the grant on every entry point and are no_cache; the identity routes that change state carry _csrf_token. Route access checks carry their cacheability, including the tenant resolver's, inherited through AssignmentMatcher. The client refuses to send the OAuth secret over http at request time as well as at save time. No file, path or shell handling exists to get wrong. mkdocs navigation, the README module table, every documented admin path and every OpenAPI path match the code.

AI-Generated: Yes (Claude Code ran the audit and drafted this summary; every finding here was verified against the tree, the CI job traces or a potx run before it was written, and it will write the fixes.)

Issue fork orchestra-3620175

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Title: Pre-alpha16 audit: the node action label stopped being translatable when it was renamed, and 33 French strings sit in the wrong module's .po » Audit before alpha15: the node action label stopped being translatable when it was renamed, and 33 French strings sit in the wrong module's .po
Issue summary: View changes
mably’s picture

Status: Active » Needs review
mably’s picture

Title: Audit before alpha15: the node action label stopped being translatable when it was renamed, and 33 French strings sit in the wrong module's .po » Audit before alpha15: the node action label stopped being translatable when it was renamed, 35 French strings sat in the wrong project's catalog, and a per-project translations job now guards them
Issue summary: View changes
mably’s picture

Issue summary: View changes

  • mably committed 9ededbac on 1.x
    task: #3620175 Audit before alpha15: the node action label stopped being...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.