Add transactional email catalog and magic-link sign-in - #2642
Add transactional email catalog and magic-link sign-in#2642timmilazzo wants to merge 11 commits into
Conversation
builder-io-integration
Bot
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@builderio-bot - review the latest PR feedback, fix what you agree with, comment on each item as you make changes, and try to make this merge-able. |
…381e5005a3dc41bcb927
|
There was a problem with your request, please try again later. Error id: |
|
@builderio-bot - what human input do you truly need to get this to a merge-able state? Looks like you've made changes and comments but not given us a fresh status update. |
|
Fresh status: I do not need any further product or implementation input to finish this.
Human actions remaining are mechanical only: push the branch changes, let CI rerun, and provide the required reviewer approval. No additional design decision is blocking mergeability. |
|
No further product or implementation input is needed. I fixed the remaining scaffold CI configuration and posted a fresh status on PR #2642. Fresh verification passes: 42/42 guards, formatting, Remaining human actions:
|
There was a problem hiding this comment.
Builder reviewed your changes and found 5 potential issues 🔴
Review Details
Incremental Code Review Summary
The latest commits address all five findings from the prior review: email logs now filter by app, SendGrid activity is no longer account-wide, Google-only deployments omit the magic-link plugin, verification tokens are hashed, and metrics follow the active transport. I resolved those previous review threads.
The remaining changes still carry high risk because they affect authentication links and privileged email-provider reporting.
New Findings
- 🔴 High: Magic-link URLs are sent without inserting the mounted app path, so links generated by path-mounted apps such as Calendar or Mail can land at the wrong auth handler and fail to establish a session.
- 🔴 High: Dispatch activity requests accept arbitrary SendGrid category IDs without catalog or app authorization, exposing recipient addresses, subjects, and delivery activity.
- 🟡 Medium: Engagement requests likewise accept arbitrary category IDs and expose aggregate delivery/open/click data outside the authorized catalog.
- 🟡 Medium: The Dispatch shared-core panel calculates core send counts from the Dispatch database only, so it misses core emails sent by other apps and can duplicate core definitions in per-app cards.
- 🟡 Medium: Daily SendGrid
unique_opensvalues are summed into a window-level unique-open rate, which can double-count recipients who open on multiple days and produce inflated rates.
The fixes from the previous review are directionally sound, but these new issues should be addressed before merge. Browser testing will be attempted again for the updated UI/auth flows.
🧪 Browser testing: Will run after this review (PR touches UI code)
| magicLink({ | ||
| storeToken: "hashed", | ||
| sendMagicLink: async ({ email, url }) => { | ||
| const { subject, html, text } = renderMagicLinkEmail({ | ||
| email, | ||
| magicLinkUrl: url, |
There was a problem hiding this comment.
🔴 Prefix generated magic-link URLs with the mounted app path
The magic-link callback sends Better Auth's generated url unchanged. In workspace deployments the app is mounted under APP_BASE_PATH (for example /calendar), while this instance uses the unprefixed /_agent-native/auth/ba path. Unlike the existing reset and verification senders, this flow does not insert the mounted prefix, so recipients can land at the wrong auth handler and the token cannot establish a session. Apply the same normalized app-base-path rewrite used by the adjacent email flows and add a mounted-app regression test.
Additional Info
Reported by 2/4 review agents; confirmed against adjacent reset/verification URL handling.
| limit: z.coerce.number().int().min(1).max(1000).default(50), | ||
| }), | ||
| http: { method: "GET" }, | ||
| run: async ({ templateId, limit }) => |
There was a problem hiding this comment.
🔴 Authorize activity categories before querying SendGrid
The action now prevents an account-wide feed, but it still accepts any caller-provided templateId and sends it directly to SendGrid. There is no catalog lookup or app/workspace authorization, so an authenticated Dispatch caller can query known or arbitrary categories and receive recipient addresses, subjects, and delivery activity for messages outside the authorized transactional-email catalog. Resolve the ID against authorized catalog entries and reject unknown or inaccessible categories before calling the provider.
Additional Info
Reported by 3/4 review agents; this is distinct from the previously fixed account-wide-feed issue.
| windowDays: z.coerce.number().int().min(1).max(365).default(30), | ||
| }), | ||
| http: { method: "POST" }, | ||
| run: async ({ templateIds, windowDays }) => |
There was a problem hiding this comment.
🟡 Authorize categories before returning engagement metrics
This endpoint accepts arbitrary templateIds and forwards them directly to SendGrid category statistics without checking that each ID is registered and accessible to the caller. An authenticated Dispatch user can therefore retrieve delivery/open/click aggregates for categories belonging to other products or workloads. Validate IDs against the authorized catalog and reject or omit unknown/inaccessible categories before querying the provider.
Additional Info
Reported by 3/4 review agents; distinct from the previously fixed transport-selection issue.
| > | null = null; | ||
| let statsError: string | null = null; | ||
| try { | ||
| const stats = await getEmailSendStats(since, app); |
There was a problem hiding this comment.
🟡 Aggregate shared core-email counts across sending apps
The action computes stats with getEmailSendStats(since, app), where app is the currently running app, but returns definitions whose definition.app is core. Dispatch uses this local result for its shared system-email section, so core emails sent by Calendar, Forms, or other mounted apps are omitted and the same core definitions can also appear in per-app cards. Aggregate core-template stats across the relevant apps or separate core definitions from per-app results so the shared section represents all senders.
Additional Info
Reported by 1/4 review agents; confirmed by the Dispatch shared section filtering core definitions from the local action result.
| for (const day of payload ?? []) { | ||
| for (const entry of day.stats ?? []) { | ||
| const name = entry.name; | ||
| if (!name) continue; | ||
| const metrics = entry.metrics ?? {}; | ||
| const current = totals.get(name) ?? { | ||
| templateId: name, | ||
| delivered: 0, | ||
| uniqueOpens: 0, | ||
| uniqueClicks: 0, | ||
| openRate: null, | ||
| }; | ||
| current.delivered += Number(metrics.delivered ?? 0); |
There was a problem hiding this comment.
🟡 Avoid summing daily unique opens as a window-wide unique count
The request uses aggregated_by=day, then sums each day's unique_opens while presenting the result as a window-level unique-open rate. A recipient who opens on multiple days is counted once per day, while delivered remains unchanged, inflating the rate and potentially producing values over 100%. Use a range-aggregated response if supported, or label/report the daily total rather than treating it as distinct recipients for the whole window.
Additional Info
Reported by 1/4 review agents; confirmed from the daily aggregation request and summation logic.
Visual recap — screenshot failedA recap was published, but the PR-comment screenshot could not be captured or uploaded. Open the interactive recap directly: Open the full interactive recap Diagnostic: light: page.goto: Timeout 45000ms exceeded. Call log: - navigating to "https://plan.agent-native.com/recaps/recap-d5daa86e7a374d04?recapScreenshot=1&recapScreenshotTheme=light", waiting until "domcontentloaded" |
2 participants
Summary
Adds a shared transactional email catalog so every app can declare the emails it sends, and introduces email magic-link sign-in as the default authentication method alongside password login.
Problem
There was no central way to see what transactional emails existed across apps, how they were triggered, who received them, who sent them, or how they performed (sends, open rate, last sent). Additionally, sign-in only supported passwords, with no lighter-weight magic-link option.
Solution
Introduced
defineTransactionalEmailin@agent-native/core/email-catalogso apps register each email with an id, trigger description, recipient/sender logic, and a dummy-data preview renderer.sendEmailnow accepts atemplateIdto tag messages at the provider and record every attempt (success or failure) to a newemail_logtable, decoupling metrics from the provider's short activity retention. Dispatch adds a Transactional email screen built on top of this catalog. Separately, Better Auth'smagicLinkplugin is wired into the shared auth instance and exposed as a default sign-in option below Google, backed by a newcore.magic-link-sign-incatalog entry.Key Changes
sendEmailtemplateIdsupport,email_logtable, Dispatch's new screens, and default magic-link sign-in.docs/plans/2026-08-04-magic-link-sign-in-design.md,docs/plans/2026-08-04-magic-link-sign-in.md).renderMagicLinkEmailand registeredcore.magic-link-sign-inin the core email templates/catalog, with corresponding tests.magicLinkplugin inbetter-auth-instance.ts, wiringsendMagicLinkto render and send the magic-link email via the coresendEmailtransport.renderPlanCommentEmail, addedtemplates/plan/server/lib/emails.tsregisteringplan.plan-commentandplan.access-requestcatalog entries, and atransactional-emails.tsplugin to register them.renderDeckCommentEmail, addedtemplates/slides/server/lib/emails.tsregisteringslides.deck-comment, updatedcomment-notifications.tsto use the renderer and passtemplateId, and added atransactional-emails.tsplugin to register it.To clone this PR locally use the Github CLI with command
gh pr checkout 2642You can tag me at @BuilderIO for anything you want me to fix or change