Payment: a second checkout can open while the first is still live

Fixes the four findings on #3621291, plus the defects that four adversarial audits of this branch found in the fixes themselves.

The double charge, and the two ways the first fix went wrong

PaymentInteraction::pay() looked up the payment already pending on the step and threw it away whenever the click carried no displayed amount:

$pending = self::findFirst($payments, [PaymentInterface::STATE_PENDING]);
$pending = $shown === NULL ? NULL : $this->keepReusablePayment($pending, $shown);

Only the reuse question was meant to be deferred, since the branch 76 lines below already asks it once the run has been priced. With $pending already NULL, startPayment() minted a second payment and a second live provider checkout while the first was still completable.

Audit 1 found that simply keeping it was not enough. getPayerWindow() deliberately closes a payment's window inside the step's own timeout, so "lapsed payment, step still parked" is a state every unfinished checkout passes through. Reusing a lapsed one is a redirect loop: the handoff refuses it and returns the payer here, which hands them back to the handoff.

Audit 2 found that declining to reuse it, and leaving it pending, was worse still. The replacement gets a fresh window while the lapsed row has none, so the reaper expires the lapsed one at the next run, and expiring it resumes the pinned token, which cancels every payment still pending on the step, including the replacement the payer is completing at the provider. They pay, kessai refuses to record it, an operator is told to reconcile by hand, and every further arrival mints another one.

The rule that survives both:

The pending payment	What the step does
Same total, window open	Reuse it: the handoff returns the payer to the same provider session
Lapsed, never reached a provider	Cancel and replace: nothing can arrive for a checkout nobody was sent to
Lapsed, payer reached a provider	Stop the pay path: show the in-flight page and let the reaper reconcile

The third case is the landing page's own findInFlight() rule, applied on the path that skipping the landing page routes past. It runs before the checkout is opened and before anything is cancelled.

The currency

SettleTask::execute() compared the re-resolved capture amount against the hold but never against the payment's currency, so a settle priced in another currency had its number captured in the one the payer authorized. Audit 2 then found that comparison was strict on a value nothing validates, so Payable upper-cases it once at construction, covering the settle comparison and the pay step's shown-amount check together rather than at each site.

Audit 3 found that normalized one side of a comparison and left the other. The shown amount and the priced amount meet as whole formatted strings, currency included, and the shown side is built from the preview, which kept whatever its resolver wrote. A resolver that overrides preview() supplies its own currency literal, and that override is the shape documented for a real domain resolver. Written in lower case there, every payer clicking "Go to payment" was bounced back to the landing page saying the total had changed, on every attempt, forever, since the new total shown was the same string, and each pass cancelled the pending payment the last one left. PayablePreview normalizes the same way, so the two sides meet.

It also found the currency guard sitting before the zero-capture branch. A zero capture takes nothing: it releases the authorization. There are no two amounts to compare and nothing to convert, so refusing it left the payer's money held on their card until the provider expired it, where before the guard existed it was released and the run advanced. Reachable with the card-verification payable the Payable class docblock itself describes. The comparison is now gated on there being a capture, and the branch below reads the same answer back instead of recomputing it.

Two smaller ones

• The $timeout docblock claimed the subscriber's window and the payer's can never drift. True for a payment the click creates; a reused one keeps its own click's deadline. Documentation only.
• CheckoutRefuser::getAskedAmounts() returned a count, not an amount, and nothing read it. A rename plus a new assertion that the refuser subscriber itself was reached, not merely that the event fired. Reverting the rename errors on an undefined method rather than failing an assertion, so this is not a red-to-green fix either.

The fourth audit: the pinned payment was judged by the wrong thing

Audit 1 above left the pinned payment alone whenever the amount claim could not be read, on the grounds that such a claim says nothing about what the visitor was shown. It does not, and the cancellation that guard skips never consulted the claim either. It compares the pinned payment's own amount with what the frozen subject prices, and both of those are known whatever happened to the signature.

So the guard's only reachable effect was to leave a payment asking a total nobody prices any more PENDING, with the handoff link the payer already holds still live at that total: on a direct-sale gateway, that checkout captures the superseded amount and the settle step passes it straight through. It also left two sentences claiming the two cases are treated alike when they no longer were: testTamperedFigureIsRefused's docblock and docs/payment.md both say an unsigned claim is treated exactly like a total that moved.

The question is now asked once, from the amount the run prices, and skipped only where the answer is already in hand: a claim that agrees with the price asked this same question before the checkout opened. That folds the deferred skip-landing cancel and the changed-total cancel into the one call site they were always two spellings of.

