Skip to content

fix(console): guard against duplicate refund and use actual refund amount#28400

Open
PanAchy wants to merge 5 commits into
anomalyco:devfrom
PanAchy:fix/billing-refund-amount
Open

fix(console): guard against duplicate refund and use actual refund amount#28400
PanAchy wants to merge 5 commits into
anomalyco:devfrom
PanAchy:fix/billing-refund-amount

Conversation

@PanAchy
Copy link
Copy Markdown
Contributor

@PanAchy PanAchy commented May 19, 2026

Issue for this PR

Closes #28398

Type of change

  • Bug fix

What does this PR do?

Fixes two bugs in the charge.refunded webhook handler (packages/console/app/src/routes/stripe/webhook.ts):

Bug 1 — Partial refunds silently dropped:
The handler set PaymentTable.timeRefunded on the first refund event and returned early on any subsequent one. Stripe sends a separate charge.refunded event per partial refund, so the second partial was silently discarded and the balance never updated.

Bug 2 — Wrong amount deducted:
The balance deduction used payment.amount (the full original charge, in microcents) instead of the actual refund amount from Stripe. For a partial refund this over-deducts.

Fix:
Added amountRefunded to PaymentTable to track cumulative refunded amount. On each event, compute delta = event.amount_refunded - payment.amountRefunded. Skip if delta <= 0 (idempotency). Deduct only the delta from balance. Set timeRefunded only when the cumulative refunded amount reaches the full payment amount.

const cumulativeRefundedMicroCents = centsToMicroCents(body.data.object.amount_refunded as number)
const deltaRefundMicroCents = cumulativeRefundedMicroCents - (payment.amountRefunded ?? 0)
if (deltaRefundMicroCents <= 0) return

await Database.transaction(async (tx) => {
  await tx.update(PaymentTable).set({
    amountRefunded: cumulativeRefundedMicroCents,
    ...(cumulativeRefundedMicroCents >= payment.amount ? { timeRefunded: new Date(body.created * 1000) } : {}),
  }).where(...)
  balance: sql`${BillingTable.balance} - ${deltaRefundMicroCents}`
})

Note for maintainers: The amountRefunded column addition requires a migration. Please run bun db generate after merging. The migrations folder has not been modified — migration generation requires SST infra access that external contributors do not have.

How did you verify your code works?

  • Ran bun typecheck in packages/console — no errors.
  • amount_refunded on the Stripe Charge object is cumulative. Delta approach correctly handles both partial and full refunds with idempotency across retries.
  • Confirmed centsToMicroCents is already imported and used identically in other handlers in the same file.
  • No automated test added: the existing console test suite only covers pure functions with no mocking infrastructure. charge.refunded handling requires a live DB and Stripe context outside the current test conventions.

Screenshots / recordings

If this is a UI change, please include a screenshot or recording.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

If you do not follow this template your PR will be automatically rejected.

…l refund amount

Two bugs in the charge.refunded webhook handler:

Bug 9: no idempotency check on timeRefunded, so Stripe retries of the
same refund event each deduct balance again. Fix: early return if
payment.timeRefunded is already set.

Bug 10: balance deduction used payment.amount (original charge) instead
of amount_refunded (actual refund). A partial refund on a larger charge
would over-deduct the workspace balance. Fix: use
centsToMicroCents(body.data.object.amount_refunded) instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refund handler uses wrong amount and runs repeatedly

1 participant