Detect a second successful capture for one period after a dunning retry, and record remote ids per attempt

Design pass against 1.0.x at 2b5af13, with #3630967: Lock the cycle per subscription, stop re-enqueuing past-due subscriptions, and refuse a pending charge result without a remote id through #3630970: Make refunds idempotent, guard refund resolution, cap and reconcile reversals against pending refunds, and carry rated and tax lines into re-bills in. The gap is as the summary describes, and reading the paths turned up two more shapes of it and one cheap way to close most of the window before any duplicate exists. Details, then the decisions.

Three shapes of the same gap

Setting: attempt 1 of a period charge is submitted, the connector throws every time, the sweep exhausts its re-submissions and failUnknown() marks the record failed with no remote id. Dunning schedules a retry, which claims attempt 2 and a new idempotency key. The processor did receive attempt 1 and completes it later.

1. Late success before the retry runs. The webhook for attempt 1 arrives while the record is still failed. resolveCharge() refuses anything not submitted, so it logs "already in state failed" and drops it. The retry then runs with attempt 2 and the customer pays twice. This is the shape nothing in the summary mentions, and it is the easiest to close: accept the late success. The state machine gains failed → succeeded, and the resolution settles the record, advances the anchor, and calls markRecovered(), which clears next_retry, so the queued retry item finds nothing to do (DunningRetryWorker re-checks it). No second capture ever happens.
2. Late success after the retry succeeded. The record is succeeded through attempt 2. The webhook for attempt 1 is logged as a replay, "already in state succeeded", at warning, and that is the last anyone hears of the second capture.
3. Late success while the retry is pending. The record is submitted under attempt 2 with attempt 2's remote id. The webhook for attempt 1 is taken as attempt 2's success: the record settles with the wrong remote id, and attempt 2's own webhook is then dropped as a replay. Two captures, one recorded, and the recorded id is the wrong one.

Shapes 2 and 3 cannot be prevented, only detected: by the time the second success arrives the money has moved. Detection needs the engine to know which attempt a resolution belongs to, which is what the summary asks for.

Per-attempt bookkeeping

data['attempts'][$n] on the charge, no schema change: remote_charge_id, state (submitted, unknown, succeeded, failed, given_up), submitted_at, resolved_at and the last error. Written by submit() when an attempt goes out, by applyChargeResult() and recordUnacknowledged() on the synchronous answer, by failUnknown() when the sweep gives up, and by resolveCharge().

The remote_charge_id base field stays, as a projection rather than a second source: the map is the record of every attempt and its id, and the field is derived from it by the engine alone, holding the id of the attempt that settled the record, or of the attempt in flight until one does, and never an id the map does not hold. It stays because it is queryable and the map is not: a connector's webhook handler finds the charge by the processor's id with an entity query on it, the listing and the Drush reports read it, and the sweep tells an acknowledged submission from a lost one by whether it is set.

resolveCharge() learns which attempt it is resolving

resolveCharge($charge, $success, $message = NULL, ?string $remote_charge_id = NULL, ?int $attempt = NULL). Both new parameters optional, so existing connectors keep working with today's semantics, but the connector catch-up asks every connector to pass the processor's charge id from the webhook, which it always has. Attribution: a remote id that matches a recorded attempt names it; a remote id matching nothing is attributed to the current attempt when that attempt has no id yet (it was never acknowledged, so this is its acknowledgement) and recorded there; an explicit attempt is taken as given; neither means the current attempt, as today.

Then the rules. A resolution for the current attempt of a submitted record behaves exactly as today. A success for any attempt of a failed record is shape 1: accepted, settled, recovered. A success for an attempt when the record is already succeeded, or is submitted under a different attempt that later also succeeds, is a duplicate capture: the attempt is recorded as succeeded with its id, data['duplicate_capture'] is set with both attempts, their ids and the amount, a critical log line is written, and a new ChargeEvents::DUPLICATE_CAPTURE (ChargeDuplicateCaptureEvent: charge, subscription, amount, the two attempts) is dispatched so a site can notify or refund. The record's state and the anchor do not change: the period was paid once and the books stay right about the period; the duplicate is money to return, which #3630970: Make refunds idempotent, guard refund resolution, cap and reconcile reversals against pending refunds, and carry rated and tax lines into re-bills's refund path can now do, by hand or from the event. Shape 3's first success settles the record under the attempt that actually succeeded, with that attempt's id, so the second success is the one flagged. A failure for a non-current attempt is only recorded; a replay of the same outcome for the same attempt is ignored as today.

No automatic refund in this issue, as the summary recommends: a duplicate capture against a card is often reversed by the processor's own duplicate detection or disputed by the customer, and refunding on top would double the credit. The flag, the event and the listing make it a decision an operator or a site takes with the processor's dashboard open.

Surfacing

The admin charge listing's state column reads "succeeded (duplicate capture)" for a flagged charge, and a new drush subscription_manager:duplicate-captures lists flagged charges with both remote ids, so an operator can find them without reading logs; stale-charges is unchanged, since a flagged charge is not stale. The optional Views listing is left alone.

Tests

