diff --git a/src/main/automations/schedule-drift-report.test.ts b/src/main/automations/schedule-drift-report.test.ts new file mode 100644 index 00000000000..f719998576b --- /dev/null +++ b/src/main/automations/schedule-drift-report.test.ts @@ -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() + }) +}) diff --git a/src/main/automations/schedule-drift-report.ts b/src/main/automations/schedule-drift-report.ts new file mode 100644 index 00000000000..a2e38d65ac7 --- /dev/null +++ b/src/main/automations/schedule-drift-report.ts @@ -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 +} diff --git a/src/main/automations/service.ts b/src/main/automations/service.ts index b683fb6c5a2..4be15f095db 100644 --- a/src/main/automations/service.ts +++ b/src/main/automations/service.ts @@ -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) { diff --git a/src/shared/automation-cron-dialect.test.ts b/src/shared/automation-cron-dialect.test.ts new file mode 100644 index 00000000000..dd6500b1549 --- /dev/null +++ b/src/shared/automation-cron-dialect.test.ts @@ -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[] => [...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) + }) +}) diff --git a/src/shared/automation-cron-field-parsing.ts b/src/shared/automation-cron-field-parsing.ts index b43e4d4fe5a..306b44032a3 100644 --- a/src/shared/automation-cron-field-parsing.ts +++ b/src/shared/automation-cron-field-parsing.ts @@ -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() === '*') +} diff --git a/src/shared/automation-cron-occurrence.ts b/src/shared/automation-cron-occurrence.ts index 6ad88468e16..3cb37a83034 100644 --- a/src/shared/automation-cron-occurrence.ts +++ b/src/shared/automation-cron-occurrence.ts @@ -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 } diff --git a/src/shared/automation-schedule-drift.test.ts b/src/shared/automation-schedule-drift.test.ts new file mode 100644 index 00000000000..751b411bd73 --- /dev/null +++ b/src/shared/automation-schedule-drift.test.ts @@ -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() + }) +}) diff --git a/src/shared/automation-schedule-drift.ts b/src/shared/automation-schedule-drift.ts new file mode 100644 index 00000000000..fc4f18564f9 --- /dev/null +++ b/src/shared/automation-schedule-drift.ts @@ -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) + } +} diff --git a/src/shared/automation-schedule-parsing.ts b/src/shared/automation-schedule-parsing.ts index af3ce324f1a..497809bcc0f 100644 --- a/src/shared/automation-schedule-parsing.ts +++ b/src/shared/automation-schedule-parsing.ts @@ -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) } } diff --git a/src/shared/automation-schedules.test.ts b/src/shared/automation-schedules.test.ts index 7005b7d59e0..71b1a2bf70b 100644 --- a/src/shared/automation-schedules.test.ts +++ b/src/shared/automation-schedules.test.ts @@ -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(), diff --git a/src/shared/automation-schedules.ts b/src/shared/automation-schedules.ts index 6d5a0053595..993d8fbf831 100644 --- a/src/shared/automation-schedules.ts +++ b/src/shared/automation-schedules.ts @@ -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}` } }