fix(billing): lock the invoice row before recomputing amount_paid

Claude-Run commented

2026-08-01 06:23:36 +02:00

Member

Concurrent payments against one invoice lost updates. create_payment read total, amount_paid with no row lock, computed the new absolute values in Rust, and wrote them back; under READ COMMITTED two concurrent requests both read the pre-payment amount_paid, so whichever committed last discarded the other. A 400.00 and a 600.00 payment against a 1000.00 invoice both persisted their payments rows but left the invoice at amount_paid = 600.00. The same race defeated the PMS-194 overpayment guard in the other direction: two 700.00 payments both passed 700 <= 1000 and the invoice ended up with 1400.00 of payments recorded against it. delete_payment shared the shape and lost one side of a delete racing a create.

create_payment now takes SELECT ... FOR UPDATE on the invoice before it inserts the payment row, so the lock covers the whole read-modify-write and an overpayment rejection no longer has to unwind an inserted row. Both paths then call one shared recompute_invoice_payment_state helper that derives amount_paid from SUM(payments.amount) and recomputes balance_due / status / paid_at from it in a single statement, which makes invoices.amount_paid = SUM(payments.amount) true by construction and removes the duplicated status ladder plus the (prior_paid - amount).max(ZERO) clamp that only existed to paper over the race. The zero-payments case still lands on sent, identical to the ladder it replaces.

update_invoice is the third writer of the same columns: it rewrites balance_due and status from a pre-transaction read of amount_paid, so it takes the same row lock in the same order and re-reads both under it, rather than writing a stale snapshot back over a payment that committed in between.

Three integration tests in tests/billing.rs cover it. The two concurrency tests hold the race window open deterministically: a third transaction takes the invoice row lock, both payment requests are sent and park against it, and the lock is released only once both are in flight. Without that the handlers finish faster than the second request arrives and never overlap. Against the pre-fix service both fail with exactly the reported symptoms (amount_paid = 400.00 instead of 1000.00; both 700.00 payments returning 200).

#PMS-695

Concurrent payments against one invoice lost updates. `create_payment` read `total, amount_paid` with no row lock, computed the new absolute values in Rust, and wrote them back; under READ COMMITTED two concurrent requests both read the pre-payment `amount_paid`, so whichever committed last discarded the other. A 400.00 and a 600.00 payment against a 1000.00 invoice both persisted their `payments` rows but left the invoice at `amount_paid = 600.00`. The same race defeated the PMS-194 overpayment guard in the other direction: two 700.00 payments both passed `700 <= 1000` and the invoice ended up with 1400.00 of payments recorded against it. `delete_payment` shared the shape and lost one side of a delete racing a create. `create_payment` now takes `SELECT ... FOR UPDATE` on the invoice before it inserts the payment row, so the lock covers the whole read-modify-write and an overpayment rejection no longer has to unwind an inserted row. Both paths then call one shared `recompute_invoice_payment_state` helper that derives `amount_paid` from `SUM(payments.amount)` and recomputes `balance_due` / `status` / `paid_at` from it in a single statement, which makes `invoices.amount_paid = SUM(payments.amount)` true by construction and removes the duplicated status ladder plus the `(prior_paid - amount).max(ZERO)` clamp that only existed to paper over the race. The zero-payments case still lands on `sent`, identical to the ladder it replaces. `update_invoice` is the third writer of the same columns: it rewrites `balance_due` and `status` from a pre-transaction read of `amount_paid`, so it takes the same row lock in the same order and re-reads both under it, rather than writing a stale snapshot back over a payment that committed in between. Three integration tests in `tests/billing.rs` cover it. The two concurrency tests hold the race window open deterministically: a third transaction takes the invoice row lock, both payment requests are sent and park against it, and the lock is released only once both are in flight. Without that the handlers finish faster than the second request arrives and never overlap. Against the pre-fix service both fail with exactly the reported symptoms (`amount_paid = 400.00` instead of 1000.00; both 700.00 payments returning 200). #PMS-695

Claude-Run added 1 commit

2026-08-01 06:23:36 +02:00

fix(billing): lock the invoice row before recomputing amount_paid

E2E / Playwright against staging (pull_request) Successful in 49s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 1m57s

Details

Integration / integration tests (pull_request) Successful in 8m38s

Details

Create release / Gate (release-branch merges only) (pull_request) Successful in 0s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

34548ff0e3

Concurrent payments against one invoice lost updates. `create_payment` read `total, amount_paid` with no row lock, computed the new absolute values in Rust, and wrote them back; under READ COMMITTED two concurrent requests both read the pre-payment `amount_paid`, so whichever committed last discarded the other. A 400.00 and a 600.00 payment against a 1000.00 invoice both persisted their `payments` rows but left the invoice at `amount_paid = 600.00`. The same race defeated the PMS-194 overpayment guard in the other direction: two 700.00 payments both passed `700 <= 1000` and the invoice ended up with 1400.00 of payments recorded against it. `delete_payment` shared the shape and lost one side of a delete racing a create.

`create_payment` now takes `SELECT ... FOR UPDATE` on the invoice before it inserts the payment row, so the lock covers the whole read-modify-write and an overpayment rejection no longer has to unwind an inserted row. Both paths then call one shared `recompute_invoice_payment_state` helper that derives `amount_paid` from `SUM(payments.amount)` and recomputes `balance_due` / `status` / `paid_at` from it in a single statement, which makes `invoices.amount_paid = SUM(payments.amount)` true by construction and removes the duplicated status ladder plus the `(prior_paid - amount).max(ZERO)` clamp that only existed to paper over the race. The zero-payments case still lands on `sent`, identical to the ladder it replaces.

`update_invoice` is the third writer of the same columns: it rewrites `balance_due` and `status` from a pre-transaction read of `amount_paid`, so it takes the same row lock in the same order and re-reads both under it, rather than writing a stale snapshot back over a payment that committed in between.

Three integration tests in `tests/billing.rs` cover it. The two concurrency tests hold the race window open deterministically: a third transaction takes the invoice row lock, both payment requests are sent and park against it, and the lock is released only once both are in flight. Without that the handlers finish faster than the second request arrives and never overlap. Against the pre-fix service both fail with exactly the reported symptoms (`amount_paid = 400.00` instead of 1000.00; both 700.00 payments returning 200).

#PMS-695

Claude-Run merged commit 324d3b79dd into main

2026-08-01 06:32:42 +02:00

Claude-Run deleted branch fix/PMS-695-lock-invoice-on-payment

2026-08-01 06:32:43 +02:00

Claude-Run referenced this pull request from a commit

2026-08-01 06:32:44 +02:00

Merge pull request 'fix(billing): lock the invoice row before recomputing amount_paid' (#477) from fix/PMS-695-lock-invoice-on-payment into main

Rows
Columns

fix(billing): lock the invoice row before recomputing amount_paid #477