Repository navigation
feat(core): cc and replyTo on plugin email messages - #3333
Conversation
🦋 Changeset detectedLatest commit: 42b49a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
This is a small, additive extension of the existing email hook/capability model and looks like the right change: plugins can now set cc and replyTo on an email, the fields flow through EmailMessage (and therefore email:beforeSend, email:deliver, and email:afterSend), and both built-in providers pass them on. The changeset package list and bump types are correct, docs are updated, and the public type surface is consistent across core, cloudflare, and workerd.
I reviewed the diff statically, traced the call sites for the email pipeline and both providers, and checked the workerd bridge validation, Cloudflare binding shape, and updated docs. Two things stand out:
- The workerd bridge's
isEmailMessagetype guard was not updated to match the now-expandedEmailMessagetype, so malformedcc/replyTovalues can cross the RPC boundary beforeEmailPipelinerejects them. - The Cloudflare provider uses
||to pick the message-levelreplyTo, which means an empty-stringreplyTofalls back to the provider's configured value rather than overriding it as documented.
Neither is a security or data-loss issue, but the first is a real validation gap at a boundary and should be fixed before merge.
Findings
-
[needs fixing]
packages/workerd/src/sandbox/bridge-handler.ts:999-1005The local
EmailMessagetype now includesccandreplyTo, butisEmailMessageonly checksto,subject,text, andhtml. A malformed value such ascc: ["a", 1]orreplyTo: 42will pass this guard, be typed as a validEmailMessage, and reachemailSendbeforeEmailPipelineeventually throws. The bridge already hasisStringArray, so the guard should use it to keep the runtime shape and the narrowed type in sync.function isEmailMessage(value: unknown): value is EmailMessage { if (!isRecord(value)) return false; if (typeof value.to !== "string") return false; if (typeof value.subject !== "string") return false; if (typeof value.text !== "string") return false; if (value.html !== undefined && typeof value.html !== "string") return false; if (value.cc !== undefined && !isStringArray(value.cc)) return false; if (value.replyTo !== undefined && typeof value.replyTo !== "string") return false; return true; } -
[suggestion]
packages/cloudflare/src/plugins/cloudflare-email.ts:132Using
||to resolve the message-levelreplyTomeans an empty-stringreplyTofalls back toconfig.replyTo, which contradicts the doc update that says a plugin-setreplyTooverrides the provider option. Prefer nullish coalescing so any value set on the message (including an empty string, which is then omitted below) takes precedence.const replyTo = message.replyTo ?? config.replyTo;
There was a problem hiding this comment.
This is a sensible, backwards-compatible extension of the plugin email API. Adding optional cc and replyTo to EmailMessage, validating them at the EmailPipeline boundary, threading them through email:beforeSend / email:deliver / email:afterSend, and updating both the Cloudflare and workerd sandbox bridges fits EmDash's architecture. The docs, skill examples, changeset, and provider-specific behavior are all aligned.
I checked the diff, the core email pipeline, the Cloudflare email provider, the workerd and Cloudflare sandbox bridges, the public docs, the changeset, and the new tests. Everything looks correct except one real inconsistency in the workerd sandbox bridge: the local EmailMessage type was expanded, but its runtime narrowing predicate (isEmailMessage) was not, so malformed cc/replyTo values can cross the RPC boundary before the pipeline rejects them.
Other minor things are fine: no new queries on logged-out routes, no SQL or auth changes, no UI/i18n impact, and the tests cover the key behaviors. The PR is close to clean once the bridge guard is tightened.
Findings
-
[needs fixing]
packages/workerd/src/sandbox/bridge-handler.ts:999-1005The
EmailMessagetype now carries optionalccandreplyTo, butisEmailMessageonly checksto,subject,text, andhtml. It will returntruefor a value like{ to: "a@b.com", subject: "s", text: "t", cc: "not-an-array" }, falsely claiming it is anEmailMessage. That lets invalid sandbox RPC payloads leak past the bridge's input contract and get rejected later byEmailPipeline.sendwith a less specific error.Validate the new optional fields in the predicate so it matches the type:
function isEmailMessage(value: unknown): value is EmailMessage { if (!isRecord(value)) return false; if (typeof value.to !== "string") return false; if (typeof value.subject !== "string") return false; if (typeof value.text !== "string") return false; if (value.html !== undefined && typeof value.html !== "string") return false; if (value.cc !== undefined && !isStringArray(value.cc)) return false; if (value.replyTo !== undefined && typeof value.replyTo !== "string") return false; return true; }isStringArrayis already defined at line 969. Consider also adding a bridge-level test inpackages/workerd/test/bridge-handler.test.tsthat assertsemail/sendrejects malformedccorreplyToat the bridge boundary, mirroring the existingMissing capabilitytest case.
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Scope checkThis PR touches 24 files. PRs with a broad scope are harder to review. Please confirm the scope hasn't drifted beyond the intended change. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
There was a problem hiding this comment.
This is a sensible, backwards-compatible extension of the plugin email API. The approach fits EmDash: new optional fields on EmailMessage, validation at the pipeline boundary, threading through the email hooks, and delivery/bridge updates for both Cloudflare and workerd.
The earlier workerd bridge-predicate issue is now fixed: isEmailMessage validates cc and replyTo, and packages/workerd/test/bridge-handler.test.ts adds a bridge-level test that rejects malformed values. I also re-checked the core email pipeline, Cloudflare provider, dev console provider, both sandbox bridges, the docs, and the tests — they all look correct.
The one remaining problem is a missing changeset entry for the published workerd package. packages/workerd/src/sandbox/bridge-handler.ts is part of @emdash-cms/sandbox-workerd, which has its own version and CHANGELOG and is not in the changeset fixed package list that rides along with emdash/@emdash-cms/cloudflare. AGENTS.md requires a changeset for any change to a published package's behavior or API, so that package should be listed too.
No logged-out route changes, no SQL/auth/i18n impact, and no new comments or drive-by refactors to flag.
| "emdash": minor | ||
| "@emdash-cms/cloudflare": minor |
There was a problem hiding this comment.
[needs fixing] The PR changes packages/workerd/src/sandbox/bridge-handler.ts to accept, validate, and forward cc/replyTo across the workerd sandbox RPC boundary. That is a behavior change to the published @emdash-cms/sandbox-workerd package, which has its own version and CHANGELOG and is not in the changeset fixed package list. AGENTS.md requires a changeset for any change to a published package's behavior or API.
| "emdash": minor | |
| "@emdash-cms/cloudflare": minor | |
| --- | |
| "emdash": minor | |
| "@emdash-cms/cloudflare": minor | |
| "@emdash-cms/sandbox-workerd": minor | |
| --- |
There was a problem hiding this comment.
This re-review confirms the one remaining issue from the prior review is fixed: the changeset at .changeset/plugin-email-cc-reply-to.md now lists @emdash-cms/sandbox-workerd alongside emdash and @emdash-cms/cloudflare, capturing the behavior change to the workerd sandbox bridge.
The approach remains sound: adding optional cc and replyTo to EmailMessage, validating them at the pipeline entry, threading them through the email hooks, and forwarding them through both the Cloudflare and workerd sandbox bridges plus the Cloudflare email provider and dev console provider. It is backwards-compatible because both fields are optional.
I re-checked:
- Email pipeline validation in
packages/core/src/plugins/email.ts— rejects non-arrayccand non-stringreplyTo. - Type definitions in
packages/core/src/plugins/types.ts,packages/core/src/plugins/sandbox/types.ts,packages/cloudflare/src/sandbox/types.ts, andpackages/workerd/src/sandbox/bridge-handler.ts— all consistent. - Cloudflare email provider delivery and
replyToprecedence logic — message-levelreplyTooverrides the provider config. - workerd bridge predicate validation —
isEmailMessagecorrectly validatesccandreplyTo. - Dev console output, hooks docs, deployment docs, skill reference docs, and template copies — all updated and consistent.
- Tests added/updated in core, cloudflare, and workerd packages — they exercise the new fields and malformed-value rejections.
No remaining logic bugs, regressions, AGENTS.md violations, or documentation inaccuracies were found.
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
…er (#4144) * fix(plugin-forms): set Reply-To on notification emails to the submitter Form notifications are sent from the site's sender address with no Reply-To, so replying to one goes to that address (often a noreply@) instead of the person who filled in the form. A help desk that builds a conversation from the email (Chatwoot, for example) also files it under the sender address, so every submission lands on one contact. #3333 added replyTo to plugin email messages for exactly this case (#2611 describes a contact-form notification); the forms plugin didn't use it yet. Notifications now set replyTo to the address from the form's email field, the one the autoresponder already sends to. The value has passed the email field's validation (no whitespace), so it can't inject headers. Without an email field, or with it left empty, notifications are unchanged. The submitter's address is now looked up once for both the notifications and the autoresponder; the autoresponder's behavior is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugin-forms): only set Reply-To for a plain email address The email field's own check accepts values like "IT-Support"<someone@example.com> or jane@example.com,. As a Reply-To, they can show the team a made-up name or get the notification rejected. Check the address with z.email() first, as the notification addresses are; the autoresponder keeps the field's check. The changeset now says the daily digest is unchanged, and that replies go to the visitor instead of the Cloudflare email provider's configured replyTo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What does this PR do?
Closes #2611
Adds optional
ccandreplyTofields to the pluginEmailMessage, so a plugin can send one message to several visible recipients and direct replies to someone other than the sender. The common case is a form or notification plugin that mails a team and wants a reply to go straight to the visitor who submitted the form. Today a plugin has to send one message per recipient and cannot set a reply address at all; only the Cloudflare provider's site-widereplyTooption exists.ctx.email.send()validates the new fields (array of strings / string) alongside the existingto/subject/textchecks.email:beforeSend,email:deliver, andemail:afterSend, and through the Cloudflare and workerd sandbox bridges.ccto thesend_emailbinding. A message'sreplyTooverrides the provider's configuredreplyTo.Cc:andReply-To:lines.hooks.mdxdocuments the fields on the event shape and tells provider authors to deliver them. The plugin skill's Resend example now forwards them.Both fields are optional. Existing plugins and providers keep working unchanged; a third-party provider that ignores
ccsimply doesn't deliver to those recipients until it is updated.Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.No admin UI strings or UI changes. No Discussion: small additive extension of the existing email API, opened directly as a maintainer-team member; happy to move it to a Discussion if preferred.
AI-generated code disclosure
Screenshots / test output
Not applicable.
Targeted tests:
packages/coreemail pipeline (40 passed; new case checks delivery ofcc/replyToand rejection of malformed values),packages/cloudflareemail provider (12 passed; new case checksccpassthrough andreplyToprecedence),packages/workerdbridge handler (76 passed; new case checks the sandbox bridge forwards both fields).