ADR-059: Guard invoice mutations while a payment is in flight

Date: 2026-06-22 Status: Accepted

Summary

A payment is in flight when its invoice’s payment_status is processing or unknown, so the captured amount is not yet known. While a payment is in flight, Void, MarkUncollectible and RecordOfflinePayment reject the invoice with a 409, and every check uses one predicate, IsInFlight(). There is one exception, added by ADR-107. A parked invoice is unknown with no PaymentIntent id, so its payment can never be identified. It may still be written off with MarkUncollectible. Dunning’s manual resolve now voids only through invoice.Service.Void, so it gets the same check, and it cancels the PaymentIntent only after the void succeeds. The automatic clawback (crediting unused service after a cancel or downgrade) cannot return an error to a person. While the payment is in flight, it keeps its credit note as a draft. A scheduled job issues the draft once the payment settles. If the invoice is instead voided or written off, the draft is voided, because an invoice that collected nothing has nothing to give back. Without these rules, Hardcount could leave captured money on a voided invoice, record too small a paid amount, or reverse tax on a completed sale. Simply skipping the clawback would instead over-charge the customer.

Context

An end-to-end audit asked: which paths mutate an invoice while its payment is “in flight”? — where in flight = payment_status ∈ {processing, unknown}: the charge is open at the provider (processing — e.g. off-session SCA requires_action) or its outcome is genuinely ambiguous (unknown — a 5xx/timeout that may or may not have captured). In both, the captured amount is not yet known. The audit found two distinct classes of bug, neither guarded.

Class 1 — status-strand (operator-reachable today). Three status writers flipped status + reversed tax with no payment_status check:

• invoice.Service.Void — voiding an invoice whose charge later succeeds strands captured money on a voided invoice and reverses tax on a sale that completed.
• invoice.Service.MarkUncollectible — MarkPaid is allowed from uncollectible (the “wrote it off but paid after all” recovery), so reversing tax + flipping to uncollectible, then the charge settling, leaves the tenant under-remitting output tax on a real collected sale.
• dunning resolveRun (manually_resolved) — called the raw store UpdateStatus(Voided), bypassing service.Void entirely: a second, less-guarded void writer that reversed no tax and emitted no invoice.voided event. Plus RecordOfflinePayment guarded only processing, not unknown (double-collect window).

Class 2 — amount-drift (“Part B”). The automated ADR-050 unpaid-source clawback (subscription cancel/downgrade/qty-decrease/item-removal) selects a finalized funding invoice via FindFundingInvoicesForPeriod, which filters only status (not payment_status). An in-flight source is classified unpaid and its amount_due is reduced immediately. Then at settle, MarkPaid records amount_paid = amount_due (the now-lowered figure) → under-record, undersized refund cap. Naively skipping the relief instead trades that for a silent over-charge (the customer paid the full amount and never gets the unused portion back). You cannot correctly size the clawback until the charge reaches a terminal state.

Reachability (honest). On cards — Hardcount’s default (Confirm+OffSession) — the in-flight window is seconds: the charge settles inline or fails. So a mutation colliding with it is rare. The window becomes days — and the collision routine — under asynchronous bank methods (ACH / SEPA Direct Debit), which legitimately sit processing for 1–5 business days and are the standard way B2B annual contracts pay. So Class 2 is “rare on cards today, regular the moment ACH/SEPA is enabled.” Stripe enforces the same rule it implies: “When an invoice has open or paid payments, you can’t void, edit, or mark it uncollectible,” and “open invoices can’t have a credit note with a pending payment_intent.”

Decision

One canonical predicate, in-flight guards on the status writers, a single void writer, and — for the automated clawback — defer the issue, not the create.

