Skip to content

refactor(cdp): move the email send checks into one send policy - #576

Closed
Silthus wants to merge 7 commits into
email-v2/01-cdp-code-sendfrom
email-v2/07-send-policy
Closed

Silthus wants to merge 7 commits into
email-v2/01-cdp-code-sendfrom
email-v2/07-send-policy

Conversation

@Silthus

@Silthus Silthus commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Part of the code-first rebuild of the email send stack; replaces parts of PostHog#121083 (one send policy for the email send checks).

Problem

  • The next PR has cdp-api refuse a send before it answers 202, using the same checks the email worker runs. Today those checks sit inside the worker's send method, so cdp-api can't reuse them.
  • Suppression has two lookup copies: one in the worker's send method and one in the recipient preferences check. The two runtime checks stay; their code becomes one lookup.

Impact

Enabler for Grow Workflows: it lets the send API refuse what it can't deliver, instead of a 202 that never sends. In scope: CDP email sends. Nothing a person sees changes.

Merge Danger

Note

🟡 Merge risk: medium · two-way door: a revert puts the checks back in the send method

  • Every workflow email goes through the moved checks, and this refactor is not behind a flag.
  • The order of the checks, each skip reason, metric and log level stay as they were. A shadow diff over 15 cases at base and head found one change: a suppressed recipient with an off-domain From override no longer logs "Ignoring the custom sender address …" before its skip line.

Changes

Kind Files Lines
Production 4 +230 −167
Tests 2 +144 −4
  • The email send checks move into one send policy in nodejs/src/cdp/services/messaging/email-send-policy.ts: team suspension, workflow pause, sender, recipient and suppression.
  • The email service asks the policy and sends or skips as before.
  • Suppression has one lookup, EmailSuppressionService.findSuppressed. Both the send policy and the recipient preferences check call it; the two runtime checks remain.

Mechanical: the whole PR is a refactor. No contract or payload changes. One log line changes, named under Merge Danger.

How did you test this code?

Test seams: the policy through its public check (email-send-policy.test.ts) and the email service through its send (email.service.test.ts).

Test rationale:

  • email-send-policy.test.ts: a check that moves out of order, or a skip reason that changes.
  • The test file changed: the missing-recipient table gained the suppressed-cc row, and every row of it now stubs isSuppressed to true, so a suppression check that moves ahead of the missing-recipient check fails.

Proofs:

  • Shadow diff: the email service at base 24644bc and at this head over the same 15 invocations: both suspension causes, both pause kinds, each sender failure, a missing to, a suppressed to or cc, and a suppressed to with an off-domain From override. Compared: outcome, run log, server log, metrics, skipped, provider calls. One difference, intended: the dropped custom-sender warning above.
  • Post-deploy watch: email_suppressed, email_suspended and email_paused per day hold their level.
  • Prod shape: a gap until read-only production access exists on 2026-11-02.

CI: the fork has no Depot runners, so those jobs never start, and no secret for the project-board app, so flags-project-board fails; the other GitHub-hosted checks pass. The lane check passes: against master the PR changes only nodejs/.

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 (claude-opus-5-5)

Question Answer Who Where
May the code carry comments? No; names carry the reasons 👤 Human map #461, 2026-10-25

Involvement: one brief from Michael with decisions; review round 1 (GPT-6.1 Sol team review, Opus fresh eyes) fixed; shadow diff captured.

🤖 Generated with Claude Code

Silthus and others added 5 commits October 25, 2026 03:28
…il route

A code send carries the content Django accepted: the rendered email, the
sender and the capture choice. cdp-api now takes it on
/hog_flows/:id/transactional_email/send and lays it over the email step.
The template route ignores a `send` field, so a stored template send never
picks up content meant for another route.

A cdp-api without this route answers 404, so Django can refuse the send
instead of delivering the stored template or nothing.

A send's capture_engagement can now turn capture off, never on: a team
that turned engagement capture off captures nothing.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The hogflow worker now hydrates a job's workflow with the same primary
fallback the transactional routes use, so an email accepted with a 202
is not dropped when the replica misses its new workflow. The
fingerprint branches on the parsed send instead of a route argument.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@Silthus
Silthus force-pushed the email-v2/06-template-editor branch from b7e93f2 to bb9ed7b Compare October 25, 2026 11:38
@Silthus
Silthus force-pushed the email-v2/07-send-policy branch from 4c04d3c to 5e57bdf Compare October 25, 2026 11:39
@Silthus
Silthus changed the base branch from email-v2/06-template-editor to email-v2/01-cdp-code-send October 25, 2026 11:49
@Silthus
Silthus force-pushed the email-v2/01-cdp-code-send branch 2 times, most recently from 4182790 to 969011f Compare October 31, 2026 08:38
@Silthus

Silthus commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Closing: superseded by the refusal and recipient checks Harley merged upstream (PostHog#123878, PostHog#125908 and the transactional suppression path). The Node chain now runs #565 → #580 → #584. Branch kept for reference.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant