Lock combined-charge purchases in id order before refunding

[gumclaw]

Fixes #5918

Combined-charge refunds were still outside the purchase-first lock order established for single-purchase refunds in #5917. Charge#refund_and_save! (and refund_for_fraud_and_block_buyer!) refund every purchase in one transaction; each Purchase#refund_purchase! locks its purchase then the shared seller balance, so the transaction's acquisition sequence was purchase₁ → balance → purchase₂ → … — it held the balance while waiting for later purchase rows. A concurrent HandleFailedRefundService reversal holding purchase₂ and waiting for the same balance completed a deadlock cycle.

Progress

• Lock all successful_purchases in id order at the top of Charge#refund_and_save!'s transaction before refunding any (new private Charge#lock_successful_purchases_in_id_order!)
• Same pre-pass inside refund_for_fraud_and_block_buyer!'s with_lock
• Apply the reload.lock! + moved partial-refund-flag read to Purchase#refund_partial_purchase! (kept rather than deleted: console/legacy callers plus a wide spec-caller surface)
• Extend the real-thread concurrency coverage with a combined-charge scenario (two purchases, one seller balance, sibling reversal on the second purchase) — the sibling reversal now queues behind the charge refund's up-front purchase lock instead of interleaving
• Update the mock-based charge_spec #refund_and_save! example for the lock pre-pass

Test results (local)

• spec/services/purchase/handle_failed_refund_service_concurrency_spec.rb — 4 examples, 0 failures (includes the new combined-charge scenario; real threads on separate DB connections)
• RED check: with only the Charge fix stashed, the new example times out exactly as the issue describes (the sibling reversal overtakes between per-purchase refunds) — the spec genuinely exercises the cycle
• spec/models/charge_spec.rb + spec/models/purchase/purchase_refunds_spec.rb — 184 examples, 1 failure: charge_spec.rb:21 fails identically on a clean origin/main worktree (merchant_account already connected — shared-test-DB env issue), pre-existing, not this change
• rubocop clean on all touched files

Before / after

Before (fix stashed — new spec reproduces the lock cycle)	After (fix in place — full concurrency file green)

Permanent copies: qa-media/ on this branch. Backend-only concurrency fix — no UI surface, no schema change; no preview URL applicable (no user-visible behavior change; refund flows behave identically except that the deadlock window is closed).

QA steps

1. Read the diff: app/models/charge.rb (id-ordered lock pre-pass + comment), app/modules/purchase/refundable.rb (refund_partial_purchase! lock + updated comment), and the new spec example.
2. Run bundle exec rspec spec/services/purchase/handle_failed_refund_service_concurrency_spec.rb — expect 4 examples, 0 failures.
3. Optional: stash the app/models/charge.rb hunk and re-run the combined-charge example to watch it reproduce the cycle (times out on the latch), then restore.

---

AI disclosure: authored autonomously by gumclaw (Hermes Agent, model claude-fable-5) from issue #5918 as part of the autonomous dev dispatcher. Prompts: standing dispatcher instruction ("plan and then build and merge fully autonomously… certainly bug fixes"), issue #5918 body as the spec.

[greptile-apps]

Greptile Summary

This PR updates refund locking so combined-charge refunds follow the same purchase-first order as single-purchase refunds. The main changes are:

• Locks all successful purchases in id order before combined-charge refund work begins.
• Applies the same up-front purchase locking to combined-charge tax refunds and fraud refunds.
• Moves partial-refund state reads after the purchase row lock.
• Adds real-thread tests for the combined-charge refund and failed-refund reversal lock cycle.
• Updates charge model specs and adds QA images for the before/after concurrency behavior.

Confidence Score: 5/5

Safe to merge with minimal risk.

The updated code is narrowly scoped to refund lock ordering and keeps existing refund behavior intact. The changed paths include targeted real-thread tests for the deadlock scenario.

No files require special attention.

T-Rex Logs

What T-Rex did

