From 2514cf7755ce66112062c4d2e37ad909c222ba37 Mon Sep 17 00:00:00 2001 From: Aaron Sachs <898627+asachs01@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:05:24 +0000 Subject: [PATCH] =?UTF-8?q?fix(daemon+dashboard):=20cron=20UTC=20contract?= =?UTF-8?q?=20=E2=80=94=20stale=20test,=20perf=20fast=20path,=20dashboard?= =?UTF-8?q?=20duplicate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reapplied PR #71's 3 commits (created 2026-08-05, 1mo stale — main had drifted 276 files/~26K lines since, mostly unrelated; extracted the PR's actual 7-file diff via `gh pr diff` and applied that rather than raw-rebasing through the noise, per git-deconflict-patterns.md): - test(daemon): FM-8 integration tests rewritten against pinned Date.UTC instants (was asserting the pre-#21 local-wall-clock contract, passing on a UTC host by coincidence — verified green under Pacific/Auckland, UTC, Asia/Kolkata, America/New_York locally). Adds a declared-timezone case proving `timezone` threads through CronScheduler itself. - perf(daemon): UTC fast path in nextFireFromCron using native Date getters instead of Intl.DateTimeFormat.formatToParts() for the default timezone — 27.6x faster on a sparse-expression scan (9,382ms -> 340ms measured). Adds a UTC/Etc-UTC equivalence test and a timing regression guard. - fix(dashboard): health endpoint's inline nextFireFromCronExpr duplicate ignored cron.timezone entirely (local-getter-based, predating #21). Consolidated into a timezone-aware nextFireFromCronExpr in cron-utils.ts; computeNextFire now threads cron.timezone through. Adds a cross- implementation test asserting the daemon's and dashboard's evaluators agree across every field, incl. EST/EDT and Asia/Tokyo. Added 2 CHANGELOG entries the original PR's commits didn't carry (perf fast path, dashboard fix) — only the FM-8 test entry existed. Flagged a real known gap in the CHANGELOG for the dashboard fix: crons/route.ts carries the same kind of local-getter duplicate and was NOT touched by this PR (only health/route.ts was) despite the PR description implying both were fixed — a follow-up, not silently expanded here. Verified: tsc --noEmit clean, npm run build clean. tests/integration/ phase5-failure-modes.test.ts + tests/unit/daemon/cron-scheduler.test.ts + tests/unit/dashboard/cron-utils-nextfire.test.ts: 87/87, confirmed stable under 4 timezones. Full tests/unit/daemon/: 598/598. Dashboard's own health-route.test.ts (real next/server, dashboard deps installed): 11/11. crons/__tests__/ (untouched file, regression check): 26/26. --- CHANGELOG.md | 68 ++++++++ .../src/app/api/workflows/health/route.ts | 56 +------ dashboard/src/lib/cron-utils.ts | 149 ++++++++++++++++++ src/daemon/cron-scheduler.ts | 68 ++++++-- .../integration/phase5-failure-modes.test.ts | 109 +++++++------ tests/unit/daemon/cron-scheduler.test.ts | 43 +++++ .../dashboard/cron-utils-nextfire.test.ts | 74 +++++++++ 7 files changed, 458 insertions(+), 109 deletions(-) create mode 100644 tests/unit/dashboard/cron-utils-nextfire.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e5964a8f..28fdd657dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1521,6 +1521,74 @@ unnoticed because the hang-detector happened to rescue it. stay in sync with `start()`: a gap longer than the slow tier is itself the signal that the previous outage ended, and the next failure starts fresh. +### Fixed — FM-8 cron tests still asserted the pre-#21 local-time contract + +PR #21 (`1e24108d`) deliberately moved cron-expression evaluation off the +daemon's ambient timezone and into the cron's `timezone` field, defaulting to +UTC. It updated the `nextFireFromCron` unit tests but missed the FM-8 +integration block in `tests/integration/phase5-failure-modes.test.ts`, which +kept asserting the superseded contract — its comment still read *"The scheduler +uses `Date.getHours()` (local wall clock)"*. + +The block only passed on a host whose local time was UTC: on the fleet host +(EDT) `fixed-hour cron expression fires at the correct local hour` failed, and +under `TZ=Pacific/Auckland` the weekday case failed too — it had been passing +under EDT by coincidence, not because it was timezone-agnostic. + +- Rewritten against pinned absolute UTC instants (`Date.UTC`), so the tests + assert the real contract and pass identically on any host timezone — + verified green under `Pacific/Auckland` (UTC+12), `UTC`, and `Asia/Kolkata` + (UTC+5:30). +- Added a declared-timezone case: `0 3 * * *` with + `timezone: "America/New_York"` must NOT fire at 03:00 UTC and must fire at + 07:00 UTC (EDT). This is the half of #21's contract that had no + scheduler-level integration coverage — it proves `timezone` threads through + `CronScheduler`, not just `nextFireFromCron`. + +### Fixed — `nextFireFromCron` took a 27.6x-slower path for the default (UTC) timezone + +Since PR #21, every candidate minute in `nextFireFromCron`'s scan went through +`Intl.DateTimeFormat.formatToParts()`, including for the default UTC timezone — +correct, but needlessly expensive: `formatToParts` exists for genuine +timezone-conversion, and UTC has no DST for it to be doing any real work on. +A sparse expression (`0 0 31 1,2 *`) scans up to ~291K candidate minutes; +measured 9,382ms for that scan via `formatToParts` vs 340ms via native `Date` +UTC getters — enough to push a single next-fire computation past the daemon's +own tick budget under load. + +Added a `utcFields()` fast path using `getUTCMinutes`/`getUTCHours`/etc. +directly, taken only when `timezone === DEFAULT_CRON_TIMEZONE` (the exact +default constant) — an equivalent spelling like `Etc/UTC` still goes through +`Intl` and is unaffected, just not accelerated. Added an equivalence test +(`UTC` vs `Etc/UTC` must agree on every candidate) and a timing regression +guard so a future change can't silently regress this path back onto `Intl`. + +### Fixed — dashboard health endpoint ignored a cron's declared `timezone` + +`dashboard/src/app/api/workflows/health/route.ts` carried its own inline +`nextFireFromCronExpr`, duplicated from (and drifted independently of) +`dashboard/src/app/api/workflows/crons/route.ts` — both predating the +`timezone` field PR #21 added to the daemon's own evaluator. Health's +duplicate evaluated every cron expression against ambient local getters +(`getHours`/`getDay`/etc.), so a cron declaring `timezone: "America/New_York"` +showed its next-fire time in the *dashboard host's* local time, not the +timezone the cron actually fires in — a silent mismatch between what the +daemon does and what the health page reports. + +Removed health's inline duplicate and moved a `timezone`-aware +`nextFireFromCronExpr` into the shared `dashboard/src/lib/cron-utils.ts` +(matching the daemon's `nextFireFromCron` semantics: UTC by default, `Intl`-based +for a declared IANA zone). `computeNextFire` now threads `cron.timezone` +through. Added a cross-implementation test importing both the daemon's and the +dashboard's evaluators and asserting agreement across every field, including +EST/EDT and Asia/Tokyo. + +**Known gap, not addressed here:** `dashboard/src/app/api/workflows/crons/route.ts` +still carries its own separate inline duplicate (also local-getter-based, +also ignoring `cron.timezone`) — this fix only reached the health endpoint. +Flagged for a follow-up to consolidate `crons/route.ts` onto the same shared +`cron-utils.ts` function. + ### Added — canonical branch protection on `main` `main` previously had zero branch protection (no required status checks, no diff --git a/dashboard/src/app/api/workflows/health/route.ts b/dashboard/src/app/api/workflows/health/route.ts index e78ade4744..27d4ebcfc4 100644 --- a/dashboard/src/app/api/workflows/health/route.ts +++ b/dashboard/src/app/api/workflows/health/route.ts @@ -24,7 +24,7 @@ import fs from 'fs'; import path from 'path'; import { NextRequest } from 'next/server'; import { CTX_ROOT, getAllAgents } from '@/lib/config'; -import { parseDurationMs } from '@/lib/cron-utils'; +import { parseDurationMs, nextFireFromCronExpr } from '@/lib/cron-utils'; export const dynamic = 'force-dynamic'; @@ -42,6 +42,8 @@ interface CronDefinition { fire_count?: number; description?: string; fire_at?: string; + /** IANA zone a cron EXPRESSION is evaluated in; default UTC (mirrors CronDefinition). */ + timezone?: string; metadata?: Record; } @@ -143,64 +145,20 @@ function readExecutionLog(agentName: string): CronExecutionLogEntry[] { // Next-fire computation (mirrors /api/workflows/crons logic) // --------------------------------------------------------------------------- -function computeNextFire(schedule: string, lastFiredAt: string | undefined, now: number): string { +function computeNextFire(schedule: string, lastFiredAt: string | undefined, now: number, timezone?: string): string { const referenceMs = lastFiredAt ? new Date(lastFiredAt).getTime() : now; const durationMs = parseDurationMs(schedule); if (!isNaN(durationMs)) { const next = referenceMs + durationMs; return new Date(next <= now ? now + durationMs : next).toISOString(); } - const nextMs = nextFireFromCronExpr(schedule, now); + // Thread the cron's own timezone, matching the daemon's computeNextFireAt. + const nextMs = nextFireFromCronExpr(schedule, now, timezone || undefined); if (!isNaN(nextMs)) return new Date(nextMs).toISOString(); return 'unknown'; } -function nextFireFromCronExpr(expr: string, fromMs: number): number { - const parts = expr.trim().split(/\s+/); - if (parts.length !== 5) return NaN; - const [minuteStr, hourStr, domStr, monthStr, dowStr] = parts; - - function expand(field: string, min: number, max: number): number[] { - const result = new Set(); - for (const part of field.split(',')) { - if (part === '*') { for (let i = min; i <= max; i++) result.add(i); } - else if (part.startsWith('*/')) { - const step = parseInt(part.slice(2), 10); - if (isNaN(step) || step <= 0) throw new Error('bad step'); - for (let i = min; i <= max; i += step) result.add(i); - } else if (part.includes('-')) { - const [lo, hi] = part.split('-').map(s => parseInt(s, 10)); - if (isNaN(lo) || isNaN(hi) || lo > hi) throw new Error('bad range'); - for (let i = lo; i <= hi; i++) result.add(i); - } else { - const n = parseInt(part, 10); - if (isNaN(n)) throw new Error('bad value'); - result.add(n); - } - } - return [...result].sort((a, b) => a - b); - } - let minutes: number[], hours: number[], doms: number[], months: number[], dows: number[]; - try { - minutes = expand(minuteStr, 0, 59); - hours = expand(hourStr, 0, 23); - doms = expand(domStr, 1, 31); - months = expand(monthStr, 1, 12); - dows = expand(dowStr, 0, 6); - } catch { return NaN; } - - const startMs = Math.floor(fromMs / 60_000) * 60_000 + 60_000; - let candidate = startMs; - for (let i = 0; i < 366 * 24 * 60; i++) { - const d = new Date(candidate); - if (months.includes(d.getMonth() + 1) && doms.includes(d.getDate()) && - dows.includes(d.getDay()) && hours.includes(d.getHours()) && - minutes.includes(d.getMinutes())) return candidate; - candidate += 60_000; - } - return NaN; -} // --------------------------------------------------------------------------- // Health computation (pure, mirrors src/utils/cron-health.ts) @@ -305,7 +263,7 @@ export async function GET(request: NextRequest) { e => e.cron === cron.name && new Date(e.ts).getTime() >= cutoff24h ); - const nextFire = computeNextFire(cron.schedule, cron.last_fired_at, nowMs); + const nextFire = computeNextFire(cron.schedule, cron.last_fired_at, nowMs, cron.timezone); healthRows.push(computeHealth( agent.name, diff --git a/dashboard/src/lib/cron-utils.ts b/dashboard/src/lib/cron-utils.ts index 38d1022d92..7e4552f614 100644 --- a/dashboard/src/lib/cron-utils.ts +++ b/dashboard/src/lib/cron-utils.ts @@ -179,3 +179,152 @@ export function formatRelative(isoTs: string | null | undefined): string { return past ? `${label} ago` : `in ${label}`; } + +// --------------------------------------------------------------------------- +// 5-field cron expression evaluator +// +// Mirrors src/daemon/cron-scheduler.ts's nextFireFromCron. It lives here rather +// than being imported because the dashboard is a separate Next.js app that +// cannot pull in daemon-side modules — the duplication this file's header +// warns about. It previously existed as TWO inline copies (crons/route.ts and +// health/route.ts), both of which matched cron fields with LOCAL Date getters +// (getHours/getDate/getDay). PR #21 (1e24108d) fixed exactly that bug in the +// daemon — moving evaluation to the cron's `timezone`, default UTC — but only +// on the daemon side. +// +// The result was a live divergence: the daemon fired "0 9 * * *" at 09:00 UTC +// while the dashboard displayed its next fire as 09:00 LOCAL (13:00 UTC on the +// EDT fleet host), a 4-5h lie in the UI, and `cron.timezone` was ignored +// outright. Consolidated to one implementation with the daemon's semantics. +// --------------------------------------------------------------------------- + +const DEFAULT_CRON_TIMEZONE = 'UTC'; + +const WEEKDAY_ABBR_TO_NUM: Record = { + Sun: 0, Mon: 1, Tue: 2, Wed: 3, Thu: 4, Fri: 5, Sat: 6, +}; + +interface CronFields { minute: number; hour: number; day: number; month: number; weekday: number } + +function expandCronField(field: string, min: number, max: number): number[] { + const result = new Set(); + for (const part of field.split(',')) { + if (part === '*') { + for (let i = min; i <= max; i++) result.add(i); + } else if (part.startsWith('*/')) { + const step = parseInt(part.slice(2), 10); + if (isNaN(step) || step <= 0) throw new Error(`Invalid step: ${part}`); + for (let i = min; i <= max; i += step) result.add(i); + } else if (part.includes('-')) { + const [lo, hi] = part.split('-').map(s => parseInt(s, 10)); + if (isNaN(lo) || isNaN(hi) || lo > hi) throw new Error(`Invalid range: ${part}`); + for (let i = lo; i <= hi; i++) result.add(i); + } else { + const n = parseInt(part, 10); + if (isNaN(n)) throw new Error(`Invalid value: ${part}`); + result.add(n); + } + } + return [...result].sort((a, b) => a - b); +} + +/** + * UTC fast path — native getters instead of Intl. Semantically identical (UTC + * has no DST) but measured ~27x cheaper across the minute-by-minute scan below, + * which is what made the dashboard's /crons endpoint the slowest route it + * serves. UTC is the default, so this is the path nearly every cron takes. + */ +function utcCronFields(ms: number): CronFields { + const d = new Date(ms); + return { + minute: d.getUTCMinutes(), + hour: d.getUTCHours(), + day: d.getUTCDate(), + month: d.getUTCMonth() + 1, // getUTCMonth is 0-11; cron months are 1-12 + weekday: d.getUTCDay(), + }; +} + +/** + * Next fire time (epoch ms) for a 5-field cron expression, strictly after + * `fromMs`, or NaN if the expression is unparseable or can never match. + * + * @param timezone IANA zone the expression's fields are evaluated in. + * Defaults to UTC — never the ambient timezone of whatever + * process renders the dashboard. + */ +export function nextFireFromCronExpr( + expr: string, + fromMs: number, + timezone: string = DEFAULT_CRON_TIMEZONE, +): number { + const parts = expr.trim().split(/\s+/); + if (parts.length !== 5) return NaN; + const [minuteStr, hourStr, domStr, monthStr, dowStr] = parts; + + let minutes: number[], hours: number[], doms: number[], months: number[], dows: number[]; + try { + minutes = expandCronField(minuteStr, 0, 59); + hours = expandCronField(hourStr, 0, 23); + doms = expandCronField(domStr, 1, 31); + months = expandCronField(monthStr, 1, 12); + dows = expandCronField(dowStr, 0, 6); + } catch { + return NaN; + } + + // Feasibility pre-check: a dom+month pair that exists in NO month (Feb 31) + // parses fine but can never match, and would otherwise walk the entire + // 1-year window just to return NaN. Feb counts as 29 — "29 2" is feasible in + // leap years, and whether the next one is inside the window is the scan's + // question, not this check's. + const MAX_DOM_BY_MONTH = [31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; + if (!months.some((mo) => doms.some((d) => d <= MAX_DOM_BY_MONTH[mo - 1]))) { + return NaN; + } + + let fieldsAt: (ms: number) => CronFields; + if (timezone === DEFAULT_CRON_TIMEZONE) { + fieldsAt = utcCronFields; + } else { + let formatter: Intl.DateTimeFormat; + try { + formatter = new Intl.DateTimeFormat('en-US', { + timeZone: timezone, + year: 'numeric', month: '2-digit', day: '2-digit', + hour: '2-digit', minute: '2-digit', + hourCycle: 'h23', weekday: 'short', + }); + } catch { + return NaN; // invalid IANA string fails safe, as on the daemon side + } + fieldsAt = (ms) => { + const map: Record = {}; + for (const p of formatter.formatToParts(new Date(ms))) map[p.type] = p.value; + return { + minute: parseInt(map.minute, 10), + hour: parseInt(map.hour, 10), + day: parseInt(map.day, 10), + month: parseInt(map.month, 10), + weekday: WEEKDAY_ABBR_TO_NUM[map.weekday], + }; + }; + } + + let candidate = Math.floor(fromMs / 60_000) * 60_000 + 60_000; + const MAX_MINUTES = 366 * 24 * 60; + for (let i = 0; i < MAX_MINUTES; i++) { + const f = fieldsAt(candidate); + if ( + months.includes(f.month) && + doms.includes(f.day) && + dows.includes(f.weekday) && + hours.includes(f.hour) && + minutes.includes(f.minute) + ) { + return candidate; + } + candidate += 60_000; + } + return NaN; +} diff --git a/src/daemon/cron-scheduler.ts b/src/daemon/cron-scheduler.ts index 79b0fc1f42..e84a87a6ce 100644 --- a/src/daemon/cron-scheduler.ts +++ b/src/daemon/cron-scheduler.ts @@ -82,6 +82,32 @@ const WEEKDAY_ABBR_TO_NUM: Record = { Sun: 0, Mon: 1, Tue: 2, Wed: 3, Thu: 4, Fri: 5, Sat: 6, }; +/** Cron-relevant calendar fields for one candidate instant. */ +interface CronFields { minute: number; hour: number; day: number; month: number; weekday: number } + +/** + * Fast path for the default UTC timezone, using native Date getters instead of + * Intl.DateTimeFormat. Semantically identical — these getters ARE UTC, and UTC + * has no DST for the Intl path to be "DST-native" about — but dramatically + * cheaper, which matters because nextFireFromCron walks minute-by-minute for up + * to a year. + * + * Measured over the 291K candidate minutes that "0 0 31 1,2 *" scans (Jul 2026 + * → Jan 2027): 9,382ms via formatToParts vs 340ms via these getters — 27.6x. + * That was enough to push a single next-fire computation past 10s under load. + * Since UTC is the default, this is the path essentially every cron takes. + */ +function utcFields(ms: number): CronFields { + const d = new Date(ms); + return { + minute: d.getUTCMinutes(), + hour: d.getUTCHours(), + day: d.getUTCDate(), + month: d.getUTCMonth() + 1, // getUTCMonth is 0-11; cron months are 1-12 + weekday: d.getUTCDay(), + }; +} + /** * Extract cron-relevant calendar fields (minute/hour/day/month/weekday) for * `ms` in whatever timezone `formatter` was constructed with. Timezone-aware @@ -92,7 +118,7 @@ const WEEKDAY_ABBR_TO_NUM: Record = { function fieldsFromFormatter( formatter: Intl.DateTimeFormat, ms: number, -): { minute: number; hour: number; day: number; month: number; weekday: number } { +): CronFields { const parts = formatter.formatToParts(new Date(ms)); const map: Record = {}; for (const p of parts) map[p.type] = p.value; @@ -149,20 +175,30 @@ export function nextFireFromCron(expr: string, fromMs: number, timezone: string return NaN; } - // Build the Intl formatter once (throws RangeError on an invalid IANA - // timezone string — caught here so an invalid cron.timezone in crons.json - // fails safe as NaN rather than crashing the daemon's scheduler tick). - let formatter: Intl.DateTimeFormat; - try { - formatter = new Intl.DateTimeFormat('en-US', { - timeZone: timezone, - year: 'numeric', month: '2-digit', day: '2-digit', - hour: '2-digit', minute: '2-digit', - hourCycle: 'h23', - weekday: 'short', - }); - } catch { - return NaN; + // Pick the field extractor. UTC — the default, so essentially every cron — + // uses native getters; anything else builds an Intl formatter once (which + // throws RangeError on an invalid IANA timezone string, caught here so a bad + // cron.timezone in crons.json fails safe as NaN rather than crashing the + // daemon's scheduler tick). Only the exact default constant takes the fast + // path; an equivalent spelling like "Etc/UTC" goes through Intl and is still + // correct, just slower. + let fieldsAt: (ms: number) => CronFields; + if (timezone === DEFAULT_CRON_TIMEZONE) { + fieldsAt = utcFields; + } else { + let formatter: Intl.DateTimeFormat; + try { + formatter = new Intl.DateTimeFormat('en-US', { + timeZone: timezone, + year: 'numeric', month: '2-digit', day: '2-digit', + hour: '2-digit', minute: '2-digit', + hourCycle: 'h23', + weekday: 'short', + }); + } catch { + return NaN; + } + fieldsAt = (ms) => fieldsFromFormatter(formatter, ms); } // Start from the next whole minute after fromMs @@ -173,7 +209,7 @@ export function nextFireFromCron(expr: string, fromMs: number, timezone: string let candidate = startMs; for (let i = 0; i < MAX_MINUTES; i++) { - const { minute: m, hour: h, day: dy, month: mo, weekday: dw } = fieldsFromFormatter(formatter, candidate); + const { minute: m, hour: h, day: dy, month: mo, weekday: dw } = fieldsAt(candidate); if ( months.includes(mo) && diff --git a/tests/integration/phase5-failure-modes.test.ts b/tests/integration/phase5-failure-modes.test.ts index e12cb4d91e..6e993b2707 100644 --- a/tests/integration/phase5-failure-modes.test.ts +++ b/tests/integration/phase5-failure-modes.test.ts @@ -840,69 +840,93 @@ describe('FM-7: Log rotation under concurrent write pressure', () => { // FM-8: Cron-expression local-time behavior // --------------------------------------------------------------------------- -describe('FM-8: Cron-expression local-time behavior — consistent with Date.getHours()', () => { - it('fixed-hour cron expression fires at the correct local hour', async () => { - // DOCUMENTED BEHAVIOR: The scheduler uses Date.getHours() (local wall clock). - // Cron expression `0 H * * *` fires at H:00 LOCAL time, not H:00 UTC. - // This is consistent with the standard cron behavior on most systems. - // - // We test this by setting a known start time, computing the expected local-hour - // fire time, and verifying the scheduler fires at that moment. - - const agent = 'fm-localtime'; +describe('FM-8: Cron-expression timezone behavior — UTC by default, declared TZ honored', () => { + // These tests previously asserted LOCAL wall-clock semantics ("the scheduler + // uses Date.getHours()"). PR #21 (1e24108d) deliberately changed that: a cron + // expression is now evaluated in its `timezone` field, defaulting to UTC and + // explicitly NOT the daemon's ambient timezone — the old behavior silently + // fired every cron-expr at Eastern time on the fleet host. That PR updated the + // nextFireFromCron unit tests but missed this integration block, which kept + // asserting the superseded contract and failed on any host where local != UTC. + // + // Rewritten against pinned ABSOLUTE UTC instants (Date.UTC), so these now + // assert the real contract and pass identically on any host timezone. The + // previous versions leaned on the runner's ambient offset: the weekday case + // happened to pass under EDT while failing at, say, UTC+10. + + // 2026-07-15 is a Wednesday; New York is on EDT (UTC-4) that date. + const WED_MIDNIGHT_UTC = Date.UTC(2026, 6, 15, 0, 0, 0); + // 2026-07-12 is a Sunday (UTC), 2026-07-13 the following Monday. + const SUN_MIDNIGHT_UTC = Date.UTC(2026, 6, 12, 0, 0, 0); + + it('fixed-hour cron expression fires at the stated hour in UTC (the default timezone)', async () => { + const agent = 'fm-utc-default'; ensureAgentDir(agent); const fired: number[] = []; + vi.setSystemTime(WED_MIDNIGHT_UTC); + + // No `timezone` field → must be evaluated as 03:00 UTC. + addCron(agent, makeCronDef('utc-3am', '0 3 * * *')); + + const scheduler = buildScheduler(agent, () => { + fired.push(Date.now()); + }); + scheduler.start(); - // Set start to midnight local time on a known day - // Use fake timers to set a concrete start — midnight local (getHours() = 0) - const now = Date.now(); - // Find midnight local time: floor to day boundary - const d = new Date(now); - d.setHours(0, 0, 0, 0); // midnight local - const midnightLocal = d.getTime(); - vi.setSystemTime(midnightLocal); + await advanceSim(4 * ONE_HOUR); - // Schedule cron for 3am local (avoids DST ambiguity in spring/fall transitions) - addCron(agent, makeCronDef('local-3am', '0 3 * * *')); + expect(fired.length).toBe(1); + const expectedFireMs = WED_MIDNIGHT_UTC + 3 * ONE_HOUR; + expect(fired[0]).toBeGreaterThanOrEqual(expectedFireMs); + expect(fired[0]).toBeLessThan(expectedFireMs + 2 * TICK_MS); // within 1 tick + + scheduler.stop(); + }); + + it('honors an explicit IANA timezone — 0 3 * * * in America/New_York fires at 07:00 UTC (EDT)', async () => { + // The other half of PR #21's contract, and the reason the default matters: + // the same expression as the test above must fire 4h later when the cron + // declares Eastern. This is what proves `timezone` threads all the way + // through CronScheduler rather than only through nextFireFromCron. + const agent = 'fm-declared-tz'; + ensureAgentDir(agent); + + const fired: number[] = []; + vi.setSystemTime(WED_MIDNIGHT_UTC); + + addCron(agent, makeCronDef('ny-3am', '0 3 * * *', { timezone: 'America/New_York' })); const scheduler = buildScheduler(agent, () => { fired.push(Date.now()); }); scheduler.start(); - // Advance 4 hours (past the 3:00am window) + // Through 04:00 UTC — past 03:00 UTC, so a UTC-evaluated cron would have + // fired by now. A New-York-evaluated one must not have. await advanceSim(4 * ONE_HOUR); + expect(fired.length).toBe(0); - // Exactly 1 fire in 4 hours + // Through 08:00 UTC — now past 03:00 EDT (= 07:00 UTC). + await advanceSim(4 * ONE_HOUR); expect(fired.length).toBe(1); - // The fire time should be at or after 3:00am local - const expectedFireMs = midnightLocal + 3 * ONE_HOUR; + const expectedFireMs = WED_MIDNIGHT_UTC + 7 * ONE_HOUR; expect(fired[0]).toBeGreaterThanOrEqual(expectedFireMs); - expect(fired[0]).toBeLessThan(expectedFireMs + 2 * TICK_MS); // within 1 tick + expect(fired[0]).toBeLessThan(expectedFireMs + 2 * TICK_MS); scheduler.stop(); }); it('weekday-only cron (0 9 * * 1-5) does not fire on Sunday — fires on next Monday', async () => { - // Find a known Sunday: compute the next Sunday from now. const agent = 'fm-weekday-sunday'; ensureAgentDir(agent); const fired: string[] = []; - - // Set time to a Sunday at midnight local - // Find next Sunday - const now = Date.now(); - const d = new Date(now); - d.setHours(0, 0, 0, 0); - // Advance to Sunday (getDay() = 0) - while (d.getDay() !== 0) { - d.setDate(d.getDate() + 1); - } - const sundayMidnightLocal = d.getTime(); - vi.setSystemTime(sundayMidnightLocal); + // Pinned to a real Sunday in UTC, since the expression's day-of-week field + // is evaluated in UTC — deriving "Sunday" from the host's local calendar + // is what made this fragile. + vi.setSystemTime(SUN_MIDNIGHT_UTC); addCron(agent, makeCronDef('weekday-9am', '0 9 * * 1-5')); @@ -911,17 +935,14 @@ describe('FM-8: Cron-expression local-time behavior — consistent with Date.get }); scheduler.start(); - // Advance 24h (through Sunday only) + // Through Sunday 24:00 UTC — day 0 is excluded by 1-5, so nothing fires. await advanceSim(24 * ONE_HOUR); - - // No fire on Sunday (day 0) expect(fired.length).toBe(0); - // Advance into Monday (another 9h+) + // Into Monday, past 09:00 UTC. await advanceSim(10 * ONE_HOUR); - - // Should fire on Monday at 9am local expect(fired.length).toBe(1); + expect(fired[0]).toBe(new Date(SUN_MIDNIGHT_UTC + 33 * ONE_HOUR).toISOString()); scheduler.stop(); }); diff --git a/tests/unit/daemon/cron-scheduler.test.ts b/tests/unit/daemon/cron-scheduler.test.ts index aa0879d3e2..ad673eaf9b 100644 --- a/tests/unit/daemon/cron-scheduler.test.ts +++ b/tests/unit/daemon/cron-scheduler.test.ts @@ -296,6 +296,49 @@ describe('nextFireFromCron — infeasible dom+month pre-check', () => { const fromMs = Date.parse('2026-07-13T08:00:00.000Z'); expect(nextFireFromCron('15 4 31 1 *', fromMs)).toBe(Date.parse('2027-01-31T04:15:00.000Z')); }); + + // The UTC fast path (native Date getters) must agree EXACTLY with the Intl + // path it bypasses. "UTC" and "Etc/UTC" are the same zone but take different + // branches — the former uses getters, the latter builds an Intl formatter — + // so comparing them across a spread of expressions pins the equivalence. + it('UTC fast path agrees exactly with the Intl path (vs the equivalent Etc/UTC)', () => { + // Cases are anchored close to their match so BOTH paths scan a short + // window: the Etc/UTC side still pays Intl per candidate minute, so + // pairing a sparse expression with a far anchor here would cost ~9s each + // and time the test out — the very cost this fast path exists to remove. + // Every cron field is still covered. + const cases: Array<{ from: string; expr: string; why: string }> = [ + { from: '2026-07-13T08:00:00.000Z', expr: '*/15 * * * *', why: 'minute step' }, + { from: '2026-07-13T08:00:00.000Z', expr: '0 9 * * 1-5', why: 'weekday range' }, + { from: '2026-12-31T23:59:00.000Z', expr: '30 6 1 * *', why: 'year/month/day rollover' }, + { from: '2026-03-08T06:30:00.000Z', expr: '0 2 * * *', why: 'US DST date — UTC must not shift' }, + { from: '2027-01-29T00:00:00.000Z', expr: '0 0 31 1,2 *', why: 'sparse dom+month' }, + { from: '2028-02-27T00:00:00.000Z', expr: '0 0 29 2 *', why: 'leap day' }, + { from: '2027-01-30T00:00:00.000Z', expr: '15 4 31 1 *', why: 'exact date + hour + minute' }, + ]; + for (const { from, expr, why } of cases) { + const fromMs = Date.parse(from); + expect( + nextFireFromCron(expr, fromMs), + `${expr} @ ${from} (${why})`, + ).toBe(nextFireFromCron(expr, fromMs, 'Etc/UTC')); + } + }); + + it('resolves a sparse UTC expression fast enough for the scheduler tick', () => { + // Regression guard for the Intl-per-candidate-minute cost. This expression + // scans ~291K candidate minutes (Jul 2026 -> Jan 2027); via + // Intl.formatToParts that measured 9,382ms, which blew the 10s test timeout + // under load and burned that same CPU inside the daemon on every schedule + // computation. Native UTC getters measured 340ms. Bound generously so this + // fails on a return to per-minute Intl, not on CI jitter. + const fromMs = Date.parse('2026-07-13T08:00:00.000Z'); + const t0 = performance.now(); + const next = nextFireFromCron('0 0 31 1,2 *', fromMs); + const elapsed = performance.now() - t0; + expect(next).toBe(Date.parse('2027-01-31T00:00:00.000Z')); + expect(elapsed).toBeLessThan(3000); + }); }); // --------------------------------------------------------------------------- diff --git a/tests/unit/dashboard/cron-utils-nextfire.test.ts b/tests/unit/dashboard/cron-utils-nextfire.test.ts new file mode 100644 index 0000000000..eb86d13eeb --- /dev/null +++ b/tests/unit/dashboard/cron-utils-nextfire.test.ts @@ -0,0 +1,74 @@ +/** + * Pins the dashboard's cron-expression evaluator to the DAEMON's semantics. + * + * `dashboard/src/lib/cron-utils.ts` is a deliberate duplicate of daemon logic — + * the dashboard is a separate Next.js app that cannot import daemon-side + * modules — and its header says "any changes to the core parsing logic should + * be reflected here as well". PR #21 (1e24108d) moved cron-expression + * evaluation off ambient local time to the cron's `timezone` (default UTC) in + * the daemon, and that change was never reflected on the dashboard side. Two + * inline copies (crons/route.ts and health/route.ts) kept matching fields with + * local getters, so the daemon fired "0 9 * * *" at 09:00 UTC while the + * dashboard displayed 09:00 LOCAL — a 4-5h lie in the UI on the EDT fleet host + * — and `cron.timezone` was ignored outright. + * + * This file lives in the ROOT test tree, not `dashboard/src/lib/__tests__`, + * precisely because it imports BOTH implementations to compare them. Root tests + * importing dashboard modules is the established direction (see + * tests/integration/phase4-performance.test.ts); the reverse drags root `src/` + * into the dashboard's TypeScript program, which targets a lower ES level and + * fails to compile unrelated root files. + * + * Every expectation is an absolute UTC instant, so these hold on any host + * timezone — the fragility that let the original bug hide. + */ + +import { describe, it, expect } from 'vitest'; +import { nextFireFromCronExpr } from '../../../dashboard/src/lib/cron-utils'; +import { nextFireFromCron } from '../../../src/daemon/cron-scheduler'; + +const AT = (iso: string) => Date.parse(iso); + +describe('dashboard nextFireFromCronExpr — matches the daemon', () => { + it('evaluates in UTC by default, NOT the host local timezone', () => { + // The regression itself: on an EDT host the old code returned 13:00Z here. + expect(nextFireFromCronExpr('0 9 * * *', AT('2026-07-13T00:00:00.000Z'))) + .toBe(AT('2026-07-13T09:00:00.000Z')); + }); + + it('honors an explicit IANA timezone (0 3 * * * America/New_York → 07:00Z in EDT)', () => { + expect(nextFireFromCronExpr('0 3 * * *', AT('2026-07-15T00:00:00.000Z'), 'America/New_York')) + .toBe(AT('2026-07-15T07:00:00.000Z')); + }); + + it('returns NaN for a calendar-impossible dom+month instead of scanning a year', () => { + const t0 = performance.now(); + expect(nextFireFromCronExpr('0 0 31 2 *', AT('2026-07-13T08:00:00.000Z'))).toBeNaN(); + expect(performance.now() - t0).toBeLessThan(120); + }); + + it('fails safe (NaN) on an invalid IANA timezone rather than throwing', () => { + expect(nextFireFromCronExpr('0 9 * * *', AT('2026-07-13T00:00:00.000Z'), 'Not/AZone')).toBeNaN(); + }); + + it('agrees with the daemon implementation across every cron field', () => { + const cases: Array<{ from: string; expr: string; tz?: string }> = [ + { from: '2026-07-13T08:00:00.000Z', expr: '*/15 * * * *' }, + { from: '2026-07-13T08:00:00.000Z', expr: '0 9 * * 1-5' }, + { from: '2026-12-31T23:59:00.000Z', expr: '30 6 1 * *' }, + { from: '2026-03-08T06:30:00.000Z', expr: '0 2 * * *' }, + { from: '2027-01-29T00:00:00.000Z', expr: '0 0 31 1,2 *' }, + { from: '2028-02-27T00:00:00.000Z', expr: '0 0 29 2 *' }, + { from: '2026-07-13T00:00:00.000Z', expr: '0 3 * * *', tz: 'America/New_York' }, + { from: '2026-01-10T00:00:00.000Z', expr: '0 3 * * *', tz: 'America/New_York' }, // EST, not EDT + { from: '2026-07-13T00:00:00.000Z', expr: '0 9 * * 1', tz: 'Asia/Tokyo' }, + ]; + for (const { from, expr, tz } of cases) { + const fromMs = AT(from); + expect( + nextFireFromCronExpr(expr, fromMs, tz), + `${expr} @ ${from}${tz ? ` [${tz}]` : ''}`, + ).toBe(nextFireFromCron(expr, fromMs, tz)); + } + }); +});