fix: retain current-minute cron occurrence on recovery - #188
Conversation
There was a problem hiding this comment.
The fix is correct and well-contained. Here's a brief summary of what was verified:
Root cause and fix: materializeDueScheduleOccurrences previously compared row.next_fire_at >= currentMinute to decide whether to emit an occurrence. After a restart spanning multiple missed minutes, row.next_fire_at was always behind currentMinute, so the gate was never entered and the current-minute occurrence was silently dropped. The state was then advanced from now rather than from the overdue scheduled time, compounding the problem. The new loop advances nextFireAt past stale misfires until it reaches or exceeds currentMinute, then conditionally materializes that occurrence (only if <= now), and advances the persisted state from nextFireAt — all three aspects are correct.
Logic walkthrough (test scenario):
createdAt= 11:59:00, so initialnext_fire_at= 11:59:00;recovery= 12:02:30,currentMinute= 12:02:00- Loop: 11:59 → 12:00 → 12:01 → 12:02 (exits because
12:02:00 < 12:02:00is false) - Condition
12:02:00 <= 12:02:30→ materialise occurrence at 12:02:00 ✓ - Next state:
nextCronOccurrence(expression, 12:02:00)= 12:03:00 ✓
Normal (non-recovery) path is unaffected: When row.next_fire_at >= currentMinute, the while loop body never executes, nextFireAt stays equal to row.next_fire_at, and the logic is identical to what it was before.
Edge cases confirmed:
nextFireAtlanding exactly oncurrentMinuteexits the loop (strict<), then the<= nowcheck emits it correctly.nextFireAtovershootingnow(e.g. future minute):nextFireAt <= nowis false → no occurrence emitted, state still advanced to the next following minute, which is the right skip-policy behaviour.ON CONFLICT … DO NOTHINGidempotency guard is untouched.
Test improvements: Fixed timestamps replace the previous new Date()-relative arithmetic, making the test deterministic and self-documenting. Unused nextCronOccurrence import correctly removed.
CI: All checks passed — typecheck, typecheck:test, test:unit, dependency review, and audit.
Summary
Verification
corepack pnpm typecheck:testcorepack pnpm exec vitest run --config vitest.e2e.config.ts test/e2e/schedule-persistence.test.ts(blocked: no container runtime available)