fix(scheduler): make per-client fan-out idempotent (no duplicate outreach on retry)#16
Merged
Merged
Conversation
…te PRs/actions
BullMQ is at-least-once. The client fan-out enqueued each child job with no
deterministic jobId, so if the parent fan-out job's worker crashed (or stalled
past its lock) after enqueuing children but before being marked completed,
BullMQ re-ran the parent and fanned out a SECOND full set of child jobs. Those
children auto-execute irreversible per-client actions (outreach emails,
directory submissions), so a single retry produced duplicate outreach.
Give each fan-out child a deterministic id `${jobName}:${clientId}:${YYYY-MM-DD}`
so BullMQ ignores a re-add of an id that already exists — a retried fan-out is
now idempotent. Scoped to the UTC day so a legitimately re-scheduled daily job
on a later day still runs.
Extracted as `fanoutChildJobId()` with a unit test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NpFtUpb53Lj1QHSMxaX774
There was a problem hiding this comment.
Pull request overview
This PR aims to make the scheduler’s per-client fan-out idempotent so BullMQ retries of the parent fan-out job do not enqueue (and execute) duplicate per-client child jobs that can trigger irreversible actions (e.g., outreach).
Changes:
- Added a helper (
fanoutChildJobId) intended to generate deterministic per-client child job IDs. - Updated the scheduler fan-out path to pass a deterministic
jobIdwhen enqueuing per-client child jobs. - Added a unit test validating the helper’s determinism.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/core/scheduler.ts |
Adds a deterministic child jobId strategy and applies it during per-client fan-out. |
tests/core/scheduler.test.ts |
Adds a unit test for deterministic ID generation, with mocks to isolate scheduler import side effects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, { | ||
| // Deterministic id → a retried parent fan-out does not double-enqueue | ||
| // this client's child job (idempotency; prevents duplicate outreach). | ||
| jobId: fanoutChildJobId(definition.name, client.id), |
Copilot correctly flagged that a YYYY-MM-DD-scoped child jobId would dedupe SEPARATE scheduled runs within the same UTC day — e.g. vitals:check-all-sources runs every 6 hours, so after the first run that day the later fan-outs would be silently skipped. Key the child id on the PARENT job's instance id instead: stable across retries of the same fire (so the retry is still deduped), unique per scheduled occurrence (so every fire runs). Update tests accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpFtUpb53Lj1QHSMxaX774
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
From the code audit — PR-2 of the remediation set (High: job-execution safety). The per-client fan-out was not idempotent, so a single BullMQ retry could duplicate irreversible per-client actions.
Finding addressed
H3 — Non-idempotent fan-out → duplicate irreversible actions.
src/core/scheduler.tsprocessJobBullMQ is at-least-once. The fan-out enqueued each child job with no deterministic
jobId. If the parent fan-out job's worker crashes (or stalls past its lock) after enqueuing children but before the job is marked completed, BullMQ re-runs the parent and fans out a second full set of child jobs. Children auto-execute real, irreversible per-client actions (outreach emails, directory submissions per the execution policy), so one retry = duplicate outreach to prospects. This also violates the AGENTS.md rule that job handlers be idempotent.Fix
${jobName}:${clientId}:${YYYY-MM-DD}onqueue.add, so BullMQ ignores a re-add of an id that already exists → a retried fan-out is idempotent.fanoutChildJobId()with a unit test.Not included (verified false positive)
The audit also raised H4 (per-client secrets persisted into Redis job payloads). On inspection this does not occur:
posthogApiKeyis a separate column, and theconfigjsonb built bybuildClientConfigcontains only non-secret fields (targetKeywords, etc.). The fan-out payload carriesclient.config+clientId+clientDomain, never the key. No change made.Scope note
Minimal change (one
jobIdoption + a helper).removeOnComplete: {count:100}retention is unchanged; dedup covers the realistic crash-retry window. A follow-up could switch to age-based retention for a stronger same-day guarantee under very high job volume.Verification
tsc --noEmit+vitestcould not run in the authoring sandbox (private@quantum-l9/llm-routerneedsNODE_AUTH_TOKEN); CI runs both. Addedtests/core/scheduler.test.tsforfanoutChildJobId(deterministic per job/client/day, differs by client and day).src/-only change; no dependencies added.🤖 Generated with Claude Code
Generated by Claude Code