Skip to content

feat(workflows): validate sandbox sender use at save time - #430

Closed
Silthus wants to merge 1 commit into
workflows-sandbox/routefrom
workflows-sandbox/validation
Closed

Silthus wants to merge 1 commit into
workflows-sandbox/routefrom
workflows-sandbox/validation

Conversation

@Silthus

@Silthus Silthus commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Problem

Workflow builders need clear save-time errors when a sandbox sender is used outside dedicated email steps or with unsupported sender settings.

Supersedes PostHog#111424 (left open as the original draft)

Stack: layer 3 of 12, stacked on #428. Next: #434.

This fork PR moves to an upstream PostHog/posthog PR after review.

Warning

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

  • Shared email validation can reject workflow and destination saves. Existing live and draft sender values are grandfathered to preserve unrelated edits.
  • Project-scoped lookups and API tests cover broadcasts, destinations, rotation, overrides, test sends, and flag changes.

Changes

  • New sandbox selections are limited to workflow email steps and test sends, with the project flag enabled.
  • Broadcasts, destinations, sender rotation, custom From values, and Reply-To overrides receive actionable errors.
  • Unchanged stored email-step sender configurations, including drafts, remain editable after the flag turns off. The worker switch controls global delivery.
  • Test overrides retain the stored broadcast origin. Oversized sender rotations fail before database lookups.
  • Request-scoped integration and flag caches avoid repeated lookups across workflow steps.
  • A workflow that sends through the sandbox sender cannot become a broadcast, and the conversion candidates leave it out.
  • That check also catches sender IDs stored as strings or whole-number floats, and a rotation that includes the sandbox sender.
  • A draft email step without a sender object no longer fails the conversion requests.

Before:

flowchart LR
    Input[Email input] --> Shape[Validate input shape] --> Save[Save workflow or destination]
    classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
    classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
    class Input,Save phYellow;
    class Shape phBlue;
Loading

After:

flowchart LR
    Input[Email input] --> Sender[Validate sandbox use] --> Shape[Validate input shape] --> Save[Save]
    Sender --> Error[Explain unsupported settings]
    classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
    classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
    class Input,Save,Error phYellow;
    class Sender,Shape phBlue;
Loading

How did you test this code?

Current evidence, 2026-10-24, head f279cc92971.

Owed proofs:

  • Live run: rerun at the stack tip 8ac8853cc82, which contains this layer. The stack's cdp-api sent through the workflow test-run endpoint to a local SES fake that records each request. 9 of 9 cases pass. 12 of 13 recorded cases match the run at fa33d78fb36. Case 2c (an unverified member on To) now shows the verify-your-email guidance with reason recipient_unverified, and makes no SES request. No case targets save-time validation or broadcast conversion; the API tests cover them.
  • Prod shape: gap, accepted by Michael on 2026-10-24 until production read access on 2026-11-02. Until then it is unknown how many stored workflow email steps take each changed branch.

Changes in this layer since the 2026-10-19 heads (restacked onto fork master 448d802: the serializer context keeps both master's live_run_scout_notes and this layer's existing_action_emails):

  • Round 1 fixed: broadcast conversion rejects sandbox senders, which master's new conversion path skipped (test_a_workflow_sending_through_the_sandbox_sender_does_not_become_a_broadcast).
  • Round 2 fixed: a sandbox sender ID stored as a whole-number float or a string is recognized (new cases in the same test, red then green).
  • Round 2 fixed: a draft email step whose value is not an object no longer raises a 500 (email step without a sender object case in test_only_workflows_the_broadcast_editor_can_own_are_offered_and_converted, red then green).
  • Round 2 fixed: the candidate list reads only each email step's sender, not the whole action. The candidate tests cover it; behavior is unchanged.
  • Final read: GPT-6.1 Sol read the G1 layer range-diffs and found nothing.
  • Round 4 (R2-11): the save-time validation tests set SES_SANDBOX_FROM_ADDRESS, the setting Django now reads, and the docs follow feat(workflows): provision a managed sandbox email sender #426. Tests and docs only; no behavior change in this layer.
  • Final read 2: GPT-6.1 Sol read the round 4 range-diffs per group and found nothing.
