From 2f82cf8a26fbb84c219fdea67b29988a288390c6 Mon Sep 17 00:00:00 2001 From: Eliauk Date: Tue, 25 Aug 2026 21:34:01 +0800 Subject: [PATCH] fix(workflow): avoid extra trigger at startsOn for cron based schedule (#10410) When a static schedule workflow repeats on a cron expression, getNextTime() returned `startTime` as soon as `startsOn` was in the future, without ever consulting `repeat`. The workflow therefore fired once at `startsOn` even when that moment did not match the expression, and only followed the cron afterwards. That shortcut is correct for a numeric `repeat`, where the period is `startsOn + n * repeat` and `startsOn` is occurrence #0, so it is now limited to the non-cron cases. For a cron string the next match is computed from `startsOn` instead of "now", stepping back one second so that a `startsOn` which does match the expression still triggers. Co-authored-by: Claude Opus 5 (1M context) --- .../triggers/schedule/mode-static.test.ts | 54 +++++++++++++++++++ .../ScheduleTrigger/StaticScheduleTrigger.ts | 11 +++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/plugins/@nocobase/plugin-workflow/src/server/__tests__/triggers/schedule/mode-static.test.ts b/packages/plugins/@nocobase/plugin-workflow/src/server/__tests__/triggers/schedule/mode-static.test.ts index 711dec0ee71..daa569f8d08 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/server/__tests__/triggers/schedule/mode-static.test.ts +++ b/packages/plugins/@nocobase/plugin-workflow/src/server/__tests__/triggers/schedule/mode-static.test.ts @@ -190,6 +190,60 @@ describe('workflow > triggers > schedule > static mode', () => { expect(date.getTime()).toBe(now.getTime()); }); + it('start in future should not trigger on a time not matching cron', async () => { + await sleepToEvenSecond(); + + const base = new Date(); + base.setMilliseconds(0); + // starts 2 seconds later, while the cron only matches 4 seconds later + const start = new Date(base.getTime() + 2000); + const cronTime = new Date(base.getTime() + 4000); + + const workflow = await WorkflowRepo.create({ + values: { + enabled: true, + type: 'schedule', + config: { + mode: 0, + startsOn: start.toISOString(), + repeat: `${cronTime.getSeconds()} * * * * *`, + }, + }, + }); + + await sleep(6000); + + const executions = await workflow.getExecutions(); + expect(executions.length).toBe(1); + expect(new Date(executions[0].context.date).getTime()).toBe(cronTime.getTime()); + }); + + it('start in future should trigger when it matches cron', async () => { + await sleepToEvenSecond(); + + const base = new Date(); + base.setMilliseconds(0); + const start = new Date(base.getTime() + 2000); + + const workflow = await WorkflowRepo.create({ + values: { + enabled: true, + type: 'schedule', + config: { + mode: 0, + startsOn: start.toISOString(), + repeat: `${start.getSeconds()} * * * * *`, + }, + }, + }); + + await sleep(3000); + + const executions = await workflow.getExecutions(); + expect(executions.length).toBe(1); + expect(new Date(executions[0].context.date).getTime()).toBe(start.getTime()); + }); + it('no repeat triggered then update to repeat', async () => { const start = await sleepToEvenSecond(); diff --git a/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts b/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts index 61d8a56d58b..646bff11fed 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts +++ b/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts @@ -63,7 +63,11 @@ export default class StaticScheduleTrigger { currentDate.setMilliseconds(nextSecond ? 1000 : 0); const timestamp = currentDate.getTime(); const startTime = parseDateWithoutMs(config.startsOn); - if (startTime > timestamp) { + // NOTE: a cron expression fully defines its own trigger moments, so `startsOn` only acts as a lower bound for them. + // Returning `startTime` here would fire once at `startsOn` even when it does not match the expression. When + // `repeat` is a number the period is `startsOn + n * repeat`, which makes `startsOn` the very first occurrence, so + // that case keeps returning it. + if (startTime > timestamp && typeof config.repeat !== 'string') { return startTime; } if (config.repeat) { @@ -72,7 +76,10 @@ export default class StaticScheduleTrigger { return null; } if (typeof config.repeat === 'string') { - const interval = parser.parseExpression(config.repeat, { currentDate }); + // NOTE: `next()` is exclusive, so step back a second to keep `startsOn` itself eligible when it happens to + // match the expression. + const base = startTime > timestamp ? new Date(startTime - 1000) : currentDate; + const interval = parser.parseExpression(config.repeat, { currentDate: base }); const next = interval.next(); return next.getTime(); } else if (typeof config.repeat === 'number') {