[Silthus]

Problem

The email refactor needs coverage that preserves send outcomes, suppression decisions and limiter charges.

Note

🔵 Merge risk: low · two-way door

• This layer changes tests and helpers only. Its blast radius is the Node.js test suite.
• Sends reach loopback SES and SMTP fakes. Production code, dependencies and CI configuration do not change in this layer.

Changes

Kind	Files	Lines
Production	0	+0 −0
Tests	3	+502 −8
Total	3	+502 −8

• Cover the missing S1 outcome cases through real SDK requests, PostgreSQL fixtures and Valkey buckets.
• Characterize unknown providers, post-send faults and spent throttle tokens for their separate behavior fixes.
• Close backlog-test Redis pools and initialize their production Lua commands.

Stacked on #112316; the GitHub diff includes its ancestors. Counts and reviews cover this layer.

How did you test this code?

Test rationale: S1 extends the local SES suite and limiter backlog tests. Each new case fails under its targeted scratch mutation.

Worker round trips are covered by the next layer. Upstream CI and real mailbox delivery remain unverified.

Evidence and context

• accepted outcome matrix
• #311
• #312
• #295
• own-layer diff
• Every new case fails under a scratch mutation
• runner
• Both complete test files pass
• Final lint, typecheck, teardown diagnostic and strict pre-push preflight
• awaited read-only Opus review and fix verification

Release status

• No feature flag controls this change

Docs update

None. This layer changes tests only.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: T3 Code / Codex, GPT-6.1 Sol

Skills: tdd, writing-tests, writing-code-comments, pr, writing-pr-descriptions, reviewing-with-coderabbit, merge-risk, running-ci-preflight, hogli, principle-prove-it-works, writing-voice, leaving-pr-comments, unslop.

Opus findings are fixed: test clients initialize Lua commands, and M10 removes a redundant assertion. The final review found no further defects.
CodeRabbit CLI was signed out and skipped.

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
• Review profile: QUIET
• Plan: Enterprise
• Run ID: f0a14d76-9b3a-479d-842b-3f2112cb5159

📥 Commits

Reviewing files that changed from the base of the PR and between c9e473b and d786508.

📒 Files selected for processing (9)

• .github/workflows/ci-nodejs.yml
• docker-compose.base.yml
• docker-compose.dev.yml
• docs/published/handbook/engineering/developing-locally.md
• nodejs/src/cdp/services/messaging/email.service.ses.test.ts
• nodejs/src/cdp/services/messaging/email.service.test.ts
• nodejs/src/cdp/services/messaging/email.service.ts
• nodejs/tests/helpers/redis-v2.ts
• nodejs/tests/helpers/ses.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Walkthrough

Walkthrough

SES-throttled email invocations now retain their email queue parameters when rescheduled. The changes add a local SES test service and Redis test helper, configure Compose and CI to use the service, and document local test commands. New integration tests cover email delivery, scheduling, provider outcomes, budget handling, errors, retries, and concurrent requests.

Priority: ⬇️ Low

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to d7865

The SES retry and test-support changes are mergeable after normal checks; no outstanding defect was established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d7865

The retry fix retains the existing sender-ownership, suspension, suppression and rate-limit checks. The new email fake is loopback-bound and uses test credentials. Risk remains low rather than minimal because interrupted persistence and concurrent recovery of the same invocation are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The production effect is successful re-entry of SES-throttled email invocations under their existing team and sender context, rather than a new caller or credential authority. The separate infrastructure addition affects the development and CI test environment.

Trust Boundaries and Controls

• observed — Retry execution enters the same send method, which checks team suspension, workflow pause, sender-integration team ownership and channel type, and suppression across to, cc and bcc before limiter claims and provider delivery.
• observed — LocalSes listens on an ephemeral loopback port and accepts only the SES send route. Its configurable upstream can receive forwarded request bodies and headers, but the inspected callers use the default loopback fake and test credentials. Per-instance MessageId filtering separates inbox reads over the shared fake store; it is test isolation, not an authentication boundary.

Resilience and Maintainability Implications

• inferred — Restored retries remain subject to the existing pre-delivery limiter checks, limiting provider exposure rather than bypassing those controls. Repeated attempts can reclaim budget because the inspected retry payload carries no limiter reservation identity; the tests intentionally retain initial charges, while cumulative accounting through durable recovery remains unverified.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description covers the problem, scoped changes, test rationale and evidence, release status, docs, and agent context. The Agent context omits the session link and explicit dispositions for the dup…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[parameterai]

Risk: No findings

This PR adds an SES-outcome test suite (local SES fake via a loopback proxy, real RateLimiterService on test Valkey), a TestRedisV2 helper, compose/CI wiring for a pinned local SES fake, and one production line: re-attaching queueParameters when an SES throttle reschedules the invocation, mirroring the two existing reschedule paths in the same file. The production change is a drop-prevention fix with no new security surface; everything else is test and CI plumbing bound to loopback.

_{Sentinel reviewed d786508 · Review settings}

[scheduled-actions-posthog]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

[Silthus]

Keeping this open. I'll pick it up when I start on Nov 2.

[Silthus]

Moved to #127175 so CI runs.