Swap SMTP email notifications for Resend HTTP API#100
Conversation
Captures the decision to replace the SMTP-based email channel with Resend's HTTP API: global API key via env, per-channel from+to, HTML plus text body, lettre dropped, migration disables rather than deletes legacy rows.
Address five review findings: add ResendConfig wiring to AppConfig, require update-path config revalidation, spell out the frontend state model change for to: string[], replace the form-scoped missing-key hint with static help text plus toast surfacing, and introduce an English-only EMAIL_TEXT_TEMPLATE instead of reusing the Chinese DEFAULT_TEMPLATE that Webhook and Telegram still rely on.
Close the open gap from the prior review: dispatch needs the Resend API key, but send_group, test_notification, and the alert/service monitor call sites currently stop at &DatabaseConnection. Spec now specifies extending send_group/test_notification/dispatch and the surrounding AlertService functions to accept &AppConfig, and lists the four caller files that already hold state.config to pass it in.
Sixteen bite-sized tasks covering config struct, schema refactor, AppConfig threading, update-path validation, html template, resend http swap, lettre removal, data migration with integration test, frontend form refactor, i18n updates, docs updates in en + cn, and the final verification gate.
Adds an integration test that exercises the email-to-resend schema migration against a real in-memory SQLite, covering the happy path (valid SMTP row → resend shape), the disable path (unconvertable row retains name marker + enabled=0), and an empty-table no-op. The new test caught a pre-existing bug: the migration referenced the table as `notification` (singular) while the actual table name is `notifications` (plural, per the entity and init migration). Fix the three SQL statements to use the correct table name so production databases with legacy email notifications will migrate cleanly.
Add an integration test that exercises NotificationService::update against an in-memory sqlite DB to verify merged-payload re-validation (empty `to` and type-switch without matching config_json are both rejected, valid partial updates succeed). Extract EmailFormFields from the notifications route into a purely presentational component and add render tests asserting that SMTP fields are absent and that the from/recipient UI is present.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 40 minutes and 5 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis pull request migrates email notifications from SMTP (via Changes
Sequence Diagram(s)sequenceDiagram
participant Alert as Alert System
participant Service as Notification Service
participant Resend as Resend API
participant DB as Database
rect rgba(100, 150, 200, 0.5)
Note over Alert,DB: New Flow: Alert → Resend Email Dispatch
Alert->>Service: send_group(db, config, group_id, ctx)
Service->>DB: Query notification channel config
DB-->>Service: Email channel { from, to: [...] }
Service->>Service: Validate from/to plausibility
Service->>Service: Render HTML body + plain text
Service->>Resend: POST /emails<br/>{from, to[], subject, html, text}
Resend-->>Service: 200 OK or error
Service->>Service: Parse response / handle errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
crates/server/src/migration/m20260416_000017_migrate_email_to_resend.rs (1)
75-86: Harden legacy-value validation by trimmingfrom/tobefore emptiness checks.Whitespace-only addresses are currently treated as valid and will stay enabled after migration.
Suggested hardening patch
- let from = val + let from = val .get("from") .and_then(|v| v.as_str()) - .ok_or_else(|| "missing 'from' field".to_string())?; - let to = val + .map(str::trim) + .ok_or_else(|| "missing 'from' field".to_string())?; + let to = val .get("to") .and_then(|v| v.as_str()) - .ok_or_else(|| "missing 'to' field".to_string())?; + .map(str::trim) + .ok_or_else(|| "missing 'to' field".to_string())?;Also applies to: 88-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/migration/m20260416_000017_migrate_email_to_resend.rs` around lines 75 - 86, The current validation in migrate_email_to_resend (the block extracting `from` and `to` from `val`) doesn't reject whitespace-only addresses; update the logic to trim both `from` and `to` (e.g., call .trim() on the &str) and perform the emptiness check on the trimmed values, returning the existing error ("empty from/to") when trimmed().is_empty(); apply the same trimming + empty-check fix to the second occurrence of the check later in the function (the block at the other occurrence referenced in the review).apps/web/src/routes/_authed/settings/notifications.tsx (1)
491-497: Consider adding user feedback for empty recipient list.When
toAddresses.length === 0for email type, the form silently returns without submitting. Users may not understand why the submit button appears to do nothing.💡 Suggested improvement
if (notifyType === 'email') { if (toAddresses.length === 0) { + toast.error(t('notifications.recipients_required')) return } submitChannel(buildEmailPayload(configFields.from ?? '', toAddresses)) return }This would require adding a new i18n key like
notifications.recipients_required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authed/settings/notifications.tsx` around lines 491 - 497, When notifyType === 'email' and toAddresses.length === 0 the handler currently returns silently; instead surface a user-facing error using the app's existing UI feedback (e.g., setFormError, setToast, or similar) with a new i18n key notifications.recipients_required so users know recipients are required; locate the email branch in the submit logic (symbols: notifyType, toAddresses, submitChannel, buildEmailPayload) and replace the early return with a call to the feedback mechanism using t('notifications.recipients_required') (or equivalent) and prevent submit until a recipient is added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/content/docs/cn/alerts.mdx`:
- Around line 196-197: Update the Chinese configuration link path in the
alerts.mdx content: replace the incorrect link target "/docs/cn/configuration"
with the site-consistent "/cn/docs/configuration" so the anchor in the sentence
"在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/docs/cn/configuration)页面)。" becomes
"在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/cn/docs/configuration)页面)。" to match
other Chinese doc links and avoid broken navigation.
In `@apps/web/src/locales/zh/settings.json`:
- Around line 90-95: The translation uses "通道" for channel in keys
notifications.update_channel, notifications.toast_channel_updated, and
notifications.channel_update_failed, which is inconsistent with existing
translations like notifications.channels and notifications.create_failed that
use "渠道"; update the values for those three keys to use "渠道" (e.g., "更新渠道",
"渠道已更新", "更新渠道失败") so terminology is consistent across the locale.
In `@docs/superpowers/plans/2026-04-16-resend-email-integration.md`:
- Around line 931-933: The SQL snippets in the migration plan use the wrong
table name "notification" — update all occurrences of the query strings like
"SELECT id, name, config_json FROM notification WHERE notify_type = 'email'"
(and the other similar SELECTs shown) to use the correct plural table name
"notifications" so they match the implemented migration and tests; search for
the exact SQL fragments in the document and replace "notification" with
"notifications" in each query.
- Around line 52-53: Summary: The test invocation uses the --exact flag which
prevents the module filter config::tests::test_resend from matching the three
concrete tests. Fix: remove the --exact flag from the test command so cargo can
match all tests under the module config::tests::test_resend, or instead run each
concrete test by its full exact name (the three concrete test functions under
config::tests::test_resend) if you truly need exact matching.
In `@docs/superpowers/specs/2026-04-16-resend-email-integration-design.md`:
- Line 149: Inline code spans contain a leading space (e.g. "` (needs
reconfiguration)`") which triggers MD038; remove the leading space inside each
code span so the text reads "(needs reconfiguration)" (i.e. change "` (needs
reconfiguration)`" to "`(needs reconfiguration)`") and do the same fix for the
other three offending inline code spans in this document so there are no spaces
immediately after the opening backtick.
---
Nitpick comments:
In `@apps/web/src/routes/_authed/settings/notifications.tsx`:
- Around line 491-497: When notifyType === 'email' and toAddresses.length === 0
the handler currently returns silently; instead surface a user-facing error
using the app's existing UI feedback (e.g., setFormError, setToast, or similar)
with a new i18n key notifications.recipients_required so users know recipients
are required; locate the email branch in the submit logic (symbols: notifyType,
toAddresses, submitChannel, buildEmailPayload) and replace the early return with
a call to the feedback mechanism using t('notifications.recipients_required')
(or equivalent) and prevent submit until a recipient is added.
In `@crates/server/src/migration/m20260416_000017_migrate_email_to_resend.rs`:
- Around line 75-86: The current validation in migrate_email_to_resend (the
block extracting `from` and `to` from `val`) doesn't reject whitespace-only
addresses; update the logic to trim both `from` and `to` (e.g., call .trim() on
the &str) and perform the emptiness check on the trimmed values, returning the
existing error ("empty from/to") when trimmed().is_empty(); apply the same
trimming + empty-check fix to the second occurrence of the check later in the
function (the block at the other occurrence referenced in the review).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96e42c17-60c8-425c-a265-ac4e933106fd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
ENV.mdREADME.mdREADME.zh-CN.mdapps/docs/content/docs/cn/alerts.mdxapps/docs/content/docs/cn/configuration.mdxapps/docs/content/docs/en/alerts.mdxapps/docs/content/docs/en/configuration.mdxapps/web/src/locales/en/common.jsonapps/web/src/locales/en/settings.jsonapps/web/src/locales/zh/common.jsonapps/web/src/locales/zh/settings.jsonapps/web/src/routes/_authed/settings/notifications.test.tsxapps/web/src/routes/_authed/settings/notifications.tsxcrates/server/Cargo.tomlcrates/server/src/config.rscrates/server/src/migration/m20260416_000017_migrate_email_to_resend.rscrates/server/src/migration/mod.rscrates/server/src/router/api/notification.rscrates/server/src/router/ws/agent.rscrates/server/src/service/alert.rscrates/server/src/service/notification.rscrates/server/src/task/alert_evaluator.rscrates/server/src/task/service_monitor_checker.rscrates/server/tests/email_migration_integration.rscrates/server/tests/notification_update_integration.rsdocs/superpowers/plans/2026-04-16-resend-email-integration.mddocs/superpowers/specs/2026-04-16-resend-email-integration-design.mdtests/alerts-notifications.md
| 1. 在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/docs/cn/configuration)页面)。 | ||
| 2. 在 [resend.com/domains](https://resend.com/domains) 添加并验证发件域名。各通道的 `from` 必须属于已验证的域名。 |
There was a problem hiding this comment.
Fix the Chinese configuration link path format.
This link uses /docs/cn/..., but other Chinese docs links on this page use /cn/docs/.... Please align it to avoid potential broken navigation.
🛠️ Suggested doc fix
-1. 在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/docs/cn/configuration)页面)。
+1. 在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/cn/docs/configuration)页面)。📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. 在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/docs/cn/configuration)页面)。 | |
| 2. 在 [resend.com/domains](https://resend.com/domains) 添加并验证发件域名。各通道的 `from` 必须属于已验证的域名。 | |
| 1. 在服务器设置 `SERVERBEE_RESEND__API_KEY`(参考[配置](/cn/docs/configuration)页面)。 | |
| 2. 在 [resend.com/domains](https://resend.com/domains) 添加并验证发件域名。各通道的 `from` 必须属于已验证的域名。 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/content/docs/cn/alerts.mdx` around lines 196 - 197, Update the
Chinese configuration link path in the alerts.mdx content: replace the incorrect
link target "/docs/cn/configuration" with the site-consistent
"/cn/docs/configuration" so the anchor in the sentence "在服务器设置
`SERVERBEE_RESEND__API_KEY`(参考[配置](/docs/cn/configuration)页面)。" becomes "在服务器设置
`SERVERBEE_RESEND__API_KEY`(参考[配置](/cn/docs/configuration)页面)。" to match other
Chinese doc links and avoid broken navigation.
| "notifications.update_channel": "更新通道", | ||
| "notifications.update_group": "更新通知组", | ||
| "notifications.toast_channel_updated": "通道已更新", | ||
| "notifications.toast_group_updated": "通知组已更新", | ||
| "notifications.channel_update_failed": "更新通道失败", | ||
| "notifications.group_update_failed": "更新通知组失败", |
There was a problem hiding this comment.
Terminology inconsistency: "通道" vs "渠道" for channel.
The new strings use "通道" (lines 90, 92, 94) while existing channel-related keys use "渠道" (e.g., notifications.channels = "通知渠道", notifications.create_failed = "创建通知渠道失败"). Consider using "渠道" consistently:
- "notifications.update_channel": "更新通道",
+ "notifications.update_channel": "更新渠道",
...
- "notifications.toast_channel_updated": "通道已更新",
+ "notifications.toast_channel_updated": "渠道已更新",
- "notifications.channel_update_failed": "更新通道失败",
+ "notifications.channel_update_failed": "更新渠道失败",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "notifications.update_channel": "更新通道", | |
| "notifications.update_group": "更新通知组", | |
| "notifications.toast_channel_updated": "通道已更新", | |
| "notifications.toast_group_updated": "通知组已更新", | |
| "notifications.channel_update_failed": "更新通道失败", | |
| "notifications.group_update_failed": "更新通知组失败", | |
| "notifications.update_channel": "更新渠道", | |
| "notifications.update_group": "更新通知组", | |
| "notifications.toast_channel_updated": "渠道已更新", | |
| "notifications.toast_group_updated": "通知组已更新", | |
| "notifications.channel_update_failed": "更新渠道失败", | |
| "notifications.group_update_failed": "更新通知组失败", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/locales/zh/settings.json` around lines 90 - 95, The translation
uses "通道" for channel in keys notifications.update_channel,
notifications.toast_channel_updated, and notifications.channel_update_failed,
which is inconsistent with existing translations like notifications.channels and
notifications.create_failed that use "渠道"; update the values for those three
keys to use "渠道" (e.g., "更新渠道", "渠道已更新", "更新渠道失败") so terminology is consistent
across the locale.
| Run: `cargo test -p serverbee-server --lib config::tests::test_resend -- --exact` | ||
| Expected: three tests fail with "cannot find type `ResendConfig` in this scope" and "no field `resend`". |
There was a problem hiding this comment.
Fix the test command: --exact currently prevents the intended tests from running.
Using --exact with config::tests::test_resend won’t match the three concrete test names.
Suggested doc fix
-Run: `cargo test -p serverbee-server --lib config::tests::test_resend -- --exact`
+Run: `cargo test -p serverbee-server --lib config::tests::test_resend`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Run: `cargo test -p serverbee-server --lib config::tests::test_resend -- --exact` | |
| Expected: three tests fail with "cannot find type `ResendConfig` in this scope" and "no field `resend`". | |
| Run: `cargo test -p serverbee-server --lib config::tests::test_resend` | |
| Expected: three tests fail with "cannot find type `ResendConfig` in this scope" and "no field `resend`". |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-04-16-resend-email-integration.md` around lines
52 - 53, Summary: The test invocation uses the --exact flag which prevents the
module filter config::tests::test_resend from matching the three concrete tests.
Fix: remove the --exact flag from the test command so cargo can match all tests
under the module config::tests::test_resend, or instead run each concrete test
by its full exact name (the three concrete test functions under
config::tests::test_resend) if you truly need exact matching.
| "SELECT id, name, config_json FROM notification WHERE notify_type = 'email'", | ||
| [], | ||
| )) |
There was a problem hiding this comment.
Use the correct table name in the migration plan SQL snippets (notifications).
This plan still shows notification (singular), which is inconsistent with the implemented migration and integration tests.
Suggested doc fix
-"SELECT id, name, config_json FROM notification WHERE notify_type = 'email'",
+"SELECT id, name, config_json FROM notifications WHERE notify_type = 'email'",
-"UPDATE notification SET config_json = ? WHERE id = ?",
+"UPDATE notifications SET config_json = ? WHERE id = ?",
-"UPDATE notification SET name = ?, enabled = 0 WHERE id = ?",
+"UPDATE notifications SET name = ?, enabled = 0 WHERE id = ?",Also applies to: 946-947, 960-961
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-04-16-resend-email-integration.md` around lines
931 - 933, The SQL snippets in the migration plan use the wrong table name
"notification" — update all occurrences of the query strings like "SELECT id,
name, config_json FROM notification WHERE notify_type = 'email'" (and the other
similar SELECTs shown) to use the correct plural table name "notifications" so
they match the implemented migration and tests; search for the exact SQL
fragments in the document and replace "notification" with "notifications" in
each query.
| 1. `SELECT * FROM notification WHERE notify_type='email'`. | ||
| 2. For each row, parse the old JSON: | ||
| - Extract `from` (string) and `to` (string). Rewrite `config_json` to `{ "from": <from>, "to": [<to>] }`. | ||
| - If either field is missing or unparseable: **do not delete the row**. Set `enabled = false` and append ` (needs reconfiguration)` to `name`. Log a `tracing::warn!` with the row id. |
There was a problem hiding this comment.
Fix markdownlint MD038 code-span spacing warnings.
These four inline code spans include leading spaces, which triggers no-space-in-code.
🛠️ Proposed doc-lint fix
- - If either field is missing or unparseable: **do not delete the row**. Set `enabled = false` and append ` (needs reconfiguration)` to `name`. Log a `tracing::warn!` with the row id.
+ - If either field is missing or unparseable: **do not delete the row**. Set `enabled = false` and append `" (needs reconfiguration)"` to `name`. Log a `tracing::warn!` with the row id.
-- Rows disabled by the migration (` (needs reconfiguration)` suffix) use the existing "disabled" visual — no special-casing.
+- Rows disabled by the migration (`"(needs reconfiguration)"` suffix, including a leading space) use the existing "disabled" visual — no special-casing.
-- Insert a malformed row (missing `from`). Run the migration. Assert `enabled = false` and `name` ends with ` (needs reconfiguration)`.
+- Insert a malformed row (missing `from`). Run the migration. Assert `enabled = false` and `name` ends with `" (needs reconfiguration)"`.
-| Migration disables a row the user did not want disabled | Renaming to ` (needs reconfiguration)` makes the state visible; user can re-enable after filling in the Resend fields. Row is never deleted. |
+| Migration disables a row the user did not want disabled | Renaming to `" (needs reconfiguration)"` makes the state visible; user can re-enable after filling in the Resend fields. Row is never deleted. |Also applies to: 223-223, 256-256, 292-292
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 149-149: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/specs/2026-04-16-resend-email-integration-design.md` at line
149, Inline code spans contain a leading space (e.g. "` (needs
reconfiguration)`") which triggers MD038; remove the leading space inside each
code span so the text reads "(needs reconfiguration)" (i.e. change "` (needs
reconfiguration)`" to "`(needs reconfiguration)`") and do the same fix for the
other three offending inline code spans in this document so there are no spaces
immediately after the opening backtick.
Resolve conflicts with the agent-recovery-merge feature: - Renumber email migration 000017 -> 000018 (main added m20260416_000017_create_recovery_job). - In ws/agent.rs, preserve the new recovery-lock gate around AlertService::check_event_rules while threading &state.config through both call sites.
Summary
lettreSMTP email channel with Resend's HTTP API. New env varSERVERBEE_RESEND__API_KEY; channel config shrinks from 6 SMTP fields to{from, to: string[]}.AppConfigis threaded through the notification/alert dispatch chain.lettreis removed.html-escape.m20260416_000017_migrate_email_to_resendrewrites legacy rows to the new shape or disables unconvertable rows with a(needs reconfiguration)suffix. The notifications settings page gains an edit flow (channels + groups) with anenabledtoggle so disabled rows can be repaired in-UI; type is locked during edit to avoid accidental conversion. Email addresses are validated on both sides (@+ dot, non-empty parts).ENV.md,configuration.mdx,alerts.mdx, README) and i18n (en + zh) updated; tests added: 8 new Rust tests innotificationservice + 3-case migration unit + 3-case migration integration + 4-caseNotificationService::updateintegration +buildEmailPayload/EmailFormFieldsvitest.Test plan
cargo build --workspacecargo clippy --workspace -- -D warningscargo test --workspace(one pre-existing unrelated failure on main:test_server_detail_returns_runtime_capability_fields)bun run typecheckbun x ultracite checkcd apps/web && bun run test(345 / 345)SERVERBEE_RESEND__API_KEY, create an email channel with a verified-domainfrom, click "Test notification" — email arrives with colour-coded header and plain-text fallback.SERVERBEE_RESEND__API_KEY.(needs reconfiguration)suffix; opening the Edit form exposesfrom, recipients, and an enabled switch that saves successfully.Summary by CodeRabbit
Release Notes
New Features
Documentation