mirror of
https://github.com/stablyai/orca.git
synced 2026-09-24 16:02:41 +00:00
fix(automations): repair cron step expansion and day restriction
The semantic half of the cron repair. Both defects change what an already-saved schedule does, so they ship together and behind a decision. #15723: parseCronField set end = start for a bare numeric field even with a slash step, so 5/15 expanded to [5] and fired hourly instead of every fifteen minutes. N/step is the open-ended N-max/step sequence now. #15896: day restriction came from expanded set cardinality, so 1-31 read as unrestricted and */2 as restricted. Restriction is lexical now: a day field restricts iff no term of it ranges over a star, matching vixie cron and robfig/cron rather than crontab(5)'s prose. Verified differentially against robfig/cron v1.2.0 across 22 expressions, 424 days, zero divergences. The two cannot ship apart: 0 9 1/1 * 1 matches 124 days under the old parser, 104 under #15723 alone, and 730 under both, because the old cardinality flags react to the corrected expansion. describeAutomationScheduleDrift reads a saved expression under both semantics and reports the ones that moved, so neither direction is silent; the service names them once at startup. No expression Orca's own presets generate drifts. Fixes #15723 Fixes #15896
This commit is contained in:
@@ -0,0 +1,61 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Automation } from '../../shared/automations-types'
|
||||
import { reportAutomationScheduleDrift } from './schedule-drift-report'
|
||||
|
||||
const makeAutomation = (name: string, rrule: string): Automation => ({
|
||||
id: `id-${name}`,
|
||||
name,
|
||||
prompt: 'Check the repo',
|
||||
precheck: null,
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
executionTargetType: 'local',
|
||||
executionTargetId: 'local',
|
||||
schedulerOwner: 'local_host_service',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
baseBranch: null,
|
||||
reuseSession: false,
|
||||
timezone: 'UTC',
|
||||
rrule,
|
||||
dtstart: 0,
|
||||
enabled: true,
|
||||
nextRunAt: 0,
|
||||
missedRunPolicy: 'run_once_within_grace',
|
||||
missedRunGraceMinutes: 720,
|
||||
createdAt: 0,
|
||||
updatedAt: 0
|
||||
})
|
||||
|
||||
describe('automation schedule drift report', () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('names each affected record and which way it moved', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const count = reportAutomationScheduleDrift([
|
||||
makeAutomation('Quarter-hourly sweep', '5/15 * * * *'),
|
||||
makeAutomation('Odd days and Mondays', '0 9 */2 * 1'),
|
||||
makeAutomation('Weekday standup', '30 9 * * 1-5')
|
||||
])
|
||||
|
||||
expect(count).toBe(2)
|
||||
const lines = warn.mock.calls.map((call) => String(call[0]))
|
||||
expect(lines[0]).toContain('2 saved schedule(s) changed meaning')
|
||||
expect(
|
||||
lines.some((l) => l.includes('Quarter-hourly sweep') && l.includes('now runs more'))
|
||||
).toBe(true)
|
||||
expect(
|
||||
lines.some((l) => l.includes('Odd days and Mondays') && l.includes('now runs fewer'))
|
||||
).toBe(true)
|
||||
// The untouched preset must not be named, or the report trains the reader to skip it.
|
||||
expect(lines.some((l) => l.includes('Weekday standup'))).toBe(false)
|
||||
})
|
||||
|
||||
it('says nothing when no saved schedule drifted', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
expect(reportAutomationScheduleDrift([makeAutomation('Hourly', '0 * * * *')])).toBe(0)
|
||||
expect(warn).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,31 @@
|
||||
/**
|
||||
* Reports saved schedules whose meaning changed in the release that repaired the cron parser.
|
||||
*
|
||||
* Both repairs were correct, but a persisted cadence can now fire several times more — or
|
||||
* several times less — than it did yesterday. The louder direction announces itself through
|
||||
* spend; the quieter one does not, because nobody notices a job that stopped running. One
|
||||
* line per affected record at startup is the smallest signal that makes either detectable.
|
||||
*/
|
||||
import type { Automation } from '../../shared/automations-types'
|
||||
import { describeAutomationScheduleDrift } from '../../shared/automation-schedule-drift'
|
||||
|
||||
export function reportAutomationScheduleDrift(automations: readonly Automation[]): number {
|
||||
const drifted = automations.flatMap((automation) => {
|
||||
const drift = describeAutomationScheduleDrift(automation.rrule)
|
||||
return drift ? [{ automation, drift }] : []
|
||||
})
|
||||
if (drifted.length === 0) {
|
||||
return 0
|
||||
}
|
||||
console.warn(
|
||||
`[automations] ${drifted.length} saved schedule(s) changed meaning when the cron parser was repaired; review them:`
|
||||
)
|
||||
for (const { automation, drift } of drifted) {
|
||||
const direction = drift.currentRunsPerYear > drift.previousRunsPerYear ? 'more' : 'fewer'
|
||||
console.warn(
|
||||
`[automations] "${automation.name}" (${automation.id}) "${drift.expression}" now runs ` +
|
||||
`${direction}: about ${drift.currentRunsPerYear}/year, was about ${drift.previousRunsPerYear}/year`
|
||||
)
|
||||
}
|
||||
return drifted.length
|
||||
}
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
type AutomationRunTerminalObserver
|
||||
} from './run-completion-watcher'
|
||||
import { createAutomationRunWriter, type AutomationRunWriter } from './automation-run-writer'
|
||||
import { reportAutomationScheduleDrift } from './schedule-drift-report'
|
||||
import {
|
||||
describeScheduledRefusal,
|
||||
recordRefusedAutomationRun,
|
||||
@@ -115,6 +116,7 @@ export class AutomationService {
|
||||
void this.evaluateDueRuns()
|
||||
}, this.tickMs)
|
||||
this.completionWatcher?.reconcileRetainedRuns(this.store.listAutomationRuns())
|
||||
reportAutomationScheduleDrift(this.store.listAutomations())
|
||||
// Why: headless serve never gets a renderer-ready IPC, but due runs still
|
||||
// need the same startup catch-up pass desktop gets after renderer attach.
|
||||
if (this.rendererReady || this.headlessDispatcher) {
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { cronMatches } from './automation-cron-occurrence'
|
||||
import { parseCronExpression } from './automation-schedule-parsing'
|
||||
|
||||
const ascending = (values: Set<number>): number[] => [...values].sort((left, right) => left - right)
|
||||
|
||||
const EVERY_DAY_OF_MAY = Array.from({ length: 31 }, (_, index) => index + 1)
|
||||
|
||||
/**
|
||||
* Independent calendar oracle. May 2026 opens on a Friday, so its Mondays are 4/11/18/25,
|
||||
* its Sundays 3/10/17/24/31, and its weekend days 2/3, 9/10, 16/17, 23/24 and 30/31. Every
|
||||
* expectation below is that hand calendar, never a second call into the parser.
|
||||
*/
|
||||
function matchingDaysOfMay2026(expression: string): number[] {
|
||||
const rule = parseCronExpression(expression)
|
||||
const days: number[] = []
|
||||
for (let day = 1; day <= 31; day += 1) {
|
||||
if (cronMatches(rule, new Date(2026, 4, day, 9, 0, 0, 0).getTime())) {
|
||||
days.push(day)
|
||||
}
|
||||
}
|
||||
return days
|
||||
}
|
||||
|
||||
describe('cron bare stepped values (#15723)', () => {
|
||||
it('expands `N/step` as the open-ended `N-max/step` sequence in every field', () => {
|
||||
expect(ascending(parseCronExpression('5/15 * * * *').minutes)).toEqual([5, 20, 35, 50])
|
||||
expect(ascending(parseCronExpression('* 2/7 * * *').hours)).toEqual([2, 9, 16, 23])
|
||||
expect(ascending(parseCronExpression('0 9 5/10 * *').daysOfMonth)).toEqual([5, 15, 25])
|
||||
expect(ascending(parseCronExpression('0 9 * MAR/3 *').months)).toEqual([3, 6, 9, 12])
|
||||
// 1, 4 and 7, with Sunday normalized off 7.
|
||||
expect(ascending(parseCronExpression('0 9 * * 1/3').daysOfWeek)).toEqual([0, 1, 4])
|
||||
})
|
||||
|
||||
it('matches the explicit `N-max/step` range it is defined to mean', () => {
|
||||
const equivalents: [string, string][] = [
|
||||
['5/15 * * * *', '5-59/15 * * * *'],
|
||||
['* 2/7 * * *', '* 2-23/7 * * *'],
|
||||
['0 9 5/10 * *', '0 9 5-31/10 * *'],
|
||||
['0 9 * MAR/3 *', '0 9 * MAR-DEC/3 *'],
|
||||
['0 9 * * 1/3', '0 9 * * 1-7/3']
|
||||
]
|
||||
for (const [bare, explicit] of equivalents) {
|
||||
const left = parseCronExpression(bare)
|
||||
const right = parseCronExpression(explicit)
|
||||
expect([
|
||||
ascending(left.minutes),
|
||||
ascending(left.hours),
|
||||
ascending(left.daysOfMonth),
|
||||
ascending(left.months),
|
||||
ascending(left.daysOfWeek)
|
||||
]).toEqual([
|
||||
ascending(right.minutes),
|
||||
ascending(right.hours),
|
||||
ascending(right.daysOfMonth),
|
||||
ascending(right.months),
|
||||
ascending(right.daysOfWeek)
|
||||
])
|
||||
}
|
||||
})
|
||||
|
||||
it('leaves a bare value with no step as itself', () => {
|
||||
expect(ascending(parseCronExpression('5 * * * *').minutes)).toEqual([5])
|
||||
expect(ascending(parseCronExpression('0 9 5 * *').daysOfMonth)).toEqual([5])
|
||||
expect(ascending(parseCronExpression('0 9 * MAR *').months)).toEqual([3])
|
||||
expect(ascending(parseCronExpression('0 9 * * FRI').daysOfWeek)).toEqual([5])
|
||||
})
|
||||
|
||||
// Separateness probe: the #15723 repair only reaches the bare-value branch, so it moves
|
||||
// neither an oversized step nor the day-restriction flags (#15896).
|
||||
it('leaves oversized-step and full-range-day expansions exactly where they were', () => {
|
||||
expect(ascending(parseCronExpression('*/90 * * * *').minutes)).toEqual([0])
|
||||
expect(ascending(parseCronExpression('5/90 * * * *').minutes)).toEqual([5])
|
||||
expect(ascending(parseCronExpression('0 9 1-31 * 1').daysOfMonth)).toEqual(EVERY_DAY_OF_MAY)
|
||||
})
|
||||
})
|
||||
|
||||
describe('cron day restriction (#15896)', () => {
|
||||
// Dialect: a day field is restricted iff no term of it ranges over a star; when both day
|
||||
// fields are restricted the day matches on either, otherwise on both. Every expectation
|
||||
// below was taken from robfig/cron v1.2.0, an independent implementation of the same rule.
|
||||
it('ORs an explicit full day-of-month range against a restricted day-of-week', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 1-31 * 1')).toEqual(EVERY_DAY_OF_MAY)
|
||||
})
|
||||
|
||||
it('ANDs a wildcard day-of-month against a restricted day-of-week', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 * * 1')).toEqual([4, 11, 18, 25])
|
||||
})
|
||||
|
||||
it('ORs two partially restricted day fields', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 1,15 * 1')).toEqual([1, 4, 11, 15, 18, 25])
|
||||
})
|
||||
|
||||
it('ANDs a restricted day-of-month against a wildcard day-of-week', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 1,15 * *')).toEqual([1, 15])
|
||||
})
|
||||
|
||||
// A star step is still a star, so it does not flip the day rule to OR. Reading `*/2` as
|
||||
// restricted would fire this ~8x more: the 18 odd-or-Monday days, not the 2 that are both.
|
||||
it('keeps AND when a day field steps over a star', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 */2 * 1')).toEqual([11, 25])
|
||||
expect(matchingDaysOfMay2026('0 9 */1 * 1')).toEqual([4, 11, 18, 25])
|
||||
expect(matchingDaysOfMay2026('0 9 * * */2')).toEqual([
|
||||
2, 3, 5, 7, 9, 10, 12, 14, 16, 17, 19, 21, 23, 24, 26, 28, 30, 31
|
||||
])
|
||||
})
|
||||
|
||||
// The star test is per comma term, so a list that reaches a star anywhere is unrestricted.
|
||||
it('treats a day list containing a star term as a star', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 */3 * 1,5')).toEqual([1, 4, 22, 25])
|
||||
})
|
||||
|
||||
it('normalizes Sunday from 0, from 7 and from the name', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 * * 0')).toEqual([3, 10, 17, 24, 31])
|
||||
expect(matchingDaysOfMay2026('0 9 * * 7')).toEqual([3, 10, 17, 24, 31])
|
||||
expect(matchingDaysOfMay2026('0 9 * * SUN')).toEqual([3, 10, 17, 24, 31])
|
||||
})
|
||||
|
||||
it('reads month and day names on both sides of the restriction rule', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 * MAY MON')).toEqual([4, 11, 18, 25])
|
||||
expect(matchingDaysOfMay2026('0 9 * JUN MON')).toEqual([])
|
||||
expect(matchingDaysOfMay2026('0 9 1-31 MAY MON')).toEqual(EVERY_DAY_OF_MAY)
|
||||
})
|
||||
|
||||
// Preset-built schedules always write a literal `*` day-of-month, so they keep AND.
|
||||
it('leaves preset-shaped schedules on AND semantics', () => {
|
||||
expect(matchingDaysOfMay2026('0 9 * * 1-5')).toEqual([
|
||||
1, 4, 5, 6, 7, 8, 11, 12, 13, 14, 15, 18, 19, 20, 21, 22, 25, 26, 27, 28, 29
|
||||
])
|
||||
expect(matchingDaysOfMay2026('0 9 * * *')).toEqual(EVERY_DAY_OF_MAY)
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,10 @@
|
||||
// Cron field parsing for Orca's automation schedules.
|
||||
// A field step is bounded by the count of distinct values the field holds: a step of 90 on
|
||||
// minutes is one value at :00, never "every 90 minutes", so it is refused as input (#15895).
|
||||
// Orca's cron dialect (vixie/POSIX):
|
||||
// - `N/step` is the open-ended sequence `N-max/step`; a bare `N` is only itself (#15723).
|
||||
// - A day field is restricted iff no term of it ranges over a star, so `1-31` restricts but
|
||||
// `*/2` does not (#15896). Restriction is lexical: the expanded set cannot tell `1-31` from
|
||||
// `*`. When both day fields are restricted the day matches on either; otherwise on both.
|
||||
// - A field step is bounded by the count of distinct values the field holds; a step of 90 on
|
||||
// minutes is one value at :00, never "every 90 minutes" (#15895).
|
||||
export type CronParseOptions = {
|
||||
/** Input-time gate: reject a step wider than the field's domain instead of silently
|
||||
* degenerating to a single value. Off for persisted rows, which must keep running the
|
||||
@@ -104,7 +108,8 @@ export function parseCronField(args: {
|
||||
end = parseCronNumber(endPart, args.names ?? null, args.field)
|
||||
} else {
|
||||
start = parseCronNumber(rangePart, args.names ?? null, args.field)
|
||||
end = start
|
||||
// `N/step` is the open-ended `N-max/step` sequence; a bare `N` is only itself.
|
||||
end = stepPart === undefined ? start : args.max
|
||||
}
|
||||
|
||||
const normalizedStart = args.normalize?.(start) ?? start
|
||||
@@ -131,3 +136,12 @@ export function parseCronField(args: {
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// A day field restricts iff none of its terms ranges over a star, matching what vixie cron
|
||||
// and robfig/cron both do. crontab(5) says "restricted (ie, are not *)", which reads as a
|
||||
// literal-`*` test, but vixie's own entry.c sets DOM_STAR/DOW_STAR off the field's leading
|
||||
// character, so `*/2` is a star there too; we follow the implementations over the prose,
|
||||
// because reading `*/2` as restricted flips its day rule to OR and fires it ~8x more.
|
||||
export function isCronDayFieldRestricted(field: string): boolean {
|
||||
return !field.split(',').some((term) => term.split('/')[0].trim() === '*')
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ export function cronDateMatches(rule: ParsedCron, timestamp: number): boolean {
|
||||
}
|
||||
const dayOfMonthMatches = rule.daysOfMonth.has(date.getDate())
|
||||
const dayOfWeekMatches = rule.daysOfWeek.has(date.getDay())
|
||||
// Dialect rule; the flags are lexical (`isCronDayFieldRestricted`), not set sizes.
|
||||
if (rule.dayOfMonthRestricted && rule.dayOfWeekRestricted) {
|
||||
return dayOfMonthMatches || dayOfWeekMatches
|
||||
}
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { describeAutomationScheduleDrift } from './automation-schedule-drift'
|
||||
|
||||
// Both lists were recorded by running the same corpus through the real parent build
|
||||
// (729491597f3) and this branch, not by re-deriving them from the detector under test.
|
||||
const DRIFTED = [
|
||||
'5/15 * * * *',
|
||||
'0/30 * * * *',
|
||||
'5/15 9 * * *',
|
||||
'0 9/4 * * *',
|
||||
'0 9 1/7 * *',
|
||||
'0 9 * 1/3 *',
|
||||
'0 9 * * 1/2',
|
||||
'0 9 1-31 * 1',
|
||||
'0 9 */2 * 1',
|
||||
'0 9 */3 * 1',
|
||||
'0 9 * MAR/3 *'
|
||||
]
|
||||
|
||||
const STABLE = [
|
||||
'0 * * * *',
|
||||
'30 9 * * *',
|
||||
'30 9 * * 1-5',
|
||||
'30 9 * * 3',
|
||||
'*/15 * * * *',
|
||||
'*/5 * * * *',
|
||||
'5 * * * *',
|
||||
'0 9 */1 * 1',
|
||||
'0 9 1,15 * 1',
|
||||
'0 9 * * 0-6',
|
||||
'0 9 * * 0-7',
|
||||
'0 9 * * 1-7',
|
||||
'0 9 */2 * *',
|
||||
'0 9 * * */2',
|
||||
'0 9 1-31 * *',
|
||||
'0 9 * * *',
|
||||
'0 9 15 * *',
|
||||
'0 9 1-15 * 1',
|
||||
'*/90 * * * *',
|
||||
'5/90 * * * *',
|
||||
'0 9 * * */8',
|
||||
'0 9 * MAY MON',
|
||||
'0 9 * * FRI'
|
||||
]
|
||||
|
||||
const ANCHOR = new Date(2026, 0, 1).getTime()
|
||||
|
||||
describe('automation schedule drift', () => {
|
||||
it('flags every schedule the repair changed', () => {
|
||||
for (const expression of DRIFTED) {
|
||||
expect(describeAutomationScheduleDrift(expression, ANCHOR), expression).not.toBeNull()
|
||||
}
|
||||
})
|
||||
|
||||
// The restriction flags move on several of these while the days they fire do not; reporting
|
||||
// those would train the reader to ignore the notice.
|
||||
it('stays silent on schedules the repair left alone', () => {
|
||||
for (const expression of STABLE) {
|
||||
expect(describeAutomationScheduleDrift(expression, ANCHOR), expression).toBeNull()
|
||||
}
|
||||
})
|
||||
|
||||
it('reports the direction and size of the change', () => {
|
||||
// 1x/hour -> 4x/hour: the cadence users will feel as spend.
|
||||
expect(describeAutomationScheduleDrift('5/15 * * * *', ANCHOR)).toEqual({
|
||||
expression: '5/15 * * * *',
|
||||
previousRunsPerYear: 8760,
|
||||
currentRunsPerYear: 35040
|
||||
})
|
||||
// The quiet direction: an automation that now skips most of the days it used to run.
|
||||
const fewer = describeAutomationScheduleDrift('0 9 */2 * 1', ANCHOR)
|
||||
expect(fewer!.currentRunsPerYear).toBeLessThan(fewer!.previousRunsPerYear / 4)
|
||||
})
|
||||
|
||||
it('ignores RRULE presets, which never used the repaired parser', () => {
|
||||
expect(describeAutomationScheduleDrift('FREQ=DAILY;BYHOUR=9;BYMINUTE=0', ANCHOR)).toBeNull()
|
||||
})
|
||||
|
||||
it('reports nothing for a schedule that cannot be read at all', () => {
|
||||
expect(describeAutomationScheduleDrift('0 9 32 * *', ANCHOR)).toBeNull()
|
||||
expect(describeAutomationScheduleDrift('not a cron', ANCHOR)).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,123 @@
|
||||
// Detects saved cron schedules whose meaning changed in the release that repaired the parser
|
||||
// (#15723, #15896). Both repairs were correct, but a persisted cadence can now fire several
|
||||
// times more — or several times less — than it did yesterday, with nothing to notice it by.
|
||||
import {
|
||||
getAutomationCronExpressionFields,
|
||||
parseCronExpression,
|
||||
type ParsedCron
|
||||
} from './automation-schedule-parsing'
|
||||
import { cronDateMatches } from './automation-cron-occurrence'
|
||||
|
||||
// Two years covers every day-of-month against day-of-week pairing a schedule can land on,
|
||||
// which is the only part of matching that depends on the calendar rather than the sets.
|
||||
const DRIFT_SCAN_DAYS = 730
|
||||
|
||||
export type AutomationScheduleDrift = {
|
||||
expression: string
|
||||
/** Runs a year under the cadence as it was read before the repair, and as it reads now. */
|
||||
previousRunsPerYear: number
|
||||
currentRunsPerYear: number
|
||||
}
|
||||
|
||||
/**
|
||||
* The pre-repair reading of a field: a bare value carrying a step lost the step, so `5/15`
|
||||
* meant `5`. A star or a range kept its step, and is left alone.
|
||||
*/
|
||||
function toPreRepairField(field: string): string {
|
||||
return field
|
||||
.split(',')
|
||||
.map((term) => {
|
||||
const [range, step] = term.split('/')
|
||||
if (step === undefined || range.includes('*') || range.includes('-')) {
|
||||
return term
|
||||
}
|
||||
return range
|
||||
})
|
||||
.join(',')
|
||||
}
|
||||
|
||||
/**
|
||||
* The pre-repair reading of a whole expression. Day restriction came from how many values a
|
||||
* field expanded to rather than from what the user wrote, so `1-31` read as unrestricted.
|
||||
*/
|
||||
function parsePreRepairCron(expression: string): ParsedCron {
|
||||
const fields = getAutomationCronExpressionFields(expression, 6)
|
||||
const parsed = parseCronExpression(fields.map(toPreRepairField).join(' '))
|
||||
return {
|
||||
...parsed,
|
||||
dayOfMonthRestricted: parsed.daysOfMonth.size !== 31,
|
||||
dayOfWeekRestricted: parsed.daysOfWeek.size !== 7
|
||||
}
|
||||
}
|
||||
|
||||
/** Walks both readings over the same calendar so the comparison is which days, not how many. */
|
||||
function compareMatchingDays(
|
||||
previous: ParsedCron,
|
||||
current: ParsedCron,
|
||||
anchor: number
|
||||
): { previousDays: number; currentDays: number; sameDays: boolean } {
|
||||
const cursor = new Date(anchor)
|
||||
cursor.setHours(12, 0, 0, 0)
|
||||
let previousDays = 0
|
||||
let currentDays = 0
|
||||
let sameDays = true
|
||||
for (let i = 0; i < DRIFT_SCAN_DAYS; i += 1) {
|
||||
const at = cursor.getTime()
|
||||
const previousMatch = cronDateMatches(previous, at)
|
||||
const currentMatch = cronDateMatches(current, at)
|
||||
if (previousMatch) {
|
||||
previousDays += 1
|
||||
}
|
||||
if (currentMatch) {
|
||||
currentDays += 1
|
||||
}
|
||||
if (previousMatch !== currentMatch) {
|
||||
sameDays = false
|
||||
}
|
||||
cursor.setDate(cursor.getDate() + 1)
|
||||
}
|
||||
return { previousDays, currentDays, sameDays }
|
||||
}
|
||||
|
||||
function runsPerYear(rule: ParsedCron, days: number): number {
|
||||
return Math.round((days / 2) * rule.hours.size * rule.minutes.size)
|
||||
}
|
||||
|
||||
/**
|
||||
* Null when the saved cadence still means what it did before the repair. Only cron schedules
|
||||
* can drift; RRULE presets never went through the repaired field parser.
|
||||
*/
|
||||
export function describeAutomationScheduleDrift(
|
||||
schedule: string,
|
||||
anchor = Date.now()
|
||||
): AutomationScheduleDrift | null {
|
||||
const expression = schedule.trim()
|
||||
if (expression.includes('=')) {
|
||||
return null
|
||||
}
|
||||
let current: ParsedCron
|
||||
let previous: ParsedCron
|
||||
try {
|
||||
current = parseCronExpression(expression)
|
||||
previous = parsePreRepairCron(expression)
|
||||
} catch {
|
||||
// An unreadable schedule drifts nowhere; the tick reports it separately (#16303).
|
||||
return null
|
||||
}
|
||||
const sameClock =
|
||||
previous.minutes.size === current.minutes.size &&
|
||||
previous.hours.size === current.hours.size &&
|
||||
[...current.minutes].every((minute) => previous.minutes.has(minute)) &&
|
||||
[...current.hours].every((hour) => previous.hours.has(hour))
|
||||
const { previousDays, currentDays, sameDays } = compareMatchingDays(previous, current, anchor)
|
||||
// Compare what the schedule fires, not how it parsed: the restriction flags move on
|
||||
// expressions whose matched days do not, and those are not worth telling anyone about.
|
||||
if (sameClock && sameDays) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
expression,
|
||||
previousRunsPerYear: runsPerYear(previous, previousDays),
|
||||
currentRunsPerYear: runsPerYear(current, currentDays)
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import { isClipboardTextByteLengthOverLimit } from './clipboard-text'
|
||||
import {
|
||||
DAY_NAMES,
|
||||
MONTH_NAMES,
|
||||
isCronDayFieldRestricted,
|
||||
parseCronField,
|
||||
type CronParseOptions
|
||||
} from './automation-cron-field-parsing'
|
||||
@@ -76,23 +77,6 @@ export function parseCronExpression(
|
||||
}
|
||||
const [minute, hour, dayOfMonth, month, dayOfWeek] = parts
|
||||
const rejectOversizedStep = options.rejectOversizedStep ?? false
|
||||
const daysOfMonth = parseCronField({
|
||||
value: dayOfMonth,
|
||||
min: 1,
|
||||
max: 31,
|
||||
field: 'day of month',
|
||||
rejectOversizedStep
|
||||
})
|
||||
const daysOfWeek = parseCronField({
|
||||
value: dayOfWeek,
|
||||
min: 0,
|
||||
max: 7,
|
||||
field: 'day of week',
|
||||
names: DAY_NAMES,
|
||||
normalize: (value) => (value === 7 ? 0 : value),
|
||||
distinctValueCount: 7,
|
||||
rejectOversizedStep
|
||||
})
|
||||
return {
|
||||
kind: 'cron',
|
||||
minutes: parseCronField({
|
||||
@@ -103,7 +87,13 @@ export function parseCronExpression(
|
||||
rejectOversizedStep
|
||||
}),
|
||||
hours: parseCronField({ value: hour, min: 0, max: 23, field: 'hour', rejectOversizedStep }),
|
||||
daysOfMonth,
|
||||
daysOfMonth: parseCronField({
|
||||
value: dayOfMonth,
|
||||
min: 1,
|
||||
max: 31,
|
||||
field: 'day of month',
|
||||
rejectOversizedStep
|
||||
}),
|
||||
months: parseCronField({
|
||||
value: month,
|
||||
min: 1,
|
||||
@@ -112,9 +102,18 @@ export function parseCronExpression(
|
||||
names: MONTH_NAMES,
|
||||
rejectOversizedStep
|
||||
}),
|
||||
daysOfWeek,
|
||||
dayOfMonthRestricted: daysOfMonth.size !== 31,
|
||||
dayOfWeekRestricted: daysOfWeek.size !== 7
|
||||
daysOfWeek: parseCronField({
|
||||
value: dayOfWeek,
|
||||
min: 0,
|
||||
max: 7,
|
||||
field: 'day of week',
|
||||
names: DAY_NAMES,
|
||||
normalize: (value) => (value === 7 ? 0 : value),
|
||||
distinctValueCount: 7,
|
||||
rejectOversizedStep
|
||||
}),
|
||||
dayOfMonthRestricted: isCronDayFieldRestricted(dayOfMonth),
|
||||
dayOfWeekRestricted: isCronDayFieldRestricted(dayOfWeek)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -242,7 +242,9 @@ describe('automation schedules', () => {
|
||||
expect(formatAutomationSchedule('0 9,17 * * MON-FRI')).toBe('Custom schedule')
|
||||
})
|
||||
|
||||
it('treats all-value cron day fields as unrestricted for DOM/DOW matching', () => {
|
||||
// Restriction is lexical (#15896), but a star step is still a star: `*/1` does not
|
||||
// restrict, so the day rule stays AND and this fires on Mondays only.
|
||||
it('treats a stepped cron day-of-month field as unrestricted for DOM/DOW matching', () => {
|
||||
const next = nextAutomationOccurrenceAfter(
|
||||
'0 9 */1 * MON',
|
||||
new Date('2026-05-01T00:00:00').getTime(),
|
||||
|
||||
@@ -94,27 +94,29 @@ function classifyParsedCronSchedule(rule: ParsedCron): AutomationCronScheduleCla
|
||||
}
|
||||
const minute = getSingleSetValue(rule.minutes)
|
||||
const hour = getSingleSetValue(rule.hours)
|
||||
const unrestrictedDayOfMonth = !rule.dayOfMonthRestricted
|
||||
const unrestrictedMonth = setContainsRange(rule.months, 1, 12)
|
||||
const unrestrictedDayOfWeek = !rule.dayOfWeekRestricted
|
||||
const unrestrictedCalendar = unrestrictedDayOfMonth && unrestrictedMonth
|
||||
if (
|
||||
minute !== null &&
|
||||
setContainsRange(rule.hours, 0, 23) &&
|
||||
unrestrictedCalendar &&
|
||||
unrestrictedDayOfWeek
|
||||
) {
|
||||
const everyDayOfMonth = setContainsRange(rule.daysOfMonth, 1, 31)
|
||||
const everyDayOfWeek = setContainsRange(rule.daysOfWeek, 0, 6)
|
||||
// Labels describe the days the rule actually fires on, so they need coverage of the
|
||||
// matched set, not the lexical restriction flags that pick OR over AND.
|
||||
const matchesEitherDayField = rule.dayOfMonthRestricted && rule.dayOfWeekRestricted
|
||||
const everyDay = matchesEitherDayField
|
||||
? everyDayOfMonth || everyDayOfWeek
|
||||
: everyDayOfMonth && everyDayOfWeek
|
||||
const unrestrictedCalendar = everyDayOfMonth && unrestrictedMonth
|
||||
if (minute !== null && setContainsRange(rule.hours, 0, 23) && unrestrictedMonth && everyDay) {
|
||||
return {
|
||||
kind: 'hourly',
|
||||
minute,
|
||||
label: `Hourly at :${String(minute).padStart(2, '0')}`
|
||||
}
|
||||
}
|
||||
if (minute !== null && hour !== null && unrestrictedCalendar) {
|
||||
if (minute !== null && hour !== null && unrestrictedMonth && everyDay) {
|
||||
return { kind: 'daily', hour, minute, label: `Daily at ${formatTime(hour, minute)}` }
|
||||
}
|
||||
// Weekday/weekly names only read true under AND; under OR the day-of-month half fires too.
|
||||
if (minute !== null && hour !== null && unrestrictedCalendar && !matchesEitherDayField) {
|
||||
const time = formatTime(hour, minute)
|
||||
if (unrestrictedDayOfWeek) {
|
||||
return { kind: 'daily', hour, minute, label: `Daily at ${time}` }
|
||||
}
|
||||
if (setContainsExactly(rule.daysOfWeek, [1, 2, 3, 4, 5])) {
|
||||
return { kind: 'weekdays', hour, minute, label: `Weekdays at ${time}` }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user