feat(billing): send email notification to admin on auto-top-up success#2793
Open
kilo-code-bot[bot] wants to merge 2 commits intomainfrom
Open
feat(billing): send email notification to admin on auto-top-up success#2793kilo-code-bot[bot] wants to merge 2 commits intomainfrom
kilo-code-bot[bot] wants to merge 2 commits intomainfrom
Conversation
Notify org owners (or the user themselves for personal accounts) when an auto-top-up is successfully processed. The email includes the amount topped up, new balance, triggering user, and timestamp. - Add autoTopUpSuccess.html email template - Add sendAutoTopUpSuccessEmail() helper in email.ts - Trigger notification in invoice.paid webhook handler (both user and org paths) - Add fixture data for admin email testing UI
| const triggeredByUserId = | ||
| autoTopUpConfig?.created_by_user_id ?? SYSTEM_AUTO_TOP_UP_USER_ID; | ||
|
|
||
| await processTopupForOrganization( |
Contributor
Author
There was a problem hiding this comment.
WARNING: Duplicate invoice.paid retries can send duplicate org success emails
processTopupForOrganization() currently returns void and just returns early on the onConflictDoNothing() path, so this branch cannot tell whether the org top-up was newly applied or already registered. Stripe retries invoice.paid, and those retries will still queue sendOrgNotification(), which means owners can receive another "Auto Top-Up Successful" email even though no new credits were added. Please return an explicit success boolean here and gate the notification the same way the user path does.
Contributor
Author
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (6 files)
Reviewed by gpt-5.4-20260305 · 426,327 tokens |
Make processTopupForOrganization return a boolean (true=credits applied, false=duplicate) mirroring the existing processTopUp pattern. Gate the org notification email on that return value so duplicate invoice.paid webhooks no longer trigger spurious emails to org admins.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When an auto-top-up is successfully processed (via the
invoice.paidStripe webhook), an email notification is now sent to the relevant admin:The email includes: amount topped up, new balance, which user triggered it, and timestamp. The notification is sent asynchronously via
after()to avoid blocking the Stripe webhook response.Changes:
autoTopUpSuccess.htmlemail template following the existing design systemsendAutoTopUpSuccessEmail()helper inemail.tsinvoice.paidwebhook handler (stripe.ts)AGENTS.mdemail template docs updatedprocessTopupForOrganization()now returns a boolean (true= credits newly applied,false= duplicate/conflict), mirroring the existingprocessTopUp()pattern. The org notification email is gated on this return value so duplicateinvoice.paidwebhook retries no longer send spurious emails to org admins.Verification
/admin/email-testing.Visual Changes
N/A
Reviewer Notes
processTopUp/processTopupForOrganizationreturns successfully (not on duplicate/already-registered cases), matching the pattern whereautoTopUpOk/orgAutoTopUpOkis true.new_balancereflects the post-top-up state.captureException— email failures never affect the billing flow.IS_IN_AUTOMATED_TESTguard ensures the email sending runs synchronously in tests (same pattern ascredits.ts).processedSuccessfully(used for lock management) is still set unconditionally — duplicates should still mark the lock as completed, since the top-up was already processed by a prior webhook.Built for Lígia Zanchet by Kilo for Slack