Repository navigation
Conversation
The posthog-node/nodemailer entry point exports createPostHogTransport({ client, defaultTemplateKey }). Pass it to nodemailer.createTransport() and every sendMail call sends one PostHog email through the client's emails.send, marked nodemailer/<version>. PostHog's options ride in sendMail({ posthog: { ... } }). A message PostHog can't carry fails with code EPOSTHOGUNSUPPORTED before any request, and a failed send says whether a retry with the same idempotency key can succeed. Nodemailer stays a dev dependency.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coherence📄 0 spec file errors · ⛔ 0 broken rules · 🚪 2 unchecked inputs · 🐛 0 bugs that came back What is Coherence?Coherence compares the code with its spec files ( This check read 44 parts of the code and 7 rules. Note These findings do not make this check fail. None of these findings are in files that this pull request changes. You do not need to fix them in this pull request. 🚪 Unchecked inputs (2)
What this means and how to fix itInput from outside this code comes in at these places. The spec does not say what checks that input. 10 more places were already like this when Coherence was added. This list does not show them. To close one of these, find the place by its name in the
To get the line to add, run |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed d1d5353: 6 Should fix, 1 Consider.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
| const hasExtraPart = | ||
| Boolean(mail.watchHtml || mail.amp || mail.icalEvent || mail.raw || mail.alternatives?.length) || | ||
| Boolean(mail.attachments?.some((attachment) => attachment.raw !== undefined)) |
There was a problem hiding this comment.
P2 · Reject nested raw MIME before normalization removes it
When html or an attachment contains both raw and content, Nodemailer gives raw precedence but normalize() drops that marker. unsupportedPart() then accepts the fallback content, and the transport successfully sends a different message. Check raw MIME on the original message before normalization so these sends fail with EPOSTHOGUNSUPPORTED.
| const { content = '', encoding, filename, contentType, cid } = attachment | ||
| return { | ||
| filename: filename || '', | ||
| content: typeof content === 'string' ? Buffer.from(content, (encoding as BufferEncoding) ?? 'utf8') : content, | ||
| ...(contentType && { contentType }), | ||
| ...(cid && { contentId: cid }), |
There was a problem hiding this comment.
P2 · Validate attachment headers before discarding them
An attachment with headers: { 'Content-ID': '<logo>' } is valid Nodemailer input and lets HTML reference cid:logo. normalize() preserves those headers, but attachmentOf() drops them, so the send succeeds without content_id and breaks the delivered image. Inspect attachment headers and either map supported values or refuse the message before sending.
| const isIgnored = (name: string): boolean => IGNORED_HEADERS.has(name) || name.startsWith('content-') | ||
| const hasHeader = Object.keys(normalizedHeaders ?? {}).some((name) => !isIgnored(name.toLowerCase())) |
There was a problem hiding this comment.
P2 · Handle array-form headers when checking ignored names
When sendMail() receives headers: [{ key: 'Message-ID', value: '<x@example.com>' }], Nodemailer accepts the array but exposes its numeric indexes in normalizedHeaders. unsupportedHeader() checks 0 instead of Message-ID and rejects the message, although this header is explicitly ignored. Resolve array entries to their header names before checking them.
| "default": "./dist/entrypoints/otel.js" | ||
| }, | ||
| "./nodemailer": { | ||
| "types": "./dist/entrypoints/nodemailer.d.ts", |
There was a problem hiding this comment.
P2 · Publish separate ESM and CommonJS declaration targets
With moduleResolution: "NodeNext", an ESM consumer loads Nodemailer's ESM declarations, but this shared .d.ts belongs to a CommonJS package and resolves the posthog augmentation against Nodemailer's separate CommonJS declarations. Consequently, sendMail({ posthog: ... }) fails type checking. Publish conditional ESM and CommonJS declaration targets.
| export interface NodemailerMail { | ||
| normalize(callback: (error: Error | null, data?: NormalizedMail) => void): void | ||
| } |
There was a problem hiding this comment.
P2 · Match Nodemailer's transport input contract
With Nodemailer 10.3.0, this narrow normalize callback makes PostHogTransport incompatible with Transport<PostHogSentMessageInfo>. createTransport() instead selects its SMTP overload, so TypeScript accepts info.accepted.map(...) although this transport returns only messageId, causing a runtime exception. Match Nodemailer's input type and narrow after normalization.
| const refusal = | ||
| unsupportedPart(mail) ?? | ||
| unsupportedHeader(mail) ?? | ||
| envelopeMismatch(mail, [...to, ...cc, ...bcc]) ?? | ||
| unsupportedPriority(mail.priority) ?? | ||
| inlineWithoutCid(mail.attachments) |
There was a problem hiding this comment.
P2 · Refuse messages whose REQUIRETLS policy cannot be enforced
A message with requireTLSExtensionEnabled: true passes every refusal check, and the mapper drops that setting before sending. An application migrating from SMTP therefore receives success without preserving its explicit REQUIRETLS delivery constraint, silently weakening its security policy. Reject this option unless PostHog can enforce the same requirement.
| const { content = '', encoding, filename, contentType, cid } = attachment | ||
| return { | ||
| filename: filename || '', | ||
| content: typeof content === 'string' ? Buffer.from(content, (encoding as BufferEncoding) ?? 'utf8') : content, |
There was a problem hiding this comment.
P3 · Preserve attachments that Nodemailer already encoded as Base64
For binary attachments, Nodemailer's normalize() returns Base64 content with encoding: 'base64'. attachmentOf() decodes it into a new Buffer, then EmailSender encodes that Buffer back into Base64. Attachment-heavy sends incur redundant allocations and synchronous CPU work on the event loop; pass Base64 strings through directly.
Node SDK acceptance v2Commit: 84935ad Capture v0✅ 83 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1✅ 148 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
…ransport for createTransport
An attachment's raw source and an attachment's own headers are refused, so PostHog never sends other bytes or loses a Content-ID. Headers passed as a list of { key, value } are read by name. The transport's normalize data is unknown to TypeScript, so createTransport() picks the transport overload and a send resolves to { messageId }. Base64 attachment content from normalize() goes to the SDK as it is.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nodemailer-transport
Problem
A Node app that sends email with Nodemailer can't send that email through PostHog without rewriting every
sendMailcall asposthog.emails.send().Ticket: https://github.com/PostHog/michaelr-context/issues/41 (private).
Stacked on #5574, which is stacked on #5514. Merge them first. This PR stays a draft until the email SDKs release.
Seams for review. The
integrationoption is a seam: a convex stack builds an@posthog/convexsender on it next, and PostHog/posthog#130017 reads the name before the/of a validX-PostHog-Integrationheader as the send'sclient. The posthog.com page for the transport is PostHog/posthog.com#21987. Check for cross-stack breaks: a marker pattern that the server reads differently, the same concept built twice, or a header that one stack sets and another doesn't read.Merge danger
Medium, two-way door until release. It adds a new entry point,
posthog-node/nodemailer, that makes real sends. Nothing that exists today imports it, and Nodemailer is a dev dependency only. After a release, the entry point and its error codes are public API.Changes
createPostHogTransport({ client, defaultTemplateKey? })returns a Nodemailer transport,{ name, version, send(mail, done) }. It reads each message withmail.normalize(). It doesn't import Nodemailer at run time, soposthog-nodeandposthog-node/nodemailerload when Nodemailer isn't installed.emails.send(), withintegration: 'nodemailer/<posthog-node version>'(from feat(node): let an integration name itself on an email send #5574). The SDK still does the checks, the request, the retries, and the idempotency key.sendMail({ posthog: { distinct_id, template_key, idempotency_key, capture_engagement } }). In TypeScript, the entry point addsposthogto Nodemailer's message options. The template key isposthog.template_key, thendefaultTemplateKey, thennodemailer.Errorthat hascode: 'EPOSTHOGUNSUPPORTED'and arefusal:multiple_to,unsupported_header,unsupported_part,unsupported_priority,envelope_mismatch,inline_without_cid, orinvalid_template_key. PostHog never sends part of a message.Errorthat hascode,status,detail,attr,retryable, andretry_after(seconds), with the SDK'sEmailErroras itscause.isRetryableis now exported inside the package for this.done(null, { messageId })with PostHog's message ID.Evidence
vitest runfor posthog-node): green, 48 files, 1568 tests. CI on this PR: see below, because the base isn'tmain.nodemailer.spec.tsfailed before the change because the module didn't exist, and passes after it.nodemailer.createTransport(createPostHogTransport(...))andsendMail(), through the real SDK, withfetchstubbed.X-PostHog-IntegrationandUser-Agentheaders. That is every rendered send, display names, cc, bcc and reply-to, attachments and inline images, theposthogoptions, the default template key, and the ignored headers.watchHtml, alternatives, raw, a high priority,inReplyTo,list, and asenderthat isn'tfrom.Retry-After), 500, 503, and a network error.nodemailer-absent.spec.tsloads the builtposthog-nodeandposthog-node/nodemailerthroughimportandrequire, withnodemailerunresolvable, and sends one message. A mutation that adds a runtimeimport 'nodemailer'makes both tests fail.types/nodemailer-transport.tscheckscreateTransport(createPostHogTransport(...)),sendMail({ posthog }), andinfo.messageId, and rejects an unknown or wrongly typedposthogoption.Spec divergence. The emails spec in sdk-specs doesn't define framework adapters. This entry point uses only
emails.send()and theintegrationoption from #5574.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built by Claude Code (Opus 5.5) test-first. A GPT-6.1 Sol design pass found one simplification: the mapper now throws the refusal error itself, with no separate refusal type.
posthogkey insendMail(), notX-PostHog-*headers, which the transport refusessenderother thanfrom?unsupported_header, because Nodemailer doesn't put it in the envelope and PostHog can't send it{ messageId }onlyposthog?Involvement: 0 grilling rounds, 1 design pass (GPT-6.1 Sol, 1 finding fixed), red/green proof captured.
🤖 Generated with Claude Code