The same audit found the settle half of the currency normalization untested. testTheLowerCasePreviewCurrencyStillPays goes red without it, but that pins the pay step's shown-amount check; the settle comparison the normalization was actually written for had nothing. The settle step now has the pair.

What else it changed

• The currency rule is named once. Two constructors called strtoupper(), which is the shape that let one side of a comparison drift in the first place, and each then spent a property docblock and a @param restating why. Payable::normalizeCurrency() states it beside isAmount() and SCALE, which PayablePreview already shares rather than restating.
• The documentation says what the settle step refuses. The over-hold refusal has a paragraph of its own; the currency refusal, written against exactly the configuration that paragraph describes, two resolvers configured apart from each other, had nothing, so an operator meeting that incident had no page to read it on. The lapsed-checkout behaviour is now in the section that already explains why the payer's window closes inside the step's, and the outcome list has the expired it was missing, which a workflow author had to find in the code.
• One home for reading a payment back from storage. The branch added inline copies of it to assert what a step did to a payment, next to a reloadPayment() that already existed in ParkedPaymentTrait. Counting the module's tests found four definitions of the same three lines beside three inline reads. ReloadPaymentTrait states it once, keeps the assertion the others lacked, and is composed into both fixture traits, so no class that uses them changes.
• A comment gave a reason the next paragraph retracted. The pinned-payment comment justified cancelling before the opening event with "a subscriber holding the subject would hear it as this checkout ending", then said two paragraphs later that a cancellation inside the bracket is safe because this module claims its own. Only one can be the rule.
• OUTCOME_EXPIRED's docblock named the backstop timeout action expire_payment. There is no such plugin: it is payment_lapsed, and that docblock was the only occurrence of the other word anywhere.

The fifth audit: the lapse was never what made a cancellation unsafe

The guard above stops the pay path for a lapsed checkout at a provider, because money may already have been taken for it, so it can be neither reused nor canceled nor replaced. None of that reasoning is about the lapse. It is about being at a provider, and the branch left the other half of it doing exactly what it forbids: a live provider checkout whose total has moved under the payer was canceled and replaced.

What that costs, checked against kessai rather than assumed. cancelPending() is a compare-and-set out of STATE_PENDING, so a payer who then completes the checkout they still have open lands an authorization on a CANCELED payment: transitionUnderLock() returns NULL, no state changes, no event fires, and nothing anywhere logs it. The payer is charged and the site keeps no record at all. kessai_worldline, the one shipped handler that records an authorization from a provider round-trip, discards the return value, so it does not notice either.

Reachable with the shipped orchestra_payment_variable resolver, whose amount lives in a process variable that nothing freezes for the duration of a checkout: any later step, action or operator can move it while the payer is away. On a skip-landing step that arrival is routed straight to the pay path.

So the question is "can this run still reuse it", asked wherever a total is known: before the checkout opens where the click carries one, and again after pricing where it does not. canReuse() states that rule once, which also collapses keepReusablePayment()'s two cancel branches into one, and what reaches that method is now only a payment nobody was ever sent to. What makes that airtight, whatever a token carries, is that both lookups read the same row.

It is not a new way for a run to hang: show() already waits on any checkout at a provider with no test of its deadline, so this makes the two paths agree on what cannot be continued while leaving the pay path free to hand back what can.

What else the last rounds changed

• The two places the rule is asked are both tested. Reverting the earlier one showed it is not the economy its comment claimed: the later refusal sits behind a claim that disagrees with the price, so the ordinary successful click never reaches it, and the cancellation just below closes the payer's open checkout and hands them a second one. The comment says so now.
• The settle half of the currency normalization was unpinned. testTheLowerCasePreviewCurrencyStillPays goes red without it, but that pins the pay step's shown-amount check; the settle comparison the normalization was written for had nothing.
• One home for reading a payment back from storage. The branch added inline copies next to a reloadPayment() that already existed in ParkedPaymentTrait; the module's tests held four definitions of the same three lines beside three inline reads. ReloadPaymentTrait states it once and is composed into both fixture traits, so no class that uses them changes.
• The currency rule is named once. Two constructors called strtoupper(), which is the shape that let one side of a comparison drift in the first place. Payable::normalizeCurrency() sits beside isAmount() and SCALE, which PayablePreview already shares rather than restating.
• The documentation says what the settle step refuses, what a payer sees when they come back with a checkout still open, and has the expired and no_payment outcomes its list was missing. A payment_lapsed timeout action was documented under a plugin id that does not exist.
• Two comments gave reasons the code retracted, one of them by claiming a cancellation before the opening event was needed for a reason the next paragraph withdrew.

What is pinned red-to-green

Ten tests, each run against the code without its own fix:

