fix: #3620826 Describe the states the code can actually reach

Two claims explained why an instance runs the live workflow instead of a pinned version, and neither describes anything that can happen.

"Created before versioning." No such instance exists on any supported install: this is pre-1.0 with no update path. The one reason an instance legitimately pins nothing is the Off mode, which ensureCurrentVersion() short-circuits before it resolves anything.

"Its snapshot has been pruned." That is what pruneOrphans() prevents: it skips every revision isReferenced() finds an instance pointing at, alongside the default revision and the workflow's own pin.

What the sweep actually covered

The report that opened this named eight sites in src/. There are eleven there, and six more outside it: comments in orchestra_ui, orchestra_modeler and two kernel tests, plus a test method named after the state that cannot happen. Scoping a grep to the paths a report happens to list is how half a claim survives a sweep.

Where the correction had to be sharper than first stated

The reachable state is larger than "the mode is Off". A run started while the mode was Off stays unpinned after the mode is turned on, so unpinned running instances are ordinary on a site that versions today. That is exactly what orchestra_modeler's edit warning exists for, and its prose already said so more precisely than my first correction did. The docblocks now name the run, not the setting.

The definition_version field description is the one piece of this that a user reads, and it had the same imprecision. It would have told an operator on a versioning site something false about their own instances. Its French follows it.

A caveat for a case that cannot arise

RunningInstanceWarningSubscriber spent six lines excusing an approximation it does not make: it claimed it could not separate an instance whose pinned snapshot was pruned, so it counted such an instance as pinned. No such instance exists, so notExists('definition_version') is exact. The caveat is gone and the docblock says why the count is right.

The invariant is now tested

Everything above rests on one fact, and isReferenced() puts no condition on instance state, and a finished run protects its snapshot exactly as a live one does, which is what keeps a completed run's history reporting the version it actually ran. testPruneOrphanedVersions covered the running case only; a ->condition('state', 'running') could have been added to that query with the whole suite still green. The test now finishes the instance and asserts the snapshot survives. Injecting that exact bug fails this test and nothing else.

Two documentation pages

versioning.md closed on "a pinned instance keeps the shape it started on for as long as it runs", scoping to live instances a guarantee that holds after a run ends.

retention.md said snapshot pruning happens "when retention is enabled". The cron hook prunes first and checks retention after, deliberately, and testCronPrunesSnapshotsWithoutRetention pins it. Believing the page gets it backwards on exactly the sites least likely to notice: an operator who never switched retention on would conclude a deleted workflow's snapshots accumulate forever, when those are the ones cron clears unconditionally.

The quorum ballots

The first two ballots are identical apart from id and label, and their comments invited a reader to find a difference. They now say the first two share an audience and the third widens it.

Not the flat assignee_roles alternative that was suggested: concepts.md and integrations.md both state these ballots demonstrate the structured form, and request_validation and request_validation_link already ship the flat one. That change would make two accurate pages inaccurate to demonstrate something two other examples already demonstrate.

A second pass over the same ground

The cause two comments named is the one the code prevents

Replacing "created before versioning" with "whose live workflow has since been deleted" swapped one unreachable cause for another. orchestra_workflow_predelete cancels and deletes every instance of a workflow before it goes, which the dead-letter test in this very branch states outright: that route cannot produce the state at all. CurrentStepResolver and getWorkItemNode() now name what does reach them, an unpinned instance whose workflow config went away without the entity delete.

A snapshot does carry a translation overlay

getLabel() justified reading the live node with "a pinned snapshot has no translation overlay", and the test mirroring it said a snapshot "would freeze the base language". testOverlayAppliesCapturedTranslations has asserted the opposite all along.

The real reason is narrower and worth stating: the capture refresh writes the snapshot's latest revision only, so a run pinned to a superseded one would read the wording that revision was cut with. Nothing pinned that, so a new test does - it cuts a second revision, retranslates, and asserts the first still answers in the French it was cut with while the latest follows the translator.

Pruning keeps a live workflow's latest version

retention.md said cron removes each version no instance pins that is neither the current nor the published one. It also skips the snapshot's default revision, the latest, because a revision delete refuses the default. An edited workflow whose runs are all gone therefore keeps the version it cut, forever, and the page promised otherwise on exactly the site that would notice.

The test drives that state. Its count assertion earns its place on its own: strip the default-revision guard and the delete is refused silently while the counter still moves, so the prune would report a deletion that never happened.

The sweep stopped at the versioning claim

Three comments in orchestra_content excused their code with "a legacy duplicate row predates attach()'s one-per-key guarantee" - the same shape as "created before versioning", one word away from the grep. attach() re-points the binding it finds rather than inserting beside it, and pre-1.0 there is no update path.

The code stays as it is; the single-row lookup asks the database for one row rather than loading a set, and the comments now name that instead. What changed is testReattachIsPerKey, which counted the keyed map: the map collapses a key to one entry, so a second row under the same key passed every assertion in the class. It counts the stored rows now.

Three docblocks still said a pinned version can go missing

The resolver and the engine both let the cron sweep skip "a token whose workflow or pinned version is gone", and getPinnedDefinition() returned NULL for a revision that "no longer loads" - the opposite of what the modeler warning above now asserts. What the sweep skips is an unpinned instance that lost its workflow config. The finished run added to testPruneOrphanedVersions is what holds that invariant down, so the docblock states it rather than hedges against it.

Nothing predates the authoring forms

The stall report explained an unusable join timeout as arriving "by config import or predates them". Config import is the whole answer, and only while the form keeps refusing an empty timeout and one it cannot parse - which no test asserted. The two refusals are now checked the way they work: a typed value through validateConfigurationForm(), an empty one through the element's own #required, which is where it has to live because core validates required fields first and keeps one error per element.

Tenants do define a delete-form link

testListBuilderOmitsDefaultTenantDeleteOperation closed on a parenthetical saying they do not, so the parent builder offers no delete op to anyone and the unset guarding the default row is belt and braces. The parent offers the op to every tenant it may; what keeps it off the default row is the access handler. The test only ever looked at the default row, where the answer is the same either way, so it now asserts a non-default tenant does carry the op.

The Off mode is not only a thing runs predate

Three sites in the modeler warning still described an unpinned run as one "started before versioning was turned on". Off is a mode a site can be in right now, and there every running instance is unpinned - which is exactly who the warning is for. The test simulated an unpinned run by clearing the field and never drove the mode that produces one; it does now, through the engine.

Two comments pointed at methods that are not there

FailingLock named WorkflowEngine::acquireJoinLock() for a method that moved to the executor when the two were split, and DelegationInteractionTest named an AssignmentResumer::authorize() the class has never had. Nothing could have caught either, because a comment is not compiled, so the sweep that found them is a test now. It judges a reference only where it can be sure: our class, not shadowed by an import of someone else's class of the same name, and an ancestry that resolves entirely inside the module.

Switching versioning off does not free a run already going

Troubleshooting a stalled join offered "edit the workflow (with versioning off, the change reaches the running instance)". Nothing consults the mode when an instance resolves its definition, only when one starts, so that operator edits and watches the instance stay stuck. versioning.md had it right all along; the two pages now agree, and the mode moving under a run already going has coverage on both sides.

The wording section widened its own scope in its last sentence

versioning.md says the refresh writes the current snapshot and that the instances pinned to it read the correction, then closes on "a wording fix therefore reaches a running instance" with the scope dropped. A run left on an earlier version keeps the wording that version was cut with, and its task label is the exception, resolved live.

And two of my own replacements implied the duplicate they removed

Both attachment rewrites read "newest first, so the first row is the one that answers", inviting the reader to picture the second row the sentence before had just ruled out. The plural reader's order turned out to be worth stating rather than explaining away: the query asks newest first and loadMultiple() returns the ids in the order it was given them, so the map a surface iterates reads newest binding first. That was resting on nothing, so it is asserted now.

A reflow in the first pass also cost a comment its noun, leaving EngineDeadLetterTest reading "nothing but the live can answer for it".

Not this MR

The getProcessInstanceAttachments() name was withdrawn from this issue into #3620621, which merged it as getAttachments() on OrchestraNotificationEvent. Rebased onto that. (AttachmentManager has a same-named method that was never part of that item and is untouched here.)

No behavior changes anywhere: the source edits are comments, plus one documentation page, a shipped string with its French, and a config comment. Everything else is test coverage.