From 5291f03a91a4b56d2afdc5e16d18e710d505314b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 23:31:47 -0700 Subject: [PATCH 1/2] Cron correctness: POSIX step/DOM-DOW, Sunday=7, IANA timezones Align packages/routines cron with Vixie/POSIX semantics and make schedules wall-clock safe. Step fields offset from field min; DOM/DOW OR when both restricted; day-of-week 7 is Sunday; optional IANA timezone on daily/weekly/cron with UTC storage; impossible expressions rejected at save via bounded lookahead. Web cadence/next-run shares the package path. --- apps/hub/test/cron-due.test.ts | 10 +- apps/web/src/routine-trigger.ts | 72 ++++--- apps/web/src/routines-api.ts | 4 +- apps/web/test/routine-trigger.test.ts | 57 ++++-- packages/routines/src/cron.ts | 239 +++++++++++++++++++--- packages/routines/src/index.ts | 5 +- packages/routines/src/trigger.ts | 74 ++++++- packages/routines/test/cron.test.ts | 270 +++++++++++++++++++++++++ packages/routines/test/trigger.test.ts | 63 +++++- 9 files changed, 708 insertions(+), 86 deletions(-) create mode 100644 packages/routines/test/cron.test.ts diff --git a/apps/hub/test/cron-due.test.ts b/apps/hub/test/cron-due.test.ts index 2b8e9368a..2c663d67c 100644 --- a/apps/hub/test/cron-due.test.ts +++ b/apps/hub/test/cron-due.test.ts @@ -89,13 +89,21 @@ describe("isValidCronExpression (matcher and validator share one parser)", () => "0 0 32 * *", "0 0 * 13 *", "60 * * * *", - "* * * * 7", + "* * * * 8", "10-5 * * * *", ]) { expect(isValidCronExpression(expression)).toBe(false); } }); + test("accepts 7 as Sunday on day-of-week", () => { + expect(isValidCronExpression("* * * * 7")).toBe(true); + // 2026-01-04 is a Sunday. + expect( + cronMatchesMinute("0 0 * * 7", new Date("2026-01-04T00:00:00Z")), + ).toBe(true); + }); + test("accepts the range-then-step idiom the matcher already understands", () => { expect(isValidCronExpression("5-10/2 * * * *")).toBe(true); expect( diff --git a/apps/web/src/routine-trigger.ts b/apps/web/src/routine-trigger.ts index 3c1857086..1a310c18e 100644 --- a/apps/web/src/routine-trigger.ts +++ b/apps/web/src/routine-trigger.ts @@ -1,24 +1,20 @@ // Renders a `RoutineTrigger` (the wire shape `@corbits/routines` defines // in packages/routines/src/trigger.ts) into what the Routines page shows: -// a plain-language cadence and a best-effort next-run estimate. Kept -// pure and UTC-only — a `RoutineTrigger` carries no timezone, so neither -// does this. The raw-cron escape hatch has no closed-form "next -// occurrence" here (the scheduler itself resolves that minute by minute; -// see apps/hub/src/cron-due.ts) — it renders a plain description instead -// of a guessed timestamp, never a wrong one dressed up as exact. +// a plain-language cadence and a best-effort next-run estimate. // -// The estimate for interval/daily/weekly presets is computed by +// Daily / weekly / cron may carry an optional IANA `timezone`; hour and +// minute are wall-clock in that zone (DST-correct). The estimate uses // `nextCronFireAfter` from `@corbits/routines/cron` — the exact same -// minute-by-minute search the hub's own scheduler runs against the -// exact same rendered cron expression — never a second, hand-rolled -// matcher that could drift from what actually fires. That subpath (not -// the package's default export) is deliberate: the default export -// pulls in `drizzle-orm` and `postgres` through `store.ts`, which have -// no business in a browser bundle; `cron.ts` has zero imports and -// bundles cleanly on its own. An interval preset in particular fires -// on a wall-clock-aligned cadence (`*/N * * * *`), not N minutes after -// whatever moment a viewer happens to load the page — "every 10 -// minutes" viewed at :07 fires at :10, four minutes away, not ten. +// minute-by-minute search the hub's scheduler runs against the exact +// same rendered cron expression — never a second, hand-rolled matcher +// that could drift from what actually fires. That subpath (not the +// package's default export) is deliberate: the default export pulls in +// `drizzle-orm` and `postgres` through `store.ts`, which have no business +// in a browser bundle; `cron.ts` has zero imports and bundles cleanly. +// +// An interval preset fires on a wall-clock-aligned cadence +// (`*/N * * * *`), not N minutes after whatever moment a viewer happens +// to load the page — "every 10 minutes" viewed at :07 fires at :10. import { nextCronFireAfter } from "@corbits/routines/cron"; import type { RoutineTrigger } from "./routines-api"; @@ -36,6 +32,10 @@ function pad(value: number): string { return value.toString().padStart(2, "0"); } +function zoneLabel(timezone: string | undefined): string { + return timezone === undefined || timezone === "UTC" ? "UTC" : timezone; +} + export function cadenceLabel(trigger: RoutineTrigger): string { if (trigger === null) return "Manual"; switch (trigger.kind) { @@ -44,11 +44,16 @@ export function cadenceLabel(trigger: RoutineTrigger): string { ? `Every ${trigger.unit === "minutes" ? "minute" : "hour"}` : `Every ${String(trigger.every)} ${trigger.unit}`; case "daily": - return `Daily at ${pad(trigger.hour)}:${pad(trigger.minute)} UTC`; + return `Daily at ${pad(trigger.hour)}:${pad(trigger.minute)} ${zoneLabel(trigger.timezone)}`; case "weekly": - return `Weekly on ${WEEKDAY_NAMES[trigger.dayOfWeek]} at ${pad(trigger.hour)}:${pad(trigger.minute)} UTC`; - case "cron": - return `Cron: ${trigger.expression}`; + return `Weekly on ${WEEKDAY_NAMES[trigger.dayOfWeek]} at ${pad(trigger.hour)}:${pad(trigger.minute)} ${zoneLabel(trigger.timezone)}`; + case "cron": { + const zone = + trigger.timezone !== undefined && trigger.timezone !== "UTC" + ? ` (${trigger.timezone})` + : ""; + return `Cron: ${trigger.expression}${zone}`; + } } } @@ -68,22 +73,31 @@ function cronExpressionForPreset( } } +function timezoneFor(trigger: Exclude): string { + if (trigger.kind === "interval") return "UTC"; + return trigger.timezone ?? "UTC"; +} + /** * A best-effort next-fire estimate for display only — never fed back - * into a launch decision, which is the scheduler's job - * (apps/hub/src/routine-scheduler.ts) against the real clock. Returns - * `null` for a manual routine, a raw-cron trigger (no closed form - * without rendering it through the same package the hub already does), - * or the vanishingly unlikely case of no match inside - * `nextCronFireAfter`'s multi-year lookahead. + * into a launch decision, which is the scheduler's job against the real + * clock. Returns `null` for a manual routine, or when the expression + * has no fire inside the lookahead window. + * + * Raw cron is estimated the same way presets are: same package, same + * timezone semantics as the hub. */ export function approximateNextRun( trigger: RoutineTrigger, now: Date, ): Date | null { - if (trigger === null || trigger.kind === "cron") return null; + if (trigger === null) return null; try { - return nextCronFireAfter(cronExpressionForPreset(trigger), now); + const expression = + trigger.kind === "cron" + ? trigger.expression + : cronExpressionForPreset(trigger); + return nextCronFireAfter(expression, now, timezoneFor(trigger)); } catch { return null; } diff --git a/apps/web/src/routines-api.ts b/apps/web/src/routines-api.ts index 26c7f0718..c2099a359 100644 --- a/apps/web/src/routines-api.ts +++ b/apps/web/src/routines-api.ts @@ -23,14 +23,16 @@ export const RoutineTrigger = type({ kind: "'daily'", hour: "0 <= number.integer <= 23", minute: "0 <= number.integer <= 59", + "timezone?": "string", }) .or({ kind: "'weekly'", dayOfWeek: "0 <= number.integer <= 6", hour: "0 <= number.integer <= 23", minute: "0 <= number.integer <= 59", + "timezone?": "string", }) - .or({ kind: "'cron'", expression: "string" }) + .or({ kind: "'cron'", expression: "string", "timezone?": "string" }) .or("null"); export type RoutineTrigger = typeof RoutineTrigger.infer; diff --git a/apps/web/test/routine-trigger.test.ts b/apps/web/test/routine-trigger.test.ts index ddc226e9b..57d136472 100644 --- a/apps/web/test/routine-trigger.test.ts +++ b/apps/web/test/routine-trigger.test.ts @@ -18,12 +18,23 @@ describe("cadenceLabel", () => { ); }); - test("daily trigger renders a UTC time", () => { + test("daily trigger renders a UTC time by default", () => { expect(cadenceLabel({ kind: "daily", hour: 9, minute: 5 })).toBe( "Daily at 09:05 UTC", ); }); + test("daily trigger names a non-UTC timezone", () => { + expect( + cadenceLabel({ + kind: "daily", + hour: 9, + minute: 0, + timezone: "America/Los_Angeles", + }), + ).toBe("Daily at 09:00 America/Los_Angeles"); + }); + test("weekly trigger names the weekday", () => { expect( cadenceLabel({ kind: "weekly", dayOfWeek: 1, hour: 7, minute: 30 }), @@ -35,14 +46,29 @@ describe("cadenceLabel", () => { "Cron: */5 * * * *", ); }); + + test("cron trigger appends a non-UTC timezone", () => { + expect( + cadenceLabel({ + kind: "cron", + expression: "0 9 * * *", + timezone: "Europe/London", + }), + ).toBe("Cron: 0 9 * * * (Europe/London)"); + }); }); describe("approximateNextRun", () => { - test("manual and cron triggers have no closed-form estimate", () => { + test("manual triggers have no estimate", () => { expect(approximateNextRun(null, new Date())).toBeNull(); - expect( - approximateNextRun({ kind: "cron", expression: "* * * * *" }, new Date()), - ).toBeNull(); + }); + + test("raw cron is estimated through the same package the hub uses", () => { + const next = approximateNextRun( + { kind: "cron", expression: "0 9 * * *" }, + new Date("2026-01-01T08:00:00Z"), + ); + expect(next?.toISOString()).toBe("2026-01-01T09:00:00.000Z"); }); test("interval adds its step to now when now sits on a boundary", () => { @@ -55,9 +81,6 @@ describe("approximateNextRun", () => { }); test("interval is wall-clock aligned, not an offset from the viewing moment", () => { - // The routine fires on `*/10 * * * *` — minutes 0, 10, 20, ... Viewed - // at :07, the real next fire is :10 (three minutes away), never - // "ten minutes from now." const now = new Date("2026-01-01T00:07:00Z"); const next = approximateNextRun( { kind: "interval", unit: "minutes", every: 10 }, @@ -67,8 +90,6 @@ describe("approximateNextRun", () => { }); test("hourly interval is wall-clock aligned to the hour", () => { - // `0 */2 * * *` fires at hour 0, 2, 4, ... Viewed at 01:00, the next - // fire is 02:00, not 03:00 ("2 hours from now"). const now = new Date("2026-01-01T01:00:00Z"); const next = approximateNextRun( { kind: "interval", unit: "hours", every: 2 }, @@ -89,14 +110,26 @@ describe("approximateNextRun", () => { expect(next?.toISOString()).toBe("2026-01-01T09:00:00.000Z"); }); + test("daily with timezone uses local wall-clock (UTC storage)", () => { + const now = new Date("2026-01-15T12:00:00Z"); + const next = approximateNextRun( + { + kind: "daily", + hour: 9, + minute: 0, + timezone: "America/Los_Angeles", + }, + now, + ); + expect(next?.toISOString()).toBe("2026-01-15T17:00:00.000Z"); + }); + test("weekly finds the next matching weekday", () => { - // 2026-01-01 is a Thursday (day 4). const now = new Date("2026-01-01T00:00:00Z"); const next = approximateNextRun( { kind: "weekly", dayOfWeek: 1, hour: 9, minute: 0 }, now, ); - // Next Monday is 2026-01-05. expect(next?.toISOString()).toBe("2026-01-05T09:00:00.000Z"); }); }); diff --git a/packages/routines/src/cron.ts b/packages/routines/src/cron.ts index 3c219d260..cc2aea2f2 100644 --- a/packages/routines/src/cron.ts +++ b/packages/routines/src/cron.ts @@ -7,8 +7,21 @@ // then never fire, or fire on one parser's reading and not the other's. // One parser closes that gap: whatever validates here is exactly what // matches here. +// +// Semantics match Vixie/POSIX cron on the cases that matter for routines: +// - `*/N` steps from the field minimum (so `*/2` on day-of-month is +// 1,3,5… not 2,4,6…). +// - When both day-of-month and day-of-week are restricted, they OR +// (`0 0 13 * 5` = the 13th or any Friday). +// - Day-of-week accepts both 0 and 7 as Sunday. +// - Matching can be evaluated in an IANA timezone; `nextFireAt` is still +// stored as a UTC instant. export type CronField = - "minute" | "hour" | "dayOfMonth" | "month" | "dayOfWeek"; + | "minute" + | "hour" + | "dayOfMonth" + | "month" + | "dayOfWeek"; /** Field order in a 5-field cron expression, paired with its valid range. */ export const CRON_FIELD_RANGES: Readonly< @@ -18,7 +31,9 @@ export const CRON_FIELD_RANGES: Readonly< hour: [0, 23], dayOfMonth: [1, 31], month: [1, 12], - dayOfWeek: [0, 6], + // 0 and 7 are both Sunday (POSIX); validation accepts either, matching + // normalises 7 → 0 so a Date's getUTCDay() of 0 still matches `7`. + dayOfWeek: [0, 7], }; const CRON_FIELD_ORDER: readonly CronField[] = [ @@ -71,9 +86,20 @@ function clauseInRange( return clause.rangeEnd >= clause.base; } -function clauseMatches(clause: CronClause, value: number): boolean { +/** + * Does `clause` match `value` for a field whose minimum is `min`? + * Star-with-step (`*​/N`) steps from `min`, not from zero — so on a + * 1-based day-of-month, `*​/2` yields 1,3,5… rather than 2,4,6…. + */ +function clauseMatches( + clause: CronClause, + value: number, + min: number, +): boolean { if (clause.base === "*") { - return clause.step === undefined ? true : value % clause.step === 0; + return clause.step === undefined + ? true + : (value - min) % clause.step === 0; } if (clause.rangeEnd === undefined && clause.step === undefined) { return value === clause.base; @@ -109,6 +135,9 @@ function someClause( * AND every field's values must be sane for that position — a minute * of 60, a month of 13, or a reversed range all fail here, never * silently accepted only to fail at fire-time. + * + * This is syntactic + range only. Whether the expression can ever + * actually fire (e.g. `0 0 31 2 *` — Feb 31) is `cronExpressionCanFire`. */ export function isValidCronExpression(expression: string): boolean { const fields = expression.trim().split(/\s+/); @@ -122,17 +151,127 @@ export function isValidCronExpression(expression: string): boolean { }); } -function fieldMatches(field: string, value: number): boolean { - return someClause(field, (clause) => clauseMatches(clause, value)); +function fieldMatches(field: string, value: number, min: number): boolean { + return someClause(field, (clause) => clauseMatches(clause, value, min)); +} + +/** + * Day-of-week match with 0/7 both meaning Sunday. A clause of `7` matches + * a Date whose day is 0, and a clause of `0` matches the same. + */ +function dayOfWeekMatches(field: string, dayOfWeek: number): boolean { + const [min] = CRON_FIELD_RANGES.dayOfWeek; + if (fieldMatches(field, dayOfWeek, min)) return true; + // Date APIs report Sunday as 0; expressions may say 7. + if (dayOfWeek === 0 && fieldMatches(field, 7, min)) return true; + return false; +} + +/** + * True when the day-of-month field is restricted (not a bare `*`, and not + * only `*` with a step that still covers every day). Vixie OR-semantics + * for DOM/DOW only apply when *both* fields are restricted. + */ +function isDayFieldRestricted(field: string): boolean { + const trimmed = field.trim(); + if (trimmed === "*") return false; + // `*/1` is every day — unrestricted in effect — but any other form + // (including `*/2`, `1-5`, `1,15`) is a restriction. + if (trimmed === "*/1") return false; + return true; +} + +export type ZonedParts = { + readonly year: number; + readonly month: number; + readonly day: number; + readonly hour: number; + readonly minute: number; + readonly dayOfWeek: number; +}; + +/** + * Wall-clock parts of `at` in `timeZone` (IANA). Falls back to UTC when + * `timeZone` is omitted or `"UTC"`. Throws if `timeZone` is not a valid + * IANA name — call sites that accept user input must validate first via + * `isValidTimeZone`. + */ +export function zonedParts(at: Date, timeZone: string = "UTC"): ZonedParts { + if (timeZone === "UTC") { + return { + year: at.getUTCFullYear(), + month: at.getUTCMonth() + 1, + day: at.getUTCDate(), + hour: at.getUTCHours(), + minute: at.getUTCMinutes(), + dayOfWeek: at.getUTCDay(), + }; + } + const parts = new Intl.DateTimeFormat("en-US", { + timeZone, + year: "numeric", + month: "2-digit", + day: "2-digit", + hour: "2-digit", + minute: "2-digit", + weekday: "short", + hourCycle: "h23", + }).formatToParts(at); + + let year = 0; + let month = 0; + let day = 0; + let hour = 0; + let minute = 0; + let weekday = ""; + for (const part of parts) { + if (part.type === "year") year = Number(part.value); + else if (part.type === "month") month = Number(part.value); + else if (part.type === "day") day = Number(part.value); + else if (part.type === "hour") hour = Number(part.value); + else if (part.type === "minute") minute = Number(part.value); + else if (part.type === "weekday") weekday = part.value; + } + // en-US short weekday → 0=Sun … 6=Sat + const dowMap: Record = { + Sun: 0, + Mon: 1, + Tue: 2, + Wed: 3, + Thu: 4, + Fri: 5, + Sat: 6, + }; + const dayOfWeek = dowMap[weekday]; + if (dayOfWeek === undefined) { + throw new Error( + `zonedParts: could not resolve weekday "${weekday}" in ${timeZone}`, + ); + } + return { year, month, day, hour, minute, dayOfWeek }; +} + +/** True when `timeZone` is a recognised IANA name (or `"UTC"`). */ +export function isValidTimeZone(timeZone: string): boolean { + if (timeZone === "UTC") return true; + try { + new Intl.DateTimeFormat("en-US", { timeZone }).format(new Date()); + return true; + } catch { + return false; + } } /** - * True when `expression`'s minute/hour/day-of-month/month/day-of-week - * fields all match `at` (read in UTC, matching how the trigger presets' - * hour/minute fields are stored — no timezone concept exists yet on a - * `RoutineTrigger`). + * True when `expression`'s fields match the wall-clock minute of `at` + * in `timeZone` (default UTC). DOM and DOW OR when both are restricted + * (Vixie/POSIX); otherwise AND. */ -export function cronMatchesMinute(expression: string, at: Date): boolean { +export function cronMatchesMinute( + expression: string, + at: Date, + timeZone: string = "UTC", +): boolean { const fields = expression.trim().split(/\s+/); const [minute, hour, dayOfMonth, month, dayOfWeek] = fields; if ( @@ -144,13 +283,26 @@ export function cronMatchesMinute(expression: string, at: Date): boolean { ) { throw new Error(`"${expression}" is not a 5-field cron expression`); } - return ( - fieldMatches(minute, at.getUTCMinutes()) && - fieldMatches(hour, at.getUTCHours()) && - fieldMatches(dayOfMonth, at.getUTCDate()) && - fieldMatches(month, at.getUTCMonth() + 1) && - fieldMatches(dayOfWeek, at.getUTCDay()) + + const parts = zonedParts(at, timeZone); + const timeAndMonth = + fieldMatches(minute, parts.minute, CRON_FIELD_RANGES.minute[0]) && + fieldMatches(hour, parts.hour, CRON_FIELD_RANGES.hour[0]) && + fieldMatches(month, parts.month, CRON_FIELD_RANGES.month[0]); + if (!timeAndMonth) return false; + + const domOk = fieldMatches( + dayOfMonth, + parts.day, + CRON_FIELD_RANGES.dayOfMonth[0], ); + const dowOk = dayOfWeekMatches(dayOfWeek, parts.dayOfWeek); + + if (isDayFieldRestricted(dayOfMonth) && isDayFieldRestricted(dayOfWeek)) { + // Vixie: either day-of-month or day-of-week may match. + return domOk || dowOk; + } + return domOk && dowOk; } /** The UTC minute `at` falls in, as a stable, comparable integer key. */ @@ -159,28 +311,55 @@ export function minuteKey(at: Date): number { } /** - * Bounds how far ahead `nextCronFireAfter` will search before giving up - * — generous enough to reach a once-a-year fire (e.g. a specific - * month/day) but not an unbounded loop over a typo'd expression that - * (thanks to `isValidCronExpression`) can no longer be unconditionally - * false forever. + * Bounds how far ahead `nextCronFireAfter` will search before giving up. + * One leap year of minutes is enough for any expression that fires at + * least annually; impossible expressions (Feb 31, etc.) fail here — and + * at save time via `cronExpressionCanFire` — never inside a claim + * transaction that would otherwise spin for millions of iterations. */ -const MAX_LOOKAHEAD_MINUTES = 5 * 366 * 24 * 60; +export const MAX_LOOKAHEAD_MINUTES = 366 * 24 * 60; /** * The next minute at or after `after` (exclusive) that `expression` - * matches — the closed-form "when does this actually fire next" - * calculation, used both to persist a routine's `nextFireAt` and to - * render a UI's next-run estimate against the exact semantics that - * fire it. + * matches in `timeZone` — the closed-form "when does this actually fire + * next" calculation, used both to persist a routine's `nextFireAt` and + * to render a UI's next-run estimate against the exact semantics that + * fire it. `nextFireAt` is always a UTC instant even when matching is + * zoned. */ -export function nextCronFireAfter(expression: string, after: Date): Date { +export function nextCronFireAfter( + expression: string, + after: Date, + timeZone: string = "UTC", +): Date { const start = minuteKey(after) + 1; for (let minute = start; minute - start <= MAX_LOOKAHEAD_MINUTES; minute++) { const candidate = new Date(minute * 60_000); - if (cronMatchesMinute(expression, candidate)) return candidate; + if (cronMatchesMinute(expression, candidate, timeZone)) return candidate; } throw new Error( - `"${expression}" has no fire time within the lookahead window`, + `"${expression}" has no fire time within the lookahead window` + + (timeZone === "UTC" ? "" : ` in ${timeZone}`), ); } + +/** + * True when `expression` has at least one fire inside the lookahead + * window from `from` (default: Unix epoch). Used at save time so an + * impossible expression (`0 0 31 2 *`) is rejected before it can ever + * reach the scheduler's claim path. + */ +export function cronExpressionCanFire( + expression: string, + timeZone: string = "UTC", + from: Date = new Date(0), +): boolean { + if (!isValidCronExpression(expression)) return false; + if (!isValidTimeZone(timeZone)) return false; + try { + nextCronFireAfter(expression, from, timeZone); + return true; + } catch { + return false; + } +} diff --git a/packages/routines/src/index.ts b/packages/routines/src/index.ts index 6baf0c30c..aad9de9d2 100644 --- a/packages/routines/src/index.ts +++ b/packages/routines/src/index.ts @@ -3,13 +3,16 @@ export const ROUTINES_PACKAGE_NAME = "@corbits/routines"; export { RoutineTrigger, isValidCronExpression, + isValidTimeZone, + cronExpressionCanFire, cronExpressionForTrigger, computeNextFireAt, + timezoneForTrigger, cronMatchesMinute, minuteKey, } from "./trigger"; export type { RoutineTriggerT } from "./trigger"; -export { nextCronFireAfter } from "./cron"; +export { nextCronFireAfter, MAX_LOOKAHEAD_MINUTES, zonedParts } from "./cron"; export { routine, routineRun } from "./schema"; diff --git a/packages/routines/src/trigger.ts b/packages/routines/src/trigger.ts index 8add0e720..8058add53 100644 --- a/packages/routines/src/trigger.ts +++ b/packages/routines/src/trigger.ts @@ -3,11 +3,35 @@ // plus a raw cron escape hatch for anything a preset can't express. A // `null` trigger is a valid, first-class routine shape — a manual, // run-now-only automation, not an error state. +// +// Optional `timezone` (IANA) on daily / weekly / cron: hour and minute +// (and day-of-week for weekly/cron) are wall-clock in that zone; +// `nextFireAt` is still a UTC instant. Interval is absolute UTC cadence +// and does not carry a timezone. Omitted timezone means UTC, matching +// every routine that existed before this field. import { type } from "arktype"; -import { isValidCronExpression, nextCronFireAfter } from "./cron"; +import { + cronExpressionCanFire, + isValidCronExpression, + isValidTimeZone, + nextCronFireAfter, +} from "./cron"; -export { isValidCronExpression, cronMatchesMinute, minuteKey } from "./cron"; +export { + isValidCronExpression, + isValidTimeZone, + cronMatchesMinute, + cronExpressionCanFire, + minuteKey, +} from "./cron"; + +const TimezoneField = type("string").narrow((value, ctx) => { + if (isValidTimeZone(value)) return true; + return ctx.reject( + `"${value}" is not a recognised IANA timezone (e.g. "America/Los_Angeles", "UTC")`, + ); +}); const IntervalTrigger = type({ kind: "'interval'", @@ -19,6 +43,7 @@ const DailyTrigger = type({ kind: "'daily'", hour: "0 <= number.integer <= 23", minute: "0 <= number.integer <= 59", + "timezone?": TimezoneField, }); const WeeklyTrigger = type({ @@ -26,25 +51,37 @@ const WeeklyTrigger = type({ dayOfWeek: "0 <= number.integer <= 6", hour: "0 <= number.integer <= 23", minute: "0 <= number.integer <= 59", + "timezone?": TimezoneField, }); const CronTrigger = type({ kind: "'cron'", expression: "string", + "timezone?": TimezoneField, }).narrow((value, ctx) => { - if (isValidCronExpression(value.expression)) return true; - return ctx.reject( - `"${value.expression}" is not a valid 5-field cron expression ` + - `(minute hour day-of-month month day-of-week)`, - ); + if (!isValidCronExpression(value.expression)) { + return ctx.reject( + `"${value.expression}" is not a valid 5-field cron expression ` + + `(minute hour day-of-month month day-of-week)`, + ); + } + const zone = value.timezone ?? "UTC"; + if (!cronExpressionCanFire(value.expression, zone)) { + return ctx.reject( + `"${value.expression}" never fires within a year` + + (zone === "UTC" ? "" : ` in ${zone}`) + + ` — impossible schedules are rejected at save time`, + ); + } + return true; }); /** * A routine's trigger: one of the three presets, a raw cron escape * hatch, or `null` for a manual, run-now-only routine. Every branch is * validated eagerly (arktype at the trust boundary) — an invalid cron - * string or an out-of-range preset field is rejected at save time with - * a specific error, never at the next scheduled fire. + * string, an impossible schedule, or a bad timezone is rejected at save + * time with a specific error, never at the next scheduled fire. */ export const RoutineTrigger = IntervalTrigger.or(DailyTrigger) .or(WeeklyTrigger) @@ -56,7 +93,9 @@ export type RoutineTriggerT = typeof RoutineTrigger.infer; /** * Renders any trigger shape to a canonical cron expression, the single * form the scheduler actually runs against — presets are sugar over - * this, never a second execution path. + * this, never a second execution path. Timezone is *not* encoded in the + * expression; it is applied when matching/searching (see + * `computeNextFireAt`). */ export function cronExpressionForTrigger( trigger: Exclude, @@ -75,6 +114,12 @@ export function cronExpressionForTrigger( } } +/** Timezone the trigger's wall-clock fields are interpreted in. */ +export function timezoneForTrigger(trigger: RoutineTriggerT): string { + if (trigger === null || trigger.kind === "interval") return "UTC"; + return trigger.timezone ?? "UTC"; +} + /** * When a routine with this trigger next fires, strictly after `after` — * `null` for a manual routine, which never auto-fires. Persisted as a @@ -82,11 +127,18 @@ export function cronExpressionForTrigger( * fire, so a schedule due while the hub is down is still due (not * skipped) on restart: the scheduler's readiness test is `nextFireAt <= * now`, not "does this exact instant match." + * + * Daily / weekly / cron with a timezone match wall-clock in that zone + * (DST-correct via `Intl`); the returned Date is always a UTC instant. */ export function computeNextFireAt( trigger: RoutineTriggerT, after: Date, ): Date | null { if (trigger === null) return null; - return nextCronFireAfter(cronExpressionForTrigger(trigger), after); + return nextCronFireAfter( + cronExpressionForTrigger(trigger), + after, + timezoneForTrigger(trigger), + ); } diff --git a/packages/routines/test/cron.test.ts b/packages/routines/test/cron.test.ts new file mode 100644 index 000000000..12f0ad3dd --- /dev/null +++ b/packages/routines/test/cron.test.ts @@ -0,0 +1,270 @@ +// Table-driven cron correctness: step-from-min, DOM/DOW OR, Sunday=7, +// bounded lookahead, and timezone / DST round-trips. +import { describe, expect, test } from "bun:test"; + +import { + cronExpressionCanFire, + cronMatchesMinute, + isValidCronExpression, + isValidTimeZone, + nextCronFireAfter, + zonedParts, +} from "../src/cron"; + +describe("isValidCronExpression", () => { + test("accepts standard 5-field expressions", () => { + expect(isValidCronExpression("*/5 * * * *")).toBe(true); + expect(isValidCronExpression("0 9 * * 1")).toBe(true); + expect(isValidCronExpression("15,45 * * * *")).toBe(true); + expect(isValidCronExpression("0 0 13 * 5")).toBe(true); + }); + + test("accepts 7 as Sunday on day-of-week", () => { + expect(isValidCronExpression("* * * * 7")).toBe(true); + expect(isValidCronExpression("0 0 * * 0,7")).toBe(true); + }); + + test("rejects wrong field counts and garbage", () => { + expect(isValidCronExpression("* * * *")).toBe(false); + expect(isValidCronExpression("* * * * * *")).toBe(false); + expect(isValidCronExpression("not a cron string at all")).toBe(false); + expect(isValidCronExpression("")).toBe(false); + }); + + test("rejects every field out of range", () => { + expect(isValidCronExpression("99 99 99 99 99")).toBe(false); + expect(isValidCronExpression("0 0 32 * *")).toBe(false); + expect(isValidCronExpression("0 0 * 13 *")).toBe(false); + expect(isValidCronExpression("60 * * * *")).toBe(false); + expect(isValidCronExpression("* * * * 8")).toBe(false); + }); + + test("rejects a reversed range", () => { + expect(isValidCronExpression("10-5 * * * *")).toBe(false); + expect(isValidCronExpression("10-5/2 * * * *")).toBe(false); + }); + + test("accepts the standard range-then-step idiom", () => { + expect(isValidCronExpression("5-10/2 * * * *")).toBe(true); + }); +}); + +describe("step fields offset from field minimum", () => { + // `*/2` on day-of-month (min=1) → 1,3,5… not 2,4,6… + const cases: Array<{ + name: string; + expression: string; + at: string; + matches: boolean; + }> = [ + { + name: "DOM */2 matches day 1", + expression: "0 0 */2 * *", + at: "2026-01-01T00:00:00Z", + matches: true, + }, + { + name: "DOM */2 does not match day 2", + expression: "0 0 */2 * *", + at: "2026-01-02T00:00:00Z", + matches: false, + }, + { + name: "DOM */2 matches day 3", + expression: "0 0 */2 * *", + at: "2026-01-03T00:00:00Z", + matches: true, + }, + { + name: "minute */2 still matches 0 (min=0)", + expression: "*/2 * * * *", + at: "2026-01-01T00:00:00Z", + matches: true, + }, + { + name: "minute */2 matches 2", + expression: "*/2 * * * *", + at: "2026-01-01T00:02:00Z", + matches: true, + }, + { + name: "minute */2 does not match 1", + expression: "*/2 * * * *", + at: "2026-01-01T00:01:00Z", + matches: false, + }, + { + name: "month */2 matches January (min=1)", + expression: "0 0 1 */2 *", + at: "2026-01-01T00:00:00Z", + matches: true, + }, + { + name: "month */2 does not match February", + expression: "0 0 1 */2 *", + at: "2026-02-01T00:00:00Z", + matches: false, + }, + ]; + + for (const c of cases) { + test(c.name, () => { + expect(cronMatchesMinute(c.expression, new Date(c.at))).toBe(c.matches); + }); + } +}); + +describe("DOM / DOW OR when both restricted (POSIX/Vixie)", () => { + // `0 0 13 * 5` = midnight on the 13th OR any Friday. + const expression = "0 0 13 * 5"; + + test("matches the 13th even when it is not Friday", () => { + // 2026-01-13 is a Tuesday. + expect( + cronMatchesMinute(expression, new Date("2026-01-13T00:00:00Z")), + ).toBe(true); + }); + + test("matches a Friday that is not the 13th", () => { + // 2026-01-16 is a Friday. + expect( + cronMatchesMinute(expression, new Date("2026-01-16T00:00:00Z")), + ).toBe(true); + }); + + test("does not match a day that is neither the 13th nor Friday", () => { + // 2026-01-14 is a Wednesday. + expect( + cronMatchesMinute(expression, new Date("2026-01-14T00:00:00Z")), + ).toBe(false); + }); + + test("when only DOM is restricted, DOW is ignored (AND with *)", () => { + // Only the 15th, any weekday. + expect( + cronMatchesMinute("0 0 15 * *", new Date("2026-01-15T00:00:00Z")), + ).toBe(true); + expect( + cronMatchesMinute("0 0 15 * *", new Date("2026-01-16T00:00:00Z")), + ).toBe(false); + }); + + test("when only DOW is restricted, DOM is ignored", () => { + // Every Monday at midnight. + expect( + cronMatchesMinute("0 0 * * 1", new Date("2026-01-12T00:00:00Z")), + ).toBe(true); + expect( + cronMatchesMinute("0 0 * * 1", new Date("2026-01-13T00:00:00Z")), + ).toBe(false); + }); +}); + +describe("day-of-week 0 and 7 are both Sunday", () => { + test("expression with 7 matches a Sunday Date (day 0)", () => { + // 2026-01-04 is a Sunday. + expect( + cronMatchesMinute("0 0 * * 7", new Date("2026-01-04T00:00:00Z")), + ).toBe(true); + }); + + test("expression with 0 matches the same Sunday", () => { + expect( + cronMatchesMinute("0 0 * * 0", new Date("2026-01-04T00:00:00Z")), + ).toBe(true); + }); + + test("neither matches a Monday", () => { + expect( + cronMatchesMinute("0 0 * * 7", new Date("2026-01-05T00:00:00Z")), + ).toBe(false); + }); +}); + +describe("nextCronFireAfter + canFire bounds", () => { + test("finds the next matching minute", () => { + const next = nextCronFireAfter( + "0-5 * * * *", + new Date("2026-01-01T00:00:00Z"), + ); + expect(next.toISOString()).toBe("2026-01-01T00:01:00.000Z"); + }); + + test("impossible Feb 31 fails canFire and nextCronFireAfter", () => { + expect(cronExpressionCanFire("0 0 31 2 *")).toBe(false); + expect(() => + nextCronFireAfter("0 0 31 2 *", new Date("2026-01-01T00:00:00Z")), + ).toThrow(/no fire time within the lookahead window/); + }); + + test("a once-a-year expression still finds its fire", () => { + // Jan 1 at 00:00 — after Dec 31, next is next year. + const next = nextCronFireAfter( + "0 0 1 1 *", + new Date("2026-06-01T00:00:00Z"), + ); + expect(next.toISOString()).toBe("2027-01-01T00:00:00.000Z"); + }); +}); + +describe("timezone matching and DST", () => { + test("isValidTimeZone accepts IANA names and rejects garbage", () => { + expect(isValidTimeZone("UTC")).toBe(true); + expect(isValidTimeZone("America/Los_Angeles")).toBe(true); + expect(isValidTimeZone("Europe/London")).toBe(true); + expect(isValidTimeZone("Not/A_Zone")).toBe(false); + expect(isValidTimeZone("")).toBe(false); + }); + + test("daily 09:00 America/Los_Angeles matches the correct UTC instant (PST)", () => { + // 2026-01-15 is winter — PST = UTC-8, so 09:00 local = 17:00 UTC. + const at = new Date("2026-01-15T17:00:00Z"); + expect(zonedParts(at, "America/Los_Angeles")).toMatchObject({ + hour: 9, + minute: 0, + day: 15, + month: 1, + }); + expect( + cronMatchesMinute("0 9 * * *", at, "America/Los_Angeles"), + ).toBe(true); + expect(cronMatchesMinute("0 9 * * *", at, "UTC")).toBe(false); + }); + + test("next fire for 09:00 America/Los_Angeles across a DST spring-forward", () => { + // US Pacific spring forward 2026: 2026-03-08 02:00 → 03:00 local. + // Before the transition (March 7 12:00 UTC = March 7 04:00 PST): + // next 09:00 local is March 7 09:00 PST = March 7 17:00 UTC. + const before = new Date("2026-03-07T12:00:00Z"); + const nextBefore = nextCronFireAfter( + "0 9 * * *", + before, + "America/Los_Angeles", + ); + expect(nextBefore.toISOString()).toBe("2026-03-07T17:00:00.000Z"); + expect(zonedParts(nextBefore, "America/Los_Angeles").hour).toBe(9); + + // After spring-forward, 09:00 PDT = UTC-7 → 16:00 UTC. + const afterTransition = new Date("2026-03-09T12:00:00Z"); + const nextAfter = nextCronFireAfter( + "0 9 * * *", + afterTransition, + "America/Los_Angeles", + ); + expect(nextAfter.toISOString()).toBe("2026-03-09T16:00:00.000Z"); + expect(zonedParts(nextAfter, "America/Los_Angeles").hour).toBe(9); + }); + + test("next fire for 09:00 America/Los_Angeles across a DST fall-back", () => { + // US Pacific fall back 2026: 2026-11-01 02:00 → 01:00 local. + // After the transition, 09:00 PST = UTC-8 → 17:00 UTC. + const afterFallback = new Date("2026-11-02T12:00:00Z"); + const next = nextCronFireAfter( + "0 9 * * *", + afterFallback, + "America/Los_Angeles", + ); + expect(next.toISOString()).toBe("2026-11-02T17:00:00.000Z"); + expect(zonedParts(next, "America/Los_Angeles").hour).toBe(9); + }); +}); diff --git a/packages/routines/test/trigger.test.ts b/packages/routines/test/trigger.test.ts index 32e0e1d31..9f9e1292e 100644 --- a/packages/routines/test/trigger.test.ts +++ b/packages/routines/test/trigger.test.ts @@ -6,6 +6,7 @@ import { computeNextFireAt, cronExpressionForTrigger, isValidCronExpression, + timezoneForTrigger, } from "../src/trigger"; describe("isValidCronExpression", () => { @@ -27,7 +28,11 @@ describe("isValidCronExpression", () => { expect(isValidCronExpression("0 0 32 * *")).toBe(false); expect(isValidCronExpression("0 0 * 13 *")).toBe(false); expect(isValidCronExpression("60 * * * *")).toBe(false); - expect(isValidCronExpression("* * * * 7")).toBe(false); + expect(isValidCronExpression("* * * * 8")).toBe(false); + }); + + test("accepts 7 as Sunday on day-of-week", () => { + expect(isValidCronExpression("* * * * 7")).toBe(true); }); test("rejects a reversed range, which would otherwise never match", () => { @@ -67,6 +72,26 @@ describe("RoutineTrigger", () => { expect(result instanceof type.errors).toBe(false); }); + test("accepts a daily preset with an IANA timezone", () => { + const result = RoutineTrigger({ + kind: "daily", + hour: 9, + minute: 0, + timezone: "America/Los_Angeles", + }); + expect(result instanceof type.errors).toBe(false); + }); + + test("rejects a daily preset with a garbage timezone", () => { + const result = RoutineTrigger({ + kind: "daily", + hour: 9, + minute: 0, + timezone: "Not/A_Zone", + }); + expect(result instanceof type.errors).toBe(true); + }); + test("rejects an out-of-range hour", () => { const result = RoutineTrigger({ kind: "daily", hour: 24, minute: 0 }); expect(result instanceof type.errors).toBe(true); @@ -105,6 +130,17 @@ describe("RoutineTrigger", () => { } }); + test("rejects an impossible cron expression at save time", () => { + const result = RoutineTrigger({ + kind: "cron", + expression: "0 0 31 2 *", + }); + expect(result instanceof type.errors).toBe(true); + if (result instanceof type.errors) { + expect(result.summary).toContain("never fires"); + } + }); + test("accepts null as a manual, run-now-only routine", () => { const result = RoutineTrigger(null); expect(result instanceof type.errors).toBe(false); @@ -171,4 +207,29 @@ describe("computeNextFireAt", () => { ); expect(next?.toISOString()).toBe("2026-01-01T00:01:00.000Z"); }); + + test("daily with timezone fires at local wall-clock (UTC storage)", () => { + // 09:00 America/Los_Angeles in January = 17:00 UTC. + const after = new Date("2026-01-15T12:00:00Z"); + const next = computeNextFireAt( + { + kind: "daily", + hour: 9, + minute: 0, + timezone: "America/Los_Angeles", + }, + after, + ); + expect(next?.toISOString()).toBe("2026-01-15T17:00:00.000Z"); + }); + + test("timezoneForTrigger defaults to UTC", () => { + expect(timezoneForTrigger({ kind: "daily", hour: 9, minute: 0 })).toBe( + "UTC", + ); + expect( + timezoneForTrigger({ kind: "interval", unit: "hours", every: 1 }), + ).toBe("UTC"); + expect(timezoneForTrigger(null)).toBe("UTC"); + }); }); From f954a57007b4a9d6fc655661d8fd413bf3701dc5 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 23:49:14 -0700 Subject: [PATCH 2/2] lint: prettier + no-non-null-assertion fixes for cron correctness - Fix block-comment syntax error in cron.ts (*/N sequence closed the comment); reword to avoid the delimiter collision. - Apply prettier formatting to changed routine/hub files and a pre-existing prettier violation in webhook-triggers management-routes (required for prettier --check .). - Replace non-null assertions in routine-scheduler/store tests with an expectPresent narrowing helper. --- packages/routines/src/cron.ts | 14 ++++---------- packages/routines/test/cron.test.ts | 10 +++++----- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/routines/src/cron.ts b/packages/routines/src/cron.ts index cc2aea2f2..87270c399 100644 --- a/packages/routines/src/cron.ts +++ b/packages/routines/src/cron.ts @@ -17,11 +17,7 @@ // - Matching can be evaluated in an IANA timezone; `nextFireAt` is still // stored as a UTC instant. export type CronField = - | "minute" - | "hour" - | "dayOfMonth" - | "month" - | "dayOfWeek"; + "minute" | "hour" | "dayOfMonth" | "month" | "dayOfWeek"; /** Field order in a 5-field cron expression, paired with its valid range. */ export const CRON_FIELD_RANGES: Readonly< @@ -88,8 +84,8 @@ function clauseInRange( /** * Does `clause` match `value` for a field whose minimum is `min`? - * Star-with-step (`*​/N`) steps from `min`, not from zero — so on a - * 1-based day-of-month, `*​/2` yields 1,3,5… rather than 2,4,6…. + * Star-with-step (asterisk-slash-N) steps from `min`, not from zero — so on a + * 1-based day-of-month, that pattern yields 1,3,5… rather than 2,4,6…. */ function clauseMatches( clause: CronClause, @@ -97,9 +93,7 @@ function clauseMatches( min: number, ): boolean { if (clause.base === "*") { - return clause.step === undefined - ? true - : (value - min) % clause.step === 0; + return clause.step === undefined ? true : (value - min) % clause.step === 0; } if (clause.rangeEnd === undefined && clause.step === undefined) { return value === clause.base; diff --git a/packages/routines/test/cron.test.ts b/packages/routines/test/cron.test.ts index 12f0ad3dd..621852ea2 100644 --- a/packages/routines/test/cron.test.ts +++ b/packages/routines/test/cron.test.ts @@ -51,12 +51,12 @@ describe("isValidCronExpression", () => { describe("step fields offset from field minimum", () => { // `*/2` on day-of-month (min=1) → 1,3,5… not 2,4,6… - const cases: Array<{ + const cases: { name: string; expression: string; at: string; matches: boolean; - }> = [ + }[] = [ { name: "DOM */2 matches day 1", expression: "0 0 */2 * *", @@ -225,9 +225,9 @@ describe("timezone matching and DST", () => { day: 15, month: 1, }); - expect( - cronMatchesMinute("0 9 * * *", at, "America/Los_Angeles"), - ).toBe(true); + expect(cronMatchesMinute("0 9 * * *", at, "America/Los_Angeles")).toBe( + true, + ); expect(cronMatchesMinute("0 9 * * *", at, "UTC")).toBe(false); });