Skip to content

feat(workflows): record sandbox deliveries as their own metric - #610

Closed
Silthus wants to merge 3 commits into
workflows-sandbox-delivery/tier-exclusionfrom
workflows-sandbox-delivery/sandbox-metric
Closed

Silthus wants to merge 3 commits into
workflows-sandbox-delivery/tier-exclusionfrom
workflows-sandbox-delivery/sandbox-metric

Conversation

@Silthus

@Silthus Silthus commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Problem

The sending tier now subtracts email_sandbox_sent (#609), but nothing records it yet.
Sandbox delivery also has no event of its own.
This PR records sandbox sends as their own metric, and captures an event when SES delivers one.

Part of Silthus/personal#199 (sandbox delivery). Stacked on #609.
Replaces the worker half of #606. Originally #454.

Stack

Impact

Grow Workflows, activation rate: sandbox delivery becomes a separate funnel step, and the tier keeps measuring a team's own sending.
No production team sees this yet: WORKFLOWS_SANDBOX_SENDER_ENABLED stays off.

Merge Danger

Warning

🟡 Merge risk: medium · two-way door (merge-risk rubric)

  • A successful non-test sandbox send records email_sandbox_sent next to email_sent. Own-domain sends record the same metrics as before.
  • The SES webhook in the CDP API now takes the sandbox sender. It captures one more event for deliveries on SES_SANDBOX_CONFIGURATION_SET, and does nothing new while that setting is empty.
  • Safe to deploy alone: if this worker deploys before feat(workflows): subtract sandbox sends from the sending tier #609's Django change, sandbox sends count in the tier as they do today, until Django catches up. The sandbox switch is off either way.

Production lines: 97 changed (git diff --numstat against #609, tests and docs excluded).

Changes

Where Before After
Worker, successful non-test sandbox send email_sent email_sent and email_sandbox_sent
SES webhook, delivery on the sandbox configuration set No event workflows sandbox email delivered with the recipient count, test sends included
  • email_sent stays, so the workflow's metrics tab still shows sandbox sends.
  • The webhook captures the event only for a signed tracking code.

How did you test this code?

Live run on the slice tip

  • Live run of the V3 tip 7e95863f7a0 on a PostHog preview. Stubs: a local SES endpoint that records each call, and example.com seeded as a valid MX domain; a local SNS signing certificate, so a signed delivery webhook verifies; the trigger event produced straight onto the CDP events topic; personhog pointed at the persons database.

  • SES tenant DISABLED, then the Django sync: the mirror row reads DISABLED, and the next sandbox send is skipped with "the sandbox sender is paused right now". ENABLED and a sync: the next send goes out. The worker picked up each change without a restart.

  • A live workflow send on the sandbox sender recorded email_sent and email_sandbox_sent once each, the pair the sending tier subtracts.

  • A signed SES Delivery notification for that message on the sandbox configuration set returned 200 and captured workflows sandbox email delivered. The same envelope with a forged signature returned 403.

  • Lane gate on HEAD: tsc passes, and the pre-push hogli ci:preflight --strict passes. The lane classifier puts this PR on the Node side only.

  • Red: on feat(workflows): subtract sandbox sends from the sending tier #609's code, 2 worker cases miss email_sandbox_sent and 4 SES webhook cases capture no delivered event.

  • Review fix: the own-domain send test fails on a mutant that drops the isSandbox guard.

  • Green on HEAD: email-tracking.service.test.ts, email.service.test.ts, sandbox-email-sender.test.ts and helpers/ses.test.ts, 359 passed.

  • Test seams: EmailService.executeSendEmail metrics for sandbox sends, and EmailTrackingService on a signed SES delivery notification.

  • Proofs owed (fleet proof gate):

    • Review: GPT-6.1 Sol, 1 round, 1 finding fixed in 69afe3e4b1f (own-domain sends never record email_sandbox_sent); final read: no findings.
    • Live run: passed at the stack tip (69afe3e4b1f), recorded on every delivery head. One real email worker process (cdp-cyclotron-worker-email) on scratch Postgres, with real Redis, Kafka and ClickHouse: a member received the email, a non-member was skipped, a paused tenant was skipped, the daily team cap held, and each delivered send recorded email_sandbox_sent, which the tier's own ClickHouse read then left out. Stubbed: the SES endpoint (a local HTTP stub behind the real SESv2 client), the MX verdict for the reserved example.com domain, and the tenant-state rows the Django sync would write.
    • Merge-queue lanes: CI computes this check against master, so it reads as mixed until the Node layers below land. Land the stack bottom-first, not with one trunk merge on the top layer.
    • Prod shape: gap, accepted until read-only production access arrives. The sandbox tenant is not configured in production.
    • CI: gap. Fork CI cannot run on Silthus/posthog.

Test rationale: the regressions that matter are a sandbox send that the tier still counts, and a delivery event on the wrong configuration set or an unsigned code.

Release status

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

Docs update

docs/internal/workflows-email-sending-tiers.md documents the sandbox metric and the delivered event.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (implementation). Review by Codex, GPT-6.1 Sol.

Split out of #605 and #606 so each PR passes upstream's merge rules: a migration does not share a PR with nodejs/, and Python does not share a PR with Node. The code is carved by file from those reviewed heads without changes; the stack's tip has the same tree as #606.
Skills: tdd, /writing-tests, pr-description, merge-risk.
Public artifact: all test data is invented.

Decisions

Question Answer Who Where
Slice order of the sandbox sender stack Option B: the Channels row moves into V1 so a team has a sender to test from; V1's worker PRs land first 👍 Accepted default map
How to split #605 and #606 Four fresh single-commit branches off #604, in deploy order: Django mirror, worker gate, Python tier, worker metric; #605 and #606 close 🤖 Agent ticket
How a mixed-lane stack lands Bottom-first; CI checks lanes against master, so each layer passes once the layers below it land 🤖 Agent ticket
Where the docs lines go Each half documents its own behavior; the tip's doc matches #606 🤖 Agent ticket
Sandbox delivery metric Keep email_sent for the workflow metrics tab, add email_sandbox_sent, and subtract it in the tier per day and in the allowance per workflow, floored at zero 🤖 Agent ticket
Sandbox metric beside master's SES metrics email_sandbox_sent stays a team app metric; master's SES counters count each call once, so nothing counts twice 🤖 Agent map

Involvement: no grilling rounds; design pass carried from #605 and #606 (the code is unchanged); review (GPT-6.1 Sol, 1 round and a final read, 1 finding fixed); proof captured as test output and a live run; master merge on 2026-10-31: review of its conflict resolutions (GPT-6.1 Sol, no findings) and a fresh live run at the slice tip.

🤖 Generated with Claude Code

Silthus and others added 2 commits October 28, 2026 09:54
A successful sandbox send records email_sandbox_sent next to email_sent,
so the sending tier can leave it out while the workflow metrics still
show it. The CDP API's SES webhook captures a workflows sandbox email
delivered event for deliveries on the sandbox configuration set.

Refs Silthus/personal#199

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus force-pushed the workflows-sandbox-delivery/sandbox-metric branch from 69afe3e to 7e95863 Compare October 28, 2026 10:00
Silthus added a commit that referenced this pull request Oct 28, 2026
…sandbox metric

Merge workflows-sandbox-delivery/sandbox-metric (#610) without a force-push.
The block migration moves to 0040, above 0039_sandbox_sender_tenant_state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keeps master's email status updates beside the sandbox deliveries in the
SES webhook. The tracking service takes the sandbox sender before master's
optional status service.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silthus added a commit that referenced this pull request Oct 31, 2026
…the block migration

Merge workflows-sandbox-delivery/sandbox-metric (#610) without a force-push.
The block migration moves to 0041, above 0040_sandbox_sender_tenant_state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus

Silthus commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Replaced by PostHog#128412.

@Silthus Silthus closed this Nov 3, 2026
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