Skip to content

feat(email): add NeverBounce verification and remove Customer.io codepath#1493

Closed
evanjacobson wants to merge 8 commits intomainfrom
improvement/check-neverbounce-email
Closed

feat(email): add NeverBounce verification and remove Customer.io codepath#1493
evanjacobson wants to merge 8 commits intomainfrom
improvement/check-neverbounce-email

Conversation

@evanjacobson
Copy link
Contributor

@evanjacobson evanjacobson commented Mar 24, 2026

Summary

Adds NeverBounce email verification before sending transactional emails to reduce our bounce rate. Removes the unused Customer.io transactional email codepath (production has been on Mailgun).

NeverBounce integration:

  • New src/lib/email-neverbounce.ts calls the NeverBounce single-check API before every send()
  • Blocks invalid and disposable emails; allows valid, catchall, unknown
  • Fails open — NeverBounce outages, timeouts (5s), or missing API key won't block email delivery
  • Blocked sends and API failures reported to Sentry

Caller-aware rejection handling:

  • send() returns SendResult ({ sent: true } | { sent: false, reason }) so callers can react
  • Magic link route returns 400 with user-facing error when email is rejected
  • Org invite route expires the invitation row and throws BAD_REQUEST when rejected
  • Billing lifecycle cron removes idempotency row on rejection, allowing retry next run
  • Admin email testing page goes through send() so NeverBounce is exercised there too

Customer.io cleanup:

  • Deleted email-customerio.ts and removed customerio-node dependency
  • Removed EMAIL_PROVIDER config and routing logic — send() always uses Mailgun
  • Simplified admin email testing page (removed provider selector)
  • Note: external-services.ts CIO user deletion is preserved (different API keys, used for marketing audience cleanup)

Verification

Automated tests (50 passing)

  • npx tsc --noEmit — no type errors
  • email-neverbounce.test.ts — 11 tests (all result types, fail-open, Sentry reporting, timeout, params)
  • magic-link/route.test.ts — 10 tests
  • autoTopUp.test.ts — 16 tests
  • email.test.ts — 13 tests
  • pnpm lint — clean
  • 3 rounds of adversarial review (code-reviewer, silent-failure-hunter, code-simplifier) — all findings addressed, round 3 found zero new actionable issues

Manual tests (all passing)

  • Invalid email blocked: sent to fakeperson@xyznotarealdomainever.com via admin email testing → HTTP 400, "Email blocked by NeverBounce verification. This address is invalid or disposable."
  • Fake domain blocked: sent to you@admin.example.com → HTTP 400, blocked (NeverBounce returns invalid)
  • Disposable email blocked: sent to test@mailinator.com → HTTP 400, blocked (NeverBounce returns disposable)
  • Valid email allowed: sent to support@neverbounce.com → HTTP 200, email sent successfully
  • Real email delivered: sent orgSubscription template to evan@kilocode.ai → HTTP 200, email received in inbox
  • Admin page UI: no provider dropdown, only Template + Recipient fields, preview renders HTML iframe correctly, all 21 templates listed
  • NeverBounce API direct verification: confirmed invalid, disposable, and valid results via curl against the real API
  • Magic link with invalid email: attempted sign-in with invalid email → "Unable to deliver email to this address" error returned
  • Org invite with invalid email: invited user with invalid email → error returned, invitation row expired (no dangling pending invitation)
  • NeverBounce not configured (fail-open): removed NEVERBOUNCE_API_KEY, emails sent normally without verification

Visual Changes

Before After
Admin email testing page has Template / Provider / Recipient selectors (3-column grid) Template / Recipient only (2-column grid), no provider dropdown

Reviewer Notes

  • NEVERBOUNCE_API_KEY has been added to Vercel. After merge, remove CUSTOMERIO_EMAIL_API_KEY and EMAIL_PROVIDER from Vercel (keep CUSTOMERIO_SITE_ID and CUSTOMERIO_API_KEY — used by external-services.ts for marketing user deletion).
  • NeverBounce API only supports auth via query parameter — no header auth option available.
  • organization-members-router.test.ts failure is pre-existing on main (server-only import chain issue) — not caused by this PR.

Add pre-send NeverBounce verification to reduce transactional email bounce
rate (~3%). Emails classified as "invalid" or "disposable" are blocked;
"valid", "catchall", and "unknown" are allowed through. Fails open if
NeverBounce is unavailable.

Also removes the unused Customer.io email sending codepath — production
has been on Mailgun. Deletes email-customerio.ts, removes EMAIL_PROVIDER
config, customerio-node dependency, and CIO provider option from the
admin email testing page.
- Add 5s fetch timeout to prevent NeverBounce latency from blocking sends
- Validate NeverBounce response status field before trusting result
- Report NeverBounce failures to Sentry (not just console.warn)
- Return SendResult from send() so callers know if email was blocked
- Magic link route now returns 400 when NeverBounce rejects the address
- Remove dead CIO template variables from creditsVars
- Make subjects the single source of truth for TemplateName
- Org invite route now throws BAD_REQUEST when email is rejected, so the
  inviter sees an error instead of false success
- Billing lifecycle cron removes the idempotency row when NeverBounce
  blocks a send, allowing retry on the next run instead of permanently
  skipping the email
