Skip to content

feat(workflows): record recipient, domain, opens and clicks per email - #127745

Merged
trunk-io[bot] merged 4 commits into
masterfrom
dmarchuk/email-status-events-writes
Nov 5, 2026
Merged

trunk-io[bot] merged 4 commits into
masterfrom
dmarchuk/email-status-events-writes

Conversation

@dmarchuk

@dmarchuk dmarchuk commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • The email log in the next PRs can't search by recipient, show opens and clicks, or group by sending domain, because the status rows don't carry them.

Changes

  • The send API's queued row names the recipient and the email's workflow.
  • The sent row names the domain the email went out from.
  • The SES webhook writes an opened or clicked row for each open and click of a transactional email, behind the same signed-code check.
  • Nothing a person sees changes yet.

Warning

Merge after the #127744 migration finishes. Without its filter, an open would replace the delivery status in email_status.

How did you test this code?

  • After merging master with the attachments change, sent real SES simulator emails: a template send, code sends, attachments, and cc and bcc. Each wrote the right rows. Forced missing attachments: the send failed with attachments_missing and the sending domain. All 310 email service tests pass.
  • Sent through real Amazon SES in sandbox mode, using its simulator addresses and one real inbox. SES posted its updates to the webhook through a public tunnel, and each landed in email_status_events (rows below).
  • A spam report that arrives before "delivered" still wins as the email's status.
  • Stopping the local mail catcher mid-send recorded failed / provider_error.
ClickHouse rows
select status, status_at, reason, sending_domain from posthog.email_status_events
where message_id = '<id>' order by status_at
Recipient Rows
success@simulator.amazonses.com queued 00:17:49, sent 00:17:51 (sending domain set), delivered 00:17:52
bounce@simulator.amazonses.com queued 00:18:02, sent 00:18:02, bounced 00:18:03 hard
complaint@simulator.amazonses.com queued 00:23:08, sent 00:23:10, complained 00:23:12, delivered 00:23:13
A real inbox queued 00:23:52, sent 00:23:55, delivered 00:23:56, opened 00:23:58, opened 00:24:02, opened 02:48:09, clicked 08:02:38

The two opens within 6 seconds of delivery are likely a mail scanner.

CI report notes
  • Comment density: the three added comment lines say why opens and clicks never replace a delivery status, and which SES records change an email.

Test rationale: ses.test.ts gets open, click and untagged-email cases. The status tests in email.service.test.ts and cdp-api.serial.test.ts now assert the new fields.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

Skills: /writing-tests, /writing-pr-descriptions, /writing-plainly

More context for AI agents
  • Session: Claude Code session on a devbox, tested on the local dev stack
  • Decisions: Opens and clicks get their own statuses ranked above complained, and the old table filters them out.
  • Read closely: resolveEmailStatusUpdate in nodejs/src/cdp/services/messaging/helpers/ses.ts

🤖 Generated with Claude Code

https://claude.ai/code/session_01BMRf2uGenMximWGgxyo3gr

@dmarchuk
dmarchuk added this pull request to stack #127749 November 2, 2026 19:35
@dmarchuk dmarchuk self-assigned this Nov 2, 2026
@dmarchuk dmarchuk changed the title feat(workflows): record the recipient, sending domain, opens and clicks of transactional emails feat(workflows): record recipient, domain, opens and clicks per email Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

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.

✅ Duplication (TypeScript) — clean

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.

⚠️ Comment density — 4% of added code lines are comments (3 of 71)

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/helpers/ses.ts 2 6
nodejs/src/cdp/services/messaging/email-status.service.ts 1 15

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

@dmarchuk dmarchuk added the cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once label Nov 2, 2026
@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch from 9b3a79b to ca830d2 Compare November 2, 2026 20:09
@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch 2 times, most recently from 3e99940 to 772af16 Compare November 3, 2026 12:47
@dmarchuk
dmarchuk marked this pull request as ready for review November 3, 2026 14:27
@parameterai

parameterai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

