Repository navigation
Conversation
…ners early When an organization's sending was stopped, the sequence worker hit the stop before the code that reschedules a failed send, so every scheduler tick retried the same deliveries and logged and reported the error again. Production saw ~25 "sending_paused" errors every 40 seconds for one team. - Sending paused for billing or abuse reasons: queued sends wait and check again every 15-20 minutes, resuming once the stop lifts. - Stops only the team can fix (no feedback connection, unverified owner): the sequence is auto-paused and the owners are emailed once. - Any other failure before the send backs off 5 minutes and counts toward the 5-in-a-row auto-pause, without using up a recipient's retries. Failed sends back off between retries. - Owners are emailed at 50%, 90% and 100% of the cloud test volume while an ESP has no healthy feedback connection (migration 0032). - GET /settings/feedback-requirement (and MCP get_feedback_requirement) powers a notice in Settings -> ESP and the new-ESP dialog. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebd37617f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [claimed] = await db | ||
| .update(organizations) | ||
| .set({ feedbackNoticePercent: reached }) |
There was a problem hiding this comment.
Keep milestone claims retryable when owner email fails
If notifyOrganizationOwners fails due to a transient SMTP error, the catch below records it but leaves feedbackNoticePercent advanced. Later sends then skip this milestone, so the 50%, 90%, or 100% warning is permanently lost; retry the claim or otherwise preserve a retry path when notification delivery fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in abae676: if emailing the owners fails, the milestone claim is released back to its previous value, so the next accepted send tries that warning again (covered by a new test). At 100% no further sends are accepted, so a failed 100% warning is not retried, but the owners still get the email when the sequence auto-pauses with sending_blocked.
| switch (code) { | ||
| case "feedback_connection_required": | ||
| return { | ||
| why: "its email provider isn't reporting bounces and spam complaints back to SendLit, which is required after your first 100 emails", |
There was a problem hiding this comment.
Use the configured volume in the blocked-send notice
When BILLING_TEST_VOLUME_THRESHOLD is set to a value other than 100, the feedback gate uses that configured threshold but this owner notification—and the matching dashboard banner—still says the requirement starts after 100 emails. Owners can therefore be told the wrong limit when the configured threshold is used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in abae676: the sending_blocked pause now records the configured testVolumeThreshold(), and both the owner email and the dashboard banner use it instead of a hard-coded 100. The test sets the threshold to 250 and checks that the email says so.
- Release a 50/90/100% milestone claim when emailing the owners fails, so the next accepted send tries that warning again. - Record the configured test volume on a sending_blocked pause and use it in the owner email and dashboard banner instead of a hard-coded 100. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
get_feedback_requirement imported team/queries and the requirement module, which load the real database client, so the mocked MCP test crashed in CI without DB_CONNECTION_STRING. Look the organization up inside getFeedbackRequirementForTeam, mock that one module in the test, and cover the tool. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
In production, one team's sequence logged
processOngoingSequence failedwithsending_pausedabout 25 times every 40 seconds. Its organization had reached 100 sends on SendLit Cloud without a bounce/complaint feedback connection for its SES ESP, soassertSendingEligibilitystopped sending (feedback_connection_required). That check is correct. The bug was that it throws inresolvePinnedDeliverySource, before the code that reschedules a failed send. Every scheduler tick retried the same 17 deliveries, logged an error and reported to PostHog again, and nobody told the owners why the sequence had stopped.Changes
No more retry floods
Owners are told
feedback_connection_required,verified_owner_email_required) auto-pause the sequence with the newsending_blockedreason and email the owners once. The dashboard banner on the paused sequence explains it.organizations.feedback_notice_percent(migration0032, additive). Nothing is sent on self-hosted.GET /settings/feedback-requirementand MCP toolget_feedback_requirement, with its parity entry and scope. They power a notice in Settings → ESP and in the new-ESP dialog that shows how many of the 100 emails the organization has sent.openapi.jsonis regenerated.Testing
maintoo. The other 10 are in tracking-link, tracking-domain and web-page tests and look like local setup problems (tracking host, HTTPS), but haven't been rerun onmain.Follow-ups (not in this PR)
info.🤖 Generated with Claude Code