Test	Result without its fix
testSkipLandingReusesThePaymentAlreadyPending	And handed off with the payment already pinned, not a second one. / Failed asserting that 2 is identical to 1.
testLapsedProviderCheckoutStopsThePayPath	The payer was not handed off anywhere., a RedirectResponse where the in-flight page was owed
testLapsedPaymentThatNeverLeftIsReplaced	And the dead one is closed, since nothing can arrive for it., and it was left pending
testCaptureInAnotherCurrencyRaisesAnIncident	Nothing was captured., with 40.00 of the payer's euros captured against a USD price
testTheLowerCasePreviewCurrencyStillPays	The link carries the currency in the case both sides compare in., where it carries 12.00 eur, and the click is then refused as a changed total
testZeroCaptureInAnotherCurrencyStillReleases	the payer's authorization is left standing and an operator is raised an incident (against a targeted revert; the whole-diff lane removes the guard entirely, so it passes there)
testTamperedFigureStillClosesTheSupersededPayment	The payment asking the superseded total is closed, not left completable. / Failed asserting that two strings are identical. -'canceled' +'pending'
testLowerCaseCurrencyCapturesWithoutAnIncident	The capture went through. / Failed asserting that two strings are identical. -'captured' +'authorized' (against a targeted revert of the normalization; the whole-diff lane removes the currency guard along with it, so there is nothing left to refuse a lower-case code and it passes there)
testLiveProviderCheckoutAtAnotherTotalStopsThePayPath	The payer was not handed off anywhere. and the redirect names /kessai/pay/2: a second checkout, opened while the first was canceled under the payer
testLiveCheckoutStopsTheClickBeforeAnythingIsFrozen	the same, on the road the later guard cannot answer

Every message above is the one the lane actually printed, read back off job 12037876 rather than written from memory. Each case is pinned against a targeted revert of its own guard, since the guards sit in one file and reverting all of them at once does not say which assertion belongs to which. Checked that way, all three of the reuse and lapse cases go red (the runner's summary line prints only the first two failures of a class; the per-class log has three).

testZeroCaptureInAnotherCurrencyStillReleases prices through a resolver that returns the card-verification shape rather than the shipped variable resolver, which answers NULL below a positive amount, which would skip the guard for a reason having nothing to do with the capture being zero, and the test would pass either way. The first version of it did exactly that, and the targeted revert is what caught it.

The reuse fixture now carries a real expiry, because kessai stamps one on essentially every payment: without it the headline fix was covered only for a shape a configured site does not produce.

Verified

• phpcs on the module root at CI's extension list: Drupal 0/0, DrupalPractice 0/0
• PHPStan level 5: no errors
• scripts/check-translations.php: exit 0
• cspell against the CI config plus the project word list: no word new to this branch
• test-only changes played on pipeline 952096: eight of the ten go red there, each naming its own assertion, no errors. testTamperedFigureStillClosesTheSupersededPayment reads 'pending' where 'canceled' is owed, and testTheLowerCasePreviewCurrencyStillPays reads 12.00 eur on the link. The two that pass on that lane are the two whose guard the whole-diff revert removes outright, both noted in the table.
• Pipeline 952096 green on every lane, phpunit (next major) and phpstan (next major) included, both played by hand since composer (next major) gates them.
• phpunit (next major) and phpstan (next major) played and green. They had been SKIPPED on every pipeline of this branch, because composer (next major) is a manual lane, so the green badge had not been covering them.
• Impacted kernel classes green: SettleTaskTest, CheckoutRefusalTest, PaymentInFlightTest, ConcurrentPayTest, PaymentRouteOutcomeTest, CheckoutOrderingTest, PaymentWorkflowSubscriberTest, the set that touches the changed files or the two test resolvers, plus the four the shared reload trait reaches: StoredCardCleanupTest, PaymentLapsedTimeoutTest, PaymentOperationsTest, PaymentOperationsWithoutUiTest

Two the review of this branch turned up itself, neither a behaviour change

• buildInFlightPage() marks itself uncacheable, and its four other call sites return it bare because of that. The one added here wrapped it again, which reads as if the builder does not, and a reader acting on that, by moving the marking out to the callers, silently makes the four bare ones cacheable and replays "your payment is being processed" to a payer whose payment has since settled.
• PayablePreview assigned its normalized currency before its own validation while Payable assigns after, in two constructors whose docblocks each point at the other as the reason they are written that way.

Note for a site already running the previous alpha

A payment left pending with a lower-case currency stored on it no longer matches the shown amount, so the next click cancels it and opens a fresh checkout. Nobody is charged twice, since that is the same path a changed total takes, but it is worth knowing if an upgrade lands while a payer is mid-flight. Pre-1.0, and reinstalling clears it.