mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix: honour the step in a cron field's bare start value
`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-<max>/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) <noreply@anthropic.com>
This commit is contained in:
committed by
Neil
co-authored by
Claude Opus 5
parent
2b1254d681
commit
d7581d1485
@@ -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)
|
||||
|
||||
@@ -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-<max>/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
|
||||
|
||||
Reference in New Issue
Block a user