fix(cloud): make reconcile refund idempotent so a repeated settle cannot double-mint (#11512)

[NubsCarson]

What

reconcileCredits' refund branch was non-idempotent. The reservation settler resets its first-call-wins guard when reconcile throws — so if the refund transaction commits and a post-refund write then throws (DB blip), the route's fallback settleReservation(0) re-invokes reconcile and mints a second, full refund to the org.

Fix: thread the request-stable idempotencyKey (already in reservation metadata via withChargeIdempotencyKey) into the refund as stripePaymentIntentId: reconcile-refund:<key>. A repeated reconcile refund now dedupes on the credit_transactions unique index (ON CONFLICT DO NOTHING in applyCreditIncrease) instead of double-crediting.

Closes #11512

Fail-without-fix proof

New regression test: "a repeated reconcile REFUND for the same reservation is idempotent — no double-refund mint (#11512)" — org starts at $10.00, reserves $2.20, first settle refunds to $9.45, then the same reservation is settled again (the #11512 guard-reset shape).

Without the fix (app-credits.ts stashed, test kept):

error: expect(received).toBeCloseTo(expected, precision)
Expected: 9.45
Received: 11.65            ← second refund MINTED above the $10 start
(fail) … no double-refund mint (#11512)
 4 pass / 1 fail

With the fix:

 5 pass
 0 fail
 57 expect() calls
Ran 5 tests across 1 file.

All 4 pre-existing tests in the file stay green.

Checks

• bun run typecheck (packages/cloud/shared, tsgo --noEmit): exit 0, no errors
• bunx biome check on both touched files: clean

Notes

Money path — do not self-merge.

• Behavior is unchanged when no idempotency key is present in metadata (the spread is conditional) — legacy callers keep today's semantics.
• The reconcile-refund: prefix namespaces the key so it can't collide with real Stripe payment-intent ids or other synthetic keys on the same unique index.

🤖 Generated with Claude Code

[greptile-apps]

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

[NubsCarson]

@lalalune money-path review requested on this idempotent reconcile-refund fix (#11512, blocks a double-mint) when you get a chance — nubs-cloud [cloud-frontdoor]

[NubsCarson]

[cloud-audit] LGTM — the refund leg of reconcileCredits is now genuinely idempotent, and the dedupe is atomic, not check-then-act.

Invariant verified (repeated settle must not double-refund/mint):

• The refund is keyed reconcile-refund:<idempotencyKey> where the key is the request-stable id already threaded into reservation metadata by withChargeIdempotencyKey (app-credits.ts:457) — the same key both routes pass at reserve time, so the re-invoked reconcile after the settler's guard-reset (#11512 shape) reuses the exact key.
• refundCredits forwards it to applyCreditIncrease (credits.ts:810-818), whose single CTE statement gates the org balance UPDATE on EXISTS (SELECT 1 FROM inserted), with the insert guarded by NOT EXISTS + ON CONFLICT (stripe_payment_intent_id) DO NOTHING against the unique index credit_transactions_stripe_payment_intent_idx (db/schemas/credit-transactions.ts). A deduped second refund therefore skips both the ledger row and the balance increase — no partial-dedupe hole, and the unique index is the concurrency backstop for racing duplicates.
• Namespace checked: reconcile-refund: can't collide with the org-credits path's recon:<txid>:refund keys (credits.ts reconKey) or real Stripe pi_... ids sharing the index.
• The companion legs were already idempotent (reverseCreatorEarnings dedupes on ${chargeKey}:inference_markup:reconcile_refund and skips the shadow aggregate writes on dedupe) — this PR closes the last non-idempotent leg, consistent with the existing scheme.
• Test is real: drives the actual reservation closure twice on real PGlite with a hard balance < $10 mint invariant, and the PR documents the fail-without-fix run (11.65 vs 9.45). No migration needed — base code already relies on the same ON CONFLICT target in prod, so nothing in the "won't apply" class.
• Conditional spread means no-key callers keep today's semantics exactly — behavior-preserving for legacy paths.

Non-blocking caveats:

1. The mirror CHARGE branch (reserveAndDeductCredits inside reconcileCredits, app-credits.ts ~:832) still passes no key — a guard-reset replay landing there could double-charge. Pre-existing and outside #11512's shape (the fallback settleReservation(0) always takes the refund branch), but worth a follow-up issue for symmetry with credits.ts's reconKey("overage").
2. A deduped repeat reconcile still returns reconciled:true, action:"refund" with the second call's larger adjustedAmount — cosmetic (balance correct), but logs will overstate the refund on the dedup path.

[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: f88a64c7-8008-4636-ba52-bdaf9bec88d8

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 fix/11512-reconcile-refund-idempotent

---

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.}

[lalalune]

Reviewed and verified after rebasing onto current origin/develop (82001b0).

Local evidence on rebased head cf42c32:

• git diff --check origin/develop...HEAD ✅
• bunx @biomejs/biome@2.5.2 check packages/cloud/shared/src/lib/services/__tests__/app-credit-hold-concurrency.test.ts packages/cloud/shared/src/lib/services/app-credits.ts ✅
• bun test src/lib/services/__tests__/app-credit-hold-concurrency.test.ts from packages/cloud/shared ✅ 5 pass / 0 fail
• bun run --cwd packages/cloud/shared typecheck ✅
• adjacent guard: bun test src/lib/utils/credit-reservation.test.ts src/lib/services/__tests__/app-credit-hold-concurrency.test.ts ✅ 13 pass / 0 fail

The new regression covers the dangerous guard-reset shape directly against PGLite and the real app-credit service: repeated refund reconcile with the same reservation remains at the expected post-refund balance and does not mint above the starting balance. The implementation scopes the synthetic idempotency key to reconcile-refund:<key>, leaving legacy metadata without an idempotency key unchanged.

[NubsCarson]

⚠️ [cloud-frontdoor] — heads up: this PR's reconcile refund leg keys its idempotency dedup on the client-controlled value against the GLOBAL credit_transactions.stripe_payment_intent_id unique index. That creates a cross-tenant collision: two orgs sending the same Idempotency-Key/x-request-id dedupe against each other → one org silently loses a legit refund. Adversarial verify (Fable-5) caught it. #11606 supersedes this with server-generated reservationTransactionId keying + cross-tenant PGlite regression tests. Recommend merging #11606 (or reverting this leg) promptly — the collision is currently live on develop's refund path. cc @lalalune / [cloud-money]

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.