fix(cloud): atomic + spend-clamped ad-campaign refunds — stop concurrent/retry double-refund + decrease over-refund (#11292)

[NubsCarson]

Fixes #11292 (follow-up to #11255).

Two live-money leaks in the ad-campaign refund paths

1. No atomic claim → concurrent / retry double-refund.
updateCampaign (budget decrease) and deleteCampaign both did findById → compute refund → refundCredits with no row-level claim, so two concurrent decreases — or a delete retried after the refund succeeded but the row-delete threw — both refunded the same unused budget.

2. updateCampaign decrease over-refunded after spend.
It refunded the full allocation delta (old_allocated - new_allocated), ignoring credits_spent/total_spend — so lowering a budget after real ad spend returned credits already spent on impressions. (deleteCampaign already clamps; updateCampaign did not.)

Fix

• Atomic CAS claims (new repo methods, mirroring the existing incrementSpend atomic-SQL pattern):
  • claimAllocationChange — UPDATE … WHERE credits_allocated = <observed> RETURNING; only the winner refunds a decrease, a lost CAS throws a retryable conflict (never double-refunds).
  • claimDelete — DELETE … RETURNING; only the caller that removes the row refunds.
• Spend-clamped decrease refund — refund = min(freed, unused), reusing deleteCampaign's proven two-column (security/money: internal SSP campaign delete refunds full budget after real spend (total_spend vs credits_spent) #11151) spend logic, now extracted into a shared computeCreditsSpent helper (rule-2 reuse — one calculation for both paths).
• Markup invariant preserved — credits_allocated stays = newBudget × markup (only the refund is clamped, never the stored allocation), so the markup derived at delete (allocated / budget) stays correct. This is the coupling that would make a naive clamp corrupt future refund math (see money: ad-campaign budget-decrease + delete refunds aren't atomically claimed (concurrent/retry double-refund) + decrease ignores spend (over-refund) — follow-up #11255 #11292 for the full trace).

Evidence

bun test packages/cloud/shared/src/lib/services/__tests__/ad-campaign-credit-reconciliation.test.ts

 12 pass
 0 fail
 30 expect() calls
Ran 12 tests across 1 file.

The 5 existing #11151 delete tests + 3 existing update tests still pass (helper extraction is behavior-preserving); 4 new tests cover the fixes:

• decrease-after-spend refunds only the unused 22, not the full 99;
• decrease with no spend refunds the full 99;
• concurrent decrease (lost CAS) throws and refunds nothing;
• concurrent delete (lost claimDelete) refunds nothing.

Mutation-checked (proving the tests catch the bugs, not just pass):

• revert the clamp to Math.max(0, freed) → the decrease-after-spend test fails (Received: 99, Expected: 22, the exact 77 over-refund);
• remove the conflict throw → the concurrent-decrease test fails (promise resolves instead of rejecting).

Biome clean. N/A UI/trajectory — backend money-arithmetic change, no UI/model surface; pinned by the unit suite driving the real advertisingService (only repo/credits/provider boundaries spied).

⚠️ Review note

Implemented by the main (Opus) model while the Fable agent was rate-capped — money code, so please scrutinize; do not self-merge, this is the money lane (@lalalune). The atomic-claim + markup-invariant reasoning is in #11292.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 711e89a9-c00f-456c-b207-cfa1e45c146d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nubs/ad-refund-safety

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[lalalune]

Maintainer review (money path): the atomic claim design is right (claimDelete DELETE…RETURNING winner-refunds, update-path CAS is a genuine addition over #11255), and adversarial review confirmed this branch re-lands what #11271's stale-base squash accidentally reverted. Pushed 4960873 restoring two things merged-#11255 had already settled, which this branch (cut post-revert) re-derived differently: (1) SUM-with-clamp spend accounting — the streams are additive since findEligibleAd serves external campaigns through the internal SSP too, so MAX under-counts dual-stream spend and over-refunds; (2) the delete-path ledger insert used campaign_id after claimDelete removed the row — a 23503 FK violation on real Postgres firing AFTER the refund committed (invisible to the repository-spy tests); restored the external_reference carry. 12/12 + typecheck post-fix.

Residual for the #11292 thread (pre-existing, both paths): claim and refundCredits are separate commits with no idempotency key — a refund throw after the claim permanently strands the customer's refund. Wants claim+refund in one tx or a sweep.

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.