Skip to content

feat(cdp): send sandbox email from the sandbox worker - #131834

Merged
trunk-io[bot] merged 127 commits into
masterfrom
workflows-sandbox/n3-sandbox-sender
Nov 12, 2026
Merged

trunk-io[bot] merged 127 commits into
masterfrom
workflows-sandbox/n3-sandbox-sender

Conversation

@Silthus

@Silthus Silthus commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

👀 Team review: the sandbox sender

I want the team to know what's coming and to hear if you'd do it differently.

Question: this lets a team send real email before it verifies a domain. Is this enough isolation and limiting to turn it on, or is something missing?

  • Sandbox email runs on its own worker, queue and SES identity. It shares only the SES send rate with ordinary email.
  • Only verified members of the team's organization receive it, under daily caps per team and per recipient.
  • WORKFLOWS_SANDBOX_SENDER_ENABLED (off) stops it in a region. A workflows_sandboxsenderorganizationblock row stops one organization.

If you're happy with that, approve this PR and these four. They're the rest of the sandbox sender, so they need no separate review:

If not, comment here with what's missing.

Stamp request: StampHog cannot auto-review this PR (auth files, and 1464 substantive lines over its 1200-line limit). Agents and the merge gate did the bug review, so it needs a stamp from someone on team-workflows.

Note

N2 #131731 has merged, so this diff shows only N3's changes.

Problem

Replaces #128943, #128946, #129053, #131463, and the send-side parts of #129060 and #129063.

Note

🔵 Merge risk: low, 🔴 once sending is on · one-way door once sending turns on: sent messages

  • Ordinary email runs two shared refactors on deploy, in both regions: the moved frequency-cap check and the shared SES bucket options. Shadow-diff, master against this head with sending off and on: ordinary job outcomes, SES calls, bucket claims, frequency-cap decisions and unsubscribe links come out the same.
  • Every Redis v2 caller, ordinary traffic included, starts its callback one microtask later (redis-v2.ts). Callers without a deadline, which is every existing caller, get no new timer and no new Redis call.
  • Shared-path latency is bounded and measured: the frequency-cap check makes the same config read and the same single Valkey call, inside one more async frame. The shadow-diff timed both paths on real Valkey, master against this head, and head stays inside master's own run-to-run spread.
  • The one behavior difference on deploy: the sandbox test path on an ordinary worker answers 404 instead of 401, because that path skips the internal secret and only the sandbox worker serves it.
  • A sandbox send, queued or an editor test, claims one token from the ordinary sender's account-wide SES bucket only after it passes every check and its daily caps. A sandbox burst delays ordinary email; it never exceeds the shared SES rate.
  • An editor test that SES throttles or rejects, or that the SES bucket refuses, gives back both daily caps. A transient SES error keeps the charge, since SES may have sent it.
  • Risk is high once sending is on: a sandbox email reaches a real inbox and cannot be recalled. Uncovered once on: a live SES send and a job replayed across versions. A bug there sends a team member an extra or wrong email.
  • Switch-off: WORKFLOWS_SANDBOX_SENDER_ENABLED, default off, is one switch for every team and stops every sandbox send in a region. A workflows_sandboxsenderorganizationblock row stops one organization. The shared refactors have none: live on deploy, fix forward.
  • Limits: the daily team and recipient caps bound the sends per team and per member. Only verified members of the team's organization receive sandbox email. A staff email suspension or the workflow's email pause stops sandbox sends too.

Rollout

