Skip to content

fix(messaging): send transactional workflow email unless the address hard-bounced - #124638

Merged
trunk-io[bot] merged 1 commit into
posthog/messaging-suppression-hard-bounced-atfrom
posthog/transactional-email-hard-bounce-only
Oct 29, 2026
Merged

trunk-io[bot] merged 1 commit into
posthog/messaging-suppression-hard-bounced-atfrom
posthog/transactional-email-hard-bounce-only

Conversation

@meikelmosby

@meikelmosby meikelmosby commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Workflow users who send transactional email (invoices, password resets) lose those emails when the recipient is on the suppression list for any reason but a spam complaint.
  • An address enters the list after 5 soft bounces, for example a full inbox, or when a user adds it by hand. The address still works in both cases.
  • The run log shows a skip, but the run still succeeds, so the missed email is easy to overlook.
  • Raised in a Slack thread (link below).

Changes

  • Transactional email steps now send to addresses that are suppressed for soft bounces, a manual entry or a spam complaint.
  • A hard bounce still blocks transactional email. The address can't receive mail, and repeat hard bounces hurt the shared sender reputation.
  • Marketing email is unchanged: every suppression still blocks it.
  • The SES hard-bounce handler sets hard_bounced_at on every row, manual entries included.
  • Rows that hard-bounced before this column existed still block, by their hard-bounce reason text. No backfill: the read path covers them.
  • A later complaint or soft-bounce run rewrites that reason text, so the write paths move the mark into hard_bounced_at first.
  • The reason check also covers the two Customer.io import reasons, so imported bounced and invalid addresses keep blocking. feat(messaging): set hard_bounced_at from the suppression writers #125374 sets the column on new imports.
  • A soft-bounce row carries no reason until it crosses the threshold, so the reason check wraps in COALESCE and the lookup stays a real boolean.
  • The complaint path no longer raises the bounce counter. That overload existed only to decide transactional blocking, which hard_bounced_at now does.

Note

An old complaint row that later hard-bounced has no marker. It lets one transactional email through, and that bounce then sets the marker.

How did you test this code?

  • Ran nodejs/src/cdp/services/messaging/ locally: 628 passed.
  • Not checked: a real SES bounce end to end.

Test rationale:

  • The suppression service table now covers soft bounces, a complaint, a manual entry and a legacy hard-bounce row. It catches a regression that blocks transactional email for a non-hard-bounce reason, or stops blocking a hard bounce.
  • The recipient-preferences cases changed from "transactional is always skipped" to "skipped only after a hard bounce". They guard the workflow step path.
  • A new case in email-tracking.service.test.ts posts an SES envelope to the webhook route and then runs the pre-send check. It catches a hard bounce that stops marking the row, and a soft bounce or complaint that starts marking it.
  • Two cases cover a row that hard-bounced before the column existed and then takes a complaint or a run of soft bounces. They catch a write path that destroys the reason text without moving the mark.

Release status

  • No feature flag controls this change

Docs update

None in this repo. Public docs that describe the suppression list and transactional email may need a line saying that only hard bounces block transactional email. Not checked.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: PostHog Slack app (Claude Code), Claude Opus 5.5


Created with PostHog from a Slack thread

🤖 Generated with Claude Code

@meikelmosby meikelmosby self-assigned this Oct 28, 2026
@meikelmosby
meikelmosby added this pull request to stack #124639 October 28, 2026 09:08
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane, mixes lanes

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

This PR changes Python or frontend code (products/messaging/backend/migrations/0011_messagesuppression_hard_bounced_at.py, products/messaging/backend/migrations/max_migration.txt, products/messaging/backend/models/message_suppression.py) and Node or Rust code (nodejs/src/cdp/services/messaging/email-suppression.service.test.ts, nodejs/src/cdp/services/messaging/email-suppression.service.ts, nodejs/src/cdp/services/messaging/email-tracking.service.test.ts and 2 more) together. In the merge queue, every Node or Rust PR behind it in the same lane then runs the Django and frontend suites too, so the cross-lane check fails. Split it, or add the cross-lane-change label if the halves must land together.

⚠️ Duplication (Python) — 1 new duplicated block (worst 116 tokens)

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/warehouse_sources/backend/temporal/data_imports/sources/incident_io/incident_io.py:17 products/warehouse_sources/backend/temporal/data_imports/sources/sendgrid/sendgrid.py:21 12 116
⚠️ Duplication (TypeScript) — 1 new duplicated block (worst 71 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/web_analytics/frontend/pageReports/pageReportDetailLogic.ts:242 products/web_analytics/frontend/pageReports/pageReportDetailLogic.ts:256 12 71
🚨 Comment density — 15% of added code lines are comments (23 of 154)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
nodejs/src/cdp/services/messaging/email-suppression.service.ts 12 52
nodejs/src/cdp/services/messaging/email-tracking.service.test.ts 5 29
nodejs/src/cdp/services/messaging/recipient-preferences.service.ts 3 3
nodejs/src/cdp/services/messaging/recipient-preferences.service.test.ts 2 13
nodejs/src/cdp/services/messaging/email-suppression.service.test.ts 1 57

This check does not block merging. It updates on every push and clears when the share drops.

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 1 Safe | 0 Needs Review | 0 Blocked

✅ Safe

Brief or no lock, backwards compatible

messaging.0011_messagesuppression_hard_bounced_at
  └─ #1 ✅ AddField
     Adding nullable field requires brief lock
     model: messagesuppression, field: hard_bounced_at

📚 How to Deploy These Changes Safely

AddField:

This operation acquires a brief lock but doesn't rewrite the table.

Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up.

Last updated: 2026-10-29 10:26 UTC (41fed74)

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 5d5a991: 2 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

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.

@trunk-io

trunk-io Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@meikelmosby
meikelmosby marked this pull request as ready for review October 29, 2026 09:12
@parameterai

parameterai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Risk: No findings

This increment hardens the hard-bounce transactional-email rule: Customer.io bounce/invalid imports now set hard_bounced_at at write time, and rows written before the column existed are recognized by their reason text (in the read path and via migration in both upserts), closing the previously reported gap where imported dead addresses let transactional email through. It also splits the Customer.io import into fetch/save helpers and dedupes opt-out emails. The main residual risk is product-level, not a security bug: manual suppression entries still don't block transactional email, and skip_frequency_cap lets workflow editors opt a step out of the admin-set marketing cap — both within the same privilege the transactional category already granted.

Sentinel reviewed 41fed74 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 29, 2026 09:13
…hard-bounced

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@meikelmosby
meikelmosby force-pushed the posthog/transactional-email-hard-bounce-only branch from 41fed74 to 2987914 Compare October 29, 2026 10:50
@mayteio mayteio added the cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once label Oct 29, 2026 — with PostHog
@mayteio
mayteio marked this pull request as draft October 29, 2026 12:23
@mayteio
mayteio marked this pull request as ready for review October 29, 2026 12:23
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 29, 2026 12:25
@trunk-io

trunk-io Bot commented Oct 29, 2026

Copy link
Copy Markdown

This pull request is queued for merge as part of 125322, which will merge 124637, 124638, 125374, 125322.

@trunk-io
trunk-io Bot merged commit 312f489 into master Oct 29, 2026
325 of 326 checks passed
@trunk-io
trunk-io Bot deleted the posthog/transactional-email-hard-bounce-only branch October 29, 2026 16:26
@trunk-io

trunk-io Bot commented Oct 29, 2026

Copy link
Copy Markdown

This pull request was merged into master as part of stacked PR 125322.

inkeep Bot added a commit to PostHog/posthog.com that referenced this pull request Oct 29, 2026
…mail

Reflects PostHog/posthog#124638 – transactional email (password resets,
invoices, login codes) now sends through soft bounces, spam complaints,
and manual suppression entries. Only a hard bounce blocks it. Marketing
email is unchanged: every suppression still blocks it.
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-29 17:20 UTC Run
prod-us ✅ Deployed 2026-10-29 17:42 UTC Run
prod-eu ✅ Deployed 2026-10-29 17:47 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants