From d7581d1485f5fb429297490f2f402603ca01bafa Mon Sep 17 00:00:00 2001 From: vam Date: Thu, 20 Aug 2026 20:27:26 +0800 Subject: [PATCH] fix: honour the step in a cron field's bare start value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parseCronField` set `end = start` for any part that was not `*` and had no `-`, so a bare value carrying a step fell into the single-value branch and the step was dropped. `5/15` in the minute field parsed to exactly `{5}`, and `isValidAutomationCronSchedule` reported it valid, so an automation the user wrote as "every 15 minutes from :05" ran once an hour with no warning. This is not a display-only path: automation-schedule-operations.ts and automation-definition-operations.ts derive `nextRunAt` from nextAutomationOccurrenceAfter, so the dropped step changed when the automation actually fired. No cron dialect reads `5/15` as "the 5th minute of every hour". The Vixie/cronie man page documents steps only after a range or an asterisk and leaves a bare start undefined; Quartz and the cron editors people paste from read `N/step` as `N-/step`. Silently producing a third answer is the defect — the choice is to expand or to reject. Expand, because: - `*` is already the shorthand for `min-max` in this same function, so treating a bare start with a step as `start-max` is that pattern's natural extension rather than a new rule. - Rejecting would turn existing saved automations invalid, which is more destructive than correcting their cadence. Verified that `5/15` and `5-59/15` now produce identical occurrence sequences, likewise `1/2` and `1-7/2` in day-of-week, and that a bare value with no step is unchanged. NOTE: this changes the cadence of any existing automation written with `N/step` — it starts firing on the interval its author wrote instead of once per field cycle. Co-Authored-By: Claude Opus 5 (1M context) --- src/shared/automation-schedules.test.ts | 28 +++++++++++++++++++++++++ src/shared/automation-schedules.ts | 6 +++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/shared/automation-schedules.test.ts b/src/shared/automation-schedules.test.ts index 5ff4ebec73b..0a867c52e4b 100644 --- a/src/shared/automation-schedules.test.ts +++ b/src/shared/automation-schedules.test.ts @@ -200,6 +200,34 @@ describe('automation schedules', () => { expect(isValidAutomationSchedule('0 9 * * 1-7')).toBe(true) }) + it('runs a bare start value with a step through the end of its field', () => { + // Why: `5/15` is the shorthand every cron editor and Quartz read as + // "from 5, every 15" — 5,20,35,50. Dropping the step silently turned it + // into a single value, so the automation ran hourly instead. + const anchor = new Date('2026-05-01T00:00:00').getTime() + const runs: number[] = [] + let cursor = anchor + for (let index = 0; index < 4; index += 1) { + const next = nextAutomationOccurrenceAfter('5/15 * * * *', anchor, cursor) + if (next === null) { + break + } + runs.push(new Date(next).getMinutes()) + cursor = next + } + + expect(runs).toEqual([5, 20, 35, 50]) + // A bare value with no step still means exactly that value. + expect(new Date(nextAutomationOccurrenceAfter('5 * * * *', anchor, anchor)!).getMinutes()).toBe( + 5 + ) + // `1/2` in day-of-week is Mon/Wed/Fri/Sun, matching `1-7/2`. + expect(isValidAutomationSchedule('0 9 * * 1/2')).toBe(true) + expect(nextAutomationOccurrenceAfter('0 9 * * 1/2', anchor, anchor)).toBe( + nextAutomationOccurrenceAfter('0 9 * * 1-7/2', anchor, anchor) + ) + }) + it('rejects cron fields with malformed separators', () => { expect(isValidAutomationSchedule('*/15/2 9 * * *')).toBe(false) expect(isValidAutomationSchedule('0 9 1--5 * *')).toBe(false) diff --git a/src/shared/automation-schedules.ts b/src/shared/automation-schedules.ts index d8a93499287..e4331361501 100644 --- a/src/shared/automation-schedules.ts +++ b/src/shared/automation-schedules.ts @@ -150,7 +150,11 @@ function parseCronField(args: { end = parseCronNumber(endPart, args.names ?? null, args.field) } else { start = parseCronNumber(rangePart, args.names ?? null, args.field) - end = start + // Why: `5/15` is the shorthand for `5-/15` that Quartz and every cron + // editor read as "from 5, every 15". Ending at `start` kept the value and + // dropped the step, which no cron dialect does — the automation then ran + // once per field cycle instead of on the interval the user wrote. + end = stepPart === undefined ? start : args.max } const normalizedStart = args.normalize?.(start) ?? start