Issue #3620614: Interfaces and docblocks that promise what the code does not do

Fixes all eleven findings in #3620614. The high-severity one is not what the issue says it is, and that correction is the most important thing in this branch.

The collision is real; the mechanism first described was not

The summary said a class could not both use OutcomeConfigTrait and implement WorkItemPresentationInterface, because PHP refuses the incompatible getOutcomeLabel() at compile time. It does not: a class's own declaration wins over a trait's, and a class implementing an interface must declare the method, so the refusal never arises for the only shape anyone writes. Proven by running it, not by reading it — and my first attempt at proving it omitted the class's own method, which is the one shape that does fatal and is not a shape anybody writes. The issue summary has been corrected and the finding regraded to med, so this branch fixes nine med and two low.

What is true is smaller and still worth fixing before a beta freezes it: the trait carried a two-line convenience adapter under the name the published interface needs, on every class that reaches the trait, which is every human task type. Such a class compiled and silently lost the helper — one name meaning two different things depending on where it was read, on an @api surface.

So the trait exposes what it actually owns, getOutcomeLabels(): array, the decoded map from its own configuration. Resolution stays in OutcomeConfig::label(), presentation stays on the interface, and the callers read the pair. UserOperationForm's private helper was that same cast plus a delegation, and is gone.

The interface had one consumer and no implementation anywhere, so its contract had never run. The task-family fixture takes it up, which makes both branches testable for the first time: the plugin answering for the outcome it claims, and deferring to configuration for the rest.

getOutcomeLabels() is now genuinely the only decoder: the issue named three hand-written reads of outcome_labels, and the first pass here retired two of them. The third, CommentForm, is retired too.

The rest

A dead validator, weaker than the accessor everyone else uses. InteractionToken::isValid() compared only the instance, so it answered true for a token-scoped grant naming a different branch. It had no callers; it is gone, and its test moved onto open(), which is what the code actually uses.

The audit trail blamed the recipient for a reassignment. actor held the person reassigned to, while the other two sites use that field for the account that acted — so the trail read as though the recipient had moved the task onto themselves. reassign() now takes the acting account, and omits actor entirely when nobody acted (an escalation timer reassigns on the clock).

retry meant two different things on one node, under three labels: the node feature's advance-retry policy, and the subprocess's re-launch budget. The subprocess side is now relaunch_attempts, "Re-launch attempts", everywhere including the schema.

Four surfaces a beta would have frozen as they are. ProcessInstanceInterface::setStatus() was an unguarded trap with no production caller — a submodule taking the promise up would write every base field from a stale snapshot and could resurrect a cancelled run; it is removed, and the two tests that used it go through orchestra.state_transitions. CompositeConditionBase instantiated child conditions unguarded, so evaluate() threw where its contract says it answers a bool. TokenParkedEvent now says it is dispatched inside the advance transaction, as both sibling lifecycle events already did. SourceProviderInterface, SourceView, TokenParkedEvent and WorkItemEvent carry @api, being the documented cross-module extension points.

The engine mutated work-item state off OrchestraAuditableEvent, which three sibling docblocks declare is "for recording, not acting". Any third-party recording listener that stopped propagation silently left every resumed task showing as claimed. The engine now dispatches TokenResumedEvent, mirroring TokenParkedEvent, and the completion subscriber listens to that.

The @api surface now says what it promises. Every interface and trait in src/ declares @api or @internal: the status vocabulary and the notification context shipped unmarked, so nothing said whether a beta freezes them. And PluginSequenceFeatureBase, an @api extension point, imported an @internal trait — publishing "not a public extension point" mechanics to every subclass. It imports them privately instead, which PHP enforces rather than merely documents; no subclass called them.

Two getters were named without get, and one of them is worse than a style slip: status() is the name ConfigEntityInterface already uses for the enabled flag, while StatusInterface extends that very interface — one word, two meanings, in one feature. InstanceStatusProviderInterface::status() becomes getStatus(), matching the getStatuses() that already sat beside it, and StatusHistory::history() becomes getHistory(). DelegationsController's private status() returns a label and now says so.

Also: four docblocks carried [[wiki-link]] syntax that phpDoc does not render, and several paragraphs a previous edit had left unwrapped mid-sentence.

What the tests prove

Three fixes have a test seen to fail against unmodified code, each failing on its own assertion rather than incidentally:

• the reassignment actor — Failed asserting that 2 is identical to 1, the recipient's uid where the operator's belongs;
• the resume event — Failed asserting that two strings are identical, a resumed task still showing as claimed once a listener stops propagation;
• the composite-condition guard — PluginNotFoundException: The "orchestra_gone_with_its_module" plugin does not exist, thrown out of createInstance().

That last one is driven on the plugin rather than through a saved workflow on purpose: config schema rejects an unknown plugin's settings, so the config route fails at validation without ever reaching the guard. A test that fails for the wrong reason proves nothing.

The getOutcomeLabel() work is a contract test, not a regression test, and is described as one: the interface had no implementation anywhere, so the fixture and its test exercise an @api surface that had never run. Do not expect the test-only job to fail for it.

