Skip to content

Missing EmailService cases through executeSendEmail (seam S1) #298

Closed

Description

@Silthus

Part of #291

Question

Add the missing EmailService cases at seam S1 from the accepted architecture brief (section 9, cases M5-M16 and M18-M21), so the refactor runs against them.

  • Seam S1: executeSendEmail on real Postgres, reaching SES through sesEndpoint to the local SES fake from chore(workflows): send test email through a local ses fake PostHog/posthog#112316, with a real RateLimiterService on the test Valkey (C6). No send() spy.
  • Cases: the reschedule footprint for all three causes, unknown and unsupported providers, maildev rejection, skip-flag characterization, integration lookup rejection, failure before the tracking decision, team cap shadow mode, tier fail-open and per-recipient charge, token non-refund (M15), test-send throttle, no token spent on skipped sends, post-send failures, cc/bcc suppression.
  • Where a case pins a bug the brief lists (M6 unknown provider, M19/M20 post-send failures), pin today's behavior and name the fix ticket that flips it.
  • Also fix the existing suite's teardown leak (a worker or timer left open; jest warns).

Builds on PostHog#112316's branch chore/workflows-local-ses-fake.

Activity

  1. Silthus commented on Oct 6, 2026

    @Silthus
    OwnerAuthor

    Scope note from the conductor: build on PostHog#112316's branch chore/workflows-local-ses-fake, which holds the SES fake, capture proxy and fault helpers. Also fold in that ticket's follow-up: the existing email.service.test.ts suite leaves a worker or timer open at teardown (it exits successfully, but jest warns). Fix it while you're in that file.

  2. changed the title [-]Outside-in EmailService tests through the agreed seams[/-] [+]Missing EmailService cases through executeSendEmail (seam S1)[/+] on Oct 6, 2026
  3. Silthus commented on Oct 6, 2026

    @Silthus
    OwnerAuthor

    Runtime: T3 delegate_task (app-owned child of the conductor thread)
    Task: wf291-implement-298-r1
    Cast: codex / gpt-6.1-sol, reasoning high (bounded implementation); cross-family review: claude -p claude-opus-5-5
    Branch: chore/workflows-email-s1-cases (stacked on chore/workflows-local-ses-fake, PostHog#112316)
    CodeRabbit slot: 1 of 3
    Cap: 150 min wall time

  4. self-assigned this
    on Oct 6, 2026
  5. Silthus commented on Oct 6, 2026

    @Silthus
    OwnerAuthor

    Lane recovery (conductor): a T3 server restart cancelled run 1 before any push. The same worker resumes in run 2 with its uncommitted worktree intact.
    Runtime: T3 delegate_task child thread, run 2 (message wf291-298-resume-after-restart-1)
    Cap: 120 min wall time from 17:45 UTC

  6. Silthus commented on Oct 6, 2026

    @Silthus
    OwnerAuthor

    Note

    🤖 GPT-6.1 Sol responding on behalf of Michael

    Draft PR: PostHog#112820. The S1 implementation, mutation proof and local gate are complete; the required read-only Opus review follows.

  7. Silthus commented on Oct 6, 2026

    @Silthus
    OwnerAuthor

    Note

    🤖 GPT-6.1 Sol responding on behalf of Michael

    Shipped draft PR #112820 for all requested S1 cases M5-M16 and M18-M21, plus the existing suite's Redis-pool teardown leak.
    Reviewed head: d786508d2f2cc5ab4292f23a35863de22bbe7e4f. The worktree is clean. The PR targets master and carries PostHog#112316 and PostHog#112295 until they merge; neither parent branch nor PR was changed.

    Proof artifacts, also linked in the PR description:

    • Red: every added case catches a scratch regression; the runner restores production source.
    • Green: both full files pass, all 148 tests, none skipped and every existing assertion intact.
    • Final local gate: hogli test, full Node.js lint/typecheck, clean --detectOpenHandles teardown and strict preflight through the actual pre-push hook. No hook bypass. The temporary launcher selects the actual PR base, upstream/master; the fork-master age advisory describes a different base.

    Merge risk is low, two-way door, graded on this layer only: three test/helper files, +502 −8, zero production lines. Sends use loopback fakes. No danger-area QA swarm applies.

    Review record: CodeRabbit was checked first and skipped because cr auth status --agent returned not_authenticated. Two awaited, read-only claude -p --model claude-opus-5-5 calls reviewed the own-layer diff and verified the fixes at the final head. Parent commits and the upstream refresh were excluded.

    • Low, missing Lua commands behind the helper's RedisClient cast: fixed by initializing both production Lua commands for every pooled client.
    • Nit, redundant new M10 input-object assertion: removed. Existing assertions remain.
    • Opus confirmed both fixes and no new defects. No findings were rejected. Its nonblocking setup limitations are recorded in the review artifact.

    Follow-ups are already tracked: #311 flips M6, #312 flips M19/M20, #313 decides skip/billing policy, and #295 graduates the refund policy that flips M15. No additional ticket is proposed.

    GitHub rejected PR assignment with a permissions error, so the draft remains unassigned; its Agent context records human direction and the exact implementation model. Upstream CI awaits maintainer approval and is outside this handoff. S2 is not claimed. No services were stopped or restarted, and no merge was attempted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions