Issue #3620610: Payment: paths that can charge, mis-charge or lose the run

Implements #3620610: thirteen of the fourteen findings, one commit each. Every behaviour change carries a test that was run against the unfixed code and seen to fail on its own assertion; two of the commits are documentation and carry none.

The two high findings

A visitor outcome taken while a payment is in flight ended the run and left the payment live. The step is normally resumed by its own payment's outcome, and by then the payment has reached a real state. Every other way out left it behind: a visitor signaling a configured outcome from a capability link they still hold (browser back after the handoff, an earlier mail), the step's backstop timeout, an operator ending the run. The token was consumed, the run routed on, and a payment the payer could still complete stayed pending, so the provider took money for a step nothing was waiting on. A payment still pending when its token reaches a terminal state is now canceled, and the abandoned checkout is logged where the payer had already been sent to a provider, since money can still arrive for it. The landing page also asked whether a payment was away at the provider only on the branch that had something to pay, while the empty and refused branches render the same outcome buttons; both questions the step's own payment answers now come before the run is priced at all.

shouldSkipLanding() tested the failed state alone, so a payer who cancelled at the provider was sent straight back to the gateway. Kessai records a payer cancellation as canceled, not failed, and the real gateway calls cancelPending() on exactly that, so the one ending the docblock named beside failed was the one the condition missed: with the setting on, every return sent that payer back to the checkout they had just left, with the cancel outcome permanently out of reach. Failed, canceled and expired are now one list, used by the skip-landing decision and the retry notice beside it.

The rest

• Two clicks on "Go to payment" pinned two payments to one token, of which only one is ever settled or released. The click now takes the token's lock across the settled check, the reuse of a pending payment and the create; a second click finds what the first left and reuses it, and one that cannot have the lock says a payment is being processed.
• The settle step captured a re-resolved amount it never bounded by the authorization. Kessai clamps a claim above the hold down to the hold and takes that, and the step recorded "settled" for money nobody collected. Nothing there can authorize the difference, so it is an incident.
• Nothing claimed kessai.canceled on a workflow-pinned payment, though this module produces that event on two paths of its own. Cancellation and partial cancellation now reach the same claim-only handler as a failure and a refund.
• An authorization from an earlier pass through the payment step was never released. Locating the payment now answers with all of them, most recent first, and the settle step releases the ones behind the current one before settling it.
• A non-numeric amount variable was an uncaught ValueError on the payer's page rather than "nothing to pay".
• Two docblocks and a schema comment promised an intrinsic cancel outcome the code never renders. Adding one would be worse than the gap, because an outcome no flow routes dead-ends the run, so the step says what it does and says where a way out comes from.
• The landing page priced the run twice per render, three times on a refused or changed click. The two derived answers share one pricing now; resolve() deliberately does not, and a test pins that.
• Every amount comparison was hard-coded to two decimals and truncates rather than rounds. Two decimals is a real constraint, because kessai stores the amount in a decimal field of scale two, so it is named once and an amount the module cannot carry is refused where it is constructed.
• Four stale names: a settle() the manager does not have, a "finalizer" kessai has none of (twice), a forInstance() that is not the name of the method beside it, and a payment timeout the schema called minutes where the setting takes seconds or an ISO 8601 duration.
• Three tests that would pass against broken code, and the settle step's transient-failure branch, which no stub could reach until this branch adds a gateway that holds funds and is never ready to let go of them.
• The schema said skip_landing was a required key of a payment step and the shipped example omitted it. The type declares itself fully validatable, so its required keys are enforced where config is validated, and the example carries the key.

The decision the issue asked for

A visitor outcome taken with a payment in flight cancels the payment; it does not refuse the signal. Refusing it is the money-safer of the two, but the gate a signal passes is getSignalOutcomes(), which the dispatcher calls on a plugin built from the node's settings alone: it is given neither the instance nor the token, so a payment step cannot consult its own payment there. Refusing would mean widening that contract in orchestra_interaction, which is not this issue's to change. Cancelling is honest about what it can do: it records the payment canceled and announces it, and if the payer completes the checkout anyway, kessai refuses to record the money and says so at error, which is the reconciliation an operator needs either way.

Three of the audit's own claims needed correcting

• The suggested guard would not have fixed the crash. is_numeric() passes " 12", "12 " and "1e3", and bcmath raises a ValueError on each of them; verified on this box. What the module needs is a decimal string of at most two decimals, which is also what kessai can store, so that is the rule and it is stated once.
• The in-flight fix cannot hang off the step's resume. The engine claims a parked token with a guarded UPDATE that fires no hooks, so the first save anything can see is the token reaching a terminal state, which is also the moment that means the step is over for good.
• Three tests, not two, would have passed against broken code, and three findings (the dead code, the vacuous tests, the untested branch) were filed as one-liners with no detail, so what they name here is what a read of the module found: the stale names above, the three tests, the transient branch, and the missing schema key.

Not done here

One finding is left out: the mark paid/failed route is the only token-drive route without the tenant access check. Its two halves both live outside this module: the check is orchestra_ui\Access\InstanceTenantAccessCheck, which an orchestra_payment route cannot name without depending on a module it is only softly integrated with, and the operations are rendered unfiltered by OrchestraUiController. Another change owns that file, so the finding stays open rather than being half-fixed from here.

What a read of the branch itself then changed

Reviewing it before merge found three defects in its own first pass, each now fixed with a test that fails against the commit that introduced it:

• The cancellation took the first pending payment and left any others live, though a declined-and-retried step leaves a row per attempt, which is exactly why the reaper's sweep of the same pin reads them all.
• It fired on a dead-lettered token too. That state is terminal but not over: an incident can be retried, which puts the token back to active, so the payer's checkout has to still be there.
• The shared pricing was private, so it reached only resolvers that leave both derived answers to the base class. The shape that gains most from it is the one that overrides preview() to show an order summary, which is what a real domain resolver does.

It also found that the branch had bought its correctness with queries, so two commits give them back and then some:

• Ending any step meant a payments query per token, for every workflow on the site. Only this step writes the pin, so its node is read from the definition the engine is already holding, and the payments table hears nothing about the tokens of every other kind of step.
• One landing render asked the same handful of rows three separate questions. The step reads its payments once now and answers all of them from that, which is fewer queries than before this branch, not more; the click repeats the read exactly once, inside the lock, because what another click left is the whole reason the lock is there.

And docs/payment.md needed the same sweep the code got: it told the reader that a late provider callback "is a harmless no-op", which is the belief this issue starts from.

Testing

Every new assertion was run against the unfixed code first and seen to fail, one class at a time, and each class touched was green with the fix in place. phpcs (Drupal and DrupalPractice), cspell, phpstan at the project's level 5 and the translations check are green locally; the Functional JavaScript class runs on CI, this checkout has no rig for it.