Canonical predicate. domain.InvoicePaymentStatus.IsInFlight() = processing || unknown. Every guard routes through it (including the prior operator credit-note gate, #295), so the definition can’t drift across call sites — the drift this audit found.

Class 1 — guards (block, Stripe-parity).

1. Void and MarkUncollectible reject in-flight with InvalidState (→409), placed before reverseInvoiceTax so no reversal fires on an in-flight invoice. The reconciler resolves the charge; the operator retries.
2. RecordOfflinePayment widened from == processing to IsInFlight() (closes the unknown double-collect window).
3. Single void writer. Dunning resolveRun (manually_resolved) now voids through invoice.Service.Void (a narrow InvoiceVoider dep, wired post-construction like SetInvoiceUncollectibleMarker), inheriting the status guards, the in-flight guard, the tax reversal, and the single-writer invoice.voided event. Its inline credit-reversal + PI-cancel now run only after the void succeeds — otherwise an in-flight invoice (whose void the service refuses) would still get its live PI canceled, defeating the guard.

Class 2 — defer-until-settle (the automated clawback). The automated path cannot 409 a cancel/downgrade back to a human, so it defers rather than reduces. The key insight: Issue() already branches on the source’s payment_status at issue time (succeeded → credit/refund channel; not-succeeded → reduce amount_due). So we defer when Issue() runs, not the create:

1. Issue() is the single chokepoint. Before claiming the draft→issued CAS it reads the source invoice and:
   • if the source is voided/uncollectible (annulled after the draft was created) → voids the draft (pure status flip) instead of issuing — the void already reversed the tax and zeroed the receivable, so issuing would double-reverse tax.
   • if the source is in flight → no-op, leaving the draft status='draft' AND issue_pending. Every issue trigger (engine CreateAndIssueAdjustment, the post-commit issueClawbackDrafts, the reconciler) defers identically — so no clawback create-site needs its own gate.
2. CreateAndIssueAdjustment marks the draft issue_pending so a deferred (or post-commit-failed) issue is recoverable by the reconciler.
3. Reconciler issues once the source settles. RetryPendingClawbackIssue’s scan (ListPendingClawbackDrafts) gains a correlated NOT EXISTS that skips drafts whose source payment_status IN ('processing','unknown'), and drops the prior 24h updated_at window — an ACH/SEPA source can sit processing for days, so a fixed horizon would age a deferred draft out of the scan and silently drop the relief. The source-terminal gate is now the only eligibility predicate; a settled source makes the draft eligible regardless of age.
4. Engine void-skip. relieveUnpaidPrebill’s fully-unused void branch is skipped for an in-flight source (it falls through to the deferring CreateAndIssueAdjustment for the full amount). At settle the draft issues down the right channel: paid → full credit; failed → amount_due → 0 (the same end-state the void produced).

No migration. The draft row + issue_pending (migration 0121) is the durable deferred record; the source invoice’s payment_status is the trigger signal. A dedicated table was rejected (a dual-write reconciliation surface — the recurring money-bug shape). Source-exclusion in FindFundingInvoicesForPeriod was rejected (it hard-fails a single-funding-source downgrade and over-credits survivors via redistribution).

Reconciler-only, not a settle-hook. A hook from payment into creditnote/billing would violate the zero-cross-peer-import rule, and the draft→issued CAS already guarantees exactly-once however many triggers race. Latency is up to one scheduler tick (~1h prod) after a charge that already sat in flight for days — negligible. Matches the 0121 precedent (scheduler tick, no settle hook).

Consequences

• Parts A + B closed for full capture. The operator vector was gated (Part A, #295); the automated vector now defers → amount_due stays full while in-flight → MarkPaid records the full captured amount → the reconciler credits the unused share. No under-record, no over-charge.

• Part C (partial capture) is not reachable in Hardcount. MarkPaid records amount_paid = amount_due. That equals the captured amount under Hardcount’s PaymentIntent-only full-capture model — Hardcount exposes no partial/manual-capture flow. If one is ever added, recording amount_paid from the PI’s amount_received is the correct source of truth; until then this is a theoretical vector, not a live gap.

• Genuinely-wedged payment → deferred draft waits (observability follow-up). If a charge never reaches terminal (e.g. a requires_action PI nobody authenticates or cancels), the deferred draft sits unissued. It is not lost — it is durably captured and auto-issues the instant the payment settles, and a wedged payment is independently visible (the invoice stuck in processing; the tenant unpaid). A dedicated stale-deferred-draft alarm (surface a draft deferred > N days so an operator can cancel/await the wedged PI, which then auto-resolves the clawback) is a deferred observability follow-up — it does not affect whether the books are correct or the customer is eventually made whole.

  Amended 2026-07-31 (ADR-107). “Auto-issues the instant the payment settles” assumed every payment eventually settles. A parked invoice never does — unknown with no PaymentIntent id means nothing can name the attempt, so there is no webhook and no sweep to end the wait. The deferral was therefore permanent, and silently so: the scan’s eligibility predicate tested payment state alone, so the draft could not reach the code that decides its fate, produced no error logs (deferred drafts log nothing, by design), and sat in the pending-drafts gauge forever. Writing the invoice off did not release it either, because a write-off deliberately does not change payment_status.

  Fixed by adding a status term to that predicate: a terminal invoice defers nothing. The outcome is unchanged from what this ADR already specified — the existing orphan guard voids a draft whose source was voided or written off, since an invoice that collected nothing has nothing to claw back, and the customer is made whole by not being charged. The bug was reachability, not behaviour. Pinned by TestListPendingClawbackDrafts_ParkedSourceReleasedByWriteOff.

• Industry parity. Operator block matches Stripe exactly. The automated defer-then-act-on-settled-state matches Lago’s terminate-subscription rule (“the unused paid amount is refunded; any unpaid unused amount is credited back”) — Hardcount just sequences it after settlement instead of guessing the channel mid-flight.

Amendment 2026-10-14: the charge hold counts as in flight (#975)

What was wrong. This ADR defined in flight as processing || unknown and checked it on the copy of the invoice the service read before taking the row lock. Neither was enough once charges took a hold. Every charge claim (the automatic charge, a dunning retry, Collect) writes only auto_charge_claimed_until, five minutes ahead, and payment_status stays pending or failed until Stripe answers. So for the whole time Stripe could be taking the money, IsInFlight() said no, and a charge claimed after the service’s read was invisible anyway. Outcomes: a void followed by a successful charge left money on a voided invoice (credits handed back, tax reversed, only an alert); an offline payment followed by a successful charge made the customer pay twice.

What changed. Void, MarkUncollectible and RecordOfflinePayment now run the refusal again on the row read under the row lock. That second check also refuses while the hold is live (domain.PaymentBlocksActionHeld). RecordOfflinePayment also refuses there when the locked row is already paid: a charge that settled after the service’s read would otherwise turn the paid transition into a quiet no-op, and the audit row and invoice.payment_recorded would record a payment that never happened. The answer is 409 payment_in_flight: “a charge on this invoice is in progress — try again in a few minutes”.

• The hold is read as COALESCE(auto_charge_claimed_until > now(), false), the hosted-page claim’s expression, on the database’s clock. (Superseded 2026-10-19: an expired hold counts too. See the amendment below.)
• It is consulted only on pending and failed. An unknown outcome keeps its hold for the full five minutes, and a parked invoice must stay writable-off (ADR-107).
• MarkUncollectible moved from the plain status write to the locking variant. RecordOfflinePayment uses MarkPaidOffline, which runs the check under the paid transition’s existing FOR UPDATE. The card settlement and the credit-covered settles do not get it: they run inside the charge’s own hold and must be able to record it.
• The shared status compare-and-set is unchanged. It also serves draft→finalized.

Accepted cost. A breaker-open or credentials failure returns without releasing the hold, so an operator can be refused for up to five minutes when no charge was sent. Dunning’s final write-off treats the refusal as a failed terminal action and retries it 24 hours later. (Superseded 2026-10-19: those failures now release the hold. See the amendment below.)

The engine’s relief void too. A cancel or plan swap voids a prepaid invoice whose period is wholly unused (VoidUnchangedTx), which happens on the period’s first day, exactly when the first automatic charge fires. It refuses on the hold the same way. The error rolls the cancel or swap back: an operator sees the 409, and dunning and the scheduled-cancel scan retry on their own.

Not covered. An amount-reducing credit note does not read the hold; that one is deferred under its own trigger in velox-ops dp-readiness. Pinned by the TestChargeHold_* tests in internal/invoice.

Amendment 2026-10-19: an expired hold is not free (ADR-127)

ADR-127 (#976) changes two things in the 2026-10-14 amendment.

How the hold is read. An expired hold no longer counts as free: it means a charge may have been sent and its result was lost. The check reads the hold as none, live or stale (ChargeHeldTx, under the row lock). Void, offline payment and the engine’s relief void refuse a stale hold as they refuse a live one. Write-off is allowed on a stale hold: it moves no money, and a payment found later still settles the invoice. The stale-hold sweep in the payment reconciler asks Stripe what the lost charge did and clears the hold. It checks a hold on the first billing tick at least 10 minutes after the hold ends, usually within about an hour; until then these refusals stand.

The accepted cost is gone. A charge that sent nothing releases its hold at once: the breaker refused it (it refuses before any request), or the credentials could not be read. Kept, such a hold would now wait for the sweep instead of expiring after five minutes, and the breaker is shared by every tenant. A 429 keeps its hold: a first request under the same key may still be running at Stripe. Dunning’s final action does not run while any hold is set, so it no longer meets this refusal on its own invoice.

Deferred / follow-ups

Item	Affects correctness?	Trigger
Stale-deferred-draft alarm (surface a clawback draft wedged on a never-settling payment)	No — obligation is durably captured and auto-issues on settle; wedged payments are independently visible	operability hardening / first ACH-SEPA design partner
Part C — record amount_paid from the PI’s amount_received	No (not reachable: full-capture only)	partial-capture support
reverseInvoiceTax failure recovery (ADR-057 §Deferred b)	Yes, on a transient reversal failure	first real tax-filing customer
ADR-057 partial-issue window (non-idempotent ApplyCreditNote after the status flip) — Part B inherits, does not widen it (the reconciler only re-drives status='draft')	Yes, on a side-effect failure after the CAS	making ApplyCreditNote idempotent (separate PR)