Skip to content

test(workflows): cover the team sending budget on the in-process SES fake - #127175

Merged
trunk-io[bot] merged 13 commits into
masterfrom
chore/workflows-email-s1-cases
Nov 13, 2026
Merged

trunk-io[bot] merged 13 commits into
masterfrom
chore/workflows-email-s1-cases

Conversation

@Silthus

@Silthus Silthus commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Engineers who change the workflow email send path have no test that the team sending cap charges the right budget. Shadow mode, the charge for each copy, and skipped sends are unchecked.
  • A bug there either overspends a team's daily email allowance, which delays its later sends, or lets sends past the cap without counting them.

Moved from #112820, a fork PR, so backend CI runs.

Note

🔵 Merge risk: low · two-way door

  • No production code changes: the diff adds eight cases to email.service.test.ts, and the Node test jobs run them.
  • CI, compose and the dev scripts match master: this PR no longer adds an SES container or a CI step.
  • Switch-off: none needed; a test-only change ships no behavior.
  • Tier: critical (a: nodejs/src/cdp/services/messaging/, a test file only)

Changes

  • New cases in the existing email service suite, which sends through the real SESv2 client to the in-process SES fake from chore(cdp): drive email service tests through a faithful SES fake #132730:
    • Shadow mode charges the budget that enforce mode reads, and still sends once that budget is empty.
    • Each to, cc and bcc copy costs one token. A send with more copies than the whole budget takes all of it and still goes out.
    • A send that a team suspension, a workflow pause or the suppression list skips spends neither the workflow pacing token nor the team budget.
  • The separate Docker SES suite from the first version of this PR is gone, with its CI wait step, compose service, pnpm script flags and handbook line. chore(cdp): drive email service tests through a faithful SES fake #132730 landed a real-SDK fake in process, and master's suite now covers the rest of the old matrix: providers, SES throttles and failures, pacing and team cap reschedules, suppression and suspension, and the engagement read after a send.
  • Nothing user-visible changes.

How did you test this code?

Test rationale: seam EmailService.executeSendEmail, with real PostgreSQL rows, real Redis buckets and the in-process SES fake. Each case reads the budget only through later sends (sendsLeftInTheTeamBudget), never through bucket keys.

  • The eight cases pass, and the whole email.service.test.ts file passes (284 tests).
  • Mutation check, one service mutation at a time. Each failed its target cases:
    • shadow mode skips the charge;
    • shadow mode enforces the cap;
    • one token per send;
    • no clamp to the bucket size;
    • the team claim moves above the skip gates;
    • the pacing claim moves above the skip gates.
  • Owed proofs: none. The change is test-only, so there is no runtime path to run live.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: T3 Code / Codex, GPT-6.1 Sol (the first outcome matrix); T3 Code / Claude Code, Claude Opus 5.5 (the move upstream, and the rework onto the in-process fake)

Skills: takeover, modernize-pr, tdd, writing-pr-descriptions, merge-risk, qa-swarm

More context for AI agents
  • Session: takeover lane for an adopted PR; fleet proof records the reviews.
  • Credit: the outcome matrix comes from chore(workflows): cover the email send outcome matrix #112820 and the first commits on this branch, kept in history.
  • Read closely: the skip gate table, which releases each gate before it measures the budget.
Question Answer Who Where
Keep the Docker SES suite after #132730? No: move the cases master lacks onto the in-process fake, and drop the container and CI step 🤖 comment
Flip the three cases that pinned old behavior? Dropped: master's suite covers the unknown provider and the engagement read; #127174 removed the asset row's catch 🤖 comment

Involvement: one qa-swarm panel round (review and fresh-eyes lanes), PostHog Review; no proof owed.

🤖 Generated with Claude Code

Send test email through a local SES fake, so the send outcome matrix runs
against real SDK requests without mocking SES.
@Silthus Silthus self-assigned this Nov 2, 2026
@trunk-io

trunk-io Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

😎 Stack merged successfully - details.

@github-actions

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

✅ Playwright — all passed

All tests passed.

View test results →

✅ Hobby preview — passed

