Skip to content

fix(workflows): keep queue and priority across email queue retries - #127102

Merged
trunk-io[bot] merged 8 commits into
masterfrom
fix/workflows-email-reschedule-keeps-queue-metadata
Nov 3, 2026
Merged

trunk-io[bot] merged 8 commits into
masterfrom
fix/workflows-email-reschedule-keeps-queue-metadata

Conversation

@Silthus

@Silthus Silthus commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Workflow runs that retry an email or push on the email queue return to hogflow at the wrong priority, so bulk sends can jump ahead of other work.

Changes

  • These runs now return to their origin queue at their original priority.
  • Email reschedules and the executor keep the stored origin.
  • A push retry restores the origin after the send.
  • The executor drops the team cap flag before later actions, so only the delayed email counts as a cap delay.
  • Mechanical: a priority helper and the doc.

Before:

flowchart LR
  A[run on hogflow, priority 2] --> B[email queue]
  B --> C[email or push retry]
  C --> D[hogflow, priority 0 or try count]
  classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
  classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  class A phYellow;
  class B,C phBlue;
  class D phRed;
Loading

After:

flowchart LR
  A[run on hogflow, priority 2] --> B[email queue]
  B --> C[email or push retry]
  C --> D[hogflow, priority 2]
  classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  class A,D phYellow;
  class B,C phBlue;
Loading

Note

During the rollout, a run an old worker rescheduled finishes at the old priority, without errors.

How did you test this code?

Test rationale: existing tests now fail without each production line.

  • Email tests, the executor round trip and a cap test pin the origin and the cap flag.
  • A local live run with fake SES and FCM gave the table below.
  • Base and head workers finished each other's jobs, both ways.
  • Not checked: real providers, degraded flows, or live workflow pacing.
Live run: priority on return to hogflow (origin priority 1)
Scenario Base 03cb9b8 Head 0d1bb84
SES throttle reschedule 0 1
Two push retries 2 1
Both 2 1
Team cap delay, then a second email Not run 1

All runs completed. Each value comes from a cyclotron_jobs snapshot taken as the run left the email queue. In the cap run, only the delayed send recorded team_capped="true".

Release status

  • No feature flag controls this change

Docs update

  • The sending-tiers doc covers retries.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: T3 Code / Claude Code, Claude Opus 5.5. An earlier version: Codex, GPT-6.1 Sol.

Skills: /tdd, /writing-tests, /writing-pr-descriptions, /recreating-fork-prs, qa-swarm, merge-risk

More context for AI agents
  • Session: T3 Code thread, Claude Code.
  • Decisions: recreated from fork PR fix(workflows): keep queue and priority across email queue retries #112850, which could not run CI from the fork, at its pinned head 29d303f; all commits are by the PR author, so no squash.
  • Decisions: fda06d5 passes the two PushNotificationService arguments master added, in the executor test only.
  • Decisions: 0d1bb84 follows a master merge that added the team sending cap's teamCapped flag (feat(cdp): label email queue waits that the team sending cap delayed #126304). The merge keeps master's cap reschedule line in the email service, and the executor drops the flag for later actions.
  • Decisions: the push fix first opened as fix(workflows): keep email origin when a push retries on email queue #115192 and folded in here, because it needs the executor line.
  • Decisions: a QA swarm ran two rounds on the fork PR across GPT-6.1 Sol and Claude Opus 5.5; findings are fixed. Three GPT-6.1 Sol reviews on this PR found nothing.
  • Follow-ups: the cloneInvocation comment does not match its code; pre-existing.
  • Read closely: keepPushRetryInEmailQueueOrder, the executor's queueMetadata line, and withoutTeamCapped.
  • No duplicate: no other open PR carries this fix.

🤖 Generated with Claude Code

Silthus and others added 3 commits October 31, 2026 12:06
A rescheduled email dropped its queue metadata, so the run returned to
the hogflow queue at the email class priority instead of its own.
Carry the metadata through SES throttles, workflow pacing, team caps
and consecutive inline sends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A push notification that fails transiently while its run is on the email
queue now keeps the origin queue and priority that the email routing
stashed. The run returns to its origin queue at its origin priority when
it leaves the email queue. The retry waits at its own message category's
email queue priority instead of its retry count. Each push action still
starts its own retry budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Extract getEmailQueuePriority so the inline email send and the push
retry share one class-to-priority lookup. Type the push provider fetch
mock and reset it before each test, so a CI retry of a failed attempt
does not start with leftover queued responses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus Silthus self-assigned this Nov 2, 2026
@trunk-io

trunk-io Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

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

@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 29d303f: 1 Must 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.

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

…cutor test

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

parameterai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Risk: No findings

This increment is a master merge bringing the UTM link-scanner rollout into the branch, plus the PR's wiring of the team gate in email.service.ts. The gate is fail-closed (empty or malformed config falls back to the unchanged legacy scanner), the new scanner's href rewrite only appends fully URL-encoded parameters and re-escapes the attribute value, and the merge resolution dropped nothing from master. No new security issues found.

Sentinel reviewed 2ba5e33 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team November 2, 2026 11:43
@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

  • @PostHog/team-ingestion (owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). For a skipped owner, the locator is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

Silthus and others added 2 commits November 2, 2026 13:01
…-metadata

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The executor now carries a run's queue metadata between actions on the
email queue, so the teamCapped flag from one capped send reached the
next email in the same dequeue and labeled its queue wait as a cap delay.
Drop the flag from the executor's result; the capped send still reads it
from the job's stored metadata.

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

Silthus commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@Silthus

Silthus commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit e60d299 into master Nov 3, 2026
234 of 241 checks passed
@trunk-io
trunk-io Bot deleted the fix/workflows-email-reschedule-keeps-queue-metadata branch November 3, 2026 21:32
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-03 22:15 UTC Run
prod-us ✅ Deployed 2026-11-03 22:48 UTC Run
prod-eu ✅ Deployed 2026-11-03 22:37 UTC Run

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.

2 participants