feat(broadcast): cron-driven ramp runner with kill-gate halt#3473
Merged
Conversation
Replaces the manual three-command ritual (assignAndExportWave →
createProLaunchBroadcast → sendProLaunchBroadcast) with a daily
cron at 13:00 UTC that:
1. Fetches the prior wave's getBroadcastStats
2. Halts (sets killGateTripped=true, deactivates ramp) if bounce
rate > 4% or complaint rate > 0.08% — operator must clear
before resume
3. Otherwise runs assignAndExportWave + create + send for the
next tier in `rampCurve`
Singleton config table `broadcastRampConfig` (keyed by literal
"current") holds the curve, current tier, kill-gate state, and last-
wave tracking. Admin mutations: initRamp / pauseRamp / resumeRamp /
clearKillGate / abortRamp / getRampStatus.
Safety rails:
- `MIN_DELIVERED_FOR_KILLGATE = 100`: kill-gate ignored until prior
wave has enough delivered events for stable rate calc (avoids
trip on sample-size noise: 1 bounce / 10 delivered = 10%)
- `MIN_HOURS_BETWEEN_WAVES = 18`: cron defers if prior wave is
fresher than 18h (bounces / complaints take time to flow back via
Resend webhook)
- `UNDERFILL_RATIO = 0.5`: deactivates ramp when assignAndExportWave
returns < 50% of requested count (pool drained signal)
- Kill-gate latch is one-way — never auto-clears. Operator runs
`clearKillGate '{"reason":"..."}'` after investigating, which
stamps the cleared reason into lastRunStatus for audit
- Partial-failure recovery: if assignAndExportWave / create /
send throws mid-flight, status records as "partial-failure" with
the offending error and the cron blocks until cleared. Throws
bubble to Convex auto-Sentry for paging
- `_recordWaveSent` mutation does an `expectedCurrentTier` check
before patching — two concurrent cron firings can't both advance
the same tier (defence-in-depth; cron isn't supposed to overlap
but Convex doesn't guarantee at-most-once on retried cron runs)
Wave-label naming: `${prefix}-${tier + offset}`. Default offset 3
means tier 0 → wave-3, tier 1 → wave-4, etc. — picks up cleanly
after manually-sent canary-250 + wave-2.
Daily-cron timing 13:00 UTC: late enough that overnight bounces /
complaints from the prior 24h have flowed back via webhook, early
enough (9am ET / 6am PT / 3pm CET) that a tripped kill-gate hits
US business hours for triage.
Files:
- convex/schema.ts: new `broadcastRampConfig` table + by_key index
- convex/broadcast/rampRunner.ts: runDailyRamp action + admin
mutations + the two recording mutations
- convex/crons.ts: wires runDailyRamp to crons.daily
- convex/_generated/api.d.ts: regenerated
Operator setup (run once after deploy):
npx convex run broadcast/rampRunner:initRamp '{
"rampCurve": [1500, 5000, 15000, 25000],
"waveLabelPrefix": "wave",
"waveLabelOffset": 3
}'
After that, the cron handles everything until either kill-gate trips
or the pool drains. Status check anytime via:
npx convex run broadcast/rampRunner:getRampStatus '{}'
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR introduces a cron-driven broadcast ramp runner that replaces a three-step manual workflow with a daily Convex action at 13:00 UTC, advancing one wave tier per run and halting automatically when bounce/complaint rates breach kill thresholds.
Confidence Score: 2/5Not safe to merge — two P1 issues could permanently stall the ramp or trigger a duplicate email blast to thousands of users. Two independent P1 findings: no programmatic recovery from partial-failure (only escape is destructive abortRamp), and a real duplicate-send risk if the runtime terminates between assignAndExportWave success and the createProLaunchBroadcast try block. convex/broadcast/rampRunner.ts — partial-failure recovery path and the gap between assignAndExportWave success and the createProLaunchBroadcast try block. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CRON["crons.daily 13:00 UTC"] --> RUN["runDailyRamp (internalAction)"]
RUN --> LOAD["_loadConfigForRunner (internalQuery)"]
LOAD --> CHK_CONFIG{Config exists?}
CHK_CONFIG -- No --> NO_CONFIG["return: no-config"]
CHK_CONFIG -- Yes --> CHK_ACTIVE{active?}
CHK_ACTIVE -- No --> INACTIVE["return: inactive"]
CHK_ACTIVE -- Yes --> CHK_KG{killGateTripped?}
CHK_KG -- Yes --> KG_SKIP["return: kill-gate-tripped"]
CHK_KG -- No --> CHK_PF{lastRunStatus === partial-failure?}
CHK_PF -- Yes --> PF_BLOCK["blocked-on-partial-failure"]
CHK_PF -- No --> CHK_PRIOR{lastWaveBroadcastId?}
CHK_PRIOR -- Yes --> CHK_SETTLE{hoursSince >= 18h?}
CHK_SETTLE -- No --> AWAIT_STATS["awaiting-prior-stats"]
CHK_SETTLE -- Yes --> GET_STATS["getBroadcastStats"]
GET_STATS --> CHK_DELIVERED{delivered >= 100?}
CHK_DELIVERED -- No --> AWAIT_STATS
CHK_DELIVERED -- Yes --> CHK_BOUNCE{bounceRate > threshold?}
CHK_BOUNCE -- Yes --> TRIP_KG["killGate=true, deactivate=true"]
CHK_BOUNCE -- No --> CHK_COMPLAINT{complaintRate > threshold?}
CHK_COMPLAINT -- Yes --> TRIP_KG
CHK_COMPLAINT -- No --> NEXT_TIER
CHK_PRIOR -- No --> NEXT_TIER["Compute nextTier"]
NEXT_TIER --> CHK_DONE{nextTier >= curve.length?}
CHK_DONE -- Yes --> COMPLETE["ramp-complete, deactivate=true"]
CHK_DONE -- No --> EXPORT["assignAndExportWave"]
EXPORT -- throws --> PF_EXPORT["partial-failure recorded"]
EXPORT -- ok, no catch --> GAP["GAP: runtime kill = no record"]
GAP --> CREATE
EXPORT -- ok --> CHK_UNDERFILL{underfilled?}
CHK_UNDERFILL -- Yes --> DRAINED["pool-drained"]
CHK_UNDERFILL -- No --> CREATE["createProLaunchBroadcast"]
CREATE -- throws --> PF_CREATE["partial-failure recorded"]
CREATE -- ok --> SEND["sendProLaunchBroadcast"]
SEND -- throws --> PF_SEND["partial-failure recorded"]
SEND -- ok --> RECORD["_recordWaveSent: advance currentTier"]
RECORD --> SUCCESS["return: sent"]
Reviews (1): Last reviewed commit: "feat(broadcast): cron-driven ramp runner..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+270
to
+283
| handler: async (ctx, args) => { | ||
| const row = await loadConfig(ctx); | ||
| if (!row) return; | ||
| const patch: Record<string, unknown> = { | ||
| lastRunStatus: args.status, | ||
| lastRunAt: Date.now(), | ||
| lastRunError: args.error, | ||
| }; | ||
| if (args.killGate) { | ||
| patch.killGateTripped = true; | ||
| patch.killGateReason = args.killGateReason; | ||
| } | ||
| if (args.deactivate) { | ||
| patch.active = false; |
Contributor
There was a problem hiding this comment.
partial-failure status
When assignAndExportWave or createProLaunchBroadcast throws, _recordRunOutcome sets lastRunStatus: "partial-failure" with killGateTripped left as false. The next cron run bails at line 270 (if (row.lastRunStatus === "partial-failure")), and the only documented recovery — calling clearKillGate — is a no-op in this state: clearKillGate returns early ({ok: true, noop: true}) when killGateTripped is false, so lastRunStatus is never patched. The operator has no mutation to clear the blockage short of abortRamp (which wipes the entire config). A dedicated clearPartialFailure mutation (or extending clearKillGate to reset lastRunStatus regardless of the kill-gate latch) is needed.
Comment on lines
+342
to
+362
| ); | ||
| return { status: "awaiting-prior-stats" }; | ||
| } | ||
|
|
||
| const stats: { | ||
| counts: Record<string, number>; | ||
| bounceRate: number | null; | ||
| complaintRate: number | null; | ||
| } = await ctx.runAction( | ||
| internal.broadcast.metrics.getBroadcastStats, | ||
| { broadcastId: row.lastWaveBroadcastId }, | ||
| ); | ||
| const delivered = stats.counts["email.delivered"] ?? 0; | ||
|
|
||
| if (delivered < MIN_DELIVERED_FOR_KILLGATE) { | ||
| console.log( | ||
| `[runDailyRamp] prior wave only ${delivered} delivered (need ${MIN_DELIVERED_FOR_KILLGATE}) — skip`, | ||
| ); | ||
| await ctx.runMutation( | ||
| internal.broadcast.rampRunner._recordRunOutcome, | ||
| { status: "awaiting-prior-stats" }, |
Contributor
There was a problem hiding this comment.
assignAndExportWave success and the next try-catch
If the Convex runtime terminates the action after assignAndExportWave resolves but before execution enters the createProLaunchBroadcast try block, no partial-failure is recorded and currentTier is not advanced. The next cron tick will re-call assignAndExportWave with the identical waveLabel — if idempotent, contacts won't be double-stamped — but createProLaunchBroadcast is explicitly documented as not idempotent on segmentId (sendBroadcast.ts lines 57-59), so a second broadcast will be created and sent to the same segment.
Consider persisting exportResult.segmentId into the config row immediately after export succeeds so a re-run can detect the partially-complete state and skip re-exporting.
Comment on lines
+214
to
+221
| .first(); | ||
| } | ||
|
|
||
| /** | ||
| * Internal mutation that the action calls to atomically advance the | ||
| * tier + record a successful wave-send. We use a mutation here (not a | ||
| * patch from the action) so the read-modify-write is transactional — | ||
| * two concurrent cron runs (which shouldn't happen, but just in case) |
Contributor
There was a problem hiding this comment.
_recordRunOutcome bypasses Convex schema type-checking
The patch object is typed as Record<string, unknown> and passed directly to ctx.db.patch, silently suppressing compile-time protection against invalid field names or wrong value types. Consider assembling the patch as a typed Partial<...> or using explicit conditional assignments to preserve schema validators.
PR #3473 review: P1 #1 — first automated wave skipped kill-gate for the last manually sent wave because `initRamp` had no way to seed prior-wave metadata. With currentTier=-1 and lastWaveBroadcastId=undefined, the kill-gate block at runDailyRamp's Step 1 was unreachable on the first tick after init. Add `seedLastWave*` optional args; require them as a pair when `waveLabelOffset > 0` (operational signal that this is a resumption after manual waves, not a fresh ramp). P1 #2 — runner narrowed `assignAndExportWave`'s return type to only `{segmentId, assigned, underfilled}`, dropping `failed` and `stampFailed`. A wave that requested 500 with 250 push failures + 250 successes would have proceeded to create + send the broadcast, marking the tier as cleanly advanced. `stampFailed > 0` is worse: contacts are in the Resend segment (will be emailed) but unstamped (re-eligible for the next pick → guaranteed duplicate-email). Now: widen the local type to the full `WaveExportStats`, export it from audienceWaveExport.ts, and abort the run with `partial-failure` status if either failure counter is non-zero. Operator clears via the existing `lastRunStatus === partial-failure` gate.
Owner
Author
|
Both P1s addressed in 35091b5: P1 #1 (first wave skips kill-gate) — added P1 #2 (partial export failures still send) — exported Updated the operator-usage docstring to show the seed args. Convex typecheck clean. |
PR #3473 review (third P1): The partial-failure block I added in 35091b5 (treat any non-zero export failure counter as halt-don't-proceed) had no recovery path. `runDailyRamp` refuses to advance while `lastRunStatus === "partial-failure"`, but `clearKillGate` no-ops when `killGateTripped` is false — so a partial-failure would block the cron forever short of `abortRamp` or hand-patching the DB. Add `clearPartialFailure(reason: string)` matching the `clearKillGate` shape: requires partial-failure status (else no-op), records audit reason in `lastRunStatus`, clears `lastRunError`. Kept separate from `clearKillGate` deliberately — kill-gate is an email-reputation investigation (bounce/complaint thresholds), partial-failure is a mechanical export/send investigation (Resend logs, Convex stamp errors). Different recovery requirements; conflating them would encourage operators to clear without reading the right log. Updated operator-usage docstring with the new command.
Owner
Author
|
Good catch — the partial-failure block I added in 35091b5 had no recovery path. Fix in 705a2e5: added Operator usage docstring updated with the new command. |
5 tasks
koala73
added a commit
that referenced
this pull request
Apr 28, 2026
…ecovery (#3476) * fix(broadcast): lease-based race guard + structured partial-failure recovery Addresses two P1 review findings on PR #3473 (rampRunner already merged). P1 #1 — Race condition that allowed DUPLICATE EMAIL SENDS The runner read currentTier and ran assignAndExportWave + createProLaunchBroadcast + sendProLaunchBroadcast BEFORE recording the tier in _recordWaveSent. Two overlapping cron runs (or cron + manual trigger, or Convex action retry) both passed kill-gate / tier checks, both proceeded through all 3 external side effects, and only collided at _recordWaveSent — by then duplicate emails had gone out to every recipient in the wave. The tier check there is post-hoc, not a pre-claim. Fix: atomic lease, claimed BEFORE any external side effect. New mutation `_claimTierForRun(runId, expectedCurrentTier)` writes pendingRunId + pendingRunStartedAt iff: - currentTier matches expected - no fresh lease is already held (or held lease is older than STALE_LEASE_MS = 30min, in which case override the abandoned lease from a crashed runner) The runner exits cleanly on claim rejection without ever touching external state. _recordWaveSent now validates the lease still belongs to its runId and clears it on success. _recordRunOutcome / clearPartialFailure also clear the lease (only when runId matches, to avoid stomping another run's claim). P1 #2 — Recovery path broken when assignAndExportWave succeeded but createProLaunchBroadcast or sendProLaunchBroadcast failed In that intermediate "stamped + segment created + not sent" state, bare clearPartialFailure makes the next cron retry the SAME waveLabel, which fails because contacts are already stamped. The cron thrashes on the same partial-failure forever short of abortRamp or hand-patching the DB. Fix: new `recoverFromPartialFailure(recovery, ...)` action with two explicit operator-declared modes: - manual-finished: operator manually completed the wave (e.g. via Resend dashboard or direct sendProLaunchBroadcast call). Pass broadcastId, segmentId, sentAt, assigned. Tier advances; next kill-gate uses the recorded broadcastId. Result equivalent to a successful cron run. - discard-and-rotate: write off the lost wave. Bumps waveLabelOffset by 1 so the next cron uses a FRESH waveLabel; the prior label's stamps remain in the audience table and exclude those contacts from future picks (lost to this campaign; operator can manually email later). Tier NOT advanced (no successful send to record). Both modes clear the lease and the partial-failure status. The runner's partial-failure error messages now include the recoverFromPartialFailure invocation hint so on-call ops can see the recovery path right next to the error. Schema additions (convex/schema.ts): pendingRunId, pendingRunStartedAt optional fields on broadcastRampConfig. getRampStatus surfaces both for operator visibility, plus a derived `leaseHeld` boolean. Tests (convex/__tests__/rampRunner.test.ts): 13 cases covering - _claimTierForRun: fresh claim, lease-held rejection (THE RACE GUARD), tier-moved rejection, stale-lease override - _recordWaveSent: lease-validated success, lease-lost rejection - _recordRunOutcome: clears own lease, leaves other lease alone - recoverFromPartialFailure: manual-finished advances tier with operator- provided broadcastId, manual-finished requires all wave fields, discard-and-rotate bumps waveLabelOffset and DOES NOT advance tier, noop when status not partial-failure - end-to-end: two concurrent claims, exactly one wins 107/107 total Convex tests pass (was 94; +13). TS dual typecheck clean, biome clean. * fix(broadcast): drop stale-lease auto-override + persist per-step progress + harden recovery (PR #3476 review round 2) Addresses 4 P1 findings on PR #3476: P1#1 (rampRunner.ts:455-474) — Stale-lease auto-override removed. A 30-min wall-clock cutoff has the same failure mode the lease exists to prevent: assignAndExportWave can legitimately exceed the cutoff for large waves / slow Resend, and overriding while the original run is alive lets a second run race and duplicate-send. A held lease now blocks until cleared by the owning run (terminal outcome path) or by the operator via forceReleaseLease (last-resort, sets lastRunStatus=partial-failure so recoverFromPartialFailure can pick up). P1#2 (rampRunner.ts:545-575) — _recordRunOutcome lease-mismatch is a hard no-op. Previously it suppressed the lease-clear but still patched lastRunStatus / lastRunError / killGateTripped / active when runId did not match pendingRunId, clobbering the winner's authoritative outcome. P1#3 (rampRunner.ts:236-249) — clearPartialFailure now requires `confirmNoExport: v.literal(true)` AND fail-closes if any pending* progress marker is set (which the runner persists as soon as any external step succeeds). The risky path — clear-then-retry after a stamped/sent wave — is rejected loudly; operator must use recoverFromPartialFailure instead. P1#4 (rampRunner.ts:276-355) — Per-step progress persistence. _recordPendingExport (after assignAndExportWave) and _recordPendingBroadcast (after createProLaunchBroadcast) write (waveLabel, segmentId, assigned, broadcastId) to the config row. When the action dies between steps (Convex action timeout, OOM) before any catch block runs, recoverFromPartialFailure falls back to the persisted state instead of requiring the operator to reconstruct from the Resend dashboard. Operator-supplied args still take precedence; sentAt remains operator-only (no marker captures send-completion time). Schema: adds pendingWaveLabel, pendingSegmentId, pendingAssigned, pendingExportAt, pendingBroadcastId, pendingBroadcastAt to broadcastRampConfig (all optional). _recordWaveSent clears all six on success; recoverFromPartialFailure clears them on completion. Tests: ramp runner suite goes 13 → 29. Inverted stale-lease test (now asserts rejection); added forceReleaseLease lifecycle, clearPartialFailure fail-closed-on-pending, _recordPendingExport/Broadcast lease validation, _recordWaveSent clears pending*, manual-finished auto-fills from persisted state, sentAt-still-required, operator-override-beats-fallback, discard-and-rotate clears pending*. * fix(broadcast): persist pending export markers BEFORE inspecting failure counters (PR #3476 review round 3) P1 from review round 3: `_recordPendingExport` was called only after the `(failed > 0 || stampFailed > 0)` and `underfilled` branches recorded partial-failure and bailed. Those branches DID see a real export — segment created, contacts may be stamped, contacts may be in the Resend segment — but with the persistence call deferred past them, the row's `pending*` markers stayed undefined. An operator running `clearPartialFailure({ confirmNoExport: true })` would then NOT trip the fail-closed guard (because the marker check sees no `pendingWaveLabel/SegmentId/BroadcastId`), silently masking the stamped contacts. Fix: move `_recordPendingExport` immediately after `assignAndExportWave` returns successfully, BEFORE inspecting `failed`, `stampFailed`, or `underfilled`. The export ran — record that fact unconditionally on success, regardless of whether the result counters then route us to a partial-failure outcome. Lease validation in `_recordPendingExport` preserves the operator-force-release safety. Test: source-grep regression test asserts `_recordPendingExport,` call site in `runDailyRamp` precedes every `exportResult.failed`/`stampFailed`/ `underfilled` source occurrence. Catches any future reordering that would re-open the gap. * fix(broadcast): route pool-drained through recoverable partial-failure (PR #3476 review round 4) P1 from review round 4: the underfilled branch was recording status= "pool-drained" + deactivate + clearing the lease. By that point assignAndExportWave had already stamped contacts AND created the Resend segment AND pushed contacts — but no broadcast was ever created or sent. The contacts are now excluded from future picks (stamped) but receive no email; recoverFromPartialFailure can't help because status is "pool-drained" not "partial-failure". Fix: route through the recoverable partial-failure path. status= "partial-failure" + deactivate=true + the persisted pending* markers (set by _recordPendingExport in round 3) make recoverFromPartialFailure({recovery:"manual-finished", sentAt:<epoch>}) able to pick up exactly where the runner stopped — operator manually completes the broadcast in Resend, then advances the tier with zero-reconstruction recovery. Or recoverFromPartialFailure({recovery: "discard-and-rotate"}) to explicitly abandon the wave (operator's call, not the runner's). Operator-facing return status renamed to "pool-drained-partial-failure" so logs distinguish this case from a generic partial-failure. Test: source-grep regression test asserts the underfilled branch records status:"partial-failure" (not "pool-drained"), keeps deactivate:true, and includes recoverFromPartialFailure guidance in the error message.
fuleinist
pushed a commit
to fuleinist/worldmonitor
that referenced
this pull request
May 9, 2026
…#3473) * feat(broadcast): cron-driven ramp runner with kill-gate halt Replaces the manual three-command ritual (assignAndExportWave → createProLaunchBroadcast → sendProLaunchBroadcast) with a daily cron at 13:00 UTC that: 1. Fetches the prior wave's getBroadcastStats 2. Halts (sets killGateTripped=true, deactivates ramp) if bounce rate > 4% or complaint rate > 0.08% — operator must clear before resume 3. Otherwise runs assignAndExportWave + create + send for the next tier in `rampCurve` Singleton config table `broadcastRampConfig` (keyed by literal "current") holds the curve, current tier, kill-gate state, and last- wave tracking. Admin mutations: initRamp / pauseRamp / resumeRamp / clearKillGate / abortRamp / getRampStatus. Safety rails: - `MIN_DELIVERED_FOR_KILLGATE = 100`: kill-gate ignored until prior wave has enough delivered events for stable rate calc (avoids trip on sample-size noise: 1 bounce / 10 delivered = 10%) - `MIN_HOURS_BETWEEN_WAVES = 18`: cron defers if prior wave is fresher than 18h (bounces / complaints take time to flow back via Resend webhook) - `UNDERFILL_RATIO = 0.5`: deactivates ramp when assignAndExportWave returns < 50% of requested count (pool drained signal) - Kill-gate latch is one-way — never auto-clears. Operator runs `clearKillGate '{"reason":"..."}'` after investigating, which stamps the cleared reason into lastRunStatus for audit - Partial-failure recovery: if assignAndExportWave / create / send throws mid-flight, status records as "partial-failure" with the offending error and the cron blocks until cleared. Throws bubble to Convex auto-Sentry for paging - `_recordWaveSent` mutation does an `expectedCurrentTier` check before patching — two concurrent cron firings can't both advance the same tier (defence-in-depth; cron isn't supposed to overlap but Convex doesn't guarantee at-most-once on retried cron runs) Wave-label naming: `${prefix}-${tier + offset}`. Default offset 3 means tier 0 → wave-3, tier 1 → wave-4, etc. — picks up cleanly after manually-sent canary-250 + wave-2. Daily-cron timing 13:00 UTC: late enough that overnight bounces / complaints from the prior 24h have flowed back via webhook, early enough (9am ET / 6am PT / 3pm CET) that a tripped kill-gate hits US business hours for triage. Files: - convex/schema.ts: new `broadcastRampConfig` table + by_key index - convex/broadcast/rampRunner.ts: runDailyRamp action + admin mutations + the two recording mutations - convex/crons.ts: wires runDailyRamp to crons.daily - convex/_generated/api.d.ts: regenerated Operator setup (run once after deploy): npx convex run broadcast/rampRunner:initRamp '{ "rampCurve": [1500, 5000, 15000, 25000], "waveLabelPrefix": "wave", "waveLabelOffset": 3 }' After that, the cron handles everything until either kill-gate trips or the pool drains. Status check anytime via: npx convex run broadcast/rampRunner:getRampStatus '{}' * fix(broadcast): seed prior wave + halt on partial export failures PR koala73#3473 review: P1 #1 — first automated wave skipped kill-gate for the last manually sent wave because `initRamp` had no way to seed prior-wave metadata. With currentTier=-1 and lastWaveBroadcastId=undefined, the kill-gate block at runDailyRamp's Step 1 was unreachable on the first tick after init. Add `seedLastWave*` optional args; require them as a pair when `waveLabelOffset > 0` (operational signal that this is a resumption after manual waves, not a fresh ramp). P1 #2 — runner narrowed `assignAndExportWave`'s return type to only `{segmentId, assigned, underfilled}`, dropping `failed` and `stampFailed`. A wave that requested 500 with 250 push failures + 250 successes would have proceeded to create + send the broadcast, marking the tier as cleanly advanced. `stampFailed > 0` is worse: contacts are in the Resend segment (will be emailed) but unstamped (re-eligible for the next pick → guaranteed duplicate-email). Now: widen the local type to the full `WaveExportStats`, export it from audienceWaveExport.ts, and abort the run with `partial-failure` status if either failure counter is non-zero. Operator clears via the existing `lastRunStatus === partial-failure` gate. * fix(broadcast): add clearPartialFailure recovery mutation PR koala73#3473 review (third P1): The partial-failure block I added in 35091b5 (treat any non-zero export failure counter as halt-don't-proceed) had no recovery path. `runDailyRamp` refuses to advance while `lastRunStatus === "partial-failure"`, but `clearKillGate` no-ops when `killGateTripped` is false — so a partial-failure would block the cron forever short of `abortRamp` or hand-patching the DB. Add `clearPartialFailure(reason: string)` matching the `clearKillGate` shape: requires partial-failure status (else no-op), records audit reason in `lastRunStatus`, clears `lastRunError`. Kept separate from `clearKillGate` deliberately — kill-gate is an email-reputation investigation (bounce/complaint thresholds), partial-failure is a mechanical export/send investigation (Resend logs, Convex stamp errors). Different recovery requirements; conflating them would encourage operators to clear without reading the right log. Updated operator-usage docstring with the new command.
fuleinist
pushed a commit
to fuleinist/worldmonitor
that referenced
this pull request
May 9, 2026
…ecovery (koala73#3476) * fix(broadcast): lease-based race guard + structured partial-failure recovery Addresses two P1 review findings on PR koala73#3473 (rampRunner already merged). P1 #1 — Race condition that allowed DUPLICATE EMAIL SENDS The runner read currentTier and ran assignAndExportWave + createProLaunchBroadcast + sendProLaunchBroadcast BEFORE recording the tier in _recordWaveSent. Two overlapping cron runs (or cron + manual trigger, or Convex action retry) both passed kill-gate / tier checks, both proceeded through all 3 external side effects, and only collided at _recordWaveSent — by then duplicate emails had gone out to every recipient in the wave. The tier check there is post-hoc, not a pre-claim. Fix: atomic lease, claimed BEFORE any external side effect. New mutation `_claimTierForRun(runId, expectedCurrentTier)` writes pendingRunId + pendingRunStartedAt iff: - currentTier matches expected - no fresh lease is already held (or held lease is older than STALE_LEASE_MS = 30min, in which case override the abandoned lease from a crashed runner) The runner exits cleanly on claim rejection without ever touching external state. _recordWaveSent now validates the lease still belongs to its runId and clears it on success. _recordRunOutcome / clearPartialFailure also clear the lease (only when runId matches, to avoid stomping another run's claim). P1 #2 — Recovery path broken when assignAndExportWave succeeded but createProLaunchBroadcast or sendProLaunchBroadcast failed In that intermediate "stamped + segment created + not sent" state, bare clearPartialFailure makes the next cron retry the SAME waveLabel, which fails because contacts are already stamped. The cron thrashes on the same partial-failure forever short of abortRamp or hand-patching the DB. Fix: new `recoverFromPartialFailure(recovery, ...)` action with two explicit operator-declared modes: - manual-finished: operator manually completed the wave (e.g. via Resend dashboard or direct sendProLaunchBroadcast call). Pass broadcastId, segmentId, sentAt, assigned. Tier advances; next kill-gate uses the recorded broadcastId. Result equivalent to a successful cron run. - discard-and-rotate: write off the lost wave. Bumps waveLabelOffset by 1 so the next cron uses a FRESH waveLabel; the prior label's stamps remain in the audience table and exclude those contacts from future picks (lost to this campaign; operator can manually email later). Tier NOT advanced (no successful send to record). Both modes clear the lease and the partial-failure status. The runner's partial-failure error messages now include the recoverFromPartialFailure invocation hint so on-call ops can see the recovery path right next to the error. Schema additions (convex/schema.ts): pendingRunId, pendingRunStartedAt optional fields on broadcastRampConfig. getRampStatus surfaces both for operator visibility, plus a derived `leaseHeld` boolean. Tests (convex/__tests__/rampRunner.test.ts): 13 cases covering - _claimTierForRun: fresh claim, lease-held rejection (THE RACE GUARD), tier-moved rejection, stale-lease override - _recordWaveSent: lease-validated success, lease-lost rejection - _recordRunOutcome: clears own lease, leaves other lease alone - recoverFromPartialFailure: manual-finished advances tier with operator- provided broadcastId, manual-finished requires all wave fields, discard-and-rotate bumps waveLabelOffset and DOES NOT advance tier, noop when status not partial-failure - end-to-end: two concurrent claims, exactly one wins 107/107 total Convex tests pass (was 94; +13). TS dual typecheck clean, biome clean. * fix(broadcast): drop stale-lease auto-override + persist per-step progress + harden recovery (PR koala73#3476 review round 2) Addresses 4 P1 findings on PR koala73#3476: P1#1 (rampRunner.ts:455-474) — Stale-lease auto-override removed. A 30-min wall-clock cutoff has the same failure mode the lease exists to prevent: assignAndExportWave can legitimately exceed the cutoff for large waves / slow Resend, and overriding while the original run is alive lets a second run race and duplicate-send. A held lease now blocks until cleared by the owning run (terminal outcome path) or by the operator via forceReleaseLease (last-resort, sets lastRunStatus=partial-failure so recoverFromPartialFailure can pick up). P1#2 (rampRunner.ts:545-575) — _recordRunOutcome lease-mismatch is a hard no-op. Previously it suppressed the lease-clear but still patched lastRunStatus / lastRunError / killGateTripped / active when runId did not match pendingRunId, clobbering the winner's authoritative outcome. P1#3 (rampRunner.ts:236-249) — clearPartialFailure now requires `confirmNoExport: v.literal(true)` AND fail-closes if any pending* progress marker is set (which the runner persists as soon as any external step succeeds). The risky path — clear-then-retry after a stamped/sent wave — is rejected loudly; operator must use recoverFromPartialFailure instead. P1#4 (rampRunner.ts:276-355) — Per-step progress persistence. _recordPendingExport (after assignAndExportWave) and _recordPendingBroadcast (after createProLaunchBroadcast) write (waveLabel, segmentId, assigned, broadcastId) to the config row. When the action dies between steps (Convex action timeout, OOM) before any catch block runs, recoverFromPartialFailure falls back to the persisted state instead of requiring the operator to reconstruct from the Resend dashboard. Operator-supplied args still take precedence; sentAt remains operator-only (no marker captures send-completion time). Schema: adds pendingWaveLabel, pendingSegmentId, pendingAssigned, pendingExportAt, pendingBroadcastId, pendingBroadcastAt to broadcastRampConfig (all optional). _recordWaveSent clears all six on success; recoverFromPartialFailure clears them on completion. Tests: ramp runner suite goes 13 → 29. Inverted stale-lease test (now asserts rejection); added forceReleaseLease lifecycle, clearPartialFailure fail-closed-on-pending, _recordPendingExport/Broadcast lease validation, _recordWaveSent clears pending*, manual-finished auto-fills from persisted state, sentAt-still-required, operator-override-beats-fallback, discard-and-rotate clears pending*. * fix(broadcast): persist pending export markers BEFORE inspecting failure counters (PR koala73#3476 review round 3) P1 from review round 3: `_recordPendingExport` was called only after the `(failed > 0 || stampFailed > 0)` and `underfilled` branches recorded partial-failure and bailed. Those branches DID see a real export — segment created, contacts may be stamped, contacts may be in the Resend segment — but with the persistence call deferred past them, the row's `pending*` markers stayed undefined. An operator running `clearPartialFailure({ confirmNoExport: true })` would then NOT trip the fail-closed guard (because the marker check sees no `pendingWaveLabel/SegmentId/BroadcastId`), silently masking the stamped contacts. Fix: move `_recordPendingExport` immediately after `assignAndExportWave` returns successfully, BEFORE inspecting `failed`, `stampFailed`, or `underfilled`. The export ran — record that fact unconditionally on success, regardless of whether the result counters then route us to a partial-failure outcome. Lease validation in `_recordPendingExport` preserves the operator-force-release safety. Test: source-grep regression test asserts `_recordPendingExport,` call site in `runDailyRamp` precedes every `exportResult.failed`/`stampFailed`/ `underfilled` source occurrence. Catches any future reordering that would re-open the gap. * fix(broadcast): route pool-drained through recoverable partial-failure (PR koala73#3476 review round 4) P1 from review round 4: the underfilled branch was recording status= "pool-drained" + deactivate + clearing the lease. By that point assignAndExportWave had already stamped contacts AND created the Resend segment AND pushed contacts — but no broadcast was ever created or sent. The contacts are now excluded from future picks (stamped) but receive no email; recoverFromPartialFailure can't help because status is "pool-drained" not "partial-failure". Fix: route through the recoverable partial-failure path. status= "partial-failure" + deactivate=true + the persisted pending* markers (set by _recordPendingExport in round 3) make recoverFromPartialFailure({recovery:"manual-finished", sentAt:<epoch>}) able to pick up exactly where the runner stopped — operator manually completes the broadcast in Resend, then advances the tier with zero-reconstruction recovery. Or recoverFromPartialFailure({recovery: "discard-and-rotate"}) to explicitly abandon the wave (operator's call, not the runner's). Operator-facing return status renamed to "pool-drained-partial-failure" so logs distinguish this case from a generic partial-failure. Test: source-grep regression test asserts the underfilled branch records status:"partial-failure" (not "pool-drained"), keeps deactivate:true, and includes recoverFromPartialFailure guidance in the error message.
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
1 participant
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
Replaces the daily 3-command ritual (assignAndExportWave → create broadcast in dashboard → send) with a single Convex cron that wakes once a day at 13:00 UTC and advances the ramp one tier per run. Halts automatically when bounce/complaint rates breach kill thresholds; halts (skips today) when the prior wave hasn't settled enough to read its delivery stats.
convex/broadcast/rampRunner.ts—runDailyRampinternalAction + admin mutations (initRamp,pauseRamp,resumeRamp,clearKillGate,abortRamp,getRampStatus).convex/schema.ts—broadcastRampConfigsingleton table (key=current), tracksrampCurve,currentTier,killGateTripped(one-way latch until cleared), last wave label/segment/sentAt for inter-wave settle gating.convex/crons.ts—crons.daily("broadcast-ramp-runner", { hourUTC: 13, minuteUTC: 0 }, …). 13:00 UTC = 9am ET / 6am PT / 3pm CET — early enough that any kill-gate trip can be triaged within US business hours, late enough that overnight bounces and complaints have flowed back via the Resend webhook.State machine
The action no-ops in any of these states:
paused: true(operator pause)killGateTripped: true(auto-halt; requires explicitclearKillGate)currentTier >= rampCurve.length)sentAtis <MIN_HOURS_BETWEEN_WAVES(18h) agoMIN_DELIVERED_FOR_KILLGATE(100) — we don't have enough signal to evaluate kill thresholds yetWhen it does run a tier:
metrics.getBroadcastStatsForSegment.bounce > 4%,complaint > 0.08%), latcheskillGateTripped, records the trip reason, returns without sending.audienceWaveExport.assignAndExportWavewithcount = rampCurve[currentTier]— same per-wave segment + stamp-after-push idempotency that PR feat(broadcast): per-wave audience export — pick N, stamp, push to fresh Resend segment #3462 shipped.sentAt, advancescurrentTier. Operator still creates the Resend Broadcast against that segmentId; the cron only does the audience picking + push.Operator usage
```bash
Initial setup — pick a curve, label prefix, and starting tier offset
npx convex run broadcast/rampRunner:initRamp '{
"rampCurve": [500, 1000, 2000, 4000, 8000],
"waveLabelPrefix": "pro-launch",
"waveLabelOffset": 3
}'
Inspect state
npx convex run broadcast/rampRunner:getRampStatus '{}'
Pause/resume (e.g. before a holiday weekend)
npx convex run broadcast/rampRunner:pauseRamp '{}'
npx convex run broadcast/rampRunner:resumeRamp '{}'
Clear an auto-halt after investigating
npx convex run broadcast/rampRunner:clearKillGate '{}'
Abort entirely (zeroes the curve)
npx convex run broadcast/rampRunner:abortRamp '{}'
```
Test plan
npx tsc --noEmit -p convex/tsconfig.jsonclean (verified locally — exit 0).crons.dailyregistered in dashboard.initRampwith the post-wave-2 curve. Wait for 13:00 UTC tomorrow. Verify a new segment is created, contacts pushed, prior-wave stats logged. Manually create the Broadcast against that segmentId and send.DEFAULT_BOUNCE_KILL_THRESHOLDlow and confirm the auto-halt path latcheskillGateTrippedand skips the send.