fix(Schedule Node): Run non-dividing minute intervals by elapsed time (#35062)

Co-authored-by: Claude Fable 5 <[email protected]>

packages/cli/src/scheduling/schedule-trigger-node/__tests__/schedule-trigger-job-registrar.deviations.test.ts

@@ -258,4 +258,46 @@ describe('ScheduleTriggerJobRegistrar deviations', () => {
 			).toThrow();
 		});
 	});
+
+	describe('D6: a non-dividing minutes cadence is an interval job in both modes', () => {
+		// every 50 minutes (50 does not divide 60): the node compiles an
+		// every-minute cron gated by an elapsed-time recurrence, which
+		// recurring_cron can't express (no minutes unit).
+		const cron: Cron = {
+			expression: '7 * * * * *' as CronExpression,
+			recurrence: { activated: true, index: 0, intervalSize: 50, typeInterval: 'minutes' },
+			source: { field: 'minutes', size: 50 },
+		};
+
+		it('persists as a steady 50-minute interval in `legacy` and `new` mode', async () => {
+			const legacy = await collect('legacy', cron);
+			const fresh = await collect('new', cron);
+
+			for (const row of [legacy, fresh]) {
+				expect(row.kind).toBe('interval');
+				expect(row.intervalSeconds).toBe(50 * 60);
+			}
+
+			// legacy seeds the first fire from the cron (the in-memory engine fires
+			// ungated at its next minute tick); new anchors to activation + 50 min
+			expect(legacy.nextRunAt).toEqual(new Date('2026-01-05T12:01:07.000Z'));
+			expect(fresh.nextRunAt).toEqual(new Date('2026-01-05T12:50:07.000Z'));
+
+			// steady cadence after the seed: every gap is exactly 50 minutes
+			expect(gaps([legacy.nextRunAt!, ...fires(scheduleOf(legacy), legacy.nextRunAt!, 3)])).toEqual(
+				Array(3).fill(50 * 60 * 1000),
+			);
+		});
+
+		it('a dividing minutes cadence stays a clock-aligned cron in `legacy` mode', async () => {
+			const dividing: Cron = {
+				expression: '7 */30 * * * *' as CronExpression,
+				recurrence: { activated: false },
+				source: { field: 'minutes', size: 30 },
+			};
+			const legacy = await collect('legacy', dividing);
+			expect(legacy.kind).toBe('cron');
+			expect(legacy.cronExpression).toBe('7 */30 * * * *');
+		});
+	});
 });

packages/cli/src/scheduling/schedule-trigger-node/schedule-trigger-job-registrar.ts

@@ -181,9 +181,16 @@ export class ScheduleTriggerJobRegistrar {
 							);
 							collected.push({ schedule, firstRunAt: null });
 						} else {
+							// Legacy interval jobs (gated minutes) first fire at their next
+							// cron tick, not activation + interval — seed from the cron.
+							const seedSchedule: Schedule =
+								this.triggerNodeMode === 'legacy' && schedule.kind === 'interval'
+									? { kind: 'cron', cronExpression: expression, timezone }
+									: schedule;
+
 							// Validates the expression/timezone and returns the first instant.
 							const computed = computeFirstRunAt(
-								withResolvedTimezone(schedule, this.defaultTimezone),
+								withResolvedTimezone(seedSchedule, this.defaultTimezone),
 								new Date(),
 							);
 
@@ -213,7 +220,10 @@ export class ScheduleTriggerJobRegistrar {
 	 *
 	 * - A second/minute cadence becomes an `interval` job in `new` mode (a steady
 	 *   elapsed-time cadence); in `legacy` mode it stays the node's plain cron so
-	 *   fires remain clock-aligned.
+	 *   fires remain clock-aligned. Exception: a minutes cadence that does not
+	 *   divide 60 (e.g. every 50 min) is an `interval` job in both modes — the
+	 *   legacy engine also runs it by elapsed time (every-minute cron gated by
+	 *   `recurrenceCheck`), and `recurring_cron` can't express a minutes unit.
 	 * - "Every N days/weeks/months" with N >= 2 becomes a `recurring_cron` job:
 	 *   the cron expression names the candidate instants and the job fires on
 	 *   every Nth of them.
@@ -232,8 +242,9 @@ export class ScheduleTriggerJobRegistrar {
 		recurrence: Cron['recurrence'],
 		source: Cron['source'],
 	): Schedule {
+		const isGatedMinutes = source?.field === 'minutes' && recurrence?.activated === true;
 		if (
-			this.triggerNodeMode === 'new' &&
+			(this.triggerNodeMode === 'new' || isGatedMinutes) &&
 			source?.size !== undefined &&
 			(source.field === 'seconds' || source.field === 'minutes')
 		) {
@@ -247,7 +258,11 @@ export class ScheduleTriggerJobRegistrar {
 		// engine rejects a recurrenceSize of 1 to keep one representation per
 		// rule). N = 0/NaN never fires (see isDegenerateRecurrence) and a negative
 		// N fires on every instant in the legacy engine; both are plain crons here.
-		if (recurrence?.activated && recurrence.intervalSize >= 2) {
+		if (
+			recurrence?.activated &&
+			recurrence.typeInterval !== 'minutes' &&
+			recurrence.intervalSize >= 2
+		) {
 			return {
 				kind: 'recurring_cron',
 				cronExpression: expression,

packages/nodes-base/nodes/Schedule/GenericFunctions.ts

@@ -54,7 +54,15 @@ export function recurrenceCheck(
 	const lastExecution = recurrenceRules[index] ?? undefined;
 
 	const momentTz = moment.tz(timezone);
-	if (typeInterval === 'hours') {
+	if (typeInterval === 'minutes') {
+		// Absolute minute count (not the 0-59 wall-clock value) so a gap longer
+		// than the hour wrap can't delay the next fire by another interval.
+		const absoluteMinute = Math.floor(momentTz.valueOf() / 60_000);
+		if (lastExecution === undefined || absoluteMinute - lastExecution >= intervalSize) {
+			recurrenceRules[index] = absoluteMinute;
+			return true;
+		}
+	} else if (typeInterval === 'hours') {
 		const hour = momentTz.hour();
 		if (lastExecution === undefined || (hour - lastExecution + 24) % 24 >= intervalSize) {
 			recurrenceRules[index] = hour;
@@ -116,7 +124,13 @@ export const toCronExpression = (interval: ScheduleInterval, nodeKey: string): C
 	if (interval.field === 'seconds') return `*/${interval.secondsInterval} * * * * *`;
 
 	const second = stableInt(nodeKey, 'second', 0, 60);
-	if (interval.field === 'minutes') return `${second} */${interval.minutesInterval} * * * *`;
+	if (interval.field === 'minutes') {
+		const minutes = interval.minutesInterval;
+		if (60 % minutes === 0) return `${second} */${minutes} * * * *`;
+		// Non-dividing `*/n` fires unevenly (e.g. */50 at :00 and :50); fire every
+		// minute instead and let recurrenceCheck enforce elapsed time.
+		return `${second} * * * * *`;
+	}
 
 	const minute = interval.triggerAtMinute ?? stableInt(nodeKey, 'minute', 0, 60);
 	if (interval.field === 'hours') {
@@ -175,6 +189,18 @@ export const toCronSource = (interval: ScheduleInterval): CronSource => {
 export function intervalToRecurrence(interval: ScheduleInterval, index: number) {
 	let recurrence: IRecurrenceRule = { activated: false };
 
+	if (interval.field === 'minutes') {
+		const { minutesInterval } = interval;
+		if (minutesInterval > 0 && 60 % minutesInterval !== 0) {
+			recurrence = {
+				activated: true,
+				index,
+				intervalSize: minutesInterval,
+				typeInterval: 'minutes',
+			};
+		}
+	}
+
 	if (interval.field === 'hours') {
 		const { hoursInterval } = interval;
 		if (hoursInterval !== 1) {
@@ -234,10 +260,14 @@ export function intervalToRecurrence(interval: ScheduleInterval, index: number)
  * pre-signature value is the old 0-11 encoding and must be discarded.
  */
 function isRecurrenceValueValidForType(
-	typeInterval: 'hours' | 'days' | 'weeks' | 'months',
+	typeInterval: 'minutes' | 'hours' | 'days' | 'weeks' | 'months',
 	value: number,
 ): boolean {
 	switch (typeInterval) {
+		case 'minutes':
+			// Minutes recurrence is newer than signatures, so a signature-less value
+			// was never written by a minutes rule — it belongs to some earlier config.
+			return false;
 		case 'hours':
 			return value >= 0 && value <= 23;
 		case 'days':

packages/nodes-base/nodes/Schedule/SchedulerInterface.ts

@@ -6,7 +6,7 @@ export type IRecurrenceRule =
 			activated: true;
 			index: number;
 			intervalSize: number;
-			typeInterval: 'hours' | 'days' | 'weeks' | 'months';
+			typeInterval: 'minutes' | 'hours' | 'days' | 'weeks' | 'months';
 	  };
 
 export type ScheduleInterval =

packages/nodes-base/nodes/Schedule/test/GenericFunctions.test.ts

@@ -15,6 +15,7 @@ vi.mock('moment-timezone');
 const mockedMoment = vi.mocked(moment);
 
 function mockMomentTz(values: {
+	epochMinute?: number;
 	hour?: number;
 	dayOfYear?: number;
 	week?: number;
@@ -22,6 +23,7 @@ function mockMomentTz(values: {
 	year?: number;
 }) {
 	const tzObj = {
+		valueOf: () => (values.epochMinute ?? 0) * 60_000,
 		hour: () => values.hour ?? 0,
 		dayOfYear: () => values.dayOfYear ?? 1,
 		week: () => values.week ?? 1,
@@ -75,6 +77,16 @@ describe('toCronExpression', () => {
 			TEST_SEED,
 		);
 		expect(result).toEqual('56 */30 * * * *');
+
+		// 50 does not divide 60 — fire every minute, gated by recurrenceCheck.
+		const result1 = toCronExpression(
+			{
+				field: 'minutes',
+				minutesInterval: 50,
+			},
+			TEST_SEED,
+		);
+		expect(result1).toEqual('56 * * * * *');
 	});
 
 	it('should return cron expression for hours interval', () => {
@@ -324,6 +336,45 @@ describe('recurrenceCheck', () => {
 		expect(result).toBe(false);
 	});
 
+	describe('minutes', () => {
+		it('should trigger when exactly on time', () => {
+			mockMomentTz({ epochMinute: 1000 });
+			const recurrenceRules = [950]; // 50 minutes elapsed, interval = 50
+			const result = recurrenceCheck(
+				{ activated: true, index: 0, intervalSize: 50, typeInterval: 'minutes' },
+				recurrenceRules,
+				'UTC',
+			);
+			expect(result).toBe(true);
+			expect(recurrenceRules[0]).toBe(1000);
+		});
+
+		it('should not trigger before interval has elapsed', () => {
+			mockMomentTz({ epochMinute: 1000 });
+			const recurrenceRules = [960]; // only 40 minutes elapsed, need 50
+			const result = recurrenceCheck(
+				{ activated: true, index: 0, intervalSize: 50, typeInterval: 'minutes' },
+				recurrenceRules,
+				'UTC',
+			);
+			expect(result).toBe(false);
+		});
+
+		it('should fire on the first tick after a gap longer than the hour wrap', () => {
+			mockMomentTz({ epochMinute: 1000 });
+			// 110 minutes elapsed: a wall-clock minute-of-hour gate would read this
+			// as 50 % 60 elapsed and wait again; the absolute count fires immediately.
+			const recurrenceRules = [890];
+			const result = recurrenceCheck(
+				{ activated: true, index: 0, intervalSize: 50, typeInterval: 'minutes' },
+				recurrenceRules,
+				'UTC',
+			);
+			expect(result).toBe(true);
+			expect(recurrenceRules[0]).toBe(1000);
+		});
+	});
+
 	describe('hours', () => {
 		it('should trigger when exactly on time', () => {
 			mockMomentTz({ hour: 5 });
@@ -660,6 +711,32 @@ describe('intervalToRecurrence', () => {
 			1,
 		);
 		expect(result.activated).toBe(false);
+
+		// Non-dividing interval needs the recurrence gate.
+		const result1 = intervalToRecurrence(
+			{
+				field: 'minutes',
+				minutesInterval: 50,
+			},
+			1,
+		);
+		expect(result1).toEqual({
+			activated: true,
+			index: 1,
+			intervalSize: 50,
+			typeInterval: 'minutes',
+		});
+	});
+
+	it('should not activate recurrence for a non-positive minutes interval (pre-1.3 nodes skip validation)', () => {
+		const result = intervalToRecurrence(
+			{
+				field: 'minutes',
+				minutesInterval: 0,
+			},
+			1,
+		);
+		expect(result.activated).toBe(false);
 	});
 
 	it('should return recurrence rule for hours interval', () => {

packages/nodes-base/nodes/Schedule/test/ScheduleTrigger.node.test.ts

@@ -5,6 +5,7 @@ import { testTriggerNode } from '@test/nodes/TriggerHelpers';
 import { ScheduleTrigger } from '../ScheduleTrigger.node';
 
 describe('ScheduleTrigger', () => {
+	const MINUTE = 60 * 1000;
 	const HOUR = 60 * 60 * 1000;
 	const mockDate = new Date('2023-12-28 12:34:56.789Z');
 	const timezone = 'Europe/Berlin';
@@ -102,6 +103,29 @@ describe('ScheduleTrigger', () => {
 			expect(emit).toHaveBeenCalledTimes(3);
 		});
 
+		it('should emit every 50 elapsed minutes instead of clock-aligned minute slots', async () => {
+			vi.setSystemTime(new Date('2023-12-28 12:49:56.789Z'));
+
+			const { emit } = await testTriggerNode(ScheduleTrigger, {
+				timezone,
+				node: { parameters: { rule: { interval: [{ field: 'minutes', minutesInterval: 50 }] } } },
+				workflowStaticData: { recurrenceRules: [] },
+			});
+
+			expect(emit).not.toHaveBeenCalled();
+
+			// First fire at 12:50 — with `*/50` cron this would also fire at 13:00,
+			// only 10 minutes later.
+			vi.advanceTimersByTime(MINUTE);
+			expect(emit).toHaveBeenCalledTimes(1);
+
+			vi.advanceTimersByTime(10 * MINUTE);
+			expect(emit).toHaveBeenCalledTimes(1);
+
+			vi.advanceTimersByTime(40 * MINUTE);
+			expect(emit).toHaveBeenCalledTimes(2);
+		});
+
 		it('should emit every 18 hours when triggerAtMinute is explicitly set to 0', async () => {
 			const { emit } = await testTriggerNode(ScheduleTrigger, {
 				timezone,

packages/workflow/src/interfaces.ts

@@ -1032,7 +1032,7 @@ type CronRecurrenceRule =
 			activated: true;
 			index: number;
 			intervalSize: number;
-			typeInterval: 'hours' | 'days' | 'weeks' | 'months';
+			typeInterval: 'minutes' | 'hours' | 'days' | 'weeks' | 'months';
 	  };
 
 /**