This PR adds recipient, workflow and sending-domain fields to transactional email status rows, and maps SES Open/Click records to new opened/clicked statuses that rank above complaint but never replace a delivery status. The new SES event types sit behind the same SNS signature and topic-ARN verification as the existing ones, and the recorded values (to is schema-validated, the domain comes from the server-side integration sender) carry no attacker-controlled injection path. No new vulnerability in the lines this pull request changes.

Sentinel reviewed 868ce99 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team November 3, 2026 14:28
@dmarchuk dmarchuk added the stamphog Request AI approval (no full review) label Nov 4, 2026
stamphog[bot]

This comment was marked as outdated.

@dmarchuk
dmarchuk requested a review from a team as a code owner November 4, 2026 01:07
@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch from 772af16 to f7c6dab Compare November 4, 2026 01:08
@stamphog
stamphog Bot dismissed their stale review November 4, 2026 01:08

A new stamphog review started for this PR — the fresh verdict replaces this approval.

stamphog[bot]

This comment was marked as outdated.

@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch from f7c6dab to 5f8c178 Compare November 4, 2026 12:01
@stamphog
stamphog Bot dismissed their stale review November 4, 2026 12:01

A new stamphog review started for this PR — the fresh verdict replaces this approval.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not approved yet — waiting on the conditions below.

Re-add the stamphog label to request another review once you have addressed this.

stamphog declined to review this pull request because the prerequisites gate failed: the branch has merge conflicts. The deny-list, size, and tier gates all passed, so the conflicts are the only thing blocking it. The author can resolve the conflicts against the base branch and push again, which re-runs the gates. If the PR still can't be auto-reviewed after that, they can ask a human reviewer to take it.

Gate mechanics and policy version
Gate Result
prerequisites ✗ merge conflicts present
deny-list ✓ no deny categories matched
size ✓ 36L, 4F substantive, 78L/8F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (78L, 8F, single-area, feat)
stamphog 2.7.0 .stamphog/policy.yml @ unknown · reviewed head 5f8c178

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Nov 4, 2026
@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch from 5f8c178 to 2a2ceeb Compare November 4, 2026 12:10
stamphog[bot]

This comment was marked as outdated.

@dmarchuk
dmarchuk removed this pull request from stack #127749 November 4, 2026 14:42
@Silthus
Silthus added this pull request to stack #129264 November 4, 2026 14:42
@dmarchuk
dmarchuk removed this pull request from stack #129264 November 4, 2026 14:44
@dmarchuk
dmarchuk added this pull request to stack #129265 November 4, 2026 14:44
@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch from 2a2ceeb to 93c0a3f Compare November 4, 2026 16:33
@stamphog
stamphog Bot dismissed their stale review November 4, 2026 16:33

A new stamphog review started for this PR — the fresh verdict replaces this approval.

stamphog[bot]

This comment was marked as outdated.

Base automatically changed from dmarchuk/email-status-events to master November 4, 2026 19:36
@trunk-io

trunk-io Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

😎 Stack merged successfully - details.

@stamphog
stamphog Bot dismissed their stale review November 4, 2026 19:36

The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.

@dmarchuk
dmarchuk force-pushed the dmarchuk/email-status-events-writes branch from 93c0a3f to 2815642 Compare November 4, 2026 19:39
stamphog[bot]

This comment was marked as outdated.

@stamphog
stamphog Bot dismissed their stale review November 5, 2026 10:24

A new stamphog review started for this PR — the fresh verdict replaces this approval.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approved.

The change enters email telemetry write paths and data models, with owning-team authorship providing the required assurance. No substantive review concerns remain; the checkout contains the delivery-status filter, and the bot discussion is not a current-head review.

  • Author wrote 0% of the modified lines and has 43 merged PRs in these paths (familiarity MODERATE).
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 36L, 4F substantive, 78L/8F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (78L, 8F, single-area, feat)
stamphog 2.7.0 .stamphog/policy.yml @ 868ce99 · reviewed head 868ce99

@dmarchuk
dmarchuk removed the request for review from a team November 5, 2026 10:27
@dmarchuk

dmarchuk commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@deployment-status-posthog

deployment-status-posthog Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-05 11:15 UTC Run
prod-us ✅ Deployed 2026-11-05 11:39 UTC Run
prod-eu ✅ Deployed 2026-11-05 11:41 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 stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant