fix(workflows): guard sandbox sends on empty settings and SES throttle by Silthus · Pull Request #112071 · PostHog/posthog

Silthus · 2026-10-05T18:15:33Z

Problem

Two sandbox sender sends misbehave when the deployment or SES is not in the expected state:

With the sandbox switch on but an empty sandbox tenant, configuration set or From address setting, a sandbox send reaches SES and fails with a provider error instead of a clear skip.
When SES throttles a sandbox send, the retry claims the daily cap tokens a second time. Under throttling a project reaches its daily sandbox cap with fewer emails than the cap allows.

Stacked on #111904. This cumulative fork PR targets master. The review delta is d9ef8d19c62...fix/workflows-sandbox-send-guards.

Note

🔵 Merge risk: low (merge-risk rubric)

The throttle path now writes to the shared rate limiter Valkey through a new Lua script. A wrong credit would let a project send past its daily sandbox cap.
Only sandbox sends call the new method, sandbox sending stays off behind WORKFLOWS_SANDBOX_SENDER_ENABLED, the existing claim scripts are unchanged, and the new paths have focused tests against real Valkey.

Changes

Kind	Files	Lines
Production	3	+109 −12
Tests	2	+102 −38
Total	5	+211 −50

A sandbox send with any empty route setting now skips before SES. The run log shows the same line as a switched-off sender: "the sandbox sender is unavailable right now". The analytics reason stays switch_off, because Django already treats empty sandbox settings as not provisioned.
The worker logs which route settings are empty once, at startup, so an operator sees the cause without a per-send flood.
A sandbox send that SES throttles now gives its daily cap claim back before it is rescheduled. The retry claims again, so the send spends its tokens once.
RateLimiterService.returnClaim adds the tokens back to each bucket that still exists. Reads cap the pool at capacity, so a return can never raise the limit above it.
ioredis resends an unanswered command after it reconnects. Each grant carries a claim ID, and the return script sets a marker first, so a replayed return changes nothing.
Each return also carries a deadline a few minutes out, checked against Valkey TIME. A replay that arrives after the marker expires is dropped.
Own-domain sends are untouched by both changes.

Why return the claim instead of carrying it with the rescheduled send: carrying it needs a marker on the queued email parameters, which the retry must trust. Returning keeps the cap state in Valkey only, and the retry re-runs every sandbox check (pause gate, membership, caps) as before. A return that fails leaves the tokens spent, which only makes the cap stricter.

How did you test this code?

The ticket seams are EmailService.executeSendEmail in email.service.test.ts, with real Postgres fixtures, the real Valkey rate limiter, and SES mocked at sesV2Client.send. The replay guard has its own seam on RateLimiterService against real Redis.

Empty settings: the existing switched-off test became a table. New rows set each route setting empty with the switch on. Each row asserts the skip, no SES call, the switch_off event, and that an own-domain send on the same service still goes out. The three new rows failed before the fix.
Throttle: a new daily caps case, live and test. SES throttles a two-recipient send once, the retry goes out, and the next send hits the project cap of 2. It catches a double claim (the retry is skipped) and a missing claim (the next send goes out). It failed before the fix.
Replay: two returnClaim cases in rate-limiter.service.test.ts. A return sent twice credits once, and a return that reaches Valkey after its deadline credits nothing. Each failed before its fix.

Verify, in nodejs:

pnpm exec jest src/cdp/services/messaging/email.service.test.ts src/cdp/services/rate-limiter/rate-limiter.service.test.ts -t 'skips sandbox sends only when|spends the daily caps once|returnClaim'

Expected at b6e1ba75e0f: Tests: 268 skipped, 11 passed, 279 total (7 empty-settings rows, 2 throttle cases, 2 return cases).

Local gate at b6e1ba75e0f: email.service.test.ts, email-tracking.service.test.ts, the rate limiter suites including the Valkey cluster test, eslint, oxlint, prettier, security semgrep on the touched files, and strict preflight. Node-wide TypeScript reports only the unbuilt @posthog/replay-anonymizer workspace package, which this PR does not touch.

Not checked: a real SES throttle response against the sandbox tenant, or a real ioredis reconnect replay. Fork CI waits for maintainer approval, so no CI run exists yet.

Release status

No feature flag controls this change
This change is behind a feature flag and is not available to users
This change makes a previously flagged feature available to everyone

Both changes only affect sandbox sends, which exist only behind workflows-sandbox-sender and send only while WORKFLOWS_SANDBOX_SENDER_ENABLED is on.

Automatic notifications

Publish to changelog?

Docs update

None. No new setting. The empty-settings behavior matches the existing rule that empty sandbox settings mean "not provisioned".

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (wrote all code in this PR). Reviews by GPT-6.1 Sol (Codex) and Claude Opus 5.5.

Built test-first: each case failed red before its fix. Skills invoked: tdd, /writing-tests, /writing-pr-descriptions, /reviewing-with-coderabbit, qa-swarm, merge-risk.
CodeRabbit CLI pass: skipped, the CLI is signed out and no person was available to sign in. The CodeRabbit bot reviewed the PR.
QA panel, round 1: GPT-6.1 Sol on correctness, risk and design, Claude Opus 5.5 on fresh eyes. The replayed-return finding was fixed. A failed return leaving tokens spent was rejected, because it only tightens the cap and skipping would drop a throttled email. Reporting empty settings as switch_off was kept, as above.
Round 2, risk lens on the fix: a replay after the marker expires. Fixed with the return deadline. CodeRabbit raised the same point and got a reply.
CodeRabbit also flagged that own-domain throttle retries do not restore the email parameters. That path is the same on master and outside this PR, so it is a follow-up.
No duplicate: an open PR search for the sandbox sender found only the earlier layers of this stack.
Public artifact: everything here comes from the public tickets and this repository. Fixture addresses use example.com.

🤖 Generated with Claude Code

…lusions Unfinished: the red/green proof runs and the OpenAPI regeneration were interrupted by a devbox rebalance. Tests are written but not yet run green; generated OpenAPI output is not regenerated. Continue test-first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>