Skip to content

feat(email): inbox notification email channel - #4105

Open
r404r wants to merge 18 commits into
multica-ai:mainfrom
r404r:feat/email-notifications
Open

r404r wants to merge 18 commits into
multica-ai:mainfrom
r404r:feat/email-notifications

Conversation

@r404r

@r404r r404r commented Jun 13, 2026

Copy link
Copy Markdown

Summary

Add email as a delivery channel for inbox notifications. When inbox writes a new
row for a workspace member, an EmailNotifier subscribed to EventInboxNew
renders a subject / text / html pair and dispatches via the existing
EmailService transport chain (SMTP relay → Resend API → DEV stdout).

Today Multica's notification fan-out only writes inbox rows and emits WS pushes;
email is wired for OTP and invitation flows but never reaches end users for
issue activity. This PR closes that gap.

Design notes

  • Strictly downstream of inbox. If a user mutes "Comments & Mentions" at the
    inbox layer, no inbox row is written and no email fires. The new
    email_notifications preference is a channel-level kill switch on top of the
    existing per-event groups.
  • Async send. The bus subscription callback spawns a goroutine; Publish
    returns immediately, so a slow SMTP host can never block the HTTP request
    that triggered the inbox row.
  • Configured gate. Notifier.Register is only called when
    EmailService.Configured() reports a real transport. Without it, the DEV
    stdout fallback would leak recipient + rendered HTML into server logs on
    every inbox row.
  • Frontend mirrors BrowserNotificationSetting. Single row under System
    Notifications. When email_configured is false, an "Unavailable" badge +
    hint pointing at SMTP_HOST / RESEND_API_KEY. Otherwise a toggle that
    writes email_notifications into notification_preference.
  • Deep links use FRONTEND_ORIGIN, the same convention SendInvitationEmail
    uses. MULTICA_PUBLIC_URL is the API host and would 404 in split self-host
    setups.

What's added

Backend

  • server/internal/integrations/email/ — new package (Notifier, Renderer,
    EmailSender interface, types).
  • server/cmd/server/email_notifier_adapter.go — bridges *db.Queries onto
    the package-local NotifierQueries interface.
  • EmailService.SendNotification(to, subject, text, html) — generic
    entrypoint, same dispatch order as the existing methods.
  • EmailService.Configured() — gate for downstream subscribers.
  • GET /api/config exposes email_configured: true when SMTP_HOST or
    RESEND_API_KEY is set.
  • notification_preference.preferences JSONB accepts a new
    email_notifications key (missing or "all" = deliver; "muted" =
    suppress). No new tables, no migration.

Frontend

  • packages/views/settings/components/email-notification-setting.tsx — new
    component mounted in notifications-tab.tsx.
  • packages/core/config/index.ts — ConfigState gains emailConfigured;
    populated at boot in auth-initializer.tsx alongside
    workspaceCreationDisabled.
  • packages/core/types/notification-preference.ts — NotificationGroupKey
    union extended.
  • packages/views/locales/{en,zh-Hans,ja,ko}/settings.json — added
    notifications.email.* keys; parity test enforced.