- Update test mocks to return SendResult instead of undefined
Derive templateNames from subjects instead of duplicating the list.
Consolidate identical fixture switch cases via fallthrough.
Without this, a rejected invite leaves a pending invitation in the DB
that blocks future invite attempts to the same email address.
Covers all result types (valid, invalid, disposable, catchall, unknown),
fail-open behavior (HTTP errors, network errors, non-success API status,
missing API key), Sentry reporting, and request parameters.
…ation

Admin test emails should go through the same path as production emails
so NeverBounce verification is exercised.
@evanjacobson evanjacobson marked this pull request as ready for review March 24, 2026 20:28
year: String(new Date().getFullYear()),
});
await sendViaMailgun({ to: params.to, subject, html });
return { sent: true };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: send() reports success even when Mailgun is unavailable

sendViaMailgun() returns undefined when MAILGUN_API_KEY or MAILGUN_DOMAIN is missing. This branch ignores that sentinel and still returns { sent: true }, so callers like the magic-link route, org invites, billing cron, and the admin test page will all think delivery succeeded even though no email was sent. Please propagate that failure here instead of unconditionally marking the send as successful.

@kilo-code-bot
Copy link
Contributor

kilo-code-bot bot commented Mar 24, 2026

Code Review Summary

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
src/lib/email.ts 99 send() returns { sent: true } even when Mailgun is unconfigured and no email was sent.

Fix these issues in Kilo Cloud

Other Observations (not in diff)

N/A

Files Reviewed (17 files)
  • .env.development.local.example - 0 issues
  • .env.test - 0 issues
  • package.json - 0 issues
  • pnpm-lock.yaml - 0 issues
  • src/app/admin/email-testing/page.tsx - 0 issues
  • src/app/api/auth/magic-link/route.test.ts - 0 issues
  • src/app/api/auth/magic-link/route.ts - 0 issues
  • src/lib/autoTopUp.test.ts - 0 issues
  • src/lib/config.server.ts - 0 issues
  • src/lib/email-customerio.ts - 0 issues
  • src/lib/email-neverbounce.test.ts - 0 issues
  • src/lib/email-neverbounce.ts - 0 issues
  • src/lib/email.ts - 1 issue
  • src/lib/kiloclaw/billing-lifecycle-cron.ts - 0 issues
  • src/routers/admin/email-testing-router.ts - 0 issues
  • src/routers/organizations/organization-members-router.test.ts - 0 issues
  • src/routers/organizations/organization-members-router.ts - 0 issues

Reviewed by gpt-5.4-20260305 · 494,615 tokens

@evanjacobson
Copy link
Contributor Author

splitting into #1498 and #1499

evanjacobson added a commit that referenced this pull request Mar 25, 2026
)

## Summary

Adds NeverBounce email verification before sending transactional emails
to reduce our ~3% bounce rate. Blocks `invalid` and `disposable` emails;
allows `valid`, `catchall`, `unknown` through. Fails open if NeverBounce
is unavailable.

**How it works:**
- New `src/lib/email-neverbounce.ts` calls the NeverBounce single-check
API with a 5s timeout
- `send()` returns `SendResult` (`{ sent: true } | { sent: false, reason
}`) so callers can react
- Blocked sends and API failures reported to Sentry

**Caller handling:**
- Magic link route returns 400 with "Unable to deliver email to this
address"
- Org invite route expires the invitation row and throws `BAD_REQUEST`
- Billing lifecycle cron removes idempotency row on rejection, allowing
retry next run
- Admin email testing routes through `send()` so NeverBounce is
exercised

## Verification

### Automated tests (50 passing)
- [x] `npx tsc --noEmit` — no type errors
- [x] `pnpm lint` — clean
- [x] `email-neverbounce.test.ts` — 11 tests (all result types,
fail-open, Sentry reporting, timeout, params)
- [x] `magic-link/route.test.ts` — 10 tests
- [x] `autoTopUp.test.ts` — 16 tests
- [x] `email.test.ts` — 13 tests
- [x] Verified via automated agent that this branch preserves all CIO
code intact (10/10 checks pass)

### Manual tests (all passing on localhost)
- [x] **Invalid email blocked**:
`fakeperson@xyznotarealdomainever.comtld` → HTTP 400
- [x] **Disposable email blocked**: `test@mailinator.com` → HTTP 400
- [x] **Valid email allowed**: `support@neverbounce.com` → HTTP 200
- [x] **Real email delivered**: `evan@kilocode.ai` → email received in
inbox
- [x] **Magic link with invalid email**: returns "Unable to deliver"
error
- [x] **Org invite with invalid email**: error returned, invitation
expired
- [x] **NeverBounce not configured (fail-open)**: emails send normally
without API key

## Visual Changes

N/A — no UI changes in this PR (admin page provider selector is
unchanged).

## Reviewer Notes

- `NEVERBOUNCE_API_KEY` has been added to Vercel.
- This PR is independent of the CIO removal PR (#1498) — they can merge
in either order. When combined, they produce the same result as the
original PR #1493.
- NeverBounce API only supports auth via query parameter — no header
auth option available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant