← Back to PRs

#22948: fix(cron): every-schedule boundary returns nowMs instead of next slot (#22895)

by echoVic open 2026-02-21 21:16 View on GitHub →
size: M trusted-contributor
## Problem When `elapsed` is an exact multiple of `everyMs` in `computeNextRunAtMs`, the ceiling-division formula `Math.floor((elapsed + everyMs - 1) / everyMs)` returns the **current** slot instead of the next one, producing `nextRunAtMs === nowMs`. This causes the job to be perpetually "due" after a gateway restart: 1. `recomputeNextRuns` sees `now >= nextRun` → recomputes → gets the same value 2. `runMissedJobs` double-executes the job 3. `applyJobResult` recomputes `nextRunAtMs` from `endedAt`, landing on a later anchor-aligned slot 4. Dashboard shows an incorrect NEXT time (e.g. **56m for a 30m job**) ### Steps to reproduce (from #22895) 1. Create a cron job with `schedule.kind = "every"`, `everyMs = 1800000` (30m) 2. Let it run successfully 3. Restart the gateway 4. Observe NEXT shows ~56m instead of ~24m ## Fix Replace the ceiling-division with `Math.floor(elapsed / everyMs) + 1` which always returns a **strictly-future** slot. Both formulas produce identical results for non-boundary cases; only the exact-multiple edge case differs. ```diff - const steps = Math.max(1, Math.floor((elapsed + everyMs - 1) / everyMs)); + const steps = Math.floor(elapsed / everyMs) + 1; ``` ## Testing - 3 new tests covering the restart scenario, stale `runningAtMs`, and all anchor offsets - All 178 existing cron tests pass (32 test files, 0 failures) ``` ✓ preserves correct nextRunAtMs for every-30m job after restart (last ran 6m ago) ✓ handles restart when every-30m job was running (stale runningAtMs) ✓ next run never exceeds everyMs from now for various anchor offsets ``` Fixes #22895 <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes a critical boundary condition bug in `every`-schedule cron jobs where `elapsed` being an exact multiple of `everyMs` caused `computeNextRunAtMs` to return `nowMs` instead of a future time. This led to perpetual "due" status after gateway restarts, resulting in double-execution and incorrect dashboard timing displays (e.g., 56m for a 30m job). **Key changes:** - Replaced ceiling-division formula with `Math.floor(elapsed / everyMs) + 1` in `src/cron/schedule.ts:45`, which always returns a strictly-future slot - Removed unnecessary `Math.max(1, ...)` wrapper since new formula guarantees result ≥ 1 - Added comprehensive test coverage for restart scenarios, stale `runningAtMs`, and anchor offset edge cases **Mathematical correctness:** - Old formula at boundary: `elapsed = everyMs` → `steps = 1` → `nextRunAtMs = nowMs` (bug) - New formula at boundary: `elapsed = everyMs` → `steps = 2` → `nextRunAtMs = nowMs + everyMs` (correct) - All non-boundary cases produce identical results **Test coverage:** - 3 new tests specifically for issue #22895 (218 lines total) - Tests verify restart behavior, stale state handling, and all anchor offsets (0-29 minutes) - All 178 existing cron tests remain passing <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - it fixes a well-defined boundary condition bug with thorough test coverage - The fix is mathematically sound and minimal (single line change), addresses a specific bug with clear reproduction steps, includes comprehensive test coverage (3 new tests covering all edge cases), and all existing tests pass. The change preserves behavior for all non-boundary cases while correctly fixing the exact-multiple edge case. No security or performance concerns. - No files require special attention <sub>Last reviewed commit: 629445e</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs