From b9919197550211cb88cb152fb06a7a3805127549 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 16 Feb 2026 17:06:10 +0000 Subject: [PATCH] refactor(cron): dedupe next-run recompute paths --- .../service.issue-17852-daily-skip.test.ts | 36 +++++------- src/cron/service/jobs.ts | 55 +++++++++---------- 2 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/cron/service.issue-17852-daily-skip.test.ts b/src/cron/service.issue-17852-daily-skip.test.ts index 2b04472a03c..be34651a05b 100644 --- a/src/cron/service.issue-17852-daily-skip.test.ts +++ b/src/cron/service.issue-17852-daily-skip.test.ts @@ -39,14 +39,8 @@ describe("issue #17852 - daily cron jobs should not skip days", () => { }; } - it("recomputeNextRunsForMaintenance should NOT advance past-due nextRunAtMs", () => { - // Simulate: job scheduled for 3:00 AM, timer processing happens at 3:00:01 - // The job was NOT executed in this tick (e.g., it became due between - // findDueJobs and the post-execution block). - const threeAM = Date.parse("2026-02-16T03:00:00.000Z"); - const now = threeAM + 1_000; // 3:00:01 - - const job: CronJob = { + function createDailyThreeAmJob(threeAM: number): CronJob { + return { id: "daily-job", name: "daily 3am", enabled: true, @@ -56,9 +50,19 @@ describe("issue #17852 - daily cron jobs should not skip days", () => { createdAtMs: threeAM - DAY_MS, updatedAtMs: threeAM - DAY_MS, state: { - nextRunAtMs: threeAM, // Past-due by 1 second + nextRunAtMs: threeAM, }, }; + } + + it("recomputeNextRunsForMaintenance should NOT advance past-due nextRunAtMs", () => { + // Simulate: job scheduled for 3:00 AM, timer processing happens at 3:00:01 + // The job was NOT executed in this tick (e.g., it became due between + // findDueJobs and the post-execution block). + const threeAM = Date.parse("2026-02-16T03:00:00.000Z"); + const now = threeAM + 1_000; // 3:00:01 + + const job = createDailyThreeAmJob(threeAM); const state = createMockState([job], now); recomputeNextRunsForMaintenance(state); @@ -75,19 +79,7 @@ describe("issue #17852 - daily cron jobs should not skip days", () => { const threeAM = Date.parse("2026-02-16T03:00:00.000Z"); const now = threeAM + 1_000; // 3:00:01 - const job: CronJob = { - id: "daily-job", - name: "daily 3am", - enabled: true, - schedule: { kind: "cron", expr: "0 3 * * *", tz: "UTC" }, - payload: { kind: "systemEvent", text: "daily task" }, - sessionTarget: "main", - createdAtMs: threeAM - DAY_MS, - updatedAtMs: threeAM - DAY_MS, - state: { - nextRunAtMs: threeAM, // Past-due by 1 second - }, - }; + const job = createDailyThreeAmJob(threeAM); const state = createMockState([job], now); recomputeNextRuns(state); diff --git a/src/cron/service/jobs.ts b/src/cron/service/jobs.ts index f5e96e2b4b7..0d529843e79 100644 --- a/src/cron/service/jobs.ts +++ b/src/cron/service/jobs.ts @@ -192,6 +192,27 @@ function walkSchedulableJobs( return changed; } +function recomputeJobNextRunAtMs(params: { state: CronServiceState; job: CronJob; nowMs: number }) { + let changed = false; + try { + const newNext = computeJobNextRunAtMs(params.job, params.nowMs); + if (params.job.state.nextRunAtMs !== newNext) { + params.job.state.nextRunAtMs = newNext; + changed = true; + } + // Clear schedule error count on successful computation. + if (params.job.state.scheduleErrorCount) { + params.job.state.scheduleErrorCount = undefined; + changed = true; + } + } catch (err) { + if (recordScheduleComputeError({ state: params.state, job: params.job, err })) { + changed = true; + } + } + return changed; +} + export function recomputeNextRuns(state: CronServiceState): boolean { return walkSchedulableJobs(state, ({ job, nowMs: now }) => { let changed = false; @@ -201,21 +222,8 @@ export function recomputeNextRuns(state: CronServiceState): boolean { const nextRun = job.state.nextRunAtMs; const isDueOrMissing = nextRun === undefined || now >= nextRun; if (isDueOrMissing) { - try { - const newNext = computeJobNextRunAtMs(job, now); - if (job.state.nextRunAtMs !== newNext) { - job.state.nextRunAtMs = newNext; - changed = true; - } - // Clear schedule error count on successful computation. - if (job.state.scheduleErrorCount) { - job.state.scheduleErrorCount = undefined; - changed = true; - } - } catch (err) { - if (recordScheduleComputeError({ state, job, err })) { - changed = true; - } + if (recomputeJobNextRunAtMs({ state, job, nowMs: now })) { + changed = true; } } return changed; @@ -236,21 +244,8 @@ export function recomputeNextRunsForMaintenance(state: CronServiceState): boolea // If a job was past-due but not found by findDueJobs, recomputing would // cause it to be silently skipped. if (job.state.nextRunAtMs === undefined) { - try { - const newNext = computeJobNextRunAtMs(job, now); - if (job.state.nextRunAtMs !== newNext) { - job.state.nextRunAtMs = newNext; - changed = true; - } - // Clear schedule error count on successful computation. - if (job.state.scheduleErrorCount) { - job.state.scheduleErrorCount = undefined; - changed = true; - } - } catch (err) { - if (recordScheduleComputeError({ state, job, err })) { - changed = true; - } + if (recomputeJobNextRunAtMs({ state, job, nowMs: now })) { + changed = true; } } return changed;