The remaining changes are contract and documentation changes with nothing to observe: the removed setStatus() and isValid() (their callers and tests moved), the @api and @internal markers, the retry rename (its round-trip tests were updated), and the private trait import, whose enforcement is PHP's own — Call to private method Base::m() from scope Sub.

One claim withdrawn

An earlier commit here says the finder dropped an outcome label authored through a modeler. It does not, and I could not reproduce it. getNestedConfigKeys() derives the structured keys from the plugin's own defaults, where outcome_labels defaults to an array, so a modeler import retypes it back before the definition is saved and no reader ever sees the flat string. Reading through the shared decoder instead of a bare (array) cast is still right — every other reader of a structured setting does — but it repairs nothing, and the code comment now says that rather than claiming a bug.

Reading the red test-only changes lane

It fails, and only three of those failures are proof. The three above are. Everything else in that lane is a revert artifact of a rename: StatusHistory::getHistory() and InstanceStatus::getStatus() are "undefined method" errors because the tests call the new names against reverted source, and TaskConfigRoundTrip, PluginSettingsForm and two SubprocessTest cases fail on the renamed relaunch_* keys for the same reason. A rename cannot be proved by that lane; it is proved by the suite passing on both core lanes with every caller moved.

Second pass

I audited this branch before asking for review, and it found seven things worth fixing, which the last four commits do.

The rename was superficial. It reached the config keys, the interface methods and the docs, and left every explanation of them saying retry: the schema comment above the key it types, the help text under the renamed field ("0 disables retry", beneath a label reading "Re-launch on failure"), twelve docblocks and comments in the coordinator plus armRetry() itself, two on the interface, and the test's method names, workflow ids and messages. SubprocessTest's docblock claimed the node "sets retry: 2" while its fixture sets relaunch_attempts — a docblock describing code no longer beneath it, which is this issue's own subject. Worse, the docs had been corrected to "disables re-launching" while the string they quote had not, so a change meant to unify the vocabulary left the two disagreeing where they had agreed. The word stays where it is the engine's own, including the poison child's retry.max_attempts in the very same fixtures, which is the other meaning and the whole reason two words are needed.

UserOperationForm hand-rolled OutcomeButtonsTrait::buildOutcomeButtons() — the same loop down to the element keys — while that trait, which this branch marks @api, claims in its own docblock to be the single place those buttons are built "for the comment form, the operation forms and any other surface". It uses the trait now, and decodes the labels once rather than once per outcome. Its use FlatConfig had also gone in out of alphabetical order; phpcs has no ordering sniff, so nothing would have caught it.

Two published types said nothing about themselves. StatusMilestoneValue is the declared return of the newly-@api getHistory(), and OutcomeConfig is where the trait sends an implementor to resolve a label. Both are @api now. The concrete services and storage schemas in src/ remain unmarked — 26 of them, mostly implementations whose right marker is @internal; classifying those is the sweep's job (#3620621), not this branch's.

Five shipped docblocks narrated their own repair — an event "the engine has always had while this one was missing", a field that "used to be the person reassigned to", a transaction the docblock "did not" mention. A contrib author reading an @api docblock at 1.0 does not need the pre-release history, and this project's docs describe the current design. It is in the commits and here instead.

And cspell was red under a green pipeline badge, on docblocks in three new docblocks — the lane is allow_failure, so the badge said success. Two of the three sentences are gone with the rewrites above and the third reads better without the word, so nothing was added to the project dictionary.

The API breaks, in one place

Every rename and removal here lands on an @api interface, which is the point of doing them now. Collected for the beta-prep freeze:

Was	Is
ProcessInstanceInterface::setStatus()	removed; write through orchestra.state_transitions
InstanceStatusProviderInterface::status()	getStatus()
StatusHistoryInterface::history()	getHistory()
SubprocessTaskInterface::getMaxRetries()	getMaxRelaunches()
SubprocessTaskInterface::retryBackoff()	relaunchBackoff()
WorkItemManagerInterface::reassign($task, $uid)	reassign($task, $uid, ?int $actor = NULL) — callers unaffected, implementors are not
OutcomeConfigTrait::getOutcomeLabel($value)	getOutcomeLabels(): array, freeing the name for the interface
PluginSequenceFeatureBase, three inherited InnerPluginSettingsTrait methods	imported privately; a subclass that called them no longer can
node config retry, retry_backoff	relaunch_attempts, relaunch_backoff

InteractionToken::isValid() is also gone, and had no callers.

The config keys are stored node settings with no update path, per the pre-1.0 reinstall-only policy: a subprocess node configured before this lands reads a re-launch budget of 0 afterwards, so it resumes its parent on the first failed child instead of re-launching. Nothing on the development site is affected — no workflow there configures a subprocess node.

Not done here

The docblock reflow was limited to the files this branch already touched. A detector run across the module finds roughly 120 paragraphs with the same mid-sentence wrap damage; reflowing those is prose work for the sweep issue (#3620621).