feat(emails): add top-up purchase confirmations#3050
Merged
evanjacobson merged 23 commits intomainfrom May 5, 2026
Merged
Conversation
Contributor
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (9 files)
Fix these issues in Kilo Cloud Reviewed by gpt-5.5-20260423 · 3,352,005 tokens |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
evanjacobson
force-pushed
the
feat/trasnactional-emails_topup
branch
from
164c76f to
93dda2e
Compare
evanjacobson
commented
May 5, 2026
| * - referral_code_usages (financial, references anonymized user) | ||
| * - kiloclaw_subscriptions, kiloclaw_earlybird_purchases, kiloclaw_email_log (retained records) | ||
| * - kiloclaw_scheduled_action_targets (retained operational records; | ||
| * - transactional_email_log (retained outbox marker, financial record) |
Contributor
Author
There was a problem hiding this comment.
The current state does not delete rows on GDPR delete request, per the standard set by kiloclaw_email_log.
Please let me know if this should change.
Contributor
Author
There was a problem hiding this comment.
My intention behind creating this table was to do the following:
Create a table that can be used for idempotency checks for non-KiloClaw emails
Allow the table to be used by future unknown, idempotent emails (or wherever we want to track emails sent)
jeanduplessis
approved these changes
May 5, 2026
Contributor
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
Send exactly-once transactional emails for two purchase events: - Credit top-up (manual Checkout or auto-top-up), hooked into processTopUp's idempotent post-processing block and gated by the existing unique constraint on credit_transactions.stripe_payment_id. - KiloClaw subscription started (first paid billing period only), hooked into applyStripeFundedKiloClawPeriod and gated by an insert-before-send into kiloclaw_email_log so renewals are skipped.
…onStarted in email-testing-router
… period
Replaces the per-instance-lifetime idempotency key on `kiloclaw_email_log`
with a per-activation key so that users who cancel and resubscribe on the
same KiloClaw instance actually receive a second subscription-started email.
- `packages/db/src/schema.ts` + migration `0106_noisy_pete_wisdom.sql`:
add `period_start timestamptz NOT NULL DEFAULT 'epoch'` to
`kiloclaw_email_log`; drop `UQ_kiloclaw_email_log_user_instance_type`;
add `UQ_kiloclaw_email_log_user_instance_type_period` on
`(user_id, instance_id, email_type, period_start)` WHERE
`instance_id IS NOT NULL`.
- `apps/web/src/lib/kiloclaw/credit-billing.ts`: the insert-before-send
in `maybeSendKiloClawSubscriptionStartedEmail` now writes `period_start`,
and the delete-on-error branch now scopes by `period_start` so a failed
send only clears its own marker.
- `apps/web/src/lib/purchase-emails.test.ts`: rewrote the unique-index
unit test for the new shape, added a sibling test proving the index
admits a second row for a new period, and added an end-to-end
regression test that activates, cancels, and resubscribes on the same
instance and asserts two sends and two log rows.
The KiloClaw transactional email work introduced a subscription-started
email gated on `kiloclaw_email_log` as the durable idempotency surface.
The existing unique index on `(user_id, instance_id, email_type)` was
designed for one-per-instance-lifetime emails (e.g. `claw_instance_ready`)
and is wrong for activation-event emails: after the first activation
wrote a row, every future resubscribe on the same instance would conflict
on the index, `onConflictDoNothing` would return `rowCount=0`, and the
function would exit early without sending. Users who cancel and rejoin
— a normal lifecycle — silently lost their activation email.
The adversarial review captured this as cloud-ib7 with the explicit
instruction to pick one of two product semantics and make the code
consistent with the test expectation. We picked "one email per activation
event" because (a) the existing test at purchase-emails.test.ts:440
already asserts a canceled-paid resubscribe should send, and (b) removing
canceled-paid rows from eligibility would deprive returning customers of
a confirmation they reasonably expect.
**Why a `period_start` column and not a `subscription_id` column.**
Resubscribe (both Stripe checkout and credit enrollment) UPDATEs the
existing `kiloclaw_subscriptions` row in place rather than inserting a
new one — Stripe path at `stripe-handlers.ts:870+` (allowed when
`existingRow.status === 'canceled'`), credit path at
`credit-billing.ts:1147` via `onConflictDoUpdate` on `instance_id`. Our
internal `kiloclaw_subscriptions.id` is therefore stable across every
activation on the instance and adds no discriminative power as a dedupe
key. `stripe_subscription_id` is NULL for pure-credit subscriptions
(`enrollWithCredits` explicitly writes `stripe_subscription_id: null`),
so it cannot serve as the key either without special-casing. What
actually differs across activations on both paths is the period
boundary: Stripe stamps fresh `current_period_start` from the invoice
line item; credits stamp `nowIso` on every enrollment. One column
handles both paths.
**Why `NOT NULL DEFAULT 'epoch'` instead of nullable.** Postgres treats
`NULL` as distinct in unique indexes by default, which would let any
other email type that omits `period_start` insert multiple rows and
break the existing one-per-instance-lifetime contract for
`claw_instance_ready`, `claw_suspended_*`, and friends. Drizzle's
`nullsNotDistinct()` is only available on `unique()` constraints, which
do not support partial `WHERE`. Defaulting to `'epoch'` lets every
existing writer keep working unchanged — they all collapse onto the
same `(user, instance, type, 'epoch')` index row — while only the
subscription-started email path opts in to per-activation keying by
explicitly writing `periodStart`.
**Why not a synthetic `dedupe_key text`.** A natural timestamp column
is queryable, self-documenting, and makes admin tooling easier
("show me all activation emails for period X"). A synthetic string key
forces every reader to parse it.
**Why the delete-on-error also got tightened.** The previous delete
cleared every row for `(user, instance, type)`, which was fine when
only one row could exist. With per-activation keying it would be a
foot-gun: a failed send on activation N could erase activation N-1's
durable marker. The new scope is `(user, instance, type, periodStart)`
so a failure only touches its own insert.
**Other writers of `kiloclaw_email_log` are unaffected.** The kiloclaw
billing worker (`services/kiloclaw-billing/src/lifecycle.ts`), the
KiloClaw router instance-destroy cleanup, the admin trial-reset flow,
and the admin instance-reset flow all write and delete rows without
referencing `period_start`. The `DEFAULT 'epoch'` fills in a stable
value so their inserts still collapse one-per-(user, instance, type)
and their deletes (filtered by `email_type IN (...)`) still match every
relevant row regardless of `period_start`.
**Existing production rows get `period_start='epoch'` on backfill.**
For `kiloclaw_subscription_started` rows written before the migration,
this means the first post-deploy activation on the same instance will
write a row with a real `periodStart` and succeed. For renewals that is
correctly suppressed upstream by
`shouldSendSubscriptionStartedEmailForActivation` (before we ever reach
the email helper), so existing active subscribers do not get duplicate
emails. For resubscribes — the exact cohort this fix exists for — a
second email correctly fires.
**Coordinates with adjacent beads.**
- cloud-4lb (enrollWithCredits does not send subscription-started on
credit activation) becomes trivial to land: pass the new period start
to `maybeSendKiloClawSubscriptionStartedEmail` and per-activation
dedupe already works for the credit path.
- cloud-j1o (template copy hard-codes "first billing period") was
previously moot because resubscribes never received the email. It is
now a real product bug and should be addressed.
**Does not touch.** Email rendering or templates, credit accounting,
Stripe webhook parsing, subscription lifecycle state machine, top-up
email flow (cloud-0zq), `softDeleteUser`/GDPR retention (the new
column is a billing-period boundary, not PII; the retention test at
`user.test.ts:1536` still passes).
Manually verified:
- `pnpm --filter web typecheck` passes
- `pnpm --filter kiloclaw-billing typecheck` passes
- `pnpm --filter web test -- purchase-emails` — 20/20 pass, including
the new per-period admit-second-row test and the end-to-end
activate→cancel→resubscribe test
- `pnpm --filter web test -- user.test` — 59/59 pass, including the
GDPR retention test
- `pnpm --filter kiloclaw-billing test` — 60/60 pass
- `pnpm format` clean
Closes cloud-ib7.
…etry processTopUp commits the credit_transactions row and then fires the top-up confirmation email via after(). If the process exited between those two steps, the credit-transactions unique index deduped the credit on webhook retry but the email was lost — the retry bailed early on the duplicate insert before reaching the email step. Add a top_up_email_log outbox marker keyed by stripe_payment_id and run the same marker-gated send on both first attempts and duplicate-webhook retries. Mirrors the existing maybeSendKiloClawSubscriptionStartedEmail pattern in credit-billing.ts: - First-attempt send inserts the marker before sending. - Duplicate-webhook path observes the committed credit, attempts the same marker-gated send, and only fires if no prior send has been recorded. - skipPostTopUpFreeStuff is respected on the retry path so Kilo-Pass-style internal reuses of processTopUp cannot send user-facing top-up emails. - provider_not_configured clears the marker so future retries can re-attempt; neverbounce_rejected is intentionally kept as a terminal state. Retain top_up_email_log rows on softDeleteUser (financial outbox record, no PII beyond user_id which references the anonymized user row). Added GDPR retention test. Generated migration 0107_magical_rattler.
Soften the comments on the two new email dedupe paths to call out the shared at-most-once-marker gaps (crash between insert and send; catch-block rollback after ambiguous provider errors). References the sibling sites that share the same pattern so the fix is scoped to a shared outbox across all of them rather than a one-off here.
Narrow the catch in resolveStripeReceiptUrl to only silence StripeInvalidRequestError (the expected outcome when the payment was refunded/voided before the webhook arrived). Route every other error — rate-limit, API 5xx, authentication, non-Stripe programmer faults — through captureException so systemic failures become visible instead of being silently swallowed. Matches the autoTopUp.ts / admin-router.ts convention of swallowing a specific known-benign Stripe subclass and reporting the rest. The email flow still never fails on receipt-lookup errors.
On webhook replays against an already-emailed, already-settled period, applyStripeFundedKiloClawPeriod was running a kiloclaw_subscription_change_log scan plus application-side JSONB filtering on every retry, only for the subsequent marker-insert to no-op against the kiloclaw_email_log unique index. Gate the expensive scan on a fast existence check covered by the UQ_kiloclaw_email_log_user_instance_type_period index: if the activation already has an email-log row, we know the send is handled and can skip both the change-log recovery logic and the downstream send call. Correctness is unchanged — the unique index remains the authoritative idempotency guard.
evanjacobson
force-pushed
the
feat/trasnactional-emails_topup
branch
from
8cb7626 to
32dc685
Compare
This was referenced May 6, 2026
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
2 participants
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
Credit top-ups now send the transactional confirmation users expect after funds are added, closing a gap where successful top-ups were only reflected in-app.
This adds a Mailgun-backed credit top-up email with durable send markers so webhook retries can recover missed sends without double-emailing users. The implementation follows the existing server-rendered email template conventions and uses a generic transactional email log for idempotent purchase confirmations.
Implementation notes
creditsTopUptransactional email template and sender.transactional_email_logfor idempotent non-KiloClaw purchase email markers keyed by email type and Stripe payment identity.skipPostTopUpFreeStuffflows, including retry recovery.Verification
Visual Changes
Reviewer Notes
Suggested review focus
transactional_email_logschema/index change.skipPostTopUpFreeStuffhandling on both first attempt and retry recovery paths.