Changes

  • A sandbox email step now sends. The sandbox worker renders it, runs the checks in order, and sends through the sandbox SES identity with the run tags feat(cdp): handle sandbox ses feedback #131828 reads, plus ph_sbx_parent_run for batch-triggered runs.
  • A template that fails to render writes the reason to the run's logs. A step whose action id SES tags cannot carry fails before any check.
  • A retryable refusal retries on the sandbox queue up to 8 times, backing off from 30 seconds to 15 minutes with jitter. A retried step charges the daily caps once: it reuses its claim id from feat(cdp): add sandbox email delivery policy #131731. The step charges its claim before SES gets the email. If that charge cannot be written, nothing sends and the step retries.
  • A replayed job is charged again, also when its worker died mid-send. Only an SES throttle or rejection hands the charge back, so that retry stays free. A transient SES error keeps the charge, since SES may have taken the email.
  • An editor test that SES throttles or rejects refunds both daily caps. Each test claims under a new random id, so holding its claim for a retry, as a queued step does, would never be reused. A refund is simpler than carrying a claim id from the editor. A transient SES error still keeps the charge.
  • Trade-off: a worker that dies after the charge costs the member one send that day, instead of granting one free.
  • Each outcome records one email_sandbox_* app metric and one product event. Sandbox sends never bill.
  • A staff email suspension of the project, or the workflow's email pause, skips a sandbox send with sending_suspended or sending_paused, before any cap, SES token or SES call. An editor test reads the pause from the workflow cdp-api loaded.
  • The suspension read runs under the 2-second authorization deadline. A read that fails or stalls sends nothing and retries; an editor test reports pending. Ordinary email keeps its own read.
  • A sandbox send claims one token from the email worker's account-wide SES bucket, after it passes every check and its daily caps.
  • When the bucket is empty, a queued step retries and an editor test is refused. Both hand back the daily caps they claimed. The editor shows "Too many emails are being sent right now. Send the test again in a moment."
  • The sandbox worker claims the SES bucket through its own pool with a 2-second claim deadline. A bucket store that stops answering fails the claim and drops the stalled connection, and the step retries with no SES call. The sandbox queue no longer gates dequeueing on the bucket. The ordinary email worker keeps its own pool unchanged.
  • The test route claims through its own pool with a 250 ms claim deadline. A bucket store that stops answering refuses the test instead of holding it past the editor's timeout. The pool stays separate because the editor needs a shorter deadline than the worker's 2 seconds.
  • The test route drops a stalled bucket connection like the worker does, so the next test send uses a fresh one.
  • A sandbox worker no longer opens the ordinary email worker's MX validation Valkey pool. That pool exits the process after repeated connection errors, so an SES Valkey that refused connections killed the sandbox worker.
  • An unsubscribe link in a sandbox email carries the address it is delivered to, the member's stored spelling. Ordinary email looks up that exact spelling, so an unsubscribe from a sandbox email also stops ordinary email to that member.
  • Opt-out checks on a sandbox send look up four spellings: as written, trimmed, stored, and lowercased.
  • The worker step reads the stored spelling under the authorization deadline before it renders. When that read fails, nothing sends and the step retries; an editor test send reports pending.
  • Ordinary email keeps its unsubscribe identifier and stays synchronous: only the sandbox render passes the new resolver.
  • An editor test renders in cdp-api, then sends inline on the sandbox worker with a scoped 2-minute token. The route skips the shared internal secret, like the other scoped-token routes.
  • Master's manual-invocation route (fix(workflows): add a CDP route that runs a manual trigger for a user #128904) joined the same exemption list. Both entries stay, each with its own token check.
  • A stalled SES call, caps store, provisioning read, or suppression or opt-out read hits a deadline and retries, instead of hanging the worker's batch. An editor test waits 44 seconds, longer than those deadlines.
  • An SES send whose AWS credentials never resolve settles at the 10-second SES deadline and retries. Before, it held the whole sandbox batch, because credential resolution ignores the abort signal.
  • Mechanical: the marketing frequency-cap check moves into a method that both senders call.

Sandbox sender isolation, N3 highlighted

This PR builds the parts outlined in blue: the sandbox worker's send through the sandbox config set.

How did you test this code?

Test rationale: Each test drives a public seam and asserts what the caller sees: the delivered mail, the daily allowance left in real Valkey, rows in the real test Postgres, and what billing recorded. SES is a local HTTP endpoint that the real AWS SDK client calls.

  • sandbox-email-step.test.ts and sandbox-email-test-route.test.ts run the real event service and team lookup, and assert the product event an in-memory posthog-node client receives. Dropping the event at the step, at the test route, or at the SDK call fails them.

  • 40 mutations of the guarded behavior (the sandbox worker's Valkey pools, refusal handling, allowance reuse, charging before SES, releasing only on a throttle or rejection, opt-out spellings, the delivered-address link and its fail-closed read, the credential stall, SES classification, the test-send bucket, its deadline and its dropped connection, the worker's SES claim deadline, the sandbox frequency cap, retry state, billing) each fail at least one test.

  • sandbox-email-step.test.ts: the real executor routes, sends with tags, hands back, then runs the next action. Also covers each failure, retries_exhausted, and the backoff bounds. The step runs the real frequency cap: with a stored cap of one email a week, the first send arrives and the second skips with frequency_capped.

  • sandbox-email-step.test.ts, unsubscribes: after a member unsubscribes from a sandbox email, the ordinary email check skips their stored spelling as opted_out. An opt-out under any of the four spellings skips the sandbox send. A delivered-address read that never answers retries with no SES call, then sends with the stored address in the link. The test gives the read a short deadline and asserts no duration; removing the deadline fails it by the Jest timeout.

  • sandbox-email-step.test.ts, credentials: with a credential provider that never resolves, the stalled send retries at the SES deadline while the rest of its batch completes.

  • sandbox-email.service.test.ts: every refusal stops before SES. A suppression or opt-out read stuck behind a real table lock retries. A step visit replayed after SES accepted it, or after its worker died while SES took the email, is charged again. A throttled or rejected send retries free; a transient one is charged. A cap store that stops answering after the claim sends nothing and retries.

  • The billing test in hogflow-executor.service.test.ts gains a sandbox step. It reads the records a local usage-ingestion gRPC endpoint accepted: only the function and own-domain email bill, and the step emits no ordinary email metric.

  • sandbox-email-test-route.test.ts: the tester posts to the route through the production app setup, with a shared secret configured. Covers the 403 cases, pending, unsaved workflows, sending off, and a full or unreadable SES bucket. A real Valkey behind a proxy that stops answering mid-connection gets a send_rate_limited answer before the editor gives up. Once the store answers, the next test send is accepted on a fresh connection. A valid token on a path below the route still needs the internal secret, so the auth exemption covers only the route itself.

  • sandbox-email-step.test.ts, SES bucket: a spent bucket retries the step and charges the daily allowance only for the send that goes out. A bucket store that stops answering retries with no SES attempt, then sends once it answers.

  • sandbox-email-step.test.ts and sandbox-email-test-route.test.ts, suspension and pause: a suspended project or paused workflow skips with its reason, leaving the daily allowance, the SES bucket and SES untouched. With the team's email settings behind a real table lock, the step retries with no SES attempt, then sends once the lock lifts.

  • sandbox-email-test-route.test.ts, with both daily caps set to one: after the SES bucket refuses a test, or SES throttles or rejects it, the next test sends. After SES fails in a way that may have sent it, the next test gets daily_cap_reached. Dropping the refund, refunding only the recipient or only the project cap, and refunding a transient failure each fail it.

  • Since the 40 above: refunding only one daily cap, moving the suspension or pause check after the caps, removing the suspension check, dropping the pause at the step, tester or route, and removing the suspension read's deadline or failing it open each fail a test.

  • server.serial.test.ts: a sandbox worker whose frequency-cap Valkey refuses connections stays up past the 10 errors that make an ordinary email worker exit, and starts once Valkey answers.

  • sandbox-ses-client.test.ts: the exact tag names feat(cdp): handle sandbox ses feedback #131828 reads, with and without a parent run. A send SES never answers aborts at the deadline and retries.

  • feat(cdp): route sandbox email steps to their own queue #131766's tests cover on_error, the live-edit route-back and the kill switch.

  • Route walk at 8cba180, on this tree alone: the real sandbox worker, Postgres, Valkey and a fake SES, driven through the HTTP test route the editor calls, with both caps at one. A send is accepted, then capped. After a throttle and a refused SES slot the next test sends; after a rejection too. After a transient SES error the next test is capped. The editor surface ships in U1 feat(workflows): choose the sandbox sender in email steps #131916.

  • Shadow-diff at 8cba180 against master 92ba810: same ordinary jobs, frequency-cap decisions and unsubscribe identifiers, and Redis v2 and frequency-cap latency inside master's own run-to-run spread. The only difference is the sandbox test path's 404 on the ordinary worker.

  • Prod-shape: the sandbox sender tables were read in prod-us and prod-eu at b1c4b98, after P1 (feat(workflows): store the sandbox sender kind and state #131749) deployed. No change since then touches those tables, and the delivered-address read reuses the same query.

  • Not checked: a live SES send, a cross-version run, cancellation while queued, and the shared bucket key, which only the code structure guarantees.

  • Not built: the separate 2-second connect timeout. Only the 10-second total timeout exists.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

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

Skills: /writing-tests, /writing-pr-descriptions, /pr-description, /merge-risk, /leaving-pr-comments

Question Answer Who Where
Opt-out casing Look up the trimmed address, the rendered spelling, the member's stored spelling, and the lowercased address. Unsubscribe links carry the delivered (stored) spelling, which ordinary email looks up exactly; replaces the lowercased link. A failed read retries. 🤖 Agent review thread
Frequency cap Extract the ordinary check unchanged. A sandbox cap hit skips with frequency_capped, no hold. 🤖 Agent comment
Feedback tag names Mirror #131828's SANDBOX_SES_TAGS in the SES client under the same name, since N3 cannot import N4's branch. One should import the other once both land. 🤖 Agent comment
Caps on retry A retried step passes the same claim id, so #131731's script charges it once. A transient SES failure moves to a new id, since SES may have accepted it. The claim is charged before SES, so a replay, even after a crash mid-send, is charged again; a throttle or rejection hands it back. Michael decided the idempotent claim and the in-flight charge on 2026-11-10. 🤖 Agent comment
Test route body Adds optional message_category_id and message_category_type, so test sends run the category check. Accepts the unsaved workflow id. 🤖 Agent comment
More context for AI agents
  • Session: Claude Code implementation lane N3 of the workflows sandbox stack.
  • Decisions: the test route uses a random run id and flow version 0 in its SES tags. Round 1 reviews (GPT-6.1 Sol, PostHog Review) led to the auth exemption, deadlines, and the pending mapping. Final reads led to checking opt-outs under every address spelling, the member's stored one included, and to binding a held cap claim to its recipient. The opt-out check runs after authorization against the address SES receives, which also closes feat(cdp): add sandbox email delivery policy #131731's casing gap (Michael chose to fix it here, 2026-11-10). A later PostHog Review led to keeping caps across retries, render diagnostics, and the provisioning deadline.
  • Decisions (Michael, 2026-11-10): tests rewritten outside-in, editor test sends claim the shared SES bucket, and the production-shape proof waits for P1's deploy.
  • Decisions (Michael, 2026-11-10): idempotent daily-cap claims and bounded recipient reads, both on feat(cdp): add sandbox email delivery policy #131731. The step's claim id is its visit key plus a count of transient SES failures; the test route uses its random run id.
  • Decisions: PostHog Review's atomic caps-claim finding is fixed on feat(cdp): add sandbox email delivery policy #131731: both buckets are claimed in one script, so a held claim always means both.
  • Follow-ups: one shared SANDBOX_SES_TAGS once N3 and N4 land. The 2-second connect timeout. The dev walk follows I1 and C1.
  • Decisions: the master merge kept both internal-api-auth.ts exemptions. The sandbox exemption's mock rows moved to a real HTTP test in sandbox-email-test-route.test.ts. Suppression now goes through master's fail-closed isSuppressed (fix(cdp): hold an email when the suppression list cannot be read #128878), taken on feat(cdp): add sandbox email delivery policy #131731.
  • N4 contract: feat(cdp): handle sandbox ses feedback #131828 accepts only a UUID flow tag, so feedback on a test send of an unsaved workflow (new) counts as unattributed.
  • Decisions: the judge at 387715c asked for an executor test of the real sandbox frequency cap and a bounded pool for the test route's SES bucket; both landed in fcf3ffb. 5e24bc1 merges feat(cdp): add sandbox email delivery policy #131731's claim and mark deadline and shares its stalling Valkey proxy with the test route test.
  • Decisions: PostHog Review round 7 led to 9c090dd, a real deadline on the sandbox worker's SES bucket claim, and to d6e3b40, which merges feat(cdp): add sandbox email delivery policy #131731's acquisition gate: a burst of reads larger than 8 waits its turn, and only 8 acquisitions stuck past their deadline refuse the next one at once.
  • Decisions (Michael, 2026-11-10): "Charge in-flight claims (Recommended)": f0e85f9 charges the claim before SES and releases it only on ses_throttled or ses_rejected, closing the window where a crash after SES acceptance sent one email free of both caps.
  • Decisions: PostHog Review round 8 led to cadfd7e, which gives sandbox workers the frequency-cap pool without the fatal email-mx-validation pool, and to d16bc6c, which gives the test route the worker's non-fatal pool settings with a claim deadline that drops a stalled connection. 3aa1057 and d34dcbf merge feat(cdp): add sandbox email delivery policy #131731's sandbox read pool with a connection timeout, so a database stalled in the startup handshake frees its gate slot.
  • Decisions: the judge at b1c4b98 found both sender suites asserting on a recorder that replaced the event service. 6e6a5cb runs the real event path into an in-memory posthog-node client instead. The risk callout now grades full rollout high, and the N2 note states the stack base instead of a review instruction.
  • Decisions: PostHog Review round 10 led to 07d3d54. Sandbox sends now settle a credential stall at the SES deadline. Unsubscribe links carry the delivered address through an opt-in async recipientIdentifier, so the ordinary default stays synchronous. The worker step's time budget now counts one more 2-second authorization read. The worker heartbeats its jobs, so no job deadline caps that budget.
  • Decisions: the judge at 07d3d54 found a wall-clock assertion in the stalled-read test and a risk callout that missed the Redis callback change. The delivered-address reader now takes a deadline, the test asserts behavior only, and the callout names the Redis change and drops the predictions the shadow-diff disproved.
  • UI walk for U1 feat(workflows): choose the sandbox sender in email steps #131916: at U1 c98f3a0 the result banner and the Logs row both show the send_rate_limited message. U1's banner needs copy for sending_suspended, sending_paused and suspension_state_unavailable (shown as pending).
  • Decisions: the security bot found the editor test claiming the shared SES slot before admission. 32748e9 claims it after the checks and caps for both paths; d68188c and ce8fa95 refund both daily caps when the bucket refuses. The sandbox queue drops its dequeue gate, since the step claims the slot.
  • Decisions: a review found sandbox sends ignoring the staff email suspension. 94cd5d9 enforces it and the workflow pause. Only the staff suspension applies, since the sandbox sends through its own SES tenant, and the read fails closed, unlike ordinary email's.
  • Decisions: PostHog Review round 11 found the suspension read unbounded. 4495d7c reads it through the authorization deadline reader, and the step's time budget counts one more 2-second read.
  • Decisions: the judge at 4495d7c found an editor test that SES throttled or rejected kept both daily caps charged, since each test claims under a new id. 8cba180 refunds both caps for a confirmed-unsent test; queued steps keep releasing their claim for the retry.
  • Decisions: 2c15289 merges master after N2 feat(cdp): add sandbox email delivery policy #131731 squash-merged. Each N2 file merged against the N2 commit this branch last held, so N2's code is neither duplicated nor reverted. N3's own SES_SANDBOX_FROM_ADDRESS config entry was dropped, since feat(cdp): handle sandbox ses feedback #131828 now declares it on master with the same empty default.
  • Read closely: sandbox-email.service.ts (check order), sandbox-ses-bucket.ts (slot claim), sandbox-email-step.ts (retry state), sandbox-render.ts (delivered address), recipient-preferences.service.ts (shared extraction).

🤖 Generated with Claude Code

Silthus and others added 9 commits November 9, 2026 11:23
Pure modules the sandbox email worker will call before it sends: a fresh
primary read of membership, organization block and tenant mirror; tenant
admission with freshness bounds; the two refill daily caps; recipient
suppression and opt-out checks; and the sandbox message builder. Nothing
calls them yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-outs

A sandbox authorization read that passes its deadline now runs no SQL on a
connection handed over late and destroys a connection still busy, so a
stalled primary cannot hold pool slots. An opt-out lookup that fails blocks
the send for a retry instead of letting it through. The caps test shares one
Redis pool per file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add EmailSuppressionService.lookupSuppression, which throws when the team
list or the global hard-bounce list cannot be read. isSuppressed keeps its
signature, its caching and its fail-open result. The sandbox recipient check
calls the strict lookup and retries on a read failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the email_sandbox workflow action, the emailsandbox queue and its
worker mode. The hogflow queue routes a sandbox step to the sandbox
worker, which stores its outcome on the step and hands the job back;
the hogflow queue then finishes the step and runs the next action.
Sending stays off: WORKFLOWS_SANDBOX_SENDER_ENABLED defaults to false
and only the literal 'true' turns it on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A step whose filter stopped matching, or that errored with on_error
continue, moved on inside the sandbox worker and ran the next action
there. The executor now runs one action per dequeue on the emailsandbox
queue, and the sandbox worker hands any job that left the sandbox step
back to the hogflow queue. Also adds email_sandbox to the integration
kind type the wrong-kind test inserts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…licy' into workflows-sandbox/n3-sandbox-sender
The sandbox worker renders a sandbox email step, runs the delivery checks
in order, and sends through the shared sandbox SES identity, tenant and
configuration set with run tags. It retries with backoff (8 tries, 30 s
to 15 min, jitter), records sent, failed and blocked as sandbox-only
metrics and events, and never bills. The worker claims the account-wide
SES bucket the email worker uses. Editor test sends render in cdp-api and
run inline on the worker through a scoped, short-lived token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus Silthus self-assigned this Nov 9, 2026
@Silthus

Silthus commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@github-actions

github-actions Bot commented Nov 9, 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.

@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 778458d: 2 Must fix, 6 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

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/server.ts Resolved
Exempt the sandbox test route from the shared internal secret, since it
verifies its own scoped token. Accept the unsaved workflow id in test
sends, and report a stalled response body as pending. Bound the caps
Valkey pool and each SES request with deadlines, and log SES errors
before classifying them. Check stored preferences under the address the
unsubscribe link carries.

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

Silthus commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

@Silthus

Silthus commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Decisions made while building this PR:

  • 🤖 Opt-out casing Match opt-outs on the rendered address case-sensitively, as ordinary email does. The unsubscribe link carries that same address, so recorded opt-outs match later sends.
  • 🤖 Opt-out identifier Opt-outs are checked under both the trimmed address and the rendered spelling its unsubscribe link carries, when the two differ. The frequency cap is claimed once, on the trimmed address. Authorization and delivery use the trimmed address.
  • 🤖 Frequency cap Extract the ordinary marketing cap check unchanged into a method both senders call. A sandbox cap hit skips the step with frequency_capped and holds nothing.
  • 🤖 Test route body Add optional message_category_id and message_category_type, so test sends run the category check. Accept the unsaved workflow id new.
  • 🤖 Test route body limit Use the app-wide JSON parser, like the other scoped routes, instead of a route-level 2 MB limit that never applied.
  • 🤖 Uncertain test results A test that times out or stalls after its headers reports pending, never failed, because SES may already have accepted it.
  • 🤖 Deadlines Each SES request aborts after 10 seconds and retries. The caps store rejects after 2 seconds with a retryable refusal, and its pool runs in non-fatal mode. An editor test waits 30 seconds for the worker, longer than all of the worker's own deadlines.
  • 🤖 Caps on retry Only a retry after an SES throttle reuses the daily cap tokens its attempt claimed, and only for the same recipient, so a throttle never turns into daily_cap_reached. A transient failure may follow an acceptance, so its retry claims again.
  • 🤖 Frequency cap order Keep the spec's order: the marketing frequency cap runs before the fresh membership read. Moving it later would add I/O between that read and SES, or spend daily cap tokens on capped sends. The slot is keyed by the step visit, so retries reuse it.
  • 🤖 Render diagnostics A render failure writes its error message to the run's logs, on queued runs and editor tests.
  • 🤖 Provisioning deadline The provisioning read uses the authorization read's 2-second primary deadline.
  • 🤖 Feedback tag names The SES client mirrors feat(cdp): handle sandbox ses feedback #131828's SANDBOX_SES_TAGS under the same name and adds ph_sbx_parent_run for batch-triggered runs. One copy should import the other once both land.
  • 🤖 Unsupported action ids A step whose action id holds a character SES tag values cannot carry fails with unsupported_action_id before any check or cap claim. The feedback webhook attributes the same character set, so rewriting the id would lose its feedback.

@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 359a51c: 4 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

- Check sandbox opt-outs under both the trimmed address and the rendered
  spelling its unsubscribe link carries, and claim the frequency cap once.
- A retry after SES throttles keeps the daily cap tokens it already holds.
- A render failure writes its reason to the run's logs, queued and in tests.
- The provisioning read runs under the authorization read's deadline.
- SES sends carry ph_sbx_parent_run for batch-triggered runs, with the tag
  names the sandbox feedback webhook reads.

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

Silthus commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 9, 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 8033197: 4 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.

Silthus and others added 3 commits November 9, 2026 14:38
- A retry reuses the daily cap tokens only after SES throttled the send,
  which means SES refused it. A transient failure may follow an
  acceptance, so the next attempt claims again.
- The held claim names a digest of the organization and stored recipient,
  so a re-render that changes the recipient claims that recipient's caps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A retryable refusal before SES, such as a failed membership read, now
carries the cap claim a throttled attempt holds, so the next attempt
does not spend the daily caps again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l alive

- The editor test waits 30 seconds for the sandbox worker, longer than the
  worker's own reads, cap claims, and 10-second SES send, so a send SES
  accepted is not reported as pending.
- The caps Valkey pool runs in non-fatal mode, so a stalled store retries
  its connections instead of leaking clients or exiting the worker.

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

Silthus commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 9, 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 5435c1f: 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

This is review round 4 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.

SES accepts only letters, digits, underscores and dashes in a tag value,
and the sandbox feedback webhook attributes the same set. A step whose
action id falls outside it now fails with unsupported_action_id before
any check or cap claim, instead of SES rejecting it after the caps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silthus and others added 3 commits November 11, 2026 15:00
The sandbox step and test-route suites asserted on a recorder that
replaced SandboxEmailEvents. They now run the real events class, team
lookup and captureTeamEvent against an in-memory posthog-node client,
and assert on the event the SDK receives.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t SES reply

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	nodejs/src/cdp/services/hog-inputs.service.ts

@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 68029af: 1 Must fix, 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

This is review round 10 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.

…livered address

A sandbox send whose AWS credentials never resolve now settles at the
SES deadline as a retry, so the rest of the sandbox batch completes.

A sandbox email's unsubscribe link now names the address it is
delivered to, the member's stored spelling, so an ordinary email to
that address honours the unsubscribe. The address comes from the
bounded authorization read; when that read is unavailable the step
retries and the editor's test send reports it pending. The ordinary
email path keeps its synchronous identity default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Piccirello Piccirello added the security-review Request a security review label Nov 11, 2026
The delivered-address reader takes a deadline, so the stalled-read test
sets a short one and drops its wall-clock assertion.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@posthog-security-review-bot

posthog-security-review-bot Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

✅ Security review complete — 07d3d54e500e

1 finding posted as a review, in 12 min.

Add the security-review label to run again on the latest commit.

The test now shows the retry through the queue, its schedule and the
later send, not the step's retry counters.

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

@posthog-security-review-bot posthog-security-review-bot 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.

Agent-driven security review - findings inline.

@posthog-security-review-bot posthog-security-review-bot Bot removed the security-review Request a security review label Nov 11, 2026
Silthus and others added 3 commits November 11, 2026 18:46
A sandbox send now claims its slot in the account-wide SES bucket after
provisioning, the organization block, recipient eligibility and the
daily caps admit it, right before SES. A refused send no longer spends
a slot ordinary email needs.

The editor test route and the sandbox worker share the order: the
sandbox queue no longer claims at dequeue, and the step claims through
its own pool with the worker's claim deadline. An empty or unreadable
bucket sends nothing and releases the held cap claim, as an SES throttle
does, so the worker's retry reuses it.

The stalled delivered-address test now asserts the unsubscribe outcome
instead of the decoded identifier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A send refused for an SES slot now returns its daily cap tokens to both
buckets and drops the claim, instead of only clearing the spent marker.
An editor test runs under a new claim id each time, so a released claim
was never reused and a rate-limited test spent one of the member's
daily sends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…box sends

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

@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 94cd5d9: 1 Must fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

This is review round 11 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.

Silthus and others added 2 commits November 11, 2026 19:30
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An editor test claims its daily caps under a fresh id on every request, so
releasing the claim after SES throttled or rejected it left both buckets
charged and the next test could hit daily_cap_reached with nothing sent.
Refund both buckets instead. An ambiguous SES failure still keeps the charge.

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

@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 won't auto-review this pull request because three gates refused it. The deny-list gate matched the auth category, which covers files like internal-api-auth.ts and jwt-utils.ts and the new sandbox authorization code. The size gate refused it because it has 2322 substantive lines against a ceiling of 1200 (6125 lines across 54 files including docs, generated files and snapshots). The tier gate classified it as T2-never.

Please ask a human reviewer to look at it. Since it sits on #131731, you could also split it into smaller pull requests, such as the auth and JWT changes separately from the sandbox email send path, so each piece is easier to review.

Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✗ matches: auth
size ✗ too large for auto-review (2322L substantive in global — ceiling is 1200L; 2322L, 34F total, 6125L/54F incl. docs/generated/snapshots)
tier ✗ classified as T2-never: T2-never (6125L, 54F, single-area, feat)
stamphog 2.8.0 .stamphog/policy.yml @ unknown · reviewed head 8cba180

Resolves N2's squash-merge (#131731) with a 3-way merge per N2 file against the N2 commit N3 last contained, so N2's hunks are neither duplicated nor reverted. Drops N3's SES_SANDBOX_FROM_ADDRESS config entry, which master now declares (#131828).

@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 won't auto-review this pull request because three gates refused it. The deny-list gate matched auth-related files, such as nodejs/src/common/api/middleware/internal-api-auth.ts, nodejs/src/cdp/utils/jwt-utils.ts, and sandbox-authorization.ts. The size gate counted 1464 substantive lines across 27 files, over the 1200-line ceiling. The tier gate classified the change as T2-never (4027 lines, 41 files, a feature), which is never auto-reviewed.

This decision is final, so the next step is a human review from the owning team, as the PR description already requests. Splitting the change into smaller pieces could bring the size under the ceiling, but the auth-related files would still need human review.

Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✗ matches: auth
size ✗ too large for auto-review (1464L substantive in global — ceiling is 1200L; 1464L, 27F total, 4027L/41F incl. docs/generated/snapshots)
tier ✗ classified as T2-never: T2-never (4027L, 41F, single-area, feat)
stamphog 2.8.0 .stamphog/policy.yml @ unknown · reviewed head 2c15289

@Silthus

Silthus commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit 3025275 into master Nov 12, 2026
240 checks passed
@trunk-io
trunk-io Bot deleted the workflows-sandbox/n3-sandbox-sender branch November 12, 2026 12:08
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-12 12:38 UTC Run
prod-us ✅ Deployed 2026-11-12 12:57 UTC Run
prod-eu ✅ Deployed 2026-11-12 12:59 UTC Run

This table shows the posthog release only. Hogstation shows every service that this PR reaches, Kargo services included.

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.

3 participants