• Shape 1: exhaust the sweep, then resolve attempt 1 as succeeded by remote id before the retry: record succeeded with that id, anchor advanced once, dunning recovered, next_retry cleared, the queued retry item drops, one connector call in total.
• Shape 2: retry succeeds synchronously, then attempt 1 resolves: state and anchor unchanged, both attempts recorded with their ids, the flag set, one critical line, the event with both attempts; a replay of the same webhook does nothing more.
• Shape 3: retry pending, attempt 1 resolves first: record settles under attempt 1 with attempt 1's id, anchor once; attempt 2's success then flags the duplicate.
• Attribution: an unknown remote id on an unacknowledged current attempt is recorded as that attempt's id; a failure for an old attempt is recorded and changes nothing; the existing replay test keeps passing; a resolution with neither parameter behaves as before.
• Listing marker and the Drush listing's rows.

Change record and connectors

One change record: the two optional parameters on resolveCharge() and the request that connectors pass the remote id; failed → succeeded for late acknowledgements; the attempts bookkeeping; the duplicate flag, event and Drush command. The connector catch-up template gains the resolveCharge() line.

Decisions to ratify

1. Per-attempt bookkeeping under data['attempts'] is the record; the remote_charge_id field is a projection of it written only by the engine, kept because it is queryable.
2. resolveCharge() gains optional $remote_charge_id and $attempt; attribution by remote id first, explicit attempt second, current attempt by default; connectors asked to pass the remote id.
3. A late success for a failed record is accepted: new failed → succeeded transition, settle, recover, the pending retry drops. (Addition to the summary; it prevents the duplicate in shape 1.)
4. The first success settles the record under its own attempt and id; a later success for another attempt is a duplicate capture: flag, critical log, ChargeEvents::DUPLICATE_CAPTURE, no state or anchor change, no automatic refund.
5. A failure for a non-current attempt is recorded only.
6. Listing marker and drush subscription_manager:duplicate-captures; Views untouched.
7. Tests as listed; one change record; connector catch-up line for resolveCharge().

Implemented on the MR in four commits per the ratified decisions: the attempt bookkeeping and resolution, the surfacing, tests, docs. One correction made during implementation, noted below; otherwise no deviations.

What changed

• Per-attempt bookkeeping (decision 1): recordAttempt() writes data['attempts'][n] from submit(), applyChargeResult(), recordUnacknowledged(), failUnknown() and the resolution paths, with state, remote id, UNIX timestamps and last error. The remote_charge_id field is written only alongside the map, as the projection the design describes.
• Attribution (decision 2): resolveCharge() gained ?string $remote_charge_id and ?int $attempt; attributeResolution() applies the rules in the order ratified: explicit attempt, then a matching recorded id, then the most recent unacknowledged attempt for an unknown id, else the attempt in flight.
• Late acknowledgement (decision 3): failed → succeeded added to ALLOWED_TRANSITIONS; a success for a given-up record settles it through the shared settleResolvedCharge(), which advances the anchor and calls markRecovered(), clearing next_retry so the queued retry item drops.
• Duplicate capture (decision 4): a success for an attempt other than the settled one, when the record is already settled, goes to recordDuplicateCapture(): the attempt recorded, data['duplicate_capture'] set, the charge added to a State list, a critical log line, ChargeEvents::DUPLICATE_CAPTURE dispatched. State and anchor untouched, no refund. Shape 3 settles under the first attempt to succeed, with its id, so the retry's own success is the one flagged.
• Late declines (decision 5): recorded on the attempt at info; a repeat is ignored as a replay.
• Surfacing (decision 6): the listing's state cell reads "succeeded (duplicate capture)" until cleared; drush subscription_manager:duplicate-captures lists flagged charges with both ids and --resolve clears one, stamping cleared_at on the flag. The review list lives in State under CycleEngine::DUPLICATE_CAPTURES_KEY, since the data map is not queryable; the engine gained the state service.

Correction during implementation

The design said an unknown remote id is attributed to the most recent attempt without one. The first cut fell through to the attempt in flight when every attempt already had an id, overwriting its recorded id and resolving it on a stranger's outcome: a stray webhook carrying an unrelated id would have settled or failed the charge. It is now ignored with a warning, and the test testUnknownRemoteIdAttribution() pins both halves: a stray id on a fully acknowledged charge changes nothing, and the first id to arrive for an unacknowledged attempt is its acknowledgement.

Tests

New DuplicateCaptureTest, seven tests, each starting from a submission the sweep gave up on (re-submission limit set to one to keep them short):

• shape 1: the late success settles, the anchor advances once, dunning recovers, the retry is dropped, two connector calls in total, the notice logged, no flag;
• shape 2: retry succeeded synchronously, late success for attempt 1: record and anchor unchanged, both attempts recorded with their ids, the flag with settled attempt 2 and duplicate attempt 1, the State list, one critical line, one event; a replay adds nothing; clearing removes it from the list and stamps the flag;
• shape 3: retry pending, attempt 1's success settles under attempt 1 with its id; attempt 2's success is then flagged with the anchor unmoved;
• a late decline for an old attempt is recorded only, and its repeat is a replay;
• the stray-id and first-acknowledgement cases above;
• a resolution without either parameter is about the attempt in flight;
• the listing marker appears and disappears with the flag.

ChargeRecordTest::testTransitionGuard() now lists failed → succeeded as legal. Kernel suite: 260 tests green, the one pre-existing deprecation; Functional suite green (6 tests) in the clean codebase.

Change record

Drafted alongside: the two optional parameters with a before/after and the request to pass the id, the transition, the bookkeeping and the projection rule, the flag, event and command. The connector catch-up template gains the resolveCharge() line.