Local gate at the stack tip 8ac8853cc82, shared by every layer

Test seams: workflow and destination HTTP saves, broadcast conversion requests, test-invocation inputs and shared sender validation.

Test rationale: Existing save-time restriction tests now consume the facade ID contract. The destination fixture previously raised AttributeError; the existing file passes after its repair.

Gaps: real AWS SES delivery and the AWS-side tenant pause (every proof uses a local SES fake or an unreachable SES), clustered Valkey, the production data shape (prod-shape gap until production read access on 2026-11-02), and fork CI, which fails during setup: path selection fails without a GitHub App client ID, and the Security aggregate rejects its fork-skipped path-filter job. Application CI is a verification gap.

Earlier evidence (superseded heads)

Round 3 heads, 2026-10-23. This layer's head: d43f182e4a2. Stack tip fa33d78fb36.

Whole-stack QA swarm, 2026-10-19. Reviewed tip cfee7e148ae. This layer's head: 0e5a659b5cc.

  • Live run at b9bd346ed0e (this stack's cdp-api against a local SES fake, workflow test-run endpoint): a sandbox send to a verified member went out from the fixed identity through the sandbox tenant and configuration set. The step's From name, From address and Reply-To were ignored with a log line. A non-member on CC and an unverified member were blocked with the clear message. A paused tenant state skipped sandbox sends while own-sender sends continued.
  • Shadow diff: an own-sender send produced the same SES request and the same stored message on master and on this stack.
  • Cross-version: a worker on master reports a sandbox send as sent without calling SES. See the deploy order in feat(workflows): provision a managed sandbox email sender #426's merge risk.
  • Checks at the tip (Node suites at 1a0fe3663c2; later fixes touch no Node code): Node CDP messaging, members and rate limiter suites (including email-tracking.service.test.ts, which now runs), the cdp-api suite, the Python sandbox, tenant, tier, reputation and validation suites, the frontend Jest suites of this stack, ruff, and the strict pre-push preflight: pass.
  • Gaps: real AWS SES delivery, clustered Valkey, and the production data shape (no production read access yet). Fork CI still fails during setup.

Rebased onto master 734468db6f9 (2026-10-19), head 8cbf743762e.

  • Drift: Master moved posthog/cdp/validation.py to products/cdp/backend/validation.py. Workflows now imports CDP serializers through products/cdp/backend/presentation/serializers.py. Master also added a request-scoped sender domain cache.
  • Adapted: validate_sandbox_email_sender is re-exported from products/cdp/backend/presentation/serializers.py, the path workflows already uses for CDP serializers. Master's domain cache is folded into this layer's shared email_integration_cache, and its comment moves with it. product:lint passes for workflows and cdp.
  • This layer: test_sandbox_sender_validation.py and products/cdp/backend/test/test_validation.py pass at the tip (below).
  • Gaps at modernize time: email-tracking.service.test.ts did not run then. It runs now; see the QA swarm block above. Fork CI still fails during setup, as noted below. Real AWS delivery and clustered Valkey remain unchecked.
Checks at the stack tip

Stack tip b9bd346ed0e (#456), shared by every layer:

  • Python, 354 passed: sandbox sender, sender validation, CDP validation, reputation, SES tenant state and events, sending tier.
  • Node CDP messaging, managers, and rate limiter: 17 suites, 599 passed.
  • Frontend Jest, 37 suites, 487 passed. Five suites timed out under host load in the batch run and passed when rerun alone.
  • Frontend tsgo: no errors. Node tsgo: no errors in CDP. The 23 errors it reports are all in session replay files that need the unbuilt Rust @posthog/replay-anonymizer module.
  • ruff, oxfmt, oxlint, prettier, eslint, product:lint (workflows, cdp), IDOR coverage, hogli build:openapi (no diff), and the strict pre-push preflight: pass.
  • Review: GPT-6.1 Sol read every conflict resolution in the stack and found no issues.
Observed targeted verification after QA fixes
test_sandbox_sender_validation.py: 26 passed
Observed combined-stack gate
Node.js typecheck and build: exit 0
Frontend typecheck and build: exit 0
Success: no issues found in 23388 source files
product:lint workflows: All checks passed
ci:preflight --strict: failures 0

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

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

Docs update

The existing workflow email sending tiers document describes supported sandbox use and grandfathered sender settings.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Decisions

Question Answer Who Where
Prod-shape proofs before production read access Accepted as gaps until production read access on 2026-11-02 👤 Human decision comment

Involvement: restack onto fork master 448d802 on 2026-10-23; round 1 with GPT-6.1 Sol lanes plus Claude Opus 5.5 fresh eyes for group G1; round 2 on the fix diff; final read 1 by GPT-6.1 Sol per group, with no findings; proofs captured; final read 2 by GPT-6.1 Sol on the round 4 decisions, with no findings; the round 4 live run and a UI walk of the two changed surfaces. Before that: the 2026-10-19 whole-stack QA swarm and the modernize rebase (see Earlier evidence). No review bots run on the fork.

Agent: Codex with GPT-6.1 Sol (gpt-6.1-sol) built the original layers, orchestrated by Claude Code with Claude Opus 5.5 (claude-opus-5-5). Restack, review fixes and this description refresh: Claude Opus 5.5 (claude-opus-5-5). Review: GPT-6.1 Sol (gpt-6.1-sol). Fresh eyes: Claude Opus 5.5 (claude-opus-5-5).

Skills: tdd, pr, stacking-prs, writing-pr-descriptions, merge-risk, writing-tests, writing-code-comments, running-ci-preflight, writing-dataclasses, and writing-voice. The description refresh used pr-description, writing-pr-descriptions, writing-voice and merge-risk.

The rebuild uses master's workflow edit-state facade and current input schema serializer. Review fixes restrict grandfathering to real stored email steps, preserve immutable origin in test overrides, and bound lookups. Test sends through invocation configuration remain intentionally allowed. Only public repository sources informed the change.

🤖 Generated with Claude Code

@Silthus
Silthus added this pull request to stack #429 October 12, 2026 10:46
@Silthus Silthus self-assigned this Oct 12, 2026
@Silthus
Silthus force-pushed the workflows-sandbox/validation branch 2 times, most recently from ce6809f to 477571b Compare October 12, 2026 11:15
@Silthus
Silthus force-pushed the workflows-sandbox/validation branch from 477571b to 16dca13 Compare October 12, 2026 21:36
@Silthus
Silthus force-pushed the workflows-sandbox/validation branch 2 times, most recently from d0dbb8d to 8cbf743 Compare October 19, 2026 10:58
@Silthus
Silthus force-pushed the workflows-sandbox/validation branch from 8cbf743 to 0e5a659 Compare October 19, 2026 13:36
@Silthus
Silthus force-pushed the workflows-sandbox/validation branch from 0e5a659 to d43f182 Compare October 23, 2026 16:26
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus force-pushed the workflows-sandbox/validation branch from d43f182 to f279cc9 Compare October 24, 2026 13:11
@Silthus

Silthus commented Oct 24, 2026

Copy link
Copy Markdown
Owner Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Decision lines for this PR. The Decisions table in the description links each row here.

  • 👤 Prod-shape proofs before production read access Accepted as gaps until production read access on 2026-11-02.

@Silthus

Silthus commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Replaced by the re-sliced fork stack: #603. Tracked in Silthus/personal#71.

@Silthus Silthus closed this Oct 27, 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