Hobby deployment smoke test passed successfully.


Run 41634797266

@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 d44f049: 1 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

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 docker-compose.dev.yml Resolved
@hosthog

hosthog Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

Silthus and others added 4 commits November 2, 2026 09:22
- Use the production Redis pool and drop the TestRedisV2 copy.
- Cap and pacing cases assert their own log reason and a drained bucket, so a limiter outage cannot pass for a denial.
- Rows carry their own setup and expected values; tests with two flows are split.
- Titles name the behavior only, without matrix numbers or fork references.
- Drop the harness self-test and the duplicate SES throttle row.
- Keep the handbook to how to run the suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The SES suite runs in the default Jest set, but the fake sits behind the
compose `ses` profile. A run without it now fails with the command that
starts it, instead of an error that reads like a regression.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RedisV2 has no close, so a pool per test left one connection per test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus
Silthus added this pull request to stack #127968 November 3, 2026 07:32
@Silthus
Silthus marked this pull request as ready for review November 3, 2026 08:16
@Silthus
Silthus requested review from gantoine and mayteio November 3, 2026 08:16
@parameterai

parameterai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

This change adds test coverage only: new cases in email.service.test.ts for email status records (UUIDT vs keyed ids, created_at derivation), the signed message-created tracking header, and the team sending budget on the in-process SES fake. No production code, configuration, or trust boundary is modified, so there is no new security risk.

Sentinel reviewed 3014dd5 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team November 3, 2026 08:17
Silthus and others added 4 commits November 3, 2026 08:46
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EmailService now takes a required WorkflowsActivationReporter (#127135).
The local SES suite built the service without it and stopped compiling
after the master merge. It passes a stub, as the email service suite does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silthus and others added 2 commits November 13, 2026 04:07
…ocess SES fake

Master's email service suite now sends through the real SESv2 client to an
in-process SES fake (#132730) and already covers most of this outcome
matrix. Add the cases it lacks there: shadow mode charges the team budget
and sends past an empty one, every to, cc and bcc copy is charged and a
send with more copies than the budget takes all of it and still sends, and
a send that a team suspension, a workflow pause or the suppression list
skips spends neither the workflow pacing nor the team budget.

Drop the Docker SES fake, its CI step, compose service, package scripts
and handbook line, which only served the separate suite.

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

veria-ai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Reviews

@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 5283efe: no findings.

…g budget

Assert that each skipped send records its own gate's metric with no error,
and that the shadow test starts from a full budget, so a send that stops
early for another reason cannot pass as a gate skip.

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

Silthus commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Decisions from the takeover of this PR onto current master:

@Silthus Silthus changed the title chore(workflows): cover the email send outcome matrix test(workflows): cover the team sending budget on the in-process SES fake Nov 13, 2026

@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 48e95af: no findings.

@Silthus

Silthus commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

@PostHog review

@posthog

posthog Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

@Silthus PostHog Review is reviewing this pull request.

stamphog[bot]

This comment was marked as outdated.

@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 3014dd5: no findings.

@stamphog
stamphog Bot dismissed their stale review November 13, 2026 13:40

A new stamphog review started for this PR — the fresh verdict replaces this approval.

@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.

Approved.

Contained test-only additions with no production behavior changes or substantive unresolved concerns. The owning-team author also has a clean current-head review.

  • PostHog Review (Standard) reviewed the current head: no findings.
  • posthog[bot] reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 0L, 0F substantive, 114L/1F incl. docs/generated/snapshots — within ceiling
tier ✓ T0 auto-approve: T0-deterministic (114L, 1F, single-area, test)
stamphog 2.8.0 .stamphog/policy.yml @ 3014dd5 · reviewed head 3014dd5

@Silthus

Silthus commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit 0280b22 into master Nov 13, 2026
215 checks passed
@trunk-io
trunk-io Bot deleted the chore/workflows-email-s1-cases branch November 13, 2026 14:09
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-13 14:46 UTC Run
prod-us ✅ Deployed 2026-11-13 15:05 UTC Run
prod-eu ✅ Deployed 2026-11-13 15:07 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