Skip to content

feat(cdp): key a code send on the sender the caller chose - #127129

Merged
trunk-io[bot] merged 46 commits into
masterfrom
email-v2/09-route-idempotency-cdp
Nov 4, 2026
Merged

trunk-io[bot] merged 46 commits into
masterfrom
email-v2/09-route-idempotency-cdp

Conversation

@Silthus

@Silthus Silthus commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Moved from Silthus#580. Part of the code-first rebuild of the email send stack; replaces parts of #121867 (one send per idempotency key on send/email/).

Problem

  • A developer who retries a keyed code send on send/email/ can get 422 idempotency_key_reused instead of a replay, although the first send already went out.
  • This happens when the send names an address that no sender owns exactly, and Django resolves it to a different integration between the send and the retry.
  • This is the cdp-api half of the work. The Django half, feat(messaging): mark a code send that leaves the sender to the project #127182, is closed: Michael decided that replays must keep working across deploys and rollbacks, so Django sends no extra marker.
  • Impact: Pays into Grow Workflows through billed workflows_emails actions per paying org: callers can retry without double sends. In scope: CDP email sends.

Changes

Warning

🔴 Merge risk: high · two-way door: a revert puts the resolved integration back in a named sender's key

  • Deduplication on the CDP email delivery path changes, where a wrong replay drops an email in silence, so the grade is high even behind the flag.
  • Only keyed code sends on transactional_email/send change, and only send/email/ makes them, behind workflows-email-from-code. The template route is unchanged.
  • Rollout window: a keyed named-sender send queued by the older cdp-api keeps the integration in its key. A retry within 24 hours after the deploy, whose address resolved to another integration, answers 422 instead of a replay. A revert does the reverse.
Kind Files Lines
Production 1 +19 −1
Tests 1 +27 −12
  • A keyed retry of a named sender replays after its address resolved to another integration. A named sender keys on the address and name the caller sent, not on the integration id Django picked.
  • Every other code send keys on the sender Django resolved. A sender without an address keys on its integration and name.
  • Trade-off: a default-sender retry after the project's default sender changed still answers 422. The first send went out from another sender, so a replay would hide that change.
  • Base: feat(cdp): deliver a code send's content on its own transactional email route #127122 is on master, so this PR now targets master. Its strict from schema answers 400 to a from.isDefault, so the test row for that retry is gone.
  • Master merge at e88ee89: cdp-api.ts keeps this PR's fingerprint and takes master's concurrent team, workflow and refusal lookups. The over-quota test row is master's folded case.

How did you test this code?

Test seams: cdp-api's accepted-send route over HTTP (cdp-api.serial.test.ts).

Test rationale: returns the first send for a retry of %s and queues nothing new holds every replay, run red first: a named address that resolved to another integration answered 422 before this change. The rejects the key for a request with %s rows pin that another named sender still answers 422, and that a sender without an address still keys on its integration.

Proofs at e88ee89, the master merge (saved with fleet proof, and reused at d1d495d, which only adds the two test rows for a sender without an address):

  • cdp-api.serial.test.ts passes 189 of 189 at e88ee89, and its 12 idempotency rows pass at d1d495d, and ty, prettier and oxlint pass. The local tsc run fails only in the session-replay ai-research code, against a stale local build of its native package, so CI's typecheck is the check there.
  • Live run, cdp-api at this head and Django at the stack top 00f9015:
    • A named address replays on retry after it moved from integration 2 to its own integration 3, with the first message_id and one mail.
    • Another named sender under the same key answers 422.
    • A send without from replays while the default sender is unchanged, and answers 422 after it changed.
    • A template send replays.
  • Shadow diff: HEAD's idempotency tests against master's cdp-api.ts move only the named-address retry, from fail to pass. The other 9 rows match.
  • Cross-version: a cdp-api swap between master e0df364 and this head, in both directions, replays default-sender and template keys. A named-sender retry across it answers 422 and sends no second mail.
  • Prod shape, read 2026-11-04 at d1d495d: in both regions the code-send route took no request in the last 7 days, and keys live 24 hours. Master's Django never mints the code_send claim that route needs; feat(messaging): send an email with inline html from code #127123 adds it. So every stored key is a template key, and the 422 window above affects none.

Note

Not covered here: a Django build without the code-send token claim gets 401 from #127122's cdp-api. That is #127122's rollout, and the workflows-email-from-code flag stays off until both sides deploy.

Release status

  • This change is behind a feature flag and is not available to users

Docs update

  • None in this PR. The send API docs follow when the flag opens.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

Skills: tdd, writing-tests, stacking-prs, modernize-pr, pr-description, writing-pr-descriptions

Question Answer Who Where
Does a reused key with a changed template.hash replay? Yes, and the key's help text says what it binds 👤 Human map Silthus/posthog#461, lane brief of 2026-10-25
May the code carry comments? No; names carry the reasons 👤 Human map Silthus/posthog#461, 2026-10-25
Shrink onto the upstream refusal code? Yes; code that duplicates master goes 👤 Human map Silthus/posthog#461, 2026-10-28
Keep the inactive refusal's own why and fix? No; Django on master maps not_sendable to its own answer 🤖 Agent map Silthus/posthog#461 lane node3, restack of 2026-10-31
Mark the default sender so its retry replays after the default changed? No; replays must keep working across deploys and rollbacks, so no marker, and #127182 closed 👤 Human map Silthus/posthog#461, 2026-11-03
Is this PR still needed after the from-fallback landed? Yes; the fallback only fills a missing from, and a named address that resolved elsewhere still needs this key 🤖 Agent map Silthus/posthog#461, 2026-11-04
More context for AI agents

🤖 Generated with Claude Code

Silthus and others added 5 commits October 31, 2026 08:21
…il route

A code send posts its accepted content to transactional_email/send. The route
runs the same reserve, replay, refusal and enqueue steps as a template send.
The accepted send binds the idempotency fingerprint, and a miss on a new
workflow reads it from the primary.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
A keyed retry of a code send replays after the project's default sender
changed, and after Django resolved a named address to another sender. The key
leaves out a default sender (`from.isDefault`) and keys a named address on the
address and name the caller sent, not on the integration Django picked.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@Silthus Silthus added hogbox-preview Spin up a per-PR PostHog preview on a hogland hogbox posthog-review Run a Standard PostHog Review labels Nov 2, 2026
@Silthus Silthus self-assigned this Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane (node:cdp)

This PR is assigned to the non-backend lane (node:cdp). 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.

✅ Hogbox preview — ready, open the preview

▶ Open the preview

🔑 Login [email protected] / 12345678 (demo data)
🧩 Running this PR's backend on the PostHog :master base (frontend unchanged by this PR)
🔗 Link stable across rebuilds: a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit e99c3df · box box-d48c87ffe7be · ready in 2494s (push → usable) · build log · rebuilds on every push, torn down on close

@Silthus
Silthus added this pull request to stack #127131 November 2, 2026 06:57

@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 64d8940: 1 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.

Comment thread nodejs/src/cdp/cdp-api.ts Resolved
Silthus and others added 2 commits November 2, 2026 17:30
A send with captureEngagement false still put the recipient's distinct_id
in its SES header and pixel/redirect tracking codes. Minting still emits
the v2 payload, which has no capture choice, so the SES delivery, bounce,
open and click callbacks for that send captured engagement events.

Every reader skips engagement capture for a code without a distinct_id,
so leaving it out honors the choice on every deployed reader now. The v3
payload and its minting switch stay as they are. The send-time capture
and the message assets row keep their distinct_id.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The test for a capture-off send covered only the SES header code. Its
email had no link, so nothing checked that the pixel and redirect codes
leave out the distinct_id too. The dev/test case now checks both codes
for each capture choice.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The base now refuses an unknown field on from, and no Django sends the
marker, so the retry cannot reach the key.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@trunk-io

trunk-io Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

😎 Stack merged successfully - details.

#127122 landed on master, so this layer now targets master. Conflicts in
cdp-api.ts keep this layer's retry fingerprint (the sender the caller chose)
and take master's concurrent team, workflow and refusal lookups. The test file
keeps this layer's retry cases and takes master's folded over-quota case.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@Silthus

Silthus commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@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 e88ee89: no findings.

A keyed retry of a code send whose sender has no address replays while its
integration is unchanged, and answers 422 after the integration changed.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@Silthus

Silthus commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@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 d1d495d: no findings.

@Silthus Silthus added the stamphog Request AI approval (no full review) label Nov 4, 2026
stamphog[bot]

This comment was marked as outdated.

Keeps both sides' rows in the rejected-key table: this PR's named and
address-less sender rows and master's changed cc row (#127985).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@stamphog
stamphog Bot dismissed their stale review November 4, 2026 17:32

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

@Silthus

Silthus commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@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 disclosed risky changes have owning-team authorship and STRONG familiarity, with focused regression coverage. All listed reviews are on older commits; no concrete unresolved substantive concern or other refusal ground is present.

  • Author wrote 100% of the modified lines and has 5 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 20L, 1F substantive, 74L/2F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1b-small (74L, 2F, single-area, feat)
stamphog 2.7.0 .stamphog/policy.yml @ e99c3df · reviewed head e99c3df

@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 e99c3df: no findings.

@Silthus

Silthus commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit 172c740 into master Nov 4, 2026
216 checks passed
@trunk-io
trunk-io Bot deleted the email-v2/09-route-idempotency-cdp branch November 4, 2026 18:47
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-04 19:26 UTC Run
prod-us ✅ Deployed 2026-11-04 19:40 UTC Run
prod-eu ⏳ Pending

This branch was successfully deployed

1 active deployment
hogbox-preview — e99c3df9 Deployed Nov 4, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hogbox-preview Spin up a per-PR PostHog preview on a hogland hogbox posthog-review Run a Standard PostHog Review stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant