From a0e30539fe39571975bdc7a1f115d08d63d84c87 Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Fri, 5 Jun 2026 19:02:10 +0300 Subject: [PATCH 1/3] Implement business window check --- README.md | 28 +++++- action.yml | 8 ++ dist/index.js | 208 +++++++++++++++++++++++++++++++++++++++++++- src/action.js | 18 ++++ src/util.js | 3 + test/action.test.js | 118 +++++++++++++++++++++++++ test/util.test.js | 15 ++++ 7 files changed, 396 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 664e315..c3c7f7a 100644 --- a/README.md +++ b/README.md @@ -35,12 +35,14 @@ Error: Resource not accessible by integration | `pr-number` | No | | A pull request number, only required if triggered from a workflow_dispatch event. Typically this would be triggered by a script running in a separate CI provider. See [Trigger action from workflow_dispatch event](#trigger-action-from-workflow_dispatch-event) example. | | `skip-commit-verification` | No | `false` | If `true`, then the action will not expect the commits to have a verification signature. It is required to set this to `true` in GitHub Enterprise Server. | | `skip-verification` | No | `false` | If true, the action will not validate the user or the commit verification status | +| `merge-window` | No | | A 5-field [cron expression](https://en.wikipedia.org/wiki/Cron) (e.g. `0 9-16 * * 1-5`) describing when merges are allowed. When set, PRs evaluated outside this window are skipped instead of merged. Supports `*`, ranges (`a-b`), lists (`a,b`), and steps (`*/n`). The day-of-week field accepts `0`-`7` (both `0` and `7` mean Sunday). See [Restricting merges to business hours](#restricting-merges-to-business-hours). | +| `merge-window-timezone` | No | `UTC` | The [IANA timezone](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) (e.g. `Europe/London`) used to evaluate `merge-window`. Defaults to `UTC`. | ## Output | outputs | Description | | ------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| merge_status | The result status of the merge. It can be one of the following: `approved`, `merged`, `auto_merge`, `merge_failed`, `skipped:commit_verification_failed`, `skipped:not_a_dependabot_pr`, `skipped:cannot_update_major`, `skipped:bump_higher_than_target`, `skipped:packaged_excluded` | +| merge_status | The result status of the merge. It can be one of the following: `approved`, `merged`, `auto_merge`, `merge_failed`, `skipped:commit_verification_failed`, `skipped:not_a_dependabot_pr`, `skipped:cannot_update_major`, `skipped:bump_higher_than_target`, `skipped:packaged_excluded`, `skipped:outside_merge_window` | ## Examples @@ -117,6 +119,30 @@ steps: target-production: 'minor' ``` +### Restricting merges to business hours + +Use `merge-window` to only auto-merge during a time window, for example to avoid +deployments while everyone is asleep. The window is a standard 5-field cron +expression and is evaluated in `merge-window-timezone` (UTC by default). PRs +evaluated outside the window are skipped (`merge_status: skipped:outside_merge_window`) +rather than merged. + +```yml +steps: + - uses: fastify/github-action-merge-dependabot@v3 + with: + # Allow merges Monday-Friday, 09:00-16:59 London time + merge-window: '* 9-16 * * 1-5' + merge-window-timezone: 'Europe/London' +``` + +Note that the `pull_request` event only fires when a PR is opened or updated, so a +PR opened outside the window stays unmerged until something triggers the action +again. To re-evaluate skipped PRs on a schedule, also run the action from a +[`schedule`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#schedule) +trigger together with the [`workflow_dispatch` approach](#trigger-action-from-workflow_dispatch-event) +described below. + ### Trigger action from workflow_dispatch event If you need to trigger this action manually, you can use the [`workflow_dispatch`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_dispatch) event. A use case might be that your CI runs on a seperate provider, so you would like to run this action as a result of a successful CI run. diff --git a/action.yml b/action.yml index 160e979..9002555 100644 --- a/action.yml +++ b/action.yml @@ -49,6 +49,14 @@ inputs: type: boolean description: 'If true, the action will not validate the user or the commit verification status' default: false + merge-window: + description: 'A 5-field cron expression (e.g. "0 9-16 * * 1-5") describing when merges are allowed. PRs evaluated outside this window are skipped.' + required: false + default: '' + merge-window-timezone: + description: 'The IANA timezone (e.g. "Europe/London") used to evaluate merge-window. Defaults to UTC.' + required: false + default: '' outputs: merge_status: diff --git a/dist/index.js b/dist/index.js index cbef4d8..d08e5a4 100644 --- a/dist/index.js +++ b/dist/index.js @@ -34700,6 +34700,7 @@ const { getTarget, } = __nccwpck_require__(3286) const { verifyCommits } = __nccwpck_require__(877) +const { isWithinMergeWindow } = __nccwpck_require__(4392) const { dependabotAuthor } = __nccwpck_require__(9138) const { updateTypes } = __nccwpck_require__(2539) const { updateTypesPriority } = __nccwpck_require__(2539) @@ -34728,6 +34729,8 @@ module.exports = async function run ({ PR_NUMBER, SKIP_COMMIT_VERIFICATION, SKIP_VERIFICATION, + MERGE_WINDOW, + MERGE_WINDOW_TIMEZONE, } = getInputs(inputs) try { @@ -34822,6 +34825,21 @@ ${changedExcludedPackages.join(', ')}. Skipping.`) return } + if ( + MERGE_WINDOW && + !isWithinMergeWindow({ + mergeWindow: MERGE_WINDOW, + timezone: MERGE_WINDOW_TIMEZONE || undefined, + }) + ) { + core.setOutput(MERGE_STATUS_KEY, MERGE_STATUS.skippedOutsideMergeWindow) + return logInfo( + `Outside of the configured merge-window ('${MERGE_WINDOW}'${ + MERGE_WINDOW_TIMEZONE ? ` in ${MERGE_WINDOW_TIMEZONE}` : '' + }), skipping.` + ) + } + await client.approvePullRequest(pr.number, MERGE_COMMENT) if (APPROVE_ONLY) { core.setOutput(MERGE_STATUS_KEY, MERGE_STATUS.approved) @@ -35030,6 +35048,191 @@ module.exports = { } +/***/ }), + +/***/ 4392: +/***/ ((__unused_webpack_module, exports) => { + +"use strict"; + + +// Standard 5-field cron layout: minute hour day-of-month month day-of-week. +const CRON_FIELDS = [ + { name: 'minute', min: 0, max: 59 }, + { name: 'hour', min: 0, max: 23 }, + { name: 'dayOfMonth', min: 1, max: 31 }, + { name: 'month', min: 1, max: 12 }, + { name: 'dayOfWeek', min: 0, max: 7 }, +] + +const WEEKDAY_TO_NUMBER = { + Sun: 0, + Mon: 1, + Tue: 2, + Wed: 3, + Thu: 4, + Fri: 5, + Sat: 6, +} + +const parseInteger = (value, field) => { + if (!/^\d+$/.test(value)) { + throw new Error( + `Invalid merge-window: '${value}' is not a valid number in the ${field.name} field` + ) + } + return Number(value) +} + +const assertInRange = (value, field) => { + if (value < field.min || value > field.max) { + throw new Error( + `Invalid merge-window: ${value} is out of range (${field.min}-${field.max}) in the ${field.name} field` + ) + } +} + +// Expands a single cron field into the set of values that satisfy it. +// Supports `*`, single values, ranges (`a-b`), lists (`a,b`) and steps (`*/n`, `a-b/n`). +const parseField = (rawField, field) => { + const values = new Set() + + for (const part of rawField.split(',')) { + const [range, stepRaw] = part.split('/') + + if (stepRaw !== undefined && !/^\d+$/.test(stepRaw)) { + throw new Error( + `Invalid merge-window: '${stepRaw}' is not a valid step in the ${field.name} field` + ) + } + const step = stepRaw === undefined ? 1 : Number(stepRaw) + if (step === 0) { + throw new Error( + `Invalid merge-window: step cannot be zero in the ${field.name} field` + ) + } + + let start + let end + if (range === '*') { + start = field.min + end = field.max + } else if (range.includes('-')) { + const [startRaw, endRaw] = range.split('-') + start = parseInteger(startRaw, field) + end = parseInteger(endRaw, field) + } else { + start = parseInteger(range, field) + // A bare value combined with a step (e.g. `5/10`) means "from value to max". + end = stepRaw === undefined ? start : field.max + } + + assertInRange(start, field) + assertInRange(end, field) + if (start > end) { + throw new Error( + `Invalid merge-window: range ${start}-${end} is inverted in the ${field.name} field` + ) + } + + for (let value = start; value <= end; value += step) { + values.add(value) + } + } + + return values +} + +const parseCron = expression => { + const fields = expression.trim().split(/\s+/) + if (fields.length !== CRON_FIELDS.length) { + throw new Error( + `Invalid merge-window: expected 5 fields but got ${fields.length} in '${expression}'` + ) + } + + const schedule = {} + CRON_FIELDS.forEach((field, index) => { + const rawField = fields[index] + schedule[field.name] = { + restricted: rawField !== '*', + values: parseField(rawField, field), + } + }) + + // In cron, both 0 and 7 represent Sunday. + if (schedule.dayOfWeek.values.has(7)) { + schedule.dayOfWeek.values.add(0) + } + + return schedule +} + +// Extracts the relevant calendar fields of `date` as seen in `timezone`. +const getTimeParts = (date, timezone) => { + let parts + try { + parts = new Intl.DateTimeFormat('en-US', { + timeZone: timezone, + hour12: false, + weekday: 'short', + month: 'numeric', + day: 'numeric', + hour: 'numeric', + minute: 'numeric', + }).formatToParts(date) + } catch { + throw new Error(`Invalid merge-window-timezone: '${timezone}'`) + } + + const lookup = {} + for (const { type, value } of parts) { + lookup[type] = value + } + + // `hour` can be reported as '24' at midnight by some runtimes; normalise to 0. + const hour = Number(lookup.hour) % 24 + + return { + minute: Number(lookup.minute), + hour, + dayOfMonth: Number(lookup.day), + month: Number(lookup.month), + dayOfWeek: WEEKDAY_TO_NUMBER[lookup.weekday], + } +} + +/** + * Returns true when `now` falls inside the window described by the cron + * expression `mergeWindow`, evaluated in `timezone`. + * + * Mirrors the standard cron rule for the day fields: when both day-of-month + * and day-of-week are restricted the match is an OR, otherwise it is an AND. + */ +const isWithinMergeWindow = ({ mergeWindow, timezone = 'UTC', now = new Date() }) => { + const schedule = parseCron(mergeWindow) + const parts = getTimeParts(now, timezone) + + const minuteMatch = schedule.minute.values.has(parts.minute) + const hourMatch = schedule.hour.values.has(parts.hour) + const monthMatch = schedule.month.values.has(parts.month) + + const domMatch = schedule.dayOfMonth.values.has(parts.dayOfMonth) + const dowMatch = schedule.dayOfWeek.values.has(parts.dayOfWeek) + + const dayMatch = + schedule.dayOfMonth.restricted && schedule.dayOfWeek.restricted + ? domMatch || dowMatch + : domMatch && dowMatch + + return minuteMatch && hourMatch && monthMatch && dayMatch +} + +exports.isWithinMergeWindow = isWithinMergeWindow +exports.parseCron = parseCron +exports.getTimeParts = getTimeParts + + /***/ }), /***/ 3286: @@ -35093,6 +35296,8 @@ exports.getInputs = inputs => { PR_NUMBER: inputs['pr-number'], SKIP_COMMIT_VERIFICATION: /true/i.test(inputs['skip-commit-verification']), SKIP_VERIFICATION: /true/i.test(inputs['skip-verification']), + MERGE_WINDOW: (inputs['merge-window'] || '').trim(), + MERGE_WINDOW_TIMEZONE: (inputs['merge-window-timezone'] || '').trim(), } } @@ -35123,6 +35328,7 @@ exports.MERGE_STATUS = { skippedBumpHigherThanTarget: 'skipped:bump_higher_than_target', skippedPackageExcluded: 'skipped:packaged_excluded', skippedInvalidVersion: 'skipped:invalid_semver', + skippedOutsideMergeWindow: 'skipped:outside_merge_window', } exports.MERGE_STATUS_KEY = 'merge_status' @@ -35184,7 +35390,7 @@ module.exports = { /***/ ((module) => { "use strict"; -module.exports = /*#__PURE__*/JSON.parse('{"name":"github-action-merge-dependabot","version":"3.12.0","description":"A GitHub action to automatically merge and approve Dependabot pull requests","main":"src/index.js","type":"commonjs","scripts":{"build":"ncc build src/index.js","lint":"eslint .","lint:fix":"eslint . --fix","test":"c8 --100 node --test"},"author":{"name":"Salman Mitha","email":"SalmanMitha@gmail.com"},"contributors":["Simone Busoli "],"license":"MIT","repository":{"type":"git","url":"git+https://github.com/fastify/github-action-merge-dependabot.git"},"bugs":{"url":"https://github.com/fastify/github-action-merge-dependabot/issues"},"homepage":"https://github.com/fastify/github-action-merge-dependabot#readme","dependencies":{"@actions/core":"^1.11.1","actions-toolkit":"github:nearform/actions-toolkit"},"devDependencies":{"@vercel/ncc":"^0.38.4","c8":"^11.0.0","eslint":"^9.39.2","neostandard":"^0.13.0","proxyquire":"^2.1.3","sinon":"^21.0.3"}}'); +module.exports = /*#__PURE__*/JSON.parse('{"name":"github-action-merge-dependabot","version":"3.12.0","description":"A GitHub action to automatically merge and approve Dependabot pull requests","main":"src/index.js","type":"commonjs","scripts":{"build":"ncc build src/index.js","lint":"eslint .","lint:fix":"eslint . --fix","test":"c8 --100 node --test"},"author":{"name":"Salman Mitha","email":"SalmanMitha@gmail.com"},"contributors":["Simone Busoli "],"license":"MIT","repository":{"type":"git","url":"git+https://github.com/fastify/github-action-merge-dependabot.git"},"bugs":{"url":"https://github.com/fastify/github-action-merge-dependabot/issues"},"homepage":"https://github.com/fastify/github-action-merge-dependabot#readme","dependencies":{"@actions/core":"^1.11.1","actions-toolkit":"github:nearform/actions-toolkit"},"devDependencies":{"@vercel/ncc":"^0.38.4","c8":"^11.0.0","eslint":"^9.39.2","neostandard":"^0.13.0","proxyquire":"^2.1.3","sinon":"^21.1.2"}}'); /***/ }) diff --git a/src/action.js b/src/action.js index 673dfe9..6dd75aa 100644 --- a/src/action.js +++ b/src/action.js @@ -14,6 +14,7 @@ const { getTarget, } = require('./util') const { verifyCommits } = require('./verifyCommitSignatures') +const { isWithinMergeWindow } = require('./mergeWindow') const { dependabotAuthor } = require('./getDependabotDetails') const { updateTypes } = require('./mapUpdateType') const { updateTypesPriority } = require('./mapUpdateType') @@ -42,6 +43,8 @@ module.exports = async function run ({ PR_NUMBER, SKIP_COMMIT_VERIFICATION, SKIP_VERIFICATION, + MERGE_WINDOW, + MERGE_WINDOW_TIMEZONE, } = getInputs(inputs) try { @@ -136,6 +139,21 @@ ${changedExcludedPackages.join(', ')}. Skipping.`) return } + if ( + MERGE_WINDOW && + !isWithinMergeWindow({ + mergeWindow: MERGE_WINDOW, + timezone: MERGE_WINDOW_TIMEZONE || undefined, + }) + ) { + core.setOutput(MERGE_STATUS_KEY, MERGE_STATUS.skippedOutsideMergeWindow) + return logInfo( + `Outside of the configured merge-window ('${MERGE_WINDOW}'${ + MERGE_WINDOW_TIMEZONE ? ` in ${MERGE_WINDOW_TIMEZONE}` : '' + }), skipping.` + ) + } + await client.approvePullRequest(pr.number, MERGE_COMMENT) if (APPROVE_ONLY) { core.setOutput(MERGE_STATUS_KEY, MERGE_STATUS.approved) diff --git a/src/util.js b/src/util.js index de2d0f0..ccadaa5 100644 --- a/src/util.js +++ b/src/util.js @@ -55,6 +55,8 @@ exports.getInputs = inputs => { PR_NUMBER: inputs['pr-number'], SKIP_COMMIT_VERIFICATION: /true/i.test(inputs['skip-commit-verification']), SKIP_VERIFICATION: /true/i.test(inputs['skip-verification']), + MERGE_WINDOW: (inputs['merge-window'] || '').trim(), + MERGE_WINDOW_TIMEZONE: (inputs['merge-window-timezone'] || '').trim(), } } @@ -85,6 +87,7 @@ exports.MERGE_STATUS = { skippedBumpHigherThanTarget: 'skipped:bump_higher_than_target', skippedPackageExcluded: 'skipped:packaged_excluded', skippedInvalidVersion: 'skipped:invalid_semver', + skippedOutsideMergeWindow: 'skipped:outside_merge_window', } exports.MERGE_STATUS_KEY = 'merge_status' diff --git a/test/action.test.js b/test/action.test.js index e92fd4f..b1a4b26 100644 --- a/test/action.test.js +++ b/test/action.test.js @@ -11,6 +11,7 @@ const toolkit = require('actions-toolkit') const actionLog = require('../src/log') const actionGithubClient = require('../src/github-client') const verifyCommits = require('../src/verifyCommitSignatures') +const mergeWindow = require('../src/mergeWindow') const { updateTypes } = require('../src/mapUpdateType') const { MERGE_STATUS, MERGE_STATUS_KEY } = require('../src/util') @@ -81,12 +82,16 @@ function buildStubbedAction ({ payload, inputs, dependabotMetadata }) { }) const verifyCommitsStub = sinon.stub(verifyCommits, 'verifyCommits') + const isWithinMergeWindowStub = sinon + .stub(mergeWindow, 'isWithinMergeWindow') + .returns(true) const action = proxyquire('../src/action', { '@actions/core': coreStub, 'actions-toolkit': toolkitStub, './log': logStub, './github-client': clientStub, + './mergeWindow': { isWithinMergeWindow: isWithinMergeWindowStub }, }) return { action: props => @@ -107,6 +112,7 @@ function buildStubbedAction ({ payload, inputs, dependabotMetadata }) { enableAutoMergeStub, prCommitsStub, verifyCommitsStub, + isWithinMergeWindowStub, }, } } @@ -1094,3 +1100,115 @@ Tried to do a '${updateTypes.major}' update but the max allowed is '${updateType ) } ) + +test('should skip when outside the configured merge-window', async () => { + const PR_NUMBER = Math.random() + const { action, stubs } = buildStubbedAction({ + payload: { + pull_request: { + number: PR_NUMBER, + user: { login: BOT_NAME }, + }, + }, + inputs: { + 'pr-number': PR_NUMBER, + target: 'any', + 'merge-window': '0-59 9-16 * * 1-5', + 'merge-window-timezone': 'Europe/London', + }, + }) + + stubs.isWithinMergeWindowStub.returns(false) + + await action() + + sinon.assert.calledWithExactly(stubs.isWithinMergeWindowStub, { + mergeWindow: '0-59 9-16 * * 1-5', + timezone: 'Europe/London', + }) + sinon.assert.calledWithExactly( + stubs.logStub.logInfo, + "Outside of the configured merge-window ('0-59 9-16 * * 1-5' in Europe/London), skipping." + ) + sinon.assert.notCalled(stubs.approveStub) + sinon.assert.notCalled(stubs.mergeStub) + sinon.assert.calledWith( + stubs.coreStub.setOutput, + MERGE_STATUS_KEY, + MERGE_STATUS.skippedOutsideMergeWindow + ) +}) + +test( + 'should skip when outside the merge-window without a timezone', + async () => { + const PR_NUMBER = Math.random() + const { action, stubs } = buildStubbedAction({ + payload: { + pull_request: { + number: PR_NUMBER, + user: { login: BOT_NAME }, + }, + }, + inputs: { + 'pr-number': PR_NUMBER, + target: 'any', + 'merge-window': '0-59 9-16 * * 1-5', + }, + }) + + stubs.isWithinMergeWindowStub.returns(false) + + await action() + + sinon.assert.calledWithExactly(stubs.isWithinMergeWindowStub, { + mergeWindow: '0-59 9-16 * * 1-5', + timezone: undefined, + }) + sinon.assert.calledWithExactly( + stubs.logStub.logInfo, + "Outside of the configured merge-window ('0-59 9-16 * * 1-5'), skipping." + ) + sinon.assert.notCalled(stubs.approveStub) + sinon.assert.notCalled(stubs.mergeStub) + sinon.assert.calledWith( + stubs.coreStub.setOutput, + MERGE_STATUS_KEY, + MERGE_STATUS.skippedOutsideMergeWindow + ) + } +) + +test('should merge when inside the configured merge-window', async () => { + const PR_NUMBER = Math.random() + const { action, stubs } = buildStubbedAction({ + payload: { + pull_request: { + number: PR_NUMBER, + user: { login: BOT_NAME }, + }, + }, + inputs: { + 'pr-number': PR_NUMBER, + target: 'any', + 'merge-window': '0-59 9-16 * * 1-5', + }, + }) + + stubs.isWithinMergeWindowStub.returns(true) + + await action() + + sinon.assert.calledOnce(stubs.isWithinMergeWindowStub) + sinon.assert.calledWithExactly( + stubs.logStub.logInfo, + 'Dependabot merge completed' + ) + sinon.assert.calledOnce(stubs.approveStub) + sinon.assert.calledOnce(stubs.mergeStub) + sinon.assert.calledWith( + stubs.coreStub.setOutput, + MERGE_STATUS_KEY, + MERGE_STATUS.merged + ) +}) diff --git a/test/util.test.js b/test/util.test.js index 5d273ec..f14f176 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -120,6 +120,21 @@ test('getInputs', async t => { await t.test('PR_NUMBER', async t => { t.assert.deepEqual(getInputs({ 'pr-number': '10' }).PR_NUMBER, '10') }) + await t.test('MERGE_WINDOW', async t => { + t.assert.deepEqual(getInputs({}).MERGE_WINDOW, '') + t.assert.deepEqual( + getInputs({ 'merge-window': ' 0 9-16 * * 1-5 ' }).MERGE_WINDOW, + '0 9-16 * * 1-5' + ) + }) + await t.test('MERGE_WINDOW_TIMEZONE', async t => { + t.assert.deepEqual(getInputs({}).MERGE_WINDOW_TIMEZONE, '') + t.assert.deepEqual( + getInputs({ 'merge-window-timezone': ' Europe/London ' }) + .MERGE_WINDOW_TIMEZONE, + 'Europe/London' + ) + }) } ) }) From 567d5fba9f9dadcd3b4bb9fa25391a3e95d63d3c Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Mon, 8 Jun 2026 17:38:07 +0300 Subject: [PATCH 2/3] Implement business window check --- src/mergeWindow.js | 177 +++++++++++++++++++++++++++ test/mergeWindow.test.js | 257 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 434 insertions(+) create mode 100644 src/mergeWindow.js create mode 100644 test/mergeWindow.test.js diff --git a/src/mergeWindow.js b/src/mergeWindow.js new file mode 100644 index 0000000..7b6e23a --- /dev/null +++ b/src/mergeWindow.js @@ -0,0 +1,177 @@ +'use strict' + +// Standard 5-field cron layout: minute hour day-of-month month day-of-week. +const CRON_FIELDS = [ + { name: 'minute', min: 0, max: 59 }, + { name: 'hour', min: 0, max: 23 }, + { name: 'dayOfMonth', min: 1, max: 31 }, + { name: 'month', min: 1, max: 12 }, + { name: 'dayOfWeek', min: 0, max: 7 }, +] + +const WEEKDAY_TO_NUMBER = { + Sun: 0, + Mon: 1, + Tue: 2, + Wed: 3, + Thu: 4, + Fri: 5, + Sat: 6, +} + +const parseInteger = (value, field) => { + if (!/^\d+$/.test(value)) { + throw new Error( + `Invalid merge-window: '${value}' is not a valid number in the ${field.name} field` + ) + } + return Number(value) +} + +const assertInRange = (value, field) => { + if (value < field.min || value > field.max) { + throw new Error( + `Invalid merge-window: ${value} is out of range (${field.min}-${field.max}) in the ${field.name} field` + ) + } +} + +// Expands a single cron field into the set of values that satisfy it. +// Supports `*`, single values, ranges (`a-b`), lists (`a,b`) and steps (`*/n`, `a-b/n`). +const parseField = (rawField, field) => { + const values = new Set() + + for (const part of rawField.split(',')) { + const [range, stepRaw] = part.split('/') + + if (stepRaw !== undefined && !/^\d+$/.test(stepRaw)) { + throw new Error( + `Invalid merge-window: '${stepRaw}' is not a valid step in the ${field.name} field` + ) + } + const step = stepRaw === undefined ? 1 : Number(stepRaw) + if (step === 0) { + throw new Error( + `Invalid merge-window: step cannot be zero in the ${field.name} field` + ) + } + + let start + let end + if (range === '*') { + start = field.min + end = field.max + } else if (range.includes('-')) { + const [startRaw, endRaw] = range.split('-') + start = parseInteger(startRaw, field) + end = parseInteger(endRaw, field) + } else { + start = parseInteger(range, field) + // A bare value combined with a step (e.g. `5/10`) means "from value to max". + end = stepRaw === undefined ? start : field.max + } + + assertInRange(start, field) + assertInRange(end, field) + if (start > end) { + throw new Error( + `Invalid merge-window: range ${start}-${end} is inverted in the ${field.name} field` + ) + } + + for (let value = start; value <= end; value += step) { + values.add(value) + } + } + + return values +} + +const parseCron = expression => { + const fields = expression.trim().split(/\s+/) + if (fields.length !== CRON_FIELDS.length) { + throw new Error( + `Invalid merge-window: expected 5 fields but got ${fields.length} in '${expression}'` + ) + } + + const schedule = {} + CRON_FIELDS.forEach((field, index) => { + const rawField = fields[index] + schedule[field.name] = { + restricted: rawField !== '*', + values: parseField(rawField, field), + } + }) + + // In cron, both 0 and 7 represent Sunday. + if (schedule.dayOfWeek.values.has(7)) { + schedule.dayOfWeek.values.add(0) + } + + return schedule +} + +// Extracts the relevant calendar fields of `date` as seen in `timezone`. +const getTimeParts = (date, timezone) => { + let parts + try { + parts = new Intl.DateTimeFormat('en-US', { + timeZone: timezone, + hour12: false, + weekday: 'short', + month: 'numeric', + day: 'numeric', + hour: 'numeric', + minute: 'numeric', + }).formatToParts(date) + } catch { + throw new Error(`Invalid merge-window-timezone: '${timezone}'`) + } + + const lookup = {} + for (const { type, value } of parts) { + lookup[type] = value + } + + // `hour` can be reported as '24' at midnight by some runtimes; normalise to 0. + const hour = Number(lookup.hour) % 24 + + return { + minute: Number(lookup.minute), + hour, + dayOfMonth: Number(lookup.day), + month: Number(lookup.month), + dayOfWeek: WEEKDAY_TO_NUMBER[lookup.weekday], + } +} + +/** + * Returns true when `now` falls inside the window described by the cron + * expression `mergeWindow`, evaluated in `timezone`. + * + * Mirrors the standard cron rule for the day fields: when both day-of-month + * and day-of-week are restricted the match is an OR, otherwise it is an AND. + */ +const isWithinMergeWindow = ({ mergeWindow, timezone = 'UTC', now = new Date() }) => { + const schedule = parseCron(mergeWindow) + const parts = getTimeParts(now, timezone) + + const minuteMatch = schedule.minute.values.has(parts.minute) + const hourMatch = schedule.hour.values.has(parts.hour) + const monthMatch = schedule.month.values.has(parts.month) + + const domMatch = schedule.dayOfMonth.values.has(parts.dayOfMonth) + const dowMatch = schedule.dayOfWeek.values.has(parts.dayOfWeek) + + const dayMatch = + schedule.dayOfMonth.restricted && schedule.dayOfWeek.restricted + ? domMatch || dowMatch + : domMatch && dowMatch + + return minuteMatch && hourMatch && monthMatch && dayMatch +} + +exports.isWithinMergeWindow = isWithinMergeWindow +exports.parseCron = parseCron +exports.getTimeParts = getTimeParts diff --git a/test/mergeWindow.test.js b/test/mergeWindow.test.js new file mode 100644 index 0000000..c16972a --- /dev/null +++ b/test/mergeWindow.test.js @@ -0,0 +1,257 @@ +'use strict' + +const { test } = require('node:test') +const { + isWithinMergeWindow, + parseCron, + getTimeParts, +} = require('../src/mergeWindow') + +// A fixed point in time used across tests: Monday, 2024-06-03 14:30 UTC. +const MONDAY_AFTERNOON_UTC = new Date('2024-06-03T14:30:00Z') + +test('isWithinMergeWindow', async t => { + await t.test('matches when inside a business-hours window', t => { + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 9-16 * * 1-5', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + }) + + await t.test('does not match outside the hour range', t => { + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 9-12 * * 1-5', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, // 14:30 + }), + false + ) + }) + + await t.test('does not match outside the day-of-week range', t => { + const saturday = new Date('2024-06-01T14:30:00Z') + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 9-16 * * 1-5', + timezone: 'UTC', + now: saturday, + }), + false + ) + }) + + await t.test('does not match outside the minute range', t => { + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-15 9-16 * * 1-5', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, // minute 30 + }), + false + ) + }) + + await t.test('respects the configured timezone', t => { + // 14:30 UTC is inside an 11-16 window... + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 11-16 * * *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + // ...but the same instant is 23:30 in Tokyo, which is outside it. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 11-16 * * *', + timezone: 'Asia/Tokyo', + now: MONDAY_AFTERNOON_UTC, + }), + false + ) + // ...and 10:30 in New York (EDT), which is also outside it. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 11-16 * * *', + timezone: 'America/New_York', + now: MONDAY_AFTERNOON_UTC, + }), + false + ) + }) + + await t.test('defaults to UTC when no timezone is given', t => { + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0-59 9-16 * * 1-5', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + }) + + await t.test('uses the current time when now is not provided', t => { + // A window that is always open should match regardless of the current time. + t.assert.strictEqual( + isWithinMergeWindow({ mergeWindow: '* * * * *' }), + true + ) + }) + + await t.test('matches a specific minute and month', t => { + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '30 14 3 6 *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '30 14 3 7 *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + false + ) + }) + + await t.test('ORs day-of-month and day-of-week when both restricted', t => { + // Monday the 3rd: day-of-week matches even though day-of-month (15) does not. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * 15 * 1', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + // Neither the day-of-month (15) nor the day-of-week (Sunday) matches. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * 15 * 0', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + false + ) + }) + + await t.test('treats both 0 and 7 as Sunday', t => { + const sunday = new Date('2024-06-02T10:00:00Z') + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * * * 7', + timezone: 'UTC', + now: sunday, + }), + true + ) + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * * * 0', + timezone: 'UTC', + now: sunday, + }), + true + ) + }) + + await t.test('supports step values', t => { + // */15 in the minute field includes 0,15,30,45 -> 30 matches. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '*/15 * * * *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + // */20 includes 0,20,40 -> 30 does not match. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '*/20 * * * *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + false + ) + }) + + await t.test('supports a bare value with a step', t => { + // 0/15 -> 0,15,30,45 -> 30 matches. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0/15 * * * *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + }) + + await t.test('supports comma separated lists', t => { + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '0,30 14 * * *', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, + }), + true + ) + }) +}) + +test('parseCron validation', async t => { + await t.test('throws on the wrong number of fields', t => { + t.assert.throws(() => parseCron('* * *'), /expected 5 fields/) + t.assert.throws(() => parseCron('* * * * * *'), /expected 5 fields/) + }) + + await t.test('throws on out-of-range values', t => { + t.assert.throws(() => parseCron('* 24 * * *'), /out of range/) + t.assert.throws(() => parseCron('60 * * * *'), /out of range/) + }) + + await t.test('throws on non-numeric values', t => { + t.assert.throws(() => parseCron('abc * * * *'), /not a valid number/) + }) + + await t.test('throws on inverted ranges', t => { + t.assert.throws(() => parseCron('* 16-9 * * *'), /inverted/) + }) + + await t.test('throws on a zero step', t => { + t.assert.throws(() => parseCron('*/0 * * * *'), /step cannot be zero/) + }) + + await t.test('throws on a non-numeric step', t => { + t.assert.throws(() => parseCron('*/x * * * *'), /not a valid step/) + }) +}) + +test('getTimeParts', async t => { + await t.test('throws on an invalid timezone', t => { + t.assert.throws( + () => getTimeParts(MONDAY_AFTERNOON_UTC, 'Not/AZone'), + /Invalid merge-window-timezone/ + ) + }) + + await t.test('extracts the expected calendar fields', t => { + t.assert.deepStrictEqual(getTimeParts(MONDAY_AFTERNOON_UTC, 'UTC'), { + minute: 30, + hour: 14, + dayOfMonth: 3, + month: 6, + dayOfWeek: 1, + }) + }) +}) From 168b108b9c0395baafc94b9c14a049f974545dae Mon Sep 17 00:00:00 2001 From: Igor Savin Date: Tue, 9 Jun 2026 14:31:37 +0300 Subject: [PATCH 3/3] Address review comments --- src/mergeWindow.js | 33 +++++++++++++++++++++++++++++---- test/mergeWindow.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/mergeWindow.js b/src/mergeWindow.js index 7b6e23a..4d0c112 100644 --- a/src/mergeWindow.js +++ b/src/mergeWindow.js @@ -87,6 +87,17 @@ const parseField = (rawField, field) => { return values } +// True when `values` already covers every value in the field's domain, in +// which case the field places no real restriction on the schedule. +const coversFullDomain = (values, field) => { + for (let value = field.min; value <= field.max; value += 1) { + if (!values.has(value)) { + return false + } + } + return true +} + const parseCron = expression => { const fields = expression.trim().split(/\s+/) if (fields.length !== CRON_FIELDS.length) { @@ -97,17 +108,31 @@ const parseCron = expression => { const schedule = {} CRON_FIELDS.forEach((field, index) => { - const rawField = fields[index] schedule[field.name] = { - restricted: rawField !== '*', - values: parseField(rawField, field), + values: parseField(fields[index], field), } }) - // In cron, both 0 and 7 represent Sunday. + // In cron, both 0 and 7 represent Sunday, so keep the set consistent in both + // directions (this also lets `0-6` count as covering every weekday). if (schedule.dayOfWeek.values.has(7)) { schedule.dayOfWeek.values.add(0) } + if (schedule.dayOfWeek.values.has(0)) { + schedule.dayOfWeek.values.add(7) + } + + // A field only constrains the schedule when it does not match its entire + // domain. Treating expressions such as `*/1` or `0-6` (day-of-week) as + // restricted would wrongly trigger the day-of-month/day-of-week OR rule in + // isWithinMergeWindow(), so derive `restricted` from the expanded values + // rather than the raw text. + CRON_FIELDS.forEach(field => { + schedule[field.name].restricted = !coversFullDomain( + schedule[field.name].values, + field + ) + }) return schedule } diff --git a/test/mergeWindow.test.js b/test/mergeWindow.test.js index c16972a..d7c44c8 100644 --- a/test/mergeWindow.test.js +++ b/test/mergeWindow.test.js @@ -144,6 +144,40 @@ test('isWithinMergeWindow', async t => { ) }) + await t.test( + 'does not OR the day fields when a day field covers its full domain', + t => { + // `*/1` day-of-week spans every weekday, so it must not be treated as a + // restriction: only the day-of-month (15) should constrain the match. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * 15 * */1', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, // the 3rd, not the 15th + }), + false + ) + // `0-6` likewise covers every weekday and must not widen the match. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * 15 * 0-6', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, // the 3rd, not the 15th + }), + false + ) + // A full-domain day-of-month behaves the same way for day-of-week. + t.assert.strictEqual( + isWithinMergeWindow({ + mergeWindow: '* * */1 * 0', + timezone: 'UTC', + now: MONDAY_AFTERNOON_UTC, // Monday, day-of-week 1 (not 0) + }), + false + ) + } + ) + await t.test('treats both 0 and 7 as Sunday', t => { const sunday = new Date('2024-06-02T10:00:00Z') t.assert.strictEqual(