Out of scope (deferred to phase 2+)

  • Email-body i18n by recipient locale (today: English only — EventInboxNew
    doesn't carry user.language).
  • Multipart/alternative for SMTP path (today: HTML only; Resend gets both).
  • Rate limiting / debounce for @all bursts.
  • Retry queue / dead letter handling.
  • Inbound email (replies threaded onto issue comments).
  • Provider abstraction beyond the existing SMTP/Resend split.

Test plan

  • go test ./internal/integrations/email/ — 9 tests pass (Renderer + 5
    Notifier tests including the goroutine decoupling regression and *string
    issue_id regression).
  • go test ./internal/service/ — EmailService.SendNotification and
    Configured() covered, no existing tests regressed.
  • go build ./... clean.
  • pnpm vitest run settings/components/email-notification-setting — 5
    tests pass.
  • pnpm vitest run parity — locale parity OK (157 tests).
  • pnpm typecheck — all packages clean.
  • Manual end-to-end (chat-driven, 2026-06-13, dev DB + MailDev SMTP sink
    at localhost:1025):
    • A. DEV mode (no SMTP_HOST, no RESEND_API_KEY) — Settings →
      Notifications shows the "Email notifications" row with "Unavailable"
      badge; no Switch rendered. Server log confirms Notifier did not
      register against the bus (Configured() gate at work).
image
  • B. SMTP configured — /api/config returns email_configured: true; toggle appears, defaults to ON; subtitle changes to
    You'll get an email when there's new inbox activity..
image
  • C. Issue assigned + earlier @mention / new_comment — MailDev
    received [Multica] You were assigned: …, [Multica] You were mentioned: …, and [Multica] New comment: … (multiple
    actionVerb switch branches exercised). POST /api/issues
    returned 201 in 54.4 ms even with downstream SMTP send —
    proves the goroutine decoupling (P1 fix).
image
  • D. Recipient toggle off → no email — Verified
    bidirectionally in one round:
    - dba toggled own email_notifications off; bob (toggle ON) was
    @-mentioned by dba → bob received the email.
    - bob (toggle ON, actor) @-mentioned dba (toggle OFF) → dba
    received the inbox row but no email (MailDev count
    unchanged).
    - POST /api/issues/.../comments returned in 23 ms in this
    round — async path holds under multi-recipient fan-out.
  • E. Inbox group muted → no inbox row → no email (the strict
    downstream invariant)
    — dba turned "Comments & Mentions" off
    AND email_notifications on; bob then @-mentioned dba again.
    Result: no new inbox row for dba, no new email, MailDev
    unchanged. The architecture invariant is observable: when
    notifyMentionedMembers early-returns, EventInboxNew is never
    published and the Notifier is never invoked. POST still
    returned in 25 ms.
  • F. Agent task (task_completed / task_failed) — covered by
    unit tests rather than manual run (setting up a daemon runtime
    is out of scope for this verification):
    - actionVerb branches for task_completed / task_failed
    render correctly (TestRender_UnknownNotifTypeFallsBackToGenericSubject
    locks down the switch path).
    - Agent recipients are skipped at the recipient_type filter
    (TestNotifier_SkipsNonMemberRecipients).
  • G. No goroutine leak under load — implicit from C/D/E timings:
    10+ POST paths all returned in 23-55 ms with zero email notifier: ERROR/WARN/panic across a ~30-minute session.
    TestNotifier_DoesNotBlockBusPublisher locks the goroutine
    decoupling pattern in CI; an explicit pprof goroutine count
    comparison would require a load-injection step that's not
    warranted at this scope.

Configuration

env Effect
SMTP_HOST (or RESEND_API_KEY) Enables the channel. With neither set, the Notifier is not registered.
SMTP_PORT, SMTP_USERNAME, SMTP_PASSWORD, SMTP_TLS, SMTP_TLS_INSECURE, SMTP_EHLO_NAME, RESEND_FROM_EMAIL Unchanged — passed through EmailService exactly as before.
FRONTEND_ORIGIN Base for the "View issue" deep link in the email body. Self-host should already have this set for invitation emails / CORS. Missing → email still ships, just without a link.

Review history

Five codex review passes during development surfaced four real issues, all
fixed before this squash:

  1. R1 / P1 — events.Bus.Publish is synchronous, so an inline SMTP send
    would block the originating HTTP request. Fixed by spawning a goroutine in
    the bus callback (regression test
    TestNotifier_DoesNotBlockBusPublisher).
  2. R1 / P2 — issue_id arrives as *string (from util.UUIDToPtr in
    inboxItemToResponse), not string; the original assertion silently
    produced "" and emails lost their deep link. Fixed via stringOrPtr
    helper (regression: happy-path test now asserts the deep link).
  3. R2 / P2 — Deep links were built from MULTICA_PUBLIC_URL (API host).
    Switched to FRONTEND_ORIGIN to match SendInvitationEmail.
  4. R3 / P2 — Notifier was registered even without a configured transport,
    so the DEV stdout fallback printed every notification body into server
    logs on self-host instances that hadn't set up email. Added
    EmailService.Configured() gate.

(There was also an R4 finding about stale Vitest assertions left over from an
earlier i18n edit — fixed in the same branch.)

Final R5 review pass returned clean.

Add email as a delivery channel for inbox notifications. When an
inbox row is created for a workspace member, an EmailNotifier
subscribed to EventInboxNew renders a subject/text/html pair and
hands it to EmailService, reusing the existing SMTP relay → Resend
API → DEV stdout transport chain.

Backend
- EmailNotifier (server/internal/integrations/email/) subscribes
  to EventInboxNew. Register's callback spawns a goroutine so
  events.Bus.Publish never blocks on SMTP / Resend latency.
- Notifier registration is gated on EmailService.Configured().
  Without a real transport the DEV stdout fallback would otherwise
  leak the recipient + rendered HTML into server logs on every
  inbox row.
- Renderer produces subject + text + html with HTML-escaped issue
  title and an absolute deep link rooted at FRONTEND_ORIGIN (the
  same convention SendInvitationEmail follows; MULTICA_PUBLIC_URL
  is the API host and would 404 on split self-host setups).
- EmailService.SendNotification: generic (to, subject, text, html)
  entrypoint, same SMTP → Resend → DEV stdout dispatch order as
  the existing SendVerificationCode / SendInvitationEmail flows.
- GET /api/config exposes email_configured: true when SMTP_HOST or
  RESEND_API_KEY is set.
- notification_preference accepts a new channel-level group
  email_notifications (missing or \"all\" = deliver; \"muted\" =
  suppress).

Frontend
- EmailNotificationSetting mirrors BrowserNotificationSetting: a
  single row under System Notifications. When email_configured is
  false, shows an \"Unavailable\" badge and a hint pointing at
  SMTP_HOST / RESEND_API_KEY. Otherwise renders a toggle that
  writes email_notifications into notification_preference.
- Localized to en / zh-Hans / ja / ko (parity test enforces
  consistency across locales).
- Zustand configStore extended with emailConfigured, populated at
  boot in auth-initializer like the existing
  workspaceCreationDisabled flag.

Architectural notes
- Email is strictly downstream of inbox: if a user mutes
  \"Comments & Mentions\" at the inbox layer, no inbox row is
  written and no email fires. The email-channel toggle is the
  kill switch above all event groups.
- Best-effort delivery. No retry queue. SMTP / Resend failures are
  logged and swallowed. Multipart/alternative for SMTP, recipient
  locale routing, rate limiting, inbound replies, and
  multi-provider abstractions are deferred to phase 2-4.
- Send path is decoupled from the bus publisher via a goroutine
  per event; tests use Notifier.WaitInflight() to synchronize.
- Payload handling tolerates both string and *string for
  issue_id, matching the production shape that
  notification_listeners.inboxItemToResponse emits via
  util.UUIDToPtr.

Tests
- Go: 9 tests in server/internal/integrations/email (Renderer +
  Notifier; includes regression tests for the goroutine
  decoupling and the *string issue_id path) plus 4 subtests for
  EmailService.Configured() in server/internal/service.
- TS: 5 tests in EmailNotificationSetting (Unavailable badge,
  toggle checked / unchecked, mutate-on, mutate-off) plus the
  locale parity suite (157 tests).
- Reviewed across 5 codex review rounds (4 findings, all fixed
  in this branch before squash).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 13, 2026

Copy link
Copy Markdown

@r404r is attempting to deploy a commit to the IndexLabs Team on Vercel.

A member of the Team first needs to authorize it.

@r404r
r404r marked this pull request as ready for review June 15, 2026 23:01
Brings in 22 upstream commits since 04a0677. Resolves one manual
conflict on packages/core/config/index.ts where upstream replaced
setCdnDomain(string) with setCdnConfig({cdnDomain, cdnSigned?}):
kept the emailConfigured field this branch adds and the extended
setAuthConfig signature, dropped the now-stale setCdnDomain in
favor of upstream's setCdnConfig. The auth-initializer.tsx caller
was auto-merged to the new API.

Verified: go build ./... clean; pnpm typecheck on
@multica/core + @multica/views + @multica/web PASS.

PR multica-ai#4105.
r404r added a commit to r404r/multica that referenced this pull request Jun 17, 2026
Resolves two conflicts in server/internal/service/email.go and
server/internal/service/email_test.go where:

- email.go imports: kept both 'encoding/base64' (added upstream by
  PR multica-ai#4157 Office 365 SMTP LOGIN fallback) and 'errors' (added by
  email PR multica-ai#4105 for SendNotification validation).

- email_test.go: kept both my notification-suite tests
  (TestConfigured, TestSendNotification_*) and upstream's new SMTP
  LOGIN security tests (TestLoginAuth_Start_*, TestSendSMTP_*) —
  both at end of file, no logical overlap.

Verified:
  - go build ./... clean
  - go test ./internal/service/... passes (email + login auth)
  - pnpm typecheck across core/views/web clean
  - Unrelated test failures (scheduler sys_cron_executions,
    middleware totp_secret_encrypted) are pre-existing test-DB
    schema drift, not regressions from this merge.

Upstream batch highlights: custom runtime profiles (MUL-3284),
freeze breadcrumb desktop crash dump, exception dedupe/redact,
agent comment retry (MUL-3328), autopilot subscriber rework,
Office 365 SMTP LOGIN fallback (PR multica-ai#4157).
r404r and others added 12 commits September 25, 2026 12:20
Brings the branch up to upstream/main e2f4a22. Conflicts resolved:

- packages/core/api/schemas.ts: keep email_configured next to upstream's
  new AppConfig capability fields (vcs_integration_available, feature flags,
  local_worktree_supported, ...), in both the type and the zod schema.
- packages/core/config/index.ts: keep emailConfigured in ConfigState, its
  default, and the setAuthConfig signature alongside upstream's
  vcsIntegrationAvailable and other new fields.
- packages/core/platform/auth-initializer.tsx: pass emailConfigured and
  vcsIntegrationAvailable to setAuthConfig.
- packages/views/settings/components/notifications-tab.tsx: take upstream's
  SettingsTab/SettingsSection layout; render EmailNotificationSetting after
  BrowserNotificationSetting in the delivery section.
- server/cmd/server/router.go: keep the email integration import next to
  upstream's channel/composio/dingtalk imports.
- server/internal/handler/config.go: keep EmailConfigured next to upstream's
  new AppConfig fields.
- server/internal/handler/notification_preference_test.go (add/add): take
  upstream's DB-backed PATCH tests (multica-ai#6486) and keep this branch's
  validNotifGroups test.
- server/internal/service/email.go: take upstream's transport changes
  (Office 365 LOGIN auth, SMTP_FROM_EMAIL, base64 headers); keep
  Configured() and SendNotification() on top.
- server/internal/service/email_test.go: keep both test sets; the conflict
  marker split TestSendNotification_RejectsEmptyTo, closed explicitly.

Verified: go build ./... and go vet on touched packages; pnpm typecheck
core/views/web/desktop.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream now sends notification preference updates as an atomic diff
built from a fixed list of group keys (MUL-4798). email_notifications was
not in that list, so the email toggle PATCHed {} and its optimistic update
was a no-op: email notifications could not be muted from Settings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
isEmailMuted treated every preference lookup error, and malformed stored
preferences, as "not muted". A transient DB error could therefore send an
email to a user who had explicitly turned email notifications off. Only a
confirmed missing row (pgx.ErrNoRows) now means the default (enabled); any
other failure skips the send.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…layout

Upstream moved the Notifications tab onto SettingsTab / SettingsSection /
SettingsCard / SettingsRow. Render the email notification row the same way
so it matches the browser notification row next to it, and give the switch
an accessible name.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream added a French locale; the email notification toggle strings
were missing, which fails the locale parity test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The notifier built issue links from FRONTEND_ORIGIN only, so deployments
that set MULTICA_APP_URL as their public frontend got no link (when
FRONTEND_ORIGIN is unset) or a link to the wrong origin. Use
appURLFromEnv(), which the other outbound integrations already use and
which gives MULTICA_APP_URL precedence.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Register launched every delivery concurrently, so a comment or mention
that notifies several subscribers could exceed the provider's request rate
(Resend defaults to 2 requests/second). The SDK returns a rate-limit error
without retrying and the notifier dropped those emails.

Deliveries now share a small concurrency limit (default 2), and a send
that fails with resend.ErrRateLimit is retried up to MaxAttempts (default
4), waiting for the provider's Retry-After (capped at 30s) or an
exponential fallback. Other errors are still not retried, since resending
after an SMTP failure can deliver a duplicate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two problems with the previous fan-out:

- Every inbox event spawned a goroutine that waited for a delivery slot,
  so a slow or unavailable provider let pending deliveries (and their
  payloads) grow without bound.
- Limiting concurrency does not limit requests per second. On Resend,
  notification bursts shared the API key's rate limit with login codes and
  invitations, which are not retried, so inbox activity could make
  send-code fail.

Deliveries now go through a bounded queue (QueueSize, default 1000)
drained by MaxConcurrent workers; the bus callback never blocks and drops
(and logs) when the queue is full. All notification send attempts,
including rate-limit retries, pass through a pacer (SendInterval, default
1s) that keeps notifications at half of Resend's default 2 requests/second,
leaving headroom for authentication email.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The notifier's timeout only covered the database lookups; the send, the
pacing wait and rate-limit backoff ignored it, so a slow or rate-limited
provider could hold both workers far longer (Resend's HTTP timeout is a
minute per attempt) and back up the queue.

SendNotification now takes a context and the Resend path uses
SendWithContext; pacing and backoff waits stop when the context is done.
Timeout now bounds a whole delivery (default 2m). The SMTP path still
relies on its own dial and I/O timeouts.

Also make the notification send interval configurable per replica with
EMAIL_NOTIFICATION_SEND_INTERVAL (default 1s) and document it: several
replicas sharing one Resend key should scale it so notifications leave
headroom for login codes and invitations.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
docker-compose.selfhost.yml lists the backend environment explicitly, so
the new setting in .env never reached the container and the backend kept
the 1s default.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Inbox items can still target a user who has left or been removed from the
workspace (for example a reaction on one of their old comments). The email
channel looked up the recipient in the global user table and sent the issue
title and workspace link outside the app. Check current membership before
delivery and fail closed when the lookup errors.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@r404r

r404r commented Sep 25, 2026

Copy link
Copy Markdown
Author

Updated onto current main via a merge commit (no force-push). Per @Bohan-J's note in #6486, the conflicts with the mentions/comments split are resolved on this side, and the email toggle now goes through the atomic preference patch (MUL-4798) and the new settings layout.

Delivery hardening from review: only current workspace members are emailed; preference lookups fail closed; links use MULTICA_APP_URL; a bounded queue with sends paced below Resend's default limit (EMAIL_NOTIFICATION_SEND_INTERVAL), with Retry-After-aware retries, so notification bursts can't starve login-code emails.

Known limitation: pacing is per replica. Related demand for multi-channel inbox push: #8152. Happy to move this behind a shared delivery-channel abstraction if you'd prefer.

r404r and others added 3 commits October 28, 2026 12:52
- config/index.ts: take upstream (daemonSetupTarget / daemonConfigStatus);
  add only emailConfigured to setAuthConfig.
- locales/zh-Hant/settings.json: upstream added a zh-Hant bundle; add the
  email notification strings so the locale parity test passes.
Upstream added nine inbox types since the email channel was written
(children_done, review_requested, reaction_added, the quick-create and
autopilot types, issue_subscribed); their emails fell back to the
generic "Update" subject. Name them with the web inbox's English labels,
and add a test that reads the inbox label file so a future sync that adds
a type fails here instead of degrading the email.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 51d602a)
Settings search (multica-ai#8874) indexes rows by anchor. Give the email toggle an
"email" anchor and add it to the search index next to the browser
notification row.
@r404r

r404r commented Oct 29, 2026

Copy link
Copy Markdown
Author

Updated onto current main again via a merge commit (no force-push).

  • Resolved the config/index.ts conflict with the new daemon config fields; this PR only adds emailConfigured.
  • Email subjects now have an action verb for every inbox notification type, with a test that fails if a type in inbox.json is missing one.
  • The email toggle has an email anchor and is listed in the settings search index.
  • Added zh-Hant strings for the email keys so locale parity passes.

Verified locally: DB-backed Go tests (handler, email, service, cmd/server), pnpm typecheck, and vitest (core and views).

packages/views/locales/es/settings.json: upstream added a Spanish
bundle; add the four email-notification keys (insertions only) so
locale parity passes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZMgLuBEGf4JN2JLaSRHwG

This branch has not been deployed

No deployments
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