• Reviewed the contract-validation proof and identified that the repository requires Ruby 3.4.3 and pins the version via a .ruby-version directive.
• Verified the sandbox is running Ruby 3.1.2p20, confirming a mismatch with the repository requirement.
• Confirmed bundle check exits with code 18 due to the Ruby version mismatch, showing a dependency resolution failure in the environment.
• Attempted to run RSpec via the concurrency path, but the command failed with exit code 127 because rspec was not found by Bundler.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
app/models/charge.rb	Adds an id-ordered purchase lock pre-pass before combined-charge refund, tax-refund, and fraud-refund iteration.
app/modules/purchase/refundable.rb	Extends purchase-first locking to partial and Gumroad tax refunds while preserving subscription state across reload.
spec/services/purchase/handle_failed_refund_service_concurrency_spec.rb	Adds cleanup for charge/order rows and a real-thread combined-charge deadlock regression test.
spec/models/charge_spec.rb	Updates charge refund specs to stub the new reload/lock pre-pass on purchase doubles.
qa-media/pr-5918-after-concurrency-spec-green.png	Adds QA evidence image showing the concurrency spec passing after the fix.
qa-media/pr-5918-before-red-check-deadlock.png	Adds QA evidence image showing the deadlock reproduction before the fix.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant ChargeRefund as Charge refund transaction
participant Purchase1 as Purchase 1 row
participant Purchase2 as Purchase 2 row
participant Balance as Seller balance
participant Reversal as Failed-refund reversal

ChargeRefund->>Purchase1: lock! in id order
ChargeRefund->>Purchase2: lock! in id order
ChargeRefund->>Balance: refund purchase 1 balance update
Reversal->>Purchase2: waits for purchase lock
ChargeRefund->>Balance: refund purchase 2 balance update
ChargeRefund-->>Purchase1: commit releases locks
ChargeRefund-->>Purchase2: commit releases locks
Reversal->>Purchase2: acquires lock after charge refund
Reversal->>Balance: reverse failed refund balance rows

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant ChargeRefund as Charge refund transaction
participant Purchase1 as Purchase 1 row
participant Purchase2 as Purchase 2 row
participant Balance as Seller balance
participant Reversal as Failed-refund reversal

ChargeRefund->>Purchase1: lock! in id order
ChargeRefund->>Purchase2: lock! in id order
ChargeRefund->>Balance: refund purchase 1 balance update
Reversal->>Purchase2: waits for purchase lock
ChargeRefund->>Balance: refund purchase 2 balance update
ChargeRefund-->>Purchase1: commit releases locks
ChargeRefund-->>Purchase2: commit releases locks
Reversal->>Purchase2: acquires lock after charge refund
Reversal->>Balance: reverse failed refund balance rows

Loading

_{Reviews (3): Last reviewed commit: "Preserve subscription association across..." | Re-trigger Greptile}

[gumclaw]

Fable-5 principal-engineer review completed on the diff. Findings and resolutions:

• P2-1 (fixed): the lock pre-pass locked one set of instances but the refund loop re-queried successful_purchases — under REPEATABLE READ that re-query reads the pre-lock snapshot, so refundable amounts could be stale relative to a reversal that committed while the pre-pass waited. lock_successful_purchases_in_id_order! now returns the locked instances and all three charge refund paths iterate those.
• P2-2 (fixed): Purchase#refund_gumroad_taxes! (standalone VAT refund) still acquired refund→purchase locks in reverse order against the reversal service's FOR UPDATE refunds scan — same deadlock class. Its transaction now starts with reload.lock!, and Charge#refund_gumroad_taxes! got the same id-ordered pre-pass.
• P3 (noted, not changed): refund_partial_purchase! has no production callers — lock added anyway per the issue checklist; deletion can be a follow-up. All purchase locks are now held across the in-transaction Stripe calls — accepted cost vs. deadlocks; moving processor calls out of the transaction is a separate refactor.

Re-verified locally after the fixes: concurrency spec 4/4 green, charge_spec + purchase_refunds_spec green apart from the pre-existing shared-test-DB merchant_account already connected factory failure (fails identically on clean origin/main). rubocop clean.

[gumclaw]

Assigning @nyomanjyotisa for review (charge/refund backend lane): although this is a bug-class fix with green local specs and fable P1/P2 findings resolved, it changes lock acquisition inside the core refund money-movement paths (Charge#refund_and_save!, fraud refunds, VAT refunds), which the autonomy policy excludes from auto-merge. CI is the arbiter for the shared-DB-dependent specs. Companion to your #5917.

[gumclaw]

CI triage: the Test Minitest failure (SubscriptionTest#test_#charge!_transfers_VAT_ID_from_the_original_purchase's_tax_refund_to_recurring_charge) was real and introduced by this branch — reload.lock! in refund_gumroad_taxes! clears the purchase's association cache, so Subscription#charge! callers holding the same in-memory subscription instance never saw the VAT ID stored during the refund. Fixed in f7905c8 by capturing the subscription instance before the reload and storing the VAT ID on it. Reproduced red locally, full test/models/subscription_test.rb (425 runs) green after the fix; the concurrency spec suite also passes. Also merged main in (a64ac03, no conflicts). Auto-merge stays armed pending green CI.